ocraft / ocraft-s2client

StarCraft II Client - Java library supported on Windows, Linux and Mac designed for building scripted bots and research using the SC2API.
MIT License
55 stars 15 forks source link

onGameEnd is not called in multiplayer configuration #15

Closed fheck closed 5 years ago

fheck commented 5 years ago

If we start a game with two bot players in stepped mode, the com.github.ocraft.s2client.bot.ClientEvents#onGameEnd method is called for neither of the bots, specifically com/github/ocraft/s2client/bot/S2Coordinator.java:747 is not triggered. This prevents our bots from reacting to the final game state (if we do not work around this by calling the interface method directly for each bot before ending the game).

ocraft commented 5 years ago

I've checked this case and it worked for me, below there is a sample code I've used for this test:

TestBot bot1 = new TestBot();
TestBot bot2 = new TestBot();

S2Coordinator s2Coordinator = S2Coordinator.setup()
        .loadSettings(new String[]{"-m", "Lava Flow"})
        .setMultithreaded(true)
        .setParticipants(
                S2Coordinator.createParticipant(Race.PROTOSS, bot1, "Adam"),
                S2Coordinator.createParticipant(Race.ZERG, bot2, "Eve")
        )
        .launchStarcraft()
        .startGame();

while (s2Coordinator.update()) {}

s2Coordinator.quit();
fheck commented 5 years ago

Thank you for checking!

First of all, strangely, when running in single player setup, it is called. The setup code for both scenarios is exactly the same, disregarding the createParticipant() vs createComputer() call. This lead me to compare all options we set, specifically we do/did not set .setMultithreaded(true). With this option set, the method is in fact called.

This poses the question, is this intended behavior? Because we would like to keep it single-threaded, since we use this as a small project for programming newbies and don't want to require to much, especially in regard to thread safety.

ocraft commented 5 years ago

The api is internally multithreaded even without that option. Multithreaded==true means that the code of both bot are run in parallel but it shouldn't require any thread-safety related action because each bot is executed in own thread and only communicate with the api, which deal with thread safety. Therefore it shouldn't require any additional effort from the bot programmer.

That said onGameEnd should be called also for multithreaded==false, it's a bug to resolve.

fheck commented 5 years ago

I haven't looked to deep into the code on how bots are executed, I only relied on the documentation which states

If set to true make sure your bots are thread-safe if they reach into shared code

So, to be on the safe side, initially I did not set it. Since that is the cleaner option, for the time being, I set it to true.

ocraft commented 5 years ago

Ok, I assumed that every bot author writes own code and the case is to make them play each other. If there is some shared code (except the api itself) then the documentation is valid, you must make sure that access to this shared code is thread safe.

fheck commented 5 years ago

Yeah, normally that should be the case, but sometimes there is helper stuff. I haven't found anything that breaks yet, so like I said changing it is fine for now.

Thanks again for your help.

ocraft commented 5 years ago

Fixed in ver. 0.3.10