AI: fixed game freeze on free cast of multiple cards (part of #13638, #13766);

refactor: fixed that TargetCard doesn't work with Zone.ALL;

Mage.Common/src/main/java/mage/utils/testers/GetMultiAmountTestableDialog.java

@@ -53,7 +53,9 @@ class GetMultiAmountTestableDialog extends BaseTestableDialog {
     }
 
     private GetMultiAmountTestableDialog aiMustChoose(Integer... needValues) {
-        // TODO: AI use default distribution (min possible values), improve someday
+        // TODO: AI use default distribution:
+        // - bad effect: min possible values
+        // - good effect: max possible and distributed values
         MultiAmountTestableResult res = ((MultiAmountTestableResult) this.getResult());
         res.aiAssertEnabled = true;
         res.aiAssertValues = Arrays.stream(needValues).collect(Collectors.toList());

Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/ComputerPlayer6.java

@@ -30,10 +30,7 @@ import mage.players.Player;
 import mage.target.Target;
 import mage.target.TargetAmount;
 import mage.target.TargetCard;
-import mage.util.CardUtil;
+import mage.util.*;
-import mage.util.RandomUtil;
-import mage.util.ThreadUtils;
-import mage.util.XmageThreadFactory;
 import org.apache.log4j.Logger;
 
 import java.util.*;
@@ -453,6 +450,7 @@ public class ComputerPlayer6 extends ComputerPlayer {
             }
         } catch (TimeoutException | InterruptedException e) {
             // AI thinks too long
+            // how-to fix: look at stack info - it can contain bad ability with infinite choose dialog
             logger.warn("AI player thinks too long - " + getName() + " - " + root.game);
             task.cancel(true);
         } catch (ExecutionException e) {

Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/PossibleTargetsSelector.java

@@ -45,9 +45,8 @@ public class PossibleTargetsSelector {
 
     public void findNewTargets(Set<UUID> fromTargetsList) {
         // collect new valid targets
-        List<MageItem> found = target.possibleTargets(abilityControllerId, source, game).stream()
+        List<MageItem> found = target.possibleTargets(abilityControllerId, source, game, fromTargetsList).stream()
                 .filter(id -> !target.contains(id))
-                .filter(id -> fromTargetsList == null || fromTargetsList.contains(id))
                 .filter(id -> target.canTarget(abilityControllerId, id, source, game))
                 .map(id -> {
                     Player player = game.getPlayer(id);

Mage.Sets/src/mage/cards/b/BoosterTutor.java

@@ -91,7 +91,7 @@ class BoosterTutorEffect extends OneShotEffect {
                     game.loadCards(cardsToLoad, controller.getId());
                     CardsImpl cards = new CardsImpl();
                     cards.addAllCards(boosterPack);
-                    if (controller.choose(Outcome.Benefit, cards, targetCard, source, game)) {
+                    if (controller.choose(Outcome.PutCardInPlay, cards, targetCard, source, game)) {
                         Card card = game.getCard(targetCard.getFirstTarget());
                         if (card != null) {
                             controller.moveCards(card, Zone.HAND, source, game);

Mage.Sets/src/mage/cards/m/MonsterManual.java

@@ -84,7 +84,7 @@ class ZoologicalStudyEffect extends OneShotEffect {
                 break;
             default:
                 TargetCard target = new TargetCard(Zone.ALL, StaticFilters.FILTER_CARD_CREATURE);
-                player.choose(outcome, cards, target, source, game);
+                player.choose(Outcome.PutCardInPlay, cards, target, source, game);
                 card = cards.get(target.getFirstTarget(), game);
         }
         player.moveCards(card, Zone.HAND, source, game);

Mage.Tests/src/test/java/org/mage/test/cards/asthough/PlayTopCardFromLibraryTest.java

@@ -2,13 +2,15 @@ package org.mage.test.cards.asthough;
 
 import mage.constants.PhaseStep;
 import mage.constants.Zone;
+import mage.player.ai.ComputerPlayer7;
+import org.junit.Ignore;
 import org.junit.Test;
-import org.mage.test.serverside.base.CardTestPlayerBase;
+import org.mage.test.serverside.base.CardTestPlayerBaseWithAIHelps;
 
 /**
  * @author JayDi85
  */
-public class PlayTopCardFromLibraryTest extends CardTestPlayerBase {
+public class PlayTopCardFromLibraryTest extends CardTestPlayerBaseWithAIHelps {
 
     /*
     Bolas's Citadel
@@ -218,4 +220,146 @@ public class PlayTopCardFromLibraryTest extends CardTestPlayerBase {
         assertGraveyardCount(playerA, "Balduvian Bears", 0);
         assertPermanentCount(playerA, "Balduvian Bears", 1);
     }
+
+    @Test
+    public void test_EtaliPrimalStorm_NoCards_Manual() {
+        removeAllCardsFromLibrary(playerA);
+        removeAllCardsFromLibrary(playerB);
+
+        // Whenever Etali, Primal Storm attacks, exile the top card of each player's library,
+        // then you may cast any number of nonland cards exiled this way without paying their mana costs.
+        addCard(Zone.BATTLEFIELD, playerA, "Etali, Primal Storm", 1); // 6/6
+        //
+        addCard(Zone.LIBRARY, playerA, "Forest", 1);
+        addCard(Zone.LIBRARY, playerB, "Forest", 1);
+
+        // nothing to free cast
+        attack(1, playerA, "Etali, Primal Storm");
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+
+        assertLife(playerB, 20 - 6);
+    }
+
+    @Test
+    public void test_EtaliPrimalStorm_NoCards_AI() {
+        removeAllCardsFromLibrary(playerA);
+        removeAllCardsFromLibrary(playerB);
+
+        // Whenever Etali, Primal Storm attacks, exile the top card of each player's library,
+        // then you may cast any number of nonland cards exiled this way without paying their mana costs.
+        addCard(Zone.BATTLEFIELD, playerA, "Etali, Primal Storm", 1); // 6/6
+        //
+        addCard(Zone.LIBRARY, playerA, "Forest", 1);
+        addCard(Zone.LIBRARY, playerB, "Forest", 1);
+
+        // ai must attack and nothing to free cast
+        attack(1, playerA, "Etali, Primal Storm");
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+
+        assertLife(playerB, 20 - 6);
+    }
+
+    @Test
+    public void test_EtaliPrimalStorm_OneCard_Manual() {
+        removeAllCardsFromLibrary(playerA);
+        removeAllCardsFromLibrary(playerB);
+
+        // Whenever Etali, Primal Storm attacks, exile the top card of each player's library,
+        // then you may cast any number of nonland cards exiled this way without paying their mana costs.
+        addCard(Zone.BATTLEFIELD, playerA, "Etali, Primal Storm", 1); // 6/6
+        //
+        addCard(Zone.LIBRARY, playerA, "Lightning Bolt", 1);
+        addCard(Zone.LIBRARY, playerB, "Forest", 1);
+
+        attack(1, playerA, "Etali, Primal Storm");
+        setChoice(playerA, true); // use free cast
+        addTarget(playerA, playerB); // to damage
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+
+        assertLife(playerB, 20 - 6 - 3);
+    }
+
+    @Test
+    public void test_EtaliPrimalStorm_OneCard_AI() {
+        removeAllCardsFromLibrary(playerA);
+        removeAllCardsFromLibrary(playerB);
+
+        // Whenever Etali, Primal Storm attacks, exile the top card of each player's library,
+        // then you may cast any number of nonland cards exiled this way without paying their mana costs.
+        addCard(Zone.BATTLEFIELD, playerA, "Etali, Primal Storm", 1); // 6/6
+        //
+        addCard(Zone.LIBRARY, playerA, "Lightning Bolt", 1);
+        addCard(Zone.LIBRARY, playerB, "Forest", 1);
+
+        // ai must attack and free cast bolt to opponent's
+        aiPlayStep(1, PhaseStep.DECLARE_ATTACKERS, playerA);
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+
+        assertLife(playerB, 20 - 6 - 3);
+    }
+
+    @Test
+    public void test_EtaliPrimalStorm_MultipleCards_Manual() {
+        removeAllCardsFromLibrary(playerA);
+        removeAllCardsFromLibrary(playerB);
+
+        // Whenever Etali, Primal Storm attacks, exile the top card of each player's library,
+        // then you may cast any number of nonland cards exiled this way without paying their mana costs.
+        addCard(Zone.BATTLEFIELD, playerA, "Etali, Primal Storm", 1); // 6/6
+        //
+        addCard(Zone.LIBRARY, playerA, "Lightning Bolt", 1); // 3 damage
+        addCard(Zone.LIBRARY, playerB, "Cleansing Screech", 1); // 4 damage
+
+        // choose cards one by one
+        attack(1, playerA, "Etali, Primal Storm");
+        // first card
+        setChoice(playerA, "Cleansing Screech");
+        setChoice(playerA, true); // use free
+        addTarget(playerA, playerB);
+        // last card (auto-chosen)
+        setChoice(playerA, true); // use free
+        addTarget(playerA, playerB);
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+
+        assertLife(playerB, 20 - 6 - 4 - 3);
+    }
+
+    @Test
+    public void test_EtaliPrimalStorm_MultipleCards_AI() {
+        removeAllCardsFromLibrary(playerA);
+        removeAllCardsFromLibrary(playerB);
+
+        // Whenever Etali, Primal Storm attacks, exile the top card of each player's library,
+        // then you may cast any number of nonland cards exiled this way without paying their mana costs.
+        addCard(Zone.BATTLEFIELD, playerA, "Etali, Primal Storm", 1); // 6/6
+        //
+        addCard(Zone.LIBRARY, playerA, "Lightning Bolt", 1); // 3 damage
+        addCard(Zone.LIBRARY, playerB, "Cleansing Screech", 1); // 4 damage
+
+        // ai must attack and free cast two cards
+        // possible bug 1: game freeze due wrong dialog/selection logic
+        // possible bug 2: TargetCard can't find ALL zone
+        aiPlayStep(1, PhaseStep.DECLARE_ATTACKERS, playerA);
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+
+        assertLife(playerB, 20 - 6 - 4 - 3);
+    }
 }

Mage.Tests/src/test/java/org/mage/test/cards/control/ExileAndReturnUnderYourControlTest.java

@@ -183,12 +183,13 @@ public class ExileAndReturnUnderYourControlTest extends CardTestPlayerBase {
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Villainous Wealth", playerB);
         setChoice(playerA, "X=3");
 
+        // first card
         setChoice(playerA, "Mox Emerald");
         setChoice(playerA, "Yes");
-
+        // second card
         setChoice(playerA, "Mox Sapphire");
         setChoice(playerA, "Yes");
-
+        // last card
         // Quicken is auto-chosen since it's the last of the 3 cards. Only need to say Yes to casting for free.
         setChoice(playerA, "Yes");
 

Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java

@@ -2102,12 +2102,12 @@ public class TestPlayer implements Player {
         return "Ability: null";
     }
 
-    private String getInfo(Target target, Ability source, Game game) {
+    private String getInfo(Target target, Ability source, Game game, Cards cards) {
         if (target == null) {
             return "Target: null";
         }
         UUID abilityControllerId = target.getAffectedAbilityControllerId(this.getId());
-        Set<UUID> possibleTargets = target.possibleTargets(abilityControllerId, source, game);
+        Set<UUID> possibleTargets = target.possibleTargets(abilityControllerId, source, game, cards);
 
         return "Target: selected " + target.getSize() + ", possible " + possibleTargets.size()
                 + ", " + target.getClass().getSimpleName() + ": " + target.getMessage(game);
@@ -2480,7 +2480,7 @@ public class TestPlayer implements Player {
             }
         }
 
-        this.chooseStrictModeFailed("choice", game, getInfo(source, game) + "\n" + getInfo(target, source, game));
+        this.chooseStrictModeFailed("choice", game, getInfo(source, game) + "\n" + getInfo(target, source, game, null));
         return computerPlayer.choose(outcome, target, source, game, options);
     }
 
@@ -2806,7 +2806,7 @@ public class TestPlayer implements Player {
             Assert.fail(message);
         }
 
-        this.chooseStrictModeFailed("target", game, getInfo(source, game) + "\n" + getInfo(target, source, game));
+        this.chooseStrictModeFailed("target", game, getInfo(source, game) + "\n" + getInfo(target, source, game, null));
         return computerPlayer.chooseTarget(outcome, target, source, game);
     }
 
@@ -2849,7 +2849,7 @@ public class TestPlayer implements Player {
             LOGGER.warn("Wrong target");
         }
 
-        this.chooseStrictModeFailed("target", game, getInfo(source, game) + "\n" + getInfo(target, source, game));
+        this.chooseStrictModeFailed("target", game, getInfo(source, game) + "\n" + getInfo(target, source, game, cards));
         return computerPlayer.chooseTarget(outcome, cards, target, source, game);
     }
 
@@ -4320,7 +4320,7 @@ public class TestPlayer implements Player {
             assertWrongChoiceUsage(choices.size() > 0 ? choices.get(0) : "empty list");
         }
 
-        this.chooseStrictModeFailed("choice", game, getInfo(source, game) + "\n" + getInfo(target, source, game));
+        this.chooseStrictModeFailed("choice", game, getInfo(source, game) + "\n" + getInfo(target, source, game, cards));
         return computerPlayer.choose(outcome, cards, target, source, game);
     }
 
@@ -4407,7 +4407,7 @@ public class TestPlayer implements Player {
             }
         }
 
-        this.chooseStrictModeFailed("target", game, getInfo(source, game) + "\n" + getInfo(target, source, game));
+        this.chooseStrictModeFailed("target", game, getInfo(source, game) + "\n" + getInfo(target, source, game, null));
         return computerPlayer.chooseTargetAmount(outcome, target, source, game);
     }
 
@@ -4756,7 +4756,7 @@ public class TestPlayer implements Player {
         Assert.fail(String.format("Found wrong choice command (%s):\n%s\n%s\n%s",
                 reason,
                 lastChoice,
-                getInfo(target, source, game),
+                getInfo(target, source, game, null),
                 getInfo(source, game)
         ));
     }

Mage/src/main/java/mage/abilities/effects/common/WishEffect.java

@@ -125,7 +125,7 @@ public class WishEffect extends OneShotEffect {
 
         TargetCard target = new TargetCard(Zone.ALL, filter);
         target.withNotTarget(true);
-        if (controller.choose(Outcome.Benefit, filteredCards, target, source, game)) {
+        if (controller.choose(Outcome.PutCardInPlay, filteredCards, target, source, game)) {
             Card card = controller.getSideboard().get(target.getFirstTarget(), game);
             if (card == null && alsoFromExile) {
                 card = game.getCard(target.getFirstTarget());

Mage/src/main/java/mage/constants/Outcome.java

@@ -14,14 +14,14 @@ public enum Outcome {
     LoseLife(false),
     ExtraTurn(true),
     BecomeCreature(true),
-    PutCreatureInPlay(true),
+    PutCreatureInPlay(true, true),
-    PutCardInPlay(true),
+    PutCardInPlay(true, true),
-    PutLandInPlay(true),
+    PutLandInPlay(true, true),
     GainControl(false),
     DrawCard(true),
     Discard(false),
     Sacrifice(false),
-    PlayForFree(true),
+    PlayForFree(true, true),
     ReturnToHand(false),
     Exile(false),
     Protect(true),
@@ -46,7 +46,7 @@ public enum Outcome {
     // AI sorting targets by priorities (own or opponents) and selects most valueable or weakest
     private final boolean good;
 
-    // no different between own or opponent targets (example: copy must choose from all permanents)
+    // no different between own or opponent targets (example: copy must choose from all permanents, free cast from selected cards, etc)
     private boolean anyTargetHasSameValue;
 
     Outcome(boolean good) {

Mage/src/main/java/mage/target/Target.java

@@ -14,6 +14,7 @@ import java.util.Collection;
 import java.util.List;
 import java.util.Set;
 import java.util.UUID;
+import java.util.stream.Collectors;
 
 /**
  * @author BetaSteward_at_googlemail.com
@@ -63,6 +64,13 @@ public interface Target extends Copyable<Target>, Serializable {
      */
     Set<UUID> possibleTargets(UUID sourceControllerId, Ability source, Game game);
 
+    default Set<UUID> possibleTargets(UUID sourceControllerId, Ability source, Game game, Set<UUID> cards) {
+        // do not override
+        return possibleTargets(sourceControllerId, source, game).stream()
+                .filter(id -> cards == null || cards.contains(id))
+                .collect(Collectors.toSet());
+    }
+
     /**
      * Priority method to make a choice from cards and other places, not a player.chooseXXX
      */

Mage/src/main/java/mage/target/TargetCard.java

@@ -110,6 +110,12 @@ public class TargetCard extends TargetObject {
                         possibleTargets += countPossibleTargetInCommandZone(game, player, sourceControllerId, source,
                                 filter, isNotTarget(), this.minNumberOfTargets - possibleTargets);
                         break;
+                    case ALL:
+                        possibleTargets += countPossibleTargetInAnyZone(game, player, sourceControllerId, source,
+                                filter, isNotTarget(), this.minNumberOfTargets - possibleTargets);
+                        break;
+                    default:
+                        throw new IllegalArgumentException("Unsupported TargetCard zone: " + zone);
                 }
                 if (possibleTargets >= this.minNumberOfTargets) {
                     return true;
@@ -214,6 +220,25 @@ public class TargetCard extends TargetObject {
         return possibleTargets;
     }
 
+    /**
+     * count up to N possible target cards in ANY zone
+     */
+    protected static int countPossibleTargetInAnyZone(Game game, Player player, UUID sourceControllerId, Ability source, FilterCard filter, boolean isNotTarget, int countUpTo) {
+        UUID sourceId = source != null ? source.getSourceId() : null;
+        int possibleTargets = 0;
+        for (Card card : game.getCards()) {
+            if (sourceId == null || isNotTarget || !game.replaceEvent(new TargetEvent(card, sourceId, sourceControllerId))) {
+                if (filter.match(card, game)) {
+                    possibleTargets++;
+                    if (possibleTargets >= countUpTo) {
+                        return possibleTargets; // early return for faster computation.
+                    }
+                }
+            }
+        }
+        return possibleTargets;
+    }
+
     @Override
     public Set<UUID> possibleTargets(UUID sourceControllerId, Game game) {
         return possibleTargets(sourceControllerId, null, game);
@@ -245,6 +270,11 @@ public class TargetCard extends TargetObject {
                     case COMMAND:
                         possibleTargets.addAll(getAllPossibleTargetInCommandZone(game, player, sourceControllerId, source, filter, isNotTarget()));
                         break;
+                    case ALL:
+                        possibleTargets.addAll(getAllPossibleTargetInAnyZone(game, player, sourceControllerId, source, filter, isNotTarget()));
+                        break;
+                    default:
+                        throw new IllegalArgumentException("Unsupported TargetCard zone: " + zone);
                 }
             }
         }
@@ -332,6 +362,22 @@ public class TargetCard extends TargetObject {
         return possibleTargets;
     }
 
+    /**
+     * set of all matching target in ANY zone
+     */
+    protected static Set<UUID> getAllPossibleTargetInAnyZone(Game game, Player player, UUID sourceControllerId, Ability source, FilterCard filter, boolean isNotTarget) {
+        Set<UUID> possibleTargets = new HashSet<>();
+        UUID sourceId = source != null ? source.getSourceId() : null;
+        for (Card card : game.getCards()) {
+            if (sourceId == null || isNotTarget || !game.replaceEvent(new TargetEvent(card, sourceId, sourceControllerId))) {
+                if (filter.match(card, sourceControllerId, source, game)) {
+                    possibleTargets.add(card.getId());
+                }
+            }
+        }
+        return possibleTargets;
+    }
+
     // TODO: check all class targets, if it override canTarget then make sure it override ALL 3 METHODS with canTarget and possibleTargets (method with cards doesn't need)
 
     @Override

Mage/src/main/java/mage/util/CardUtil.java

@@ -1556,7 +1556,7 @@ public final class CardUtil {
         return result;
     }
 
-    private static boolean checkForPlayable(Cards cards, FilterCard filter, Ability source, Player player, Game game, SpellCastTracker spellCastTracker, boolean playLand) {
+    private static boolean cardsHasCastableParts(Cards cards, FilterCard filter, Ability source, Player player, Game game, SpellCastTracker spellCastTracker, boolean playLand) {
         return cards
                 .getCards(game)
                 .stream()
@@ -1580,19 +1580,40 @@ public final class CardUtil {
             CardUtil.castSpellWithAttributesForFree(player, source, game, cards, filter);
             return;
         }
-        int spellsCast = 0;
+        int castCount = 0;
+        int maxCastCount = Integer.min(cards.size(), maxSpells);
         cards.removeZone(Zone.STACK, game);
-        while (player.canRespond() && spellsCast < maxSpells && !cards.isEmpty()) {
+        if (!cardsHasCastableParts(cards, filter, source, player, game, spellCastTracker, playLand)) {
-            if (CardUtil.castSpellWithAttributesForFree(player, source, game, cards, filter, spellCastTracker, playLand)) {
+            return;
-                spellsCast++;
+        }
-                cards.removeZone(Zone.STACK, game);
+
-            } else if (!checkForPlayable(
+        while (player.canRespond()) {
-                    cards, filter, source, player, game, spellCastTracker, playLand
+            boolean wasCast = CardUtil.castSpellWithAttributesForFree(player, source, game, cards, filter, spellCastTracker, playLand);
-            ) || !player.chooseUse(
+
-                    Outcome.PlayForFree, "Continue casting spells?", source, game
+            // nothing to cast
-            )) {
+            cards.removeZone(Zone.STACK, game);
+            if (cards.isEmpty() || !cardsHasCastableParts(cards, filter, source, player, game, spellCastTracker, playLand)) {
                 break;
             }
+
+            if (wasCast) {
+                // no more tries to cast
+                castCount++;
+                if (castCount >= maxCastCount) {
+                    break;
+                }
+            } else {
+                // player want to cancel
+                if (player.isComputer()) {
+                    // AI can't choose good spell, so stop
+                    break;
+                } else {
+                    // Human can choose wrong spell part, so allow to continue
+                    if (!player.chooseUse(Outcome.PlayForFree, "Continue casting spells?", source, game)) {
+                        break;
+                    }
+                }
+            }
         }
     }