From 70d7f8056404efe76feed12b6e64dfa75fad4a72 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Mon, 28 Nov 2022 13:40:14 +0000 Subject: [PATCH] Make adding children idempotent (#6763) # Objective * Fix #6668 * There is no need to panic when a parenting operation is redundant, as no invalid state is entered. ## Solution Make `push_children` idempotent. --- crates/bevy_hierarchy/src/child_builder.rs | 24 ++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 8e6309e481..f73e8ebd35 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -51,7 +51,11 @@ fn update_old_parents(world: &mut World, parent: Entity, children: &[Entity]) { let mut moved: SmallVec<[HierarchyEvent; 8]> = SmallVec::with_capacity(children.len()); for child in children { if let Some(previous) = update_parent(world, *child, parent) { - debug_assert!(parent != previous); + // Do nothing if the entity already has the correct parent. + if parent == previous { + continue; + } + remove_from_children(world, previous, *child); moved.push(HierarchyEvent::ChildMoved { child: *child, @@ -287,7 +291,8 @@ pub trait BuildChildren { /// # } /// ``` fn add_children(&mut self, f: impl FnOnce(&mut ChildBuilder) -> T) -> T; - /// Pushes children to the back of the builder's children + /// Pushes children to the back of the builder's children. For any entities that are + /// already a child of this one, this method does nothing. /// /// If the children were previously children of another parent, that parent's [`Children`] component /// will have those children removed from its list. Removing all children from a parent causes its @@ -739,4 +744,19 @@ mod tests { let child = world.spawn_empty().id(); world.spawn_empty().push_children(&[child]); } + + #[test] + fn push_children_idempotent() { + let mut world = World::new(); + let child = world.spawn_empty().id(); + let parent = world + .spawn_empty() + .push_children(&[child]) + .push_children(&[child]) + .id(); + + let mut query = world.query::<&Children>(); + let children = query.get(&world, parent).unwrap(); + assert_eq!(**children, [child]); + } }