Page MenuHomeWildfire Games

Split VictoryDuration
ClosedPublic

Authored by bb on Sep 5 2017, 7:00 PM.

Details

Summary

The victoryDuration dropdown is now used for both Relic and Wonder. However with #4014 both can be applied and imo one can do a 5 min wonder and 1h relic or something. Thus splitting the VictoryDuration in 2 everywhere.

Test Plan

grep and see all related things are changed,
play a wonder and a relic game
Notice that we don't need to change the g_VictoryDurations from settings.js, since that can be used for both

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

bb created this revision.Sep 5 2017, 7:00 PM
Owners added subscribers: Restricted Owners Package, Restricted Owners Package.Sep 5 2017, 7:00 PM
Vulcan added a comment.Sep 5 2017, 7:15 PM
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/gui/common/gamedescription.js
| 396| »   »   "label":·"[color=\""·+·g_DescriptionHighlight·+·"\"]"·+·title.label·+·":"·+·"[/color]",
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/common/gamedescription.js
|  89| »   »   if·(playerData·==·null·||·playerData.Civ·&&·playerData.Civ·==·"gaia")
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'null'.

binaries/data/mods/public/gui/common/gamedescription.js
|  94| »   »   let·isAI·=·playerData.AI·&&·playerData.AI·!=·"";
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with ''.

binaries/data/mods/public/gui/common/gamedescription.js
| 311| »   »   »   »   g_GameAttributes.settings.Ceasefire·==·0·?
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1682| »   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
|1507| »   »   »   »   if·(g_Settings.Biomes.every(bio·=>·bio.Id·!=·biome))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1508| »   »   »   »   »   warn("Map·'"·+·g_GameAttributes.map·+·"'·contains·unknown·biome·'"·+·biome·+·"'")
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

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

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1716| »   »   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
|1717| »   »   »   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
|1905| »   »   »   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
|1919| »   »   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.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jenkins-master:8080/job/phabricator_lint/490/ for more details.

bb updated this revision to Diff 3519.Sep 5 2017, 8:51 PM

Rename victoryDuration{vc} -> {vc}Duration

elexis added a subscriber: elexis.Sep 5 2017, 8:58 PM

If you ever wanted to add a "as proposed by leper" to a commit message, this one might be your chance
https://code.wildfiregames.com/rP19345#20089

Vulcan added a comment.Sep 5 2017, 9:38 PM

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!
Checking XML files...

http://jenkins-master:8080/job/phabricator/1977/ for more details.

Vulcan added a comment.Sep 5 2017, 9:39 PM
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/gui/common/gamedescription.js
| 396| »   »   "label":·"[color=\""·+·g_DescriptionHighlight·+·"\"]"·+·title.label·+·":"·+·"[/color]",
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/common/gamedescription.js
|  89| »   »   if·(playerData·==·null·||·playerData.Civ·&&·playerData.Civ·==·"gaia")
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'null'.

binaries/data/mods/public/gui/common/gamedescription.js
|  94| »   »   let·isAI·=·playerData.AI·&&·playerData.AI·!=·"";
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with ''.

binaries/data/mods/public/gui/common/gamedescription.js
| 311| »   »   »   »   g_GameAttributes.settings.Ceasefire·==·0·?
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1682| »   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
|1507| »   »   »   »   if·(g_Settings.Biomes.every(bio·=>·bio.Id·!=·biome))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1508| »   »   »   »   »   warn("Map·'"·+·g_GameAttributes.map·+·"'·contains·unknown·biome·'"·+·biome·+·"'")
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

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

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1716| »   »   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
|1717| »   »   »   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
|1905| »   »   »   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
|1919| »   »   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.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jenkins-master:8080/job/phabricator_lint/492/ for more details.

bb updated this revision to Diff 3523.Sep 5 2017, 9:39 PM

correct diff now

Vulcan added a comment.Sep 5 2017, 9:40 PM
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/gui/common/gamedescription.js
| 396| »   »   "label":·"[color=\""·+·g_DescriptionHighlight·+·"\"]"·+·title.label·+·":"·+·"[/color]",
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/common/gamedescription.js
|  89| »   »   if·(playerData·==·null·||·playerData.Civ·&&·playerData.Civ·==·"gaia")
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'null'.

binaries/data/mods/public/gui/common/gamedescription.js
|  94| »   »   let·isAI·=·playerData.AI·&&·playerData.AI·!=·"";
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with ''.

binaries/data/mods/public/gui/common/gamedescription.js
| 311| »   »   »   »   g_GameAttributes.settings.Ceasefire·==·0·?
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1682| »   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
| 610| »   »   »   g_GameAttributes.settings.RelicDuration·=·g_VictoryDurations.Duration[idx]
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1507| »   »   »   »   if·(g_Settings.Biomes.every(bio·=>·bio.Id·!=·biome))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1508| »   »   »   »   »   warn("Map·'"·+·g_GameAttributes.map·+·"'·contains·unknown·biome·'"·+·biome·+·"'")
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

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

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1716| »   »   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
|1717| »   »   »   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
|1905| »   »   »   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
|1919| »   »   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.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jenkins-master:8080/job/phabricator_lint/493/ for more details.

elexis added inline comments.Sep 5 2017, 10:22 PM
binaries/data/mods/public/gui/gamesetup/gamesetup.js
602 ↗(On Diff #3519)

This label seems too long. It barely fits into two rows for english. You bet theres a language where it won't fit.
Also it seems unproportionally long with regards to reading duration.

Can we get away with Victory Duration twice and hoping the reader is smart enough to figure out that they related to the victory condition above?

Alternative to that would be "Relic Duration" and "Wonder Duration" (which sounds grammatically incomplete)

603 ↗(On Diff #3519)

Number of minutes or just minutes?
"for" sounds wrong. "has achieved relic victory?" (or Relic Victory (proper vs common noun distinction not really relevant as there is nothing to confuse the common noun with)

binaries/data/mods/public/maps/scripts/WonderVictory.js
41 ↗(On Diff #3519)

The default value is something leper wanted to have nuked (must have been irc as the ticket doesn't contain that comment) when we implemented the wonder timeout setting (rP18075).
We had followed that with chaning the default 20 to default 0 :P
Unless you uncover a reason why this 0 is really necessary, I'd rather delete it too.

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!
Checking XML files...

http://jenkins-master:8080/job/phabricator/1978/ for more details.

bb marked 2 inline comments as done.Sep 5 2017, 10:55 PM
bb added inline comments.
binaries/data/mods/public/gui/gamesetup/gamesetup.js
602 ↗(On Diff #3519)

keeping in mind that both dropdowns will be visible when both victory conditions enabled, the alternative seems to be best option. (Notice we also have Relic Count the same way)

binaries/data/mods/public/maps/scripts/WonderVictory.js
41 ↗(On Diff #3519)

possible when changing the checks in setup.js so done (notice that Atlas has no support for these things yet but that can come later and isn't breaking anything now), same is done for other options already

bb updated this revision to Diff 3527.EditedSep 5 2017, 11:03 PM
bb marked an inline comment as done.

remove defaults, change some strings

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!
Checking XML files...

http://jenkins-master:8080/job/phabricator/1979/ for more details.

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

binaries/data/mods/public/gui/common/gamedescription.js
| 396| »   »   "label":·"[color=\""·+·g_DescriptionHighlight·+·"\"]"·+·title.label·+·":"·+·"[/color]",
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/common/gamedescription.js
|  89| »   »   if·(playerData·==·null·||·playerData.Civ·&&·playerData.Civ·==·"gaia")
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'null'.

binaries/data/mods/public/gui/common/gamedescription.js
|  94| »   »   let·isAI·=·playerData.AI·&&·playerData.AI·!=·"";
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with ''.

binaries/data/mods/public/gui/common/gamedescription.js
| 311| »   »   »   »   g_GameAttributes.settings.Ceasefire·==·0·?
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1682| »   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
|1507| »   »   »   »   if·(g_Settings.Biomes.every(bio·=>·bio.Id·!=·biome))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1508| »   »   »   »   »   warn("Map·'"·+·g_GameAttributes.map·+·"'·contains·unknown·biome·'"·+·biome·+·"'")
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

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

binaries/data/mods/public/gui/gamesetup/gamesetup.js
|1716| »   »   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
|1717| »   »   »   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
|1905| »   »   »   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
|1919| »   »   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.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jenkins-master:8080/job/phabricator_lint/494/ for more details.

elexis accepted this revision.Sep 6 2017, 12:56 AM

Code reads correct.
The victory_times.json is still used for both, but that's perfectly fine. Hence patch complete.
Tested.

Still dubious about the "Relic Duration" and "Wonder Duration" label.
Should be clear to the player what is meant. Yet technically a relic doesn't have a duration, only a relic victory.

"Relic Timeout" would also be something to consider and possibly reject.

(00:53:42) s0600204: elexis, They make sense in context.

So I guess the strings proposed in this patch are acceptable.

Assuming you're sure about the || 0 removal, ack.

Thanks for the patch! (Might want to ref that relic revision proposal)

binaries/data/mods/public/gui/gamesetup/gamesetup.js
603 ↗(On Diff #3519)

period (so glad I can yell about missing periods too :P)

This revision is now accepted and ready to land.Sep 6 2017, 12:56 AM
This revision was automatically updated to reflect the committed changes.