Stand-alone CollisionPipeline doesn't account for changes to bodies

[BrettHemes-3M]

Related to #191 (Stand-alone QueryPipeline doesn't account for changes to bodies)

I am using the CollisionPipeline with colliders attached to parent bodies (with some positional offsets). Upon stepping the pipeline, the respective modified bodies lists are cleared but the actual changed bits of the bodies in the RigidBodySet are not. This prevents the body from ever getting re-added to the modified list upon subsequent changes (i.e., when I update the parent body's position using get_mut).

The blocking check starts here:

rapier/src/dynamics/rigid_body_set.rs

Line 177 in 2f9d9ba

pub fn get_mut(&mut self, handle: RigidBodyHandle) -> Option<&mut RigidBody> {

and the modification addition skipped here:

rapier/src/dynamics/rigid_body_set.rs

Line 169 in 2f9d9ba

if !rb.changes.contains(RigidBodyChanges::MODIFIED) {

The if check never passes because the modified bits are already set (from never being cleared).

propagate_modified_body_positions_to_colliders doesn't work either due to the above (uses modification list, not changed bits)

The physics pipeline calls is own helpers, clear_modified_colliders and clear_modified_bodies while the CollisionPipeline only calls a single clear_modified_colliders

Should the CollisionPipeline clear the modified body bits? If not, and/or adding a way to clear the changed bits in my RigidBodySet manually would help (not in public API currently).

Workaround

As of right now, I can get the changes to propagate by manually updating the collider positions wrt their parent (to the same value 🙃) every time I move a body like so:

let collider_handle = self.name_collider_map.get(name).unwrap();
let collider = self.colliders.get_mut(*collider_handle).unwrap();
collider.set_position_wrt_parent(*collider.position_wrt_parent().unwrap());

Awesome library btw, thanks for all the hard work 🎉

[BrettHemes-3M]

Also to add, this affects all body changes via get_mut... (enable/disable, etc.) so I think some form of solution is required.

[ThierryBerger]

I share your concerns about this being error-prone, if you have a minimal reproduction use case, this could be a good base to make a new test and make sure it works in an expected way :)

• I'll link Fix user changes handling #803 and Assertion failure: position change, enabled state change, and missing MODIFIED flag #802 here, as they discuss those changed flags too.

[BrettHemes-3M]

(had put this in the wrong issue... moving it here)

The same/similar issue is mentioned in #662

So this is what gave me the majority of the headache. For a variety of reasons it doesn't really look like CollisionPipeline actually handles rigid body changes properly. For my game I have mostly Fixed bodies and a couple of Kinematic bodies. I think this is pretty much the typical case CollisionPipeline was written for.

However, there seems to be a couple of bugs causing the rigid bodies not to be able to update. For one thing CollisionPipeline is calling bodies.take_modified() at the beginning of CollisionPipeline::step but then never doing anything equivalent to PhysicsPipeline::clear_modified_bodies which removes the MODIFIED flag from any bodies in that list. This means once a body is in the modified list, it gets taken once, but then can never be added there again due to a check for MODIFIED in RigidBodySet::get_mut. Personally I think it would probably be cleaner to just reset that bit in take_modified before returning the vector since it has to be done at some point for things to work propertly anyway.

Secondly, CollisionPipeline never calls anything like PhysicsPipeline::advance_to_final_positions which seems to be required for the RigidBody::set_next_kinematic_position family of functions to work. I think this would be pretty trivial to add but...

Here removing the rigid body set from the collision pipeline is proposed. I was using them to have colliders offset from some other origin. I could keep track of this manually with the proposed change OR I could probably get the same behavior using a compound shape with only one shape in the vec (with a non-identity position).