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

Fix overwriting of Node3D's local space transform #44803

Closed
wants to merge 1 commit into from

Conversation

mrushyendra
Copy link
Contributor

Currently, when the DIRTY_LOCAL flag is set and a new transform is supplied using set_transform, the new transform may be overwritten the next time it is read, because the transform is reset using the Node3D's previous rotation and scale, even though the new transform should supersede those.

This PR modifies when the DIRTY_LOCAL flag is set to prevent a transform applied using set_transform being overwritten by any previous calls that changed the node's rotation, translation or scale.

Closes #43130

Modifies when 'DIRTY_LOCAL' flag is set to prevent a transform applied
using `set_transform` to be overwritten by previous calls to change the
node's rotation, translation or scale.
@akien-mga akien-mga added bug cherrypick:3.x Considered for cherry-picking into a future 3.x release topic:core labels Dec 30, 2020
@akien-mga akien-mga added this to the 4.0 milestone Dec 30, 2020
@akien-mga akien-mga requested a review from reduz December 30, 2020 08:44
@akien-mga akien-mga requested a review from a team February 22, 2021 20:35
@Calinou
Copy link
Member

Calinou commented Jun 3, 2022

cc @lawnjelly
(not sure who would be the best person to review this, but pinging just in case)

@YuriSizov YuriSizov modified the milestones: 4.0, 4.1 Feb 12, 2023
@akien-mga akien-mga requested a review from lawnjelly June 16, 2023 15:25
@akien-mga
Copy link
Member

akien-mga commented Jun 16, 2023

Sorry for the delay reviewing this.

This looks like it's no longer applicable in master as the code has changed significantly:

void Node3D::set_transform(const Transform3D &p_transform) {
        ERR_THREAD_GUARD;
        data.local_transform = p_transform;
        _replace_dirty_mask(DIRTY_EULER_ROTATION_AND_SCALE); // Make rot/scale dirty.

Would be good to check if the original bug is still reproducible in latest 4.1 betas, and if so how this change should be adapted for it.

Edit: From a quick test in #43130 (comment), the issue may not be reproducible in master, but still is in 3.x. So a version of this PR for 3.x could be useful (if @lawnjelly can confirm it's the right approach).

@lawnjelly
Copy link
Member

Sorry I hadn't seen this before, I agree it does seem to make sense (for 3.x).

@akien-mga
Copy link
Member

Redone for 3.x as #78439.

@akien-mga akien-mga closed this Jun 19, 2023
@akien-mga akien-mga added archived and removed cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Jun 19, 2023
@YuriSizov YuriSizov removed this from the 4.x milestone Dec 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set transform ignored after rotation_degrees has been set
5 participants