HomeWildfire Games

Implement combine victory conditions
AuditedrP21474

Description

Implement combine victory conditions

Excluding the changes to scenario and skirmisch maps
Transform gameType string to victoryCondition array in load/replaymenu
Adapt the gamesetup to use checkboxes for every victory condition and an array for storing them
Allow multiple queries in conquestCommon
Remove conquest from regicide, wonder and capture the relic
Move the endless gamedescription from settings to gamedescription
Fixing wrong tabulation from rP21421

This commit will break all scenario and skirmisch maps, their "Gametype" string needs to be transformed in a "VictoryCondition" array as is done in the tutorial map (counting endless as an empty array). This counts for mods too!
Old svn replays and savegame will throw warnings/errors as they are incompatible after this commit. So svn users will need to delete all those.

Comments on ai and autostart games By: mimo
Comments on Atlas By: Vladislav
Reviewed By: elexis
Differential Revision: https://code.wildfiregames.com/D1240
fixes: #4014

Event Timeline

Imarok added a subscriber: Imarok.Mar 13 2018, 4:01 PM

Untick "Conquest" → tick "Conquest Units" → tick "Conquest" again: "Conquest Units" checkbox is disabled, but still ticked; it is also shown in the descriptions below the tabs.
Is this behaviour intended? If so why?
(same goes for "Conquest Structures")

bb added inline comments.Mar 13 2018, 4:10 PM
/ps/trunk/binaries/data/mods/public/gui/gamesetup/gamesetup.js
824–825

yup json naming artifact

Also it seems the "Game Type" tab in gamesetup can't handle all enabled victory conditions (because of the additional relic count and wonder/relic duration tabs) as it exceeds the window width in minres and throws ERROR: CRenderer::EndFrame: GL errors GL_INVALID_VALUE (0501) occurred.

bb added a comment.EditedMar 13 2018, 4:20 PM

Also it seems the "Game Type" tab in gamesetup can't handle all enabled victory conditions (because of the additional relic count and wonder/relic duration tabs) as it exceeds the window width in minres and throws ERROR: CRenderer::EndFrame: GL errors GL_INVALID_VALUE (0501) occurred.

Well that is not this commit, rP20945 would be closer. But however I doubt you were on minres, since there the minres is explicitely set as the minimum space required for two columns, so I think you were even below minres.

(Also testing it here reveals on min res everything is ok, but below minres the error comes up)

Also it seems the "Game Type" tab in gamesetup can't handle all enabled victory conditions (because of the additional relic count and wonder/relic duration tabs) as it exceeds the window width in minres and throws ERROR: CRenderer::EndFrame: GL errors GL_INVALID_VALUE (0501) occurred.

Is it reproducible? Because it seems this error is more about the window size.

bb added a comment.Mar 13 2018, 4:57 PM

Also it seems the "Game Type" tab in gamesetup can't handle all enabled victory conditions (because of the additional relic count and wonder/relic duration tabs) as it exceeds the window width in minres and throws ERROR: CRenderer::EndFrame: GL errors GL_INVALID_VALUE (0501) occurred.

Is it reproducible? Because it seems this error is more about the window size.

entirely: make your window so small at least 3 columns are needed for a gamesetup tab and there is not enough width for them, and open gamesetup

In rP21474#29467, @bb wrote:

Also it seems the "Game Type" tab in gamesetup can't handle all enabled victory conditions (because of the additional relic count and wonder/relic duration tabs) as it exceeds the window width in minres and throws ERROR: CRenderer::EndFrame: GL errors GL_INVALID_VALUE (0501) occurred.

Well that is not this commit, rP20945 would be closer. But however I doubt you were on minres, since there the minres is explicitely set as the minimum space required for two columns, so I think you were even below minres.

(Also testing it here reveals on min res everything is ok, but below minres the error comes up)

Just that we talk about the same: this is minres:

