From bb09751cf0d760bcc98124a947e60a8db198433c Mon Sep 17 00:00:00 2001 From: andriyDev Date: Mon, 24 Feb 2025 13:25:31 -0800 Subject: [PATCH] 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. --- crates/bevy_ecs/src/world/entity_ref.rs | 61 +++++++++++++++++++------ 1 file changed, 47 insertions(+), 14 deletions(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 4feb96bccf82a..297131e512bca 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -2653,34 +2653,29 @@ unsafe fn trigger_on_replace_and_on_remove_hooks_and_observers( bundle_info: &BundleInfo, caller: MaybeLocation, ) { + let bundle_components_in_archetype = || { + bundle_info + .iter_explicit_components() + .filter(|component_id| archetype.contains(*component_id)) + }; if archetype.has_replace_observer() { deferred_world.trigger_observers( ON_REPLACE, entity, - bundle_info.iter_explicit_components(), + bundle_components_in_archetype(), caller, ); } - deferred_world.trigger_on_replace( - archetype, - entity, - bundle_info.iter_explicit_components(), - caller, - ); + deferred_world.trigger_on_replace(archetype, entity, bundle_components_in_archetype(), caller); if archetype.has_remove_observer() { deferred_world.trigger_observers( ON_REMOVE, entity, - bundle_info.iter_explicit_components(), + bundle_components_in_archetype(), caller, ); } - deferred_world.trigger_on_remove( - archetype, - entity, - bundle_info.iter_explicit_components(), - caller, - ); + deferred_world.trigger_on_remove(archetype, entity, bundle_components_in_archetype(), caller); } /// A view into a single entity and component in a world, which may either be vacant or occupied. @@ -5900,4 +5895,42 @@ mod tests { assert_eq!(archetype_pointer_before, archetype_pointer_after); } + + #[test] + fn bundle_remove_only_triggers_for_present_components() { + let mut world = World::default(); + + #[derive(Component)] + struct A; + + #[derive(Component)] + struct B; + + #[derive(Resource, PartialEq, Eq, Debug)] + struct Tracker { + a: bool, + b: bool, + } + + world.insert_resource(Tracker { a: false, b: false }); + let entity = world.spawn(A).id(); + + world.add_observer(|_: Trigger, mut tracker: ResMut| { + tracker.a = true; + }); + world.add_observer(|_: Trigger, mut tracker: ResMut| { + tracker.b = true; + }); + + world.entity_mut(entity).remove::<(A, B)>(); + + assert_eq!( + world.resource::(), + &Tracker { + a: true, + // The entity didn't have a B component, so it should not have been triggered. + b: false, + } + ); + } }