As reported by Imarok in rP19504, if the host changes the settings while a client was ready, that button still shows that the client is ready after the change.
This is because of a camouflaged behavior change in D322: Almost everything in updateGUIObjects only changed the GUI objects to represent the current settings, but it also called resetReadyData.
This function was made read-only and the reset call was removed, but not added in all places that where it was called before, specifically when receiving a gamesetting-changed message.
Details
Details
- Reviewers
wraitii Imarok - Commits
- rP19586: Fix the readystate following D322 / rP19504.
Correctness:
Start a game with one client.
Join with a second instance.
Click on ready in the second instance.
Change the settings with the first instance.
With the patch applied, the button is reset, without the patch it isn't.
Completeness:
Search the prior code for updateGUIObjects occurances. See there were three, of one is added above.
Notice the resetReadyData call is now absent from loadPersistMatchSettings() too, but doesn't matter since noone really clicks on ready in the first fraction of a second.
Notice the call is absent from updateGameAttributes() as well, which isn't an issue since there is no ready state in singleplayer and since the multiplayer case is dealt with already.
Diff Detail
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
Comment Actions
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/1177/ for more details.