kodi-game/game.libretro.genplus

Swapped buttons in default buttonmap

Closed this issue · 5 comments

Bug report

Describe the bug

Here is a clear and concise description of what the problem is:

I was wondering what logic is behind the following default buttonmap:

    <feature name="a" mapto="RETRO_DEVICE_ID_JOYPAD_Y"/>
    <feature name="b" mapto="RETRO_DEVICE_ID_JOYPAD_B"/>
    <feature name="c" mapto="RETRO_DEVICE_ID_JOYPAD_A"/>
    <feature name="x" mapto="RETRO_DEVICE_ID_JOYPAD_L"/>
    <feature name="y" mapto="RETRO_DEVICE_ID_JOYPAD_X"/>
    <feature name="z" mapto="RETRO_DEVICE_ID_JOYPAD_R"/>

While browsing Retroarch's forum, I've come across several 6 buttons Genesis controller mapping suggestions, but none of them is like this.

Finally, I think I get it! What happened here, is that the buttonmap is copied over from Retroarch's default, but the A-B and the X-Y are swapped. That's because Retroarch is using SNES controller as the default map while Retroplayer is using Xbox controller as a default (and these have swapped A-B and X-Y).

So based on the suggestions from the Retroarch's users I propose the following mapping:

I've found this image on the Retroarch's forum, so it has swapped A-B and X-Y as well:
obrazek

For 6 button controller:

    <feature name="a" mapto="RETRO_DEVICE_ID_JOYPAD_A"/>
    <feature name="b" mapto="RETRO_DEVICE_ID_JOYPAD_B"/>
    <feature name="c" mapto="RETRO_DEVICE_ID_JOYPAD_R"/>
    <feature name="x" mapto="RETRO_DEVICE_ID_JOYPAD_X"/>
    <feature name="y" mapto="RETRO_DEVICE_ID_JOYPAD_Y"/>
    <feature name="z" mapto="RETRO_DEVICE_ID_JOYPAD_L"/>

For 3 button controller (if we introduce it later maybe):

    <feature name="a" mapto="RETRO_DEVICE_ID_JOYPAD_X"/>
    <feature name="b" mapto="RETRO_DEVICE_ID_JOYPAD_A"/>
    <feature name="c" mapto="RETRO_DEVICE_ID_JOYPAD_B"/>

Please note that the same problem affects also the Picodrive and the Genplus Wide, so we should correct all of them.

RetroArch Behavior

Here is a clear and concise description of what happens in RetroArch with the same core/game:

If we want to preserve Retroarch's default, then we should at least correct it like this:

    <feature name="a" mapto="RETRO_DEVICE_ID_JOYPAD_X"/>
    <feature name="b" mapto="RETRO_DEVICE_ID_JOYPAD_A"/>
    <feature name="c" mapto="RETRO_DEVICE_ID_JOYPAD_B"/>
    <feature name="x" mapto="RETRO_DEVICE_ID_JOYPAD_L"/>
    <feature name="y" mapto="RETRO_DEVICE_ID_JOYPAD_Y"/>
    <feature name="z" mapto="RETRO_DEVICE_ID_JOYPAD_R"/>

But I would vote for the more logical map above.

To Reproduce

Steps to reproduce the behavior:

Debuglog

The debuglog can be found here:

Screenshots

Here are some links or screenshots to help explain the problem:

Your Environment

Used Operating system:

  • Android
  • iOS
  • Linux
  • OSX
  • Raspberri-Pi
  • Windows
  • Windows UWP

Operating system version/name:

Kodi version: 20.0

note: Once the issue is made we require you to update it with new information or Kodi versions should that be required.
Team Kodi will consider your problem report however, we will not make any promises the problem will be solved.

Good catch. I made libretro buttonmaps for like half our 130 cores by hand, by searching and reading the core source code. It took months.

Manually testing each core would have added years, so I'm relying on people like you to find the mistakes. We'll fix them little by little. Best would be to PR the fix: https://github.com/kodi-game/game.libretro.genplus/blob/master/game.libretro.genplus/resources/buttonmap.xml

