Unplugging gamepad crashes

[naezith]

A player reported today that it crashes on unplug. I asked him to give system specs, I'll update when he tells.

Also I found this on internet if it helps:
https://twitter.com/hartlabs/status/631934662378590210

Searched that joyGetPosEx function in SFML.

SFML/src/SFML/Window/Win32/JoystickImpl.cpp

Line 198 in f053871

cache.connected = joyGetPosEx(JOYSTICKID1 + i, &joyInfo) == JOYERR_NOERROR;

It loops from 1 to Joystick::Count which is generally 8. I limited "Joystick::isConnected" checks to check only 1 to 4 in my code but I don't want to do the same in SFML. Maybe you guys would consider it.

There are a lot of forum topics about this and they are not solved.

https://en.sfml-dev.org/forums/index.php?topic=18790.0
https://en.sfml-dev.org/forums/index.php?topic=13318.0

---

SFML 2.4.2
gcc (x86_64-posix-seh-rev1, Built by MinGW-W64 project) 6.3.0

[MarioLiebisch]

Yeah, it's some odd and weird race condition inside the Windows API. It's not really related to the number of controllers (I've been able to reproduce this with just a single Xbox One gamepad). Unfortunately I don't know any sane solution to this. One of the reasons I'd prefer using XInput, but then again it limits you to 4 controllers unfortunately.

[naezith]

Who needs more than 4 though? I'd be okay with it if it will work nicely. Is this the reason why local party games are targeting 4 players max mostly?

[MarioLiebisch]

It's probably some unwritten rule most abide to. :) Also many classic consoles were limited to 4 players (SNES being a notable exception). I wouldn't mind cutting it to 4, but given that SFML supported 8 before people might not be happy with such a change.

[naezith]

I don't think there is a +4 players SFML game. I still do not know if it is a solution though.

[binary1248]

I'm guessing joyGetNumDevs() returns 16?

[MarioLiebisch]

No. It's no out of bounds issue there (at least not for the crashes I experienced). joyGetPosEx() crashes somewhere, no matter which index is accessed, even if it's just 0. There's obviously a small concurrency issue somewhere (i.e. joyGetPosEx() processing while the controller is actually removed from internal state stuff).

[naezith]

More people reported this because the game is played mostly with gamepads. I really need it to be fixed. It's the only crash game has.

[binary1248]

I guess we could try adding like a 1-2 second delay between when we receive WM_DEVICECHANGE and when we update the joysticks via joyGetPosEx. It sounds stupid, but Windows might notify applications of the device node change before it even allows the driver to sort itself out... Would explain this phenomenon anyways...

[MarioLiebisch]

If only XInput would support 8 joysticks. :( Or someone has the time to implement Direct Input directly. ;) The problem is, it's a stupid race condition you can't really prevent. I think it can even happen before your window message gets processed.

[naezith]

We still have this crash, here is more information from the debugger.

[binary1248]

Try applying this patch and see if it changes anything:

 src/SFML/Window/Win32/WindowImplWin32.cpp | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/SFML/Window/Win32/WindowImplWin32.cpp b/src/SFML/Window/Win32/WindowImplWin32.cpp
index 78d13d31..fa48a809 100755
--- a/src/SFML/Window/Win32/WindowImplWin32.cpp
+++ b/src/SFML/Window/Win32/WindowImplWin32.cpp
@@ -39,6 +39,7 @@
 #include <GL/gl.h>
 #include <SFML/System/Err.hpp>
 #include <SFML/System/Utf.hpp>
+#include <dbt.h>
 #include <vector>
 #include <cstring>
 
@@ -216,6 +217,10 @@ m_cursorGrabbed   (m_fullscreen)
     // Create the window
     m_handle = CreateWindowW(className, title.toWideString().c_str(), win32Style, left, top, width, height, NULL, NULL, GetModuleHandle(NULL), this);
 
+    // Register to receive device interface change notifications (used for joystick connection handling)
+    DEV_BROADCAST_HDR deviceBroadcastHeader{sizeof(DEV_BROADCAST_HDR), DBT_DEVTYP_DEVICEINTERFACE, 0};
+    RegisterDeviceNotification(m_handle, &deviceBroadcastHeader, DEVICE_NOTIFY_WINDOW_HANDLE | DEVICE_NOTIFY_ALL_INTERFACE_CLASSES);
+
     // If we're the first window handle, we only need to poll for joysticks when WM_DEVICECHANGE message is received
     if (m_handle)
     {
@@ -982,9 +987,15 @@ void WindowImplWin32::processEvent(UINT message, WPARAM wParam, LPARAM lParam)
         }
         case WM_DEVICECHANGE:
         {
-            // Some sort of device change has happened, update joystick connections
-            if (wParam == DBT_DEVNODES_CHANGED)
-                JoystickImpl::updateConnections();
+            if ((wParam == DBT_DEVICEARRIVAL) || (wParam == DBT_DEVICEREMOVECOMPLETE))
+            {
+                // Some sort of device change has happened, update joystick connections if it is a device interface
+                DEV_BROADCAST_HDR* deviceBroadcastHeader = reinterpret_cast<DEV_BROADCAST_HDR*>(lParam);
+
+                if (deviceBroadcastHeader && (deviceBroadcastHeader->dbch_devicetype == DBT_DEVTYP_DEVICEINTERFACE))
+                    JoystickImpl::updateConnections();
+            }
+
             break;
         }
     }

