Update ready status immediately in the gamesetup
ClosedPublic
Actions

Authored by elexis on Apr 9 2017, 4:55 AM.

Details

Reviewers

scythetwirler
Imarok
vladislavbelov

Commits

rP19396: Update ready state immediately when receiving the message.

Summary

#2447 / rP15006 implemented initial support for ready states in the gamesetup.
When the server receives a ready message, it uses both a SendPlayerAssignments call _and_ a broadcasting of the ready message to inform the clients of the most recent ready status of all clients.

Once the ready message is received by a client, a chat message is printed and if it's a host, the status will be updated immediately.
If it's not a host, it will have to wait for the updated player assignments sent by the server.

However this distinciton can be simply removed. All clients can show the most recent ready status once they are notified about the ready change.
Thus the NetServer doesn't have to send the redundant player assignments.

Test Plan

Apply patch, compile, join with a second client and click on ready.
Read the code and convince yourself that there is no kind of race condition that might be solved with the current check,
because the server will always keep the most recent correct state and update it immediately if a client changes the ready state or
if another event changes the assignments.

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Apr 9 2017, 4:55 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/716/ for more details.

Harbormaster completed remote builds in B1053: Diff 1162.Apr 9 2017, 6:00 AM

I've tested it, it works good, there shouldn't be a race condition, because the patched code is subset of the not patched.

This revision is now accepted and ready to land.Apr 9 2017, 6:17 PM

vladislavbelov added inline comments.Apr 9 2017, 6:20 PM

source/network/NetServer.cpp
1112 ↗	(On Diff #1162)	The difference is only that SendPlayerAssignments also sends NSS_JOIN_SYNCING and NSS_INGAME, which means that a game already started and ready station isn't needed.