New gamepad types, and "type:" db field#15970
Conversation
72bd319 to
286513f
Compare
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 |
I have an SInput gamepad that is gamecube form factor, but it has all the features.
That was what the face enumeration was for. :) |
I was wondering about controllers which are missing features.
The problem with this is that it doesn't feed into |
Yeah, I'm not sure what to do here. |
I'm leaning toward adding The alternative is to say that |
Yeah, that seems reasonable to me. |
|
If at some point we want gamepads to support custom labels, such unusual SInput gamepads could use that system too. |
286513f to
762b00c
Compare
…N, verify some switch pro features for sinput
762b00c to
636f333
Compare
|
Added |
| SDL_GAMEPAD_TYPE_PS4, /**< At least Cross/Circle/Square/Triangle, d-pad, two bumpers, two clickable sticks, two analog triggers, share, start, touchpad, guide */ | ||
| SDL_GAMEPAD_TYPE_PS5, /**< At least Cross/Circle/Square/Triangle, d-pad, two bumpers, two clickable sticks, two adaptive triggers, create, start, touchpad, guide, mic */ | ||
| SDL_GAMEPAD_TYPE_NINTENDO_SWITCH_PRO, /**< At least B/A/Y/X, d-pad, two bumpers, two clickable sticks, two triggers (digital or better), -, +, and guide */ | ||
| SDL_GAMEPAD_TYPE_NINTENDO_SWITCH_JOYCON_LEFT, |
There was a problem hiding this comment.
| SDL_GAMEPAD_TYPE_NINTENDO_SWITCH_JOYCON_LEFT, | |
| SDL_GAMEPAD_TYPE_XBOX360, /**< An Xbox 360 or compatible third party controller */ | |
| SDL_GAMEPAD_TYPE_XBOXONE, /**< An Xbox One, Xbox Series X/S, XBox Elite, or compatible third party controller */ | |
| SDL_GAMEPAD_TYPE_PS3, /**< A Sony Dualshock 3 or compatible third party controller */ | |
| SDL_GAMEPAD_TYPE_PS4, /**< A Sony Dualshock 4 or compatible third party controller */ | |
| SDL_GAMEPAD_TYPE_PS5, /**< A Sony DualSense or compatible third party controller */ | |
| SDL_GAMEPAD_TYPE_NINTENDO_SWITCH_PRO, /**< A Nintendo Switch Pro controller or compatible third party controller */ |
These are supposed to be the actual controllers or official third party controllers of this type.
There was a problem hiding this comment.
why not unofficial third party controllers and knock-offs too?
There was a problem hiding this comment.
I'm mentally including those in "compatible third party controller"
| SDL_GAMEPAD_TYPE_GAMECUBE, | ||
| SDL_GAMEPAD_TYPE_GAMECUBE, /**< At least A/X/B/Y, d-pad, one bumper, two sticks, two analog triggers, start */ | ||
| SDL_GAMEPAD_TYPE_STEAM, | ||
| SDL_GAMEPAD_TYPE_STANDARD_BAYX, /**< A non-specific gamepad with B/A/Y/X face button layout */ |
There was a problem hiding this comment.
| 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 */ |
There was a problem hiding this comment.
could equally say SNES style
| SDL_GAMEPAD_TYPE_STEAM, | ||
| SDL_GAMEPAD_TYPE_STANDARD_BAYX, /**< A non-specific gamepad with B/A/Y/X face button layout */ | ||
| SDL_GAMEPAD_TYPE_STANDARD_AXBY, /**< A non-specific gamepad with A/X/B/Y face button layout */ | ||
| SDL_GAMEPAD_TYPE_STANDARD_SONY, /**< A non-specific gamepad with Cross/Circle/Square/Triangle face button layout */ |
There was a problem hiding this comment.
| SDL_GAMEPAD_TYPE_STANDARD_SONY, /**< A non-specific gamepad with Cross/Circle/Square/Triangle face button layout */ | |
| SDL_GAMEPAD_TYPE_STANDARD_SONY, /**< A non-specific gamepad with Sony PlayStation style (Cross/Circle/Square/Triangle) face button layout */ |
| SDL_GAMEPAD_TYPE_N64, | ||
| SDL_GAMEPAD_TYPE_WII, | ||
| SDL_GAMEPAD_TYPE_SEGA_GENESIS, |
There was a problem hiding this comment.
Descriptive comments here, similar to above?
| if (SDL_GetHintBoolean(SDL_HINT_JOYSTICK_HIDAPI_VERTICAL_JOY_CONS, false)) { | ||
| // Vertical mode | ||
| SDL_strlcat(mapping_string, "a:b0,b:b1,guide:b5,rightshoulder:b10,rightstick:b8,righttrigger:a5,rightx:a2,righty:a3,start:b6,x:b2,y:b3,misc2:b12,paddle1:b13,paddle3:b15,hint:!SDL_GAMECONTROLLER_USE_BUTTON_LABELS:=1,", sizeof(mapping_string)); | ||
| SDL_strlcat(mapping_string, "a:b0,b:b1,guide:b5,rightshoulder:b10,rightstick:b8,righttrigger:a5,rightx:a2,righty:a3,start:b6,x:b2,y:b3,misc2:b12,paddle1:b13,paddle3:b15,type:joyconright,", sizeof(mapping_string)); |
There was a problem hiding this comment.
Did you accidentally remove the hints for Joy-Con controllers?
| SDL_strlcat(mapping_string, "back:b4,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,leftshoulder:b9,leftstick:b7,lefttrigger:a4,leftx:a0,lefty:a1,misc1:b11,paddle2:b13,paddle4:b15,", sizeof(mapping_string)); | ||
| SDL_strlcat(mapping_string, "back:b4,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,leftshoulder:b9,leftstick:b7,lefttrigger:a4,leftx:a0,lefty:a1,misc1:b11,paddle2:b13,paddle4:b15,type:joyconleft,", sizeof(mapping_string)); | ||
| } else { | ||
| SDL_strlcat(mapping_string, "a:b0,b:b1,guide:b5,rightshoulder:b10,rightstick:b8,righttrigger:a5,rightx:a2,righty:a3,start:b6,x:b2,y:b3,paddle1:b12,paddle3:b14,", sizeof(mapping_string)); |
There was a problem hiding this comment.
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.
| if (SDL_IsJoystickSteamController(vendor, product)) { | ||
| // Steam controllers have 2 back paddle buttons | ||
| SDL_strlcat(mapping_string, "paddle1:b11,paddle2:b12,", sizeof(mapping_string)); | ||
| SDL_strlcat(mapping_string, "paddle1:b11,paddle2:b12", sizeof(mapping_string)); |
There was a problem hiding this comment.
For historical reasons, the mappings must have a trailing comma.
There was a problem hiding this comment.
Did you mean to add ",type:steam," here?
| {"wiiclassic", SDL_GAMEPAD_TYPE_NINTENDO_SWITCH_PRO}, | ||
| {"wiiu", SDL_GAMEPAD_TYPE_NINTENDO_SWITCH_PRO}, |
There was a problem hiding this comment.
Should these be SDL_GAMEPAD_TYPE_WII?
I'm assuming the Wii Pro controllers are mapped as SDL_GAMEPAD_TYPE_NINTENDO_SWITCH_PRO and the remote is SDL_GAMEPAD_TYPE_WII?
There was a problem hiding this comment.
I don't think so. These controllers are closer to a pro controller than to a wii controller. We can set them to standard_bayx.
|
|
||
| // microsoft | ||
| {"xbox", SDL_GAMEPAD_TYPE_XBOX360}, /* original xbox */ | ||
| {"xboxsx", SDL_GAMEPAD_TYPE_XBOXONE}, /* series s/x */ |
There was a problem hiding this comment.
Do we need a separate "xboxelite"?
| * Controllers which are already b/a/y/x to begin with can stay as they are. */ | ||
| if (SDL_GetGamepadButtonLabelForType(gamepad->type, SDL_GAMEPAD_BUTTON_SOUTH) != SDL_GAMEPAD_BUTTON_LABEL_B || | ||
| SDL_GetGamepadButtonLabelForType(gamepad->type, SDL_GAMEPAD_BUTTON_EAST) != SDL_GAMEPAD_BUTTON_LABEL_A) { | ||
| gamepad->type = SDL_GAMEPAD_TYPE_NINTENDO_SWITCH_PRO; |
There was a problem hiding this comment.
I don't think this is right. If I'm reading this correctly, this means that any gamepad in the database that doesn't have a type will get mapped to Nintendo Switch Pro. Did you mean to use SDL_GAMEPAD_TYPE_STANDARD_BAYX instead?
| case SDL_GAMEPAD_TYPE_NES: | ||
| case SDL_GAMEPAD_TYPE_SNES: | ||
| case SDL_GAMEPAD_TYPE_NINTENDO_SWITCH_PRO: | ||
| // TODO: confirm joycon label conventions |
| { | ||
| SDL_GamepadFaceStyle face_style; | ||
|
|
||
| SDL_GamepadType t; |
There was a problem hiding this comment.
| SDL_GamepadType t; | |
| SDL_GamepadType type; |
| "03000000c82d00000161000000000000,8BitDo SN30 Pro,crc:190b,a:b1,b:b0,back:b10,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,guide:b12,guide:b2,leftshoulder:b6,leftstick:b13,lefttrigger:b8,leftx:a0,lefty:a1,rightshoulder:b7,rightstick:b14,righttrigger:b9,rightx:a3,righty:a4,start:b11,x:b4,y:b3,hint:!SDL_GAMECONTROLLER_USE_BUTTON_LABELS:=1,", /* Bluetooth, firmware v1.26 uses b2 for guide, firmware v2.00 uses b12 for guide */ | ||
| "030000003512000020ab000000000000,8BitDo SNES30 Gamepad,a:b1,b:b0,back:b10,dpdown:+a1,dpleft:-a0,dpright:+a0,dpup:-a1,leftshoulder:b6,rightshoulder:b7,start:b11,x:b4,y:b3,hint:!SDL_GAMECONTROLLER_USE_BUTTON_LABELS:=1,", | ||
| "03000000c82d00001930000000000000,8BitDo 64 Bluetooth Controller,a:b0,b:b1,back:-a5,back:b10,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,guide:b12,leftshoulder:b6,leftstick:b13,lefttrigger:b8,leftx:a0,lefty:a2,misc1:b17,misc2:+a3,rightshoulder:b7,righttrigger:b9,start:b11,x:+a5,y:-a3,type:n64,", | ||
| "03000000c82d00000090000000000000,8BitDo FC30 Pro,a:b1,b:b0,back:b10,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,leftshoulder:b6,leftstick:b13,lefttrigger:b8,leftx:a0,lefty:a1,rightshoulder:b7,rightstick:b14,righttrigger:b9,rightx:a3,righty:a4,start:b11,x:b4,y:b3,hint:!SDL_GAMECONTROLLER_USE_BUTTON_LABELS:=1,type:switchpro,", |
There was a problem hiding this comment.
These should probably use type:bayx instead. In general "switchpro" should be used for actual Nintendo Switch Pro form factor controllers.
| "030000000d0f00008700000000000000,HORI Fighting Stick mini 4 (PS4),a:b1,b:b2,back:b8,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,guide:b12,leftshoulder:b4,lefttrigger:b6,rightshoulder:b5,righttrigger:b7,start:b9,x:b0,y:b3,", | ||
| "030000000d0f00006e00000000000000,HORIPAD 4 (PS3),a:b1,b:b2,back:b8,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,guide:b12,leftshoulder:b4,leftstick:b10,lefttrigger:b6,leftx:a0,lefty:a1,rightshoulder:b5,rightstick:b11,righttrigger:b7,rightx:a2,righty:a3,start:b9,x:b0,y:b3,", | ||
| "030000000d0f00006600000000000000,HORIPAD 4 (PS4),a:b1,b:b2,back:b8,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,guide:b12,leftshoulder:b4,leftstick:b10,lefttrigger:a3,leftx:a0,lefty:a1,rightshoulder:b5,rightstick:b11,righttrigger:a4,rightx:a2,righty:a5,start:b9,x:b0,y:b3,", | ||
| "030000000d0f00008800000000000000,HORI Fighting Stick mini 4 (PS3),a:b1,b:b2,back:b9,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,guide:b12,leftshoulder:b4,lefttrigger:b6,rightshoulder:b5,righttrigger:b7,start:b8,x:b0,y:b3,type:sony,", |
There was a problem hiding this comment.
Technically this should be type:ps3 and the one below this is type:ps4, but this is probably fine for arcade sticks.
| @@ -877,7 +878,7 @@ static const char *s_GamepadMappings[] = { | |||
| "0000000050535669746120436f6e7400,PSVita Controller,crc:d598,a:b2,b:b1,back:b6,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,leftshoulder:b4,leftstick:b10,lefttrigger:a4,leftx:a0,lefty:a1,rightshoulder:b5,rightstick:b11,righttrigger:a5,rightx:a2,righty:a3,start:b7,x:b3,y:b0,", | |||
| #endif | |||
| #ifdef SDL_JOYSTICK_N3DS | |||
| "000000004e696e74656e646f20334400,Nintendo 3DS,crc:3210,a:b1,b:b0,back:b2,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,leftshoulder:b5,lefttrigger:b10,leftx:a0,lefty:a1,rightshoulder:b4,righttrigger:b11,rightx:a2,righty:a3,start:b3,x:b7,y:b6,hint:!SDL_GAMECONTROLLER_USE_BUTTON_LABELS:=1,", | |||
| "000000004e696e74656e646f20334400,Nintendo 3DS,crc:3210,a:b1,b:b0,back:b2,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,leftshoulder:b5,lefttrigger:b10,leftx:a0,lefty:a1,rightshoulder:b4,righttrigger:b11,rightx:a2,righty:a3,start:b3,x:b7,y:b6,hint:!SDL_GAMECONTROLLER_USE_BUTTON_LABELS:=1,type:bayx", | |||
There was a problem hiding this comment.
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.
| return "Steam"; | ||
| case SDL_GAMEPAD_TYPE_SEGA_GENESIS: | ||
| return "Sega Genesis"; | ||
| case SDL_GAMEPAD_TYPE_STANDARD: |
There was a problem hiding this comment.
Missing face button variants here?
|
In general this is looking good. Lots of feedback above. @sezero, can you review once the feedback is addressed? |
The whole thing is rather alien to me, and I couldn't read it - 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: |
Co-authored-by: Sam Lantinga <slouken@libsdl.org>
Co-authored-by: Sam Lantinga <slouken@libsdl.org>
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). Iftype: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_LABELShint is still respected. However, I removed support forface: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 newtype:field instead for that information.)Also adds these gamepad types:
n64,wii, andgenesis. 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)