From 3899e0955f1145686a5250a9b33bb31afbf37633 Mon Sep 17 00:00:00 2001 From: Madwand99 Date: Wed, 29 Apr 2026 19:22:46 -0700 Subject: [PATCH 1/4] Improve AI simulation stack resolution for multiplayer choices --- .../java/forge/ai/simulation/GameCopier.java | 2 +- .../forge/ai/simulation/GameSimulator.java | 33 ++-- .../ai/simulation/GameStateEvaluator.java | 2 +- .../ai/sacrifice/AiSacrificeDecisionTest.java | 159 +++++++++++++++--- .../ai/simulation/GameSimulationTest.java | 39 +++++ 5 files changed, 199 insertions(+), 36 deletions(-) diff --git a/forge-ai/src/main/java/forge/ai/simulation/GameCopier.java b/forge-ai/src/main/java/forge/ai/simulation/GameCopier.java index a367ab10d3d..291bf7c9966 100644 --- a/forge-ai/src/main/java/forge/ai/simulation/GameCopier.java +++ b/forge-ai/src/main/java/forge/ai/simulation/GameCopier.java @@ -175,7 +175,7 @@ public Game makeCopy(PhaseType advanceToPhase, Player aiPlayer) { // TODO update thisTurnCast if (advanceToPhase != null) { - newGame.getPhaseHandler().devAdvanceToPhase(advanceToPhase, () -> GameSimulator.resolveStack(newGame, aiPlayer.getWeakestOpponent())); + newGame.getPhaseHandler().devAdvanceToPhase(advanceToPhase, () -> GameSimulator.resolveStack(newGame, aiPlayer)); } return newGame; diff --git a/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java b/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java index aac320753a8..32846e62118 100644 --- a/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java +++ b/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java @@ -53,7 +53,7 @@ public GameSimulator(SimulationController controller, Game origGame, Player orig debugLines = origLines; Game copyOrigGame = copier.makeCopy(); Player copyOrigAiPlayer = copyOrigGame.getPlayers().get(1); - resolveStack(copyOrigGame, copyOrigGame.getPlayers().get(0)); + resolveStack(copyOrigGame, copyOrigAiPlayer); origScore = eval.getScoreForGameState(copyOrigGame, copyOrigAiPlayer); } @@ -225,9 +225,7 @@ public Score simulateSpellAbility(SpellAbility origSa, GameStateEvaluator eval, } if (resolve) { - // TODO: Support multiple opponents. - Player opponent = aiPlayer.getWeakestOpponent(); - resolveStack(simGame, opponent); + resolveStack(simGame, aiPlayer); } // TODO: If this is during combat, before blockers are declared, @@ -260,11 +258,9 @@ public Score simulateSpellAbility(SpellAbility origSa, GameStateEvaluator eval, return score; } - public static void resolveStack(final Game game, final Player opponent) { - // TODO: This needs to set an AI controller for all opponents, in case of multiplayer. - PlayerControllerAi sim = new PlayerControllerAi(game, opponent, opponent.getLobbyPlayer()); - sim.setUseSimulation(true); - opponent.runWithController(() -> { + public static void resolveStack(final Game game, final Player preservedPlayer) { + List players = new ArrayList<>(game.getPlayers()); + runWithSimulationControllers(game, preservedPlayer, players, 0, () -> { final Set allAffectedCards = new HashSet<>(); game.getAction().checkStateEffects(false, allAffectedCards); game.getStack().addAllTriggeredAbilitiesToStack(); @@ -288,7 +284,24 @@ public static void resolveStack(final Game game, final Player opponent) { // Continue until stack is empty. } - }, sim); + }); + } + + private static void runWithSimulationControllers(final Game game, final Player preservedPlayer, + final List players, final int index, final Runnable proc) { + if (index >= players.size()) { + proc.run(); + return; + } + + Player player = players.get(index); + if (player.equals(preservedPlayer)) { + runWithSimulationControllers(game, preservedPlayer, players, index + 1, proc); + return; + } + PlayerControllerAi sim = new PlayerControllerAi(game, player, player.getLobbyPlayer()); + sim.setUseSimulation(true); + player.runWithController(() -> runWithSimulationControllers(game, preservedPlayer, players, index + 1, proc), sim); } public Game getSimulatedGameState() { diff --git a/forge-ai/src/main/java/forge/ai/simulation/GameStateEvaluator.java b/forge-ai/src/main/java/forge/ai/simulation/GameStateEvaluator.java index 979c61052fc..720f701c0f2 100644 --- a/forge-ai/src/main/java/forge/ai/simulation/GameStateEvaluator.java +++ b/forge-ai/src/main/java/forge/ai/simulation/GameStateEvaluator.java @@ -59,7 +59,7 @@ private CombatSimResult simulateUpcomingCombatThisTurn(final Game evalGame, fina gameCopy = copier.makeCopy(null, aiPlayer); } - gameCopy.getPhaseHandler().devAdvanceToPhase(PhaseType.COMBAT_DAMAGE, () -> GameSimulator.resolveStack(gameCopy, aiPlayer.getWeakestOpponent())); + gameCopy.getPhaseHandler().devAdvanceToPhase(PhaseType.COMBAT_DAMAGE, () -> GameSimulator.resolveStack(gameCopy, aiPlayer)); CombatSimResult result = new CombatSimResult(); result.copier = copier; result.gameCopy = gameCopy; diff --git a/forge-gui-desktop/src/test/java/forge/ai/sacrifice/AiSacrificeDecisionTest.java b/forge-gui-desktop/src/test/java/forge/ai/sacrifice/AiSacrificeDecisionTest.java index 7d0adf95a8a..76e4e15730b 100644 --- a/forge-gui-desktop/src/test/java/forge/ai/sacrifice/AiSacrificeDecisionTest.java +++ b/forge-gui-desktop/src/test/java/forge/ai/sacrifice/AiSacrificeDecisionTest.java @@ -1,9 +1,13 @@ package forge.ai.sacrifice; import forge.ai.AITest; +import forge.ai.PlayerControllerAi; import forge.game.Game; import forge.game.card.Card; +import forge.game.card.CardCollection; +import forge.game.card.CardCollectionView; import forge.game.player.Player; +import forge.game.spellability.SpellAbility; import forge.game.zone.ZoneType; import org.testng.AssertJUnit; import org.testng.annotations.Test; @@ -13,6 +17,90 @@ */ public class AiSacrificeDecisionTest extends AITest { + @Test + public void testSimulationSacrificeChoiceIsDeterministic() { + Game game = initAndCreateGame(); + + Player ai = game.getPlayers().get(1); + SpellAbility source = sacrificeSource(ai); + + Card bear = addCard("Runeclaw Bear", ai); + addCard("Serra Angel", ai); + addCard("Colossal Dreadmaw", ai); + + CardCollection validTargets = creatures(ai); + + for (int i = 0; i < 5; i++) { + CardCollectionView chosen = chooseWithSimulationController(game, ai, source, 1, validTargets); + + AssertJUnit.assertEquals("Simulation sacrifice choice should be stable", 1, chosen.size()); + AssertJUnit.assertEquals("AI should consistently choose the least valuable creature", + bear, chosen.get(0)); + } + } + + @Test + public void testSimulationSacrificeChoiceUsesOnlyLegalCandidates() { + Game game = initAndCreateGame(); + + Player ai = game.getPlayers().get(1); + SpellAbility source = sacrificeSource(ai); + + addCard("Runeclaw Bear", ai); + Card angel = addCard("Serra Angel", ai); + Card dreadmaw = addCard("Colossal Dreadmaw", ai); + + CardCollection validTargets = new CardCollection(); + validTargets.add(angel); + validTargets.add(dreadmaw); + + CardCollectionView chosen = chooseWithSimulationController(game, ai, source, 1, validTargets); + + AssertJUnit.assertEquals("AI should choose one legal creature to sacrifice", 1, chosen.size()); + AssertJUnit.assertEquals("AI should choose the least valuable legal creature", + angel, chosen.get(0)); + } + + @Test + public void testSimulationSacrificeChoiceChoosesWorstCreaturesFirst() { + Game game = initAndCreateGame(); + + Player ai = game.getPlayers().get(1); + SpellAbility source = sacrificeSource(ai); + + Card bear = addCard("Runeclaw Bear", ai); + Card angel = addCard("Serra Angel", ai); + Card dreadmaw = addCard("Colossal Dreadmaw", ai); + + CardCollectionView chosen = chooseWithSimulationController(game, ai, source, 2, creatures(ai)); + + AssertJUnit.assertEquals("AI should choose the requested number of creatures", 2, chosen.size()); + AssertJUnit.assertTrue("AI should sacrifice Runeclaw Bear before better creatures", chosen.contains(bear)); + AssertJUnit.assertTrue("AI should sacrifice Serra Angel before Colossal Dreadmaw", chosen.contains(angel)); + AssertJUnit.assertFalse("AI should preserve the most valuable creature when sacrificing two of three", + chosen.contains(dreadmaw)); + } + + @Test + public void testSimulationSacrificeChoiceCanChooseLeastValuablePermanent() { + Game game = initAndCreateGame(); + + Player ai = game.getPlayers().get(1); + SpellAbility source = sacrificeSource(ai); + + Card ornithopter = addCard("Ornithopter", ai); + addCard("Sol Ring", ai); + addCard("Serra Angel", ai); + + CardCollection validTargets = new CardCollection(ai.getCardsIn(ZoneType.Battlefield)); + + CardCollectionView chosen = chooseWithSimulationController(game, ai, source, 1, validTargets); + + AssertJUnit.assertEquals("AI should choose one permanent to sacrifice", 1, chosen.size()); + AssertJUnit.assertEquals("AI should choose the cheapest artifact/enchantment in a mixed permanent set", + ornithopter, chosen.get(0)); + } + @Test public void testAiSacrificesCreatureToAvoidLethalLifeLoss() { Game game = initAndCreateGame(); @@ -33,11 +121,7 @@ public void testAiSacrificesCreatureToAvoidLethalLifeLoss() { // Opponent controls a creature they can sacrifice Card adelbert = addCard("Adelbert Steiner", opponent); - // Fill libraries to prevent draw-death - for (int i = 0; i < 10; i++) { - addCardToZone("Plains", p, ZoneType.Library); - addCardToZone("Plains", opponent, ZoneType.Library); - } + fillLibraries(p, opponent, 10); // Play until next turn (after Fandaniel's end step trigger resolves) this.playUntilNextTurn(game); @@ -45,9 +129,8 @@ public void testAiSacrificesCreatureToAvoidLethalLifeLoss() { // The game should NOT be over - opponent should have sacrificed to survive AssertJUnit.assertFalse("Game should not be over - AI should have sacrificed to survive", game.isGameOver()); - // Adelbert should be in graveyard (sacrificed) - AssertJUnit.assertTrue("Adelbert Steiner should be in graveyard after being sacrificed", - opponent.getZone(ZoneType.Graveyard).contains(adelbert)); + assertInZone(opponent, adelbert, ZoneType.Graveyard, + "Adelbert Steiner should be in graveyard after being sacrificed"); // Opponent should still have 1 life (didn't lose life because they sacrificed) AssertJUnit.assertEquals("Opponent should still have 1 life after sacrificing", 1, opponent.getLife()); @@ -74,11 +157,7 @@ public void testAiDoesNotSacrificeWhenLifeLossIsNotLethal() { // Opponent controls a creature Card adelbert = addCard("Adelbert Steiner", opponent); - // Fill libraries - for (int i = 0; i < 10; i++) { - addCardToZone("Plains", p, ZoneType.Library); - addCardToZone("Plains", opponent, ZoneType.Library); - } + fillLibraries(p, opponent, 10); // Play until next turn (after Fandaniel's end step trigger resolves) this.playUntilNextTurn(game); @@ -86,9 +165,8 @@ public void testAiDoesNotSacrificeWhenLifeLossIsNotLethal() { // Game should not be over AssertJUnit.assertFalse("Game should not be over", game.isGameOver()); - // Adelbert should still be on battlefield (not sacrificed) - AssertJUnit.assertTrue("Adelbert Steiner should still be on battlefield when life loss isn't lethal", - opponent.getZone(ZoneType.Battlefield).contains(adelbert)); + assertInZone(opponent, adelbert, ZoneType.Battlefield, + "Adelbert Steiner should still be on battlefield when life loss isn't lethal"); // Opponent should have 8 life (lost 2 from not sacrificing) AssertJUnit.assertEquals("Opponent should have lost 2 life from not sacrificing", 8, opponent.getLife()); @@ -112,11 +190,7 @@ public void testAiSacrificesToPillarTombsToAvoidLethalLifeLoss() { // Opponent controls a creature they can sacrifice Card bear = addCard("Runeclaw Bear", opponent); - // Fill libraries to prevent draw-death - for (int i = 0; i < 10; i++) { - addCardToZone("Plains", p, ZoneType.Library); - addCardToZone("Plains", opponent, ZoneType.Library); - } + fillLibraries(p, opponent, 10); // Play two turns - first ends Player 1's turn, second plays through opponent's upkeep // where Pillar Tombs triggers and the AI must decide whether to sacrifice @@ -126,11 +200,48 @@ public void testAiSacrificesToPillarTombsToAvoidLethalLifeLoss() { // The game should NOT be over - AI should have sacrificed to survive AssertJUnit.assertFalse("Game should not be over - AI should have sacrificed to survive", game.isGameOver()); - // Bear should be in graveyard (sacrificed) - AssertJUnit.assertTrue("Runeclaw Bear should be in graveyard after being sacrificed", - opponent.getZone(ZoneType.Graveyard).contains(bear)); + assertInZone(opponent, bear, ZoneType.Graveyard, + "Runeclaw Bear should be in graveyard after being sacrificed"); // Opponent should still have 4 life (didn't lose life because they sacrificed) AssertJUnit.assertEquals("Opponent should still have 4 life after sacrificing", 4, opponent.getLife()); } + + private SpellAbility sacrificeSource(Player ai) { + SpellAbility source = createCard("Innocent Blood", ai).getFirstSpellAbility(); + source.setActivatingPlayer(ai); + return source; + } + + private CardCollection creatures(Player player) { + CardCollection creatures = new CardCollection(); + for (Card card : player.getCardsIn(ZoneType.Battlefield)) { + if (card.isCreature()) { + creatures.add(card); + } + } + return creatures; + } + + private void fillLibraries(Player first, Player second, int count) { + for (int i = 0; i < count; i++) { + addCardToZone("Plains", first, ZoneType.Library); + addCardToZone("Plains", second, ZoneType.Library); + } + } + + private void assertInZone(Player player, Card card, ZoneType zone, String message) { + AssertJUnit.assertTrue(message, player.getZone(zone).contains(card)); + } + + private CardCollectionView chooseWithSimulationController(Game game, Player ai, SpellAbility source, + int amount, CardCollectionView validTargets) { + PlayerControllerAi controller = new PlayerControllerAi(game, ai, ai.getLobbyPlayer()); + controller.setUseSimulation(true); + + final CardCollectionView[] chosen = new CardCollectionView[1]; + ai.runWithController(() -> chosen[0] = ai.getController().choosePermanentsToSacrifice( + source, amount, amount, validTargets, "Choose permanents to sacrifice"), controller); + return chosen[0]; + } } diff --git a/forge-gui-desktop/src/test/java/forge/ai/simulation/GameSimulationTest.java b/forge-gui-desktop/src/test/java/forge/ai/simulation/GameSimulationTest.java index ecd30c6ff50..ce45e1854f4 100644 --- a/forge-gui-desktop/src/test/java/forge/ai/simulation/GameSimulationTest.java +++ b/forge-gui-desktop/src/test/java/forge/ai/simulation/GameSimulationTest.java @@ -2,6 +2,7 @@ import com.google.common.collect.Lists; import forge.ai.ComputerUtilAbility; +import forge.ai.PlayerControllerAi; import forge.card.CardStateName; import forge.card.MagicColor; import forge.game.Game; @@ -61,6 +62,44 @@ public void testActivateAbilityTriggers() { AssertJUnit.assertEquals(1, warriorToken.getCurrentToughness()); } + @Test + public void testResolveStackUsesSimulationControllersForAllNonPreservedPlayers() { + Game game = initAndCreateThreePlayerGame(); + + Player opponent = game.getPlayers().get(0); + Player ai = game.getPlayers().get(1); + Player otherOpponent = game.getPlayers().get(2); + + opponent.setTeam(0); + ai.setTeam(1); + otherOpponent.setTeam(2); + + opponent.setLife(1, null); + otherOpponent.setLife(20, null); + + addCard("Runeclaw Bear", opponent); + addCard("Runeclaw Bear", ai); + Card otherOpponentBear = addCard("Runeclaw Bear", otherOpponent); + + Card spell = addCardToZone("Innocent Blood", opponent, ZoneType.Hand); + SpellAbility sa = spell.getFirstSpellAbility(); + sa.setActivatingPlayer(opponent); + game.getStack().addAndUnfreeze(sa); + + PlayerControllerAi throwingController = new PlayerControllerAi(game, otherOpponent, otherOpponent.getLobbyPlayer()) { + @Override + public CardCollectionView choosePermanentsToSacrifice(SpellAbility ability, int min, int max, + CardCollectionView validTargets, String message) { + throw new AssertionError("resolveStack should install a simulation controller for this player"); + } + }; + + otherOpponent.runWithController(() -> GameSimulator.resolveStack(game, ai), throwingController); + + AssertJUnit.assertTrue("The non-preserved non-weakest player should have sacrificed during stack resolution", + otherOpponent.getZone(ZoneType.Graveyard).contains(otherOpponentBear)); + } + @Test public void testStaticAbilities() { String sliverCardName = "Sidewinder Sliver"; From 7fee4253ad585ff6c82c6498655f2aa17018b990 Mon Sep 17 00:00:00 2001 From: Madwand99 Date: Fri, 1 May 2026 04:46:40 -0700 Subject: [PATCH 2/4] Skip creating simulation controller for AIs --- .../main/java/forge/ai/simulation/GameSimulator.java | 2 +- .../java/forge/ai/simulation/GameSimulationTest.java | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java b/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java index 32846e62118..af47b564c01 100644 --- a/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java +++ b/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java @@ -295,7 +295,7 @@ private static void runWithSimulationControllers(final Game game, final Player p } Player player = players.get(index); - if (player.equals(preservedPlayer)) { + if (player.equals(preservedPlayer) || player.getController().isAI()) { runWithSimulationControllers(game, preservedPlayer, players, index + 1, proc); return; } diff --git a/forge-gui-desktop/src/test/java/forge/ai/simulation/GameSimulationTest.java b/forge-gui-desktop/src/test/java/forge/ai/simulation/GameSimulationTest.java index ce45e1854f4..b02baf249a2 100644 --- a/forge-gui-desktop/src/test/java/forge/ai/simulation/GameSimulationTest.java +++ b/forge-gui-desktop/src/test/java/forge/ai/simulation/GameSimulationTest.java @@ -2,7 +2,6 @@ import com.google.common.collect.Lists; import forge.ai.ComputerUtilAbility; -import forge.ai.PlayerControllerAi; import forge.card.CardStateName; import forge.card.MagicColor; import forge.game.Game; @@ -16,6 +15,7 @@ import forge.game.player.Player; import forge.game.spellability.SpellAbility; import forge.game.zone.ZoneType; +import forge.gamesimulationtests.util.PlayerControllerForTests; import forge.util.StreamUtil; import org.testng.AssertJUnit; @@ -63,7 +63,7 @@ public void testActivateAbilityTriggers() { } @Test - public void testResolveStackUsesSimulationControllersForAllNonPreservedPlayers() { + public void testResolveStackUsesSimulationControllersForNonAiPlayers() { Game game = initAndCreateThreePlayerGame(); Player opponent = game.getPlayers().get(0); @@ -86,17 +86,17 @@ public void testResolveStackUsesSimulationControllersForAllNonPreservedPlayers() sa.setActivatingPlayer(opponent); game.getStack().addAndUnfreeze(sa); - PlayerControllerAi throwingController = new PlayerControllerAi(game, otherOpponent, otherOpponent.getLobbyPlayer()) { + PlayerControllerForTests throwingController = new PlayerControllerForTests(game, otherOpponent, otherOpponent.getLobbyPlayer()) { @Override public CardCollectionView choosePermanentsToSacrifice(SpellAbility ability, int min, int max, CardCollectionView validTargets, String message) { - throw new AssertionError("resolveStack should install a simulation controller for this player"); + throw new AssertionError("resolveStack should install a simulation controller for non-AI players"); } }; otherOpponent.runWithController(() -> GameSimulator.resolveStack(game, ai), throwingController); - AssertJUnit.assertTrue("The non-preserved non-weakest player should have sacrificed during stack resolution", + AssertJUnit.assertTrue("The non-AI non-weakest player should have sacrificed during stack resolution", otherOpponent.getZone(ZoneType.Graveyard).contains(otherOpponentBear)); } From 3a7a9e997faf9d5f9f1e3a679c588e2afa95c05e Mon Sep 17 00:00:00 2001 From: Madwand99 Date: Fri, 1 May 2026 06:34:09 -0700 Subject: [PATCH 3/4] Simplify simulation stack controller wrapping --- .../java/forge/ai/simulation/GameCopier.java | 2 +- .../forge/ai/simulation/GameSimulator.java | 21 +++++++++---------- .../ai/simulation/GameStateEvaluator.java | 2 +- .../ai/simulation/GameSimulationTest.java | 2 +- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/forge-ai/src/main/java/forge/ai/simulation/GameCopier.java b/forge-ai/src/main/java/forge/ai/simulation/GameCopier.java index 291bf7c9966..546a8259071 100644 --- a/forge-ai/src/main/java/forge/ai/simulation/GameCopier.java +++ b/forge-ai/src/main/java/forge/ai/simulation/GameCopier.java @@ -175,7 +175,7 @@ public Game makeCopy(PhaseType advanceToPhase, Player aiPlayer) { // TODO update thisTurnCast if (advanceToPhase != null) { - newGame.getPhaseHandler().devAdvanceToPhase(advanceToPhase, () -> GameSimulator.resolveStack(newGame, aiPlayer)); + newGame.getPhaseHandler().devAdvanceToPhase(advanceToPhase, () -> GameSimulator.resolveStack(newGame)); } return newGame; diff --git a/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java b/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java index af47b564c01..b8f3328df1e 100644 --- a/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java +++ b/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java @@ -53,7 +53,7 @@ public GameSimulator(SimulationController controller, Game origGame, Player orig debugLines = origLines; Game copyOrigGame = copier.makeCopy(); Player copyOrigAiPlayer = copyOrigGame.getPlayers().get(1); - resolveStack(copyOrigGame, copyOrigAiPlayer); + resolveStack(copyOrigGame); origScore = eval.getScoreForGameState(copyOrigGame, copyOrigAiPlayer); } @@ -225,7 +225,7 @@ public Score simulateSpellAbility(SpellAbility origSa, GameStateEvaluator eval, } if (resolve) { - resolveStack(simGame, aiPlayer); + resolveStack(simGame); } // TODO: If this is during combat, before blockers are declared, @@ -258,9 +258,9 @@ public Score simulateSpellAbility(SpellAbility origSa, GameStateEvaluator eval, return score; } - public static void resolveStack(final Game game, final Player preservedPlayer) { + public static void resolveStack(final Game game) { List players = new ArrayList<>(game.getPlayers()); - runWithSimulationControllers(game, preservedPlayer, players, 0, () -> { + runWithSimulationControllers(game, players, () -> { final Set allAffectedCards = new HashSet<>(); game.getAction().checkStateEffects(false, allAffectedCards); game.getStack().addAllTriggeredAbilitiesToStack(); @@ -287,21 +287,20 @@ public static void resolveStack(final Game game, final Player preservedPlayer) { }); } - private static void runWithSimulationControllers(final Game game, final Player preservedPlayer, - final List players, final int index, final Runnable proc) { - if (index >= players.size()) { + private static void runWithSimulationControllers(final Game game, final List players, final Runnable proc) { + if (players.isEmpty()) { proc.run(); return; } - Player player = players.get(index); - if (player.equals(preservedPlayer) || player.getController().isAI()) { - runWithSimulationControllers(game, preservedPlayer, players, index + 1, proc); + Player player = players.remove(0); + if (player.getController().isAI()) { + runWithSimulationControllers(game, players, proc); return; } PlayerControllerAi sim = new PlayerControllerAi(game, player, player.getLobbyPlayer()); sim.setUseSimulation(true); - player.runWithController(() -> runWithSimulationControllers(game, preservedPlayer, players, index + 1, proc), sim); + player.runWithController(() -> runWithSimulationControllers(game, players, proc), sim); } public Game getSimulatedGameState() { diff --git a/forge-ai/src/main/java/forge/ai/simulation/GameStateEvaluator.java b/forge-ai/src/main/java/forge/ai/simulation/GameStateEvaluator.java index 720f701c0f2..112c824984b 100644 --- a/forge-ai/src/main/java/forge/ai/simulation/GameStateEvaluator.java +++ b/forge-ai/src/main/java/forge/ai/simulation/GameStateEvaluator.java @@ -59,7 +59,7 @@ private CombatSimResult simulateUpcomingCombatThisTurn(final Game evalGame, fina gameCopy = copier.makeCopy(null, aiPlayer); } - gameCopy.getPhaseHandler().devAdvanceToPhase(PhaseType.COMBAT_DAMAGE, () -> GameSimulator.resolveStack(gameCopy, aiPlayer)); + gameCopy.getPhaseHandler().devAdvanceToPhase(PhaseType.COMBAT_DAMAGE, () -> GameSimulator.resolveStack(gameCopy)); CombatSimResult result = new CombatSimResult(); result.copier = copier; result.gameCopy = gameCopy; diff --git a/forge-gui-desktop/src/test/java/forge/ai/simulation/GameSimulationTest.java b/forge-gui-desktop/src/test/java/forge/ai/simulation/GameSimulationTest.java index b02baf249a2..deb1cf12351 100644 --- a/forge-gui-desktop/src/test/java/forge/ai/simulation/GameSimulationTest.java +++ b/forge-gui-desktop/src/test/java/forge/ai/simulation/GameSimulationTest.java @@ -94,7 +94,7 @@ public CardCollectionView choosePermanentsToSacrifice(SpellAbility ability, int } }; - otherOpponent.runWithController(() -> GameSimulator.resolveStack(game, ai), throwingController); + otherOpponent.runWithController(() -> GameSimulator.resolveStack(game), throwingController); AssertJUnit.assertTrue("The non-AI non-weakest player should have sacrificed during stack resolution", otherOpponent.getZone(ZoneType.Graveyard).contains(otherOpponentBear)); From 98ef7ae0d655c4ddc1447321224b711f8deaa2b5 Mon Sep 17 00:00:00 2001 From: Madwand99 Date: Fri, 1 May 2026 07:16:34 -0700 Subject: [PATCH 4/4] Reverted AiSacrificeDecisionTest --- .../ai/sacrifice/AiSacrificeDecisionTest.java | 159 +++--------------- 1 file changed, 24 insertions(+), 135 deletions(-) diff --git a/forge-gui-desktop/src/test/java/forge/ai/sacrifice/AiSacrificeDecisionTest.java b/forge-gui-desktop/src/test/java/forge/ai/sacrifice/AiSacrificeDecisionTest.java index 76e4e15730b..7d0adf95a8a 100644 --- a/forge-gui-desktop/src/test/java/forge/ai/sacrifice/AiSacrificeDecisionTest.java +++ b/forge-gui-desktop/src/test/java/forge/ai/sacrifice/AiSacrificeDecisionTest.java @@ -1,13 +1,9 @@ package forge.ai.sacrifice; import forge.ai.AITest; -import forge.ai.PlayerControllerAi; import forge.game.Game; import forge.game.card.Card; -import forge.game.card.CardCollection; -import forge.game.card.CardCollectionView; import forge.game.player.Player; -import forge.game.spellability.SpellAbility; import forge.game.zone.ZoneType; import org.testng.AssertJUnit; import org.testng.annotations.Test; @@ -17,90 +13,6 @@ */ public class AiSacrificeDecisionTest extends AITest { - @Test - public void testSimulationSacrificeChoiceIsDeterministic() { - Game game = initAndCreateGame(); - - Player ai = game.getPlayers().get(1); - SpellAbility source = sacrificeSource(ai); - - Card bear = addCard("Runeclaw Bear", ai); - addCard("Serra Angel", ai); - addCard("Colossal Dreadmaw", ai); - - CardCollection validTargets = creatures(ai); - - for (int i = 0; i < 5; i++) { - CardCollectionView chosen = chooseWithSimulationController(game, ai, source, 1, validTargets); - - AssertJUnit.assertEquals("Simulation sacrifice choice should be stable", 1, chosen.size()); - AssertJUnit.assertEquals("AI should consistently choose the least valuable creature", - bear, chosen.get(0)); - } - } - - @Test - public void testSimulationSacrificeChoiceUsesOnlyLegalCandidates() { - Game game = initAndCreateGame(); - - Player ai = game.getPlayers().get(1); - SpellAbility source = sacrificeSource(ai); - - addCard("Runeclaw Bear", ai); - Card angel = addCard("Serra Angel", ai); - Card dreadmaw = addCard("Colossal Dreadmaw", ai); - - CardCollection validTargets = new CardCollection(); - validTargets.add(angel); - validTargets.add(dreadmaw); - - CardCollectionView chosen = chooseWithSimulationController(game, ai, source, 1, validTargets); - - AssertJUnit.assertEquals("AI should choose one legal creature to sacrifice", 1, chosen.size()); - AssertJUnit.assertEquals("AI should choose the least valuable legal creature", - angel, chosen.get(0)); - } - - @Test - public void testSimulationSacrificeChoiceChoosesWorstCreaturesFirst() { - Game game = initAndCreateGame(); - - Player ai = game.getPlayers().get(1); - SpellAbility source = sacrificeSource(ai); - - Card bear = addCard("Runeclaw Bear", ai); - Card angel = addCard("Serra Angel", ai); - Card dreadmaw = addCard("Colossal Dreadmaw", ai); - - CardCollectionView chosen = chooseWithSimulationController(game, ai, source, 2, creatures(ai)); - - AssertJUnit.assertEquals("AI should choose the requested number of creatures", 2, chosen.size()); - AssertJUnit.assertTrue("AI should sacrifice Runeclaw Bear before better creatures", chosen.contains(bear)); - AssertJUnit.assertTrue("AI should sacrifice Serra Angel before Colossal Dreadmaw", chosen.contains(angel)); - AssertJUnit.assertFalse("AI should preserve the most valuable creature when sacrificing two of three", - chosen.contains(dreadmaw)); - } - - @Test - public void testSimulationSacrificeChoiceCanChooseLeastValuablePermanent() { - Game game = initAndCreateGame(); - - Player ai = game.getPlayers().get(1); - SpellAbility source = sacrificeSource(ai); - - Card ornithopter = addCard("Ornithopter", ai); - addCard("Sol Ring", ai); - addCard("Serra Angel", ai); - - CardCollection validTargets = new CardCollection(ai.getCardsIn(ZoneType.Battlefield)); - - CardCollectionView chosen = chooseWithSimulationController(game, ai, source, 1, validTargets); - - AssertJUnit.assertEquals("AI should choose one permanent to sacrifice", 1, chosen.size()); - AssertJUnit.assertEquals("AI should choose the cheapest artifact/enchantment in a mixed permanent set", - ornithopter, chosen.get(0)); - } - @Test public void testAiSacrificesCreatureToAvoidLethalLifeLoss() { Game game = initAndCreateGame(); @@ -121,7 +33,11 @@ public void testAiSacrificesCreatureToAvoidLethalLifeLoss() { // Opponent controls a creature they can sacrifice Card adelbert = addCard("Adelbert Steiner", opponent); - fillLibraries(p, opponent, 10); + // Fill libraries to prevent draw-death + for (int i = 0; i < 10; i++) { + addCardToZone("Plains", p, ZoneType.Library); + addCardToZone("Plains", opponent, ZoneType.Library); + } // Play until next turn (after Fandaniel's end step trigger resolves) this.playUntilNextTurn(game); @@ -129,8 +45,9 @@ public void testAiSacrificesCreatureToAvoidLethalLifeLoss() { // The game should NOT be over - opponent should have sacrificed to survive AssertJUnit.assertFalse("Game should not be over - AI should have sacrificed to survive", game.isGameOver()); - assertInZone(opponent, adelbert, ZoneType.Graveyard, - "Adelbert Steiner should be in graveyard after being sacrificed"); + // Adelbert should be in graveyard (sacrificed) + AssertJUnit.assertTrue("Adelbert Steiner should be in graveyard after being sacrificed", + opponent.getZone(ZoneType.Graveyard).contains(adelbert)); // Opponent should still have 1 life (didn't lose life because they sacrificed) AssertJUnit.assertEquals("Opponent should still have 1 life after sacrificing", 1, opponent.getLife()); @@ -157,7 +74,11 @@ public void testAiDoesNotSacrificeWhenLifeLossIsNotLethal() { // Opponent controls a creature Card adelbert = addCard("Adelbert Steiner", opponent); - fillLibraries(p, opponent, 10); + // Fill libraries + for (int i = 0; i < 10; i++) { + addCardToZone("Plains", p, ZoneType.Library); + addCardToZone("Plains", opponent, ZoneType.Library); + } // Play until next turn (after Fandaniel's end step trigger resolves) this.playUntilNextTurn(game); @@ -165,8 +86,9 @@ public void testAiDoesNotSacrificeWhenLifeLossIsNotLethal() { // Game should not be over AssertJUnit.assertFalse("Game should not be over", game.isGameOver()); - assertInZone(opponent, adelbert, ZoneType.Battlefield, - "Adelbert Steiner should still be on battlefield when life loss isn't lethal"); + // Adelbert should still be on battlefield (not sacrificed) + AssertJUnit.assertTrue("Adelbert Steiner should still be on battlefield when life loss isn't lethal", + opponent.getZone(ZoneType.Battlefield).contains(adelbert)); // Opponent should have 8 life (lost 2 from not sacrificing) AssertJUnit.assertEquals("Opponent should have lost 2 life from not sacrificing", 8, opponent.getLife()); @@ -190,7 +112,11 @@ public void testAiSacrificesToPillarTombsToAvoidLethalLifeLoss() { // Opponent controls a creature they can sacrifice Card bear = addCard("Runeclaw Bear", opponent); - fillLibraries(p, opponent, 10); + // Fill libraries to prevent draw-death + for (int i = 0; i < 10; i++) { + addCardToZone("Plains", p, ZoneType.Library); + addCardToZone("Plains", opponent, ZoneType.Library); + } // Play two turns - first ends Player 1's turn, second plays through opponent's upkeep // where Pillar Tombs triggers and the AI must decide whether to sacrifice @@ -200,48 +126,11 @@ public void testAiSacrificesToPillarTombsToAvoidLethalLifeLoss() { // The game should NOT be over - AI should have sacrificed to survive AssertJUnit.assertFalse("Game should not be over - AI should have sacrificed to survive", game.isGameOver()); - assertInZone(opponent, bear, ZoneType.Graveyard, - "Runeclaw Bear should be in graveyard after being sacrificed"); + // Bear should be in graveyard (sacrificed) + AssertJUnit.assertTrue("Runeclaw Bear should be in graveyard after being sacrificed", + opponent.getZone(ZoneType.Graveyard).contains(bear)); // Opponent should still have 4 life (didn't lose life because they sacrificed) AssertJUnit.assertEquals("Opponent should still have 4 life after sacrificing", 4, opponent.getLife()); } - - private SpellAbility sacrificeSource(Player ai) { - SpellAbility source = createCard("Innocent Blood", ai).getFirstSpellAbility(); - source.setActivatingPlayer(ai); - return source; - } - - private CardCollection creatures(Player player) { - CardCollection creatures = new CardCollection(); - for (Card card : player.getCardsIn(ZoneType.Battlefield)) { - if (card.isCreature()) { - creatures.add(card); - } - } - return creatures; - } - - private void fillLibraries(Player first, Player second, int count) { - for (int i = 0; i < count; i++) { - addCardToZone("Plains", first, ZoneType.Library); - addCardToZone("Plains", second, ZoneType.Library); - } - } - - private void assertInZone(Player player, Card card, ZoneType zone, String message) { - AssertJUnit.assertTrue(message, player.getZone(zone).contains(card)); - } - - private CardCollectionView chooseWithSimulationController(Game game, Player ai, SpellAbility source, - int amount, CardCollectionView validTargets) { - PlayerControllerAi controller = new PlayerControllerAi(game, ai, ai.getLobbyPlayer()); - controller.setUseSimulation(true); - - final CardCollectionView[] chosen = new CardCollectionView[1]; - ai.runWithController(() -> chosen[0] = ai.getController().choosePermanentsToSacrifice( - source, amount, amount, validTargets, "Choose permanents to sacrifice"), controller); - return chosen[0]; - } }