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

Fix key mapping changes when moving from macOS to other platform. #65241

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Sep 2, 2022

  • Removes separate Command key (use Meta instead).
  • Instead of it and store_command property, adds an event property to automatically remap CommandControl (if set Control or Meta can't be set, and it is matched to Control or Meta depending on platform).
  • Uses platform specific key names on macOS ("Command" instead of "Meta", "Option" instead of "Alt").

Example:
Screenshot 2022-09-02 at 14 00 35

  • First mapping is mapped to Option+Command+Q on macOS and to Alt+Ctrl+Q on other platforms.
  • Second is mapped to Option+Command+A on macOS and to Alt+Meta+A on other platforms.

Fixes #35355

@bruvzg bruvzg added this to the 4.0 milestone Sep 2, 2022
@bruvzg bruvzg force-pushed the no_keymap_ambiguity branch 2 times, most recently from 645b9c0 to 91d0694 Compare September 3, 2022 09:01
@bruvzg bruvzg changed the title [WIP] Fix key mapping changes when moving from macOS to other platform. Fix key mapping changes when moving from macOS to other platform. Sep 3, 2022
@bruvzg bruvzg marked this pull request as ready for review September 3, 2022 09:01
@bruvzg bruvzg requested review from a team as code owners September 3, 2022 09:01
@fire
Copy link
Member

fire commented Sep 3, 2022

Maybe @aaronfranke knows more cases where working on a Mac causes changes to project that differ from Windows and Linux?

I support this.

@aaronfranke
Copy link
Member

aaronfranke commented Sep 3, 2022

Why is Command spelled out, but Ctrl shortened? I think it should be "Cmd/Ctrl".

Actually, I think it would be better to have names like this:

  • Ctrl is Ctrl on Apple platforms and Ctrl on others.
  • Cmd is Cmd on Apple platforms and Ctrl on others.
  • Super is Cmd on Apple platforms and Super on others.

@bruvzg
Copy link
Member Author

bruvzg commented Sep 3, 2022

I guess shortened Cmd and Win might be more consistent with others, or we can spell out Control.

Cmd is Cmd on Apple platforms and Ctrl on others.
Super is Cmd on Apple platforms and Super on others.

That was the reason of the issue, Cmd is not Control.

Right now, naming is:

  • Key::SHIFT is Shift on all platforms.

  • Key::CTRL is Ctrl on all platforms.

  • Key::ALT is Option on macOS and, Alt on other platforms.

  • Key::META is Command on macOS, Windows on Windows, and Meta on other platforms.

  • KeyModifierMask::CMD_OR_CTRL do not have a Key equivalent and exist only for mappings and if it's set Key::META and Key::CTRL are ignored, it is auto translated to either Key::META or Key::CTRL, the name is only visible in the mapping editor and is Command/Ctrl on all platforms.

Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a quick test, I can confirm that this fixes the issue, the mappings saved to project.godot are now consistent between Windows and macOS.

@akien-mga akien-mga requested a review from a team September 5, 2022 09:26
Removes separate `Command` key (use `Meta` instead).
Adds an event flag to automatically remap `Command` <-> `Control` (cannot be set alongside `Control` or `Meta`).
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. It's still somewhat complex but I don't think there's any non-complex solution here, and this one is much cleaner :)

@akien-mga akien-mga merged commit 6923309 into godotengine:master Sep 8, 2022
@akien-mga
Copy link
Member

Thanks!

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.

Input map is changed when project is opened on macOS
4 participants