From 43f2962ad5bf85369860822dcc967a98728f040f Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Sun, 20 Aug 2023 23:21:01 +0100 Subject: [PATCH 1/4] Adds a `Without` to the query used by `sync_simple_transforms` to retrieve entities orphaned since the last update. Without this filter, `sync_simple_transforms` will overwrite the `GlobalTransform` of entities that had their parent removed and then replaced in the same frame. --- crates/bevy_transform/src/systems.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index bf60b58922dfa..ec4fba493c016 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -21,7 +21,7 @@ pub fn sync_simple_transforms( Without, ), >, - Query<(Ref, &mut GlobalTransform), Without>, + Query<(Ref, &mut GlobalTransform), (Without, Without)>, )>, mut orphaned: RemovedComponents, ) { From 5588057003858407c020e443ebaf25e9bba69c3c Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Mon, 21 Aug 2023 13:46:34 +0100 Subject: [PATCH 2/4] Added regression test. --- crates/bevy_transform/src/systems.rs | 52 +++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index ec4fba493c016..529e246270ee6 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -183,7 +183,7 @@ mod test { use bevy_app::prelude::*; use bevy_ecs::prelude::*; use bevy_ecs::system::CommandQueue; - use bevy_math::vec3; + use bevy_math::{vec3, Vec3}; use bevy_tasks::{ComputeTaskPool, TaskPool}; use crate::components::{GlobalTransform, Transform}; @@ -481,4 +481,54 @@ mod test { app.update(); } + + #[test] + fn global_transform_should_not_be_overwritten_after_reparenting() { + let mut world = World::new(); + + // Create transform propagation schedule + let mut schedule = Schedule::default(); + schedule.add_systems((sync_simple_transforms, propagate_transforms)); + + // Spawn a `TransformBundle` entity with a local translation of `Vec3::ONE` + let mut spawn_transform_bundle = || { + world + .spawn(TransformBundle::from_transform( + Transform::from_translation(Vec3::ONE), + )) + .id() + }; + + // Spawn parent and child with the same transform + let parent = spawn_transform_bundle(); + let child = spawn_transform_bundle(); + world.entity_mut(parent).add_child(child); + + // Run schedule to propagate transforms + schedule.run(&mut world); + + let check_translation = |world: &World, entity, translation: Vec3| { + assert!(world + .entity(entity) + .get::() + .unwrap() + .translation() + .abs_diff_eq(translation, 0.001)) + }; + + // Child should be positioned relative to its parent + check_translation(&world, parent, Vec3::ONE); + check_translation(&world, child, 2. * Vec3::ONE); + + // Reparent child + world.entity_mut(child).remove_parent(); + world.entity_mut(parent).add_child(child); + + // Run schedule to propagate transforms + schedule.run(&mut world); + + // Translations should be unchanged after update + check_translation(&world, parent, Vec3::ONE); + check_translation(&world, child, 2. * Vec3::ONE); + } } From f6c7446c9f5039aa9957d978da80b14bb18c42a7 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Mon, 21 Aug 2023 14:07:45 +0100 Subject: [PATCH 3/4] test refactoring --- crates/bevy_transform/src/systems.rs | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index 529e246270ee6..035cae34d37eb 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -484,6 +484,7 @@ mod test { #[test] fn global_transform_should_not_be_overwritten_after_reparenting() { + let translation = Vec3::ONE; let mut world = World::new(); // Create transform propagation schedule @@ -494,12 +495,12 @@ mod test { let mut spawn_transform_bundle = || { world .spawn(TransformBundle::from_transform( - Transform::from_translation(Vec3::ONE), + Transform::from_translation(translation), )) .id() }; - // Spawn parent and child with the same transform + // Spawn parent and child with identical transform bundles let parent = spawn_transform_bundle(); let child = spawn_transform_bundle(); world.entity_mut(parent).add_child(child); @@ -507,19 +508,12 @@ mod test { // Run schedule to propagate transforms schedule.run(&mut world); - let check_translation = |world: &World, entity, translation: Vec3| { - assert!(world - .entity(entity) - .get::() - .unwrap() - .translation() - .abs_diff_eq(translation, 0.001)) - }; - // Child should be positioned relative to its parent - check_translation(&world, parent, Vec3::ONE); - check_translation(&world, child, 2. * Vec3::ONE); - + let parent_global_transform = *world.entity(parent).get::().unwrap(); + let child_global_transform = *world.entity(child).get::().unwrap(); + assert!(parent_global_transform.translation().abs_diff_eq(translation, 0.1)); + assert!(child_global_transform.translation().abs_diff_eq(2.* translation, 0.1)); + // Reparent child world.entity_mut(child).remove_parent(); world.entity_mut(parent).add_child(child); @@ -528,7 +522,7 @@ mod test { schedule.run(&mut world); // Translations should be unchanged after update - check_translation(&world, parent, Vec3::ONE); - check_translation(&world, child, 2. * Vec3::ONE); + assert_eq!(parent_global_transform, *world.entity(parent).get::().unwrap()); + assert_eq!(child_global_transform, *world.entity(child).get::().unwrap()); } } From f4ef24d778891626cf86767b884d2608e3a7330f Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Mon, 21 Aug 2023 15:26:25 +0100 Subject: [PATCH 4/4] cargo fmt --all --- crates/bevy_transform/src/systems.rs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index 035cae34d37eb..48d76fa14e1ab 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -511,9 +511,13 @@ mod test { // Child should be positioned relative to its parent let parent_global_transform = *world.entity(parent).get::().unwrap(); let child_global_transform = *world.entity(child).get::().unwrap(); - assert!(parent_global_transform.translation().abs_diff_eq(translation, 0.1)); - assert!(child_global_transform.translation().abs_diff_eq(2.* translation, 0.1)); - + assert!(parent_global_transform + .translation() + .abs_diff_eq(translation, 0.1)); + assert!(child_global_transform + .translation() + .abs_diff_eq(2. * translation, 0.1)); + // Reparent child world.entity_mut(child).remove_parent(); world.entity_mut(parent).add_child(child); @@ -522,7 +526,13 @@ mod test { schedule.run(&mut world); // Translations should be unchanged after update - assert_eq!(parent_global_transform, *world.entity(parent).get::().unwrap()); - assert_eq!(child_global_transform, *world.entity(child).get::().unwrap()); + assert_eq!( + parent_global_transform, + *world.entity(parent).get::().unwrap() + ); + assert_eq!( + child_global_transform, + *world.entity(child).get::().unwrap() + ); } }