OpenTTD

Tasklist

FS#5679 - Allow characters as hotkey instead of assuming everyone uses a Latin keyboard

Attached to Project: OpenTTD
Opened by Grzegorz Duczyński (adf88) - Sunday, 28 July 2013, 13:37 GMT
Last edited by Remko Bijker (Rubidium) - Wednesday, 13 November 2013, 15:55 GMT
Type Feature Request
Category Core
Status New
Assigned To No-one
Operating System All
Severity Low
Priority Normal
Reported Version trunk
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No

Details

Inside _keycode_to_name table (hothey.cpp) wrong keykode for comma key causes failures. Currently it's ',' which is 44 in decimal. This keycode (44) is assigned to F12 key. When reading a 'COMMA' from .cfg file it gets read as F12 key. When saving a WKC_COMMA an assertion pops up.

Bug exists since the begging of _keycode_to_name table existence, when save/loading code has been added (r20055).
Fix patch attached - changes ',' to WKC_COMMA.
This task depends upon

Comment by Sébastien Brissaud (sbr) - Tuesday, 30 July 2013, 16:39 GMT
Plus (+) is also unusable as an hotkey. For the same reason than comma is assigned to F12, it's assigned to F11.
Moreover there is no WKC_PLUS to map 'PLUS' from the hotkey.cfg file.

-- plus-hotkey_00add-plus-windowkeycode_370db44761d.patch
This patch defines a new WindowKeyCode WKC_PLUS and map it to the video driver keycodes for SDL, Allegro and Win32.

