[PATCH 0/1] MR1551: SDL: Always make unique serial numbers

[Evan Tang (＠etang-cw)]

Overcooked! All You Can Eat only adds new controllers if existing controller has the same serial as it.  SDL GUIDs mark the model of controller, but don't differentiate between multiple controllers of the same model.  This is especially bad with Steam virtual controllers, which all have the same GUID.

Switch to using SDL_JoystickGetSerial if available, and otherwise combining the GUID with the controller index

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1551

[PATCH 1/1] SDL: Always make unique serial numbers

[Evan Tang]

From: Evan Tang <etang(a)codeweavers.com>

---
 dlls/winebus.sys/bus_sdl.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/dlls/winebus.sys/bus_sdl.c b/dlls/winebus.sys/bus_sdl.c
index 1af029bed65..844f8957abc 100644
--- a/dlls/winebus.sys/bus_sdl.c
+++ b/dlls/winebus.sys/bus_sdl.c
@@ -120,6 +120,7 @@ static Uint16 (*pSDL_JoystickGetProduct)(SDL_Joystick * joystick);
 static Uint16 (*pSDL_JoystickGetProductVersion)(SDL_Joystick * joystick);
 static Uint16 (*pSDL_JoystickGetVendor)(SDL_Joystick * joystick);
 static SDL_JoystickType (*pSDL_JoystickGetType)(SDL_Joystick * joystick);
+static const char *(*pSDL_JoystickGetSerial)(SDL_Joystick * joystick);
 
 /* internal bits for extended rumble support, SDL_Haptic types are 16-bits */
 #define WINE_SDL_JOYSTICK_RUMBLE  0x40000000 /* using SDL_JoystickRumble API */
@@ -936,9 +937,8 @@ static void sdl_add_device(unsigned int index)
 
     SDL_Joystick* joystick;
     SDL_JoystickID id;
-    SDL_JoystickGUID guid;
     SDL_GameController *controller = NULL;
-    const char *product;
+    const char *product, *sdl_serial;
     char guid_str[33], buffer[ARRAY_SIZE(desc.product)];
     int axis_count, axis_offset;
 
@@ -969,9 +969,19 @@ static void sdl_add_device(unsigned int index)
         desc.version = 0;
     }
 
-    guid = pSDL_JoystickGetGUID(joystick);
-    pSDL_JoystickGetGUIDString(guid, guid_str, sizeof(guid_str));
-    ntdll_umbstowcs(guid_str, strlen(guid_str) + 1, desc.serialnumber, ARRAY_SIZE(desc.serialnumber));
+    if (pSDL_JoystickGetSerial && (sdl_serial = pSDL_JoystickGetSerial(joystick)))
+    {
+        ntdll_umbstowcs(sdl_serial, strlen(sdl_serial) + 1, desc.serialnumber, ARRAY_SIZE(desc.serialnumber));
+    }
+    else
+    {
+        // Overcooked! All You Can Eat only adds controllers with unique serial numbers
+        // Prefer keeping serial numbers unique over keeping them consistent across runs
+        TRACE("Making up serial number for %s %s\n", product, debugstr_w(desc.serialnumber));
+        pSDL_JoystickGetGUIDString(pSDL_JoystickGetGUID(joystick), guid_str, sizeof(guid_str));
+        snprintf(buffer, sizeof(buffer), "%s.%d", guid_str, index);
+        ntdll_umbstowcs(buffer, strlen(buffer) + 1, desc.serialnumber, ARRAY_SIZE(desc.serialnumber));
+    }
 
     if (controller)
     {
@@ -1126,6 +1136,7 @@ NTSTATUS sdl_bus_init(void *args)
     pSDL_JoystickGetProductVersion = dlsym(sdl_handle, "SDL_JoystickGetProductVersion");
     pSDL_JoystickGetVendor = dlsym(sdl_handle, "SDL_JoystickGetVendor");
     pSDL_JoystickGetType = dlsym(sdl_handle, "SDL_JoystickGetType");
+    pSDL_JoystickGetSerial = dlsym(sdl_handle, "SDL_JoystickGetSerial");
 
     if (pSDL_Init(SDL_INIT_GAMECONTROLLER | SDL_INIT_HAPTIC) < 0)
     {
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/1551

[Etaash Mathamsetty (＠etaash.mathamsetty)]

Etaash Mathamsetty (@etaash.mathamsetty) commented about dlls/winebus.sys/bus_sdl.c:
> desc.version = 0;
>      }
>
> -    guid = pSDL_JoystickGetGUID(joystick);
> -    pSDL_JoystickGetGUIDString(guid, guid_str, sizeof(guid_str));
> -    ntdll_umbstowcs(guid_str, strlen(guid_str) + 1, desc.serialnumber, ARRAY_SIZE(desc.serialnumber));
> +    if (pSDL_JoystickGetSerial && (sdl_serial = pSDL_JoystickGetSerial(joystick)))
> +    {
> +        ntdll_umbstowcs(sdl_serial, strlen(sdl_serial) + 1, desc.serialnumber, ARRAY_SIZE(desc.serialnumber));
> +    }
> +    else
> +    {
> +        // Overcooked! All You Can Eat only adds controllers with unique serial numbers

these should be replaced with C style comments (i.e /* comment here */)

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1551#note_17616

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winebus.sys/bus_sdl.c:
> desc.version = 0;
>      }
>
> -    guid = pSDL_JoystickGetGUID(joystick);
> -    pSDL_JoystickGetGUIDString(guid, guid_str, sizeof(guid_str));
> -    ntdll_umbstowcs(guid_str, strlen(guid_str) + 1, desc.serialnumber, ARRAY_SIZE(desc.serialnumber));
> +    if (pSDL_JoystickGetSerial && (sdl_serial = pSDL_JoystickGetSerial(joystick)))
> +    {
> +        ntdll_umbstowcs(sdl_serial, strlen(sdl_serial) + 1, desc.serialnumber, ARRAY_SIZE(desc.serialnumber));
> +    }
> +    else
> +    {
> +        // Overcooked! All You Can Eat only adds controllers with unique serial numbers
> +        // Prefer keeping serial numbers unique over keeping them consistent across runs
> +        TRACE("Making up serial number for %s %s\n", product, debugstr_w(desc.serialnumber));

Tracing `desc.serialnumber` isn't very useful here, you could move the trace a bit below and trace the joystick name and its made up serial instead.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1551#note_17637

[Rémi Bernon]

Please also change the commit title to `winebus.sys: Report unique serial numbers for SDL devices.` or something like that.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1551#note_17638