Run observers before hooks for on_replace and on_remove (#16499)

# Objective

- Fixes #16498 

## Solution

- Trivially swaps ordering of hooks and observers for all call sites
where they are triggered for `on_replace` or `on_remove`

## Testing

- Just CI

---

## Migration Guide

The order of hooks and observers for `on_replace` and `on_remove` has
been swapped. Observers are now run before hooks. This is a more natural
ordering where the removal ordering is inverted compared to the
insertion ordering.
This commit is contained in:
Nuutti Kotivuori 2024-12-06 02:24:27 +02:00 committed by GitHub
parent 0070514f54
commit 912da04699
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 10 additions and 10 deletions

View File

@ -902,7 +902,6 @@ impl<'w> BundleInserter<'w> {
let mut deferred_world = self.world.into_deferred();
if insert_mode == InsertMode::Replace {
deferred_world.trigger_on_replace(archetype, entity, add_bundle.iter_existing());
if archetype.has_replace_observer() {
deferred_world.trigger_observers(
ON_REPLACE,
@ -910,6 +909,7 @@ impl<'w> BundleInserter<'w> {
add_bundle.iter_existing(),
);
}
deferred_world.trigger_on_replace(archetype, entity, add_bundle.iter_existing());
}
}

View File

@ -1863,14 +1863,14 @@ impl<'w> EntityWorldMut<'w> {
// SAFETY: All components in the archetype exist in world
unsafe {
deferred_world.trigger_on_replace(archetype, self.entity, archetype.components());
if archetype.has_replace_observer() {
deferred_world.trigger_observers(ON_REPLACE, self.entity, archetype.components());
}
deferred_world.trigger_on_remove(archetype, self.entity, archetype.components());
deferred_world.trigger_on_replace(archetype, self.entity, archetype.components());
if archetype.has_remove_observer() {
deferred_world.trigger_observers(ON_REMOVE, self.entity, archetype.components());
}
deferred_world.trigger_on_remove(archetype, self.entity, archetype.components());
}
for component_id in archetype.components() {
@ -2118,7 +2118,6 @@ unsafe fn trigger_on_replace_and_on_remove_hooks_and_observers(
entity: Entity,
bundle_info: &BundleInfo,
) {
deferred_world.trigger_on_replace(archetype, entity, bundle_info.iter_explicit_components());
if archetype.has_replace_observer() {
deferred_world.trigger_observers(
ON_REPLACE,
@ -2126,10 +2125,11 @@ unsafe fn trigger_on_replace_and_on_remove_hooks_and_observers(
bundle_info.iter_explicit_components(),
);
}
deferred_world.trigger_on_remove(archetype, entity, bundle_info.iter_explicit_components());
deferred_world.trigger_on_replace(archetype, entity, bundle_info.iter_explicit_components());
if archetype.has_remove_observer() {
deferred_world.trigger_observers(ON_REMOVE, entity, bundle_info.iter_explicit_components());
}
deferred_world.trigger_on_remove(archetype, entity, bundle_info.iter_explicit_components());
}
const QUERY_MISMATCH_ERROR: &str = "Query does not match the current entity";
@ -4910,14 +4910,14 @@ mod tests {
"OrdB hook on_insert",
"OrdB observer on_insert",
"OrdB command on_add", // command added by OrdB hook on_add, needs to run before despawn command
"OrdA hook on_replace", // start of despawn
"OrdB hook on_replace",
"OrdA observer on_replace",
"OrdA observer on_replace", // start of despawn
"OrdB observer on_replace",
"OrdA hook on_remove",
"OrdB hook on_remove",
"OrdA hook on_replace",
"OrdB hook on_replace",
"OrdA observer on_remove",
"OrdB observer on_remove",
"OrdA hook on_remove",
"OrdB hook on_remove",
];
world.flush();
assert_eq!(world.resource_mut::<TestVec>().0.as_slice(), &expected[..]);