Opened 9 years ago

Last modified 2 years ago

#3049 new defect

Rewrite/restructure gamesetup

Reported by: leper Owned by: leper
Priority: Should Have Milestone: Backlog
Component: UI – Game setup Keywords:
Cc: Patch:

Description (last modified by wraitii)

Currently gamesetup.js is not really extensible to support trigger/campaign maps where some settings should be fixed while others can be changed (e.g. allowing the player to assign himself to slots 1 and 3 on a 4 player map). See also #4838.

The logic to decide what settings can be changed (for RMS, Skirmish Maps, and Scenarios) is also quite duplicated. (done in r19504)

The code could also use some improvement such that the persistent game settings can work with just storing the minimum of information needed to recreate the settings instead of saving everything and then hoping that it doesn't cause conflicts/issues later (see #3044, #3033).

--

See https://trac.wildfiregames.com/ticket/3049#comment:16 for updates

Change History (19)

comment:1 by Itms, 9 years ago

In 16346:

Some hacks to fix game setup persisting problems, patch by @aBothe.
Fixes #3033, #3034.

The whole gamesetup.js system should be eventually re-factored to remove those hacks and to improve the robustness of the code: see #3049.

comment:2 by Alex, 9 years ago

Is there a gamesetup-wishlist?

Things like the ability that each player in a networked gamesetup should be able to choose his/her own civ/color/position - perhaps a more AOE-like behaviour could be taken, so that colors are hardwired to positions or such.

Anyway, yes, gamesetup needs a rewrite.

comment:3 by Stan, 9 years ago

If you could have a look at #1580 I'd be happy :) else #1834 also #812

:)

comment:4 by fabio, 9 years ago

Description: modified (diff)

See also #3013.

comment:5 by leper, 9 years ago

In 16445:

Some gamesetup cleanup. Refs #3049.

comment:6 by elexis, 9 years ago

Milestone: Alpha 19Backlog

comment:7 by Imarok, 8 years ago

I CC me to this ticket

comment:8 by elexis, 7 years ago

In 19504:

Unify gamesetup option handling, fixes #3994, refs #3049.

Removes a lot of duplication and ugly GUI handling code with very similar, yet different code paths.
Move the setting specific logic to the functions of that setting and keep the universal logic in global atomic functions.
Make gamesetup.xml agnostic of all gamesetup settings.

Adding a new gamesetup option now only requires adding one hunk with the titles and values and one line in g_OptionOrderGUI.
Opens up the possibility to iterate over all settings, refs #3806, #3883.

Allow starting singleplayer games in observermode with only AIs assigned, fixes #4078.
Autocomplete translations of all setting titles and selected title values like playernames and victory conditions.

Transfer the AI difficulty when swapping with a player.
Move logic from onTick to the GUI handling functions.
Change some global consts to var, so that contributors become invited to change them from a different place.
Add missing startGameButton tooltip translation.

Differential Revision: https://code.wildfiregames.com/D322
Reviewed By: wraitii
Some strings reviewed By: leper

comment:9 by elexis, 6 years ago

Description: modified (diff)

comment:10 by elexis, 6 years ago

Description: modified (diff)

comment:11 by Imarok, 5 years ago

Component: UI & SimulationGame setup

Move tickets to Game Setup as UI & Simulation got some sub components.

comment:12 by elexis, 4 years ago

Currently gamesetup.js is not really extensible to support trigger/campaign maps where some settings should be fixed while others can be changed

Phab:D2483 rewrites/restructures gamesetup to be really extensible to support trigger/campaign maps where some settings should be fixed while others can be changed.

The question is only what "some" means.

Either it means a map can predetermine every mapsetting, which would be the greatest extent of freedom of map authors, or it means only a subset of the settings, such as the example you mentioned (which is covered in that patch):

(e.g. allowing the player to assign himself to slots 1 and 3 on a 4 player map)

Notice that allowing the map to fix every type of setting will also break the "Random" random map selection. That item inherently requires the random map to adapt to the user given settings.

For example if the users chose 8 players with 4 AIs on a large mapsize with revealed map and locked teams on a random map, but then the randomly chosen map only supports medium size with 2 players, diplomacy mode and unrevealed map, either the players will get the opposite of what they ordered, or that map will be excluded from the list of random maps that can be chosen (idea by bb few years ago on irc). The latter however means that no map at all may be compatible with the user chosen settings if all maps make restrictions. Random map scripts are made from code, so they can perfectly shape the map according to user provided settings. So that gamesetup support may be implemented, but it would still be a poor map design decision for the stated reasons, which is why I closed #1834 as won't fix.

So the points mentioned in this ticket may either be closed as fixed or closed as "needs info", where info is actual feature design.

Especially notice that there is literally no distinction between Skirmish and Scenario maps anymore if both maptypes can restrict or open up the same amount of settings, which is something that should have been planned before introducing the Skirmish map type.

