Page MenuHomeWildfire Games

Allow specifying gaia settings in maps, and do so in the maps.
AbandonedPublic

Authored by leper on Jul 13 2017, 12:46 AM.

Details

Reviewers
elexis
Sandarac
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Owners Package(Owns No Changed Paths)
Trac Tickets
#3886
#3998
Summary

This is WIP, but I'd like a few comments/suggestions on the gamesetup changes.

NOTE: I just include one map in this, since arc seems to fail uploading the diff where I change all maps. (Add gaia, some cleanup of things that are specified in the defaults.)

TODO: make this and D642 a new map xml version.
TODO: Add a script to update maps. (Also for D642, see P59)
TODO: Actually just have {} instead of { "Civ": "gaia" } since that is the default that gets used.
TODO: Make Atlas emit {} for gaia instead of null
TODO: cleanup the gui changes.
TODO: Remove debugging code (warns/commented out code), fix TODOs

Test Plan

Test that starting maps and the gamesetup still works as before.

Add another player template, possibly with a new custom component that does something (eg warn something), specify it as the civ in the map, start that map, see that it works.

Diff Detail

Event Timeline

leper created this revision.Jul 13 2017, 12:46 AM
Owners added a subscriber: Restricted Owners Package.Jul 13 2017, 12:46 AM
leper edited the test plan for this revision. (Show Details)Jul 13 2017, 12:49 AM
leper updated the Trac tickets for this revision.
Vulcan added a subscriber: Vulcan.Jul 13 2017, 3:45 AM

Build has FAILED

Link to build: http://jw:8080/job/phabricator/1741/
See console output for more information: http://jw:8080/job/phabricator/1741/console

Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/helpers/Player.js
|  81| »   »   let·cmpPlayer·=·QueryPlayerIDInterface(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'cmpPlayer' is already declared in the upper scope.

binaries/data/mods/public/simulation/helpers/Player.js
| 174| »   »   »   let·cmpPlayer·=·QueryPlayerIDInterface(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'cmpPlayer' is already declared in the upper scope.

binaries/data/mods/public/simulation/helpers/Player.js
|  68| »   »   var·civ·=·getSetting(playerData,·playerDefaults,·i,·"Civ");
|    | [NORMAL] JSHintBear:
|    | 'civ' is already defined.

binaries/data/mods/public/simulation/helpers/Player.js
|  69| »   »   var·template·=·cmpTemplateManager.TemplateExists("special/player_"+civ)·?·"special/player_"+civ·:·"special/player";
|    | [NORMAL] JSHintBear:
|    | 'template' is already defined.

binaries/data/mods/public/simulation/helpers/Player.js
|  70| »   »   var·entID·=·cmpPlayerManager.GetPlayerByID(i);
|    | [NORMAL] JSHintBear:
|    | 'entID' is already defined.

binaries/data/mods/public/simulation/helpers/Player.js
|  79| »   for·(var·i·=·0;·i·<·numPlayers;·++i)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/helpers/Player.js
|  89| »   »   if·(i·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/helpers/Player.js
| 141| »   »   »   for·(var·j·=·0;·j·<·numPlayers;·++j)
|    | [NORMAL] JSHintBear:
|    | 'j' is already defined.

binaries/data/mods/public/simulation/helpers/Player.js
| 172| »   »   for·(var·i·=·0;·i·<·numPlayers;·++i)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'g_GameStanzaTimer' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 318| 318| /**
| 319| 319|  * Index of the GUI timer.
| 320| 320|  */
| 321|    |-var g_GameStanzaTimer = undefined;
|    | 321|+var g_GameStanzaTimer;
| 322| 322| 
| 323| 323| /**
| 324| 324|  * Only send a lobby update if something actually changed.
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'yPos' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1148|1148| 
|1149|1149| function verticallyDistributeGUIObjects(parent, objectHeight, ignore)
|1150|1150| {
|1151|    |-	let yPos = undefined;
|    |1151|+	let yPos;
|1152|1152| 
|1153|1153| 	let parentObject = Engine.GetGUIObjectByName(parent);
|1154|1154| 	for (let child of parentObject.children)

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 696| »   »   »   let·pData·=·playerData.find(pData·=>·sameColor(g_PlayerColorPickerList[selectedIdx],·pData.Color));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'pData' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1604| »   while·(g_IsNetworked)
|    | [NORMAL] ESLintBear (no-unmodified-loop-condition):
|    | 'g_IsNetworked' is not modified in this loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2061| »   »   for·(let·guid·in·g_PlayerAssignments)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'guid' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2061| »   »   for·(let·guid·in·g_PlayerAssignments)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'guid' is already declared in the upper scope.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 321| var·g_GameStanzaTimer·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'g_GameStanzaTimer' to 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 947| »   g_IsTutorial·=·attribs.tutorial·&&·attribs.tutorial·==·true;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'true'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
| 982| if·(i·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1151| »   let·yPos·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'yPos' to 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1324| »   »   i!=0·&&·Object.keys(g_PlayerAssignments).every(guid·=>·g_PlayerAssignments[guid].player·!=·i)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1492| »   »   »   if·(i·!=·0)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1537| »   »   if·(index·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1586| »   if·(g_LoadingState·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1638| »   »   if·(playerData.some((pData,·j)·=>·i·!=·j·&&·sameColor(playerData[i].Color,·pData.Color)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1639| »   »   »   playerData[i].Color·=·g_PlayerColorPickerList.find(color·=>·playerData.every(pData·=>·!sameColor(color,·pData.Color)));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1811| »   »   if·(i·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1819| »   »   »   chosenCiv·=·pickRandom(Object.keys(g_CivData).filter(civ·=>·g_CivData[civ].Culture·==·culture));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1833| »   »   let·usedName·=·g_GameAttributes.settings.PlayerData.filter(pData·=>·pData.Name·&&·pData.Name.indexOf(chosenName)·!==·-1).length;
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1866| »   »   »   if·(i·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2185| »   »   if·(i·!=·0)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2194| »   »   if·(i·!=·0)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2275| »   if·(g_GameStanzaTimer·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|2315| »   if·(g_GameStanzaTimer·!=·undefined)
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/314/ for more details.

elexis edited edge metadata.Jul 13 2017, 2:43 PM

Every n'th place adding a -1 and every n'th +1st place removing a -1 will be fun to review.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
432

Arbitrary design decision from rP9336

487

Maybe the above default could be moved here by changing g_OptionOrderInit.
(reloadMapFilterList only depends on the map type and should ideally remain independent of the number of players.)

elexis requested changes to this revision.Dec 19 2017, 5:37 PM

It is a good patch. If the TODOs and debug code is removed and more -1 +1 exceptions removed, it can be committed.

The idea is to remove any gaia exceptions, assumptions where possible, minimizing the complexity (while incidentally gaining more control over gaia settings).
Adding gaia dropdown and returning true from`hidden` of g_PlayerDropdowns[dropdown] for gaia should remove any +1 and -1.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
1003

Compensating a -1 in one function with a +1 in the other function was indeed what we wanted to fight.

1494

&&.
A civ != "gaia" check rather than a i != 0 check would remove one more unneeded assumption in the code and reveal meaning.
(I'm not taking any responsibility for this function, it should be deleted for the most part without losing the feature.)

1528

yay

1537

civ == "gaia"?

This revision now requires changes to proceed.Dec 19 2017, 5:37 PM
leper abandoned this revision.Dec 19 2017, 6:40 PM

Feel free to take over, I guess here you're unlikely to break things you don't care to understand.