Page MenuHomeWildfire Games

Add the ability to output AI commands to commands.txt
Needs RevisionPublic

Authored by Sandarac on Feb 21 2017, 3:51 AM.

Details

Reviewers
Itms
Trac Tickets
#3392
Summary

With the help of @elexis, this patch was created that allows AI commands to be written to the commands.txt when using a command-line option. It is useful for AI development.

Test Plan

When Pyrogenesis is run with the "-aicommands" option, the commands for all AI players will also be written to the commands.txt.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 562
Build 889: Vulcan BuildJenkins

Event Timeline

Sandarac created this revision.Feb 21 2017, 3:51 AM
Vulcan added a subscriber: Vulcan.Feb 21 2017, 4:35 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

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

Sandarac updated this revision to Diff 568.Feb 21 2017, 5:39 AM

Access g_args through "ps/GameSetup/CmdLineArgs.h".

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

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

Itms requested changes to this revision.Feb 21 2017, 3:09 PM
Itms added a subscriber: Itms.

Thanks a lot for this patch, which will be very useful, not only for the development of AIs, but also for automated tests of the game ?

Unfortunately the thing you propose is a hack and the AI manager component shouldn't access the turn manager like this. I believe a more sensible thing to do would be to work on the CommandQueue component so that local commands are sent to the command logger when the command-line option is given.

This revision now requires changes to proceed.Feb 21 2017, 3:09 PM
In D153#5806, @Itms wrote:

I believe a more sensible thing to do would be to work on the CommandQueue component so that local commands are sent to the command logger when the command-line option is given.

Using the command queue would mean that a number of commands would end up in there that the Petra bot didn't send, adding a range of false positives to the developer trying to debug the AI:
Search the code for ProcessCommand to find existing occurances:

  • ProductionQueue.js send a command whenever a unit spawns as determined by the rallypoint component (which is also the reason the follow-player thing focuses on spawned units)
  • GarrisonHolder.js sends a command via OrderWalkToRallyPoint each time upon PerformEject
  • Commands.js sends a repair command from TryConstructBuilding for each newly created foundation

And this list can grow arbitrarily depending on the most recent state of simulation features.
In particular trigger scripts could also send commands for AI players.

Furthermore when replaying the commands.txt, those commands are then added twice, once explicitly and once implicitly.

I guess this could be addressed by adding a new parameter "local": true or similar to that command and then ignoring the output of those commands,
or adding a new function doing the same.

In D153#5806, @Itms wrote:

Unfortunately the thing you propose is a hack and the AI manager component shouldn't access the turn manager like this.

Why? It is exactly what was ordered.
Why shouldn't the AI use the simulation directly instead of queueing the command through a global mechanism that is equal to all participants that comes before the simulation, like the turnmanager?

Independently of that discussion, notice that we can't replay said commands.txt without replacing AI="petra" with AI="" in the header.
That could be done automatically when writing the file, but then we don't know anymore which player was the AI.

mimo added a subscriber: mimo.Feb 21 2017, 7:30 PM
In D153#5819, @elexis wrote:

Independently of that discussion, notice that we can't replay said commands.txt without replacing AI="petra" with AI="" in the header.
That could be done automatically when writing the file, but then we don't know anymore which player was the AI.

I don't think that would work, because the simulation rely on some place on the result of isAI which would then be false. I think it would be safer that, when this option is set, the AIManager creates dummy AIs (which would do nothing), but the isAI should respond as when the option is not set.

In D153#5820, @mimo wrote:
In D153#5819, @elexis wrote:

Independently of that discussion, notice that we can't replay said commands.txt without replacing AI="petra" with AI="" in the header.
That could be done automatically when writing the file, but then we don't know anymore which player was the AI.

I don't think that would work, because the simulation rely on some place on the result of isAI which would then be false. I think it would be safer that, when this option is set, the AIManager creates dummy AIs (which would do nothing), but the isAI should respond as when the option is not set.

(When writing the file (replay logger), not changing things in the simulation.)

With regards to the duplicate commands, if such a command would be adding a GUI notification, that notification would be added twice, so they must be avoided.

Sandarac updated this revision to Diff 572.Feb 22 2017, 9:08 AM
Sandarac edited edge metadata.

Access the TurnManager through the CommandQueue component instead of in the AIManager, as the CommandQueue already directly accesses the TurnManager in PostNetworkCommand.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

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

Itms requested changes to this revision.Mar 8 2017, 12:27 PM

I took a closer look and you solution can be improved without too many efforts. Also it looks like the local queue code could be greatly improved very easily, so if you're up to the task I'd love to see it happen!

binaries/system/readme.txt
64

Since you are making all local commands logged, a better name for that would be -localcommands, but that would be obscure for the end user.

Alternatively, since apparently the AI is the only system that uses "local commands", we could rename the entire local commands thing to "AI commands". It would be far cleaner and easier to maintain IMO. Overall those local commands look like a hack that evolved into an important system: fix the access modifier of the local queue while you're at it ?

source/simulation2/components/CCmpCommandQueue.cpp
89

See comment in the other file below. As a rule of thumb, take a look at the code around: here PostNetworkCommand does a similar thing to what you want to achieve.

source/simulation2/system/TurnManager.h
189

Instead of making the class a friend, just add a new public method to the turn manager that adds a local command. That would also allow you to encapsulate the code you wrote for the command queue component.

This revision now requires changes to proceed.Mar 8 2017, 12:27 PM