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

Expose global transform properties in Node2D, Node3D and Control #90764

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

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Apr 16, 2024

This can be used to view and assign global transform in the inspector. This is useful to quickly move a node back to the origin regardless of its node hierarchy, or view its current global transform in the live scene debugger.

The following properties are now exposed:

  • Node2D:
    • Global Position
    • Global Rotation
    • Global Scale
    • Global Skew
  • Node3D:
    • Global Position
    • Global Rotation
    • Global Basis
  • Control:
    • Global Position

The basic functionality seems to work OK from my testing. However, there's an issue with this error being spammed about 162 times when you open the editor, or even when you use --doctool:

ERROR: Condition "!is_inside_tree()" is true. Returning: Transform3D()
   at: get_global_transform (./scene/3d/node_3d.cpp:346)

This does not occur with Node2D and Control.

Preview

Subgroups are collapsed by default. Here, I expanded them to take the screenshot.

Node2D Node3D Control
Screenshot_20240417_154403 image Screenshot_20240417_154534

@Calinou Calinou requested review from a team as code owners April 16, 2024 16:54
@Calinou Calinou added this to the 4.x milestone Apr 16, 2024
@AdriaandeJongh
Copy link
Contributor

Could you post a screenshot of how the inspector looks with this change?

I can also imagine a toggle icon somewhere, that toggles the UI between global and local transform values.

@Mickeon
Copy link
Contributor

Mickeon commented Apr 16, 2024

Having both Local and Global Transform always visible in the Inspector at the same time sounds like trouble.

@KoBeWi
Copy link
Member

KoBeWi commented Apr 16, 2024

  • That's doubling the properties:
    image
    Maybe globals could be in their own Global sub-section?

  • If this is going to be exposed, the description needs improvement.
    image

  • Changing global rotation (2D) will modify global scale and you can't even revert it:

godot.windows.editor.dev.x86_64_Hsmada8uD0.mp4

I know it's expected, but we'll likely get bug reports about it. Maybe it should be noted in the documentation at least.

  • Global rotation in 3D doesn't have sliders:
    image
  • When you change rotation mode to Quaternion, there is no global one:
    image
  • There is no global scale.

@Calinou
Copy link
Member Author

Calinou commented Apr 16, 2024

  • Changing global rotation will modify global scale and you can't even revert it:

This is due to #74162. There's a PR to fix this: #90584

  • When you change rotation mode to Quaternion, there is no global one:

There is no global quaternion property yet, but it should be possible to add one.

  • There is no global scale.

There's no global scale property in Node3D yet, but since we already have a global_scale() method in that class, we can't expose a property with the same name.

@KoBeWi
Copy link
Member

KoBeWi commented Apr 16, 2024

There's no global scale property in Node3D yet, but since we already have a global_scale() method in that class, we can't expose a property with the same name.

Ah, I didn't notice it's a method .-.

This is due to #74162. There's a PR to fix this: #90584

It's in 2D though.

@Jordyfel
Copy link
Contributor

IMO globals being in a subcategory would be better

@JoNax97
Copy link
Contributor

JoNax97 commented Apr 17, 2024

I vote for a toggle. I think having local and global properties visible and editable at the same time (even if they're collapsed by default) will confuse new users.

@Calinou Calinou force-pushed the editor-expose-global-transform-properties branch from 4c01ed7 to 08aeb92 Compare April 17, 2024 13:47
@Calinou
Copy link
Member Author

Calinou commented Apr 17, 2024

  • If this is going to be exposed, the description needs improvement.

I've improved the descriptions somewhat. Let me know if more could be added.

  • Global rotation in 3D doesn't have sliders:

Fixed (see OP for screenshots).

IMO globals being in a subcategory would be better

I've amended the PR to use a Global Transform subcategory for all 3 nodes. This is indeed much more tidy while preserving the ability to edit those values.

There's an issue with error spam on startup I need to figure out a solution to, so I'm marking the PR as draft.

@Calinou Calinou marked this pull request as draft April 17, 2024 13:50
@Calinou Calinou force-pushed the editor-expose-global-transform-properties branch from 08aeb92 to 053968b Compare April 17, 2024 13:53
This can be used to view and assign global transform in the inspector.
This is useful to quickly move a node back to the origin regardless
of its node hierarchy, or view its current global transform in the
live scene debugger.

The following properties are now exposed:

- Node2D:
  - Global Position
  - Global Rotation
  - Global Scale
  - Global Skew
- Node3D:
  - Global Position
  - Global Rotation
  - Global Basis
- Control:
  - Global Position
@Calinou Calinou force-pushed the editor-expose-global-transform-properties branch from 053968b to b396611 Compare April 17, 2024 14:02
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.

Add Global Position to the inspector (for Control/Node2D)
6 participants