Skip to content
This repository has been archived by the owner on Nov 29, 2022. It is now read-only.

PendingConvexCollision should apply to same Entity level #232

Open
agg23 opened this issue Apr 2, 2022 · 4 comments
Open

PendingConvexCollision should apply to same Entity level #232

agg23 opened this issue Apr 2, 2022 · 4 comments
Labels
enhancement New feature or improvement

Comments

@agg23
Copy link
Contributor

agg23 commented Apr 2, 2022

The current implementation of PendingConvexCollision applies the generated CollisionShape to the child containing the mesh. This is confusing and likely not what the user intended if the component was .insert()ed to some parent of the mesh. This is even more confusing when you attempt to use the collision data through Collisions or CollisionEvent, as the entity in question is not the one where the PendingConvexCollision component was added.

@Sheepyhead
Copy link

I'll add that I ran into a really obtuse issue when attempting to raycast into a PendingConvexCollision because my filter wasn't catching what I expected due to this. If it's intended, it certainly can be confusing, especially since importing meshes from glb, for example, creates some pretty steep hierarchies pretty quickly

@jcornaz
Copy link
Owner

jcornaz commented Apr 12, 2022

Thanks for the feedback. What API and behavior would you prefer instead?

@jcornaz jcornaz added the design label Apr 12, 2022
@bardt
Copy link

bardt commented May 5, 2022

I experience the same confusion. I would expect that all collision-related components would replace PendingConvexCollision on the same Entity it was set on.

Use case:

  1. I start prototyping with a simplified collision shape and set RigidBody::Dynamic, CollisionShape::Sphere { radius: 1.0 }, CollisionLayers::default(), Velocity::default() etc. on an entity A, and spawn a scene B as that entity's child.
  2. Later I replace CollisionShape::Sphere { radius: 1.0 } with PendingConvexCollision::default() to make a more precise collision shape
  3. I expect a new CollisionShape to be generated based on meshes from scene B, but added to the entity A, not B or any children of B. I expect components of A to stay exactly the same except for CollisionShape::Sphere { radius: 1.0 } replaced.

I hope this clarification helps!

@Sheepyhead
Copy link

bardt's use case is spot on, but I would like to add that the current method of adding indepentent colliders to every child mesh actually does have some useful cases, such as if you spawn a scene and just want all meshes to have independent colliders. I'd suggest keeping the current functionality as some kind of feature

@jcornaz jcornaz added enhancement New feature or improvement and removed design labels May 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or improvement
Projects
None yet
Development

No branches or pull requests

4 participants