From c3a0c731d66e96c47d624c30b5fd928e225fe4d5 Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Sat, 28 Jun 2025 22:51:34 +0400 Subject: [PATCH] AI, tests: added stability tests to make sure AI simulations can process errors and freezes (part of #13638, #13766); --- .../src/mage/player/ai/ComputerPlayer6.java | 15 +- .../src/mage/player/ai/ComputerPlayer7.java | 3 +- .../java/mage/player/ai/ComputerPlayer.java | 6 +- .../mage/player/ai/ComputerPlayerMCTS.java | 2 +- .../AI/basic/SimulationStabilityAITest.java | 237 ++++++++++++++++++ .../java/org/mage/test/player/TestPlayer.java | 23 +- .../main/java/mage/abilities/AbilityImpl.java | 2 +- Mage/src/main/java/mage/game/GameImpl.java | 2 +- .../main/java/mage/game/combat/Combat.java | 3 +- Mage/src/main/java/mage/players/Player.java | 10 +- .../main/java/mage/players/PlayerImpl.java | 13 +- .../main/java/mage/util/FuzzyTestsUtil.java | 2 +- 12 files changed, 298 insertions(+), 20 deletions(-) create mode 100644 Mage.Tests/src/test/java/org/mage/test/AI/basic/SimulationStabilityAITest.java diff --git a/Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/ComputerPlayer6.java b/Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/ComputerPlayer6.java index b606a20d75e..da953db2726 100644 --- a/Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/ComputerPlayer6.java +++ b/Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/ComputerPlayer6.java @@ -114,6 +114,13 @@ public class ComputerPlayer6 extends ComputerPlayer { this.actionCache = player.actionCache; } + /** + * Change simulation timeout - used for AI stability tests only + */ + public void setMaxThinkTimeSecs(int maxThinkTimeSecs) { + this.maxThinkTimeSecs = maxThinkTimeSecs; + } + @Override public ComputerPlayer6 copy() { return new ComputerPlayer6(this); @@ -431,6 +438,8 @@ public class ComputerPlayer6 extends ComputerPlayer { * @return */ protected Integer addActionsTimed() { + // TODO: all actions added and calculated one by one, + // multithreading do not supported here // run new game simulation in parallel thread FutureTask task = new FutureTask<>(() -> addActions(root, maxDepth, Integer.MIN_VALUE, Integer.MAX_VALUE)); threadPoolSimulations.execute(task); @@ -446,15 +455,15 @@ public class ComputerPlayer6 extends ComputerPlayer { } } catch (TimeoutException | InterruptedException e) { // AI thinks too long - logger.info("ai simulating - timed out"); + logger.warn("AI player thinks too long - " + getName() + " - " + root.game); task.cancel(true); } catch (ExecutionException e) { // game error - logger.error("AI simulation catch game error: " + e, e); + logger.error("AI player catch game error in simulation - " + getName() + " - " + root.game + ": " + e, e); task.cancel(true); // real games: must catch and log // unit tests: must raise again for fast fail - if (this.isTestsMode()) { + if (this.isTestMode() && this.isFastFailInTestMode()) { throw new IllegalStateException("One of the simulated games raise the error: " + e, e); } } catch (Throwable e) { diff --git a/Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/ComputerPlayer7.java b/Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/ComputerPlayer7.java index 31b9bee41ec..09af732d422 100644 --- a/Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/ComputerPlayer7.java +++ b/Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/ComputerPlayer7.java @@ -142,7 +142,8 @@ public class ComputerPlayer7 extends ComputerPlayer6 { } } } else { - logger.info('[' + game.getPlayer(playerId).getName() + "][pre] Action: skip"); + // nothing to choose or freeze/infinite game + logger.info("AI player can't find next action: " + getName()); } } else { logger.debug("Next Action exists!"); diff --git a/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java b/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java index b9c6f3ff1b8..20a8114e4f2 100644 --- a/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java +++ b/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java @@ -52,7 +52,7 @@ public class ComputerPlayer extends PlayerImpl { protected static final int PASSIVITY_PENALTY = 5; // Penalty value for doing nothing if some actions are available // debug only: set TRUE to debug simulation's code/games (on false sim thread will be stopped after few secs by timeout) - protected static final boolean COMPUTER_DISABLE_TIMEOUT_IN_GAME_SIMULATIONS = true; // DebugUtil.AI_ENABLE_DEBUG_MODE; + public static final boolean COMPUTER_DISABLE_TIMEOUT_IN_GAME_SIMULATIONS = false; // DebugUtil.AI_ENABLE_DEBUG_MODE; // AI agents uses game simulation thread for all calcs and it's high CPU consumption // More AI threads - more parallel AI games can be calculate @@ -64,7 +64,7 @@ public class ComputerPlayer extends PlayerImpl { // * use yours CPU cores for best performance // TODO: add server config to control max AI threads (with CPU cores by default) // TODO: rework AI implementation to use multiple sims calculation instead one by one - final static int COMPUTER_MAX_THREADS_FOR_SIMULATIONS = 1;//DebugUtil.AI_ENABLE_DEBUG_MODE ? 1 : 5; + final static int COMPUTER_MAX_THREADS_FOR_SIMULATIONS = 5;//DebugUtil.AI_ENABLE_DEBUG_MODE ? 1 : 5; // remember picked cards for better draft choices @@ -104,7 +104,7 @@ public class ComputerPlayer extends PlayerImpl { @Override public boolean chooseMulligan(Game game) { if (hand.size() < 6 - || isTestsMode() // ignore mulligan in tests + || isTestMode() // ignore mulligan in tests || game.getClass().getName().contains("Momir") // ignore mulligan in Momir games ) { return false; diff --git a/Mage.Server.Plugins/Mage.Player.AIMCTS/src/mage/player/ai/ComputerPlayerMCTS.java b/Mage.Server.Plugins/Mage.Player.AIMCTS/src/mage/player/ai/ComputerPlayerMCTS.java index 8619ef2aeb8..4d65b8bc61c 100644 --- a/Mage.Server.Plugins/Mage.Player.AIMCTS/src/mage/player/ai/ComputerPlayerMCTS.java +++ b/Mage.Server.Plugins/Mage.Player.AIMCTS/src/mage/player/ai/ComputerPlayerMCTS.java @@ -196,7 +196,7 @@ public class ComputerPlayerMCTS extends ComputerPlayer { } catch (ExecutionException e) { // real games: must catch and log // unit tests: must raise again for fast fail - if (this.isTestsMode()) { + if (this.isTestMode() && this.isFastFailInTestMode()) { throw new IllegalStateException("One of the simulated games raise the error: " + e, e); } } diff --git a/Mage.Tests/src/test/java/org/mage/test/AI/basic/SimulationStabilityAITest.java b/Mage.Tests/src/test/java/org/mage/test/AI/basic/SimulationStabilityAITest.java new file mode 100644 index 00000000000..d1aa17af189 --- /dev/null +++ b/Mage.Tests/src/test/java/org/mage/test/AI/basic/SimulationStabilityAITest.java @@ -0,0 +1,237 @@ +package org.mage.test.AI.basic; + +import mage.abilities.Ability; +import mage.abilities.common.LimitedTimesPerTurnActivatedAbility; +import mage.abilities.costs.mana.ManaCostsImpl; +import mage.abilities.effects.Effect; +import mage.abilities.effects.OneShotEffect; +import mage.abilities.effects.common.GainLifeEffect; +import mage.constants.Outcome; +import mage.constants.PhaseStep; +import mage.constants.Zone; +import mage.game.Game; +import mage.player.ai.ComputerPlayer; +import mage.player.ai.ComputerPlayer7; +import mage.util.ThreadUtils; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Test; +import org.mage.test.serverside.base.CardTestPlayerBaseWithAIHelps; + +/** + * Tests for AI simulation stability (AI must process simulations with freezes or errors) + * + * @author JayDi85 + */ +public class SimulationStabilityAITest extends CardTestPlayerBaseWithAIHelps { + + @Before + public void prepare() { + // comment it to enable AI code debug + Assert.assertFalse("AI stability tests must be run under release config", ComputerPlayer.COMPUTER_DISABLE_TIMEOUT_IN_GAME_SIMULATIONS); + } + + @Test + public void test_GameFreeze_OnlyGoodAbilities() { + removeAllCardsFromLibrary(playerA); + + // possible combinations: from 1 to 3 abilities - all fine + addFreezeAbility("good 1", false); + addFreezeAbility("good 2", false); + addFreezeAbility("good 3", false); + + // AI must activate all +3 life + aiPlayStep(1, PhaseStep.PRECOMBAT_MAIN, playerA); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertLife(playerA, 20 + 3); + } + + @Test + public void test_GameFreeze_OnlyFreezeAbilities() { + removeAllCardsFromLibrary(playerA); + + // possible combinations: from 1 to 3 bad abilities - all bad + addFreezeAbility("freeze 1", true); + addFreezeAbility("freeze 2", true); + addFreezeAbility("freeze 3", true); + + // AI can't finish any simulation and do not choose to activate in real game + aiPlayStep(1, PhaseStep.PRECOMBAT_MAIN, playerA); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertLife(playerA, 20); + } + + @Test + @Ignore + // TODO: AI actions simulation do not support multithreading, so whole next action search + // will fail on any problem (enable after new simulation implement) + public void test_GameFreeze_GoodAndFreezeAbilities() { + removeAllCardsFromLibrary(playerA); + + // possible combinations: some good chains, some bad chains + addFreezeAbility("good 1", false); + addFreezeAbility("good 2", false); + addFreezeAbility("good 3", false); + addFreezeAbility("freeze 1", true); + + // AI must see and filter bad combinations with freeze ability in the chain + // so only 1 + 2 + 3 will give best score and will be chosen for real game + aiPlayStep(1, PhaseStep.PRECOMBAT_MAIN, playerA); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertLife(playerA, 20 + 3); + } + + @Test + public void test_GameError_OnlyGoodAbilities() { + removeAllCardsFromLibrary(playerA); + + // possible combinations: from 1 to 3 abilities - all fine + addErrorAbility("good 1", false); + addErrorAbility("good 2", false); + addErrorAbility("good 3", false); + + // AI must activate all +3 life + aiPlayStep(1, PhaseStep.PRECOMBAT_MAIN, playerA); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertLife(playerA, 20 + 3); + } + + @Test + public void test_GameError_OnlyErrorAbilities() { + removeAllCardsFromLibrary(playerA); + + // it's ok to have error logs in output + + // possible combinations: from 1 to 3 bad abilities - all bad + addErrorAbility("error 1", true); + addErrorAbility("error 2", true); + addErrorAbility("error 3", true); + + // AI can't finish any simulation and do not choose to activate in real game + aiPlayStep(1, PhaseStep.PRECOMBAT_MAIN, playerA); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertLife(playerA, 20); + } + + @Test + @Ignore + // TODO: AI actions simulation do not support multithreading, so whole next action search + // will fail on any problem (enable after new simulation implement) + public void test_GameError_GoodAndFreezeAbilities() { + removeAllCardsFromLibrary(playerA); + + // it's ok to have error logs in output + + // possible combinations: some good chains, some bad chains + addErrorAbility("good 1", false); + addErrorAbility("good 2", false); + addErrorAbility("good 3", false); + addErrorAbility("error 1", true); + + // AI must see and filter bad combinations with error ability in the chain + // so only 1 + 2 + 3 will give best score and will be chosen for real game + aiPlayStep(1, PhaseStep.PRECOMBAT_MAIN, playerA); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertLife(playerA, 20 + 3); + } + + private void addFreezeAbility(String name, boolean isFreeze) { + // change max think timeout to lower value, so test can work faster + ComputerPlayer7 aiPlayer = (ComputerPlayer7) playerA.getRealPlayer(); + aiPlayer.setMaxThinkTimeSecs(1); + + Effect effect; + if (isFreeze) { + effect = new GameFreezeEffect(); + } else { + effect = new GainLifeEffect(1); + } + effect.setText(name); + addCustomCardWithAbility(name, playerA, new LimitedTimesPerTurnActivatedAbility(effect, new ManaCostsImpl<>("{G}"))); + addCard(Zone.BATTLEFIELD, playerA, "Forest", 1); + } + + private void addErrorAbility(String name, boolean isError) { + // change error processing, so test can continue simulations after catch error - like a real game + playerA.setFastFailInTestMode(false); + + Effect effect; + if (isError) { + effect = new GameErrorEffect(); + } else { + effect = new GainLifeEffect(1); + } + effect.setText(name); + addCustomCardWithAbility(name, playerA, new LimitedTimesPerTurnActivatedAbility(effect, new ManaCostsImpl<>("{G}"))); + addCard(Zone.BATTLEFIELD, playerA, "Forest", 1); + } +} + +class GameFreezeEffect extends OneShotEffect { + + GameFreezeEffect() { + super(Outcome.Benefit); + } + + private GameFreezeEffect(final GameFreezeEffect effect) { + super(effect); + } + + public GameFreezeEffect copy() { + return new GameFreezeEffect(this); + } + + public boolean apply(Game game, Ability source) { + // freeze simulation, AI must close sim thread by timeout + System.out.println("apply freeze effect on " + game); // for debug only, show logs from any sim thread + while (true) { + ThreadUtils.sleep(1000); + } + } +} + +class GameErrorEffect extends OneShotEffect { + + GameErrorEffect() { + super(Outcome.Benefit); + } + + private GameErrorEffect(final GameErrorEffect effect) { + super(effect); + } + + public GameErrorEffect copy() { + return new GameErrorEffect(this); + } + + public boolean apply(Game game, Ability source) { + // error simulation, AI must close error thread, do not use rollback + System.out.println("apply error effect on " + game); // for debug only, show logs from any sim thread + throw new IllegalStateException("Test error"); + } +} diff --git a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java index 257266776f3..43357453e4a 100644 --- a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java +++ b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java @@ -775,7 +775,12 @@ public class TestPlayer implements Player { AIRealGameControlUntil = endStep; // disable on end step computerPlayer.priority(game); actions.remove(action); - computerPlayer.resetPassed(); // remove AI's pass, so runtime/check commands can be executed in same priority + // remove AI's pass, so runtime/check commands can be executed in same priority + // aiPlayStep can cause double priority call, but it's better to have workable checkXXX commands + // (AI will do nothing on second priority call anyway) + if (!actions.isEmpty()) { + computerPlayer.resetPassed(); + } return true; } @@ -3438,7 +3443,7 @@ public class TestPlayer implements Player { @Override public boolean isComputer() { // all players in unit tests are computers, so it allows testing different logic (Human vs AI) - if (isTestsMode()) { + if (isTestMode()) { // AIRealGameSimulation = true - full plyable AI // AIRealGameSimulation = false - choose assisted AI (Human) return AIRealGameSimulation; @@ -3874,8 +3879,8 @@ public class TestPlayer implements Player { } @Override - public boolean isTestsMode() { - return computerPlayer.isTestsMode(); + public boolean isTestMode() { + return computerPlayer.isTestMode(); } @Override @@ -3883,6 +3888,16 @@ public class TestPlayer implements Player { computerPlayer.setTestMode(value); } + @Override + public boolean isFastFailInTestMode() { + return computerPlayer.isFastFailInTestMode(); + } + + @Override + public void setFastFailInTestMode(boolean value) { + computerPlayer.setFastFailInTestMode(value); + } + @Override public boolean isTopCardRevealed() { return computerPlayer.isTopCardRevealed(); diff --git a/Mage/src/main/java/mage/abilities/AbilityImpl.java b/Mage/src/main/java/mage/abilities/AbilityImpl.java index 6366a6a4209..50340177526 100644 --- a/Mage/src/main/java/mage/abilities/AbilityImpl.java +++ b/Mage/src/main/java/mage/abilities/AbilityImpl.java @@ -377,7 +377,7 @@ public abstract class AbilityImpl implements Ability { // unit tests only: it allows to add targets/choices by two ways: // 1. From cast/activate command params (process it here) // 2. From single addTarget/setChoice, it's a preferred method for tests (process it in normal choose dialogs like human player) - if (controller.isTestsMode()) { + if (controller.isTestMode()) { if (!controller.addTargets(this, game)) { return false; } diff --git a/Mage/src/main/java/mage/game/GameImpl.java b/Mage/src/main/java/mage/game/GameImpl.java index e48ef287395..10ce4cd1fab 100644 --- a/Mage/src/main/java/mage/game/GameImpl.java +++ b/Mage/src/main/java/mage/game/GameImpl.java @@ -1782,7 +1782,7 @@ public abstract class GameImpl implements Game { // count total errors Player activePlayer = this.getPlayer(getActivePlayerId()); - if (activePlayer != null && !activePlayer.isTestsMode()) { + if (activePlayer != null && !activePlayer.isTestMode() && !activePlayer.isFastFailInTestMode()) { // real game - try to continue priorityErrorsCount++; continue; diff --git a/Mage/src/main/java/mage/game/combat/Combat.java b/Mage/src/main/java/mage/game/combat/Combat.java index 333e786b95a..1fa776ffc2f 100644 --- a/Mage/src/main/java/mage/game/combat/Combat.java +++ b/Mage/src/main/java/mage/game/combat/Combat.java @@ -695,7 +695,8 @@ public class Combat implements Serializable, Copyable { // real game: send warning // test: fast fail game.informPlayers(controller.getLogName() + ": WARNING - AI can't find good blocker combination and will skip it - report your battlefield to github - " + game.getCombat()); - if (controller.isTestsMode()) { + if (controller.isTestMode() && controller.isFastFailInTestMode()) { + // fast fail in tests // how-to fix: AI code must support failed abilities or use cases throw new IllegalArgumentException("AI can't find good blocker combination"); } diff --git a/Mage/src/main/java/mage/players/Player.java b/Mage/src/main/java/mage/players/Player.java index eb4fe3f69c0..5e3e89f8a8d 100644 --- a/Mage/src/main/java/mage/players/Player.java +++ b/Mage/src/main/java/mage/players/Player.java @@ -79,7 +79,13 @@ public interface Player extends MageItem, Copyable { */ boolean isHuman(); - boolean isTestsMode(); + boolean isTestMode(); + + void setTestMode(boolean value); + + boolean isFastFailInTestMode(); + + void setFastFailInTestMode(boolean value); /** * Current player is AI. Use it in card's code and all other places. @@ -398,8 +404,6 @@ public interface Player extends MageItem, Copyable { */ void setGameUnderYourControl(Game game, boolean value, boolean fullRestore); - void setTestMode(boolean value); - void setAllowBadMoves(boolean allowBadMoves); /** diff --git a/Mage/src/main/java/mage/players/PlayerImpl.java b/Mage/src/main/java/mage/players/PlayerImpl.java index e830b0d70bb..df96f2a2874 100644 --- a/Mage/src/main/java/mage/players/PlayerImpl.java +++ b/Mage/src/main/java/mage/players/PlayerImpl.java @@ -147,6 +147,7 @@ public abstract class PlayerImpl implements Player, Serializable { protected Set inRange = new HashSet<>(); // players list in current range of influence (updates each turn due rules) protected boolean isTestMode = false; + protected boolean isFastFailInTestMode = false; protected boolean canGainLife = true; protected boolean canLoseLife = true; protected PayLifeCostLevel payLifeCostLevel = PayLifeCostLevel.allAbilities; @@ -4602,7 +4603,7 @@ public abstract class PlayerImpl implements Player, Serializable { } @Override - public boolean isTestsMode() { + public boolean isTestMode() { return isTestMode; } @@ -4611,6 +4612,16 @@ public abstract class PlayerImpl implements Player, Serializable { this.isTestMode = value; } + @Override + public boolean isFastFailInTestMode() { + return isFastFailInTestMode; + } + + @Override + public void setFastFailInTestMode(boolean value) { + this.isFastFailInTestMode = value; + } + @Override public boolean isTopCardRevealed() { return topCardRevealed; diff --git a/Mage/src/main/java/mage/util/FuzzyTestsUtil.java b/Mage/src/main/java/mage/util/FuzzyTestsUtil.java index 1523c9040a5..1c7f29a43b6 100644 --- a/Mage/src/main/java/mage/util/FuzzyTestsUtil.java +++ b/Mage/src/main/java/mage/util/FuzzyTestsUtil.java @@ -58,7 +58,7 @@ public class FuzzyTestsUtil { return; } Player samplePlayer = game.getPlayers().values().stream().findFirst().orElse(null); - if (samplePlayer == null || !samplePlayer.isTestsMode()) { + if (samplePlayer == null || !samplePlayer.isTestMode()) { return; }