Allow numpad comma , to be used for 3D Blender-Style Transforms
#107441
Conversation
AThousandShips
added
cherrypick:4.3
| @@ -2283,7 +2283,7 @@ void Node3DEditorViewport::_sinput(const Ref<InputEvent> &p_event) { | |||
| } else if (unicode == '-') { | |||
| _edit.numeric_negate = !_edit.numeric_negate; | |||
| update_transform_numeric(); | |||
| } else if (unicode == '.') { | |||
| } else if (unicode == '.' || unicode == ',') { | |||
There was a problem hiding this comment.
The comment above says:
// Use the Unicode value because we care about the text, not the actual keycode.
// This ensures numbers work consistently across different keyboard language layouts.
But I think that's a misunderstanding of the API, you shoudl use get_keycode() instead of get_physical_keycode() to get the keycode from the user's keyboard mapping.
And then for period specifically, using get_physical_keycode() to check that it's the en_US KP_PERIOD should also work I expect.
There was a problem hiding this comment.
Makes sense, I agree that using the main keyboard comma too might be overdoing it, since some software dedicates some specific functionality to it.
akien-mga
removed
the
cherrypick:4.3
ryevdokimov
force-pushed
the
blender-comma-transform
branch
from
f981b61 to
cc3246b
Compare
| if (unicode >= '0' && unicode <= '9') { | ||
| uint32_t value = uint32_t(unicode - Key::KEY_0); | ||
|
|
||
| if ((keycode >= Key::KEY_0 && keycode <= Key::KEY_9) || (keycode >= Key::KP_0 && keycode <= Key::KP_9)) { |
There was a problem hiding this comment.
You might want to keep using physical keycode here, at least for the main numbers above QWERTYUIOP.
On a French AZERTY keyboard, the numbers are in the same place but they're secondary keys, the first keys are accented characters and other symbols, you have to press "Shift+1" to get "1" (technically "Shift+&"): https://en.wikipedia.org/wiki/AZERTY
There was a problem hiding this comment.
Part of the fun of making PRs is learning about stuff like this 😂
Thanks for the review.
| @@ -2283,11 +2289,11 @@ void Node3DEditorViewport::_sinput(const Ref<InputEvent> &p_event) { | |||
| } else if (unicode == '-') { | |||
There was a problem hiding this comment.
You can use KEY_MINUS and KEY_KP_MINUS here (non-physical), and remove unicode.
| @@ -2283,11 +2289,11 @@ void Node3DEditorViewport::_sinput(const Ref<InputEvent> &p_event) { | |||
| } else if (unicode == '-') { | |||
| _edit.numeric_negate = !_edit.numeric_negate; | |||
| update_transform_numeric(); | |||
| } else if (unicode == '.') { | |||
| } else if (keycode == Key::PERIOD || physical_keycode == Key::KP_PERIOD) { | |||
There was a problem hiding this comment.
I guess the use of physical for KP_PERIOD might warrant a comment.
ryevdokimov
force-pushed
the
blender-comma-transform
branch
2 times, most recently
from
f1567c7 to
ecbffff
Compare
|
Hmm, I was able to dig up a couple different layout keyboards I had laying around and found a few issues. Will investigate more. |
|
Nevermind, I think it's fine. Should be good. |
ryevdokimov
force-pushed
the
blender-comma-transform
branch
from
ecbffff to
6f48795
Compare
|
Accidently put the main keyboard comma back, so removed that. Now it should be good for real. |
akien-mga
changed the title
, to be used for 3D Blender-Style Transforms, to be used for 3D Blender-Style Transforms
|
Thanks! |
Fixes: #107432