New gamepad types, and "type:" db field

[nstbayless]

• I confirm that I am the author of this code and release it to the SDL project under the Zlib license. This contribution does not contain code from other sources, including code generated by a Large Language Model ("AI").

The gamepad db lacks a good way to actually indicate the gamepad's type (e.g. switchpro, ps5, ...). With this PR, it's now part of the db format (e.g. type:switchpro). If type: is omitted it will still use the same checks as before to determine the type.

Changes

The database parsing remains backward-compatible -- the (confusingly-named) USE_BUTTON_LABELS hint is still respected. However, I removed support for face: since it seems like these were not documented nor used anywhere, not even in the larger SDL gamepad db, except essentially as a private way for SInput controllers to squeeze some additional info through the db string. (SInput now uses the new type: field instead for that information.)

Also adds these gamepad types: n64, wii, and genesis. These are common but very different kinds of gamepads to anything else in the list, so I think it's important they have their own label.

This is part of a longer effort to make it easier for application developers to implement standardized support for button labels and confirm/cancel/etc/actions easier, which vary from gamepad to gamepad.

Existing Issue(s)

• New gamepad types, labels, and confirm/cancel: New gamepad types, button labels, conventional/default "confirm"/"cancel" buttons #15891

[nstbayless]

• Added comments explaining that the gamepad types have certain minimum requirements
• restored hint: to the db entries that used them
• SInput now falls back to SNES or NES if it doesn't meet the requirements for switch pro
• No such system for SInput gamecube and sony yet... do we really need one though? How likely is it for a SInput gamepad to report gamecube button layout but not have all gamecube features?

If we're concerned about sony-labelled gamepads which don't satisfy the minimum requirements for any specific playstation gamepads, I'm thinking it might be sensible to add hint:USE_SONY_LABELS to the db entries for such sinput devices.

[slouken]

• Added comments explaining that the gamepad types have certain minimum requirements
• restored hint: to the db entries that used them
• SInput now falls back to SNES or NES if it doesn't meet the requirements for switch pro
• No such system for SInput gamecube and sony yet... do we really need one though? How likely is it for a SInput gamepad to report gamecube button layout but not have all gamecube features?

I have an SInput gamepad that is gamecube form factor, but it has all the features.

If we're concerned about sony-labelled gamepads which don't satisfy the minimum requirements for any specific playstation gamepads, I'm thinking it might be sensible to add hint:USE_SONY_LABELS to the db entries for such sinput devices.

That was what the face enumeration was for. :)

[nstbayless]

I have an SInput gamepad that is gamecube form factor, but it has all the features.

I was wondering about controllers which are missing features.

That was what the face enumeration was for. :)

The problem with this is that it doesn't feed into SDL_GetGamepadButtonLabelForType, which takes only the gamepad type as an argument. I can restore the "face:" entry in the db for SInput controllers, but it won't actually affect how SDL determines face button labels.

[slouken]

The problem with this is that it doesn't feed into SDL_GetGamepadButtonLabelForType, which takes only the gamepad type as an argument. I can restore the "face:" entry in the db for SInput controllers, but it won't actually affect how SDL determines face button labels.

Yeah, I'm not sure what to do here.

[nstbayless]

I'm not sure what to do here.

I'm leaning toward adding SDL_GAMEPAD_TYPE_STANDARD_SONY and SDL_GAMEPAD_TYPE_STANDARD_AXBY as new gamepad types, which imply nothing about the gamepad except for the face button labels. Maybe SDL_GAMEPAD_TYPE_STANDARD_BAYX too if SDL_GAMEPAD_TYPE_NES would be too weird as the fallback.

The alternative is to say that SDL_GAMEPAD_TYPE_GAMECUBE and SDL_GAMEPAD_TYPE_PS3 or something have no actual minimum requirements and imply only button labels. Maybe for switchpro as well.

[slouken]

I'm leaning toward adding SDL_GAMEPAD_TYPE_STANDARD_SONY and SDL_GAMEPAD_TYPE_STANDARD_AXBY as new gamepad types, which imply nothing about the gamepad except for the face button labels.

Yeah, that seems reasonable to me.

[nstbayless]

If at some point we want gamepads to support custom labels, such unusual SInput gamepads could use that system too.

[nstbayless]

Added type:sony, type:axby, type:bayx. Ready for a new review now I think.

[slouken]

Suggested change

	SDL_GAMEPAD_TYPE_STANDARD_BAYX, /**< A non-specific gamepad with B/A/Y/X face button layout */
	SDL_GAMEPAD_TYPE_STANDARD_BAYX, /**< A non-specific gamepad with Nintendo Switch Pro style (B/A/Y/X) face button layout */

[nstbayless]

could equally say SNES style

[slouken]

Descriptive comments here, similar to above?

[nstbayless]

For Genesis and Wii okay. But N64 doesn't have a clear mapping scheme -- SDL gamepad DB doesn't define one, for example. And the "obvious" thing (map c-stick to right stick) is deficient (doesn't support holding left-C and right-C simultaneously). I'd rather leave this unspecified in this PR for now than risk doing damage by introducing a bad standard here.

Also... it's quite astonishing that Genesis maps L and R to LB and LT respectively instead of to LB and RB. But I guess that can't be fixed at this point.

[slouken]

Should we add the button label hints here? Conceptually this is a way to let older code know that this is BAYX face button layout.

[nstbayless]

Right, good point. I forgot that this mapping string might not stay private.

[slouken]

For historical reasons, the mappings must have a trailing comma.

[slouken]

Did you mean to add ",type:steam," here?

[nstbayless]

Yes. But I don't understand what a "steam" controller is. The documentation is unclear. Am I right to assume that it includes steam deck, the original steam controller, and steam controller 2? Even though the original steam controller has a very different layout?

Does "steam" refer to any controller under Steam Input? (I believe underlying controller type is opaque through SI but I could be mistaken.)

[slouken]

Confirmed.

[nstbayless]

My understanding is that on the Switch, horizontal joycon button labels usually look like this (screenshot from retroarch) rather than actually using "A"/"B" etc.:

We can still report this as "A"/"B" for but I'm unsure what the "right" thing to do is.

[slouken]

Technically this should be type:ps3 and the one below this is type:ps4, but this is probably fine for arcade sticks.

[nstbayless]

They don't match the form factor of a PS3/PS4 controller. Well, we can come down on this issue more cleanly in a future PR?

[slouken]

I haven't gone through this exhaustively, but I assume that you made sure everything using hint:!SDL_GAMECONTROLLER_USE_BUTTON_LABELS:=1 has some form of BAYX type and every controller with some form of BAYX type has that hint? Also the same for the gamecube hint?

You should be careful, because if there are entries for BAYX controllers without the hint, they may actually be mapping face buttons as ABXY.

[smcv]

I assume that you made sure everything using hint:!SDL_GAMECONTROLLER_USE_BUTTON_LABELS:=1 has some form of BAYX type and every controller with some form of BAYX type has that hint?

This seems like a job for a unit test that will fail the build if we get this wrong in future?

[nstbayless]

I agree.

[slouken]

In general this is looking good. Lots of feedback above. @sezero, can you review once the feedback is addressed?

[sezero]

@sezero, can you review

The whole thing is rather alien to me, and I couldn't read it
in entirety, but this change:

-    int version = analogStyle;
-    version = (version * (int)SINPUT_BUMPERSTYLE_MAX) + bumperStyle;
-    version = (version * (int)SINPUT_TRIGGERSTYLE_MAX) + triggerStyle;
-    version = (version * (int)SINPUT_PADDLESTYLE_MAX) + paddleStyle;
-    version = (version * (int)SINPUT_METASTYLE_MAX) + metaStyle;
-    version = (version * (int)SINPUT_TOUCHSTYLE_MAX) + touchStyle;
-    version = (version * (int)SINPUT_MISCSTYLE_MAX) + miscStyle;
+    Uint16 version = SINPUT_PACK_VERSION(miscStyle, touchStyle, metaStyle, paddleStyle, triggerStyle, bumperStyle, analogStyle);

/****/

+/* Pack SInput style values into a single Uint16
+ * (can fit in the version field of the GUID) */
+#define SINPUT_PACK_VERSION(misc, touch, meta, paddle, trigger, bumper, analog) \
+    ((Uint16)(((((((                                                            \
+        (analog)) * SINPUT_BUMPERSTYLE_MAX +                                    \
+        (bumper)) * SINPUT_TRIGGERSTYLE_MAX +                                   \
+        (trigger)) * SINPUT_PADDLESTYLE_MAX +                                   \
+        (paddle)) * SINPUT_METASTYLE_MAX +                                      \
+        (meta)) * SINPUT_TOUCHSTYLE_MAX +                                       \
+        (touch)) * SINPUT_MISCSTYLE_MAX +                                       \
+        (misc)))

... unless I'm missing something, doesn't seem to yield the same vale before:
Intentional, I assume?

[nstbayless]

... unless I'm missing something, doesn't seem to yield the same vale before: Intentional, I assume?

I'm not sure I see how it's different.