Skip to content

Commit bb09751

Browse files
authored
Fix observer/hook OnReplace and OnRemove triggering when removing a bundle even when the component is not present on the entity (#17942)
# Objective - Fixes #17897. ## Solution - When removing components, we filter the list of components in the removed bundle based on whether they are actually in the archetype. ## Testing - Added a test.
1 parent 910e405 commit bb09751

File tree

1 file changed

+47
-14
lines changed

1 file changed

+47
-14
lines changed

crates/bevy_ecs/src/world/entity_ref.rs

Lines changed: 47 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2653,34 +2653,29 @@ unsafe fn trigger_on_replace_and_on_remove_hooks_and_observers(
26532653
bundle_info: &BundleInfo,
26542654
caller: MaybeLocation,
26552655
) {
2656+
let bundle_components_in_archetype = || {
2657+
bundle_info
2658+
.iter_explicit_components()
2659+
.filter(|component_id| archetype.contains(*component_id))
2660+
};
26562661
if archetype.has_replace_observer() {
26572662
deferred_world.trigger_observers(
26582663
ON_REPLACE,
26592664
entity,
2660-
bundle_info.iter_explicit_components(),
2665+
bundle_components_in_archetype(),
26612666
caller,
26622667
);
26632668
}
2664-
deferred_world.trigger_on_replace(
2665-
archetype,
2666-
entity,
2667-
bundle_info.iter_explicit_components(),
2668-
caller,
2669-
);
2669+
deferred_world.trigger_on_replace(archetype, entity, bundle_components_in_archetype(), caller);
26702670
if archetype.has_remove_observer() {
26712671
deferred_world.trigger_observers(
26722672
ON_REMOVE,
26732673
entity,
2674-
bundle_info.iter_explicit_components(),
2674+
bundle_components_in_archetype(),
26752675
caller,
26762676
);
26772677
}
2678-
deferred_world.trigger_on_remove(
2679-
archetype,
2680-
entity,
2681-
bundle_info.iter_explicit_components(),
2682-
caller,
2683-
);
2678+
deferred_world.trigger_on_remove(archetype, entity, bundle_components_in_archetype(), caller);
26842679
}
26852680

26862681
/// A view into a single entity and component in a world, which may either be vacant or occupied.
@@ -5900,4 +5895,42 @@ mod tests {
59005895

59015896
assert_eq!(archetype_pointer_before, archetype_pointer_after);
59025897
}
5898+
5899+
#[test]
5900+
fn bundle_remove_only_triggers_for_present_components() {
5901+
let mut world = World::default();
5902+
5903+
#[derive(Component)]
5904+
struct A;
5905+
5906+
#[derive(Component)]
5907+
struct B;
5908+
5909+
#[derive(Resource, PartialEq, Eq, Debug)]
5910+
struct Tracker {
5911+
a: bool,
5912+
b: bool,
5913+
}
5914+
5915+
world.insert_resource(Tracker { a: false, b: false });
5916+
let entity = world.spawn(A).id();
5917+
5918+
world.add_observer(|_: Trigger<OnRemove, A>, mut tracker: ResMut<Tracker>| {
5919+
tracker.a = true;
5920+
});
5921+
world.add_observer(|_: Trigger<OnRemove, B>, mut tracker: ResMut<Tracker>| {
5922+
tracker.b = true;
5923+
});
5924+
5925+
world.entity_mut(entity).remove::<(A, B)>();
5926+
5927+
assert_eq!(
5928+
world.resource::<Tracker>(),
5929+
&Tracker {
5930+
a: true,
5931+
// The entity didn't have a B component, so it should not have been triggered.
5932+
b: false,
5933+
}
5934+
);
5935+
}
59035936
}

0 commit comments

Comments
 (0)