Some cleanup

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

@@ -1090,7 +1090,6 @@ public class ChangeZoneAi extends SpellAbilityAi {
 
         // Exile and bounce opponents stuff
         if (destination.equals(ZoneType.Exile) || origin.contains(ZoneType.Battlefield)) {
-
             // don't rush bouncing stuff when not going to attack
             if (!immediately && game.getPhaseHandler().getPhase().isBefore(PhaseType.MAIN2)
                     && game.getPhaseHandler().isPlayerTurn(ai)
@@ -1433,17 +1432,14 @@ public class ChangeZoneAi extends SpellAbilityAi {
         final ZoneType destination = ZoneType.smartValueOf(sa.getParam("Destination"));
         final TargetRestrictions tgt = sa.getTargetRestrictions();
 
-        CardCollection list = CardLists.getValidCards(ai.getGame().getCardsIn(tgt.getZone()), tgt.getValidTgts(), ai, source, sa);
+        CardCollection list = new CardCollection(CardUtil.getValidCardsToTarget(tgt, sa));
-        list = CardLists.getTargetableCards(list, sa);
-
-        list.removeAll(sa.getTargets().getTargetCards());
 
         if (list.isEmpty()) {
             return false;
         }
 
         // target loop
-        while (sa.getTargets().size() < tgt.getMinTargets(sa.getHostCard(), sa)) {
+        while (!sa.isMinTargetChosen()) {
             // AI Targeting
             Card choice = null;
 

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

@@ -757,7 +757,7 @@ public class DamageDealAi extends DamageAiBase {
                 return false;
             }
             // TODO: Improve Damage, we shouldn't just target the player just because we can
-            if (sa.canTarget(enemy) && tcs.size() < tgt.getMaxTargets(source, sa)) {
+            if (sa.canTarget(enemy) && sa.canAddMoreTarget()) {
                 if (((phase.is(PhaseType.END_OF_TURN) && phase.getNextTurn().equals(ai))
                         || (SpellAbilityAi.isSorcerySpeed(sa, ai) && phase.is(PhaseType.MAIN2))
                         || ("PingAfterAttack".equals(logic) && phase.getPhase().isAfter(PhaseType.COMBAT_DECLARE_ATTACKERS) && phase.isPlayerTurn(ai))

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

@@ -130,7 +130,7 @@ public class DamagePreventAi extends SpellAbilityAi {
                 ComputerUtilCard.sortByEvaluateCreature(combatants);
 
                 for (final Card c : combatants) {
-                    if (ComputerUtilCombat.combatantWouldBeDestroyed(ai, c, combat) && tcs.size() < tgt.getMaxTargets(hostCard, sa)) {
+                    if (ComputerUtilCombat.combatantWouldBeDestroyed(ai, c, combat) && sa.canAddMoreTarget()) {
                     	tcs.add(c);
                         chance = true;
                     }

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

@@ -17,6 +17,7 @@ import forge.game.ability.AbilityUtils;
 import forge.game.card.Card;
 import forge.game.card.CardCollection;
 import forge.game.card.CardLists;
+import forge.game.card.CardUtil;
 import forge.game.combat.Combat;
 import forge.game.cost.Cost;
 import forge.game.phase.PhaseHandler;
@@ -24,7 +25,6 @@ import forge.game.phase.PhaseType;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.game.spellability.TargetRestrictions;
-import forge.game.zone.ZoneType;
 
 public class DebuffAi extends SpellAbilityAi {
 
@@ -195,16 +195,13 @@ public class DebuffAi extends SpellAbilityAi {
      */
     private boolean debuffMandatoryTarget(final Player ai, final SpellAbility sa, final boolean mandatory) {
         final TargetRestrictions tgt = sa.getTargetRestrictions();
-        CardCollection list = CardLists.getTargetableCards(ai.getGame().getCardsIn(ZoneType.Battlefield), sa);
+        List<Card> list = CardUtil.getValidCardsToTarget(tgt, sa);
 
         if (list.size() < tgt.getMinTargets(sa.getHostCard(), sa)) {
             sa.resetTargets();
             return false;
         }
 
-        // Remove anything that's already been targeted
-        list.removeAll(sa.getTargets().getTargetCards());
-
         final CardCollection pref = CardLists.filterControlledBy(list, ai.getOpponents());
         final CardCollection forced = CardLists.filterControlledBy(list, ai);
         final Card source = sa.getHostCard();
@@ -219,7 +216,7 @@ public class DebuffAi extends SpellAbilityAi {
             sa.getTargets().add(c);
         }
 
-        while (sa.getTargets().size() < tgt.getMinTargets(source, sa)) {
+        while (!sa.isMinTargetChosen()) {
             if (forced.isEmpty()) {
                 break;
             }
@@ -237,13 +234,13 @@ public class DebuffAi extends SpellAbilityAi {
             sa.getTargets().add(c);
         }
 
-        if (sa.getTargets().size() < tgt.getMinTargets(source, sa)) {
+        if (!sa.isMinTargetChosen()) {
             sa.resetTargets();
             return false;
         }
 
         return true;
-    } // pumpMandatoryTarget()
+    }
 
     @Override
     protected boolean doTriggerAINoCost(Player ai, SpellAbility sa, boolean mandatory) {

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

@@ -20,13 +20,13 @@ import forge.game.ability.effects.ProtectEffect;
 import forge.game.card.Card;
 import forge.game.card.CardCollection;
 import forge.game.card.CardLists;
+import forge.game.card.CardUtil;
 import forge.game.combat.Combat;
 import forge.game.phase.PhaseHandler;
 import forge.game.phase.PhaseType;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.game.spellability.TargetRestrictions;
-import forge.game.zone.ZoneType;
 import forge.util.MyRandom;
 
 public class ProtectAi extends SpellAbilityAi {
@@ -168,23 +168,23 @@ public class ProtectAi extends SpellAbilityAi {
     
     @Override
     protected boolean checkApiLogic(final Player ai, final SpellAbility sa) {
-        if (!sa.usesTargeting()) {
+        if (sa.usesTargeting()) {
-            final List<Card> cards = AbilityUtils.getDefinedCards(sa.getHostCard(), sa.getParam("Defined"), sa);
-            if (cards.size() == 0) {
-                return false;
-            } else if (cards.size() == 1) {
-                // Affecting single card
-                return getProtectCreatures(ai, sa).contains(cards.get(0));
-            }
-            /*
-             * when this happens we need to expand AI to consider if its ok
-             * for everything? for (Card card : cards) { // TODO if AI doesn't
-             * control Card and Pump is a Curse, than maybe use?
-             * }
-             */
-        } else {
             return protectTgtAI(ai, sa, false);
         }
+
+        final List<Card> cards = AbilityUtils.getDefinedCards(sa.getHostCard(), sa.getParam("Defined"), sa);
+        if (cards.size() == 0) {
+            return false;
+        } else if (cards.size() == 1) {
+            // Affecting single card
+            return getProtectCreatures(ai, sa).contains(cards.get(0));
+        }
+        /*
+         * when this happens we need to expand AI to consider if its ok
+         * for everything? for (Card card : cards) { // TODO if AI doesn't
+         * control Card and Pump is a Curse, than maybe use?
+         * }
+         */
         return false;
     }
 
@@ -256,21 +256,15 @@ public class ProtectAi extends SpellAbilityAi {
     } // protectTgtAI()
 
     private static boolean protectMandatoryTarget(final Player ai, final SpellAbility sa) {
-        final Game game = ai.getGame();
-
         final TargetRestrictions tgt = sa.getTargetRestrictions();
-        CardCollection list = CardLists.getTargetableCards(game.getCardsIn(ZoneType.Battlefield), sa);
+        final Card source = sa.getHostCard();
+        final List<Card> list = CardUtil.getValidCardsToTarget(tgt, sa);
 
-        if (list.size() < tgt.getMinTargets(sa.getHostCard(), sa)) {
+        if (list.size() < tgt.getMinTargets(source, sa)) {
             sa.resetTargets();
             return false;
         }
 
-        // Remove anything that's already been targeted
-        for (final Card c : sa.getTargets().getTargetCards()) {
-            list.remove(c);
-        }
-
         CardCollection pref = CardLists.filterControlledBy(list, ai);
         pref = CardLists.filter(pref, new Predicate<Card>() {
             @Override
@@ -286,7 +280,6 @@ public class ProtectAi extends SpellAbilityAi {
             }
         });
         final List<Card> forced = CardLists.filterControlledBy(list, ai);
-        final Card source = sa.getHostCard();
 
         while (sa.canAddMoreTarget()) {
             if (pref.isEmpty()) {
@@ -308,13 +301,13 @@ public class ProtectAi extends SpellAbilityAi {
             sa.getTargets().add(c);
         }
 
-        while (sa.getTargets().size() < tgt.getMinTargets(source, sa)) {
+        while (!sa.isMinTargetChosen()) {
             if (forced.isEmpty()) {
                 break;
             }
 
             Card c;
-            if (CardLists.getNotType(forced, "Creature").size() == 0) {
+            if (CardLists.getNotType(forced, "Creature").isEmpty()) {
                 c = ComputerUtilCard.getWorstCreatureAI(forced);
             } else {
                 c = ComputerUtilCard.getCheapestPermanentAI(forced, sa, false);
@@ -323,7 +316,7 @@ public class ProtectAi extends SpellAbilityAi {
             sa.getTargets().add(c);
         }
 
-        if (sa.getTargets().size() < tgt.getMinTargets(source, sa)) {
+        if (!sa.isMinTargetChosen()) {
             sa.resetTargets();
             return false;
         }

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

@@ -616,23 +616,16 @@ public class PumpAi extends PumpAiBase {
     }
 
     private boolean pumpMandatoryTarget(final Player ai, final SpellAbility sa) {
-        final Game game = ai.getGame();
         final TargetRestrictions tgt = sa.getTargetRestrictions();
-        CardCollection list = CardLists.getTargetableCards(game.getCardsIn(ZoneType.Battlefield), sa);
+        List<Card> list = CardUtil.getValidCardsToTarget(tgt, sa);
 
         if (list.size() < tgt.getMinTargets(sa.getHostCard(), sa)) {
             sa.resetTargets();
             return false;
         }
 
-        // Remove anything that's already been targeted
-        for (final Card c : sa.getTargets().getTargetCards()) {
-            list.remove(c);
-        }
-
         CardCollection pref;
         CardCollection forced;
-        final Card source = sa.getHostCard();
 
         if (sa.isCurse()) {
             pref = CardLists.filterControlledBy(list, ai.getOpponents());
@@ -652,7 +645,7 @@ public class PumpAi extends PumpAiBase {
             sa.getTargets().add(c);
         }
 
-        while (sa.getTargets().size() < tgt.getMinTargets(source, sa)) {
+        while (!sa.isMinTargetChosen()) {
             if (forced.isEmpty()) {
                 break;
             }
@@ -668,7 +661,7 @@ public class PumpAi extends PumpAiBase {
             sa.getTargets().add(c);
         }
 
-        if (sa.getTargets().size() < tgt.getMinTargets(source, sa)) {
+        if (!sa.isMinTargetChosen()) {
             sa.resetTargets();
             return false;
         }

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

@@ -97,13 +97,13 @@ public class SetStateAi extends SpellAbilityAi {
                 for (final Card c : list) {
                     if (shouldTransformCard(c, ai, ph) || "Always".equals(logic)) {
                         sa.getTargets().add(c);
-                        if (sa.getTargets().size() == tgt.getMaxTargets(source, sa)) {
+                        if (sa.isMaxTargetChosen()) {
                             break;
                         }
                     }
                 }
 
-                return sa.getTargets().size() >= tgt.getMinTargets(source, sa);
+                return sa.isMinTargetChosen();
             }
         } else if ("TurnFace".equals(mode)) {
             if (!sa.usesTargeting()) {

forge-game/src/main/java/forge/game/ability/AbilityUtils.java

@@ -96,6 +96,13 @@ public class AbilityUtils {
         final Game game = hostCard.getGame();
 
         Card c = null;
+        Player player = null;
+        if (sa instanceof SpellAbility) {
+            player = ((SpellAbility)sa).getActivatingPlayer();
+        }
+        if (player == null) {
+            player = hostCard.getController();
+        }
 
         if (defined.equals("Self")) {
             c = hostCard;
@@ -143,7 +150,7 @@ public class AbilityUtils {
                 }
             }
         } else if (defined.equals("TopOfGraveyard")) {
-            final CardCollectionView grave = hostCard.getController().getCardsIn(ZoneType.Graveyard);
+            final CardCollectionView grave = player.getCardsIn(ZoneType.Graveyard);
 
             if (grave.size() > 0) {
                 c = grave.getLast();
@@ -153,7 +160,7 @@ public class AbilityUtils {
             }
         }
         else if (defined.endsWith("OfLibrary")) {
-            final CardCollectionView lib = hostCard.getController().getCardsIn(ZoneType.Library);
+            final CardCollectionView lib = player.getCardsIn(ZoneType.Library);
             int libSize = lib.size();
             if (libSize > 0) { // TopOfLibrary or BottomOfLibrary
                 if (defined.startsWith("TopThird")) {
@@ -354,7 +361,7 @@ public class AbilityUtils {
                 candidates = game.getCardsIn(ZoneType.smartValueOf(zone));
                 validDefined = s[1];
             }
-            cards.addAll(CardLists.getValidCards(candidates, validDefined, hostCard.getController(), hostCard, sa));
+            cards.addAll(CardLists.getValidCards(candidates, validDefined, player, hostCard, sa));
             return cards;
         } else if (defined.startsWith("ExiledWith")) {
             cards.addAll(hostCard.getExiledCards());
@@ -375,7 +382,7 @@ public class AbilityUtils {
             for (int i = 0; i < valids.length; i++) {
                 valids[i] = "Card." + valids[i];
             }
-            cards = CardLists.getValidCards(cards, valids, hostCard.getController(), hostCard, sa);
+            cards = CardLists.getValidCards(cards, valids, player, hostCard, sa);
         }
 
         return cards;

forge-game/src/main/java/forge/game/ability/effects/PumpAllEffect.java

@@ -115,7 +115,7 @@ public class PumpAllEffect extends SpellAbilityEffect {
         sb.append(desc);
 
         return sb.toString();
-    } // pumpAllStackDescription()
+    }
 
     @Override
     public void resolve(final SpellAbility sa) {

forge-game/src/main/java/forge/game/ability/effects/PumpEffect.java

@@ -415,10 +415,7 @@ public class PumpEffect extends SpellAbilityEffect {
         final ZoneType pumpZone = sa.hasParam("PumpZone") ? ZoneType.smartValueOf(sa.getParam("PumpZone"))
                 : ZoneType.Battlefield;
 
-        final int size = tgtCards.size();
+        for (Card tgtC : tgtCards) {
-        for (int j = 0; j < size; j++) {
-            final Card tgtC = tgtCards.get(j);
-
             // CR 702.26e
             if (tgtC.isPhasedOut()) {
                 continue;

forge-game/src/main/java/forge/game/ability/effects/RepeatEachEffect.java

@@ -32,14 +32,6 @@ public class RepeatEachEffect extends SpellAbilityEffect {
 
         final SpellAbility repeat = sa.getAdditionalAbility("RepeatSubAbility");
 
-        if (repeat != null && !repeat.getHostCard().equalsWithTimestamp(source)) {
-            // TODO: for some reason, the host card of the original additional SA is set to the cloned card when
-            // the ability is copied (e.g. Clone Legion + Swarm Intelligence). Couldn't figure out why this happens,
-            // so this hack is necessary for now to work around this issue.
-            System.out.println("Warning: RepeatSubAbility had the wrong host set (potentially after cloning the root SA or changing zones), attempting to correct...");
-            repeat.setHostCard(source);
-        }
-
         final Player player = sa.getActivatingPlayer();
         final Game game = player.getGame();
         if (sa.hasParam("Optional") && sa.hasParam("OptionPrompt") && //for now, OptionPrompt is needed
@@ -74,10 +66,6 @@ public class RepeatEachEffect extends SpellAbilityEffect {
         }
         else if (sa.hasParam("DefinedCards")) {
             repeatCards = AbilityUtils.getDefinedCards(source, sa.getParam("DefinedCards"), sa);
-            if (sa.hasParam("AdditionalRestriction")) { // lki cards might not be in game
-                repeatCards = CardLists.getValidCards(repeatCards,
-                        sa.getParam("AdditionalRestriction"), source.getController(), source, sa);
-            }
         }
         boolean loopOverCards = repeatCards != null && !repeatCards.isEmpty();
 
@@ -125,13 +113,10 @@ public class RepeatEachEffect extends SpellAbilityEffect {
 
         // for a mixed list of target permanents and players, e.g. Soulfire Eruption
         if (sa.hasParam("RepeatTargeted")) {
-            final List <GameObject> tgts = getTargets(sa);
+            for (final GameObject o : getTargets(sa)) {
-            if (tgts != null) {
+                source.addRemembered(o);
-                for (final Object o : tgts) {
+                AbilityUtils.resolve(repeat);
-                    source.addRemembered(o);
+                source.removeRemembered(o);
-                    AbilityUtils.resolve(repeat);
-                    source.removeRemembered(o);
-                }
             }
         }
 

forge-game/src/main/java/forge/game/ability/effects/RepeatEffect.java

@@ -26,17 +26,9 @@ public class RepeatEffect extends SpellAbilityEffect {
         // setup subability to repeat
         SpellAbility repeat = sa.getAdditionalAbility("RepeatSubAbility");
 
-        if (repeat != null && !repeat.getHostCard().equals(source)) {
-            // TODO: for some reason, the host card of the original additional SA is set to the cloned card when
-            // the ability is copied (e.g. Clone Legion + Swarm Intelligence). Couldn't figure out why this happens,
-            // so this hack is necessary for now to work around this issue.
-            System.out.println("Warning: RepeatSubAbility had the wrong host set (potentially after cloning the root SA), attempting to correct...");
-            repeat.setHostCard(source);
-        }
-
         Integer maxRepeat = null;
         if (sa.hasParam("MaxRepeat")) {
-            maxRepeat = AbilityUtils.calculateAmount(sa.getHostCard(), sa.getParam("MaxRepeat"), sa);
+            maxRepeat = AbilityUtils.calculateAmount(source, sa.getParam("MaxRepeat"), sa);
             if (maxRepeat.intValue() == 0) return; // do nothing if maxRepeat is 0. the next loop will execute at least once
         }
 
@@ -50,7 +42,7 @@ public class RepeatEffect extends SpellAbilityEffect {
                 // Helm of Obedience vs Graveyard to Library replacement effect
 
                 if (source.getName().equals("Helm of Obedience")) {
-                StringBuilder infLoop = new StringBuilder(sa.getHostCard().toString());
+                StringBuilder infLoop = new StringBuilder(source.toString());
                     infLoop.append(" - To avoid an infinite loop, this repeat has been broken ");
                     infLoop.append(" and the game will now continue in the current state, ending the loop early. ");
                     infLoop.append("Once Draws are available this probably should change to a Draw.");
@@ -84,7 +76,7 @@ public class RepeatEffect extends SpellAbilityEffect {
             } else {
                 list = game.getCardsIn(ZoneType.Battlefield);
             }
-            list = CardLists.getValidCards(list, repeatPresent, sa.getActivatingPlayer(), sa.getHostCard(), sa);
+            list = CardLists.getValidCards(list, repeatPresent, activator, sa.getHostCard(), sa);
 
             final String rightString = repeatCompare.substring(2);
             int right = AbilityUtils.calculateAmount(sa.getHostCard(), rightString, sa);
@@ -114,7 +106,7 @@ public class RepeatEffect extends SpellAbilityEffect {
         if (sa.hasParam("RepeatOptional")) {
             Player decider = sa.hasParam("RepeatOptionalDecider")
                     ? AbilityUtils.getDefinedPlayers(sa.getHostCard(), sa.getParam("RepeatOptionalDecider"), sa).get(0)
-                    : sa.getActivatingPlayer();
+                    : activator;
             return decider.getController().confirmAction(sa, null, Localizer.getInstance().getMessage("lblDoYouWantRepeatProcessAgain"), null);
         }
 

forge-gui/res/cardsfolder/d/desolation.txt

@@ -3,7 +3,7 @@ ManaCost:1 B B
 Types:Enchantment
 T:Mode$ Phase | Phase$ End of Turn | TriggerZones$ Battlefield | Execute$ TrigSac | TriggerDescription$ At the beginning of each end step, each player who tapped a land for mana this turn sacrifices a land. Desolation deals 2 damage to each player who sacrificed a Plains this way.
 SVar:TrigSac:DB$ Sacrifice | SacValid$ Land | Defined$ Player.TappedLandForManaThisTurn | RememberSacrificed$ True | SubAbility$ DBRepeat
-SVar:DBRepeat:DB$ RepeatEach | DefinedCards$ Remembered | AdditionalRestriction$ Plains | UseImprinted$ True | RepeatSubAbility$ DBDamage | SubAbility$ DBCleanup
+SVar:DBRepeat:DB$ RepeatEach | DefinedCards$ Remembered.Plains | UseImprinted$ True | RepeatSubAbility$ DBDamage | SubAbility$ DBCleanup
 SVar:DBDamage:DB$ DealDamage | Defined$ ImprintedController | NumDmg$ 2
 SVar:DBCleanup:DB$ Cleanup | ClearRemembered$ True
 AI:RemoveDeck:All

forge-gui/res/cardsfolder/w/wave_of_vitriol.txt

@@ -2,7 +2,7 @@ Name:Wave of Vitriol
 ManaCost:5 G G
 Types:Sorcery
 A:SP$ SacrificeAll | Cost$ 5 G G | ValidCards$ Artifact,Enchantment,Land.nonBasic | RememberSacrificed$ True | SubAbility$ DBRepeat | SpellDescription$ Each player sacrifices all artifacts, enchantments, and nonbasic lands they control. For each land sacrificed this way, its controller may search their library for a basic land card and put it onto the battlefield tapped. Then each player who searched their library this way shuffles.
-SVar:DBRepeat:DB$ RepeatEach | DefinedCards$ DirectRemembered | AdditionalRestriction$ Land | UseImprinted$ True | RepeatSubAbility$ DBSearch | ClearRemembered$ True | SubAbility$ DBShuffle
+SVar:DBRepeat:DB$ RepeatEach | DefinedCards$ DirectRemembered.Land | UseImprinted$ True | RepeatSubAbility$ DBSearch | ClearRemembered$ True | SubAbility$ DBShuffle
 SVar:DBSearch:DB$ ChangeZone | Origin$ Library | Destination$ Battlefield | ChangeType$ Land.Basic | ChangeNum$ 1 | Tapped$ True | RememberChanged$ True | DefinedPlayer$ ImprintedController | Chooser$ ImprintedController | NoShuffle$ True | Optional$ True
 SVar:DBShuffle:DB$ RepeatEach | RepeatPlayers$ Player | RepeatSubAbility$ ShuffleSearched | SubAbility$ DBCleanup
 SVar:ShuffleSearched:DB$ Shuffle | Defined$ Player.IsRemembered | ConditionCheckSVar$ X | ConditionSVarCompare$ GE1