Opened 2 years ago

Closed 10 months ago

Last modified 9 months ago

#5322 closed defect (fixed)

Object orientation gamesetup rewrite

Reported by: elexis Owned by: elexis
Priority: Must Have Milestone: Alpha 24
Component: UI – Game setup Keywords:
Cc: Krinkle Patch: Phab:D2483

Description

#3994 / r19504 / Phab:D322 has unified all gamesetup dropdowns, checkboxes and some miscellaneous GUI objects.

There are still about 1900 lines of code that are in procedural style and use global variables where they could use local variables.

This will be the precondition for future work on the gamesetup, for intsance:

  • sliders in the gamesetup (for example ceasefire duration)
  • map specific gamesetup options #4838 (for example day/night variant on danubius or water-level-rise-time on extinct volcano)
  • anything

It will also make it more obvious to gamesetup developer that procedural code is not the right way to keep this massive GUI page grouped by logic.

Change History (13)

comment:1 by elexis, 2 years ago

In r21882:

Gamesetup cleanup.

Move civInfo press event handling from XML to the JS GUI object data from rP19504 / D322. Equally move civInfo tooltip from init (which should remain agnostic of content) following rP21339 / D846, refs #4970.

comment:2 by Imarok, 20 months ago

Component: UI & SimulationGame setup

Seems like that flew under my radar.

comment:3 by Krinkle, 16 months ago

Cc: Krinkle added

comment:4 by elexis, 15 months ago

In 22676:

PushGuiPage support for passing a function instead of a function name.
Allows coding the GUI without global functions which break prototype-oriented coding, refs #5322, fixing the concern in rP14496.

Supports stacked message boxes and removes the according workaround.
Change structree / civinfo switch-dialog code from rP21339 to perform the callback for page that actually registered the callback.
Ensure the parent that the callbackhandler is always called if the page is closed.
Merge PopGuiPage and PopGuiPageCB following that choice, incidentally leaving cleaner code.

Differential Revision: https://code.wildfiregames.com/D1684
Comments by: Yves, Vladislav, wraitii, leper

comment:5 by elexis, 14 months ago

In 22826:

Gamesetup OOP cleanup, refs #5322.

Move procedural updateGameDescription and getMapPreview to OOP g_MiscControls from rP19504/D322.
Delete unused variable childSize following rP20945.
Remove host check in launchGame from rP7653 needless since rP15006.

comment:6 by elexis, 14 months ago

In 22827:

Gamesetup OOP refactoring, refs #5322.

Move rightAlignCancelButton from rP18701 to g_MiscControls from D322 / rP19504.
Move civResetButton from rP18239 and teamResetButton from rP18459 to g_MiscControls, refs #3805.
The constant string and constant function value assignment can be reverted if performance is valued over code structuring.

comment:7 by elexis, 14 months ago

In 22828:

Gamesetup cleanup, refs #5322.

Inline toggleReady from rP15006 which became a oneline proxy to setReady in rP18299 used only in one place.
Inline senderFont and use setStringTags using rP20697 from Freagarachs patch in D2151.

comment:8 by elexis, 14 months ago

In 22831:

Gamesetup OOP cleanup, refs #5322.

Move updateAutocompleteEntries to g_MiscControls from rP19504/D322 to remove a global variable, a global procedure, reduce updateGUIObjects logic and increase logical grouping.
Rename autoCompleteNick to autoCompleteText since it's not only used for nicknames following rP19504.

comment:9 by elexis, 14 months ago

The XmppClient and NetClient GUI message handlers can be called from C++ rather than by pulling in onTick: #5585.

comment:10 by elexis, 11 months ago

Milestone: BacklogAlpha 24
Owner: set to elexis

(Started Nov 28th after having idled Nov 27th, refs https://code.wildfiregames.com/D2445#102076)

Last edited 11 months ago by elexis (previous) (diff)

comment:11 by elexis, 11 months ago

Patch: Phab:D2483

comment:12 by elexis, 10 months ago

Resolution: fixed
Status: newclosed

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:13 by elexis, 9 months ago

In 23419:

Rewrite Gamesetup AIConfig page to use class syntax, decouple settings and move it to the Gamesetup subfolder context, refs #5322, #5387.

This establishes a code architecture where only one class and only one page is responsible per AI setting,
removes the hardcodings from the XML and JS file,
allowing future patches and mods to insert, remove and edit independent AI settings without having to overwrite the existing code.

Differential Revision: https://code.wildfiregames.com/D2577
See also rP23413/D2581 and comments linked there.

Note: See TracTickets for help on using tickets.