From aa20565f75e839adb3f8846aa15fbdaeb73c79f7 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Mon, 28 Aug 2023 18:29:46 +0100 Subject: [PATCH] Add `Without` filter to `sync_simple_transforms`' orphaned entities query (#9518) # Objective `sync_simple_transforms` only checks for removed parents and doesn't filter for `Without`, so it overwrites the `GlobalTransform` of non-orphan entities that were orphaned and then reparented since the last update. Introduced by #7264 ## Solution Filter for `Without`. Fixes #9517, #9492 ## Changelog `sync_simple_transforms`: * Added a `Without` filter to the orphaned entities query. --- crates/bevy_transform/src/systems.rs | 58 +++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 2 deletions(-) diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index bf60b58922dfa..48d76fa14e1ab 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, ) { @@ -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,58 @@ mod test { app.update(); } + + #[test] + fn global_transform_should_not_be_overwritten_after_reparenting() { + let translation = Vec3::ONE; + 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(translation), + )) + .id() + }; + + // 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); + + // Run schedule to propagate transforms + schedule.run(&mut world); + + // 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)); + + // 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 + assert_eq!( + parent_global_transform, + *world.entity(parent).get::().unwrap() + ); + assert_eq!( + child_global_transform, + *world.entity(child).get::().unwrap() + ); + } }