Page MenuHomeWildfire Games

Choose AI behavior in gamesetup
ClosedPublic

Authored by Sandarac on Jul 14 2017, 10:43 AM.
Tags
None
Subscribers
Restricted Owners Package
Restricted Owners Package
Restricted Owners Package
Restricted Owners Package
Tokens
"100" token, awarded by Itms.

Details

Summary

Adds a new drop-down in the AI configuration menu (for A23).

This diff mainly handles the GUI/AIManager changes; in follow-up diffs, more changes to Petra's code can be done based on the selected behavior.

Choices (open to suggestions), and intended behavior:
Aggressive - will rush frequently, not focus on building defenses, etc.
Generalist - intended to behave as Petra does now (i.e. a mix).
Defensive - will not attack often, will keep standing armies, etc.
Random - a random behavior from the above is chosen.

Petra already has a "personality" defined in config.js, which is currently given random values based slightly on difficulty. I propose to keep this personality Object, and do the following:

  • Use Config.behavior for "larger scale" decisions (like when to create attack plans, and which player to target), and initialize the Config.personality values based on Config.Behavior.
  • Use Config.personality for "smaller scale" decisions (i.e. in most of the cases it is already used, for things like number of sentry towers), as a subset of the Config.Behavior.

I didn't add support for it in Autostart() in GameSetup.cpp yet, I think that should be done separately, maybe after fixing the duplication there.

(Adding elexis as a reviewer for the GUI/gamesetup stuff, and for comments/concerns.)

Test Plan

Check that the menu works as intended in-game, see Petra's behavior when set to "Defensive".

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Sandarac created this revision.Jul 14 2017, 10:43 AM
Owners added subscribers: Restricted Owners Package, Restricted Owners Package, Restricted Owners Package, Restricted Owners Package.Jul 14 2017, 10:43 AM
Vulcan added a subscriber: Vulcan.Jul 14 2017, 12:00 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1747/ for more details.

Imarok added a subscriber: Imarok.Jul 14 2017, 12:34 PM
Imarok added inline comments.
binaries/data/mods/public/simulation/ai/petra/config.js
9 ↗(On Diff #2910)

this.behaviour = behaviour || "generalist";?

Itms awarded a token.Jul 14 2017, 2:05 PM
elexis added inline comments.Jul 14 2017, 3:33 PM
binaries/data/mods/public/gui/aiconfig/aiconfig.js
39 ↗(On Diff #2910)

(Would be nice to do the same abstraction as in D322 or the options. page here if the code is already refactored, but sounds like overkill for a page with 3 options that is almost never extended)

binaries/data/mods/public/gui/aiconfig/aiconfig.xml
42 ↗(On Diff #2910)

<foo/>

binaries/data/mods/public/gui/common/settings.js
142 ↗(On Diff #2910)

Random shouldn't be in here if that means one of the other behaviors is chosen.

367 ↗(On Diff #2910)

Not sure if Default is the right word. Above code uses Unknown, which sounds like it should be removed rather (or a comment added explaining which edge case is covered. Ancient replays? Replays/Lobby games of mods?).

binaries/data/mods/public/simulation/data/settings/player_defaults.json
10 ↗(On Diff #2910)

These defaults are overwritten for random maps in initDefaults() of gamesetup.js. (Also I'm still missing the Team property here, refs D426.)

Since Atlas copies these properties to the XML file, so they must appear in the (not for all properties implemented) atlas dropdowns.
At least for the civ, "random" won't work out on scenario maps I think.
So we shouldn't really use "random" for any of the simulation settings if there is no simulation setting that is named random.

source/ps/GameSetup/GameSetup.cpp
1428 ↗(On Diff #2910)

Why converting a std::string to a std::string? The one from the AI name also seems unneeded.

leper added inline comments.
source/ps/GameSetup/GameSetup.cpp
1428 ↗(On Diff #2910)

Since we are being slightly pedantic I'll turn this to 11 as "AIBehavior" is a string literal, not std::string.

(Not that I disagree with the rest.)

elexis accepted this revision.Dec 12 2017, 5:34 PM

Shaping different AI behaviors can finally be selected by players and tested by AI developers. The feature was request since ages.

Review:

  • We should have a way to find out if the AI behavior in running matches.
  • Could think about having multiple behavior options, like a control for the cooperativeness and one control for the aggressivity. But it seems few clarly shaped characters would be nicer than using sliders to fine-tune them (useless complexity).
  • Diff misses an atlas implementation like every other gamesetting does. Can be added separately and maybe it will be even easier one day to generate atlas dropdowns and checkboxes dynamically, like we do in gamesetup.js.
  • Bit inconsistent to allow randomization of behavior but not the one of difficulties. But rather that than the other way around.
  • Eventually the AI difficulty and behavior choices should remain in simulation/ai/petra/data.json rather than in the GUI

Test:
I ran an entire game with 2 AIs but I didn't notice the behavior difference. They were, well, fighting.
I have modified their aggr vs def to 0 and 1 and then let the do a 4v4 with 7 def and 1 aggr and I could clearly see that that one aggressive bot was the only troublemaker.

Thanks Sandarac!

binaries/data/mods/public/gui/aiconfig/aiconfig.js
16 ↗(On Diff #2910)

These loop+switch constructs only add unneeded cyclomatic complexity.
The switch should be moved out of the loop.

31 ↗(On Diff #2910)

|| 0 hack alert, hardcoded default or fallback. Looks like the latter and being unneeded.

binaries/data/mods/public/gui/common/settings.js
142 ↗(On Diff #2910)

and the other three choices should be defined in the bot directory, not hardcoded in the gui eventually.
(In the same diff that moves the AI difficulty).
Also generalist should be the default first.

153 ↗(On Diff #2910)

Aggressive and Defensive are properties, but Generalist is a noun.

Might yield some translation issues, but Very Hard Generalist Petra AI vs Very Hard Aggressive Petra AI might still work.
(Using the same order as in the gamedescriptions.js string)

binaries/data/mods/public/gui/gamesetup/gamesetup.js
1818 ↗(On Diff #2910)

first condition redundant with the continue condition below.

The pickRandom calls in launchGame should eventually be moved to a separate function.

binaries/data/mods/public/simulation/ai/petra/config.js
9 ↗(On Diff #2910)

That.

Could use a comment, possibly even validation.

In fact start settings should be considered broken if they don't define all values.
Fallbacks and randomization should only be done in one layer of the application (i.e. gamesetup.js, Gamesetup.cpp and atlas, but not additionally in the simulation). However I'm ok with adding it for symmetry now.

129 ↗(On Diff #2910)

Good idea splitting the numbers like that.
I wonder why there are two properties: aggressiveness and defensiveness. (Would have expected those to be antonyms).

cooperativeness could be less with very aggressive and very defensive bots..

binaries/data/mods/public/simulation/data/settings/player_defaults.json
10 ↗(On Diff #2910)

seems odd to specify AI settings if there is no AI. (not adding a regression however)

source/ps/GameSetup/GameSetup.cpp
1428 ↗(On Diff #2910)

Actually the function requires a string and can't handle a literal.

This revision is now accepted and ready to land.Dec 12 2017, 5:34 PM
This revision was automatically updated to reflect the committed changes.