Return-To-The-Roots / s25client

Return To The Roots (Settlers II(R) Clone)
http://www.rttr.info
GNU General Public License v2.0
477 stars 75 forks source link

Add ai-battle cli #1652

Closed wichern closed 3 months ago

wichern commented 6 months ago

Adds an optional binary that runs headless (no UI) AI battles. This can be useful when creating training data or testing new AIs.

image

I'm not sure if this is placed correctly or whether it should even be part of the master branch. In any case, I would like your feedback on this tool. It aims to reuse as much of s25main as possible in order to create accurate replays.

wichern commented 4 months ago

Few remarks to fix compilation

This will likely glitch on Windows though:

   `printf("\x1b[%dA", 8 + world_.GetNumPlayers()); // Move cursor back up`

You are right.

It seems that I could fix this by calling SetConsoleMode. https://learn.microsoft.com/en-us/windows/console/setconsolemode

I will have to set up a windows build machine to test it.

Flamefire commented 4 months ago

It seems that I could fix this by calling SetConsoleMode. https://learn.microsoft.com/en-us/windows/console/setconsolemode

There is a condition that ENABLE_PROCESSED_OUTPUT only works for "WriteFile or WriteConsole". I suggest you use the C++ streams everywhere but PrintState and for that implement a wrapper instead of printf calling vsprintf and passing the result to either std::puts or WriteConsole.

Something like

#ifdef WIN32
HANDLE setupStdOut()
{
  HANDLE h = GetStdHandle(STD_OUTPUT_HANDLE);
  SetConsoleMode(h, ENABLE_PROCESSED_OUTPUT);
  return h;
}
#endif

void printConsole(const char* fmt, ... )
{
   ...
   vsprintf(...)
#ifdef WIN32
  static auto h = setupStdOut();
  WriteConsole(h, formattedString);
#else
  std::puts(formattedString();
#endif
}

Core ideas:

BTW: #include <cstdio> is missing although it seemingly gets pulled in by an other header

wichern commented 3 months ago

I marked a few comments as "Optional", you can skip those if you want. If you fix the others (and CI passes) we can merge this.

Thank's a lot. I will try to apply the other suggestions as well.

Flamefire commented 3 months ago

Once you are done and have squashed the changes the replay test will still fail (on appveyor) This will be fixed by https://github.com/Return-To-The-Roots/s25client/pull/1676 If that isn't merged yet when you are ready (and CI here succeeds except that one test) you could merge that branch into yours to make CI pass here so we can merge it.

Flamefire commented 3 months ago

I rebased your branch onto current master and combined (squashed) some commits. I also fixed the (now) failing test on appveyor and merged the replay-fix branch again (instead of the previous merge)

As this was the only failing test and the diff of the rebased branch and yours is just the appveyor fix it should pass now. So I'll enable auto-merge. Thanks a lot!