Well, I've tested it and it is even more complicated. I've found out that it is already working the way I wanted.

There is another button translation done in the core:
https://github.com/libretro/Genesis-Plus-GX/blob/4d400a673daa2ff393f21ef48fd6a10a6150bd0b/libretro/libretro.c#L461
https://github.com/libretro/picodrive/blob/b2d43acfbc288038749d8a8fdfbcb0474568e043/platform/libretro/libretro.c#L1362

Basically it does the opposite of the current buttonmap.xml, so the mapping is NOT as in Retroarch, but it is what I proposed.

Example:
We map Xbox's controller "A" to RETRO_DEVICE_ID_JOYPAD_Y and then the core itself maps RETRO_DEVICE_ID_JOYPAD_Y to the Sega's joypad "A".
So it is "A" -> "Y" -> "A".

While this is fine for me and it may be fine for Sega Genesis in general (because the mapping of Xbox-like controller to Sega is somewhat specific), it is definitely not the mapping intended by the core developers. It might be a subject to discussion if we prefer to maintain the original Retroarch mapping or the current one. Personally I would keep the current mapping for the 6 button controller and I'd add a 3 button controller which would use "X", "A", "B" on Xbox's controller for Sega's "A", "B", "C" like in Retroarch. That way we can get the best of both, the "X", "A", "B" for 3 button games (so you don't need the right shoulder button), but keep the fully compatible 6 button mapping for advanced games (like the WWF WrestleMania).

The following mapping works 100% like the Retroarch's default (we can use it for the 3 button joypad):

    <feature name="a" mapto="RETRO_DEVICE_ID_JOYPAD_B"/>
    <feature name="b" mapto="RETRO_DEVICE_ID_JOYPAD_A"/>
    <feature name="c" mapto="RETRO_DEVICE_ID_JOYPAD_R"/>
    <feature name="x" mapto="RETRO_DEVICE_ID_JOYPAD_Y"/>
    <feature name="y" mapto="RETRO_DEVICE_ID_JOYPAD_X"/>
    <feature name="z" mapto="RETRO_DEVICE_ID_JOYPAD_L"/>

Please note the swapped "A" <-> "B" and "X" <-> "Y".

P.S.
Which brings me to next idea. Unfortunately, it is likely that buttonmaps for many of the emulators have swapped "A" <-> "B" and "X" <-> "Y".

I've looked at the mappings for NES (Nestopia and FCEUmm) and SNES (Snes9x) and those are definitely swapped as well. We are mapping "A" on Xbox controller (bottom button) to RETRO_DEVICE_ID_JOYPAD_A (which is on SNES controller, so it is the right button). The bottom button on the SNES controller is "B".

It means that when playing Contra on Retroplayer with the default mapping we currently have jump and fire swapped.

EDIT:
You can search for name="a" mapto="RETRO_DEVICE_ID_JOYPAD_A" in buttonmaps. Most of these are probably swapped.

@garbear I've found out that we cannot do what I was suggesting. It might have helped to get better default mapping, but it would break the manual re-mapping, which is even worse.

The core's buttonmap.xml is exactly defined by the code in the libretro part of the core, so it needs to stay exactly as it is. This applies to all cores, so even my suggestions for changing the buttonmap.xml for NES and SNES cores cannot be done, because it would break the manual mapping.

If there is a different place where we can change the default mapping per controller or per system, then we shall do it, but the buttonmap.xml is correct and cannot be changed. Feel free to close this issue as "won't fix". I'm sorry for the false alarm.

We have the "nuclear" option: patch the core. Our build system will apply any patches in depends/common/genplus. However, then the core from the libretro buildbot repo will have swapped buttons because it wasn't built with the patch.

No, the "nuclear" option is NOT needed. It won't work anyway, see the more detailed explanation here:
kodi-game/controller-topology-project#203 (comment)

Let's close this one as "won't fix", because it can't be fixed in game.libretro.genplus. I will play with the maps in peripheral.joystick and see if we can achieve the desired effect there.