HomeWildfire Games

Add a -autostart-nonvisual option. Patch by sacha_vrand. Fixes #4577.
AuditedrP19645

Description

Add a -autostart-nonvisual option. Patch by sacha_vrand. Fixes #4577.

This allows automated testing of AIs without any GUI or sound (similar to non-visual replays).

Differential Revision: https://code.wildfiregames.com/D379

Event Timeline

Thanks for the patch and review @sacha_vrand and @leper!
Could have used this very well already for D145, D156, D229 and D204. In each of these occurances I had non-visually replayed an empty extended replay file to see how my trigger script would behave. The optional and disabled by default debug messages are still in there, so that these unit-like tests can now be run from command-line to see that the scripts still work and spawn intended number of enemies (rise the water at a reasonable rate):

DEBUG [4]  Spawning 4 attackers
DEBUG [4]  Siege Ratio: 21%
DEBUG [4]  Spawning:({siege:1, champions:3})
DEBUG [4]  Templates: [{template:"units/gaul_mechanical_siege_ram", count:1}, {template:"units/gaul_champion_cavalry", count:1}, {template:"units/gaul_champion_fanatic", count:2}, {template:"units/gaul_champion_infantry", count:0}]
DEBUG [7]  Spawning 6 attackers
DEBUG [7]  Siege Ratio: 40%
DEBUG [7]  Spawning:({siege:2, champions:4})
DEBUG [7]  Templates: [{template:"units/iber_mechanical_siege_ram", count:2}, {template:"units/iber_champion_cavalry", count:2}, {template:"units/iber_champion_infantry", count:2}]
DEBUG [10]  Spawning 6 attackers
DEBUG [10]  Siege Ratio: 34%
DEBUG [10]  Spawning:({siege:2, champions:4})
DEBUG [10]  Templates: [{template:"units/rome_mechanical_siege_ballista_packed", count:1}, {template:"units/rome_mechanical_siege_ram", count:1}, {template:"units/rome_mechanical_siege_scorpio_packed", count:0}, {template:"units/rome_centurio_imperial", count:1}, {template:"units/rome_champion_cavalry", count:1}, {template:"units/rome_champion_infantry", count:1}, {template:"units/rome_legionnaire_imperial", count:1}, {template:"units/rome_legionnaire_marian", count:0}]

https://trac.wildfiregames.com/wiki/PetraBot#Batchsimulatinggames (maybe others?) should be updated I guess.

As an example of what we can do with that, here is a graph showing the win percentage of the first player on the map 'skirmishes/Acropolis Bay (2)' (doesn't seem well balanced). 250 games in conquest with Petra against Petra and a seed from 0 to 249. What do you guys think ?
Graph's link

Presumably AI devs would be very happy to find out how such graphs can be produced.
I heard you created this patch as a student project? How did it go, maybe you want to share some material (slideshow or something)?

/ps/trunk/binaries/data/mods/public/maps/scripts/NonVisualTrigger.js
1

Always wondered why victory condition scripts (and now this one) are in the map directory while this isn't more related to maps, like almost all of the other mapsettings. (I've talked to sanderd17 once about this, but I'm not really convinced). Notice our map trigger scripts aren't in maps/scripts/ either.
Maybe that should sometime become public/scripts/victory_condition/.
Is it actually correct that this file is part of the public mod?

27

good (until we have SM45, though even then it might be good to keep the scope so that people don't convert that let to a var assuming it wouldn't bug)

/ps/trunk/binaries/data/mods/public/simulation/components/StatisticsTracker.js
221
In D379?id=1822#inline-9262, @leper wrote:

@elexis opinion on this function comment?

"indentation"?
Guess the information could have been packed into a single sentence, but nothing wrong with this.

/ps/trunk/binaries/system/readme.txt
16

since this isn't a simulation setting, maybe it fit's better as the last item of the list before Multiplayer or as the first one

Itms awarded a token.May 25 2017, 6:28 PM
elexis raised a concern with this commit.May 28 2017, 6:26 PM
elexis added inline comments.
/ps/trunk/binaries/data/mods/public/maps/scripts/NonVisualTrigger.js
5

whitespace

9

whitespace

This commit now has outstanding concerns.May 28 2017, 6:26 PM
elexis accepted this commit.May 28 2017, 7:43 PM

Fixed by r19683.
It's not crazy to remove trailing whitespace, but I agree that it's not worth raising a concern for whitespace IMO and
what pushed me to raise a concern is not the coding convention but the conern raised in rP19667

All concerns with this commit have now been addressed.May 28 2017, 7:43 PM
mimo added a subscriber: mimo.May 28 2017, 7:56 PM

Fixed by r19683.
It's not crazy to remove trailing whitespace, but I agree that it's not worth raising a concern for whitespace IMO and
what pushed me to raise a concern is not the coding convention but the conern raised in rP19667

I agree fixing the whitespace is justified, my worry is rather to the fact that we put concern for that, which require more work than fixing it directly.
In addition, the main concern should rather have been on the fact that this file use blank rather than tabs for indentation (i just noticed it when modifying the file)
And rP19667 is a bit different as we have there two competing coding conventions, and so the best way to fix it could be debatable, while it was not when only removing two trailing spaces, but i'm fine with your rP19684 :)

elexis added a comment.Feb 2 2018, 5:33 AM

Hi @sacha_vrand, in case you read this and have some more interest in playing with this code, gentz has reported a bug here: #5016