flyinghead / flycast

Flycast is a multiplatform Sega Dreamcast, Naomi, Naomi 2 and Atomiswave emulator
GNU General Public License v2.0
1.46k stars 174 forks source link

Different controllers with the same name overwrite the same cfg mapping #516

Open baarreth opened 2 years ago

baarreth commented 2 years ago

Is your feature request related to a problem? Please describe. I have two (crappy) controllers with the same name "USB Gamepad ", but they have different vendors and different mappings. So, when I set one controller, it creates "~/.config/flycast/mappings/USB Gamepad .cfg". However, when I set the second one, it overwrites the same cfg file.

I am on Linux (Xubuntu 20.04).

from /proc/bus/input/devices: I: Bus=0003 Vendor=0079 Product=0011 Version=0110 I: Bus=0003 Vendor=0810 Product=0007 Version=0110

SDL_JoystickGetGUIDString() from SDL lib returns the following GUIDs: 03000000790000001100000010010000 03000000100800000700000010010000

Describe the solution you'd like If possible, please, include the SDL GUID string into the gamepad file name. So, instead of a single "SDL_USB Gamepad .cfg" file, I would have two files: "SDL_03000000790000001100000010010000_USB Gamepad .cfg" "SDL_03000000100800000700000010010000_USB Gamepad .cfg"

Describe alternatives you've considered This is not an alternative, but an informal Pull request. I believe you might find a better solution.

I did the following changes to put the GUID string on the cfg mapping filenames.

In core/input/gamepad_device.cpp, in std::string GamepadDevice::make_mapping_filename function:

-       std::string mapping_file = api_name() + "_" + name();
+       std::string mapping_file = (!api_id().empty()? api_name() + "_" + api_id() + "_" : api_name() + "_") + name();

In core/input/gamepad_device.h, in class GamepadDevice:

        const std::string& api_name() { return _api_name; }
+       const std::string& api_id() { return _api_id; }
-       GamepadDevice(int maple_port, const char *api_name, bool remappable = true)
-               : _api_name(api_name), _maple_port(maple_port), _input_detected(nullptr), _remappable(remappable)
+       GamepadDevice(int maple_port, const char *api_name, const char *api_id = NULL, bool remappable = true)
+               : _api_name(api_name), _api_id(api_id?:""), _maple_port(maple_port), _input_detected(nullptr), _remappable(remappable)
        std::string _api_name;
+    std::string _api_id;

In core/sdl/sdl_gamepad.h, in class SDLGamepad:

-   SDLGamepad(int maple_port, int joystick_idx, SDL_Joystick* sdl_joystick)
-       : GamepadDevice(maple_port, "SDL"), sdl_joystick(sdl_joystick)
+   SDLGamepad(int maple_port, int joystick_idx, SDL_Joystick* sdl_joystick, const char * api_id)
+       : GamepadDevice(maple_port, "SDL", api_id), sdl_joystick(sdl_joystick)
-       INFO_LOG(INPUT, "SDL: Opened joystick %d on port %d: '%s' unique_id=%s", sdl_joystick_instance, maple_port, _name.c_str(), _unique_id.c_str());
+       INFO_LOG(INPUT, "SDL: Opened joystick %d on port %d: '%s' unique_id=%s guid=%s", sdl_joystick_instance, maple_port, _name.c_str(), _unique_id.c_str(), api_id);

In core/sdl/sdl.cpp, in static void sdl_open_joystick function:

-       std::shared_ptr<SDLGamepad> gamepad = std::make_shared<SDLGamepad>(index < MAPLE_PORTS ? index : -1, index, pJoystick);
+       SDL_JoystickGUID sdl_guid = SDL_JoystickGetGUID(pJoystick);
+       char sdl_guid_str[1024];
+       SDL_JoystickGetGUIDString(sdl_guid, sdl_guid_str, sizeof(sdl_guid_str));
+       std::shared_ptr<SDLGamepad> gamepad = std::make_shared<SDLGamepad>(index < MAPLE_PORTS ? index : -1, index, pJoystick, sdl_guid_str);

In shell/android-studio/flycast/src/main/jni/src/android_gamepad.h

-               : GamepadDevice(maple_port, "Android", id != VIRTUAL_GAMEPAD_ID), android_id(id)
+               : GamepadDevice(maple_port, "Android", NULL, id != VIRTUAL_GAMEPAD_ID), android_id(id)

In core/input/keyboard_device.h

