dimforge / rapier

2D and 3D physics engines focused on performance.
https://rapier.rs
Apache License 2.0
3.77k stars 235 forks source link

Panics when setting collider via `ColliderSet::get_mut()` #595

Closed tadeohepperle closed 3 months ago

tadeohepperle commented 4 months ago

I have some code in which I need to update colliders every frame, because the size of my objects changes over time:

use rapier3d::{
    dynamics::RigidBodySet,
    geometry::{BroadPhase, Collider, ColliderBuilder, ColliderHandle, ColliderSet, NarrowPhase},
    na::Vector3,
    pipeline::{CollisionPipeline, QueryPipeline},
};

pub struct PhysicsWorld {
    collision_pipeline: CollisionPipeline,
    query_pipeline: QueryPipeline,
    collider_set: ColliderSet,
    rigid_body_set: RigidBodySet,
    broad_phase: BroadPhase,
    narrow_phase: NarrowPhase,
}

impl PhysicsWorld {
    pub fn new() -> Self {
        let collision_pipeline = CollisionPipeline::new();
        let query_pipeline = QueryPipeline::new();
        let collider_set = ColliderSet::new();
        let rigid_body_set = RigidBodySet::new();
        let broad_phase = BroadPhase::new();
        let narrow_phase = NarrowPhase::new();

        PhysicsWorld {
            collision_pipeline,
            query_pipeline,
            collider_set,
            rigid_body_set,
            broad_phase,
            narrow_phase,
        }
    }

    /// should be called every frame
    pub fn step(&mut self) {
        // right now no fixed timestep but that does not matter since we do not do physics simulation anyways, just collision and queries.
        self.collision_pipeline.step(
            0.002,
            &mut self.broad_phase,
            &mut self.narrow_phase,
            &mut self.rigid_body_set,
            &mut self.collider_set,
            Some(&mut self.query_pipeline),
            &(),
            &(),
        );
    }
}

fn ball_collider(i: usize, time: f32) -> Collider {
    let v = i as f32 + time;
    ColliderBuilder::ball(v * 0.2)
        .mass(1.0)
        .translation(Vector3::new(v, v, v))
        .build()
}

fn main() {
    let mut world = PhysicsWorld::new();

    let mut time: f32 = 0.0;
    let mut balls: Vec<ColliderHandle> = vec![];

    // add some balls to the world:
    for i in 0..100 {
        let collider = ball_collider(i, time);
        let collider_handle = world.collider_set.insert(collider);
        balls.push(collider_handle)
    }

    // run for 100 frames
    for _ in 0..100 {
        time += 0.01666;
        // update the collide for each ball, to make it bigger:
        for (i, ball) in balls.iter_mut().enumerate() {
            let new_collider = ball_collider(i, time);
            let old_collider = world.collider_set.get_mut(*ball).unwrap();
            *old_collider = new_collider;
        }
        world.step();
    }
}

Running this gives me:

thread 'main' panicked at /home/tadeo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rapier3d-0.18.0/src/data/coarena.rs:112:9:
assertion `left != right` failed: Cannot index the same object twice.
  left: 57
 right: 57

Why do I have mutable access to the collider if I am not allowed setting it? Should I instead just set the shape of the collider via set_shape() instead of replacing the entire collider? :)

sebcrozet commented 4 months ago

The Collider contains some internal indices related to other parts of the physics engine. You have mutable access to che collider so you can call its mutable methods, but overwriting it completely will be problematic.

sebcrozet commented 3 months ago

I will add a Collider::copy_from function that you can use instead of direct assignation old_collider.copy_from(&new_collider). But, yes, it will be preferable to only set the shape if that’s what you need to achieve.