setGamepadMapping does not correctly replace old mappings

[slime73]

Original report by enra64 (Bitbucket: enra64, GitHub: enra64).

---

It seems that the setGamepadMapping function in src/modules/joystick/sdl/JoystickModule.cpp does not correctly find old instances of a mapping. So, for example, when starting out with the following mapstr (for the bluetooth-xbox-one-2016-controller on linux)

#!c++

050000005e040000fd02000003090000,Xbox one 2016 wireless bluetooth linux,a:b0,b:b1,x:b3,y:b4,start:b11,leftstick:b13,rightstick:b14,leftshoulder:b7,rightshoulder:b8,dpup:h0.1,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,leftx:a0,lefty:a1,rightx:a2,righty:a3,lefttrigger:a5,righttrigger:a4,

we end up with this when trying to set leftshoulder:

#!c++

050000005e040000fd02000003090000,Xbox one 2016 wireless bluetooth linux,a:b0,b:b1,x:b3,y:b4,start:b11,leftstick:b13,rightstick:b14,leftshoulder:b7,rightshoulder:b8,dpup:h0.1,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,leftx:a0,lefty:a1,rightx:a2,righty:a3,lefttrigger:a5,righttrigger:a4,leftshoulder:b6,

containing leftshoulder twice.

I see two issues here, feel free to correct me though: in line 244 we check for ", " in the mapstr, even though all examples I saw and the getGamepadMapping lower in the file searches for ",", resulting in no find results when there is no whitespace after the comma, and using mapstr.find_first_of(",", findpos) instead of mapstr.find_first_of(",", findpos + 1), resulting in always getting a length of zero, because find_first_of can also return results at the given index, thus always returning findpos.

[slime73]

Original comment by Bart van Strien (Bitbucket: bartbes, GitHub: bartbes).

---

That looks like the right diagnosis, I'll fix this tomorrow, when I'll be able to test the fix.

[slime73]

Original comment by enra64 (Bitbucket: enra64, GitHub: enra64).

---

That's cool, thanks. I think there may be some more to this bug, because I was not yet able to use a remapped control. Maybe I overlooked something though.

See also the forum post I created earlier, especially my last post. The first post has an attachment that should let you test a resolution easily.

I will test if it works with remapping manually set mappings when I have time.

[slime73]

Original comment by enra64 (Bitbucket: enra64, GitHub: enra64).

---

So remapping a button that was previously mapped using setGamepadMapping is effective.

After playing around with SDLs https://hg.libsdl.org/SDL/file/007dfe83abf8/test/testgamecontroller.c (I compiled with gcc testgamecontroller.c -D_REENTRANT -I/usr/include/SDL2 -lSDL2 -o test && ./test 0), I determined that it is possible to remap the controller (only?) by calling

#!c++

int status = SDL_GameControllerAddMapping("050000005e040000fd02000003090000,Xbox one 2016 wireless bluetooth linux,a:b0,b:b1,x:b3,y:b4,start:b11,leftstick:b13,rightstick:b14,leftshoulder:b6,rightshoulder:b7,dpup:h0.1,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,leftx:a0,lefty:a1,rightx:a2,righty:a3,lefttrigger:a5,righttrigger:a4,Linux,");

before opening the joystick, like here or here or here.

Unsurprisingly that also means re-connecting the joystick will load the mapping. It would, however, be nice if that was not necessary, as especially with wireless controllers that takes some time, not to think of multiple controllers...

Unfortunately, neither retrieving the mapping from SDL nor the return status of SDL_GameControllerAddMapping provide any indication if the mapping will not be applied.

Maybe controllers with a matching GUID could be re-opened when their mapping is changed? Presumably this would be done before the player is ingame, so it would not cause too much of a problem.

[slime73]

Original comment by Bart van Strien (Bitbucket: bartbes, GitHub: bartbes).

---

I had hoped my commit would show up.. anyway, I fixed the substitution issue in a6278a406bfbdafaf35f0aaac4fa716518a8e738.

Testing this, however, the mapping applies immediately on my machine. What version of SDL2 are you using? Note that there's already code that reopens all joysticks with a matching GUID (checkGamepads).

[slime73]

Original comment by enra64 (Bitbucket: enra64, GitHub: enra64).

---

I used version 2.0.5. At some point I checked that it was also compiled against this version.

The issue occurred for me only when I tried to remap the controller that was recognized as a gamepad per default, creating a new mapping for a joystick and changing it again did not have any problems.

