Page MenuHomeWildfire Games

Choose AI behavior in gamesetup
ClosedPublic

Authored by mimo on Dec 19 2017, 7:52 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Owners Package(Owns No Changed Paths)
Restricted Owners Package(Owns No Changed Paths)
Commits
rP20671: Choose AI behavior in gamesetup
Summary

Revival of D746 patch from sandarac, commited in rP20646 and reverted in rP20654

I've been through the patch, and here is a modified/fixed version. The main changes are:

  • random is now a behavior which is sent to the ai, and the randomization is done there instead of inside gamesetup
  • behavior choice disabled when sandbox is chosen as not applicable
  • i didn't like the name "generalist" which does not match with the two others. Replaced by "balanced", but any better suggestion welcomed
  • to keep the ai behavior consistent, its two internal quantities (aggressive and defensive) are made nearly anticorrelated, with some jitter
  • the additions in attackManager have been removed. The one on Rushes was useless, while the one on Attacks was succeptible to fool its strategy. It will have to be tuned later.
  • typos fixed in the options.json
Test Plan

Test this modified version

Event Timeline

mimo created this revision.Dec 19 2017, 7:52 PM
Owners added subscribers: Restricted Owners Package, Restricted Owners Package.Dec 19 2017, 7:52 PM
Vulcan added a subscriber: Vulcan.Dec 19 2017, 8:44 PM

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Executing section Default...
Executing section Source...
Executing section JS...
elexis added a subscriber: elexis.Dec 19 2017, 9:35 PM

Thanks for the incorporation.
Still eager to see different AI personalities, even if it will remain forever in this simple definition.
No defects read. Comments just posted for completeness and recorded verification.
Didn't test.
So the patch should be correct (unless there is a simple handcrafting issue that can be fixed by anyone) and complete.

binaries/data/mods/public/gui/aiconfig/aiconfig.js
51