The gamesetup rewrite at Phab:D2483 will allow Skirmish/random maps to fix AIs and Civs per playerslot while for the other setting types overwriting the user chosen values with the map values while still leaving the user the freedom to change these settings, and simultaneously discouraging maps from using this feature in preference of keeping user setting value choices when there is no strict necessity to overwrite those.

Notice one could also change the mapData format to allow both specifying only a default value for a setting type and fixing a setting value. For example { "Civ": { "value": "rome", "fixed": true } }. This would cover the point "some settings should be fixed while others can be changed" to the fullest extent but would come at the disadvantage of introducing a new format and more complexity (and still breaking the random-random map item and still surprising the user with settings he didn't chose). So either this is what is wanted by the ticket description, or close as fixed, or close as needs info, or close as invalid.

comment:13 by elexis, 4 years ago

In 23374:

Gamesetup class rewrite, fixes #5322, refs #5387.

  • Decouples settings logically which in turn allows fixing many problems arising from previous coupling.
  • Fixes the persist-match-settings feature, refs #2963, refs #3049.
  • Improves performance of the matchsetup by rebuilding GUI objects only when necessary.

Provides groundwork for:

  • UI to control per-player handicap, such as StartingResources, PopulationCap, StartingTechnologies, DisabledTechnologies, DisabledTemplates, ..., refs #812.
  • Map specific settings (onMapChange event), refs #4838.
  • Chat notifications announcing which settings changed, refs D1195,
  • Multiple controllers setting up the game (since setting types can check for permissions in onUpdateGameAttributes without the need for a new data model or a second gamesetup data network message type), refs #3806, subsequently dedicated server, refs #3556.
  • MapBrowser (MapCache, MapTypes, onUpdateGameAttributes interface), refs D1703 and D1777,
  • Multiplayer saved games (decoupling and setting dependent unique logic), refs #1088.

Refs https://wildfiregames.com/forum/index.php?/topic/20787-paid-development-2016/ https://wildfiregames.com/forum/index.php?/topic/20789-paid-development-2016/

Enable maps to restrict setting values:

  • If a map specifies an AI or Civs for a playerslot, the controller can't assign a player/other AI or Civ to that slot, refs #3049, #3013.

Fix per player StartingResources, PopulationCap, StartingTechnologies, DisabledTechnologies, DisabledTemplates following rP12756, refs #812, fixes #4504. Use this for DisabledTechnologies on Polar Sea.

Persist user settings for Skirmish maps:

  • All user chosen settings are persisted when changing the selected map or maptype, except where the selected map overwrites the setting value and except for Scenario maps which still use the default value where the map doesn't specify the setting value.
  • Tickets relating to that Skirmish mapchange user setting persistance:
    • Selecting a map doesn't change the selected civilizations, fixes #3120 (together with r23279 removing map specified Civs).
    • Selecting a map type doesn't reset the selected settings, fixes #5372.
    • Selecting a map doesn't change the selected victory conditions, unless the map specifies those, refs #4661, #3209. (Atlas still writes VictoryConditions to every map.)
    • Consume the player color palette from Skirmish maps, refs rP17040 / #1580. Preserve the selected playercolors when switching the Skirmish/Random map by chosing the most similar colors if the map comes with a different palette.

Rated games:

  • Hide and disable Rated game setting unless there are exactly two players, fixes #3950, supersedes D2117.
  • Display conspicuous warning if the game is rated, so players are perfectly aware.

Autostarted games:

  • Allow using the gamesetup page to autostart matches with arbitrary maps, not only this one tutorial, as reported in D194 and rP19599, refs D11.

Networking:

  • Keep gamesetup page open after disconnect, allowing players to read chat messages indicating why the host stopped the server, fixes #4114.
  • The message subscription system allows new and mod settings to run custom logic on arbitrary setting changes (most importantly on map change). This removes hardcoded logic restrictions from the gamesetup option unification rewrite in rP19504/D322, refs #3994, such as the hardcoding of setting references in selectMap to biomes from rP20115/D852 and the difficulty from rP20760/D1189, RelicDuration, WonderDuration, LastManStanding, RegicideGarrison, TriggerScripts, CircularMap, Garrison, DisabledTemplates.

Checkboxes:

  • Display values of disabled checkboxes with Yes/No labels, fixes D2349, reviewed by nani.

Clean g_GameAttributes of invalid values and gamesetup GUI temporaries, refs #3049, #3883:

MapCache:

  • Refactor to MapCache class, store maps of all types and use it in the replaymenu, lobby and session as well.

SettingTabsPanel:

  • Remove hardcodings and coupling of the SettingTabsPanel with biomes/difficulties/chat UI from D1027/rP20945.

GamesetupPage.xml:

  • Restructure the page to use hierarchical object organization (topPanel, centerPanel, centerLeftPanel, bottomPanel, centerCenterPanel, centerRightPanel, bottomLeftPanel, bottomRightPanel), allowing to deduplicate object position margins and size math and ease navigation.

New defaults:

  • Check LockedTeams default in multiplayer (not only rated games).
  • Persist the rated game setting instead of defaulting to true when restarting a match, which often lead to unintentional rated games when rehosting.
  • 60 FPS in menus since they are animated

Autocomplete sorting fixed (playernames should be completed first).
Refactoring encompasses the one proposed in Polakrity and bb D1651.

Differential Revision: https://code.wildfiregames.com/D2483
Tested by: nani
Discussed with:

Emojis by: asterix, Imarok, fpre, nani, Krinkle, Stan, Angen, Freagarach

comment:14 by wraitii, 3 years ago

In 25077:

Separate Game Settings from the GUI Gamesetup

Split the gamesetup in two: the 'GameAttributes' part into gamesettings/ and the GUI/Presentation part in gamesetup/. This makes it easier to separate the presentation from the data.

The immediate benefit is that campaigns & autostart scripts don't need to load the gamesetup folder at all. This also makes it much easier for any modder that would want to to change the GameSetup itself.

Each 'game attribute' is given a unique class extending GameSetting (with a few exceptions), in charge of:

  • 'Serializing' to the JSON-compatible 'InitAttributes' format, which is used for persisted settings, network synchronization, map script settings, hotloading.
  • Deserializing from the same format.
  • Watching for settings it depends on (such that e.g. unexploring the map also unreveals it).

The GUI controls remain in charge of presenting the state accurately, however they now directly subscribe to changes of the GameSettings for update. The updating logic in general has been lightened on the GUI side, making it more straightforward to know when something will update, and reducing un-necessary computations (in theory - in practice, I believe the gamesetup was already fairly good about this).
The 'Controller' class of the gamesetup have also been lightened, since more responsibility now lies with GameSettings. In particular, this include code to actually launch a game.

In general the GameSettings class is permissive - the GUI gamesetup has tighter restriction for what the player can/cannot modify. This is intended to give flexibility for campaign maps, which may want to change arbitrary settings.

Further work would be useful, non-exhaustively:

  • the setting of default values remains messy. They currently exist somethings in GameSettings, sometimes in the GUI gamesetup, and in the simulation itself (in case attributes are not set).
  • the availability and 'lockedness' of settings remains a work-in-progress.
  • some attributes, like disabled technologies, should probably be removed and triggers used instead.
  • the Handling of AI and player-specific data could be improved.
  • settings Persistence should follow its own path - not all settings are worth persisting.
  • GAIA settings are added simulation-side but not in the GUI, which is confusing.

Thanks langbart & Freagarach for testing.

Follows the gamesetup rewrite in rP23374.

Refs #3049

Differential Revision: https://code.wildfiregames.com/D3243

comment:15 by wraitii, 3 years ago

In 25099:

MP: don't enforce game init attributes synchronization in PREGAME.

The NetServer stored a complete copy of the game Init Attributes, which it sent to new clients on updates from the controller. This worked well, but prevents incremental updates and other unrelated messages from being sent.

This changes the system so that:

  • in PREGAME state, the server does not update its copy of the game init attributes
  • the server forwards game setup messages from the controller to all clients
  • Joining clients get a full copy of the Settings, when joining, from the controller (this is a js-driven behaviour - other situations might not need do it).
  • Make the StartNetworkGame message take a copy of the final init attributes, to ensure synchronization (and simplify some logic).

In practice, this:

  • makes it possible to send different types of gamesetup messages (this introduces two: a regular update and the full 'initial-update' for new clients).
  • moves some C++ hardcoding into JS - here in essence the PREGAME server state is now init-attributes-agnostic.
  • does not change much for readiness control - the server already needed to force a change at game start to set random elements.

Note that the loading page is currently still receiving the 'local' game attributes, which assumes that all clients are correctly synchronized (they should be).

Refs #3806, #3049

Differential Revision: https://code.wildfiregames.com/D3714

comment:16 by wraitii, 3 years ago

Milestone: BacklogAlpha 26

Adding that the player assignments should be reworked. First, the GUID -> data map is inconvenient for actual assignments, and second the C++ should not care until the game is started, letting JS handle it (see also game settings, where that's the case).

The initial description is still valid though some work towards lockable setting is in place, and settings are decoupled from the GUI somewhat.

Persistent settings should be improved by having their own deserialisation function IMO.

comment:17 by wraitii, 3 years ago

Description: modified (diff)

comment:18 by wraitii, 3 years ago

In 25699:

Fix gamesetup player assignment issue when joining

Switch some logic from C++ to JS in PREGAME for player assignments. Refs #3049

Fixes #6204

Reported by: Imarok

Tested By: Imarok

Differential Revision: https://code.wildfiregames.com/D4092

comment:19 by Freagarach, 2 years ago

Milestone: Alpha 26Backlog
Note: See TracTickets for help on using tickets.