From b0120eedf06a42f50d7a0af4df6d8518c5507b84 Mon Sep 17 00:00:00 2001 From: asvitkine Date: Wed, 2 Nov 2022 10:26:37 -0400 Subject: [PATCH] Fix simulation AI logic related to playing lands. This got broken by the following refactor: https://github.com/Card-Forge/forge/commit/79c9c914e21521c03ed219954810105fac3a639a The result was that simulation AI was ignoring certain decision trees that involved playing lands, leading to not considering certain lines of play and some log messages printed to standard error. I've added a test that covers this logic to prevent it breaking again in the future. Also, a couple small clean ups to related tests. --- .../forge/ai/simulation/GameSimulator.java | 5 +-- .../ai/simulation/SpellAbilityPicker.java | 17 +++------ .../ai/simulation/GameSimulationTest.java | 4 +-- .../SpellAbilityPickerSimulationTest.java | 36 +++++++++++++++++-- 4 files changed, 42 insertions(+), 20 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 a3208e5e6ce..0cbd88374b5 100644 --- a/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java +++ b/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java @@ -1,5 +1,6 @@ package forge.ai.simulation; +import forge.game.spellability.LandAbility; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; @@ -154,7 +155,7 @@ public class GameSimulator { } public Score simulateSpellAbility(SpellAbility origSa, GameStateEvaluator eval) { SpellAbility sa; - if (origSa instanceof SpellAbilityPicker.PlayLandAbility) { + if (origSa instanceof LandAbility) { Card hostCard = (Card) copier.find(origSa.getHostCard()); if (!aiPlayer.playLand(hostCard, false)) { System.err.println("Simulation: Couldn't play land! " + origSa); @@ -164,7 +165,7 @@ public class GameSimulator { // TODO: optimize: prune identical SA (e.g. two of the same card in hand) sa = findSaInSimGame(origSa); if (sa == null) { - System.err.println("Simulation: SA not found! " + origSa); + System.err.println("Simulation: SA not found! " + origSa + " / " + origSa.getClass()); return new Score(Integer.MIN_VALUE); } diff --git a/forge-ai/src/main/java/forge/ai/simulation/SpellAbilityPicker.java b/forge-ai/src/main/java/forge/ai/simulation/SpellAbilityPicker.java index 11921d1c5c9..1b955c85336 100644 --- a/forge-ai/src/main/java/forge/ai/simulation/SpellAbilityPicker.java +++ b/forge-ai/src/main/java/forge/ai/simulation/SpellAbilityPicker.java @@ -74,7 +74,7 @@ public class SpellAbilityPicker { continue; } landsDeDupe.put(land.getName(), land); - all.add(new PlayLandAbility(land)); + all.add(new LandAbility(land)); } } List candidateSAs = ComputerUtilAbility.getOriginalAndAltCostAbilities(all, player); @@ -147,8 +147,8 @@ public class SpellAbilityPicker { private static boolean isSorcerySpeed(SpellAbility sa, Player player) { // TODO: Can we use the actual rules engine for this instead of trying to do the logic ourselves? - if (sa instanceof PlayLandAbility) { - return false; + if (sa instanceof LandAbility) { + return true; } if (sa.isSpell()) { return !sa.withFlash(sa.getHostCard(), player); @@ -334,7 +334,7 @@ public class SpellAbilityPicker { } private AiPlayDecision canPlayAndPayForSim(final SpellAbility sa) { - if (sa instanceof PlayLandAbility) { + if (sa instanceof LandAbility) { return AiPlayDecision.WillPlay; } if (!sa.canPlay()) { @@ -452,13 +452,4 @@ public class SpellAbilityPicker { } return ComputerUtil.chooseSacrificeType(player, type, ability, ability.getTargetCard(), effect, amount, exclude); } - - public static class PlayLandAbility extends LandAbility { - public PlayLandAbility(Card land) { - super(land); - } - - @Override - public String toUnsuppressedString() { return "Play land " + (getHostCard() != null ? getHostCard().getName() : ""); } - } } 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 d5a8bb8d9ea..f7de3b03591 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 @@ -2242,8 +2242,8 @@ public class GameSimulationTest extends SimulationTest { simGame.getPhaseHandler().devAdvanceToPhase(PhaseType.MAIN2); AssertJUnit.assertEquals(21, simGame.getPlayers().get(0).getLife()); - AssertJUnit.assertEquals(true, simGoblin.isRed() && simGoblin.isBlack()); - AssertJUnit.assertEquals(true, simGoblin.getType().hasSubtype("Zombie")); + AssertJUnit.assertTrue(simGoblin.isRed() && simGoblin.isBlack()); + AssertJUnit.assertTrue(simGoblin.getType().hasSubtype("Zombie")); } @Test diff --git a/forge-gui-desktop/src/test/java/forge/ai/simulation/SpellAbilityPickerSimulationTest.java b/forge-gui-desktop/src/test/java/forge/ai/simulation/SpellAbilityPickerSimulationTest.java index df7cef72064..d51bf8968b6 100644 --- a/forge-gui-desktop/src/test/java/forge/ai/simulation/SpellAbilityPickerSimulationTest.java +++ b/forge-gui-desktop/src/test/java/forge/ai/simulation/SpellAbilityPickerSimulationTest.java @@ -1,5 +1,6 @@ package forge.ai.simulation; +import forge.game.spellability.LandAbility; import java.util.List; import org.testng.AssertJUnit; @@ -79,16 +80,44 @@ public class SpellAbilityPickerSimulationTest extends SimulationTest { SpellAbilityPicker picker = new SpellAbilityPicker(game, p); SpellAbility sa = picker.chooseSpellAbilityToPlay(null); - // assertEquals(game.PLAY_LAND_SURROGATE, sa); + AssertJUnit.assertTrue(sa instanceof LandAbility); AssertJUnit.assertEquals(mountain, sa.getHostCard()); Plan plan = picker.getPlan(); AssertJUnit.assertEquals(2, plan.getDecisions().size()); - AssertJUnit.assertEquals("Play land Mountain", plan.getDecisions().get(0).saRef.toString()); + AssertJUnit.assertEquals("Mountain (1) -> Play land", plan.getDecisions().get(0).saRef.toString(true)); AssertJUnit.assertEquals("Shock deals 2 damage to any target.", plan.getDecisions().get(1).saRef.toString()); AssertJUnit.assertTrue(plan.getDecisions().get(1).targets.toString().contains("Runeclaw Bear")); } + @Test + public void testPlayingLandAfterSpell() { + Game game = initAndCreateGame(); + Player p = game.getPlayers().get(1); + + addCard("Island", p); + addCard("Island", p); + addCard("Forest", p); + addCard("Forest", p); + addCard("Forest", p); + + Card tatyova = addCardToZone("Tatyova, Benthic Druid", p, ZoneType.Hand); + addCardToZone("Forest", p, ZoneType.Hand); + addCardToZone("Forest", p, ZoneType.Library); + + game.getPhaseHandler().devModeSet(PhaseType.MAIN1, p); + game.getAction().checkStateEffects(true); + + SpellAbilityPicker picker = new SpellAbilityPicker(game, p); + SpellAbility sa = picker.chooseSpellAbilityToPlay(null); + AssertJUnit.assertEquals(tatyova, sa.getHostCard()); + + Plan plan = picker.getPlan(); + AssertJUnit.assertEquals(2, plan.getDecisions().size()); + AssertJUnit.assertEquals("Tatyova, Benthic Druid - Creature 3 / 3", plan.getDecisions().get(0).saRef.toString()); + AssertJUnit.assertEquals("Play land", plan.getDecisions().get(1).saRef.toString()); + } + @Test public void testModeSelection() { Game game = initAndCreateGame(); @@ -279,7 +308,8 @@ public class SpellAbilityPickerSimulationTest extends SimulationTest { AssertJUnit.assertEquals(abbot.getSpellAbilities().get(0), sa); Plan plan = picker.getPlan(); AssertJUnit.assertEquals(3, plan.getDecisions().size()); - AssertJUnit.assertEquals("Play land Mountain", plan.getDecisions().get(1).saRef.toString()); + AssertJUnit.assertEquals("Mountain (5) -> Play land by Abbot of Keral Keep (3)", + plan.getDecisions().get(1).saRef.toString(true)); AssertJUnit.assertEquals("Lightning Bolt deals 3 damage to any target.", plan.getDecisions().get(2).saRef.toString()); }