Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Despawned Node Entity Continues to Affect Layout #351

Closed
ncallaway opened this issue Aug 26, 2020 · 3 comments
Closed

Despawned Node Entity Continues to Affect Layout #351

ncallaway opened this issue Aug 26, 2020 · 3 comments
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior

Comments

@ncallaway
Copy link
Contributor

An UI Node that is despawned continues to impact the layout of it's parent Node as if it were still present. I think it's likely that the despawn does not cause the underlying Stretch node to be removed.

Example Reproduction

use bevy::prelude::*;

/// This example illustrates how to create a button that changes color and text based on its interaction state.
fn main() {
    App::build()
        .add_default_plugins()
        .init_resource::<ButtonMaterials>()
        .init_resource::<ButtonCount>()
        .add_startup_system(setup.system())
        .add_system(button_system.system())
        .run();
}

fn button_system(
    mut commands: Commands,
    button_materials: Res<ButtonMaterials>,
    asset_server: Res<AssetServer>,
    mut button_count: ResMut<ButtonCount>,
    mut interaction_query: Query<(
        Entity,
        &Button,
        Mutated<Interaction>,
        &mut Handle<ColorMaterial>,
        &Children,
    )>,
    mut root_query: Query<(Entity, &Root, &Children)>,
) {
    for (e, _, interaction, mut material, _) in &mut interaction_query.iter() {
        match *interaction {
            Interaction::Clicked => {
                let font = asset_server
                    .get_handle("assets/fonts/FiraMono-Medium.ttf")
                    .unwrap();

                // we're going to remvoe the button
                commands.despawn_recursive(e);

                // spawn a new button in the same spot
                for (parent, _, children) in &mut root_query.iter() {
                    println!("Now have: {} children", children.0.len());
                    let e = spawn_button_node(
                        &mut commands,
                        &button_materials,
                        font,
                        &(format!("Button {}", button_count.0))[..],
                    );
                    button_count.0 = button_count.0 + 1;
                    commands.push_children(parent, &[e]);
                    break;
                }
            }
            Interaction::Hovered => {
                *material = button_materials.hovered;
            }
            Interaction::None => {
                *material = button_materials.normal;
            }
        }
    }
}

fn setup(
    mut commands: Commands,
    asset_server: Res<AssetServer>,
    button_materials: Res<ButtonMaterials>,
) {
    let font = asset_server
        .load("assets/fonts/FiraMono-Medium.ttf")
        .unwrap();

    let root = Entity::new();
    commands
        .spawn(UiCameraComponents::default())
        .spawn_as_entity(
            root,
            NodeComponents {
                style: Style {
                    size: Size::new(Val::Percent(100.0), Val::Percent(100.0)),
                    flex_direction: FlexDirection::ColumnReverse,
                    justify_content: JustifyContent::FlexStart,
                    align_items: AlignItems::Center,
                    ..Default::default()
                },
                ..Default::default()
            },
        )
        .with(Root);
    let button = spawn_button_node(&mut commands, &button_materials, font, "First button");
    commands.push_children(root, &[button]);
}

struct ButtonMaterials {
    normal: Handle<ColorMaterial>,
    hovered: Handle<ColorMaterial>,
    pressed: Handle<ColorMaterial>,
}

impl FromResources for ButtonMaterials {
    fn from_resources(resources: &Resources) -> Self {
        let mut materials = resources.get_mut::<Assets<ColorMaterial>>().unwrap();
        ButtonMaterials {
            normal: materials.add(Color::rgb(0.02, 0.02, 0.02).into()),
            hovered: materials.add(Color::rgb(0.05, 0.05, 0.05).into()),
            pressed: materials.add(Color::rgb(0.1, 0.5, 0.1).into()),
        }
    }
}

#[derive(Default)]
struct ButtonCount(u32);

struct Root;

fn spawn_button_node(
    commands: &mut Commands,
    mats: &Res<ButtonMaterials>,
    font: Handle<Font>,
    label: &str,
) -> Entity {
    let e = Entity::new();
    commands
        .spawn_as_entity(
            e,
            ButtonComponents {
                style: Style {
                    size: Size::new(Val::Px(300.0), Val::Px(65.0)),
                    // center button
                    margin: Rect::all(Val::Auto),
                    // horizontally center child text
                    justify_content: JustifyContent::Center,
                    // vertically center child text
                    align_items: AlignItems::Center,
                    ..Default::default()
                },
                material: mats.normal,
                ..Default::default()
            },
        )
        .with_children(|parent| {
            parent.spawn(TextComponents {
                text: Text {
                    value: label.to_string(),
                    font: font,
                    style: TextStyle {
                        font_size: 28.0,
                        color: Color::rgb(0.8, 0.8, 0.8),
                    },
                },
                ..Default::default()
            });
        });

    e
}

In this example everytime the button is clicked we despawn the existing button, and then spawn a new button inside the same container. My expectation is that since the container only ever holds one child, that child remains in the same position.

