Allow players to change some options. The host can control with a setting which options players can change (none/color/color & civ/color, civ & team/all).
This patch is currently WIP.
(Seems to work for everything besides the mapSelection dropdown. I know, this contains one commented warning for debugging atm.)
Details
- Reviewers
- None
- Trac Tickets
- #3806
Test every setting is changed correctly and let the host send a correct gameAttributes message. Players shouldn't not be able to modify settings they aren't allowed to.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- https://svn.wildfiregames.com/public/ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 1483 Build 2341: Vulcan Build Jenkins Build 2340: arc lint + arc unit
Event Timeline
Perhaps this would be better off not introducing a new method to change a setting, but waiting for a gamesetup rewrite x) so that only the settings (but not derived settings) are saved?
Then every client could (attempt to) broadcast gamesettings, the NetServer could then broadcast these with the sender ID and then clients could decide which of these settings they accept and which to reject.
This way the NetServer is still gamesetting agnostic, has minimal changes and only one function to send gamesettings.
binaries/data/mods/public/gui/common/settings.js | ||
---|---|---|
36 | that file wasnt uploaded |
Is the build server down or broken, I haven't got any new pyrogenesis.exe and I've seen that a lot has changed lately?
I guess that question is not related to the discussion of the patch, but yes, we're working on it.
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/1028/ for more details.
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/1030/ for more details.
This will need to be completely rewritten after D3714 and the various gamesetup rewrites that happened in between, of course, but it should now be reasonably easy to do.
I think using a different message is a good idea -> it can make the C++ enforce that only the controller can send actual gamesetup messages, which is a good idea, as we need a single source of truth to maintain message ordering & thus synching clients
You don't need a custom message class, since you could just pass a string instead of a JS::RootedValue (I may or may not change the existing gamesetup message in the meantime).