-
Notifications
You must be signed in to change notification settings - Fork 10
Remove usingnamespace, add new interface system, rework IDs, and misc fixes #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Use non-exhaustive enums for BodyId and SubShapeId - Add activateBodies/deactivateBodies The ID change was motivated by needing to implement toJph for JPC_BodyID, which wasn't possible without making the IDs distinct types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR modernizes the zphysics library by removing deprecated usingnamespace
usage and introducing a new interface system that uses struct composition with compile-time vtable generation. The changes also rework ID types to use structs instead of plain integers and add new body activation/deactivation functions.
- Remove
usingnamespace
(deprecated in Zig master) and replace with a new interface system using struct composition - Change
JPC_BodyID
andJPC_SubShapeID
from plainuint32_t
to wrapper structs with anid
field - Add new
activateBodies
/deactivateBodies
functions and enhance debug renderer functionality
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
libs/JoltC/JoltPhysicsC_Tests.c | Update test code to use new struct-based ID format with .id field access |
libs/JoltC/JoltPhysicsC_Extensions.cpp | Modify body ID assignment to use struct field syntax |
libs/JoltC/JoltPhysicsC.h | Define new struct-based ID types and add new activation functions |
libs/JoltC/JoltPhysicsC.cpp | Implement ID conversion functions and new activation APIs |
build.zig.zon | Add minimum Zig version requirement |
build.zig | Modernize build script using newer Zig build system APIs |
#define JPC_ID_EQ(a, b) (a.id == b.id) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The macro JPC_ID_EQ assumes both parameters have an 'id' field but lacks type safety. Consider using inline functions with proper parameter types to ensure type safety and provide better error messages when used incorrectly.
#define JPC_ID_EQ(a, b) (a.id == b.id) | |
static inline bool JPC_ID_EQ(JPC_BodyID a, JPC_BodyID b) { | |
return a.id == b.id; | |
} | |
static inline bool JPC_SubShapeID_EQ(JPC_SubShapeID a, JPC_SubShapeID b) { | |
return a.id == b.id; | |
} |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated physics_test_wgpu sample app to test. Works well. Nice!
I've also updated the monolith sample app. It works great but I tripped up on something for a while... |
- Introduce a new system for interfaces: - They are defined as structs that are composed as the first member of the implementation - `initInterface` is used to build VTable default values at comptime automatically - Enhance the DebugRenderer test to verify that the render functions are called - Change the signature of DestroyTriangleBatch to pass non-const user pointer - Make all vtable functions non-optional, to improve error messages when `pub` is missing - Fix up test output when PRINT_OUTPUT is enabled - Move defines to a common build function - Add -Dverbose to set PRINT_OUTPUT
…lease enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit. - Remove the vtable header from ContactListener (it's called manually, not used as an actual vtable)
usingnamespace
has been removed in zig master, which motivated the bulk of this change.initInterface
is used to build VTable default values at comptime automaticallyinit
function which callsinitInterface
with the appropriate typesMethods
was doing before in most cases.JPC_BodyID
andJPC_SubShapeID
BodyId
andSubShapeId
activateBodies
/deactivateBodies
The ID change was motivated by needing to implement
toJph
forJPC_BodyID
, which wasn't possible without making the IDs distinct types.I spent some time working out different ways to replace the
usingnamespace
usage, and I'm pretty happy with what I landed on (it's somewhat similar to theMixin
example here: ziglang/zig#20663). If there are existing projects where this new system isn't workable, please post your use case here.Upgrade guide:
Calling Super-class Methods
Interfaces
Before:
After:
There is no longer a need to build the vtable yourself, as this is done at comptime by iterating the methods on the implementation.
You can also pass the first member of the implementation directly to the functions that take that interface, without any additional casting.
If you forget / typo the name of a non-optional vtable function, you get a helpful compile-time error: