Panic after spawning NodeComponents in POST_UPDATE stage

[alec-deason]

If I spawn a NodeComponent bundle from a system running in the POST_UPDATE stage I get a panic on this line: https://github.com/bevyengine/bevy/blob/master/crates/bevy_ui/src/flex/mod.rs#L135

The same system running in UPDATE doesn't trigger a panic. In this example, changing line 6 to add_system(node_adder.system()) makes the panic disappear.

 use bevy::prelude::*;

 fn main() {
     App::build()
         .add_default_plugins()
         .add_system_to_stage(stage::POST_UPDATE, node_adder.system())
         .add_resource(DidSpawn(false))
         .add_startup_system(setup.system())
         .run();
 }

 struct DidSpawn(bool);

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

 fn node_adder(
     mut commands: Commands,
     mut materials: ResMut<Assets<ColorMaterial>>,
     mut did_spawn: ResMut<DidSpawn>,
 ) {
     if !did_spawn.0 {
         did_spawn.0 = true;
         commands
             .spawn(NodeComponents {
                 style: Style {
                     size: Size::new(Val::Px(200.0), Val::Percent(100.0)),
                     border: Rect::all(Val::Px(2.0)),
                     ..Default::default()
                 },
                 material: materials.add(Color::rgb(0.65, 0.65, 0.65).into()),
                 ..Default::default()
             });
     }
 }

[scristall]

I ran into this issue too and dug into it a bit. Here's what I found out.

bevy/crates/bevy_ui/src/flex/mod.rs

Lines 193 to 201 in 871b47f

	if scale_factor_reader.latest(&scale_factor_events).is_some() {
	update_changed(
	&mut *flex_surface,
	logical_to_physical_factor,
	full_node_query,
	);
	} else {
	update_changed(&mut *flex_surface, logical_to_physical_factor, node_query);
	}

update_changed is being called in the else clause, with the node_query argument, which is filtered based on Changed<Style>. When the NodeBundle is added at the POST_UPDATE stage, this node_query appears to be empty which later on causes a panic on this line due to the entity being missing from self.entity_to_stretch:

bevy/crates/bevy_ui/src/flex/mod.rs

Lines 139 to 141 in 871b47f

	let child_nodes = children
	.map(|e| *self.entity_to_stretch.get(&e).unwrap())
	.collect::<Vec<stretch::node::Node>>();

Changing the first section of code to always call update_changed using full_node_query does appear to fix the issue, for what that's worth, though I know that's not a viable solution. It appears to be because the full_node_query arg does not filter based on Changed and thus populates the entities appropriately.

I'm guessing this is related to #68 ?

[alec-deason]

Yeah, fixing #68 likely would fix this. I think it's really related to the larger problem of how bevy manages and documents system dependencies.

[cart]

My default answer here is that you shouldn't spawn a game entity in POST_UPDATE. Bevy's stages each have different assumptions. UPDATE is where "game logic" goes. POST_UPDATE is where "responses to game logic" go. Spawning a NodeBundle entity is "game logic". Trying to do that in POST_UPDATE means we're skipping the relevant "responses to game logic" for that entity.

Similar: #1102 (comment)

[alec-deason]

Yeah, I've come to agree with that since I created this issue. The only reason I think it might still be an issue is the lack of a clear error when it happens. But I also don't have a good suggestion for how to get a better error.

[cart]

Yeah the problem is that it will fail differently for each context. Handling each specific error case for each stage in code seems like a maintenance / performance nightmare.

[cart]

I think this probably needs to be handled with better documentation for each stage.

[alice-i-cecile]

Closing as #68 is now resolved.