Use the APPLE_STYLE_KEYS define consistently for macOS shortcuts
#47619
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`APPLE_STYLE_KEYS` is always defined when `OSX_ENABLED` is defined (and vice versa). The point of that define is to semantically distinguish between alternate codepaths for shortcuts from alternate codepaths for other purposes (such as code signing).
Calinou
commented
Apr 4, 2021
Comment on lines
+3155
to
3158
| #ifdef APPLE_STYLE_KEYS | ||
| if (k->get_keycode() == KEY_META) { | ||
| #else | ||
| if (k->get_keycode() == KEY_CONTROL) { |
There was a problem hiding this comment.
I wonder if we could have an OS-specific keycode to avoid this kind of #ifdef. Something like KEY_MASK_CMD, but for standard keys rather than modifiers (KEY_CMD?).
cc @bruvzg
There was a problem hiding this comment.
Adding KEY_CMD define to the core/os/keyboard.h/core/core_constants.cpp should work in the same way.
// core/os/keyboard.h //
#ifdef APPLE_STYLE_KEYS
KEY_CMD = KEY_META,
#else
KEY_CMD = KEY_CONTROL,
#endif// core/core_constants.cpp //
BIND_CORE_ENUM_CONSTANT_NO_VAL(KEY_CMD); //to avoid listing value in the docsThere was a problem hiding this comment.
Sounds good, but I might leave this for a future PR.
There was a problem hiding this comment.
- I added bool Input::is_modifier_pressed(int p_keycode_mask) where you can pass a mask like KEY_MASK_CMD or KEY_MASK_SHIFT and it will return whether one of the keys that make up that mask is pressed.
Does something like that. I think in that PR I also replace all cases like this #ifdef with that method usage.
|
This was superseded by #65241. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While working on #47618, I noticed that
APPLE_STYLE_KEYSis always defined whenOSX_ENABLEDis defined (and vice versa). The point of that define is to semantically distinguish between alternate codepaths for shortcuts from alternate codepaths for other purposes (such as code signing).Alternatively, if we deem that doing this separation isn't important, we could remove
APPLE_STYLE_KEYSentirely and replace all its existing uses withOSX_ENABLED.