Merge pull request #1786 from asvitkine/fix_lands

Fix simulation AI logic related to playing lands.

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);
             }
 

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<SpellAbility> 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) {
+        if (sa instanceof LandAbility) {
-            return false;
+            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() : ""); }
-    }
 }

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.assertTrue(simGoblin.isRed() && simGoblin.isBlack());
-        AssertJUnit.assertEquals(true, simGoblin.getType().hasSubtype("Zombie"));
+        AssertJUnit.assertTrue(simGoblin.getType().hasSubtype("Zombie"));
     }
 
     @Test

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,46 @@ 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());
+
+        // The plan should involve playing Tatyova first and then playing a land, to benefit from
+        // the landfall trigger.
+        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 +310,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());
     }