Page MenuHomeWildfire Games

Allow remapping hotkeys from inside the game / fix QWERTY hotkeys
Needs ReviewPublic

Authored by wraitii on Jun 13 2020, 8:14 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Trac Tickets
#2850
#2604
#1810
#1917
Summary

This ended up rather large...


The goal is to fix a number of issues related to hotkeys:

  • No way to set them inside the game
  • Default hotkeys are good for English QWERTY keyboards but may fail on other keyboards.

Design wise, this adds a simple GUI page to set up hotkeys, similar to the Options page but kept separate since it wouldn't mesh too well.
The hotkey picker is a C++ Component to be able to hook into events. This required changing how many events were sent to the GUI. Seems OK.

My system waits for 1s of the same hotkey until the combination is registered, which requires adding a Tick() function to all GUI Objects. I don't think this is a dealbreaker, but refs D2638)

This also refactors Hotkey.h somewhat, as the comments were rather outdated. It cleans up the SDL include.

Another change is that we rely on SDL functions more for names. We can probably clean up our existing code more now that everybody is using SDL2.


The "default hotkeys" cleverness is by processing them as scancodes (which they are really), and converting to user-keys. This seems to work very well on my Mac French keyboard, so I'm assuming it'll work OK in general.
User hotkeys are saved properly and not interpreted as scancodes, so that the user can save his own hotkey as wanted.


(see D2838)
This fixes a latent issue with how we converted std::string to JS strings. JS strings treated std::string as Latin-1, whereas we assume they are utF8. Further, FromJSVal wasn't handled correctly either. Basically, this fixes std::string.


Known issues:

  • No handling of multiple combinations for one hotkey.
  • No warning about duplicated hotkeys on the main screen (though you can see conflicts in the hotkey picker). ----

Fixes #2850, Fixes #2604, Refs #1810, might fix #1917 (unsure).

See D303 for earlier work on this.


Screenshots:


Test Plan

Test hotkeys in-game.
Test changing your hotkeys and seeing that they get picked up in-game.
Test resetting hotkeys.

Test various combinations of hotkeys in the hotkey picker.

Event Timeline

wraitii created this revision.Jun 13 2020, 8:14 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/1893/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2424/display/redirect

wraitii added inline comments.Jun 13 2020, 8:19 PM
source/gui/ObjectTypes/CHotkeyPicker.cpp
66

This isn't very conventional, I could use a free-function come D2768

