Sort Remaps by Connected Controller

[s3gfaultx]

Description

This pull request introduces a new feature to RetroArch that enables users to save remapped controls with different controllers. RetroArch will now sort and apply remap configurations based on the specific controller used. This ensures that users custom remaps are respected across different controllers, enhancing the overall gaming experience.

Note: By default, this feature is NOT enabled. Due to the new folder structure introduced, existing remap files will not be loaded once this feature is enabled without manually moving the existing remaps to the appropriate folders for each controller.

Key Features:

1. Controller-Specific Remap Saving: Users can save remap configurations for individual controllers, allowing for seamless transitions between different controllers without losing custom settings.

2. Automatic Sorting: RetroArch automatically sorts and applies the correct remap configuration based on the controller connected, ensuring consistent gameplay.

3. Improved User Experience: This feature eliminates the need for manual remap adjustments when switching controllers, providing a smoother and more personalized gaming experience.

[hizzlekizzle]

@sonninnos and @LibretroAdmin

Probably good to ping @warmenhoven too, in case there is some unforeseen complication in Apple land.

[warmenhoven]

Working off memory, this looks fine to me, the controller name on Apple should get picked up from the mfi api correctly. I won't have a chance to test it for the next week or so though. If people are itching to get this in more quickly than that, I don't need you to wait on me, I can test it when I'm back.

[Ryunam]

This is amazing! I haven't had the time to test the implementation yet, but this could probably address a pretty ancient feature request of mine, dating back to 2019: #9012

[cmitu]

If this does what I assume it does (i.e. use the controller name for the remap file), some consideration for 'sanitizing' the name may be in order, since it may contain reserved characters (i.e. / on Linux/Unix, : on Windows, etc.).

For instance, the Xbox xpad driver can return a device name with / (see here).

[s3gfaultx]

@cmitu, good catch and thank you for the advice!

I've added sanitization to the device name and should resolve that concern.

[s3gfaultx]

This is amazing! I haven't had the time to test the implementation yet, but this could probably address a pretty ancient feature request of mine, dating back to 2019: #9012

I think this will accommodate your feature request, even though the implementation is a bit different then how it was described. I would encourage you to give it a try and share any feedback you might have. Otherwise, I would consider this complete, as it is.

[s3gfaultx]

@LibretroAdmin, thank you for the review -- I've resolved the requested changes.

[s3gfaultx]

Anything needed on my part to get this moving?

[hizzlekizzle]

@LibretroAdmin any further changes needed?

[bcat]

Hey, this is an awesome feature! I've been wanting RetroArch to support something like this for a long time, so I figured I'd test it out (in a clean install built from this PR on Windows, if that matters). Unfortunately, although saving remaps to the per-controller path seems to work fine, the remaps don't appear to load next time RA is started.

Log files:

• Saving remap. Works. (Note the [INFO] [Remap]: File saved successfully: "C:\msys64\home\bcat\retroarch\config\remaps\Mupen64Plus-Next/8BitDo N64 Modkit/Mupen64Plus-Next.rmp". line.)
• Loading remap after restarting RA. Does not work. (Nothing in the log file related to the remap, and the core uses the default controls, not the mapping I configured in the first run.)

Just to be explicit, I had the same (single) controller connected for each run, which uses autoconfig dinput/8Bitdo_N64_Modkit.cfg.

Let me know if there's anything else I can do to help debug this, and thanks again for implementing this change!

---

Also, two other bits of feedback (less urgent, please ignore if unwanted):

1. The mapping seems to be based on the input_device name from the controller driver, but AFAIK, these aren't always unique. IMO, it would make sense to be consistent with the autoconfig files and put USB VID/PID/name in the remap file itself, and match remaps using the same logic as autoconfigs do.

2. Right now, the mapping for all players is loaded based on the first connected controller. That works well for single-player use cases (e.g., sometimes I play N64 games with an N64-style controller, and other times I use one with a more "regular" layout), but it isn't as useful for multiplayer use cases. I know it would involve deeper changes to the remap system, but it would be super nice the per-controller remap files only stored settings for one player, and they applied to the player with the connected controller, regardless of controller/player number.

   In other words, if I connect an Xbox controller as player 1 and an N64 controller as player 2, it would be awesome if player 1 got the default Mupen core mappings that suit RetroPad-style controllers, but player 2 got the custom mappings configured for N64-style controllers. And if I reversed the player assignments, the opposite mappings applied.

(I know these things are probably outside the scope of the PR, but I just wanted to write them down while I was thinking about them.)

[bcat]

I hacked in some extra logging, and it looks like config_load_remap isn't actually looking in the per-controller remap path, even though I have the new "Sort Remaps by Gamepad" setting enabled:

[INFO] XXX: remap path: Mupen64Plus-Next
[INFO] XXX: game path: C:\msys64\home\bcat\retroarch\config\remaps\Mupen64Plus-Next\N64 Controller Test (202008190836).rmp
[INFO] XXX: content path: C:\msys64\home\bcat\retroarch\config\remaps\Mupen64Plus-Next\\downloads.rmp
[INFO] XXX: core path: C:\msys64\home\bcat\retroarch\config\remaps\Mupen64Plus-Next\Mupen64Plus-Next.rmp

It saves to the new path, just doesn't load from it.

[bcat]

Moar logging. Seems input_device_name is coming back null when it tries to load the remap file:

[INFO] XXX: sort_remaps_by_controller: 1
[INFO] XXX: joypad_port: 0
[INFO] XXX: input_device_name: (null)

