Since rP19504, g_PlayerAssignments wasn't sanitized each updatePlayerList call anymore.
This means it is possible to have both a player and an AI assigned to playerslot 1 in single and multiplayer mode.
Details
- Enable Persist Match Settings
- Open an SP or MP gamesetup page
- Assign an AI to the first playerslot
- Get back to the main menu
- Open the page again
- You're now player 1 + an AI is assigned to that slot. If you're starting a game, the AI will play in your place.
Notice it's because we set g_PlayerAssignments.local.player = 1 and then set AI = "petra" from the loadPersistMatchSettigns call without santizing it as we did before with the updatePlayerListCall (since we now only sanitize in onClientJoin and swapPlayer)
Do the same in MP and see that onClientJoin took care of assigning the player to an unassigned slot, so it only applies to SP.
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
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/1037/ for more details.
Tested, fixes the bug as advertised.
binaries/data/mods/public/gui/gamesetup/gamesetup.js | ||
---|---|---|
941 ↗ | (On Diff #1690) | As you mentioned, this comment could possibly be more clear that this is about unassigning AIs. |
Actually it bugged in MP as well.
Resetting the AIDiff doesn't seem necessary.
In the MP case:
initGUIObjects calls loadPersistMatchSettings (which restores sg_GameAttributes.settings.PlayerData[1].AI = "petra") and
then calls updateGameAttributes (which tells the netserver to broadcast g_GameAttributes to all clients.
However, before this message arrives on the clients at handleGamesetupMessage, onClientJoin is called which sets AI = "" on the g_GameAttributes variable which will be overwritten by that handleGamesetupMessage call.
So the order of execution is:
loadPersistMatchSettings onClientJoin handleGamesetupMessage
while it might have been expected to be:
loadPersistMatchSettings handleGamesetupMessage onClientJoin
but since we can't really chose when the client joins, just sanitizing it every time we receive new assignments works (and the previous code worked like this and sanitized each time in updatePlayerList which was called very often).
The SP case is solved too, since sanitizePlayerData is also called from selectMap while the player is assigned to 1.
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/1176/ for more details.