Page MenuHomeWildfire Games

Try to recognise unknown hotkey mappings using SDL2
ClosedPublic

Authored by wraitii on Apr 8 2017, 5:47 PM.

Details

Reviewers
Itms
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Owners Package(Owns No Changed Paths)
Commits
rP19624: Try to recognise unknown hotkey mappings using SDL2.
Summary

Our hotkey system is a little barebones and inflexible. Changes to SDL2 have made it impossible to use some keys on some keyboards (e.g. French keyboards, can't use accents for any hotkey).

It would be nice to completely overhaul it, but in the meantime I would suggest adding a call to SDL_GetKeyFromName to try and recognise an unknown mapping. This fixes the issue on my keyboard and allows me to setup hotkeys I want.

Test Plan

Test out fancy hotkey mappings.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

wraitii created this revision.Apr 8 2017, 5:47 PM
Itms awarded a token.Apr 8 2017, 5:49 PM
Vulcan added a subscriber: Vulcan.Apr 8 2017, 6:33 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/714/ for more details.

leper added inline comments.
source/ps/Hotkey.cpp
81 ↗(On Diff #1151)

Why not

if (!mapping)
    mapping = SDL_GetKeyFromName(it->c_str());

if (!mapping)
{
    LOGWARNING("...");
    continue;
}

?

wraitii updated this revision to Diff 1375.Apr 20 2017, 3:33 PM

No reason whatsoever.

wraitii marked an inline comment as done.Apr 20 2017, 3:33 PM
wraitii added inline comments.
source/ps/Hotkey.cpp
81 ↗(On Diff #1151)

updated

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/834/ for more details.

leper added inline comments.
source/ps/Hotkey.cpp
80 ↗(On Diff #1375)

\n

Itms accepted this revision as: Itms.Apr 30 2017, 1:07 PM

This works for me (AZERTY on Linux).

I can test on Windows tomorrow but it should work, if SDL_GetKeyFromName is platform dependent that's a bit sad ?

This revision is now accepted and ready to land.Apr 30 2017, 1:07 PM
wraitii updated this revision to Diff 1561.Apr 30 2017, 7:03 PM
wraitii marked an inline comment as done.

Update keys.txt, I'm taking suggestions on improvements

@Itms : I'm ok with testing on windows, won't commit this today anyhow.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/943/ for more details.

@Itms: confirmed on Windows?

Itms added a comment.May 14 2017, 8:40 PM

Sorry I shouldn't have accepted it because it disappeared from my review list ?

On Windows there is a discrepancy, it acts like I have a QWERTY keyboard (on SVN and with your patch), so the regular commands work even without your patch. However, binding hotkeys with French characters works with your patch, so no issue, please commit ?

This revision was automatically updated to reflect the committed changes.