Desktop: Controller improvements - readable labels, controller hotkeys, option to apply profiles to all controllers

[DavidRGriswold]

This PR started as controller hotkeys, but in the process of making it I realized that the SDL controller code could use some serious cleanup and refactor in order to make things make more sense. So this PR does that, in addition to implementing the hotkeys and making button presses have more meaningful names in QT configuration windows.

User-facing Changes

1. Mapped buttons now show meaningful names whenever they can. Currently shows Xbox-style and PS-style names (A / ⬜).
2. Controller Hotkeys can be created with up to two controller presses, including axes, back buttons, etc, and should work as expected without interfering with normal controller presses (obviously if a button is mapped to both, both actions will happen).
3. For both input profiles and controller hotkeys, you can now select whether the mappings should apply to only THIS controller (old method, uses guid and port), controllers of exactly this type (uses guid only), or ALL controllers (attaches to any controller).
4. When auto-mapping on desktop, you are now told to press the "A (right) button" with an icon, similar to how it works on mobile. If the user presses the button that shows up as "A" to SDL, it either means they have a nintendo-layout controller or they are choosing to map the visible "A" to A rather than the right button to A, and it honors that mapping. Any other button defaults to the old xinput-style auto-map.

Changes Made in shared/common code

• Updated SDL code to prefer to use the GameController API in SDL rather than the Joystick API whenever possible, enabling the various features above. This required a lot of little tweaks and changes for how controller parameters are created and events are monitored and interpreted in sdl_impl, and required refactoring part of the code into a new file sdl_joystick.h
• Reduced the complexity of the internal SDL state - rather than manually tracking the status of buttons and axes, we now ask SDL their status directly, whenever needed. Based on my research this should have no performance negatives and maybe even slight (unnoticeable) improvements.
• added a ButtonToText() method to input_common that converts from a button parameter package to a readable name. For now this is only used in QT, but it could be useful for any frontend that uses SDL so I thought it worth moving there; it replaces some similar functions that used to live in QT.
• modified the params for SDL buttons and axes to support multiple maptypes as described in user-facing changes, as well as a parameter to determine whether this is a controller or joystick API mapping so that the correct button value is read. Controller mapping is preferred whenever possible for more portable maps, but joystick will be used as a fallback for special buttons like back buttons that don't map to the standard xinput buttons.
• Added settings to input profile and controller hotkey sections to manage mapping style

Changes Made in QT code specifically

• Updated Hotkey struct to include both a controller_keyseq field (essentially controller param string) and two ButtonDevice objects to attach a hotkey to one or two controller buttons
• Also updated Hotkey struct and HotkeyRegistry class to keep track of the QActions associated with the hotkey, so they can manually be called by the controller
• Created a HotkeyMonitor class that keeps track of what controller buttons are associated with hotkeys and checks their status 60 times per second, triggering associated shortcuts and actions as appropriate
• Added a configuration tab for controller hotkeys, and a special dialog that pops up when mapping.

To do now:

• review my c++ code to make sure I didn't do anything insane
• Test dpads that register as hats rather than buttons
• test for crashes or unexpected behavior on all platforms
  - [x] Windows
  - [x] Linux
  - [x] Mac OS X
• Add support for analog triggers to be detected as buttons
• move the hotkey settings into input profiles to support multiple controllers OR try to set it up so they work with any plugged-in controller (?)
• Enable the choice of using mappings with any controller or just this controller. (I think this satisfies most user needs and so I no longer plan to put controller hotkeys in the input profiles)
• Improve behavior when a controller is connected after Azahar is running to make sure it works for both hotkeys and inputs after the connection

Closes #119
Closes #1728
Closes #1920

[retrogamedesigner]

On Linux Debian Trixie:

Testing on a build of a3d518b with a few different games for around 90 minutes, everything seemed to work as intended. I had hotkeys assigned to each d-pad direction, each left analog stick direction, both analog shoulders, and several face buttons. No crashes, regular inputs functioned normally and consistently, hotkeys consistently worked as assigned.

This morning, I rebuilt from 65e7cc7 and tested again. I reassigned my controller hotkeys. The names of the inputs were updated, as expected, however none of my assigned hotkeys worked. I tried both single-button assignments and two-button combinations. Regular controller inputs were fine and worked as usual.

My controller is an 8-Bit Do Pro 2 set to X-input. I've also tested with a Logitech F310 set to D-Input and X-Input and experience the same behavior.

[DavidRGriswold]

I think I broke a couple of things with the gamepad detection and updating in my refactor. I'm working on it now.