Recently I only used loadGamepadMappings, because I think it uses the same functions anyways.

I will try to reproduce the problem on my laptop for now.

[slime73]

Original comment by Bart van Strien (Bitbucket: bartbes, GitHub: bartbes).

---

Same, SDL 2.0.5, using a gamepad that has a default mapping, but I only tried love.joystick.setGamepadMapping.

[slime73]

Original comment by Alex Szpakowski (Bitbucket: slime73, GitHub: slime73).

---

I wonder if #1244 is related?

[slime73]

Original comment by enra64 (Bitbucket: enra64, GitHub: enra64).

---

So with my controller this does not work on my laptop either. I was finally able to "fix it" by adding

SDL_GameControllerAddMapping("050000005e040000fd02000003090000,Xbox one 2016 wireless bluetooth linux,a:b0,b:b1,x:b3,y:b4,start:b11,leftstick:b13,rightstick:b14,leftshoulder:b6,rightshoulder:b7,dpup:h0.1,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,leftx:a0,lefty:a1,rightx:a2,righty:a3,lefttrigger:a5,righttrigger:a4,Linux,");
SDL_GameControllerClose(controller);
controller = SDL_GameControllerOpen(deviceindex);

to Joystick::openGamepad:208.

The weird thing is that i checked that SDL_GameControllerAddMapping is called with the exact same parameter from JoystickModule:425, after which SDL_GameControllerClose and SDL_GameControllerOpen are called in Joystick::openGamepad, so it should work? I am kinda at my wit's end here :/

Please note that I modified Joystick::openGamepad:203 to always close the joystick by changing the line to if (1 || isGamepad()).

setGamepadMapping does not work, either. Or I am using it wrong? Shouldn't calling love.joystick.setGamepadMapping("050000005e040000fd02000003090000", "leftshoulder", "button", 7, nil) at least fix the leftshoulder mapping? (the mapping string is correct)

[slime73]

Original comment by enra64 (Bitbucket: enra64, GitHub: enra64).

---

I think I found the problem for real, now. I can use setGamepadMapping as well as loadGamepadMapping, using the following patch:

# HG changeset patch
# User enra64
# Date 1501531680 -7200
#      Mon Jul 31 22:08:00 2017 +0200
# Branch minor
# Node ID e412b68a58b23ae5b2aea57575e361e9118098f9
# Parent  22349978bbefed05c65d1e8290594b80adb69230
Fixed SDL not loading modified mapping strings for some cases

diff -r 22349978bbef -r e412b68a58b2 src/modules/joystick/sdl/JoystickModule.cpp
--- a/src/modules/joystick/sdl/JoystickModule.cpp   Sat Jul 29 23:22:54 2017 -0300
+++ b/src/modules/joystick/sdl/JoystickModule.cpp   Mon Jul 31 22:08:00 2017 +0200
@@ -346,7 +346,7 @@

        for (auto stick : activeSticks)
        {
-           if (stick->isGamepad() || guid.compare(stick->getGUID()) != 0)
+           if (guid.compare(stick->getGUID()) != 0)
                continue;

            // Big hack time: open the index as a game controller and compare
@@ -355,12 +355,14 @@
            if (controller == nullptr)
                continue;

+           // GameController objects are reference-counted in SDL.
            SDL_Joystick *sdlstick = SDL_GameControllerGetJoystick(controller);
-           if (sdlstick == (SDL_Joystick *) stick->getHandle())
-               stick->openGamepad(d_index);
+           bool reopen_gamepad = sdlstick == (SDL_Joystick *) stick->getHandle();
+           SDL_GameControllerClose(controller);

-           // GameController objects are reference-counted in SDL.
-           SDL_GameControllerClose(controller);
+           // re-open GameController if necessary
+           if (reopen_gamepad)
+               stick->openGamepad(d_index);    
        }
    }
 }

My guess is that the reference counting didn't like having the GameController object still open while re-opening it in openGamepad. Would be cool if this made it into the next love2d version. Do you have any suggestions for workarounds in 0.10.2?

Also, it would be cool if the wiki mentioned that setGamepadMapping uses the SDL button id + 1. I'm not quite sure if that is also true for axes, or I would have edited the wiki.

[slime73]

It seems your fix was applied years ago but we forgot to close the bug report. I also edited the wiki pages to clarify that set/getGamepadMapping uses 1-based index values.