Not sure why though. I only have the one controller connected, and the autoconfig file loaded just fine: [INFO] [Autoconf]: 8BitDo N64 Modkit configured in port 1.

[sonninnos]

Always using / as the path separator is not good, and also having the same path creation code duplicated twice instead of using a common function is not good either.

PATH_DEFAULT_SLASH() and the common path stuff in libretro-common/file/file_path.c to the rescue.

And also that true in menu/menu_setting.c will make the default reset state as enabled.

[hizzlekizzle]

@s3gfaultx any thoughts on the path issue?

[s3gfaultx]

The path issue didn't affect me on Linux.

I have a Windows machine to test on now, I'll take the feedback raised in this thread and get it all fixed up.

Thanks for being gentle on me, there's a ton of code all over the place and I'm still getting up to speed with the architecture.

[bcat]

I have a Windows machine to test on now, I'll take the feedback raised in this thread and get it all fixed up.

I can also help with Windows testing if that's useful (feel free to ping me here). This is functionality that I would love to have, so I'd be happy to help. :)

[hizzlekizzle]

@s3gfaultx no worries! We very much appreciate the contribution and your continuing interest in seeing it through.

[s3gfaultx]

@bcat, I just pushed some more changes -- seems to work with the limited testing I performed.

I'm leaning on continuing to use the device name instead of VID/PID just for ease of identification when working directly with the filesystem. I don't have any devices in my possession that use the same name, but if you have some examples of this, I'll look into it further. I'd assume that any devices with the same name, likely are spoofing the same VID/PID anyways (counterfeit Xbox periperals, etc).

[bcat]

@bcat, I just pushed some more changes -- seems to work with the limited testing I performed.

Nice! I'll try and build/test over this weekend.

I'm leaning on continuing to use the device name instead of VID/PID just for ease of identification when working directly with the filesystem. I don't have any devices in my possession that use the same name, but if you have some examples of this, I'll look into it further. I'd assume that any devices with the same name, likely are spoofing the same VID/PID anyways (counterfeit Xbox periperals, etc).

I want to verify I'm not mistaken, but one place I think the current approach will run into problems is Bluetooth dinput controllers controllers on Windows. IIRC, those currently all report the same input_device (see #13520 for all the gory details), and so the autoconfig impl only matches them on VID/PID.

This actually affects me as a real user. I have a Bluetooth 8BitDo M30 and N64 Modkit, both of which are perfect candidates for this new feature (since they have a physical layout sufficiently different from the RetroPad abstraction), but I suspect they'll end up being indistinguishable without VID/PID matching.

[bcat]

The more I think about this, the more I wish the autoconfig mechanism itself were a little more flexible, since it already has flexible device matching logic that works reasonably well across all the various joypad drivers. More tightly integrating into the autoconfig layer might also help with the second point here regarding applying the mappings to the correct controller number regardless of the order in which controllers were connected.

To be clear though, this might be scope creep for this particular PR, and I'm not saying the current approach is bad. Just brainstorming... :)

[s3gfaultx]

I want to verify I'm not mistaken, but one place I think the current approach will run into problems is Bluetooth dinput controllers controllers on Windows. IIRC, those currently all report the same input_device (see #13520 for all the gory details), and so the autoconfig impl only matches them on VID/PID.

That thead implies they use the same VID/PID and is the cause of the issue -- however, I did change the logic to use the display name in commit ce1d652 and should work with these controllers.

This actually affects me as a real user. I have a Bluetooth 8BitDo M30 and N64 Modkit, both of which are perfect candidates for this new feature (since they have a physical layout sufficiently different from the RetroPad abstraction), but I suspect they'll end up being indistinguishable without VID/PID matching.

I have both of these controllers too, and they work in Linux -- perhaps you can test them for me in Windows.

[bcat]

Sorry for the delay, real life got in the way. :) I'll build and retest on Windows this weekend.

[bcat]

Okay, retested on Windows. Looking good!

• New remappings still save properly. :)
• And now they load and work properly as well: [INFO] [Remaps]: Core-specific remap found at "C:\msys64\home\bcat\RetroArch\config\remaps\Mupen64Plus-Next\8BitDo N64 Modkit\Mupen64Plus-Next.rmp".
• I misunderstood how you were computing the filenames before. I had forgotten it was using input_device_display_name, not input_device. I agree using the display name largely resolves the uniqueness issue.

Two other thoughts from a user perspective:

1. It looks like right now, the new remap mode is mutually exclusive with the previous one. Would it make sense to first look for a per-controller path, and then fall back to the old path if a per-controller mapping isn't found? That would allow your new remap files for "weird-shaped controllers", while still using a default remap for "controllers that look like RetroPads".
2. I still think it would be cool for remaps to automatically apply to the player where the associated controller is connected, rather than always using the first controller to determine which remap is loaded. But I still don't know a good way to do that without significant changes to the remap system, so this is more a "it'd be nice to have someday" type comment. :)

[s3gfaultx]

I'm happy to hear that it's working for you now -- thank you for your help testing.

Once this is merged, I'll investigate improving the remapping experience across multiple devices in a new PR.

[hizzlekizzle]

Alright, with the paths fixed and the requested changes made, do we need any more testing and/or review? @LibretroAdmin @sonninnos @Ryunam

[dvessel]

Discovered a bug related to this. It's ignored when launching a rom through the command line on MacOS. I have not tested it on other platforms. #17321