Fixed bug 3299 - DirectInput: Incorrect joystick mapping when attaching new joysticks

Jimb Esser

Note: This is using DirectInput, I have to disable XInput as that causes all but the first 4 controllers to be completely ignored by SDL (I can find no way to reconcile XInput devices with DirectInput devices, otherwise I would make a patch that accepts the fifth and later controllers with DirectInput...).  XInput does not seem to have the problem below, only DirectInput.

I plug in 3 identical wireless Xbox 360 controllers, call them J1, J2, J3.  Direct Input shows them as having GUIDs G1, G2, G3.  I unplug J1, then J2 and J3 show up as having GUIDs G1 and G2!  Not so "unique"...  I start my SDL app when just J2 and J3 are plugged in, and open J2 and J3.  Then I plug in a new controller, SDL sees that now G3 exists, assigns that a new SDL joystick instance ID, which I request to be opened, but G3 at this point is J3, which I already had opened!  So I end up with two instances of J3 opened, and none of J1.  "Re-"opening G1 would get the actual handle to the newly attached controller, but there's no current way to know this.  This is clearly a bug or poor design in DirectInput or my wireless receiver drivers, but is a showstopping bug for my 8-20 player games (as soon as any one controller runs out of battery or goes to sleep and gets turned back on, suddenly things are busted requiring a restart (or, at least, a reinitialization of all controllers - the game can't go on)).

The solution I found is to use HID paths instead of GUIDs to uniquely identify joysticks.  GUIDs are still needed to open a controller, however I have added code to re-find the GUIDs for all joysticks whenever a new joystick is attached or removed.  This does now require opening of all joysticks (instead of just enumerating them), though if your app, like mine, is opening all of them anyway so that any can press a button to join, that doesn't change much (although perhaps they joysticks should be kept open in this case, instead of closed and re-opened).  If your app only ever opens one joystick, this will do more work at startup than it did previously.

src/joystick/windows/SDL_dinputjoystick.c

@@ -354,6 +354,7 @@ EnumJoysticksCallback(const DIDEVICEINSTANCE * pdidInstance, VOID * pContext)
     JoyStick_DeviceData *pPrevJoystick = NULL;
     const DWORD devtype = (pdidInstance->dwDevType & 0xFF);
     Uint16 *guid16;
+    WCHAR hidPath[MAX_PATH];
 
     if (devtype == DI8DEVTYPE_SUPPLEMENTAL) {
         /* Add any supplemental devices that should be ignored here */
@@ -375,9 +376,42 @@ EnumJoysticksCallback(const DIDEVICEINSTANCE * pdidInstance, VOID * pContext)
         return DIENUM_CONTINUE;  /* ignore XInput devices here, keep going. */
     }
 
+    {
+        HRESULT result;
+        LPDIRECTINPUTDEVICE8 device;
+        LPDIRECTINPUTDEVICE8 InputDevice;
+        DIPROPGUIDANDPATH dipdw2;
+
+        result = IDirectInput8_CreateDevice(dinput, &(pdidInstance->guidInstance), &device, NULL);
+        if (FAILED(result)) {
+            return DIENUM_CONTINUE; /* better luck next time? */
+        }
+
+        /* Now get the IDirectInputDevice8 interface, instead. */
+        result = IDirectInputDevice8_QueryInterface(device, &IID_IDirectInputDevice8, (LPVOID *)&InputDevice);
+        /* We are done with this object.  Use the stored one from now on. */
+        IDirectInputDevice8_Release(device);
+        if (FAILED(result)) {
+            return DIENUM_CONTINUE; /* better luck next time? */
+        }
+        dipdw2.diph.dwSize = sizeof(dipdw2);
+        dipdw2.diph.dwHeaderSize = sizeof(dipdw2.diph);
+        dipdw2.diph.dwObj = 0; // device property
+        dipdw2.diph.dwHow = DIPH_DEVICE;
+
+        result = IDirectInputDevice8_GetProperty(InputDevice, DIPROP_GUIDANDPATH, &dipdw2.diph);
+        IDirectInputDevice8_Release(InputDevice);
+        if (FAILED(result)) {
+            return DIENUM_CONTINUE; /* better luck next time? */
+        }
+
+        /* Get device path, compare that instead of GUID, additionally update GUIDs of joysticks with matching paths, in case they're not open yet. */
+        SDL_wcslcpy(hidPath, dipdw2.wszPath, SDL_arraysize(hidPath));
+    }
+
     pNewJoystick = *(JoyStick_DeviceData **)pContext;
     while (pNewJoystick) {
-        if (!SDL_memcmp(&pNewJoystick->dxdevice.guidInstance, &pdidInstance->guidInstance, sizeof(pNewJoystick->dxdevice.guidInstance))) {
+        if (SDL_wcscmp(pNewJoystick->hidPath, hidPath) == 0) {
             /* if we are replacing the front of the list then update it */
             if (pNewJoystick == *(JoyStick_DeviceData **)pContext) {
                 *(JoyStick_DeviceData **)pContext = pNewJoystick->pNext;
@@ -385,6 +419,9 @@ EnumJoysticksCallback(const DIDEVICEINSTANCE * pdidInstance, VOID * pContext)
                 pPrevJoystick->pNext = pNewJoystick->pNext;
             }
 
+            // Update with new guid/etc, if it has changed
+            pNewJoystick->dxdevice = *pdidInstance;
+
             pNewJoystick->pNext = SYS_Joystick;
             SYS_Joystick = pNewJoystick;
 
@@ -401,6 +438,7 @@ EnumJoysticksCallback(const DIDEVICEINSTANCE * pdidInstance, VOID * pContext)
     }
 
     SDL_zerop(pNewJoystick);
+    SDL_wcslcpy(pNewJoystick->hidPath, hidPath, SDL_arraysize(pNewJoystick->hidPath));
     pNewJoystick->joystickname = WIN_StringToUTF8(pdidInstance->tszProductName);
     if (!pNewJoystick->joystickname) {
         SDL_free(pNewJoystick);

src/joystick/windows/SDL_windowsjoystick_c.h

@@ -37,6 +37,7 @@ typedef struct JoyStick_DeviceData
     BYTE SubType;
     Uint8 XInputUserId;
     DIDEVICEINSTANCE dxdevice;
+    WCHAR hidPath[MAX_PATH];
     struct JoyStick_DeviceData *pNext;
 } JoyStick_DeviceData;