Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ai-battle cli #1652

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

Conversation

wichern
Copy link
Contributor

@wichern wichern commented Mar 10, 2024

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.

Copy link
Member

@Flamefire Flamefire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few remarks to fix compilation

This will likely glitch on Windows though:

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

extras/ai-battle/main.cpp Outdated Show resolved Hide resolved
extras/ai-battle/HeadlessGame.cpp Outdated Show resolved Hide resolved
extras/ai-battle/HeadlessGame.cpp Outdated Show resolved Hide resolved
extras/ai-battle/main.cpp Show resolved Hide resolved
extras/ai-battle/main.cpp Show resolved Hide resolved
extras/ai-battle/HeadlessGame.cpp Outdated Show resolved Hide resolved
wichern and others added 8 commits June 5, 2024 21:16
Co-authored-by: Alexander Grund <Flamefire@users.noreply.github.com>
Co-authored-by: Alexander Grund <Flamefire@users.noreply.github.com>
Co-authored-by: Alexander Grund <Flamefire@users.noreply.github.com>
Co-authored-by: Alexander Grund <Flamefire@users.noreply.github.com>
Co-authored-by: Alexander Grund <Flamefire@users.noreply.github.com>
Co-authored-by: Alexander Grund <Flamefire@users.noreply.github.com>
@wichern
Copy link
Contributor Author

wichern commented Jun 5, 2024

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
Copy link
Member

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:

  • Use a local static variable to get the stdout handle and ensure the console mode is set exactly once
  • Keep the code until std::puts/WriteConsole the same to be able to spot mistakes in formatting on both systems

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

Copy link
Member

@Flamefire Flamefire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

extras/ai-battle/HeadlessGame.cpp Outdated Show resolved Hide resolved
extras/ai-battle/HeadlessGame.cpp Outdated Show resolved Hide resolved
extras/ai-battle/HeadlessGame.cpp Outdated Show resolved Hide resolved
extras/ai-battle/HeadlessGame.h Outdated Show resolved Hide resolved
extras/ai-battle/HeadlessGame.cpp Outdated Show resolved Hide resolved
extras/ai-battle/main.cpp Outdated Show resolved Hide resolved
extras/ai-battle/main.cpp Outdated Show resolved Hide resolved
extras/ai-battle/main.cpp Outdated Show resolved Hide resolved
extras/ai-battle/main.cpp Outdated Show resolved Hide resolved
extras/ai-battle/main.cpp Outdated Show resolved Hide resolved
wichern and others added 4 commits June 16, 2024 21:12
Co-authored-by: Alexander Grund <Flamefire@users.noreply.github.com>
Co-authored-by: Alexander Grund <Flamefire@users.noreply.github.com>
Co-authored-by: Alexander Grund <Flamefire@users.noreply.github.com>
Co-authored-by: Alexander Grund <Flamefire@users.noreply.github.com>
@wichern
Copy link
Contributor Author

wichern commented Jun 16, 2024

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.

Copy link
Member

@Flamefire Flamefire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, great!
There are 2 constness issues (Rule is: Use const always unless you really need to modify the variable/parameter)

CI fails due to 1 conversion issue and an unrelated issue with test replays on master which will be by #1676

("objective", po::value<std::string>()->default_value("domination"),"domination(default)|conquer")
("replay", po::value(&replay_path),"Filename to write replay to (optional)")
("save", po::value(&savegame_path),"Filename to write savegame to (optional)")
("random_init", po::value<unsigned>()->default_value(std::chrono::high_resolution_clock::now().time_since_epoch().count()),"Seed value for the random number generator (optional)")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compiler complains about signed->unsigned.

  1. use an optional and value_or(std::chrono...)
  2. Or: const auto defaultSeed = static_cast<unsigned>(std::chrono::high_resolution_clock::now().time_since_epoch().count()) next to replay/savegame_path and use it here

extras/ai-battle/main.cpp Outdated Show resolved Hide resolved
extras/ai-battle/main.cpp Outdated Show resolved Hide resolved
extras/ai-battle/main.cpp Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants