Skip to content
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

WIP: Collisions Upgrade! #83

Merged
merged 22 commits into from
Sep 15, 2022
Merged

WIP: Collisions Upgrade! #83

merged 22 commits into from
Sep 15, 2022

Conversation

hmans
Copy link
Contributor

@hmans hmans commented Sep 10, 2022

tl;dr

Collision callbacks can now also be set on colliders:

<CuboidCollider
  onCollisionEnter={(payload) => { /* ... */ }} 
  onCollisionExit={(payload) => { /* ... */ }} 
/>

Collision callback payloads now include the collider that was involved in the collision:

<CapsuleCollider
  onCollisionEnter={({ collider }) => console.log("ENTER collider / collider", collider)}
  onCollisionExit={({ collider }) => console.log("EXIT collider / collider", collider)}
/>

Colliders can be configured with collisionGroup (and solverGroup) props. Since these expect bitmasks and creating bitmasks can be a bit cumberson, this PR also introduces the interactionGroups helper function:

<CapsuleCollider collisionGroups={interactionGroups(1, [1, 2])} />

The first argument is one or multiple group memberships, the second one or multiple groups to interact with. If the second argument is omitted, it defaults to interacting with all groups.

RigidBody can also be configured with these props. Rapier doesn't have collision/solver groups on Rigidbodies, so instead these values act as defaults for all colliders in the rigidbody:

<RigidBody collisionGroups={interactionGroups(5)}>

It's possible that the current implementation doesn't connect to all colliders. utils.ts seems to include multiple separate/duplicate code paths that create collider descriptors. This would need to be consolidated somehow, but I would like to ask for help from someone who is a little more familiar with the implementation.

Onto some more detailed notes!

Notes:

  • Allow colliders to declare their own onCollisionEnter/Exit events
  • Invoke these event callbacks when the collider enters/exits a collision
  • Pass colliding colliders back into collision events, too (not just the bodies)
  • Allow the user to configure collision groups on colliders
  • Allow the user to configure collsion groups on rigidbodies, which then act as defaults for the body's colliders if no other mask is specified
  • Allow the user to configure active event types (note to self: read up on those again, I think their semantics have changed in Rapier recently!)
    • This is already being handled through the hasCollisionsEvents argument.
  • Figure out how hasCollisionEvents should work now that colliders themselves can have collision events
  • Provide/try alternative bitmask creation API, group(...) + mask(...) (adding numbers is fun!)
  • Apparently there are multiple entirely separate paths for creating collider descriptors (createColliderFromOptions vs. colliderDescFromGeometry.) These should/must be consolidated.
  • Write a changeset, amend README (I would like to get some feedback on these changes and potentially amend them as needed before adding these.)

Other changes/additions in this PR:

  • I've used the opportunity to add a testing setup to the package (and wrote tests for some of the new bits in this PR.)
  • Exports some new types for event callbacks (helps DRY things up a little, and often useful in userland)
  • Adds an .editorconfig. I know this is not related to the change, but it contains important configuration for the new bits in the README.
  • I disabled Prettier formatting for this repo because apparently Prettier has not been used so far, but enabled VS Code's built-in TS autoformatting. It did shift some minor bits and pieces around that are not related to this change. Please let me know if you want me to clean these up, but I would generally recommend adding an .editorconfig and a Prettier configuration to the project to make things a little easier for contributors.

Further suggestions I have for changes, some of them breaking:

  • This PR adds some first tests and makes sure they are run as part of the yarn ci script. As a next step, we may add a GitHub Actions runner for this script; I would also recommend making the building of the library part of the script. We can do this in a separate PR though.
  • The library is currently not prepared for any kind of useful HMR support. The Rapier API allows many collider and rigidbody attributes to be updated after these have been spawned into the physics world, and the library should make use of that.
  • target in the event callback payload might benefit from a better name (especially now that the other collider is also part of the callback payload.) I would suggest otherBody and otherCollider.
  • I was surprised by there being a UseColliderOptions, but no useCollider (which is fine). Is this a leftover from an earlier version that had such a hook? It might be good to rename this to just ColliderOptions.

@changeset-bot
Copy link

changeset-bot bot commented Sep 10, 2022

🦋 Changeset detected

Latest commit: dc67ce2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@react-three/rapier Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Sep 10, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
react-three-rapier ✅ Ready (Inspect) Visit Preview Sep 15, 2022 at 7:59PM (UTC)

@hmans
Copy link
Contributor Author

hmans commented Sep 12, 2022

@wiledal I've made some additional changes as discussed on Discord, please take a look. I've established an applyColliderOptions functions that I'm now calling from at least two of the collider constructor functions, but it's really hard for me to tell if this now covers all code paths.

I've deliberately made this function apply the options to the collider and not a collider descriptor, as a first tiny step towards improving the support for HMR.

Let me know what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't configure collision masks and groups for colliders, and active collision types
2 participants