Allow SDL joysticks to be reconfigured after init #1326
Conversation
|
Thanks for this patch. I wonder what that count variable was supposed to do. Could you please confirm there are no unintended side effects of removing this count check? |
|
Nothing seems to have broken for me, with 0 joysticks or 1 connected. However, I only own a single gamepad so I can't test the more complex cases of multiple gamepads and them being reconfigured in an expected manner on connect/disconnects. I'll test more thoroughly that disconnecting and reconnecting the one joystick works. |
|
That would be great, thanks. If you could buy or borrow a second joypad or joystick, that would even be better. They are relatively cheap these days. |
|
I should be able to test with multiple controllers. |
src/sdl/sdl_joystick.c
Outdated
| @@ -83,6 +82,7 @@ void _al_sdl_joystick_event(SDL_Event *e) | |||
| } | |||
| else if (e->type == SDL_JOYDEVICEADDED || e->type == SDL_JOYDEVICEREMOVED) { | |||
| event.joystick.type = ALLEGRO_EVENT_JOYSTICK_CONFIGURATION; | |||
| sdl_reconfigure_joysticks(); | |||
There was a problem hiding this comment.
looking at ex_joystick_hotplugging it seems the expectation is that programs should decide themselves to reconfigure. If so, should this line be removed (and any program that needs to should call al_reconfigure_joysticks on event ALLEGRO_EVENT_JOYSTICK_CONFIGURATION themselves?)
There was a problem hiding this comment.
Ah, so does ex_joystick_events. OK, this line probably doesn't belong here.
connorjclark
force-pushed
the
sdl-joy-web
branch
from
5e7f507 to
b748c65
Compare
| @@ -97,7 +94,7 @@ void _al_sdl_joystick_event(SDL_Event *e) | |||
| static bool sdl_init_joystick(void) | |||
| { | |||
| count = SDL_NumJoysticks(); | |||
| joysticks = calloc(count, sizeof * joysticks); | |||
| joysticks = count > 0 ? calloc(count, sizeof * joysticks) : NULL; | |||
There was a problem hiding this comment.
In my testing I got errors when going from 1 -> 0 controllers because although joysticks was being freed in sdl_exit_joystick, this line here would have a count of zero and I noticed joysticks[0] still had a pointer to (free'd) data. calloc has odd behavior when size is zero. To fix, manually set to NULL when size is zero.
Also, set joysticks to NULL after freeing for good measure.
connorjclark
changed the title
|
I confirmed that going from 0->1 and 1->0 controllers works as expected in the demos. @SiegeLord I've uploaded emscripten builds for your convenience: https://comfortable-badge.surge.sh/ex_joystick_events.html https://comfortable-badge.surge.sh/ex_joystick_hotplugging.html |
|
Seems to work fine with two controller as well. Thanks for the change! |
|
Indeed, thank you very much for your extra tests and for |
Currently joysticks do not work with programs built with emscripten+SDL backend. You can verify this by trying to use a gamepad with the joystick events example here.
The SDL joystick code reads the number of joysticks connected on init, keeping track of the number found in a
countvariable. If a new controller connects, SDL emits an event, but currently the event handling code ignores all events if on init it found zero controllers.On the web, the
gamepadconnectedevent only fires after the first input on the controller. This means that on the web, SDL will have zero controllers on setup... unless you happen to be twiddling the controller inputs on setup. The example above initializes too quickly to see this behavior, but it was the first thing I noticed while debugging this issue in my (more setup-intensive) application: controllers only worked if I was twiddling the controller during setup.The fix is:
sdl_reconfigure_joystickswhen a controller connects or disconnectsI confirmed this patch works for my application and for the above example:
