Fix #803 child collision volumes #885
Conversation
kevzettler
changed the title
| @@ -99,6 +99,13 @@ pub fn sync_ecs_physics() -> SystemGroup { | |||
| query((translation().changed(), rotation().changed())).incl(physics_shape()); | |||
| let translation_rotation_q2 = translation_rotation_q.query.clone(); | |||
|
|
|||
| // we need a seperate query here for child entities | |||
| // Though they have rotation and translation those values may not update | |||
| // the parents will and local_to_world.changed() will need to be updated | |||
There was a problem hiding this comment.
Hm... it might make more sense to just replace the translation_rotation one, and then maybe have something that reads back changes to ltw to translation rotation.
In general we need to look over the physics system, it was built a long time ago for slightly different requirements that what's needed now
There was a problem hiding this comment.
How do you feel about merging this as-is and then fixing it up later? The child colliders have been an issue for some time (#534), so I think unblocking this and revisiting it later is a decent trade.
There was a problem hiding this comment.
I think as is it will conflict with the translation_rotation one. @kevzettler if you add .incl(local_to_parent()) on the query (line 106) it should only affect entities that are children.
Also line 107 it's cloning the wrong query.
But in general; Let's revisit this once the launch is out; I'm not sure how else this will affect things so not sure if anything will break by doing this. We need to set up more test cases for physics also.
There was a problem hiding this comment.
@FredrikNoren thanks for the code review I have made the fixes in 9af616c
I will look into adding test cases but I do not want to make a large investment if this physics implementation gets rewritten. I will work to keep this branch up todate with main because I will need this child collision behavior for my project.
There was a problem hiding this comment.
@kevzettler Great! With those fixes I think it actually can't affect anything else and it looks good to me so I'll go ahead and merge it
This PR addresses #803
This PR adds a new query to the physx system to update entities that have
physics_shapecomponents and theirlocal_to_worldchanges. This covers the case where an entity is the child of a hierarchy and has a physics collider attached. The child entities may not change itstranslationorrotationcomponents but its parents will and the child needs to update its collider state accordingly.Heres an example gif with a
visualize_collidercomponent. The collider is generate from the asset pipeline and the ship model is a child of a parent entity that handles all the movement and rotation.I need feedback and guidance on this. I basically copied this from the
query((translation().changed(), rotation().changed())).incl(physics_shape());query implementation. Let me know any thoughts on how to improve.