Add 'Move to view' and make 'Align to view' only align#23334
Add 'Move to view' and make 'Align to view' only align#23334akien-mga merged 1 commit intogodotengine:masterfrom
Conversation
|
Well, if we're being pedantic on the semantics of "Align", then one would very much expect "Move" to only impact translation, not the rotation or scale. To me align means the same as sync, as in "align/sync property X with property Y", so it's not limited to the rotation/"look at" angle only. So more than nitpicking about semantics, it would be interesting to know what are the use cases and if people need to be able to align rotation only, translation only, scale only, or full transform. Edit: That's said, I'm not against the change per se :P Sorry if the above message comes off a bit grumpy. IMO the added feature might be useful, but maybe we need to go full bikeshedding on the naming as I'm not convinced by either "Align with view" and "Move to view" usage (both before and after this PR). |
|
Moving to 3.2 milestone as we're about to enter the beta stage and release freeze for Godot 3.1. Only bug fixes against issues of the 3.1 milestone (or enhancements to core features of the 3.1 roadmap) will be considered until 3.1-stable is released. Note: If it's merged after 3.1-stable is released, this change might be considered for cherry-picking into a later 3.1.x release. |
|
Sorry for the late review. We finally got to discuss this in a PR review meeting and bikeshed on the naming, see logs from 15:08. We'd suggest to use the names "Align Transform with View" and "Align Rotation with View" to be explicit about what each operation does. Could you do this change, if you agree that it makes things clear? (And change the enum names accordingly.) |
akien-mga
force-pushed
the
align_sel_view
branch
from
f797b87 to
afcb7fd
Compare
|
I pushed an update with the proposed label change. |
akien-mga
force-pushed
the
align_sel_view
branch
from
afcb7fd to
c740c59
Compare
akien-mga
force-pushed
the
align_sel_view
branch
from
c740c59 to
4a218b9
Compare
|
Thanks! |
|
hi, |
4 participants
Hi,
After discovering "Align with view" I played with it a bit and was dissatisfied with how it worked.
Maybe I'm picky, but for me "Align with view" should rotate but not move the selected node(s) 🙅♂️
So I made it only rotate the selected nodes to be aligned with the view, and added a menu option "Move to view" which only does what "Align with view" did (do you follow ? 😛 ).
I also added a "focus_selection" at the end to avoid errors when the selected node is a camera (as reported in #23063).