[naezith]

I opened the game with gamepad connected. Then unplugged it, it was fine. Then plugged it again and it crashed but I was in Release mode so there is no debug data. Then I run it in debug mode and I couldn't get the crash no matter what I do. Same for Release mode after that. Very random and hard to catch it.

[NikoGandon]

Hello,

I have a problem with SFML 2.4, and it's pretty much the same thing:

When I open the program, I have access to a graphical part (with SFML) and non-graphical (cmd base). At first, it opens it in cmd. The problem is that when I plug in an xbox 360 controller, the program in cmd works, but as soon as I switch to the graphic (with SFML), the program hangs. In release or debug mode, it does not change anything, just in the debugguer where there is no problem.

Does anyone have the answer to the problem, or should I use the patch given earlier? Thank you for your answers

[LaurentGomila]

If would be great if you could test the proposed patch, that would help to validate it and integrate it into the next version of SFML.

[NikoGandon]

Hello,

I tried but it did not work. Maybe I misconfigured CMake. But it does not solve the problem.

Otherwise, the debugguer happens to make it turn despite the controller but I do not have the impression that it tells me where is the problem

[NikoGandon]

And it's the result after compiling with the debugger :

[eXpl0it3r]

That call stack makes no sense in ragards to a crash with game pad, I mean what does the texture destructor have to do with the game pad?

What was your test code?

[NikoGandon]

I put my "old" source code with SFML base libraries, because those compiled by Cmake do not stop bugging. But if I put the minimal code, it gives that:

