Issue #1298 open

setGamepadMapping does not correctly replace old mappings

enra64
created an issue

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)

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:

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.

Comments (9)

  1. enra64 reporter

    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.

  2. enra64 reporter

    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

    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.

  3. enra64 reporter

    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.

  4. enra64 reporter

    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)

  5. enra64 reporter

    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.

  6. Log in to comment