HomeWildfire Games

Reveal AI settings in the gamesetup to all players. Based on patch by Imarok…

Description

Reveal AI settings in the gamesetup to all players. Based on patch by Imarok, fixes #3911.

Details

Committed
elexisApr 21 2016, 10:07 PM
Parents
rP18077: Add capture animation to champion infantry of different civs
Branches
Unknown
Tags
Unknown

Event Timeline

elexis added a subscriber: elexis.Dec 15 2018, 11:58 AM
elexis added inline comments.
/ps/trunk/binaries/data/mods/public/gui/gamesetup/gamesetup.js
1434

Since noone else mentioned it yet: dirty hack.

It would be nicer to update the existing page with the new GameAttributes rather than assembling the page state, sending the page state to the parent page, closing the child page, opening a new child page with the new parent page data and previous child page data.

This has 2 advantages:

  1. Closing a GUI page should not be closed if we want to change it. This is untrue code-wise and it it's mere luck that the user doesn't notice that the page is closed (it might just as well be visible for few frames).
  2. The parent GUI page should be agnostic of the internals of the child pages, but the approach of this commit hardcodes the internals of the child GUI Page. This may not be a big problem as this is the only case where the topmost page is updated by the parent page. But #3787 is another page that will have substantially more user-selection data that the parent page shouldn't have to remember as well (in particular since multiple parent pages would have to store the data: session, gamesetup, ...). Perhaps that also explains why some sessions dialogs such as the diplomacy dialog or trade dialog aren't new pages but just new objects in the same page.
lyv added a subscriber: lyv.Dec 15 2018, 12:48 PM
lyv added inline comments.
/ps/trunk/binaries/data/mods/public/gui/gamesetup/gamesetup.js
1434

Kinda related to this. More of a side-effect of this implementation.

  • In a networked game, the second player views the config of the 3rd AI player.
  • The host changes number of players to 2.
  • The second player would try to Pop the non-existent page.
elexis raised a concern with this commit.Dec 15 2018, 2:17 PM
elexis added inline comments.
/ps/trunk/binaries/data/mods/public/gui/gamesetup/gamesetup.js
1434

Thanks for that hint, I was already wanting to mention that we two saw this PopGuiPage stacktrace, but I never know how to reproduce it other than guessing hotloading could be involved.

This commit now has outstanding concerns.Dec 15 2018, 2:17 PM
elexis removed an auditor: elexis.Jan 7 2020, 3:19 PM
elexis added inline comments.
/ps/trunk/binaries/data/mods/public/gui/gamesetup/gamesetup.js
1434

Fixed by rP23341.

While I wrote the patch to allow the one gui page (gamesetup) to send data to another GUI page (aiconfig),
I'm not sure anymore whether this is the most clean approach.

The alternative is to make the aiconfig dialog a GUI object of the gamesetup page, similar to the diplomacy dialog in the session.

This has the disadvantage that there is less separation of concerns (aiconfig has full access to GUI page and may unintentionally shadow variables for instance) but the advantage that actually shared concerns can be covered (for example reacting to g_GameAttributes changes).

Its not only relevant for this gamesetup subpage but also for pages where the players can select per-player starting resources, heroes, or the MapBrowser (see also D1777). For the mapbrowser for example it would mean it could also reuse the same MapCache and with D2483 it could reuse the GameSettingsControl or PlayerAssignmentsControl without cross context access and cloning.

This commit no longer requires audit.Jan 7 2020, 3:19 PM