-
-
Notifications
You must be signed in to change notification settings - Fork 21.8k
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
Display a matrix for Node2D #52236
Display a matrix for Node2D #52236
Conversation
I am sorry for closing the previous one, I have an alternative proposal for doing this same thing that I believe makes more sense. It just takes time to write. |
I have not a strict opinion about the changes, but I'd rather have those changes base on user needs than what we feel is right. Regarding exposing the raw 2D transform, I don't mind, but we have to make sure this is useful. If no one end up using it, then it's bloat. I'd rather be sure this is needed exposed in the inspector. For the 3D part, I guess the changes make sense, though I hope users are familiar enough with what a basis is. Also, @reduz pointed out on RocketChat that all of this impacts how you can animate things. If users cannot animate the whole transform easily anymore, this might be problematic. I don't know what reduz has in mind as a proposal, but I wonder if, instead of deleting the duplicate "translation" property (as an individual property and in the matrix), if it would not be possible instead to change the inspector to provide two views (via tabs or maybe an enum ?). That could be a little bit similar to how we discussed things to be in the Control node UI rework : godotengine/godot-proposals#2841. You would then be able to switch between the position/rotation/skew view and the raw matrix view. |
0c75af0
to
f4d0669
Compare
f4d0669
to
4046c0f
Compare
4046c0f
to
0ccae06
Compare
Rebased and updated after #54084, now it only adds the matrix to Node2D. The screenshot has also been updated. Another option is to do a dropdown that has an option to replace rotation/scale/skew with the basis matrix like in #54084, but for 2D since there are only ever just those two things, I think it's simpler / better to just have it the way it is in this PR. |
0ccae06
to
8097d21
Compare
Also update Node3D basis doc
8097d21
to
7259eff
Compare
Closing as discussed in the PR meeting for lack of consensus/support |
This was closed, but I still think this is worth having, so here's a branch: https://github.com/aaronfranke/godot/tree/node2d3d-transform
EDIT: The comments from @reduz are now out-of-date because they refer to when this PR used to change Node3D.
The matrix is now displayed for Node2D, while previously it just wasn't displayed at all. I also updated the docs for Node3D's basis property (not super related but it's a holdover from a previous version of this PR).
Before/after pic (left is before, right is after):