Opened 3 years ago

Closed 19 months ago

Last modified 17 months ago

#3234 closed enhancement (fixed)

[PATCH] Different timeouts for wonder victory

Reported by: elexis Owned by: elexis
Priority: Should Have Milestone: Alpha 21
Component: UI & Simulation Keywords: patch
Cc: Patch:

Description

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.

Attachments (10)

wonderOptions.patch (52.5 KB) - added by svott 23 months ago.
wonderOptions_v2.patch (22.0 KB) - added by elexis 23 months ago.
Fixed most things noted above besides resizeMoreOptionsWindow. Didn't test.
wonderOptions_V2.1.patch (16.0 KB) - added by svott 23 months ago.
according to comment 5 + resizeMoreOptionsWindow + passed multiplayer test
wonderOptions_V2.2.patch (15.1 KB) - added by sanderd17 21 months ago.
wonderOptions_V2.3.patch (21.0 KB) - added by elexis 21 months ago.
wonderOptions_V2.4.patch (16.1 KB) - added by svott 20 months ago.
compared to 2.3: only relevant changes in gamesetup.js line 307-339
wonderOptions_V2.5.patch (15.7 KB) - added by svott 19 months ago.
relating to comment 22 and 23
wonderOptions_V3.patch (20.8 KB) - added by elexis 19 months ago.
Rewrote resizeMoreOptionsWindow to (almost) not use magic-numbers.
wonderOptions_V3.1.patch (16.7 KB) - added by svott 19 months ago.
relating to 24
wonderOptions_V3.2.patch (17.8 KB) - added by svott 19 months ago.

Download all attachments as: .zip

Change History (41)

comment:1 Changed 3 years ago by Florian Finke

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*

comment:2 Changed 23 months ago by svott

Owner: set to svott
Status: newassigned

Changed 23 months ago by svott

Attachment: wonderOptions.patch added

comment:3 Changed 23 months ago by svott

Keywords: review patch added
Milestone: BacklogAlpha 20
Summary: Different timeouts for wonder victory[PATCH] Different timeouts for wonder victory

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

comment:4 Changed 23 months ago by elexis

Component: Core engineUI & Simulation
Keywords: review removed

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).

Changed 23 months ago by elexis

Attachment: wonderOptions_v2.patch added

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

comment:5 Changed 23 months ago by 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"

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

Changed 23 months ago by svott

Attachment: wonderOptions_V2.1.patch added

according to comment 5 + resizeMoreOptionsWindow + passed multiplayer test

comment:6 Changed 21 months ago by Itms

Keywords: review added
Milestone: Alpha 20Alpha 21

comment:7 Changed 21 months ago by svott

Owner: svott deleted
Status: assignednew

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.

comment:8 Changed 21 months ago by mimo

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

comment:9 Changed 21 months ago by Itms

Milestone: Alpha 21Alpha 20

comment:10 Changed 21 months ago by 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?

comment:11 Changed 21 months ago by 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

comment:12 Changed 21 months ago by 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.

Changed 21 months ago by sanderd17

Attachment: wonderOptions_V2.2.patch added

comment:13 Changed 21 months ago by svott

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

comment:14 Changed 21 months ago by Itms

Keywords: review removed
Milestone: Alpha 20Alpha 21

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. :)

comment:15 Changed 21 months ago by 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.

Changed 21 months ago by elexis

Attachment: wonderOptions_V2.3.patch added

comment:16 Changed 21 months ago by 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.

comment:17 Changed 21 months ago by svott

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

comment:18 Changed 21 months ago by elexis

In 17912:

Update credits, refs #3234.

comment:19 Changed 20 months ago by svott

Resolution: fixed
Status: newclosed

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?()]

Changed 20 months ago by svott

Attachment: wonderOptions_V2.4.patch added

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

comment:20 Changed 20 months ago by svott

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

comment:21 Changed 20 months ago by bb

Keywords: review added
Resolution: fixed
Status: closedreopened

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

Last edited 20 months ago by bb (previous) (diff)

comment:22 Changed 19 months ago by elexis

Keywords: simple removed

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?

comment:23 Changed 19 months ago by 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

Changed 19 months ago by svott

Attachment: wonderOptions_V2.5.patch added

relating to comment 22 and 23

Changed 19 months ago by elexis

Attachment: wonderOptions_V3.patch added

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

comment:24 Changed 19 months ago by 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.

comment:25 Changed 19 months ago by svott

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

Changed 19 months ago by svott

Attachment: wonderOptions_V3.1.patch added

relating to 24

comment:26 Changed 19 months ago by 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());

Changed 19 months ago by svott

Attachment: wonderOptions_V3.2.patch added

comment:27 Changed 19 months ago by elexis

Owner: set to elexis
Resolution: fixed
Status: reopenedclosed

In 18075:

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

comment:28 Changed 19 months ago by elexis

Keywords: review removed

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
Last edited 19 months ago by elexis (previous) (diff)

comment:29 Changed 19 months ago by elexis

In 18189:

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

comment:30 Changed 17 months ago by 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.

comment:31 Changed 17 months ago by 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.

Note: See TracTickets for help on using tickets.