Closed lbschenkel closed 1 year ago
Before merging, please let me know if I should add crc:6ce8
to the mapping to make it easier for SDL to disambiguate controllers (it might not help a lot since USB Gamepad
is still very generic, but better than nothing).
Here's a fun example of our position vs. label problem :-)
Retrolink (and a few other vendors) ship 2 formats of device with this ICU (and thus same GUID and CRC).
Here's what you've got
and here's the other
With both devices in hand, I'm happy with the mapping currently in the DB, which is an effective superset of both. The mapping conforms to the Sega layout, which seems like an acceptable compromise. This is not the only mapping we differ on from upstream! I may well in the future PR our mapping to SDL, but have not to date. Further input welcome :-)
You're right, I have the SNES one. No problem closing this, a choice must be made.
Thanks for the explanation. Just one remark regarding "effective superset of both": I interpret a "superset" as one being larger but compatible with the other. I disagree with this characterization, because when I test this mapping with my controller then the wrong button presses are registered: A (EAST) becomes B (SOUTH), X (NORTH) becomes "R shoulder", etc. (something like that — writing from memory because I don't have the controller in front of me as I write this) when at least the ABXY buttons should work (in the sense that the correct button position should trigger, not talking about labels). So the mappings are actually incompatible and the SNES controller unusable under this mapping Not that it changes anything regarding the conclusion; just wanted to clarify this.
But as I said, I completely understand that one controller must win. I was not sure which one was the one in the DB, nor if the mapping was actually correct, so I thought that maybe the upstream version should be given preference as I have the exact controller and can verify that it is correct.
I interpret a "superset" as one being larger but compatible with the other
This is the case for the community mapping, which includes all available buttons for both the SNES (note the back
field for the Select button) and Saturn (note shoulders/triggers per Sega layout in README) style device.
Position can not be mapped correctly for both devices. The included map is correct for the Saturn style, and face button labels (thought not position) are correct for the SNES style, which I believe is the best we can do to accommodate both for now.
Probably I'm misunderstanding what you're explaining, but let me explain what I see happening when I use my controller under this mapping. Just to have us in the same page, this is a "SNES controller" with B in the bottom and AXY going counter-clockwise.
A good mapping will produce this (I will use Linux cardinal notation to reduce risk of confusion):
This is how my controller behaves under the mapping in the DB:
So from the perspective of my controller the main 4 buttons are behaving as if they were mapped using label and not positional layout, and the two shoulder buttons are all messed up.
That is why I would not call it a "superset", but an incompatible mapping -- but that is fine, it changes nothing, I was basically nitpicking your usage of superset (or we are seeing it under different light). There's nothing that can be done about this: there's a GUID clash and one controller must win, and you confirmed that the mapping already in the DB is a known good mapping for a different controller, not some old one that might have been done incorrectly.
And given that there are so many controllers in the wild with this particular PID/VID with different wiring, it's totally possible that there's some "SNES controller" out there with different wiring than mine, and that one is actually wired in a way that is closer to the Sega one, and then the mapping in the DB actually works as a superset.
The values are correct as you've laid them out. As mentioned
Position can not be mapped correctly for both devices. The included map is correct for the Saturn style, and face button labels (though not position) are correct for the SNES style, which I believe is the best we can do to accommodate both [device cases] for now.
I edited the above comment several times, so this may well have been miscommunicated.
As for
I would not call it a "superset", but an incompatible mapping
Fair enough, I am talking in terms of "all buttons accounted for", not "positionally correct in all cases", given a correct mapping for both layouts is not possible per device differences.
Off-topic observation:
It's really unfortunate that there are those cheap manufacturers are all using the same board and not bothering to change the PID/VID for their different products. This makes it really hard for non-technical users who want to game in Linux (I am a developer and I can always find a way, but I'm not the typical user) and want things to work out-of-the-box.
Were I the maintainer of SDL, I would have included the number of buttons (and perhaps the order) in the controller GUID (better call it a hash) calculation so in cases like this perhaps the two different controllers would actually result in different hashes. But then again, perhaps not: I'm not sure what the different controllers report at the HID level, and maybe they're so identical underneath that the Linux driver would still expose them in an identical way (same buttons, same order, only that some buttons never trigger since they're not wired or trigger differently due to different wiring).
I know the right place to suggest this is upstream, not here. :-)
Different methods have been suggested but it's a losing battle, see #202. I agree that the more features accounted for in the unique identifier the better. I'm not convinced the latest addition (product name CRC) is much of an improvement, if any.
lsusb:
I personally have this controller and the mappings from SDL upstream are correct, while the mappings in the community DB are not.
This controller is likely to be a case where different manufacturers use the same board (and VID/PID) but have different mappings due to different firmware and/or wired differently.
This commit overwrites the community DB version with the upstream version. While this can overwrite a good mapping for a different controller, in case of conflicts such as this it's better if the community DB agrees with upstream.
Joystick name used for CRC calculation:
USB Gamepad
CRC from joystick name:0xE86C
GUID with CRC:03006ce8790000001100000010010000