* SDL provides SDLK_PLUS as scancode for the '+' key (http://www.libsdl.org/docs/html/sdlkey.html).
* No scancode for '+' exists in the Allegro driver (https://www.allegro.cc/manual/4/api/keyboard-routines/key). The unicode representation of '+' is tested before the scancode mapping loop.
* Win32 provide 0xBB (VK_OEM_PLUS) as scancode for the '+' key (http://www.kbdedit.com/manual/low_level_vk_list.html). I have no Windows computer and thus haven't tested the Win32 mapping.


-- plus-hotkey_01fix-map-hotkey-plus-to-key-plus_370db44761d.patch
This patch should be applied on top of both adf88's patch and the 00 patch above.
It map 'PLUS' from hotkeys.cfg to WKC_PLUS allowing 'PLUS' to be used as an hotkey.


Please find attached these two patches against r25634.
Comment by Grzegorz Duczyński (adf88) - Tuesday, 30 July 2013, 18:42 GMT
Allegro doesn't have it probably because most keyboards don't have a '+' key. You can't "fix" it the way you did. Allegro's ureadkey returns translated char code rather then key code. When you press [SHIFT]+[=] on a regular PC keyboard you will get '+' char code. With your patch pressing [SHIFT]+[=] will be interpreted as WKC_SHIFT + WKC_PLUS rather then expected WKC_SHIFT + WKC_EQUALS.

I think that the 'PLUS' should be just removed from the _keycode_to_name table because that key is really rare.

Comment by frosch (frosch) - Tuesday, 30 July 2013, 18:42 GMT
Hotkeys are assigned to scancodes, not to characters. A "+" character hotkey makes no sense wrt. several keyboard layouts. I guess it should be "EQUAL" or so (i.e. sdl and stuff uses US layout).
Comment by Sébastien Brissaud (sbr) - Tuesday, 30 July 2013, 22:08 GMT
adf88:
I had hesitate to post my patches in this issue. I thought it was related with yours and added them here. I hope that doesn't mess up too much with your bug report and patch.

> Allegro doesn't have it probably because most keyboards don't have a '+' key.

Granted for US keyboard layout and most of its derivative. Nevertheless, of the three keyboard layout I use regularly (dvorak-dvp, Latin American & French) only French has no '+' key.

> You can't "fix" it the way you did. Allegro's ureadkey returns translated char code rather then key code. When you press [SHIFT]+[=] on a regular PC keyboard you will get '+' char code. With your patch pressing [SHIFT]+[=] will be interpreted as WKC_SHIFT + WKC_PLUS rather then expected WKC_SHIFT + WKC_EQUALS.

Effectively. I should have returned WKC_PLUS as soon as it matched the unicode test. The modifiers tests make only sense when testing the scancodes.
But for example, SHIFT+= has no meaning in the Latin American layout as there is no '=' key, '=' being in fact SHIFT+0.

> I think that the 'PLUS' should be just removed from the _keycode_to_name table because that key is really rare.

The 'PLUS' entry in _keycode_to_name is buggy and should be fixed. As long as hotkeys could only be bound to US layout scancodes, the sane path is to remove the 'PLUS' entry in _keycode_to_name.


frosch:
> Hotkeys are assigned to scancodes, not to characters. A "+" character hotkey makes no sense wrt. several keyboard layouts. I guess it should be "EQUAL" or so (i.e. sdl and stuff uses US layout).

As wrote above, that make sense from a US layout point of view. I was mixing two things, the fix to an incorrect _keycode_to_name entry and the localization of the hotkeys.cfg file.


OpenTTD is translated in many languages but we can only use US layout scancodes as hotkeys. I think that the localization of hotkeys is desirable to give the users the ability to define as hotkey whatever keys they have in their keyboards. Its an improvement I would like to investigate in another issue.

Please find attached a patch against r25637 to be applied on top of the one of adf88.
Comment by Grzegorz Duczyński (adf88) - Wednesday, 31 July 2013, 05:18 GMT
Actually there is a way to deal with '+' and other keys too. Reassign F1-F12 and WKC_PAUSE to other codes. Why do they interfere with printable ASCII range (0x20..0x7E)?! That makes problems not only with '+' key but also with other keys. We don't need these constants: WKC_SLASH, WKC_SEMICOLON, WKC_EQUALS ... WKC_MINUS. It would be simpler and it would not cause conflicts if using ASCII representation: '/', ';', '=', ..., '-'. I think that all keycodes in range 0x20..0x7E should be considered as keys with corresponding ASCII representation (keycode == charcode).

Sébastien, your Allegro hack could work. Test against '+' after scancode test, not before. That makes conflicts only with unknown scancodes, thus no conflict at all.
Comment by Remko Bijker (Rubidium) - Tuesday, 12 November 2013, 21:46 GMT
To me it looks like the PLUS should never been added to the hotkey options. Furthermore the ',' should be replaced by WKC_COMMA and then it should work properly, right?

Basing it on the entered characters might not be such a good idea. Assume the % is above the 5. How do I write that down in the configuration? SHIFT+5, or the actually pressed character %... but is that with or without shift? In the former case the character is %, but it doesn't know that it means SHIFT+5 was pressed... so it isn't triggered. In the latter case, the character is % again, but the SHIFT is pressed as well... so it doesn't match the character.

It will also do weird things with composite keys; they won't be useful for hotkeys as you have to press them multiple times. On the other hand, you can use it to construct a huge number of extra hotkeys by using dead keys that two keys to construct characters.
Comment by Grzegorz Duczyński (adf88) - Wednesday, 13 November 2013, 10:57 GMT
This is exactly what I'm talking about. That would fix issues.

But to extend current functionality to a wide range of keyboard layouts, I think there is one solution. Split keys and chars so there is no interference. Allow to define hotkeys by a key or by a character.

To set up a character hotkey, user would have to put it's Unicode representation into the cfg. A certain character hotkey would be fired up when user presses key combination that translates to the given character. Characters would be allowed only alone, no combinations.

Key hotkeys would have their mnemonics ('A_KEY', 'B_KEY', ..., 'COMMA', 'EQUEALS', ..., 'SHIFT', 'CTRL', ...). Only limited number may be allowed because of video drives i.e. Allegro doesn't know the [+] key so probably there would be no 'PLUS'. Any key combination would have consist of a single non-special key and any number of special keys.

Above ideas are similar to what we have currently. But currently keys interfere with chars.
Comment by Remko Bijker (Rubidium) - Wednesday, 13 November 2013, 15:55 GMT
  • Field changed: Type (Bug → Feature Request)
  • Field changed: Summary (Comma key unusable as a hotkey because of wrong keycode entry → Allow characters as hotkey instead of assuming everyone uses a Latin keyboard)
The bug is fixed in r25973. The feature request remains open.

Loading...