source/ps/ConfigDB.cpp
418 ↗(On Diff #12291)

debug left in, sorry

source/ps/GameSetup/GameSetup.cpp
770 ↗(On Diff #12291)

also useless, will remove

source/ps/Hotkey.cpp
79

Reverts D303 by moving it to KeyName

source/scriptinterface/tests/test_ScriptConversions.h
154 ↗(On Diff #12291)

This one is tricky. s2 is utf-8 of cyrillic characters, which are 2 bytes each. Since we modify the string manually to set the 3rd and 4th bytes to 0, we end up from тест to т0ст. The size of the string is still 8. SO the UTF16 conversion, operating on each character, will see each 0 separately and convert them to a full UTF16 zero.

It's thus correct, but really we shouldn't hack std::string like that, the test is a little odd.

285 ↗(On Diff #12291)

This does not work at all on SVN.

wraitii edited the summary of this revision. (Show Details)Jun 13 2020, 8:25 PM
Imarok added a subscriber: Imarok.Jun 13 2020, 8:34 PM

I think you should split the string fix.
And you should show some translated explanation for each hotkey instead of it's config name.

I think you should split the string fix.

Yeah, I'm also thinking of splitting the "load defaults as scancodes".

And you should show some translated explanation for each hotkey instead of it's config name.

I actually wasn't aware that there was one, but elexis screenshots show them ,so they must be somewhere :p


There is a design decision on whether to switch to ScanCodes completely for hotkeys. The drawback is that it makes it tricky to edit user.cfg in a text editor, since what you type won't by what you get. The pro is that it's easier to setup, and you can reuse hotkeys across configurations more easily. I'm not entirely decided yet.

Thank you @wraitii this patch will be a big help for casual and mp players especially if the keys could rebindable as well since mac users have problems with mouse or custom made keyboards. I am on windows 10 64 bit btw.

badosu added a subscriber: badosu.Jun 14 2020, 1:58 AM

Not sure if related to the scope of this change, but it would also be good to be able to bind keys that (I, currently) are not able to, for example Tab and CapsLock. What's strange is that I looked at the codebase and there are constants for handling these keys but they can't be used for hotkeys.

Not sure if related to the scope of this change, but it would also be good to be able to bind keys that (I, currently) are not able to, for example Tab and CapsLock. What's strange is that I looked at the codebase and there are constants for handling these keys but they can't be used for hotkeys.

I use tab, works for me.

I know @elexis had a working version?

wraitii updated this revision to Diff 12468.Jun 27 2020, 12:26 PM
wraitii edited the summary of this revision. (Show Details)

Switch to a filter instead of tabs because that's likely a better interface. I'd like to add a text field to filter hotkeys further.

Still TODO:

  • adding the hotkeys to the in-game menu?
  • Warning about hotkey conflicts.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2002/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2535/display/redirect

wraitii updated this revision to Diff 13098.Aug 6 2020, 12:13 PM
wraitii edited the summary of this revision. (Show Details)

Done here:

  • Show hotkey conflicts in the hotkey picker window (nothing on the main window - the data structures aren't good for it and I don't think it's too bad).
  • Adds a simple text filter for hotkeys.
  • Clean up the "left/right" meta keys situation.
  • Include the hotkeys in the session menu.

Update screenshots in the description above.

To check: do I need to change something in the translations files?

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2911/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2369/display/redirect

wraitii updated this revision to Diff 13099.Aug 6 2020, 12:35 PM

This might want to compile.

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

Project wide:
**** CPPCheckBear (syntaxError) [Section: Source | Severity: MAJOR] ****
!    ! Code 'classIGUIObject{' is invalid C code. Use --std or --language to configure the language.

Project wide:
**** CPPCheckBear (syntaxError) [Section: Source | Severity: MAJOR] ****
!    ! Code 'classCHotkeyPicker:' is invalid C code. Use --std or --language to configure the language.

Project wide:
**** CPPCheckBear (syntaxError) [Section: Source | Severity: MAJOR] ****
!    ! Code 'classCStrW:' is invalid C code. Use --std or --language to configure the language.

Project wide:
**** CPPCheckBear (syntaxError) [Section: Source | Severity: MAJOR] ****
!    ! Code 'std::vector' is invalid C code. Use --std or --language to configure the language.

Project wide:
**** CPPCheckBear (syntaxError) [Section: Source | Severity: MAJOR] ****
!    ! Code 'classCStr:' is invalid C code. Use --std or --language to configure the language.

Project wide:
**** CPPCheckBear (syntaxError) [Section: Source | Severity: MAJOR] ****
!    ! Code 'namespaceJSI_Hotkey{' is invalid C code. Use --std or --language to configure the language.
Executing section JS...

binaries/data/mods/public/gui/hotkeys/HotkeyPicker.js
[  24] »   »   this.input.onKeyChange·=·keys·=>·this.text.caption·=·keys.join("+")·+·translate("\n(Hold·for·one·second·to·register)");
**** ESLintBear (no-return-assign) [Section: JS | Severity: NORMAL] ****
!    ! Arrow function should not return assignment.

binaries/data/mods/public/gui/hotkeys/HotkeyPicker.js
[  31] »   »   »   »   .map(translate).filter(name·=>·name·!=·this.name);
**** ESLintBear (no-shadow) [Section: JS | Severity: NORMAL] ****
!    ! 'name' is already declared in the upper scope.
**** ESLintBear (comma-spacing) [Section <empty> | Severity NORMAL] ****
!    ! A space is required after ','.
[----] /zpool0/trunk/binaries/data/mods/public/gui/hotkeys/HotkeysPage.js
[++++] /zpool0/trunk/binaries/data/mods/public/gui/hotkeys/HotkeysPage.js
[  83] 		return a.localeCompare(b, Engine.GetCurrentLocale().substr(0,2), { "numeric": true });
[  83] 		return a.localeCompare(b, Engine.GetCurrentLocale().substr(0, 2), { "numeric": true });

binaries/data/mods/public/gui/hotkeys/HotkeysPage.js
[   7] »   »   »   new·HotkeyPicker(
**** ESLintBear (no-new) [Section: JS | Severity: NORMAL] ****
!    ! Do not use 'new' for side effects.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2912/display/redirect

Stan added inline comments.Aug 21 2020, 11:35 AM
source/ps/Hotkey.cpp
33

?

source/ps/KeyName.cpp
173

Do we still the comment ?