Skip to content

Commit

Permalink
Fix observer/hook OnReplace and OnRemove triggering when removing a b…
Browse files Browse the repository at this point in the history
…undle 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.
  • Loading branch information
andriyDev authored Feb 24, 2025
1 parent 910e405 commit bb09751
Showing 1 changed file with 47 additions and 14 deletions.
61 changes: 47 additions & 14 deletions crates/bevy_ecs/src/world/entity_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<OnRemove, A>, mut tracker: ResMut<Tracker>| {
tracker.a = true;
});
world.add_observer(|_: Trigger<OnRemove, B>, mut tracker: ResMut<Tracker>| {
tracker.b = true;
});

world.entity_mut(entity).remove::<(A, B)>();

assert_eq!(
world.resource::<Tracker>(),
&Tracker {
a: true,
// The entity didn't have a B component, so it should not have been triggered.
b: false,
}
);
}
}

0 comments on commit bb09751

Please sign in to comment.