-               : GamepadDevice(maple_port, apiName, remappable) {
+               : GamepadDevice(maple_port, apiName, NULL, remappable) {

Additional context IMG-20220128-WA0002

Screenshot_2022-01-28_00-13-23

image

baarreth commented 2 years ago

I've created a fork with this change. Maybe it is easier to see there the code I am suggesting. https://github.com/baarreth/flycast/commit/bd3196323c6c427d91adf07a166fbed17af43d2e

flyinghead commented 2 years ago

Thank you for providing a patch!

In the mean time, there's an existing workaround to this issue by renaming/copying the .cfg file manually to the following names:

SDL_USB Gamepad -sdl_joystick_0.cfg
SDL_USB Gamepad -sdl_joystick_1.cfg

If these files exist, then each gamepad will use its own mapping file.

I'll see how I can merge your changes to avoid this problem in the future.

j8r commented 2 years ago

Not really related to this issue, currently there is no way to configure a default mapping for all controllers.l (e.g. a Default.cfg). I may be good to also have also this use case in mind when redesigning the code.

baarreth commented 2 years ago

Not really related to this issue, currently there is no way to configure a default mapping for all controllers.l (e.g. a Default.cfg). I may be good to also have also this use case in mind when redesigning the code.

I agree. There is no way to create a default mapping. For example, each one of the four modes of a 8BitDo controller has a different layout. Also, there are extreme cases, like two controllers with exactly the same hardware ("DragonRise Inc. Generic USB Joystick ", sdl_guid:03000000790000000600000010010000 axes:5 hats:1 balls:0 buttons:12), but with completely different housings:

image

Buttons 4 and 5 are respectively B and A on the N64 controller, and L1 and R1 on the Dualshock controller.

Currently, I am doing the same as Recalbox/Retropie/Batocera... I set the layouts for each controller on EmulationStation, and then I use scripts to generate the mappings just before each emulator call. So, the workaround proposed by flyinghead really works for me. Before the emulator call, I know the sdl ids for each controller, and then I can create a "SDL_USB Gamepad -sdl_joystick_0.cfg" and "SDL_USB Gamepad -sdl_joystick_1.cfg". The only small drawback is that if I plug and unplug 4 controllers, the sdl ids easily change among them (especially if they are Bluetooth), and soon the mapping folder will have a lot of unnecessary and redundant cfg files.

Also, I like the idea of keeping the guid on the name of the mapping file because it is EmulationStation friendly. The es_input.cfg from ES identify the controllers by its sdl_guid.

shakangel commented 2 years ago

I have the same issue but with 2 Brook PS1/PS2 to PC Adapters. Both adapter get recognized by windows as "P4 Wired GamepadV2.8". In flycast there appears 4 names actually but when I edit one of them... all get edited. I tried the renaming suggestion found above but it doesn't seem to work for me or I'm missing something. Please advise since the controllers attached to both adapters both will have different button layouts Brook Adapters .

baarreth commented 2 years ago

I have the same issue but with 2 Brook PS1/PS2 to PC Adapters. Both adapter get recognized by windows as "P4 Wired GamepadV2.8". In flycast there appears 4 names actually but when I edit one of them... all get edited. I tried the renaming suggestion found above but it doesn't seem to work for me or I'm missing something. Please advise since the controllers attached to both adapters both will have different button layouts

Make sure you are using the current version of flycast, you are working on the mapping folder (where flycast read/write the cfg files) and the name of the cfg mapping files are correct. In my case, the name of my controller is "USB Gamepad " (with this extra space). Therefore, the cfg files must have an annoying extra space before the "-": "USB Gamepad -sdl_joystick_0.cfg" "USB Gamepad -sdl_joystick_1.cfg"

EDIT: It hit me just now that each one of your adapters appear doubled. Do you understand why that happens? Is this an SDL/Windows issue?

shakangel commented 2 years ago

I have the same issue but with 2 Brook PS1/PS2 to PC Adapters. Both adapter get recognized by windows as "P4 Wired GamepadV2.8". In flycast there appears 4 names actually but when I edit one of them... all get edited. I tried the renaming suggestion found above but it doesn't seem to work for me or I'm missing something. Please advise since the controllers attached to both adapters both will have different button layouts

Make sure you are using the current version of flycast, you are working on the mapping folder (where flycast read/write the cfg files) and the name of the cfg mapping files are correct. In my case, the name of my controller is "USB Gamepad " (with this extra space). Therefore, the cfg files must have an annoying extra space before the "-": "USB Gamepad -sdl_joystick_0.cfg" "USB Gamepad -sdl_joystick_1.cfg"

EDIT: It hit me just now that each one of your adapters appear doubled. Do you understand why that happens? Is this an SDL/Windows issue?

It just happens on Flycast since windows just sees two normal adapters. Other emulators work fine with both adapters... it just the way Flycast seems to handle the configuration files for an adapter. I tried the naming convention on flycast dojo-0.4.22 but it didn't work. I will try it in another newer version.

shakangel commented 2 years ago

Nope, I wasn't able to make it even read the configuration file with that naming convention. I tried naming them "SDL_P4 Wired GamepadV2.8_arcade -sdl_joystick_0.cfg" and "P4 Wired GamepadV2.8 -sdl_joystick_0.cfg" but both conventions didn't work. I tried them both on the flycast dojo-0.4.22 (lastest for fightcade) and the latest master build... both failed. I'm not sure what I'm doing wrong here. From where does the "-sdl_joystick_0" part of the naming come from? Is it from some windows configuration that maps a joystick to index 0, 1, 2 and so forth? Maybe in my setup it is a different number? Any advice?

baarreth commented 2 years ago

From where does the "-sdl_joystick_0" part of the naming come from? Is it from some windows configuration that maps a joystick to index 0, 1, 2 and so forth? Maybe in my setup it is a different number? Any advice?

The SDL lib creates an sequential "instance ID" for each joystick it finds on the system, starting from 0. The SDLGamepad class from flycast use this "instance ID" to create a "_unique_id" variable (see core/sdl/sdl_gamepad.h).

_unique_id = "sdl_joystick_" + std::to_string(sdl_joystick_instance);

The GamepadDevice::find_mapping method calls GamepadDevice::make_mapping_filename two times. On the first try, it includes the _unique_id in the mapping filename (see core/input/gamepad_device.cpp).

It just happens on Flycast since windows just sees two normal adapters.

Windows sees two adapters, but SDL may be seeing four. Idk.