Skip to content

Add Context to AnyCollider#665

Merged
Jondolf merged 8 commits into
avianphysics:mainfrom
NiseVoid:collider_context
Mar 20, 2025
Merged

Add Context to AnyCollider#665
Jondolf merged 8 commits into
avianphysics:mainfrom
NiseVoid:collider_context

Conversation

@NiseVoid

@NiseVoid NiseVoid commented Mar 7, 2025

Copy link
Copy Markdown
Contributor

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 <14833076+oxkitsune@users.noreply.github.com>
Co-authored-by: aecsocket <aecsocket@tutanota.com>
@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

@Jondolf Jondolf left a comment

Copy link
Copy Markdown
Member

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.

Comment thread src/collision/collider/mod.rs
Comment thread src/collision/collider/mod.rs
Comment thread src/collision/collider/mod.rs
Comment thread src/collision/collider/mod.rs Outdated
Comment thread src/collision/collider/mod.rs Outdated
@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

@Jondolf Jondolf left a comment

Copy link
Copy Markdown
Member

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
@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