Skip to content

Commit c3dab1a

Browse files
authored
Detach reattach propagation fix (#24019)
# Objective Fixes #23893 Chain of events as far as I can follow it: 1. Remove a relationship from an entity E. 2. Immediately restore the relationship. 3. The `on_r_removed<C, R>` observer system triggered by `On<Remove, R>` removes the `Inherited<C>` component from E. 4. The `on_r_inserted<C, R>` observer system triggered by `On<Insert, R>` inserts `Inherited<C>` back again on E. 5. `propagated_inherited` queries for changes, adds `(E, Inherited<C>)` to `to_process`. 6. `propagate_inherited` queries for `RemovedComponents<Inherited<C>>`, adds `(F, None)` to `to_process` for each F descended from E. 7. Then `to_process` is processed. 8. The `(F, None)` entries are processed first, recursively removing `Inherited<C>` from E's descendants using deferred commands. 9. Then `(E, Inherited<C>)` is ignored as the defererred commands haven't been applied yet so the current inherited state matches the queued inherited state. ## Solution If an entity from `RemovedComponents<Inherited<C>>` has `Inherited<C>`, don't add its descendants to the `to_process` stack. Not super confident in this change, but the new test passes and none of the old tests fail. ## Testing New test: `test_detach_and_reattach_propagates_to_descendants`
1 parent 5754300 commit c3dab1a

1 file changed

Lines changed: 40 additions & 1 deletion

File tree

crates/bevy_app/src/propagate.rs

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ pub fn propagate_inherited<C: Component + Clone + PartialEq, F: QueryFilter, R:
263263

264264
// and removed
265265
for entity in removed.read() {
266-
if let Ok((Some(targets), _, _)) = recurse.get(entity) {
266+
if let Ok((Some(targets), None, _)) = recurse.get(entity) {
267267
to_process.extend(targets.iter().map(|target| (target, None)));
268268
}
269269
}
@@ -479,6 +479,45 @@ mod tests {
479479
assert_eq!(query.get(app.world(), propagatee), Ok(&TestValue(2)));
480480
}
481481

482+
#[test]
483+
fn test_detach_and_reattach_propagates_to_descendants() {
484+
let mut app = App::new();
485+
app.add_schedule(Schedule::new(Update));
486+
app.add_plugins(HierarchyPropagatePlugin::<TestValue>::new(Update));
487+
488+
let mut query = app.world_mut().query::<&TestValue>();
489+
490+
let propagator = app.world_mut().spawn(Propagate(TestValue(1))).id();
491+
let intermediate = app
492+
.world_mut()
493+
.spawn_empty()
494+
.insert(ChildOf(propagator))
495+
.id();
496+
let propagatee = app
497+
.world_mut()
498+
.spawn_empty()
499+
.insert(ChildOf(intermediate))
500+
.id();
501+
502+
app.update();
503+
504+
assert_eq!(
505+
query.get_many(app.world(), [intermediate, propagatee]),
506+
Ok([&TestValue(1), &TestValue(1)])
507+
);
508+
509+
app.world_mut()
510+
.entity_mut(intermediate)
511+
.remove::<ChildOf>()
512+
.insert(ChildOf(propagator));
513+
app.update();
514+
515+
assert_eq!(
516+
query.get_many(app.world(), [intermediate, propagatee]),
517+
Ok([&TestValue(1), &TestValue(1)])
518+
);
519+
}
520+
482521
#[test]
483522
fn test_propagate_over() {
484523
let mut app = App::new();

0 commit comments

Comments
 (0)