/**\n

59

<logic>
So the behavior selection depends on the difficulty setting. If we eventually have a manifesto JSON file in simulation/ai/ that could be formalized there and then we could remove the magic strings here. Works well until then.
</logic>
<linting>
Having the same name in XML and JS makes things a bit easier globally (let aiBehavior = Engine.GetGUIObjectByName("aiBehavior"); ).
Also if it's the first variable then we can avoid one getter (performance rarely matters in most GUI context).
</linting>

binaries/data/mods/public/gui/common/gamedescription.js
117

<sidequest>
The string %(AIdifficulty)s %(AIbehavior)s %(AIname)s occurs in the gamesetup and this file. The same string could be useful in the diplomacy dialog and summary screen as a tooltip to indicate which opponent had which behavior and difficulty. So the patch in D1145 proposes to make that one %(AIdescription)s filled by translateAISettings.
Can be committed as separately however (even without merge conflicts if I see correctly).
</sidequest>

binaries/data/mods/public/gui/common/settings.js
152

<naming>
I wasn't able to find a more catchy name in both reviews. Renames and behavior changes are cheap, so certainly works until someone invents an improvement.
</naming>

binaries/data/mods/public/simulation/ai/petra/attackManager.js
43

That's indeed much better than the attackManager code rP20646, because it reduces the set of magic numbers instead of increasing it to gain the same feature.

binaries/data/mods/public/simulation/ai/petra/config.js
6

<linting>one copy should be sufficient within those few lines</linting>
<logic>Sounds like trouble. Do you have some additional information on it, function names, tests? We (read me if you don't want) should write a ticket so we can avoid the trap.</logic>

101

Not too clear what it's used for and what it shouldn't be used for. How about something with "rushProbability" and keeping it separate from other behavior properties?

147

antiproportional, typo

148

Related to that other comment: The seed generator is confirmed to be initialized now?
(For the sake of complete code comments, about linting, there was this function that would reduce the repetition while increasing the number of characters)

Is the sqrt+square done to fix the sign or a design choice? (Probability distribution of variation is not "linear" (forgot the correct term), not a defect however.)

152

infrastructure:

Envisioning JSON templates of AI behaviors, the constants should be grouped by behavior, not the behavior a property of the trait, i.e.
balanced.json
{

"Title":  "Balanced",
 "Description": "Not too aggressive, but not too defensive either."
 "Difficulties": [1, 5],
  "Aggressiveness": [0.37, 0.63],
  "Cooperativeness": [0, 1]

}

A nice benefit will be being able to show these settings in the GUI too, or possibly reading them in rmgen if there was some use case.

So keyword "later" (but it's possible to reduce the amount of later refactoring in advance in these two lines)
binaries/data/mods/public/simulation/helpers/InitGame.js
54

<gamesetups>
As mentioned in D746, fallbacks and defaults eventually ought to be moved to setup codes of games.
</gamesetups>

source/simulation2/components/CCmpAIManager.cpp
775

<encoding>
Someone mentioned long ago that we can use a null terminated string or something. Not important and should be done separately if wanted, at least for the AI difficulty if not in many more equal places.
</encoding>

mimo added inline comments.Dec 19 2017, 10:10 PM
binaries/data/mods/public/simulation/ai/petra/config.js
6

yep, i just forgot to remove one.
I just found it when testing the patch with the definition of behavior in the config function instead of the setConfig function. Maybe not dramatic, i may have time to investigate it when this patch is done: i supposed that in helpers/InitGame.js the ai is created with the AddPlayer, while the seed is initialized just after. Moving the seed initialization before adding players may solve it.

101

I think it may be useful for other purposes when we differentiate more the behaviors (either in aggressivity or defense). The goal was to be able to separate the specific behavior from gamesetup:
the aggressive one has always a value of personality.aggressive > personalityCut.strong and the defensive one has always personality.aggressive < personalityCut.weak, although personality.aggressive and personality.defensive are continuous numbers.

148

ok, i will switch to math.square
The use of square is mainly aesthetic :) if you would draw personality.aggressive vs personality.defensive it would have a ellipse shape, while otherwise it would be a diamond shape. (But the difference has nearly no impact on the ai behavior).

binaries/data/mods/public/simulation/helpers/InitGame.js
54

ok i will remove it. The main reason was to be able to reload my old replays with several AIs for test purposes.

elexis added inline comments.Dec 19 2017, 11:01 PM
binaries/data/mods/public/simulation/ai/petra/config.js
6

Correct.

The SetSeed call must be moved 30 lines above.

InitGame of InitGame.js should first call cmpAIManager.SetRNGSeed before the cmpAIManager.AddPlayer calls the Config function in config.js. (The cmpAIManager.RunGamestateInit call in InitGame will calls setConfig)

(rP15973)

101

Perhaps one can leave a short comment for the posterity

binaries/data/mods/public/simulation/helpers/InitGame.js
54

gamesetup.js should be guaranteed to always specify values, so for that the fallback isn't needed.
The autostart in GameSetup.cpp also specifies a custom fallback.
But I would assume the default is needed for atlas (especially since the diff doesn't touch Atlas).

elexis added inline comments.Dec 20 2017, 10:35 AM
binaries/data/mods/public/simulation/ai/petra/config.js
6

And I couldn't find a clean way to move it to CAIWorker init,
since the AIManager by design has no access to the ComponentManager and since we likely don't want to call it from CSimulation2::InitGame.

So it's still better to move that one call 30 lines above in a separate commit than adding a warning here.

(If we don't want to play with different AI seeds on the same random map+simulation seed, the AISeed could be removed and use the Seed could be used following r18604.)

(Besides, the cmpPlayer hunk of helpers/InitGame.js ought to be at helpers/Player.js and called whenever LoadPlayerSettings is called. But it seems like the C++ components rely on the settings being initialized prior to that, so not easily achievable.)

binaries/data/mods/public/simulation/data/settings/player_defaults.json
10

If you prefer the "Random" type by default that would be legal here - in Sandaracs patch "Random" was not a custom type but a random of the defined types.

mimo added inline comments.Dec 20 2017, 6:23 PM
binaries/data/mods/public/simulation/ai/petra/config.js
6

ok to remove the warning as it seems that the fix for the seed is easy enough, and will be done soon.

it is better to keep seeds separated if somebody want to have several ai simulations run on a same map in order to collect stats on the ais

binaries/data/mods/public/simulation/data/settings/player_defaults.json
10

ok will change it

binaries/data/mods/public/simulation/helpers/InitGame.js
54

then i will keep it, at least for the time being, if that's needed to be run inside atlas

mimo updated this revision to Diff 4830.Dec 20 2017, 6:35 PM

updated patch

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/ai/petra/config.js
| 115| »   this.personalityCut·=·{·"weak":·0.3,·"medium":·0.5,·"strong":·0.7·};
|    | [NORMAL] JSHintBear:
|    | Expected an assignment or function call and instead saw an expression.

Code looks ok to me. Not a verification. Only tested one game. We worked out where further improvements are applicable. Thanks for the addition.

binaries/data/mods/public/gui/aiconfig/aiconfig.js
59

linting thing could be done when committing.
logic thing when going for json behavior templates.

binaries/data/mods/public/gui/aiconfig/aiconfig.xml
32

unused argument

41

XML linting thing:

<object properties... />

binaries/data/mods/public/simulation/ai/petra/config.js
6

is easy enough, and will be done soon.

I guess it ought to be tested better than starting one game.

Index: binaries/data/mods/public/simulation/helpers/InitGame.js
===================================================================
--- binaries/data/mods/public/simulation/helpers/InitGame.js	(revision 20670)
+++ binaries/data/mods/public/simulation/helpers/InitGame.js	(working copy)
@@ -44,6 +44,7 @@
 	// Sandbox, Very Easy, Easy, Medium, Hard, Very Hard
 	let rate = [ 0.50, 0.64, 0.80, 1.00, 1.25, 1.56 ];
 	let cmpAIManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_AIManager);
+	cmpAIManager.SetRNGSeed(settings.AISeed || 0);
 	for (let i = 0; i < settings.PlayerData.length; ++i)
 	{
 		let cmpPlayer = QueryPlayerIDInterface(i);
@@ -72,8 +73,6 @@
 	// Map or player data (handicap...) dependent initialisations of components (i.e. garrisoned units)
 	Engine.BroadcastMessage(MT_InitGame, {});
 
-	let seed = settings.AISeed ? settings.AISeed : 0;
-	cmpAIManager.SetRNGSeed(seed);
 	cmpAIManager.TryLoadSharedComponent();
 	cmpAIManager.RunGamestateInit();
 }

run on a same map in order to collect stats on the ais

Good use case.

152

I don't know if the constants were important, but I don't mind not having them until there is good reason to having them.

This revision was automatically updated to reflect the committed changes.
mimo added a comment.Dec 20 2017, 11:03 PM

Thanks elexis for the comments.