From 59477d9cc29fc5fe08e753fb4e00aa8da54273b8 Mon Sep 17 00:00:00 2001
From: Thierry Berger <contact@thierryberger.com>
Date: Tue, 23 Jul 2024 10:10:16 +0200
Subject: [PATCH]  Fix a crash when removing colliders in
 `TimestepMode::Interpolated` (#563)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Sébastien Crozet <sebastien@crozet.re>
---
 CHANGELOG.md              |  5 +++
 src/pipeline/events.rs    | 80 ++++++++++++++++++++++++++++++++++++++-
 src/plugin/context.rs     |  8 ++++
 src/plugin/systems/mod.rs |  1 -
 4 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ac5fcce5..368514c5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+### Fix
+
+- Fix a crash when using `TimestepMode::Interpolated` and removing colliders
+during a frame which would not run a simulation step.
+
 ### Added
 
 - Added a `TriMeshFlags` parameter for `ComputedColliderShape`,
diff --git a/src/pipeline/events.rs b/src/pipeline/events.rs
index c039b1fc..850d4a8d 100644
--- a/src/pipeline/events.rs
+++ b/src/pipeline/events.rs
@@ -125,7 +125,13 @@ impl<'a> EventHandler for EventQueue<'a> {
 
 #[cfg(test)]
 mod test {
-    use bevy::time::{TimePlugin, TimeUpdateStrategy};
+    use bevy::{
+        app::{App, Startup, Update},
+        prelude::{Commands, Component, Entity, Query, With},
+        time::{TimePlugin, TimeUpdateStrategy},
+        transform::{bundles::TransformBundle, components::Transform, TransformPlugin},
+        MinimalPlugins,
+    };
     use systems::tests::HeadlessRenderPlugin;
 
     use crate::{plugin::*, prelude::*};
@@ -231,4 +237,76 @@ mod test {
             ));
         }
     }
+
+    #[test]
+    pub fn spam_remove_rapier_entity_interpolated() {
+        let mut app = App::new();
+        app.add_plugins((
+            HeadlessRenderPlugin,
+            MinimalPlugins,
+            TransformPlugin,
+            RapierPhysicsPlugin::<NoUserData>::default(),
+        ))
+        .insert_resource(RapierConfiguration {
+            timestep_mode: TimestepMode::Interpolated {
+                dt: 1.0 / 30.0,
+                time_scale: 1.0,
+                substeps: 2,
+            },
+            ..RapierConfiguration::new(1f32)
+        })
+        .add_systems(Startup, setup_physics)
+        .add_systems(Update, remove_collider);
+        // Simulates 60 updates per seconds
+        app.insert_resource(TimeUpdateStrategy::ManualDuration(
+            std::time::Duration::from_secs_f32(1f32 / 60f32),
+        ));
+
+        for i in 0..100 {
+            dbg!(i);
+            app.update();
+        }
+        return;
+
+        #[derive(Component)]
+        pub struct ToRemove;
+
+        #[cfg(feature = "dim3")]
+        fn cuboid(hx: Real, hy: Real, hz: Real) -> Collider {
+            Collider::cuboid(hx, hy, hz)
+        }
+        #[cfg(feature = "dim2")]
+        fn cuboid(hx: Real, hy: Real, _hz: Real) -> Collider {
+            Collider::cuboid(hx, hy)
+        }
+        pub fn setup_physics(mut commands: Commands) {
+            for _i in 0..100 {
+                commands.spawn((
+                    TransformBundle::from(Transform::from_xyz(0.0, 0.0, 0.0)),
+                    RigidBody::Dynamic,
+                    cuboid(0.5, 0.5, 0.5),
+                    ActiveEvents::all(),
+                    ToRemove,
+                ));
+            }
+            /*
+             * Ground
+             */
+            let ground_size = 5.1;
+            let ground_height = 0.1;
+            let starting_y = -0.5 - ground_height;
+
+            commands.spawn((
+                TransformBundle::from(Transform::from_xyz(0.0, starting_y, 0.0)),
+                cuboid(ground_size, ground_height, ground_size),
+            ));
+        }
+
+        fn remove_collider(mut commands: Commands, query: Query<Entity, With<ToRemove>>) {
+            let Some(entity) = query.iter().next() else {
+                return;
+            };
+            commands.entity(entity).despawn();
+        }
+    }
 }
diff --git a/src/plugin/context.rs b/src/plugin/context.rs
index eb7fe305..ce8bda56 100644
--- a/src/plugin/context.rs
+++ b/src/plugin/context.rs
@@ -225,6 +225,7 @@ impl RapierContext {
             .or_else(|| event_queue.as_ref().map(|q| q as &dyn EventHandler))
             .unwrap_or(&() as &dyn EventHandler);
 
+        let mut executed_steps = 0;
         match timestep_mode {
             TimestepMode::Interpolated {
                 dt,
@@ -271,6 +272,7 @@ impl RapierContext {
                             hooks,
                             events,
                         );
+                        executed_steps += 1;
                     }
 
                     sim_to_render_time.diff -= dt;
@@ -302,6 +304,7 @@ impl RapierContext {
                         hooks,
                         events,
                     );
+                    executed_steps += 1;
                 }
             }
             TimestepMode::Fixed { dt, substeps } => {
@@ -326,9 +329,14 @@ impl RapierContext {
                         hooks,
                         events,
                     );
+                    executed_steps += 1;
                 }
             }
         }
+
+        if executed_steps > 0 {
+            self.deleted_colliders.clear();
+        }
     }
 
     /// This method makes sure tha the rigid-body positions have been propagated to
diff --git a/src/plugin/systems/mod.rs b/src/plugin/systems/mod.rs
index c24571bb..658ddf0a 100644
--- a/src/plugin/systems/mod.rs
+++ b/src/plugin/systems/mod.rs
@@ -50,7 +50,6 @@ pub fn step_simulation<Hooks>(
             &mut sim_to_render_time,
             Some(interpolation_query),
         );
-        context.deleted_colliders.clear();
     } else {
         context.propagate_modified_body_positions_to_colliders();
     }