Fix inconsistent children removal behavior (#6017)

# Objective

Fixes #6010

## Solution

As discussed in #6010, this makes it so the `Children` component is removed from the entity whenever all of its children are removed. The behavior is now consistent between all of the commands that may remove children from a parent, and this is tested via two new test functions (one for world functions and one for commands).

Documentation was also added to `insert_children`, `push_children`, `add_child` and `remove_children` commands to make this behavior clearer for users.

## Changelog

- Fixed `Children` component not getting removed from entity when all its children are moved to a new parent.

## Migration Guide

- Queries with `Changed<Children>` will no longer match entities that had all of their children removed using `remove_children`.
- `RemovedComponents<Children>` will now contain entities that had all of their children remove using `remove_children`.

Author: oceantume

crates/bevy_hierarchy/src/child_builder.rs

@@ -77,10 +77,15 @@ fn remove_children(parent: Entity, children: &[Entity], world: &mut World) {
     }
     push_events(world, events);
 
-    if let Some(mut parent_children) = world.get_mut::<Children>(parent) {
+    let mut parent = world.entity_mut(parent);
+    if let Some(mut parent_children) = parent.get_mut::<Children>() {
         parent_children
             .0
             .retain(|parent_child| !children.contains(parent_child));
+
+        if parent_children.is_empty() {
+            parent.remove::<Children>();
+        }
     }
 }
 
@@ -258,12 +263,26 @@ pub trait BuildChildren {
     /// ```
     fn add_children<T>(&mut self, f: impl FnOnce(&mut ChildBuilder) -> T) -> T;
     /// Pushes children to the back of the builder's children
+    ///
+    /// 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
+    /// [`Children`] component to be removed from the entity.
     fn push_children(&mut self, children: &[Entity]) -> &mut Self;
     /// Inserts children at the given index
+    ///
+    /// 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
+    /// [`Children`] component to be removed from the entity.
     fn insert_children(&mut self, index: usize, children: &[Entity]) -> &mut Self;
     /// Removes the given children
+    ///
+    /// Removing all children from a parent causes its [`Children`] component to be removed from the entity.
     fn remove_children(&mut self, children: &[Entity]) -> &mut Self;
     /// Adds a single child
+    ///
+    /// 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
+    /// [`Children`] component to be removed from the entity.
     fn add_child(&mut self, child: Entity) -> &mut Self;
 }
 
@@ -672,6 +691,96 @@ mod tests {
         assert!(world.get::<Parent>(child4).is_none());
     }
 
+    /// Tests what happens when all children are removed from a parent using world functions
+    #[test]
+    fn children_removed_when_empty_world() {
+        let mut world = World::default();
+        let entities = world
+            .spawn_batch(vec![(C(1),), (C(2),), (C(3),)])
+            .collect::<Vec<Entity>>();
+
+        let parent1 = entities[0];
+        let parent2 = entities[1];
+        let child = entities[2];
+
+        // push child into parent1
+        world.entity_mut(parent1).push_children(&[child]);
+        assert_eq!(
+            world.get::<Children>(parent1).unwrap().0.as_slice(),
+            &[child]
+        );
+
+        // move only child from parent1 with `push_children`
+        world.entity_mut(parent2).push_children(&[child]);
+        assert!(world.get::<Children>(parent1).is_none());
+
+        // move only child from parent2 with `insert_children`
+        world.entity_mut(parent1).insert_children(0, &[child]);
+        assert!(world.get::<Children>(parent2).is_none());
+
+        // remove only child from parent1 with `remove_children`
+        world.entity_mut(parent1).remove_children(&[child]);
+        assert!(world.get::<Children>(parent1).is_none());
+    }
+
+    /// Tests what happens when all children are removed form a parent using commands
+    #[test]
+    fn children_removed_when_empty_commands() {
+        let mut world = World::default();
+        let entities = world
+            .spawn_batch(vec![(C(1),), (C(2),), (C(3),)])
+            .collect::<Vec<Entity>>();
+
+        let parent1 = entities[0];
+        let parent2 = entities[1];
+        let child = entities[2];
+
+        let mut queue = CommandQueue::default();
+
+        // push child into parent1
+        {
+            let mut commands = Commands::new(&mut queue, &world);
+            commands.entity(parent1).push_children(&[child]);
+            queue.apply(&mut world);
+        }
+        assert_eq!(
+            world.get::<Children>(parent1).unwrap().0.as_slice(),
+            &[child]
+        );
+
+        // move only child from parent1 with `push_children`
+        {
+            let mut commands = Commands::new(&mut queue, &world);
+            commands.entity(parent2).push_children(&[child]);
+            queue.apply(&mut world);
+        }
+        assert!(world.get::<Children>(parent1).is_none());
+
+        // move only child from parent2 with `insert_children`
+        {
+            let mut commands = Commands::new(&mut queue, &world);
+            commands.entity(parent1).insert_children(0, &[child]);
+            queue.apply(&mut world);
+        }
+        assert!(world.get::<Children>(parent2).is_none());
+
+        // move only child from parent1 with `add_child`
+        {
+            let mut commands = Commands::new(&mut queue, &world);
+            commands.entity(parent2).add_child(child);
+            queue.apply(&mut world);
+        }
+        assert!(world.get::<Children>(parent1).is_none());
+
+        // remove only child from parent2 with `remove_children`
+        {
+            let mut commands = Commands::new(&mut queue, &world);
+            commands.entity(parent2).remove_children(&[child]);
+            queue.apply(&mut world);
+        }
+        assert!(world.get::<Children>(parent2).is_none());
+    }
+
     #[test]
     fn regression_push_children_same_archetype() {
         let mut world = World::new();

crates/bevy_ui/src/flex/mod.rs

@@ -130,6 +130,13 @@ without UI components as a child of an entity with UI components, results may be
             .unwrap();
     }
 
+    /// Removes children from the entity's taffy node if it exists. Does nothing otherwise.
+    pub fn try_remove_children(&mut self, entity: Entity) {
+        if let Some(taffy_node) = self.entity_to_taffy.get(&entity) {
+            self.taffy.set_children(*taffy_node, &[]).unwrap();
+        }
+    }
+
     pub fn update_window(&mut self, window: &Window) {
         let taffy = &mut self.taffy;
         let node = self.window_nodes.entry(window.id()).or_insert_with(|| {
@@ -216,6 +223,7 @@ pub fn flex_node_system(
         (With<Node>, Changed<CalculatedSize>),
     >,
     children_query: Query<(Entity, &Children), (With<Node>, Changed<Children>)>,
+    removed_children: RemovedComponents<Children>,
     mut node_transform_query: Query<(Entity, &mut Node, &mut Transform, Option<&Parent>)>,
     removed_nodes: RemovedComponents<Node>,
 ) {
@@ -262,7 +270,10 @@ pub fn flex_node_system(
         flex_surface.set_window_children(primary_window.id(), root_node_query.iter());
     }
 
-    // update children
+    // update and remove children
+    for entity in &removed_children {
+        flex_surface.try_remove_children(entity);
+    }
     for (entity, children) in &children_query {
         flex_surface.update_children(entity, children);
     }