Fix AI cloning creatures that shrink under its own control

forge-ai/src/main/java/forge/ai/ComputerUtilCard.java

@@ -14,12 +14,14 @@
 import java.util.stream.Stream;
 
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.mutable.MutableBoolean;
 import org.apache.commons.lang3.tuple.MutablePair;
 import org.apache.commons.lang3.tuple.Pair;
 
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
 
 import forge.StaticData;
 import forge.ai.simulation.GameStateEvaluator;
@@ -219,6 +221,34 @@ public static Card getBestEnchantmentAI(final List<Card> list, final SpellAbilit
      * @param list
      * @return a {@link forge.game.card.Card} object.
      */
+
+    /**
+     * Drops creatures whose copy would arrive dead under {@code newController}. A toughness-setting
+     * characteristic-defining ability reads its controller's board, so an opponent's 8/8 can be a
+     * 0/0 for us. Only cards with such an ability are priced, the rest are kept untouched.
+     */
+    public static CardCollection filterOutFatalCopies(final Iterable<Card> list, final Player newController) {
+        final Game game = newController.getGame();
+        final MutableBoolean reset = new MutableBoolean(false);
+        final CardCollection kept = CardLists.filter(list, c -> {
+            if (!c.isCreature() || c.getController().equals(newController)) {
+                return true;
+            }
+            if (c.getStaticAbilities().stream().anyMatch(st -> st.isCharacteristicDefining() && st.hasParam("SetToughness"))) {
+                final Card copy = CardCopyService.getLKICopy(c);
+                copy.setController(newController, game.getNextTimestamp());
+                game.getAction().checkStaticAbilities(false, Sets.newHashSet(copy), new CardCollection(copy));
+                reset.setTrue();
+                return copy.getNetToughness() >= 1;
+            }
+            return true;
+        });
+        if (reset.isTrue()) {
+            game.getAction().checkStaticAbilities(false);
+        }
+        return kept;
+    }
+
     public static Card getBestLandAI(final Iterable<Card> list) {
         final List<Card> land = CardLists.filter(list, CardPredicates.LANDS);
         if (land.isEmpty()) {

forge-ai/src/main/java/forge/ai/ability/CloneAi.java

@@ -132,8 +132,7 @@ private boolean cloneTgtAI(final SpellAbility sa, boolean mandatory) {
         }
 
         if (mandatory || "CloneBestCreature".equals(sa.getParam("AILogic"))) {
-            sa.getTargets().add(ComputerUtilCard.getBestCreatureAI(targets));
-            return true;
+            return cloneBestTarget(sa, targets, mandatory);
         }
 
         // Default:
@@ -144,6 +143,27 @@ private boolean cloneTgtAI(final SpellAbility sa, boolean mandatory) {
         return false;
     }
 
+    /**
+     * A creature is evaluated under whoever controls it now, but the copy arrives under ours: an
+     * opponent's Nightmare is an 8/8 for them and a 0/0 for us.
+     */
+    private boolean cloneBestTarget(final SpellAbility sa, final List<Card> targets, final boolean mandatory) {
+        if (targets.isEmpty()) {
+            return false;
+        }
+        final Player self = sa.getActivatingPlayer();
+        CardCollection viable = ComputerUtilCard.filterOutFatalCopies(targets, self);
+        if (viable.isEmpty()) {
+            if (!mandatory) {
+                return false;
+            }
+            // no choice about it, so take the one the evaluation liked best
+            viable = new CardCollection(targets);
+        }
+        sa.getTargets().add(ComputerUtilCard.getBestCreatureAI(viable));
+        return true;
+    }
+
     /* (non-Javadoc)
      * @see forge.card.ability.SpellAbilityAi#confirmAction(forge.game.player.Player, forge.card.spellability.SpellAbility, forge.game.player.PlayerActionConfirmMode, java.lang.String)
      */

forge-ai/src/main/java/forge/ai/ability/CopyPermanentAi.java

@@ -1,7 +1,6 @@
 package forge.ai.ability;
 
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Sets;
 import forge.ai.*;
 import forge.game.Game;
 import forge.game.GameEntity;
@@ -15,7 +14,6 @@
 import forge.game.player.PlayerCollection;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.ZoneType;
-import org.apache.commons.lang3.mutable.MutableBoolean;
 
 import java.util.Collection;
 import java.util.List;
@@ -182,25 +180,9 @@ protected AiAbilityDecision doTriggerNoCost(final Player aiPlayer, SpellAbility
                     if (sa.hasParam("TargetingPlayer")) {
                         choice = ComputerUtilCard.getWorstCreatureAI(list);
                     } else {
-                        MutableBoolean reset = new MutableBoolean(false);
-                        list = CardLists.filter(list, c -> {
-                            if (!c.isCreature() || c.getController().equals(aiPlayer)) {
-                                return true;
-                            }
-                            if (!sa.hasParam("SetToughness") && c.getStaticAbilities().stream().anyMatch(st -> st.isCharacteristicDefining() && st.hasParam("SetToughness"))) {
-                                // if the copy has a toughness-setting CDA better check it doesn't die under our control
-                                final Card copy = CardCopyService.getLKICopy(c);
-                                copy.setController(aiPlayer, game.getNextTimestamp());
-                                game.getAction().checkStaticAbilities(false, Sets.newHashSet(copy), new CardCollection(copy));
-                                reset.setTrue();
-                                if (copy.getNetToughness() < 1) {
-                                    return false;
-                                }
-                            }
-                            return true;
-                        });
-                        if (reset.isTrue()) {
-                            game.getAction().checkStaticAbilities(false);
+                        if (!sa.hasParam("SetToughness")) {
+                            // the copy would land under our control, where a toughness-setting CDA reads a different board
+                            list = ComputerUtilCard.filterOutFatalCopies(list, aiPlayer);
                         }
                         choice = ComputerUtilCard.getBestCreatureAI(list);
                     }

forge-gui-desktop/src/test/java/forge/ai/ability/CloneOpponentCreatureAiTest.java

@@ -0,0 +1,86 @@
+package forge.ai.ability;
+
+import org.testng.annotations.Test;
+
+import forge.ai.AITest;
+import forge.game.Game;
+import forge.game.card.Card;
+import forge.game.phase.PhaseType;
+import forge.game.player.Player;
+import forge.game.zone.ZoneType;
+
+import static org.testng.AssertJUnit.assertEquals;
+
+/**
+ * A creature is evaluated under its current controller, so a characteristic-defining body is worth
+ * a different amount once the copy arrives under us.
+ */
+public class CloneOpponentCreatureAiTest extends AITest {
+
+    private Card upkeep(Game game, Player ai, Player opp) {
+        Card crypto = addCard("Cryptoplasm", ai);
+        crypto.setSickness(false);
+        addCards("Island", 5, ai);
+        for (Player p : game.getPlayers()) {
+            fillLibrary(p, 20);
+        }
+        game.getPhaseHandler().devModeSet(PhaseType.END_OF_TURN, opp);
+        game.getAction().checkStateEffects(true);
+        return crypto;
+    }
+
+    private void run(Game game) {
+        playUntilPhase(game, PhaseType.MAIN1);
+        playUntilStackClear(game);
+    }
+
+    @Test
+    public void doesNotCopyABodyThatOnlyExistsOnTheirBoard() {
+        Game game = initAndCreateGame();
+        Player ai = game.getPlayers().get(1);
+        Player opp = game.getPlayers().get(0);
+
+        // 8/8 for them, 0/0 for us: copying it is suicide
+        addCard("Nightmare", opp);
+        addCards("Swamp", 8, opp);
+        Card crypto = upkeep(game, ai, opp);
+
+        run(game);
+
+        assertEquals("still ours and alive", ZoneType.Battlefield, crypto.getZone().getZoneType());
+        assertEquals(0, countCardsWithName(game, "Nightmare", ZoneType.Graveyard));
+    }
+
+    @Test
+    public void takesTheNextBestWhenTheTopPickIsATrap() {
+        Game game = initAndCreateGame();
+        Player ai = game.getPlayers().get(1);
+        Player opp = game.getPlayers().get(0);
+
+        addCard("Nightmare", opp);
+        addCards("Swamp", 8, opp);
+        addCard("Serra Angel", opp);
+        Card crypto = upkeep(game, ai, opp);
+
+        run(game);
+
+        assertEquals("Serra Angel", crypto.getName());
+        assertEquals(4, crypto.getNetPower());
+        assertEquals(ZoneType.Battlefield, crypto.getZone().getZoneType());
+    }
+
+    @Test
+    public void ordinaryBoardStillCopiesTheBest() {
+        Game game = initAndCreateGame();
+        Player ai = game.getPlayers().get(1);
+        Player opp = game.getPlayers().get(0);
+
+        addCard("Serra Angel", opp);
+        addCard("Grizzly Bears", opp);
+        Card crypto = upkeep(game, ai, opp);
+
+        run(game);
+
+        assertEquals("Serra Angel", crypto.getName());
+    }
+}