Did you try relaunching azahar after mapping to see if the hotkeys work on a fresh launch?

[retrogamedesigner]

Yes, I've tried relaunching Azahar, as well as remapping after relaunching, but nothing changes.

[DavidRGriswold]

@retrogamedesigner I have fixed the bugs I was having on Mac OS X with controller initialization and closing, and everything now works as expected for me on Mac. I can test on Linux later tonight, but if you beat me to it I would love to hear if this fixes the issue you were having. If not, I'll have to do some deeper debugging later.

[retrogamedesigner]

Still experiencing the same behavior on Linux. I also tried reassigning the basic controller inputs - they received the new naming scheme AND still work correctly. Controller hotkeys still aren't working for me.

[retrogamedesigner]

So, I have a little more insight into what's happening with controller hotkeys and an additional observation.

Going back to basics for testing, I have all the general inputs set (A, B, X, Y, L, R, etc.) and assigned the left analog trigger by on my controller (by itself) to toggle mute. The Controller Hotkey displays as "Left Trigger+" During gameplay, the left trigger does not active the mute toggle, however moving the Right Analog Stick Down does! I have no idea why, but it appears that the functions are not translating correctly to the user-assigned inputs and instead ending up bound to different inputs.

With further testing, I have determined this is also effecting general input functionality. Controller inputs appear to be assigned correctly, based on the button presses matching the labels that are then assigned, but many of the inputs during gameplay DO NOT match the labels or the buttons I pressed when assigning the input. For example, I didn't assign L3 or R3 (clicking the analog sticks down on the physical controller) at all, but those are functioning as L: LB (left bumper) and R: RB (right bumper).

I also noticed that the D-Pad doesn't function anymore during gameplay. I have the inputs for each direction assigned to the corresponding directions on my controller's D-Pad (X-Input mode). Nothing appears to be mapped to them at all, even though they were accepted as inputs when assigning controls and controller hotkeys.

[DavidRGriswold]

okay, interesting and helpful! I intend to do some testing on linux myself in a few hours and I'll see if I can recreate this issue, but it sure sounds like a disconnect between the Joystick and the GameController API and how our own input API translates those differently when configuring vs interpreting. I will hopefully be able to lock that down. I think Mac OS X has an extra layer of translation between the gamepad and the OS (essentially its own steam input thing) that maybe makes testing on Mac not the most useful for finding the weird issues in SDL.

[DavidRGriswold]

@retrogamedesigner I was able to recreate your exact issue on Linux and traced the problem. The latest commit fixes it for me and likely for you. Give it a test when you get a chance!

Unfortunately, this change does break backwards compatibility with old input maps, so we will need to decide if that's okay (as I do think this is an improvement worth doing overall) or if I should change some things up to make it so the code can attempt correctly interpret both old and new profiles.

[DavidRGriswold]

@retrogamedesigner Another update to fix the backwards compatibility issue and enable special back buttons; I tested with my own 8bitdo pro 2 in Dinput mode and I can now map my back buttons in Linux as their own buttons for input or hotkeys. You'll need to redo your maps from before, as I have broken the way the settings are stored in this PR to make backwards compatibility with old maps work again.

[retrogamedesigner]

Yes. I remapped inputs, including Controller Hotkeys, and everything works perfectly in X-Input mode on Linux! Amazing work! I truly appreciate it.

[DavidRGriswold]

@retrogamedesigner I have added one more feature to this PR - now for both input profiles and hotkeys, there is a combobox that lets you choose whether to apply the mappings to "just this controller" (the old system that uses guid+port), "controllers of this type" (just uses guid - so it should work for two different controllers of exactly the same type and connection mode), or "all controllers" (so it tries to apply to whatever controllers are hooked up). Whenever you have time, I'd love if you could test if those features work for you. If you don't have two controllers of the same type, that's okay, the main thing is the difference between All Controllers vs This Controller.

No rush, this PR won't be looked at until after the next release, and I think it is feature-complete at this point.

[retrogamedesigner]

Tested the three settings for applying Profiles/Inputs/Hotkeys (Only the Mapped Controller, Controllers of the Mapped Type, and All Controllers), both with two identical controllers and two different controllers. All three options appear to be working as expected for me.

[GottemHams]

It seems multiple input profiles no longer work correctly with this PR, i.e. the non-hotkey stuff under Controls > Input tab.

I have 2 profiles and in the main Azahar release it shows:

• Keyboard: A is set to Return
• Switch Pro Controller: A is set to Button 0

But when I download the macos artifact from the latest build of this PR:

• Switch Pro Controller (last selected profile): A is set to Button 0
• Keyboard: switching to this profile actually doesn't update any UI, as it remains Button 0

At first I thought maybe I messed up the mapping myself, so I "switched" to Keyboard and updated the button to Return again. This results in the Keyboard profile being renamed to Switch Pro Controller, with the mapping changed to Return. Now I have 2 profiles named Switch Pro Controller and since the UI doesn't update when switching between them, they both look like they're using keyboard controls.

As a workaround I simply configured the controller-based hotkeys first (which does seem to work), then used main Azahar to set the actual button configurations.

[DavidRGriswold]

Thanks for this feedback I will attempt to replicate and fix later this week.

[DavidRGriswold]

@GottemHams The version building now should fix this bug. It was a very silly mistake fixed in one line! Thanks for catching it. Big PRs like this have so many little things to test...

[GottemHams]

Seems to work better, but still not 100% right. :>

On the Keyboard profile, I'm trying to map WASD to the circle pad and arrow keys to the C-stick (I don't normally use M&K, this is just to have something when the controller is unavailable). This results in something pretty weird:

Opening the exact same profile in Azahar main works just fine though:

I made no changes via the main release, so it looks like the profile actually did store the correct mappings but it's just being displayed incorrectly. This problem does not seem to occur with any other buttons, only these 2 sections (including the diagonals).

[DavidRGriswold]

Okay! I will check that soon.

[Daisouji]

The Msys2 build crashes at config.

[codeman4033]

MSVC wasn't building, but to save you some time, here's a patch that fixes all the errors

