From 3994ac1154844e44d8f957700074c706d08e7612 Mon Sep 17 00:00:00 2001 From: Brice DAVIER Date: Thu, 5 Aug 2021 20:48:24 +0200 Subject: [PATCH 1/3] Update EntityMut's location in push_children and insert_children --- crates/bevy_transform/src/hierarchy/child_builder.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/bevy_transform/src/hierarchy/child_builder.rs b/crates/bevy_transform/src/hierarchy/child_builder.rs index 9731c8c02b0e5..c924edf689c34 100644 --- a/crates/bevy_transform/src/hierarchy/child_builder.rs +++ b/crates/bevy_transform/src/hierarchy/child_builder.rs @@ -219,7 +219,7 @@ impl<'w> BuildWorldChildren for EntityMut<'w> { fn push_children(&mut self, children: &[Entity]) -> &mut Self { let parent = self.id(); { - // SAFE: parent entity is not modified + // SAFE: parent entity is not modified and its location is updated manually let world = unsafe { self.world_mut() }; for child in children.iter() { world @@ -227,6 +227,8 @@ impl<'w> BuildWorldChildren for EntityMut<'w> { // FIXME: don't erase the previous parent (see #1545) .insert_bundle((Parent(parent), PreviousParent(parent))); } + // Inserting a bundle in the children entities may change the parent entity's location + self.update_location(); } if let Some(mut children_component) = self.get_mut::() { children_component.0.extend(children.iter().cloned()); @@ -239,7 +241,7 @@ impl<'w> BuildWorldChildren for EntityMut<'w> { fn insert_children(&mut self, index: usize, children: &[Entity]) -> &mut Self { let parent = self.id(); { - // SAFE: parent entity is not modified + // SAFE: parent entity is not modified and its location is updated manually let world = unsafe { self.world_mut() }; for child in children.iter() { world @@ -247,6 +249,8 @@ impl<'w> BuildWorldChildren for EntityMut<'w> { // FIXME: don't erase the previous parent (see #1545) .insert_bundle((Parent(parent), PreviousParent(parent))); } + // Inserting a bundle in the children entities may change the parent entity's location + self.update_location(); } if let Some(mut children_component) = self.get_mut::() { From 16295dda068f1d2a23b3cfc11b05ac564cabec5a Mon Sep 17 00:00:00 2001 From: Brice DAVIER Date: Thu, 5 Aug 2021 21:20:24 +0200 Subject: [PATCH 2/3] Add regression test case --- crates/bevy_transform/src/hierarchy/child_builder.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/bevy_transform/src/hierarchy/child_builder.rs b/crates/bevy_transform/src/hierarchy/child_builder.rs index c924edf689c34..df5e5e288ddd7 100644 --- a/crates/bevy_transform/src/hierarchy/child_builder.rs +++ b/crates/bevy_transform/src/hierarchy/child_builder.rs @@ -475,4 +475,11 @@ mod tests { PreviousParent(parent) ); } + + #[test] + fn regression_push_children() { + let mut world = World::new(); + let child = world.spawn().id(); + world.spawn().push_children(&[child]); + } } From 9257fbc36f54e766f27a64f7e70692c64e12a9f7 Mon Sep 17 00:00:00 2001 From: Brice DAVIER Date: Sun, 8 Aug 2021 11:32:32 +0200 Subject: [PATCH 3/3] Improve comments and fn name --- crates/bevy_transform/src/hierarchy/child_builder.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_transform/src/hierarchy/child_builder.rs b/crates/bevy_transform/src/hierarchy/child_builder.rs index df5e5e288ddd7..71fc54aeb24ce 100644 --- a/crates/bevy_transform/src/hierarchy/child_builder.rs +++ b/crates/bevy_transform/src/hierarchy/child_builder.rs @@ -227,7 +227,7 @@ impl<'w> BuildWorldChildren for EntityMut<'w> { // FIXME: don't erase the previous parent (see #1545) .insert_bundle((Parent(parent), PreviousParent(parent))); } - // Inserting a bundle in the children entities may change the parent entity's location + // Inserting a bundle in the children entities may change the parent entity's location if they were of the same archetype self.update_location(); } if let Some(mut children_component) = self.get_mut::() { @@ -249,7 +249,7 @@ impl<'w> BuildWorldChildren for EntityMut<'w> { // FIXME: don't erase the previous parent (see #1545) .insert_bundle((Parent(parent), PreviousParent(parent))); } - // Inserting a bundle in the children entities may change the parent entity's location + // Inserting a bundle in the children entities may change the parent entity's location if they were of the same archetype self.update_location(); } @@ -477,7 +477,7 @@ mod tests { } #[test] - fn regression_push_children() { + fn regression_push_children_same_archetype() { let mut world = World::new(); let child = world.spawn().id(); world.spawn().push_children(&[child]);