sync_removals: skip removal events when a fresh handle is live#691
Open
svaterlaus wants to merge 2 commits into
Open
sync_removals: skip removal events when a fresh handle is live#691svaterlaus wants to merge 2 commits into
svaterlaus wants to merge 2 commits into
Conversation
When `RapierPhysicsPlugin` runs in a non-`PostUpdate` schedule (e.g. `.in_fixed_schedule()`), `sync_removals` is registered twice — once in `PhysicsSet::SyncBackend` and once as a `PostUpdate` safety net for events that would otherwise be missed. Each registration maintains an independent `RemovedComponents` cursor and re-processes the same removal events. This breaks cross-context migration. When a system removes `RapierRigidBodyHandle` and swaps `RapierContextEntityLink`, `init_rigid_bodies` correctly creates a fresh body in the new context's `entity2body` between the two `sync_removals` runs. The PostUpdate copy then re-reads the original removal event, walks all contexts via `find_context`, finds the entity in the *new* context's `entity2body`, and deletes the freshly-inserted body — leaving the entity with a live `RapierRigidBodyHandle` component but no body in any rapier set, so it is no longer integrated. Add a `live_handles` query and short-circuit each removal-event loop when the entity still has the corresponding handle component. A live handle means a fresh instance was inserted after the original removal (`init_rigid_bodies` / `init_colliders` / `init_joints`), and that instance must not be deleted by the stale event. The orphan-handling paths and the standard "user removed handle and meant it" path are unchanged. Applied to bodies, colliders, impulse joints, and multibody joints for parity.
Add `body_survives_context_migration_via_handle_swap` to the `multiple_rapier_contexts::test` module. The test runs `RapierPhysicsPlugin::<NoUserData>::default().in_fixed_schedule()` so that `sync_removals` is registered both inside `PhysicsSet::SyncBackend` and as the `PostUpdate` safety net. It spawns a `RigidBody::Dynamic` body in the default context, spawns a second `RapierContextSimulation`, and performs the context-migration pattern: insert a new `RapierContextEntityLink` and remove the entity's `RapierRigidBodyHandle` and `RapierColliderHandle` in the same step. After a few app updates it asserts the body is registered in the new context's `entity2body`, absent from the old context, and that the freshly-attached `RapierRigidBodyHandle` matches the new context's mapping. Without the live-handle guard in `sync_removals`, the PostUpdate copy re-processes the same removal event with its own cursor, walks all contexts via `find_context`, and deletes the body that `init_rigid_bodies` just inserted in the new context — failing the "body must live in the new context after migration" assertion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix sync_removals deleting a freshly-reinserted Rapier body/collider/joint when an entity is migrated between RapierContextSimulations.
Background
When RapierPhysicsPlugin runs in a non-PostUpdate schedule (e.g. .in_fixed_schedule()), sync_removals is registered twice:
Each registration owns an independent RemovedComponents cursor, so both copies see the same removal event.
The bug
A common cross-context migration pattern is to atomically swap an entity's RapierContextEntityLink and remove its RapierRigidBodyHandle / RapierColliderHandle. Between the two sync_removals runs, init_rigid_bodies / init_colliders correctly create a fresh body/collider in the new context's entity2body / entity2collider. The PostUpdate copy then re-reads the original removal event, walks all contexts via find_context, finds the entity in the new context's map, and deletes the freshly-inserted instance — leaving the entity with a live handle component but no body/collider in any rapier set, so it stops being integrated.
Fix
Add a live_handles query and short-circuit each removal-event loop when the entity still has the corresponding handle component. A live handle means a fresh instance was inserted after the original removal (init_rigid_bodies / init_colliders / init_joints), and that instance must not be deleted by the stale event. The orphan-handling paths and the standard "user removed handle and meant it" path are unchanged.
Applied to bodies, colliders, impulse joints, and multibody joints for parity.
Test
Adds body_survives_context_migration_via_handle_swap in multiple_rapier_contexts::test. It runs the plugin in .in_fixed_schedule() mode (so both sync_removals registrations are live), spawns a RigidBody::Dynamic body, spawns a second RapierContextSimulation, performs the link-swap + handle-removal pattern, and asserts that after a few app updates the body lives in the new context's entity2body, is gone from the original context, and that the freshly-attached RapierRigidBodyHandle matches the new context's mapping.
Confirmed the test fails with the expected assertion message when the fix is reverted. Full bevy_rapier2d and bevy_rapier3d --lib suites pass on the branch.