[Merged by Bors] - Fix inconsistent children removal behavior

[oceantume]

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.

[oceantume]

I updated this PR to reflect the latest discussion in the referenced issue.

This is ready for a review, but before merging I will need to go over the bevy_ui flex system and bevy_transform propagation system to make sure that they work expectedly, and possibly add RemovedComponent<Children> if needed. However if they don't work, that means they had issues before as well (because I only made an already-existing behavior be consistent across all commands) so it's not like they're broken by this PR.

[rparrett]

I think that there shouldn't be an issue with transform propagation, but there there's definitely an issue with UI.

Tested with this, derived from the ui example.

Pressing space moves the yellow box from the left to the right side of the screen. With this PR, the green container on the left side isn't properly updated and stays at its previous size despite having been emptied.

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_startup_system(setup)
        .add_system(detach)
        .run();
}

#[derive(Component)]
struct Destination;

#[derive(Component)]
struct Source;

fn detach(
    mut commands: Commands,
    input: Res<Input<KeyCode>>,
    query: Query<(Entity, &Children), With<Source>>,
    dest_query: Query<Entity, With<Destination>>,
) {
    if input.just_pressed(KeyCode::Space) {
        let (source_entity, children) = query.single();
        let dest_entity = dest_query.single();

        commands.entity(source_entity).remove_children(children);
        commands.entity(dest_entity).push_children(children);
    }
}

fn setup(mut commands: Commands) {
    commands.spawn_bundle(Camera2dBundle::default());

    commands
        .spawn_bundle(NodeBundle {
            style: Style {
                size: Size::new(Val::Percent(100.0), Val::Percent(100.0)),
                justify_content: JustifyContent::SpaceBetween,
                ..default()
            },
            color: Color::NONE.into(),
            ..default()
        })
        .with_children(|parent| {
            parent
                .spawn_bundle(NodeBundle {
                    style: Style {
                        size: Size::new(Val::Percent(50.0), Val::Undefined),
                        ..default()
                    },
                    color: Color::GREEN.into(),
                    ..default()
                })
                .insert(Source)
                .with_children(|parent| {
                    parent.spawn_bundle(NodeBundle {
                        style: Style {
                            size: Size::new(Val::Percent(100.0), Val::Px(100.0)),
                            ..default()
                        },
                        color: Color::YELLOW.into(),
                        ..default()
                    });
                });

            parent
                .spawn_bundle(NodeBundle {
                    style: Style {
                        size: Size::new(Val::Percent(50.0), Val::Undefined),
                        ..default()
                    },
                    color: Color::RED.into(),
                    ..default()
                })
                .insert(Destination);
        });
}

[irate-devil]

The issue with UI is that the flex_node_system does not take removed Children components into account.
Adding removed_children: RemovedComponents<Children> as a system parameter and then doing

for entity in &removed_children {
    if let Some(&node) = flex_surface.entity_to_taffy.get(&entity) {
        flex_surface.taffy.set_children(node, &[]).unwrap();
    }
}

seems to solve the issue.

[oceantume]

My latest commit should fix the UI flex system. Thanks for the info 👍

@devil-ira Have you tested the GlobalTransform propagation? I'm not sure what the expectation is exactly there, but I can see that the propagation code doesn't explicitly handle RemovedComponent<Children>. It's possible that it works implicitly but I'm not sure since it has a Changed<Children> query. If you haven't tried it I will test it out.

[irate-devil]

The issue with UI is that taffy has a copy of the hierarchy that needs to be kept up to date and that children can affect the parent. Neither of those apply to transforms, so it should be good.

[irate-devil]

LGTM aside from one thing.

[alice-i-cecile]

bors r+

[bors]

Pull request successfully merged into main.

Build succeeded:

• build-and-install-on-iOS
• build-android
• build (macos-latest)
• build (ubuntu-latest)
• build-wasm
• build (windows-latest)
• check-compiles
• check-doc
• check-missing-examples-in-docs
• ci
• markdownlint
• run-examples
• run-examples-on-wasm
• run-examples-on-windows-dx12

[hmeine]

I am not sure this is the right place to give feedback, but... I have run into a problem that I can certainly solved myself, but feels like I should mention it:

• I have been using a query_stacks: Query<(Entity, &CardStack, &Children, &Transform)> to query CardStack entities and their children at the same time. Obviously, this is no longer supported, so maybe it should be mentioned in the migration guide as well.
• More importantly, the way it fails feels rather strange: Initially, that query still works, because entities still seem to have (empty) Children by default! Hence, my game runs until I have added and removed children, and then suddenly crashes. In bevy-inspector-egui, I can see a diference between "No children" (after removal) and empty "Children" (initially).