SDL: Call multiplayer->update() from the user code thread #716
Conversation
32blit-sdl/System.cpp
Outdated
| @@ -269,6 +269,7 @@ void System::loop() | |||
| blit::joystick.y = shadow_joystick[1]; | |||
| SDL_UnlockMutex(m_input); | |||
| blit::tick(::now()); | |||
| blit_multiplayer->update(); | |||
There was a problem hiding this comment.
I know the indentation is a horrible mess, but that's definitely a bit wonky. (I guess github is getting 2 spaces from the editorconfig file)
ali1234
force-pushed
the
multiplayer_threadsafe_sdl
branch
2 times, most recently
from
8618be9
to
b2cf450
Compare
|
This didn't actually fix the problems I'm seeing with multiplayer. Also it will lock the SDL framerate to 50 fps. I'm not sure if calling render outside the user thread does any better job there though. |
The multiplayer update scans for messages and passes them in to the user defined function on_message(). This should be done in the user code thread for two reasons: 1. Calling it from a different thread means on_message() and update() are being run in different threads, and therefore all code in both of them has to be thread safe. This is pretty hard to do with the 32blit API and not really in the spirit of what this is supposed to be. 2. on_message() is a user-supplied function and so bugs can make it hang. We can detect hangs in update() and render() thanks to running them on the user thread (via blit::tick()). Running on_message() on that thread allows to detect hangs there too.
Someone moved the call to render out of the thread loop and into the texture update function. Same rationale for putting it back. It should not overlap other user code and it should be possible to detect if the user code hangs.
ali1234
force-pushed
the
multiplayer_threadsafe_sdl
branch
from
405ccdb
to
e8876fc
Compare
|
Looks like I put render into All this threading, event posting and semaphore waiting makes my head spin! Do the last two commits fix your multiplayer woes? |
|
Multiplayer does seem to be completely solid after this change, but I'm not totally sure this is the reason, or if it is just because I fixed all the bugs in my code. On the other hand I have not noticed anything behaving worse with this change, and it is technically the right thing to do. The idea behind the thread is that any code written by the user (init, update, render, on_message) can potentially go into an infinite loop and we want to detect it. So for that reason it is (supposed to) all run on the user thread. |
|
Thank you! Looking forward to one day getting some multiplayer blit action 😆 |
The multiplayer update scans for messages and passes them in to the
user defined function on_message(). This should be done in the user
code thread for two reasons:
Calling it from a different thread means on_message() and update()
are being run in different threads, and therefore all code in both of
them has to be thread safe. This is pretty hard to do with the 32blit
API and not really in the spirit of what this is supposed to be.
on_message() is a user-supplied function and so bugs can make it
hang. We can detect hangs in update() and render() thanks to running
them on the user thread (via blit::tick()). Running on_message() on
that thread allows to detect hangs there too.