Skip to content

Conversation

@NiseVoid
Copy link
Contributor

@NiseVoid NiseVoid commented Mar 7, 2025

Objective

Solution

  • Reapply the changes from the previous PR
  • Add a SimpleCollider trait that works for colliders with no context to shorten function signatures

Changelog

  • Custom colliders can now specify a SystemParam via AnyCollider::Context
  • The context is passed to all AnyCollider methods, allowing data from the World to be fetched
  • AnyCollider functions have been suffixed with _with_collider
  • A new SimpleCollider trait has been added to retain the simplified methods for colliders that don't need context

Migration Guide

  • Custom colliders now need to specify a Context SystemParam, if this is unnecessary () should be used
  • When trying to use methods from AnyCollider on an implementation with () context, SimpleCollider should be used instead
  • Methods on AnyCollider have been suffixed with _with_context

Co-authored-by: Gijs de Jong <[email protected]>
Co-authored-by: aecsocket <[email protected]>
@Jondolf Jondolf added C-Feature A new feature, making something new possible A-Collision Relates to the broad phase, narrow phase, colliders, or other collision functionality labels Mar 10, 2025
Copy link
Member

@Jondolf Jondolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good, mainly a few stylistic nits and documentation suggestions.

Just to be sure, I'll test later today if this has any measurable perf impact in the default case of an empty context, but I assume it should be basically zero-cost.

@Jondolf Jondolf added this to the 0.3 milestone Mar 19, 2025
@Jondolf Jondolf added the M-Migration-Guide A breaking change to Avian's public API that needs to be noted in a migration guide label Mar 19, 2025
Copy link
Member

@Jondolf Jondolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now! Thanks for the patience on this @NiseVoid and @oxkitsune, I've kept the collider context work sitting here for too long.

I made a couple more small changes just to clean things up, and also ran a quick test to see if this has any measurable perf impact. I didn't notice anything, so it should be good to go :)

@Jondolf Jondolf enabled auto-merge (squash) March 20, 2025 16:03
@Jondolf Jondolf merged commit 2c3ba42 into avianphysics:main Mar 20, 2025
5 checks passed
@Jondolf Jondolf mentioned this pull request Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Collision Relates to the broad phase, narrow phase, colliders, or other collision functionality C-Feature A new feature, making something new possible M-Migration-Guide A breaking change to Avian's public API that needs to be noted in a migration guide

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants