Skip to content

Commit 325030e

Browse files
authored
Fix panic caused by despawning entity with ColliderOf (#677)
# Objective Fixes #676. #671 changed the `ColliderParent` component to a `ColliderOf` relationship. Because Bevy does not normally allow relationships to point to their own entities, this required a partial manual implementation of the `Relationship` trait. However, I left the default `on_replace` implementation, which (as it turns out) panics if the target entity does not exist, which is possible when despawning an entity and the relationship points to itself. ## Solution Manually implement `on_replace` in a non-panicking way. Also replace some panicking uses of `remove` with `try_remove`. Ideally this would be fixed by Bevy allowing self-relationships though.
1 parent fb0158d commit 325030e

File tree

3 files changed

+55
-4
lines changed

3 files changed

+55
-4
lines changed

src/collision/collider/backend.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ impl<C: ScalableCollider> Plugin for ColliderBackendPlugin<C> {
183183
world
184184
.commands()
185185
.entity(ctx.entity)
186-
.remove::<ColliderMarker>();
186+
.try_remove::<ColliderMarker>();
187187

188188
let entity_ref = world.entity_mut(ctx.entity);
189189

src/collision/collider/collider_hierarchy/mod.rs

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::prelude::*;
99
use bevy::{
1010
ecs::{
1111
component::HookContext,
12-
relationship::{Relationship, RelationshipHookMode},
12+
relationship::{Relationship, RelationshipHookMode, RelationshipSourceCollection},
1313
world::DeferredWorld,
1414
},
1515
prelude::*,
@@ -127,6 +127,57 @@ impl Relationship for ColliderOf {
127127
);
128128
}
129129
}
130+
131+
fn on_replace(
132+
mut world: DeferredWorld,
133+
HookContext {
134+
entity,
135+
relationship_hook_mode,
136+
..
137+
}: HookContext,
138+
) {
139+
// This is largely the same as the default implementation,
140+
// but does not panic if the relationship target does not exist.
141+
142+
match relationship_hook_mode {
143+
RelationshipHookMode::Run => {}
144+
RelationshipHookMode::Skip => return,
145+
RelationshipHookMode::RunIfNotLinked => {
146+
if <Self::RelationshipTarget as RelationshipTarget>::LINKED_SPAWN {
147+
return;
148+
}
149+
}
150+
}
151+
let rigid_body = world.entity(entity).get::<Self>().unwrap().get();
152+
if let Ok(mut rigid_body_mut) = world.get_entity_mut(rigid_body) {
153+
if let Some(mut relationship_target) =
154+
rigid_body_mut.get_mut::<Self::RelationshipTarget>()
155+
{
156+
RelationshipSourceCollection::remove(
157+
relationship_target.collection_mut_risky(),
158+
entity,
159+
);
160+
if relationship_target.len() == 0 {
161+
if let Ok(mut entity) = world.commands().get_entity(rigid_body) {
162+
// this "remove" operation must check emptiness because in the event that an identical
163+
// relationship is inserted on top, this despawn would result in the removal of that identical
164+
// relationship ... not what we want!
165+
entity.queue_handled(
166+
|mut entity: EntityWorldMut| {
167+
if entity
168+
.get::<Self::RelationshipTarget>()
169+
.is_some_and(RelationshipTarget::is_empty)
170+
{
171+
entity.remove::<Self::RelationshipTarget>();
172+
}
173+
},
174+
|_, _| {},
175+
);
176+
}
177+
}
178+
}
179+
}
180+
}
130181
}
131182

132183
/// A [`RelationshipTarget`] component that tracks which colliders are attached to a [`RigidBody`].

src/collision/collider/collider_hierarchy/plugin.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ impl Plugin for ColliderHierarchyPlugin {
3636

3737
// Make sure the collider is on the same entity as the rigid body.
3838
if query.contains(entity) {
39-
commands.entity(entity).remove::<ColliderOf>();
39+
commands.entity(entity).try_remove::<ColliderOf>();
4040
}
4141
},
4242
);
@@ -117,7 +117,7 @@ fn on_rigid_body_removed(
117117
for collider_entity in colliders.iter() {
118118
commands
119119
.entity(collider_entity)
120-
.remove::<(ColliderOf, ColliderTransform)>();
120+
.try_remove::<(ColliderOf, ColliderTransform)>();
121121
}
122122
}
123123
}

0 commit comments

Comments
 (0)