Skip to content

Guard remaining level_colliders lookups against missing keys (extends #1396) #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mynameisgrass merged 1 commit into from
Jul 31, 2026
Merged

Guard remaining level_colliders lookups against missing keys (extends #1396) #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 18 additions & 16 deletions sable_rapier/src/main/rust/rapier/src/contraptions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,8 @@ macro_rules! extract_jint_array {
fn get_kinematic_collider_info(
sable: &mut SableSceneData,
id: jint,
) -> &mut ActiveLevelColliderInfo {
sable
.level_colliders
.get_mut(&(id as LevelColliderID))
.expect("No kinematic contraption with given ID!")
) -> Option<&mut ActiveLevelColliderInfo> {
sable.level_colliders.get_mut(&(id as LevelColliderID))
}

#[unsafe(no_mangle)]
Expand All @@ -66,18 +63,17 @@ pub extern "system" fn Java_dev_ryanhcode_sable_physics_impl_rapier_Rapier3D_cre
.insert(RigidBodyBuilder::kinematic_position_based());
Some(new_body)
} else {
Some(
*sable_data
.rigid_bodies
.get(&(mount_id as LevelColliderID))
.unwrap(),
)
sable_data
.rigid_bodies
.get(&(mount_id as LevelColliderID))
.copied()
};

let mount_rigid_body: RigidBodyHandle = if let Some(body) = mount_rigid_body {
body
} else {
panic!("woops!")
// The mount body was unloaded before this call reached us; nothing to attach to.
return;
};

let level_collider = LevelCollider::new(Some(id as LevelColliderID), false);
Expand Down Expand Up @@ -140,7 +136,9 @@ pub extern "system" fn Java_dev_ryanhcode_sable_physics_impl_rapier_Rapier3D_set
let mut sim_data = scene.sim_data.write().unwrap();
let mut sable_data = scene.sable_data.write().unwrap();

let info = get_kinematic_collider_info(&mut sable_data, id);
let Some(info) = get_kinematic_collider_info(&mut sable_data, id) else {
return;
};
let collider_handle = info.collider;

let collider = sim_data.collider_set.get_mut(collider_handle);
Expand Down Expand Up @@ -218,7 +216,9 @@ pub extern "system" fn Java_dev_ryanhcode_sable_physics_impl_rapier_Rapier3D_add

with_handle(handle, |scene| {
let mut sable_data = scene.sable_data.write().unwrap();
let info = get_kinematic_collider_info(&mut sable_data, id);
let Some(info) = get_kinematic_collider_info(&mut sable_data, id) else {
return;
};
if let Some(chunk_map) = &mut info.chunk_map {
chunk_map.insert(crate::scene::pack_section_pos(x, y, z), chunk);
}
Expand All @@ -240,8 +240,10 @@ pub extern "system" fn Java_dev_ryanhcode_sable_physics_impl_rapier_Rapier3D_rem
let sim_data = &mut *sim_data;
let mut sable_data = scene.sable_data.write().unwrap();

let info = sable_data.level_colliders.remove(&(id as LevelColliderID));
let info = info.unwrap();
// Already removed (e.g. a duplicate/late removal after an unload) - nothing to do.
let Some(info) = sable_data.level_colliders.remove(&(id as LevelColliderID)) else {
return;
};

sim_data.collider_set.remove(
info.collider,
Expand Down
69 changes: 46 additions & 23 deletions sable_rapier/src/main/rust/rapier/src/dispatcher.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,25 +184,35 @@ where
let sable_data = self.sable_data.read().unwrap();
let body_1 = g1
.id
.map(|id| &sable_data.level_colliders[&(id as LevelColliderID)])
.unwrap();
.and_then(|id| sable_data.level_colliders.get(&(id as LevelColliderID)));
let body_2 = g2
.id
.map(|id| &sable_data.level_colliders[&(id as LevelColliderID)])
.unwrap();

let extents_1 = body_1.local_bounds_max.unwrap()
- body_1.local_bounds_min.unwrap()
+ IVec3::ONE;
let extents_2 = body_2.local_bounds_max.unwrap()
- body_2.local_bounds_min.unwrap()
+ IVec3::ONE;

let volume_1 = extents_1.x * extents_1.y * extents_1.z;
let volume_2 = extents_2.x * extents_2.y * extents_2.z;

// Swap the bodies so we're always doing the least amount of work possible for collision detection
volume_1 < volume_2
.and_then(|id| sable_data.level_colliders.get(&(id as LevelColliderID)));

// A body can be unloaded mid-step, or its bounds may not be set yet.
// Swapping is only a performance choice, so fall back to the unswapped
// order rather than aborting the process.
match (body_1, body_2) {
(Some(body_1), Some(body_2)) => match (
body_1.local_bounds_max,
body_1.local_bounds_min,
body_2.local_bounds_max,
body_2.local_bounds_min,
) {
(Some(max_1), Some(min_1), Some(max_2), Some(min_2)) => {
let extents_1 = max_1 - min_1 + IVec3::ONE;
let extents_2 = max_2 - min_2 + IVec3::ONE;

let volume_1 = extents_1.x * extents_1.y * extents_1.z;
let volume_2 = extents_2.x * extents_2.y * extents_2.z;

// Swap the bodies so we're always doing the least amount of work possible for collision detection
volume_1 < volume_2
}
_ => false,
},
_ => false,
}
};

if swap {
Expand Down Expand Up @@ -258,8 +268,10 @@ impl SableDispatcher {

let collider_info = g1
.id
.map(|id| &sable_data.level_colliders[&(id as LevelColliderID)]);
let center_of_mass_1 = collider_info.map_or(DVec3::ZERO, |b| b.center_of_mass.unwrap());
.and_then(|id| sable_data.level_colliders.get(&(id as LevelColliderID)));
let center_of_mass_1 = collider_info
.and_then(|b| b.center_of_mass)
.unwrap_or(DVec3::ZERO);

let mut local_aabb = g2.compute_aabb(pos12);

Expand Down Expand Up @@ -443,10 +455,21 @@ impl SableDispatcher {

let collider_info_1 = g1
.id
.map(|id| &sable_data.level_colliders[&(id as LevelColliderID)]);
let collider_info_2 = &sable_data.level_colliders[&(g2.id.unwrap() as LevelColliderID)];
let center_of_mass_1 = collider_info_1.map_or(DVec3::ZERO, |b| b.center_of_mass.unwrap());
let center_of_mass_2 = collider_info_2.center_of_mass.unwrap();
.and_then(|id| sable_data.level_colliders.get(&(id as LevelColliderID)));
// g2 is required below; if it was unloaded mid-step, skip contact generation for
// this pair instead of aborting the process.
let Some(collider_info_2) = g2
.id
.and_then(|id| sable_data.level_colliders.get(&(id as LevelColliderID)))
else {
return;
};
let center_of_mass_1 = collider_info_1
.and_then(|b| b.center_of_mass)
.unwrap_or(DVec3::ZERO);
let Some(center_of_mass_2) = collider_info_2.center_of_mass else {
return;
};

let chunk_access_1: &dyn ChunkAccess = if let Some(info) = collider_info_1
&& info.has_own_chunks()
Expand Down
14 changes: 9 additions & 5 deletions sable_rapier/src/main/rust/rapier/src/hooks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,15 @@ impl SablePhysicsHooks {
level_collider_a: Option<&LevelCollider>,
) -> Vec3 {
if let Some(level_collider_a) = level_collider_a
&& level_collider_a.id.is_some()
&& let Some(id) = level_collider_a.id
{
let sable_data = self.sable_data.read().unwrap();

let collider_info =
&sable_data.level_colliders[&(level_collider_a.id.unwrap() as LevelColliderID)];
// The body may have been unloaded mid-step; treat it as having no fake velocity.
let Some(collider_info) = sable_data.level_colliders.get(&(id as LevelColliderID))
else {
return Vec3::ZERO;
};

if let Some(fake_velo) = collider_info.fake_velocities {
let transform = collider_a.position();
Expand All @@ -160,8 +163,9 @@ impl SablePhysicsHooks {

let (tangent_velo, center_of_mass, skip_contact_events) = {
let sable_data = self.sable_data.read().unwrap();
let collider_info =
level_collider.and_then(|lc| lc.id.map(|id| &sable_data.level_colliders[&(id)]));
let collider_info = level_collider
.and_then(|lc| lc.id)
.and_then(|id| sable_data.level_colliders.get(&(id)));

let mut tangent_velo = Vec3::ZERO;
if let Some(fake_velo) = collider_info.and_then(|info| info.fake_velocities) {
Expand Down
19 changes: 9 additions & 10 deletions sable_rapier/src/main/rust/rapier/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,10 +551,9 @@ pub extern "system" fn Java_dev_ryanhcode_sable_physics_impl_rapier_Rapier3D_set
) {
with_handle(handle, |scene| {
let mut sable_data = scene.sable_data.write().unwrap();
let info = sable_data
.level_colliders
.get_mut(&(id as LevelColliderID))
.unwrap();
let Some(info) = sable_data.level_colliders.get_mut(&(id as LevelColliderID)) else {
return;
};
info.center_of_mass = Some(DVec3::new(x, y, z));
let mut sim_data = scene.sim_data.write().unwrap();
update_collider_aabb(&mut sim_data, info);
Expand Down Expand Up @@ -586,7 +585,9 @@ pub extern "system" fn Java_dev_ryanhcode_sable_physics_impl_rapier_Rapier3D_set
..
} = &mut *sable_data;

let info = level_colliders.get_mut(&(id as LevelColliderID)).unwrap();
let Some(info) = level_colliders.get_mut(&(id as LevelColliderID)) else {
return;
};
info.set_local_bounds(
IVec3::new(min_x, min_y, min_z),
IVec3::new(max_x, max_y, max_z),
Expand Down Expand Up @@ -796,11 +797,9 @@ pub extern "system" fn Java_dev_ryanhcode_sable_physics_impl_rapier_Rapier3D_add
let chunk = main_level_chunks.get(&pack_section_pos(x, y, z)).unwrap();
if global == 0 {
if object_id != -1 {
let body = level_colliders
.get_mut(&(object_id as LevelColliderID))
.unwrap();

body.insert_chunk(chunk, x, y, z, collider_map);
if let Some(body) = level_colliders.get_mut(&(object_id as LevelColliderID)) {
body.insert_chunk(chunk, x, y, z, collider_map);
}
}
} else {
for bx in 0..16 {
Expand Down
61 changes: 35 additions & 26 deletions sable_rapier/src/main/rust/rapier/src/rope.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,39 +54,48 @@ pub fn tick(scene: &PhysicsScene) {
if !sim.impulse_joint_set.contains(attachment.joint) {
dead_start_attachments.push(id.clone());
} else {
let local_anchor = attachment.location
- if let Some(id_b) = attachment.sub_level_id {
let rb_b = &sable_data.level_colliders[&id_b];
rb_b.center_of_mass.unwrap()
} else {
DVec3::ZERO
};

let impulse_joint = sim
.impulse_joint_set
.get_mut(attachment.joint, false)
.unwrap();
impulse_joint.data.set_local_anchor1(local_anchor.as_vec3());
// The attached sub-level may have been unloaded while the rope is still
// alive; in that case leave the anchor untouched instead of aborting.
let offset = match attachment.sub_level_id {
Some(id_b) => sable_data
.level_colliders
.get(&id_b)
.and_then(|rb_b| rb_b.center_of_mass),
None => Some(DVec3::ZERO),
};

if let Some(offset) = offset {
let local_anchor = attachment.location - offset;
if let Some(impulse_joint) =
sim.impulse_joint_set.get_mut(attachment.joint, false)
{
impulse_joint.data.set_local_anchor1(local_anchor.as_vec3());
}
}
}
}

if let Some(attachment) = &rope.end_attachment {
if !sim.impulse_joint_set.contains(attachment.joint) {
dead_end_attachments.push(id.clone());
} else {
let local_anchor = attachment.location
- if let Some(id_b) = attachment.sub_level_id {
let rb_b = &sable_data.level_colliders[&id_b];
rb_b.center_of_mass.unwrap()
} else {
DVec3::ZERO
};

let impulse_joint = sim
.impulse_joint_set
.get_mut(attachment.joint, false)
.unwrap();
impulse_joint.data.set_local_anchor1(local_anchor.as_vec3());
// Same guard as the start attachment above.
let offset = match attachment.sub_level_id {
Some(id_b) => sable_data
.level_colliders
.get(&id_b)
.and_then(|rb_b| rb_b.center_of_mass),
None => Some(DVec3::ZERO),
};

if let Some(offset) = offset {
let local_anchor = attachment.location - offset;
if let Some(impulse_joint) =
sim.impulse_joint_set.get_mut(attachment.joint, false)
{
impulse_joint.data.set_local_anchor1(local_anchor.as_vec3());
}
}
}
}
}
Expand Down