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

Keep 2D items global transform the same when set to top level #87316

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ryevdokimov
Copy link
Contributor

Fixes: #76205

This solution might be trying to kill the fly with a sledgehammer, but it seemed the most intuitive and keeps in line with what 3D does.

Otherwise, depending on the transform of the parent, the child node being set to top level could result in it appearing and being positioned in an unexpectedly different way. I can't think of any scenarios where this is the desired result.

Before:

2024-01-17.19-08-10.mp4

After:

2024-01-17.19-04-19.mp4

@YuriSizov
Copy link
Contributor

Do we still need this, or was the other PR sufficient?

@ryevdokimov
Copy link
Contributor Author

It’s up to the Godot team, the other PR does solve the problem while keeping the functionality the same, but I personally think the functionality is strange.

In 3D the world coordinates and transforms don’t change when you set top level. In 2D it does which seems undesirable. This PR makes the behavior between 3D and 2D the same.

@lawnjelly
Copy link
Member

The current 2D behaviour is probably the same in 3.x. It makes sense, by setting to toplevel, you are removing the parent transform, so you would expect it to move.

If anything, the behaviour in 3D (if as you say it stays in position) would be more "unexpected", but if there is a difference it likely occurs because in 3D the global transform is set and used directly in the server (and presumably is dirty in the server), whereas for 2D the global transform is recalculated through the hierarchy in the server on each render.

@ryevdokimov
Copy link
Contributor Author

ryevdokimov commented Jan 23, 2024

Just to play devil's advocate, should I expect it to move if I reparent a child node of a child of root, to root? If I right-click the child node and select 'Reparent'. It gives me an option change the global transform, where maintaining it is the default (at least on my editor where I've never touched this setting). Additionally, if you drag to reparent, you aren't even prompted, maintaining the global transform is forced.

This can't both be the expected behavior, right?

2024-01-23.07-25-15.mp4

image

@KoBeWi
Copy link
Member

KoBeWi commented Jan 23, 2024

Additionally, if you drag to reparent, you aren't even prompted, maintaining the global transform is forced.

You can hold Shift to bypass that.

@ryevdokimov
Copy link
Contributor Author

ryevdokimov commented Jan 23, 2024

Additionally, if you drag to reparent, you aren't even prompted, maintaining the global transform is forced.

You can hold Shift to bypass that.

Oh cool, I did not know that, thanks. 😅

But my point still stands, in both circumstances maintaining the global transform is the "default" behavior. If you all accept that changing a node to top-level doesn't have to be consistent with the default of reparenting, and additionally don't have that option that's fine with me, and I can close this.

@AThousandShips, this PR does what the title says as alternative solution to the original issue.

@AThousandShips
Copy link
Member

(I missed the conversation above, hence deleted my comment)

@ryevdokimov
Copy link
Contributor Author

I suppose what you could also do is just flip the default behavior of reparenting. Let me know if that's something that makes sense to do.

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.

Toggling top-level does not update 2D editor viewport
6 participants