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

[macOS] Fix keyboard shortcuts on non QWERTY keyboard layouts. #17827

Merged

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Mar 28, 2018

Hopefully fix shortcuts on all non QWERTY layouts, gets key mapping from os keyboard layout instead of hardcoded maps.

Fixes (on macOS) #11777

Please test this with physical non-QWERTY keyboards before merging, I have tested it with software layouts only.

@Calinou
Copy link
Member

Calinou commented Mar 28, 2018

Does this pull request affect running projects? Also, will this fix be ported to other platforms as well?

@bruvzg
Copy link
Member Author

bruvzg commented Mar 28, 2018

Yes this will affect projects, but macOS already had remapping for main layout (AZERTY, QWERTZ, Colemak, Dvorak and Neo), this PR just extends remapping to all possible layouts.

No idea about other platforms, I didn't look how layouts work on Windows/X11, currently there's not any remapping on other platform. (see comment below)

@green-coder
Copy link

green-coder commented Mar 29, 2018

It works!

I compiled that version on my computer and tested it. The editor works, both the text input and the shortcuts are correct. Nice !

For the record, my computer runs on OSX 10.13.3, the keyboard was a QWERTY which I use as a Programmer Dvorak via software remapping (in the macOS' settings).

@bruvzg
Copy link
Member Author

bruvzg commented Apr 4, 2018

I have removed some of the previous comments and consolidated all testing data from different platforms in this one.

Notes:

  • Text input is NOT affected by this PR in any way, only scancodes.
  • Other platforms are not affected by this PR (latin_keyboard_keycode_convert was only used on macOS).
  • With this PR non-QWERTY latin layouts works similar on all platforms.
  • Layouts w/o latin letters at all does not work correctly on Linux.

Testing process:

Test code:

func _unhandled_input(event):
	if event is InputEventKey:
		if event.pressed:
			print("%d %s" % [event.scancode, OS.get_scancode_string(event.scancode)])

Steps to reproduce:

  1. Select specified keyboard layout in OS settings.
  2. Press all keys in top letter row (QWERTYUIOP[] on QWERTY keyboard) left-to-right one by one.

Layout: DVORAK L (Left handed)

dvrk

macOS without PR macOS with PR Debian GNU/Linux 9, Xfce 4 Manjaro Linux 17, KDE Windows 10
81 Q 59 Semicolon 59 Semicolon 59 Semicolon 59 Semicolon
87 W 81 Q 81 Q 81 Q 81 Q
69 E 66 B 66 B 66 B 66 B
82 R 89 Y 89 Y 89 Y 89 Y
84 T 85 U 85 U 85 U 85 U
89 Y 82 R 82 R 82 R 82 R
85 U 83 S 83 S 83 S 83 S
73 I 79 O 79 O 79 O 79 O
79 O 46 Period 46 Period 46 Period 46 Period
80 P 16777356 Kp 6 54 6 54 6 54 6
123 BraceLeft 16777355 Kp 5 53 5 53 5 53 5
125 BraceRight 61 Equal 61 Equal 61 Equal 61 Equal

Layout: Crimean Tatar (Turkish F)

turkf

macOS without PR macOS with PR Debian GNU/Linux 9, Xfce 4 Manjaro Linux 17, KDE Windows 10
81 Q 70 F 70 F 70 F 70 F
87 W 71 G 71 G 71 G 71 G
69 E 69 E 0 0 59 Semicolon
82 R 82 R 0 0 73 I
84 T 79 O 79 O 79 O 79 O
89 Y 68 D 68 D 68 D 68 D
85 U 82 R 82 R 82 R 82 R
73 I 78 N 78 N 78 N 78 N
79 O 72 H 72 H 72 H 72 H
80 P 80 P 80 P 80 P 80 P
123 BraceLeft 81 Q 81 Q 81 Q 81 Q
125 BraceRight 87 W 87 W 87 W 87 W

Layout: Russian (Phonetic)

rusp

macOS without PR macOS with PR Debian GNU/Linux 9, Xfce 4 Manjaro Linux 17, KDE Windows 10
81 Q 81 Q 0 0 81 Q
87 W 87 W 0 0 87 W
69 E 69 E 0 0 69 E
82 R 82 R 0 0 82 R
84 T 84 T 0 0 84 T
89 Y 89 Y 0 0 89 Y
85 U 85 U 0 0 85 U
73 I 73 I 0 0 73 I
79 O 79 O 0 0 79 O
80 P 80 P 0 0 80 P
123 BraceLeft 123 BraceLeft 0 0 123 BraceLeft
125 BraceRight 125 BraceRight 0 0 125 BraceRight

@akien-mga
Copy link
Member

Thanks for the very thorough test matrix @bruvzg, this shows that it's definitely a good improvement.

Let's merge and maybe open an issue about the non-working Russian layout on Linux?

@akien-mga akien-mga merged commit 8662543 into godotengine:master Apr 5, 2018
@bruvzg bruvzg deleted the macos_shortcut_key_remapping_fix branch April 5, 2018 12:45
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.

4 participants