(It's 1024x786 and not 1366x786 (what your laptop has))

And to the reproducability: It only occurs in multiplayer, so to reproduce:
start 0ad in windowed mode (the window size should then be minres) →
host game → game type → enable all VCs → back to mainmenu →
host game → game type → disable all VCs → back to mainmenu→
host game → game type → enable all VCs

somewhere in this process the error occurs.

bb added a comment.Mar 13 2018, 5:18 PM

Just that we talk about the same: this is minres: (It's 1024x786 and not 1366x786 (what your laptop has))

We are, using windowed mode

And to the reproducability: It only occurs in multiplayer, so to reproduce:

Also get it in singleplayer with the above procedure

start 0ad in windowed mode (the window size should then be minres) →
host game → game type → enable all VCs → back to mainmenu →
host game → game type → disable all VCs → back to mainmenu→
host game → game type → enable all VCs

somewhere in this process the error occurs.

Actually don't get the error here that way (could be that my height is already <minres)

In rP21474#29485, @bb wrote:

Actually don't get the error here that way (could be that my height is already <minres)

strange

mimo raised a concern with this commit.Mar 15 2018, 7:00 PM
mimo added a subscriber: mimo.

I've not tried to check if this patch is really the faulty one, but i guess so. In autostart, the "-autostart-victory="capture_the_relic" does not anymore spawn any relics.

This commit now has outstanding concerns.Mar 15 2018, 7:00 PM
In rP21474#29602, @mimo wrote:

I've not tried to check if this patch is really the faulty one, but i guess so. In autostart, the "-autostart-victory="capture_the_relic" does not anymore spawn any relics.

certainly a bug, but not this commit, happens even in A22 already. Also in Atlas the same is observed (in both current svn and A22)
The problem is that no relicCount argument is passed and thus the creating loop doesn't create anything
see D1393

bb requested verification of this commit.Mar 16 2018, 12:48 AM
This commit now requires verification by auditors.Mar 16 2018, 12:48 AM
mimo accepted this commit.Mar 16 2018, 7:17 PM

Sorry, i thought it had worked. But i must have been confused by some non-autostart games.
and thanks for D1393

All concerns with this commit have now been addressed.Mar 16 2018, 7:17 PM
bb added a comment.Mar 16 2018, 9:02 PM

No problem, the important part is that it is fixed :)

elexis raised a concern with this commit.May 9 2018, 1:19 PM
elexis added a subscriber: elexis.

As you mentioned in rP21108 this added an endless option to the autostart.
It should be added to the GameSetup.cpp and README.txt description.
(I flag it as a concern so we don't forget to fix it in alpha 24.)
(The default victory condition should also be mentioned in both copies of the description, see above commit)

This commit now has outstanding concerns.May 9 2018, 1:19 PM
bb requested verification of this commit.Mar 16 2019, 3:21 PM
This commit now requires verification by auditors.Mar 16 2019, 3:21 PM
elexis accepted this commit.Mar 16 2019, 5:49 PM

Thanks bb!

All concerns with this commit have now been addressed.Mar 16 2019, 5:49 PM
elexis added inline comments.Nov 15 2019, 7:20 PM
/ps/trunk/binaries/data/mods/public/gui/gamesetup/gamesetup.js
2652

The join was correct, but the property change was wrong, since the users of that property were not changed; XmppClient.cpp, and the ones that display the value (those are none either which is an independent bug, from rP14098, refs rP19642)

Silier raised a concern with this commit.May 10 2021, 2:48 PM
Silier added a subscriber: Silier.
Silier added inline comments.
/ps/trunk/binaries/data/mods/public/maps/scripts/ConquestStructures.js
4

This looks like is broken from the day 1.
There is ConquestCritical class in structure(and maybe unit also) templates to mark entities required to be destroyed, but that class is not queried here

This commit now has outstanding concerns.May 10 2021, 2:49 PM
bb added inline comments.May 10 2021, 3:01 PM
/ps/trunk/binaries/data/mods/public/maps/scripts/ConquestStructures.js
4

If you think the class is wrong, this is the wrong commit to contest: look at L3 and see the class is the same.
Iirc the victory condition is there to be defeated if all structures are destroyed, not all conquest critical structures. If you want to change this, I will refer you to a commit implementing this VC and look for any discussion there.

bb requested verification of this commit.May 10 2021, 3:01 PM
This commit now requires verification by auditors.May 10 2021, 3:02 PM
Silier resigned from this commit.May 10 2021, 4:02 PM
All concerns with this commit have now been addressed.May 10 2021, 4:02 PM