Demo

despawn-layout

For comparison, this is the behaviour without despawning the button

no-despawn-layout

@ncallaway
Copy link
Contributor Author

Suggested labels: bug | ui

@karroffel karroffel added C-Bug An unexpected or incorrect behavior A-UI Graphical user interfaces, styles, layouts, and widgets labels Aug 26, 2020
@Cupnfish
Copy link
Contributor

Cupnfish commented Sep 22, 2020

In the new version of bevy, I’ve replicated your code, and it’s working exactly as you’d expect.
I simplified a few places and changed the font file because I don’t have that font asset of yours. I think this bug has been fixed.

use bevy::prelude::*;

/// This example illustrates how to create a button that changes color and text based on its interaction state.
fn main() {
    App::build()
        .add_default_plugins()
        .init_resource::<ButtonMaterials>()
        .init_resource::<ButtonCount>()
        .add_startup_system(setup.system())
        .add_system(button_system.system())
        .run();
}

fn button_system(
    mut commands: Commands,
    button_materials: Res<ButtonMaterials>,
    mut button_count: ResMut<ButtonCount>,
    asset_server: Res<AssetServer>,
    mut interaction_query: Query<(
        Entity,
        &Button,
        Mutated<Interaction>,
        &mut Handle<ColorMaterial>,
        &Children,
    )>,
    mut root_query: Query<(Entity, &Root, &Children)>,
) {
    for (_, _, interaction, mut material, _) in &mut interaction_query.iter() {
        match *interaction {
            Interaction::Clicked => {
                // we're going to remvoe the button
                // commands.despawn_recursive(e);
                let font = asset_server.get_handle("assets/fonts/FiraCode-Light.ttf").unwrap_or_default();
                // spawn a new button in the same spot
                for (parent, _, children) in &mut root_query.iter() {
                    println!("Now have: {} children", children.0.len());
                    let e = spawn_button_node(
                        &mut commands,
                        font,
                        &button_materials,
                        &(format!("Button {}", button_count.0))[..],
                    );
                    button_count.0 = button_count.0 + 1;
                    commands.push_children(parent, &[e]);
                    break;
                }
            }
            Interaction::Hovered => {
                *material = button_materials.hovered;
            }
            Interaction::None => {
                *material = button_materials.normal;
            }
        }
    }
}

fn setup(
    mut commands: Commands,
    mut font_manager:ResMut<Assets<Font>>,
    asset_server: Res<AssetServer>,
    button_materials: Res<ButtonMaterials>,
) {
    let font = asset_server
    .load_sync(&mut font_manager,"assets/fonts/FiraCode-Light.ttf")
    .unwrap();

    let root = commands
        .spawn(UiCameraComponents::default())
        .spawn(NodeComponents {
                style: Style {
                    size: Size::new(Val::Percent(100.0), Val::Percent(100.0)),
                    flex_direction: FlexDirection::ColumnReverse,
                    justify_content: JustifyContent::FlexStart,
                    align_items: AlignItems::Center,
                    ..Default::default()
                },
                ..Default::default()
            }
        )
        .with(Root{})
        .current_entity().unwrap();

    let button = spawn_button_node(&mut commands, font,&button_materials,  "First button");
    commands.push_children(root, &[button]);
}

struct ButtonMaterials {
    normal: Handle<ColorMaterial>,
    hovered: Handle<ColorMaterial>,
}

impl FromResources for ButtonMaterials {
    fn from_resources(resources: &Resources) -> Self {
        let mut materials = resources.get_mut::<Assets<ColorMaterial>>().unwrap();
        ButtonMaterials {
            normal: materials.add(Color::rgb(0.02, 0.02, 0.02).into()),
            hovered: materials.add(Color::rgb(0.05, 0.05, 0.05).into()),
        }
    }
}

#[derive(Default)]
struct ButtonCount(u32);

struct Root;

fn spawn_button_node(
    commands: &mut Commands,
    font: Handle<Font>,
    mats: &Res<ButtonMaterials>,
    label: &str,
) -> Entity {
    commands
        .spawn(ButtonComponents {
                style: Style {
                    size: Size::new(Val::Px(300.0), Val::Px(65.0)),
                    // center button
                    margin: Rect::all(Val::Auto),
                    // horizontally center child text
                    justify_content: JustifyContent::Center,
                    // vertically center child text
                    align_items: AlignItems::Center,
                    ..Default::default()
                },
                material: mats.normal,
                ..Default::default()
            }
        )
        .with_children(|parent| {
            parent.spawn(TextComponents {
                text: Text {
                    value: label.to_string(),
                    style: TextStyle {
                        font_size: 28.0,
                        color: Color::rgb(1.0, 0.1, 0.1),
                    },
                    font:font,
                },
                ..Default::default()
            });
        })
        .current_entity().unwrap()
}

2
3

@alice-i-cecile
Copy link
Member

Closed by #386.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

No branches or pull requests

4 participants