He looks alone :(

[naezith]

I am planning to release the game in Q1 2018, this bug is being reported super often. It's the only critical bug my game has. Can you prioritize it so I can release the game potentially bug-free?

[binary1248]

I can't think of any way to permanently fix this problem short of rewriting the joystick processing to make use of DirectInput instead of the Windows joystick API. The latter is already implemented by Microsoft on top of the former since at least as early as Windows XP, so there shouldn't be any compatibility concerns.

Fact of the matter is that nobody really uses the Windows joystick API in production environments any more, which is why Microsoft gets away with neglecting to actively maintain it. DirectInput on the other hand has become and still is the de facto way of interfacing with HID devices for almost 20 years.

[MarioLiebisch]

DirectInput is basically deprecated as well (even XInput is in some way). Rewriting the joystick API to use XInput is basically a task for half an hour – I've done it before and it's straightforward and easy to do, The real problem is SFML supporting up to 8 controllers (while XInput only supports 4).

[binary1248]

If this comparison is anything to go by, there is no real value in preferring XInput over DirectInput. XInput might be newer, but newer doesn't always imply better.

[MarioLiebisch]

The problem is, nobody knows Microsoft's internal plan for DirectInput – it's basically dead since DirectX 9c IIRC, so potentially unmaintained similar to the classic Joystick API. The issue from this thread might even happen when using DirectInput, I'm not sure whether there's a significant difference. Also if we ever want to add vibration/force feedback, we'll have to use XInput as well. In addition, don't forget DirectInput won't solve the inherited issue of triggers sharing one axis.

[naezith]

I already coded XInput next to SFML, not into. Whenever XInput fails to recognize the gamepad, SFML can. When XInput detects one, game uses that one and ignores all the SFML joystick events but unplugging crashes the game since SFML detects that and crashes internally at updateConnections(). That's the problem part for me.

[eXpl0it3r]

To be honest, has anyone ever used more that four controllers with SFML? I wouldn't mind just dropping that "requirement" as it's not a real use case.

What I'm more confused about XInput is, that the official documentation only ever talks about Xbox 360 controller support. Is that just "marketing talk" and it actually supports many other types of controllers as well?

How does this affect the dependency list?

[MarioLiebisch]

It's basically a driver thing and just got never updated. Most modern controllers will work with both Xinput and Direct Input. This includes Microsoft controllers but also third party stuff like Logitech and others. If they feature the colored ABXY buttons or are advertised as working with the Xbox 360/One, they're Xinput.

Controllers not supporting it are mostly very old or cheap controllers and those featuring far more buttons or controls you can't map to a single controller, most notably H.O.T.A.S. sticks. However, it's possible to use tools such as [url=https://www.x360ce.com/]x360ce[/url] to even make those compatible.

There are the other limitations you've linked, but most controllers won't reach these limits, Sony, Microsoft, Nintendo, etc. all are typically only using 4 axes, 10 buttons, two triggers, etc. or less.

Xinput would be a new dependency, but the runtime files are present on any supported platform and the development files are included in both Visual Studio (as part of the Windows SDK) and MinGW by default.

[DeathRay2K]

XInput severely limits controller support. Any controller that is not specifically designed for Windows support is not likely to work with XInput, whereas DirectInput support is ubiquitous.
It is not just old/cheap controllers that do not support XInput, but also DualShock controllers, Nintendo controllers, mobile-focused controllers, etc.
There are plenty of controllers out there that are not targeted at Windows, that would be left unsupported if DirectInput were dropped. The only way to get around this restriction is to run third party software in the background that translates DirectInput commands to XInput, which introduces its own set of restrictions and problems. Not to mention the hassle for users of DirectInput controllers.

For the record, I have also taken advantage of SFML's support for over 4 controllers in the past, it's not unheard of for games to support more than 4 players in local multiplayer these days.

The only upside of XInput is improved support for Microsoft's own controllers. Their recommended solution for this is to implement DirectInput and XInput side by side:
https://msdn.microsoft.com/en-ca/library/windows/desktop/ee417014(v=vs.85).aspx

[DeathRay2K]

Also worth noting that XInput has been superseded by Windows.Gaming.Input (https://docs.microsoft.com/en-us/uwp/api/Windows.Gaming.Input)
Which supports more than 4 XBox controllers, and is seemingly future-proof thanks to features like RawGameController supporting arbitrary numbers of axes and buttons. Only available in Windows 10 though.

[MarioLiebisch]

@DeathRay2K Yep, know it. But it's also UWP only, so not even for Windows 10 desktop apps, unfortunately.

[DeathRay2K]

You can use UWP APIs in desktop apps. See this StackOverflow question:
https://stackoverflow.com/questions/41286503/how-to-use-the-namespace-windows-gaming-input-in-c-without-using-uwp

[MarioLiebisch]

Okay, certainly interesting. But unfortunately this doesn't solve our issue, since we'd still have to reimplement everything anyway, at least for Windows 7.

[naezith]

Can you simply give me an option like sf::Joystick::disableEvents() which would disable that part of the code? I am already using XInput for everything an Xbox gamepad needs. I ignore all the sf::Joystick events but it's detecting the gamepad internally and crashing there. That's the only problem.

[DeathRay2K]

@MarioLiebisch Or use DirectInput and Windows.Input.Gaming (When available) side by side to support the broadest range of controllers and Windows versions.

[MarioLiebisch]

Let's rephrase it: That option certainly wouldn't hurt. :) sf::Joystick::setEnabled(), sf::Keyboard::setEnabled(), etc.?

[naezith]

Yeah just having a bool which simply deactivates this part of the code.

SFML/src/SFML/Window/Win32/WindowImplWin32.cpp

Line 983 in 34ea68b

case WM_DEVICECHANGE:

[naezith]

If XInput detects a controller, I will disable SFML joystick. That would solve my problem.

[binary1248]

Don't hesitate to give #1326 a try.

[naezith]

I always thought we were using DirectInput. :) I will try this, I wonder how DirectInput handles triggers. Does it separate triggers to two axises or use single axis?

[MarioLiebisch]

It's basically the same as before without to compatibility layer emulating the classic Windows joystick API from 16 bit times. Behavior should be 1:1 to before.

[naezith]

Okay it is fine I have XInput on top for the dual trigger lovers.

[naezith]

Not only it fixed the crash, it also has healthy triggers! Thank you so much, it is a great update for me before release. @binary1248