XboxDev / nxdk-sdl

SDL2 (adapted for original Xbox / nxdk toolchain)
Other
20 stars 8 forks source link

SDL Joystick information might change unexpectedly #31

Open JayFoxRox opened 4 years ago

JayFoxRox commented 4 years ago

I believe SDL expects no updates to return values of SDL_JoystickNameForIndex or SDL_NumJoysticks inbetween SDL event pumping and / or SDL_JoystickUpdate. Otherwise there might be race conditions.

Imagine these steps in an app:

  1. A user asks SDL for the number of connected joystick devices
  2. User disconnects the USB device
  3. User takes device count from step 1. to loop over SDL joystick devices to ask for their name

I don't think our driver respects this. We should check if / how other backends take care of this.

See #30 for another description of this / why I think this is violated.

JayFoxRox commented 3 years ago

@Ryzee119 briefly looked into this here: https://github.com/XboxDev/nxdk-sdl/pull/38#discussion_r611511381; so it looks like this might be broken / poor design in SDL:

I started messing with caching the device list locally and only sync to the 'real' list on JoystickDetect but SDL backend was calling things in a way I didnt expect. (I.e GetNumofJoysticks was being called before JoystickDetect() on hotplugs for example which messed with my inital plans.

Ryzee119 commented 3 years ago

Ive looked into this more and to do it properly you must only pump hotplug connection events (SDL_PrivateJoystickRemoved and SDL_PrivateJoystickAdded) in JoystickDetect(). I was doing in the usb stack callbacks, instead these should only queue device removal/additions in the SDL backend.

I think JoystickDetect() should do this (in this order):

I havent got a working prototype yet, but some quick testing seems like this could work.

Looks like how the windows backend does it. https://github.com/XboxDev/nxdk-sdl/blob/nxdk/src/joystick/windows/SDL_windowsjoystick.c