Skip to content

Commit ef60d04

Browse files
authored
Sync UI children on Node addition (#20266)
# Objective For context: > Yeah it seems like it would be a `bevy_ui` issue, which just becomes easy to hit since `spawn_scene` is deferred. You end up with an invalid (has non-Node parent) Node until the scene is resolved and spawned. And it seems like the UI systems can’t handle the parent adding Node, making the child valid https://discord.com/channels/691052431525675048/1264881140007702558/1397443915900256338 - The UI layout systems do not take node addition into account when deciding to sync children - Multiple people have encountered this when evaluating the BSN draft PR which involves spawning scenes asynchronously. ## Solution - Added a check for `node.is_added()` alongside the `children.ui_children.is_changed()` check ## Testing - Added a new test case: `node_addition_should_sync_children` ---
1 parent 9445481 commit ef60d04

File tree

1 file changed

+26
-5
lines changed
  • crates/bevy_ui/src/layout

1 file changed

+26
-5
lines changed

crates/bevy_ui/src/layout/mod.rs

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ pub fn ui_layout_system(
7979
Option<&mut ContentSize>,
8080
Ref<ComputedNodeTarget>,
8181
)>,
82-
computed_node_query: Query<(Entity, Option<Ref<ChildOf>>), With<ComputedNode>>,
82+
computed_node_query: Query<(Entity, Ref<Node>, Option<Ref<ChildOf>>), With<ComputedNode>>,
8383
ui_children: UiChildren,
8484
mut node_update_query: Query<(
8585
&mut ComputedNode,
@@ -128,7 +128,7 @@ pub fn ui_layout_system(
128128

129129
computed_node_query
130130
.iter()
131-
.for_each(|(entity, maybe_child_of)| {
131+
.for_each(|(entity, node, maybe_child_of)| {
132132
if let Some(child_of) = maybe_child_of {
133133
// Note: This does not cover the case where a parent's Node component was removed.
134134
// Users are responsible for fixing hierarchies if they do that (it is not recommended).
@@ -141,7 +141,7 @@ with UI components as a child of an entity without UI components, your UI layout
141141
}
142142
}
143143

144-
if ui_children.is_changed(entity) {
144+
if node.is_added() || ui_children.is_changed(entity) {
145145
ui_surface.update_children(entity, ui_children.iter_ui_children(entity));
146146
}
147147
});
@@ -154,8 +154,8 @@ with UI components as a child of an entity without UI components, your UI layout
154154
);
155155

156156
// Re-sync changed children: avoid layout glitches caused by removed nodes that are still set as a child of another node
157-
computed_node_query.iter().for_each(|(entity, _)| {
158-
if ui_children.is_changed(entity) {
157+
computed_node_query.iter().for_each(|(entity, node, _)| {
158+
if node.is_added() || ui_children.is_changed(entity) {
159159
ui_surface.update_children(entity, ui_children.iter_ui_children(entity));
160160
}
161161
});
@@ -649,6 +649,27 @@ mod tests {
649649
assert_eq!(ui_surface.entity_to_taffy.len(), 1);
650650
}
651651

652+
#[test]
653+
fn node_addition_should_sync_children() {
654+
let (mut world, mut ui_schedule) = setup_ui_test_world();
655+
656+
// spawn an invalid UI root node
657+
let root_node = world.spawn(()).with_child(Node::default()).id();
658+
659+
ui_schedule.run(&mut world);
660+
661+
// fix the invalid root node by inserting a Node
662+
world.entity_mut(root_node).insert(Node::default());
663+
664+
ui_schedule.run(&mut world);
665+
666+
let ui_surface = world.resource_mut::<UiSurface>();
667+
let taffy_root = ui_surface.entity_to_taffy[&root_node];
668+
669+
// There should be one child of the root node after fixing it
670+
assert_eq!(ui_surface.taffy.child_count(taffy_root.id), 1);
671+
}
672+
652673
/// regression test for >=0.13.1 root node layouts
653674
/// ensure root nodes act like they are absolutely positioned
654675
/// without explicitly declaring it.

0 commit comments

Comments
 (0)