Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The GlobalTransform did not get updated when Parent was removed #7263

Closed
vby opened this issue Jan 18, 2023 · 4 comments · Fixed by #7264
Closed

The GlobalTransform did not get updated when Parent was removed #7263

vby opened this issue Jan 18, 2023 · 4 comments · Fixed by #7264
Labels
A-Hierarchy Parent-child entity hierarchies A-Transform Translations, rotations and scales C-Bug An unexpected or incorrect behavior

Comments

@vby
Copy link

vby commented Jan 18, 2023

What problem does this solve or what need does it fill?

The GlobalTransform did not get updated (it should be) when Parent was removed.

What solution would you like?

  • Always recalculate the new GlobalTransform then set_if_neq.
if transform_changed {
    *global_transform = GlobalTransform::from(*transform);
} else {
    global_transform.set_if_neq(GlobalTransform::from(*transform));
}
  • Or use RemovedComponents<Parent>, which may have poor performance if there have so many entities without Transform.

  • Or use Removed change detection, but does not exist currently.

Query<(&Transform, &mut GlobalTransform, Removed<Parent>)>
if parent_removed || transform_changed {
  *global_transform = GlobalTransform::from(*transform);
}

What alternative(s) have you considered?

none

Additional context

none

@vby vby added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Jan 18, 2023
@nicopap
Copy link
Contributor

nicopap commented Jan 18, 2023

@nicopap nicopap closed this as completed Jan 18, 2023
@nicopap nicopap added A-Transform Translations, rotations and scales A-Hierarchy Parent-child entity hierarchies and removed S-Needs-Triage This issue needs to be labelled labels Jan 18, 2023
@nicopap nicopap reopened this Jan 18, 2023
@nicopap nicopap added C-Bug An unexpected or incorrect behavior and removed C-Feature A new feature, making something new possible labels Jan 18, 2023
@nicopap
Copy link
Contributor

nicopap commented Jan 18, 2023

I may have misunderstood the issue. Could you provide a minimal reproducible example or at least try with the last main commit? It might have been fixed with a recent commit.

@vby
Copy link
Author

vby commented Jan 18, 2023

I may have misunderstood the issue. Could you provide a minimal reproducible example or at least try with the last main commit? It might have been fixed with a recent commit.

Here is a simple test case for this.

#[test]
fn correct_parent_removed() {
    ComputeTaskPool::init(TaskPool::default);
    let mut world = World::default();

    let mut update_stage = SystemStage::parallel();
    update_stage.add_system(sync_simple_transforms);
    update_stage.add_system(propagate_transforms);

    let mut schedule = Schedule::default();
    schedule.add_stage(Update, update_stage);

    let (root, parent, child) = {
        let mut command_queue = CommandQueue::default();
        let mut commands = Commands::new(&mut command_queue, &world);
        let root = commands.spawn(TransformBundle::from_transform(Transform::from_xyz(1.0, 0.0, 0.0))).id();
        let parent = commands.spawn(TransformBundle::IDENTITY).id();
        let child = commands.spawn(TransformBundle::IDENTITY).id();
        commands.entity(parent).set_parent(root);
        commands.entity(child).set_parent(parent);
        command_queue.apply(&mut world);
        schedule.run(&mut world);
        (root, parent, child)
    };

    assert_ne!(
        world.get::<GlobalTransform>(parent).unwrap(),
        &GlobalTransform::from(Transform::IDENTITY)
    );

    // Remove parent of `parent`
    {
        let mut command_queue = CommandQueue::default();
        let mut commands = Commands::new(&mut command_queue, &world);
        commands.entity(parent).remove_parent();
        command_queue.apply(&mut world);
        schedule.run(&mut world);
    }

    assert_eq!(
        world.get::<GlobalTransform>(parent).unwrap(),
        &GlobalTransform::from(Transform::IDENTITY)
    );
}

@nicopap
Copy link
Contributor

nicopap commented Jan 18, 2023

yeah I can reproduce. I've a fix working, I'll open a PR shortly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Hierarchy Parent-child entity hierarchies A-Transform Translations, rotations and scales C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants