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

Adding just a Node component to an empty entity causes a panic #9615

Closed
SludgePhD opened this issue Aug 29, 2023 · 6 comments · Fixed by #12213
Closed

Adding just a Node component to an empty entity causes a panic #9615

SludgePhD opened this issue Aug 29, 2023 · 6 comments · Fixed by #12213
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior P-Crash A sudden unexpected crash

Comments

@SludgePhD
Copy link
Contributor

Bevy version

0.11

What you did

Spawned an entity with a SpatialBundle and added a Node component to it.

What went wrong

Since this can easily be done from an editor or a script, this shouldn't cause a crash, but it caused a panic inside bevy:

thread 'Compute Task Pool (8)' panicked at 'called `Option::unwrap()` on a `None` value', /home/sludge/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_ui-0.11.0/src/layout/mod.rs:169:52
@SludgePhD SludgePhD added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Aug 29, 2023
@alice-i-cecile alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets and removed S-Needs-Triage This issue needs to be labelled labels Aug 29, 2023
@bushrat011899
Copy link
Contributor

Can confirm this harness will reproduce this behaviour on the main branch:

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .run();
}

fn setup(mut commands: Commands) {
    commands.spawn((
        SpatialBundle::default(),
        Node::default(),
    ));
}

Looks like the ui_layout_system finds an Entity with the query root_node_query: Query<Entity, (With<Node>, Without<Parent>)> that breaks the assumptions made for this line.

This indicates that the Entity found in root_node_query was never added to the UiSurface. Scrolling up to here, it's clear that the Entity is only added to the surface if it has a Style component.

Modifying my test to add a default Style component removes the error and the app runs as expected:

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .run();
}

fn setup(mut commands: Commands) {
    commands.spawn((
        SpatialBundle::default(),
        Node::default(),
        Style::default(),
    ));
}

My question for someone more familiar with Bevy's UI system would then be: is it intended that a Node should exist on an Entity even if it does not have a Style component?

@nicopap
Copy link
Contributor

nicopap commented Aug 29, 2023

You are supposed to use NodeBunlde for UI. But I don't think a panic is an OK thing to do. In a loading scenario, it is expected that part of the UI might be ready while others are still loading. Panicking unconditionally makes bevy_ui pretty much unusable in such a common scenario.

@nicopap nicopap added the P-Crash A sudden unexpected crash label Aug 29, 2023
@ickshonpe
Copy link
Contributor

ickshonpe commented Aug 29, 2023

My question for someone more familiar with Bevy's UI system would then be: is it intended that a Node should exist on an Entity even if it does not have a Style component?

Ideally yes. Users might want to write their own layout system but use Bevy UI's rendering.

@SludgePhD
Copy link
Contributor Author

A panic also happens when spawning an entity without Node or Style as a child of a UI node. It seems to me that all UI systems probably need to have all of their .unwrap()s reviewed.

@nicopap
Copy link
Contributor

nicopap commented Sep 1, 2023

@SludgePhD the specific issue of panicking on non-UI child of UI should be fixed by #9621

@nicoburns
Copy link
Contributor

My question for someone more familiar with Bevy's UI system would then be: is it intended that a Node should exist on an Entity even if it does not have a Style component?

This seems like it ought to be allowed to me. The UI system can set the entity's style to Style::default() in the Taffy tree if it doesn't have a Style component.

github-merge-queue bot pushed a commit that referenced this issue Feb 29, 2024
# Objective

- Fixes #10826
- Fixes #9615

## Solution

- Early-out when components are missing.
mockersf pushed a commit that referenced this issue Feb 29, 2024
# Objective

- Fixes #10826
- Fixes #9615

## Solution

- Early-out when components are missing.
spectria-limina pushed a commit to spectria-limina/bevy that referenced this issue Mar 9, 2024
# Objective

- Fixes bevyengine#10826
- Fixes bevyengine#9615

## Solution

- Early-out when components are missing.
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 P-Crash A sudden unexpected crash
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants