[PATCH] Different timeouts for wonder victory

[elexis]

If you play the wonder mode, then the party who builds the wonder will have won 5 minutes after the wonder was built.

It is very hard to kill it in those 5 minutes, if the wonder building team rushed it (since it is possible to build it minute 11 with two players). You need at least 20-30 champions (rather 40-50) to kill it (minute 14) and that only works if the wonder building team didnt build three layers of walls around it.

The host should be able to select different timeouts in the gamesetup - like 5, 10, 15, 20 and 30 minutes.

[FeXoR]

Though in general I like options IMO we should be careful with host options for victory/defeat conditions (game modes) because with to many options the players joining the game have to look through all that options and might miss an important one for choosing their strategy.

Adjusting the time needed for a wonder to stand until victory is granted should be adjusted to be fun ofc. *Thumbs up*

[svott]

I implemented different timeouts (given in wonder_times.json [1, 5, 10, 15, 20, 25, 30])

Quite a lot of changes in different files. I hope it is not too messy.

Anyway, my 2 cents 1) I don't like gamesteup.js file. It's too big and not clean. 2) The options dialog could look better. However, I won't produce bad code. I suggest keeping the GUI code more dynamical to avoid the black line in the dialog when wonder isn't selected and to keep the code small. 3) I added two new functions to EndGameManager.js to make the wonder timeout accessible. That makes the js file less generic. It's might be a better solution to use a global wonder object that stores the timeout property

[elexis]

Thanks for working on this (especially since gamesetup is messy). I've attached a readable version of the patch that fixes the following issues I saw, except finishing resizeMoreOptionsWindow. Please follow the wiki:Coding_Conventions.

Whitespace:

• Almost every line has wrong whitespace (don't have trailing whitespace, use tabs and check the indentation)
• Like 3 of 4 changed lines only change whitespace without changing the code, which makes this patch unreadable unless applying the patch and then creating a patch that ignores whitespace changes. There are some options for diff tools to do that. In case your editor introduced them, teach it a lesson.

gamesetup.js:

• The host may not change the duration on scenario maps (something I wasn't aware about when writing the ticket)
• if (victoryTitle == "Wonder") -> don't check for the value of a translated string
• Missing semicolon at initWonderDurations. You might want to check using jshint for syntax errors.

resizeMoreOptionsWindow

• The GUI objects need to be updated for both singleplayer and ratedgames. That function must become generic now, otherwise we need like 4 if's and like 10 hardcoded GUI sizes.
• Calling this function must be done in updateGUIObjects(), otherwise it's not updated for the clients (since the dropdown is only updated for the host)
• The hiding of the element should not be done in this function, it should only resize.
• A simplification: If you have the pattern if (x) y="foo" else y="bar", just use the ​https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Conditional_Operator. Since it's true|false in this case, you can just say .hidden = g_VictoryConditions.Title[victoryIdx] == "wonder".
• Add a comment explaining where the magic numbers come from, like that one for the multiplayer-options

updateGUIObjects

• g_GameAttributes.settings.WonderDurations should be g_GameAttributes.settings.WonderDuration, since it only saves the selected value

setMapDescription

• Build the victory title string in one action (hence not moving around that g_VictoryConditions.Default check)
• " (" + translate("Wonder victory time") + ": " doesn't properly support internationalization. Translators should be able to translate the formatting too. How about this thing:

  		victoryTitle = sprintf(
  				translatePluralWithContext(
  					"victory condition",
  					"Wonder (%(min)s minute)",
  					"Wonder (%(min)s minutes)",
  					g_GameAttributes.settings.WonderDuration
  				),
  				{ "min": g_GameAttributes.settings.WonderDuration }
  		);
  

EndgameManager.js

• missing semicolons

gamesetup.xml

• Rename "wonder" / "wonderText" to "wonderDuration" / "wonderDurationText" (so it's more specific and other wonder options might be added later hypothetically)
• Rename "wonderEntry" to "optionWonderDuration", so it's in line with the other objectnames of that level
• Tooltip: "Set the survival time after a wonder has build" => How about: "Number of minutes that the player has to keep the wonder in order to win"? (Since surviving itself is not sufficient to win)

settings.js:

• wonder survival times => wonder victory times
• I'd suggest using "wonder victory" instead of "wonder" as context

wonder_times.json:

• trailing whitespace

Wonder.js

• Don't comment out, remove
• Now we have an unused number in the template. I asked the author sanderd17 on irc and the intended usage for that number in the template is to make the timeout for different civs a balancing decision too. So just changing TimeTillVictory to DurationMultiplier in the template is a nice solution to both use the tempalte and gamesetup setting.
• Maybe we can get scythewirler to change that value for ptolemians, since they don't have any resource costs for houses and economic buildings, hence a big advantage in wondergames.

---

Stopped looking now, in particular couldn't I test the patch or had the time to check all the XML number changes. I think about making all of that generic. If I don't find the time to do that, you can fix that resizeMoreOptionsWindow to work in both cases, check the XML numbers again and test the actual patch (ideally in multiplayer-mode by using two windows of 0ad, in case that works on your OS).

[elexis]

Fixed most things noted above besides resizeMoreOptionsWindow. Didn't test.

[svott]

Hey! I found three mistakes

• nonNegativeDecimal does not exists and kill the entire game!

"ERROR: CXeromyces: Parse error: in_memory_buffer:9: Error type 'nonNegativeDecimal' is not exported by type library '​http://www.w3.org/2001/XMLSchema-datatypes'"

• There is a logic mistake in gamesetup.js

Engine.GetGUIObjectByName("optionWonderDuration").hidden =

g_GameAttributes.settings.GameType && g_GameAttributes.settings.GameType == "wonder";

=> != "wonder"

• WonderDuration not WonderDurations in Setup.js

line 52: if (settings.WonderDuration)

• if you do thinks like "Maybe we can get scythewirler to change that value for ptolemians, since they don't have any resource costs for houses and economic buildings, hence a big advantage in wondergames."

-- then we should display it somewhere

Test: Only single game yet

resizeMoreOptionsWindow: not done yet

[svott]

according to comment 5 + resizeMoreOptionsWindow + passed multiplayer test

[svott]

Too bad, because elexis reviewed the patch already two month ago (talked about it in IRC) and the only problem is that scythetwirler opinion about the general behaviour is needed.

[mimo]

a few comments: line 334 minuts -> minutes in the Wonder schema, some comment indicating what DurationMultiplier is used for

[elexis]

Guess we should use decimal instead of an integer type, as the multiplier should be something like 0.8 or 1.2.

I added you to the credits already, but do you want to be mentioned there with your real name too?

[elexis]

Some remarks by sanderd17:

• <ref name='nonNegativeDecimal'/>" instead of "<data type='nonNegativeInteger'/>
• Don't set it to 0 on init but keep it undefined, to make it obvious that we rely on the setter to be called (otherwise we could also set a timeout of 5 minutes by default)
• the json file is not in the most recent patch, but one patch before
• 5 hunks of the patch above are conflicting, shouldn't stop us though

[sanderd17]

I have updated the patch again so it applies cleanly to the current SVN.

But IMO the gui looks really ugly, it shows a hole when the wonder victory isn't selected.

I also still doubt whether it's a good idea to store the time as minutes. So far, we only use seconds and milliseconds to store time, and it's already confusing enough. Using another unit will only make it more confusing.

[svott]

Didn't I implement automatically resize!? Too long ago and can't remember exactly.

[Itms]

So this is too much subject to discussion to be included now apparently.

Thanks for working on it, please address all the remarks above and we'll look into committing it in A21. :)

[elexis]

The issue with this patch is the missing automatic-resizing.

• The wonder-option should appear right after the victory condition
• In session.xml every option below the item needs a name (like "optionRatings")
• If the game mode is not victory-condition, all elements below that dropdown should move one line up
• The new height of the dialog should be derived from the number of options visible

Should be possible with like 30 lines of code.

[elexis]

Patch update:

• Fixes typo, adds comment to schema mentioned by mimo
• Uses a float type in the schema
• Includes wonder_times.json
• Saves time in milliseconds as sugested by sanderd17
• Starts with 5 minute timeout if SetWonderDuration wasn't called (should never happen (make it undefined to indicate that?))

remaining TODO:

• Automatic resizing (comment:15)
• Still don't know if svott wants his realname in the credits.

[svott]

:D Before you ask another time or have sleepless nights: Sven Ott

[elexis]

In 17912:

Update credits, refs #3234.

[svott]

Finally had some time for 0AD

I tried to use as few code as possible. Don't know if you accept a Boolean summation [let reduceOptions = optionWonderDurationHiddenState + !g_IsNetworked + !Engine.HasXmppClient()]

[svott]

compared to 2.3: only relevant changes in gamesetup.js line 307-339

[svott]

arrrrg.. I know you don't like tabs in empty lines. I will remove them if further changes are required.

[bb]

Didn't think you wanted to do that...

[elexis]

Hi svott, thanks for the update, this patch is really needed! :-)

I checked only the new parts of the patch and found few style optimizations should be done:

• The comments above let reduceOptions = optionWonderDurationHiddenState + !g_IsNetworked + !Engine.HasXmppClient() are somewhat outdated (since the new option is taken into account). Remove them and just add one line mentioning that we reduce the size of the dialog by up to three options (wonderduration, cheats and rated game).

• if(g_IsController) -> if (g_IsController)
• resizeOptionButton and resizeOptionDialog used only once -> nuke / inline
• missing semicolons after statements all over the place
• Don't hardcode that many values like 218, 246, but compute them using optionButtonHeight, so only 1-3 values will have to be changed when new options are added (instead of 12) optionButtonHeight sounds like it should be a global const, so that you don't need to compute nor pass the y2 argument of that function, but compute it inplace. You also don't need to pass hiddenWonderReduction but can immediately subtrat if from the y1 argument.
• The computation optionButtonHeight * reduceOptions is done twice, could be done only once by multiplying in the line above that.
• The variable names could be improved, for example
  • optionWonderDurationHiddenState -> wonderDurationHidden
  • maybe hiddenWonderReduction -> offset?

[svott]

Thx for the fast review :) I reworked all your hint -- didn't find the if(

• g_optionButtonHeight: Size 30 is kind of wrong because 28 + 2 for the gap would be right, but 30 reduces the code a bit
• I don't know if you allow nested functions. For me, its makes the code clearer and strengthen encapsulation

[svott]

relating to comment 22 and 23

[elexis]

Rewrote resizeMoreOptionsWindow to (almost) not use magic-numbers.

[elexis]

Changelog:

Enhancements:

• Rewrote resizeMoreOptionsWindow to (almost) not use magic-numbers and work with arbitrary options
• Added some more choices to the json

Fixed minor mistakes:

• You forgot again to svn add the wonder_times.json!
• this.wonderDuration = 5; must be in milliseconds, not minutes
• that moreOptions number change in the XML seems wrong

Stylechanges:

• Did I mention missing semicolons? newline before {
• g_optionButtonHeight -> g_OptionButtonHeight
• wonderDurationHidden -> optionWonderDuration.hidden (more consistent with the rest of the code)
• not fond of defining a function inside a function (esp. if it uses parent function vars)

TODO:

• the wonder setting should be shown right below the victory condition
• therefore add names to all options and add those to the function

Otherwise, the end is near.

[svott]

Nice loop, interesting javascsript syntax for yPos || gSize.top

[svott]

relating to 24

[svott]

I am testing it right now I just realised that there is a warn left in simulation/helpers/Setup.js at line 52

warn(cmpEndGameManager.GetWonderDuration());

[elexis]

In 18075:

New gamesetup option enabling the host to vary the time required to win the wonder-gamemode. Patch by svott, fixes #3234.

[elexis]

Finally, thanks for all the work and persistance! :)

Two things I had changed:

• optionCeasefireText -> optionCeasefire (occured in two other places as well)
• added two lines to selectMap to ensure setting the default duration when a scenario map sets wondermode but no timeout

[elexis]

In 18189:

Allow more choices for the wonder timeout, refs #3234.

[elexis]

In 18398:

Wonder mode cleanup.

Remove unused, seemingly unusable wonder duration multiplier, refs #3234.
Replace wonderDuration of the EndGameManager with gameTypeSettings, since the component should be agnostic about victory condition logic.
Remove an unused cmpEndGameManager.
Inline a variable, few newlines, let intsead of var.

[elexis]

In 18411:

Implement wonder victory games that are finished instantaneously after build, refs #3234.

Thus don't supplement the hypothetically missing wonderduration gameattribute with a hardcoded simulation default.