diff --git a/src/citra_qt/hotkey_monitor.cpp b/src/citra_qt/hotkey_monitor.cpp
index 7ffce7df1..97301524d 100644
--- a/src/citra_qt/hotkey_monitor.cpp
+++ b/src/citra_qt/hotkey_monitor.cpp
@@ -61,12 +61,12 @@ void ControllerHotkeyMonitor::checkAllButtons() {
             if (it.hk->action) {
                 it.hk->action->trigger();
             }
-            for (auto const& [name, hotkey_shortcut] : it.hk->shortcuts) {
+            for (auto const& [hotkey_name, hotkey_shortcut] : it.hk->shortcuts) {
                 if (hotkey_shortcut && hotkey_shortcut->isEnabled()) {
                     QWidget* parent = qobject_cast<QWidget*>(hotkey_shortcut->parent());
                     if (!parent)
                         continue;
-                    if (name == QStringLiteral("move down")) {
+                    if (hotkey_name == QStringLiteral("move down")) {
                         std::cout << "move down triggered before context check" << std::endl;
                     }
                     bool shouldFire = true;
diff --git a/src/input_common/sdl/sdl_impl.cpp b/src/input_common/sdl/sdl_impl.cpp
index 29a972f74..eba692d23 100644
--- a/src/input_common/sdl/sdl_impl.cpp
+++ b/src/input_common/sdl/sdl_impl.cpp
@@ -463,7 +463,7 @@ public:
         : joysticks(joysticks_), button(button_), isController(isController_), port(port_) {}
 
     bool GetStatus() const override {
-        if (port >= 0 && joysticks && joysticks->size() > port && joysticks->at(port)) {
+        if (port >= 0 && joysticks && (int)(joysticks->size()) > port && joysticks->at(port)) {
             return joysticks->at(port)->GetButton(button, isController);
         }
         for (const auto& joystick : *joysticks) {
@@ -489,7 +489,7 @@ public:
         : joysticks(joysticks_), hat(hat_), direction(direction_), port(port_) {}
 
     bool GetStatus() const override {
-        if (port >= 0 && joysticks && joysticks->size() > port && joysticks->at(port)) {
+        if (port >= 0 && joysticks && (int)(joysticks->size()) > port && joysticks->at(port)) {
             return joysticks->at(port)->GetHatDirection(hat, direction);
         }
         for (const auto& joystick : *joysticks) {
@@ -515,7 +515,7 @@ public:
           trigger_if_greater(trigger_if_greater_), isController(isController_), port(port_) {}
 
     bool GetStatus() const override {
-        if (port >= 0 && joysticks && joysticks->size() > port && joysticks->at(port)) {
+        if (port >= 0 && joysticks && (int)(joysticks->size()) > port && joysticks->at(port)) {
             return joysticks->at(port)->GetAxis(axis, isController);
         }
         for (const auto& joystick : *joysticks) {
@@ -548,7 +548,7 @@ public:
 
     std::tuple<float, float> GetStatus() const override {
         float rMax = 0.0f, xMax = 0.0f, yMax = 0.0f;
-        if (port >= 0 && joysticks && joysticks->size() > port && joysticks->at(port)) {
+        if (port >= 0 && joysticks && (int)(joysticks->size()) > port && joysticks->at(port)) {
             const auto [x, y] = joysticks->at(port)->GetAnalog(axis_x, axis_y, isController);
             const float r = std::sqrt((x * x) + (y * y));
             if (r > deadzone) {
@@ -663,7 +663,7 @@ public:
         }
 
         if (params.Has("axis")) {
-            bool controller = params.Get("api", "joystick") == "controller";
+            bool is_controller = params.Get("api", "joystick") == "controller";
             const int axis = params.Get("axis", 0);
             const float threshold = params.Get("threshold", 0.5f);
             const std::string direction_name = params.Get("direction", "");
@@ -677,7 +677,7 @@ public:
                 LOG_ERROR(Input, "Unknown direction {}", direction_name);
             }
             return std::make_unique<SDLAxisButton>(joysticks, axis, threshold, trigger_if_greater,
-                                                   port, controller);
+                                                   port, is_controller);
         }
         const int button = params.Get("button", 0);
         return std::make_unique<SDLButton>(joysticks, button, port, controller);
@@ -730,7 +730,7 @@ public:
         auto joysticks = state.GetJoysticksByGUID(guid);
         if (joysticks->empty())
             return std::make_unique<SDLMotion>(nullptr);
-        auto joystick = joysticks->size() > port ? joysticks->at(port) : joysticks->at(0);
+        auto joystick = (int)(joysticks->size()) > port ? joysticks->at(port) : joysticks->at(0);
         return std::make_unique<SDLMotion>(joystick);
     }
 
@@ -755,7 +755,7 @@ public:
         const int port = params.Get("port", 0);
         const int touchpad = params.Get("touchpad", 0);
         auto joysticks = state.GetJoysticksByGUID(guid);
-        auto joystick = joysticks->size() > port ? joysticks->at(port) : joysticks->at(0);
+        auto joystick = (int)(joysticks->size()) > port ? joysticks->at(port) : joysticks->at(0);
         return std::make_unique<SDLTouch>(joystick, touchpad);
     }
 
@@ -975,7 +975,7 @@ public:
             auto id = event.jaxis.which;
             auto value = event.jaxis.value;
             auto timestamp = event.jaxis.timestamp;
-            auto button = event.jbutton.button;
+            //auto button = event.jbutton.button;
             bool controller = false;
             switch (event.type) {
             case SDL_CONTROLLERAXISMOTION: {

[DavidRGriswold]

@codeman4033 Thanks for your patch! I didn't implement it exactly but it helped a lot to identify the places MSVC that my own linter/compiler don't care about. Hopefully the version I just pushed will build, I'll check back in an hour on the CI.

@Daisouji I can't test msys2 easily right now (my windows PC is not with me) so if you could test the latest push once it builds and see if I magically fixed your issue that would be great. Otherwise I will have to try to repro later this week/weekend or whenever I can get to my Windows PC.

@GottemHams I believe I have fixed your issue with the latest push, works fine on my Mac now. Keyboard-as-analog-directions was a case I had not checked (and as you suspected it was working fine, just not displaying correctly).

[Daisouji]

@codeman4033 Thanks for your patch! I didn't implement it exactly but it helped a lot to identify the places MSVC that my own linter/compiler don't care about. Hopefully the version I just pushed will build, I'll check back in an hour on the CI.

@Daisouji I can't test msys2 easily right now (my windows PC is not with me) so if you could test the latest push once it builds and see if I magically fixed your issue that would be great. Otherwise I will have to try to repro later this week/weekend or whenever I can get to my Windows PC.

@GottemHams I believe I have fixed your issue with the latest push, works fine on my Mac now. Keyboard-as-analog-directions was a case I had not checked (and as you suspected it was working fine, just not displaying correctly).

It's not crashing upon opening configuration with @codeman4033 change.

[GottemHams]

@GottemHams I believe I have fixed your issue with the latest push, works fine on my Mac now. Keyboard-as-analog-directions was a case I had not checked (and as you suspected it was working fine, just not displaying correctly).

It does seem to work fine here as well. :>

[schuay]

Fwiw, I've tried your patch and Left/Right Stick controls seem broken for me. Mapping works fine, but no in-game response. Let me know what addtl info would be useful.

[PabloMK7]

@OpenSauce04 LGTM, will wait for your input as well.