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

Enable custom collider shapes #550

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Neo-Zhixing
Copy link

This PR proposes some changes to enable custom colliders in bevy_rapier.

Requires dimforge/parry#217
Requires dimforge/parry#216

@Vrixyz
Copy link
Contributor

Vrixyz commented Jul 2, 2024

Thanks ! the diff looks appealing ! More features less code 👍

I'd love an example and a changelog line to accompany this

@@ -93,6 +94,7 @@ impl<'a> fmt::Debug for ColliderView<'a> {
ColliderView::RoundConvexPolyhedron(view) => write!(f, "{:?}", view.raw),
#[cfg(feature = "dim2")]
ColliderView::RoundConvexPolygon(view) => write!(f, "{:?}", view.raw),
ColliderView::Custom(_) => write!(f, "Custom"),
Copy link
Contributor

Choose a reason for hiding this comment

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

if I'm understanding correctly, a good information to add here would by the number identifying the shape ?

maybe as_typed_shape() would be interesting ?

Suggested change
ColliderView::Custom(_) => write!(f, "Custom"),
ColliderView::Custom(shape) => write!(f, "{:?}", shape.as_typed_shape()),

Copy link
Author

@Neo-Zhixing Neo-Zhixing Jul 2, 2024

Choose a reason for hiding this comment

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

This wouldn’t be very useful because as_typed_shape would just return TypedShape::Custom which now contains a &dyn Shape instead after dimforge/parry#216.
Unfortunately, we don’t know if the custom shape implements Debug.

We’re getting rid of the number identifying the custom shape because it’s not very useful. Instead, the user should use downcasts in their custom QueryDispatcher.

The PR allowing custom QueryDispatcher is incoming.

@Neo-Zhixing
Copy link
Author

Neo-Zhixing commented Jul 2, 2024

Note that this isn’t really less code, I just think that it’s a better idea to move the scaling code to the rapier::Shape trait so that custom shapes can more easily implement its own logic about scaling.

On example and changelog: We can add the examples after I put out the PR for custom QueryDispatcher.

@Neo-Zhixing
Copy link
Author

@Vrixyz This should be ready to merge now.

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.

2 participants