heron icon indicating copy to clipboard operation
heron copied to clipboard

PendingConvexCollision should apply to same Entity level

Open agg23 opened this issue 2 years ago • 4 comments

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.

agg23 avatar Apr 02 '22 00:04 agg23

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

Sheepyhead avatar Apr 09 '22 19:04 Sheepyhead

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

jcornaz avatar Apr 12 '22 08:04 jcornaz

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!

bardt avatar May 05 '22 18:05 bardt

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

Sheepyhead avatar May 11 '22 08:05 Sheepyhead