Merge pull request #1967 from tool4ever/targeteffects

Forward another PR part to extract targeting checks from effects

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

@@ -843,4 +843,15 @@ public abstract class SpellAbilityEffect {
             game.getEndOfTurn().addUntil(until);
         }
     }
+
+    public Player getNewChooser(final SpellAbility sa, final Player activator, final Player loser) {
+        // CR 800.4g
+        final PlayerCollection options;
+        if (loser.isOpponentOf(activator)) {
+            options = activator.getOpponents();
+        } else {
+            options = activator.getAllOtherPlayers();
+        }
+        return activator.getController().chooseSingleEntityForEffect(options, sa, Localizer.getInstance().getMessage("lblChoosePlayer") , null);
+    }
 }

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

@@ -12,7 +12,6 @@ import forge.game.card.Card;
 import forge.game.card.CardLists;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import forge.game.zone.ZoneType;
 import forge.util.Lang;
 import forge.util.Localizer;
@@ -36,12 +35,14 @@ public class ActivateAbilityEffect extends SpellAbilityEffect {
 
     @Override
     public void resolve(SpellAbility sa) {
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
         final boolean isManaAb = sa.hasParam("ManaAbility");
         // TODO: improve ai and fix corner cases
 
         for (final Player p : getTargetPlayers(sa)) {
-            if ((tgt == null) || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
+
             List<Card> list = CardLists.getType(p.getCardsIn(ZoneType.Battlefield), sa.getParamOrDefault("Type", "Card")); 
             for (Card c : list) {
                 List<SpellAbility> possibleAb = Lists.newArrayList(c.getAllPossibleAbilities(p, true));
@@ -57,6 +58,5 @@ public class ActivateAbilityEffect extends SpellAbilityEffect {
             }
         }
     }
-    }
 
 }

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

@@ -1,7 +1,5 @@
 package forge.game.ability.effects;
 
-import java.util.List;
-
 import forge.game.Game;
 import forge.game.ability.AbilityFactory;
 import forge.game.ability.AbilityKey;
@@ -15,6 +13,7 @@ import forge.game.trigger.Trigger;
 import forge.game.trigger.TriggerHandler;
 import forge.game.trigger.TriggerType;
 import forge.game.zone.ZoneType;
+import forge.util.Lang;
 import forge.util.Localizer;
 
 public class AddTurnEffect extends SpellAbilityEffect {
@@ -24,11 +23,7 @@ public class AddTurnEffect extends SpellAbilityEffect {
         final StringBuilder sb = new StringBuilder();
         final int numTurns = AbilityUtils.calculateAmount(sa.getHostCard(), sa.getParam("NumTurns"), sa);
 
-        List<Player> tgtPlayers = getTargetPlayers(sa);
+        sb.append(Lang.joinHomogenous(getTargetPlayers(sa)));
-
-        for (final Player player : tgtPlayers) {
-            sb.append(player).append(" ");
-        }
 
         sb.append("takes ");
         sb.append(numTurns > 1 ? numTurns : "an");
@@ -45,10 +40,10 @@ public class AddTurnEffect extends SpellAbilityEffect {
     public void resolve(SpellAbility sa) {
         final int numTurns = AbilityUtils.calculateAmount(sa.getHostCard(), sa.getParam("NumTurns"), sa);
 
-        List<Player> tgtPlayers = getTargetPlayers(sa);
+        for (final Player p : getTargetPlayers(sa)) {
-
+            if (!p.isInGame()) {
-        for (final Player p : tgtPlayers) {
+                continue;
-            if (!sa.usesTargeting() || p.canBeTargetedBy(sa)) {
+            }
             for (int i = 0; i < numTurns; i++) {
                 ExtraTurn extra = p.getGame().getPhaseHandler().addExtraTurn(p);
                 if (sa.hasParam("ExtraTurnDelayedTrigger")) {
@@ -71,7 +66,6 @@ public class AddTurnEffect extends SpellAbilityEffect {
             }
         }
     }
-    }
 
     public static void createCantSetSchemesInMotionEffect(SpellAbility sa) {
         final Card hostCard = sa.getHostCard();

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

@@ -27,12 +27,13 @@ public class BecomeMonarchEffect extends SpellAbilityEffect {
         final String set = sa.getHostCard().getSetCode();
 
         for (final Player p : getTargetPlayers(sa)) {
-            if (!sa.usesTargeting() || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
             if (p.canBecomeMonarch()) {
                 p.getGame().getAction().becomeMonarch(p, set);
             }
         }
     }
-    }
 
 }

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

@@ -34,7 +34,6 @@ public class BecomesBlockedEffect extends SpellAbilityEffect {
         final Game game = sa.getActivatingPlayer().getGame();
         List<Card> blocked = Lists.newArrayList();
         for (final Card c : getTargetCards(sa)) {
-            if ((!sa.usesTargeting()) || c.canBeTargetedBy(sa)) {
             game.getCombat().setBlocked(c, true);
             if (!c.getDamageHistory().getCreatureGotBlockedThisCombat()) {
                 blocked.add(c);
@@ -46,7 +45,6 @@ public class BecomesBlockedEffect extends SpellAbilityEffect {
                 game.getTriggerHandler().runTrigger(TriggerType.AttackerBlocked, runParams, false);
             }
         }
-        }
 
         if (!blocked.isEmpty()) {
             final Map<AbilityKey, Object> runParams = AbilityKey.newMap();

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

@@ -18,7 +18,6 @@ import forge.game.event.GameEventCombatChanged;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.game.spellability.SpellAbilityStackInstance;
-import forge.game.spellability.TargetRestrictions;
 import forge.util.CardTranslation;
 import forge.util.Localizer;
 import forge.util.collect.FCollection;
@@ -42,7 +41,6 @@ public class ChangeCombatantsEffect extends SpellAbilityEffect {
         boolean isCombatChanged = false;
         final Player activator = sa.getActivatingPlayer();
         final Game game = activator.getGame();
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
         // TODO: may expand this effect for defined blocker (False Orders, General Jarkeld, Sorrow's Path, Ydwen Efreet)
         for (final Card c : getTargetCards(sa)) {
             String cardString = CardTranslation.getTranslatedName(c.getName()) + " (" + c.getId() + ")";
@@ -51,7 +49,7 @@ public class ChangeCombatantsEffect extends SpellAbilityEffect {
                     Localizer.getInstance().getMessage("lblChangeCombatantOption", cardString), null)) {
                 continue;
             }
-            if ((tgt == null) || c.canBeTargetedBy(sa)) {
+
             final Combat combat = game.getCombat();
             final GameEntity originalDefender = combat.getDefenderByAttacker(c);
             final FCollection<GameEntity> defs = new FCollection<>();
@@ -61,7 +59,7 @@ public class ChangeCombatantsEffect extends SpellAbilityEffect {
             Map<String, Object> params = Maps.newHashMap();
             params.put("Attacker", c);
 
-                final GameEntity defender = sa.getActivatingPlayer().getController().chooseSingleEntityForEffect(defs, sa, title, false, params);
+            final GameEntity defender = activator.getController().chooseSingleEntityForEffect(defs, sa, title, false, params);
             if (originalDefender != null && !originalDefender.equals(defender)) {
                 AttackingBand ab = combat.getBandOfAttacker(c);
                 if (ab != null) {
@@ -84,7 +82,6 @@ public class ChangeCombatantsEffect extends SpellAbilityEffect {
                 isCombatChanged = true;
             }
         }
-        }
 
         if (isCombatChanged) {
             game.updateCombatForView();

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

@@ -537,15 +537,13 @@ public class ChangeZoneEffect extends SpellAbilityEffect {
             if (gameCard == null || !tgtC.equalsWithTimestamp(gameCard) || gameCard.isPhasedOut()) {
                 continue;
             }
-            if (sa.usesTargeting() && !gameCard.canBeTargetedBy(sa)) {
+
-                continue;
-            }
             if (sa.hasParam("RememberLKI")) {
                 hostCard.addRemembered(CardUtil.getLKICopy(gameCard));
             }
 
             final String prompt = TextUtil.concatWithSpace(Localizer.getInstance().getMessage("lblDoYouWantMoveTargetFromOriToDest", CardTranslation.getTranslatedName(gameCard.getName()), Lang.joinHomogenous(origin, ZoneType.Accessors.GET_TRANSLATED_NAME), destination.getTranslatedName()));
-            if (optional && !chooser.getController().confirmAction(sa, null, prompt, null) )
+            if (optional && !chooser.getController().confirmAction(sa, null, prompt, null))
                 continue;
 
             final Zone originZone = game.getZoneOf(gameCard);

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

@@ -22,7 +22,6 @@ import forge.game.card.CardPredicates.Presets;
 import forge.game.player.Player;
 import forge.game.player.PlayerActionConfirmMode;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import forge.game.zone.ZoneType;
 import forge.util.Aggregates;
 import forge.util.Lang;
@@ -61,7 +60,6 @@ public class ChooseCardEffect extends SpellAbilityEffect {
         final Game game = activator.getGame();
         CardCollection chosen = new CardCollection();
 
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
         final List<Player> tgtPlayers = getTargetPlayers(sa);
 
         List<ZoneType> choiceZone = Lists.newArrayList(ZoneType.Battlefield);
@@ -101,7 +99,10 @@ public class ChooseCardEffect extends SpellAbilityEffect {
             return;
         }
 
-        for (final Player p : tgtPlayers) {
+        for (Player p : tgtPlayers) {
+            if (!p.isInGame()) {
+                p = getNewChooser(sa, activator, p);
+            }
             boolean dontRevealToOwner = true;
             if (sa.hasParam("EachBasicType")) {
                 // Get all lands, 
@@ -219,8 +220,7 @@ public class ChooseCardEffect extends SpellAbilityEffect {
                 chosen.addAll(p.getController().chooseCardsForEffect(notTgtPlayerCtrl, sa, title + " " + "you don't control", minAmount, validAmount,
                         !sa.hasParam("Mandatory"), null));
 
-            } else if ((tgt == null) || p.canBeTargetedBy(sa)) {
+            } else if (sa.hasParam("AtRandom") && !choices.isEmpty()) {
-                if (sa.hasParam("AtRandom") && !choices.isEmpty()) {
                 // don't pass FCollection for direct modification, the Set part would get messed up
                 chosen = new CardCollection(Aggregates.random(choices, validAmount));
                 dontRevealToOwner = false;
@@ -258,7 +258,6 @@ public class ChooseCardEffect extends SpellAbilityEffect {
                             !sa.hasParam("Mandatory"), null));
                 }
             }
-            }
             if (sa.hasParam("Reveal") && !sa.hasParam("SecretlyChoose")) {
                 game.getAction().reveal(chosen, p, dontRevealToOwner, sa.hasParam("RevealTitle") ?
                         sa.getParam("RevealTitle") : Localizer.getInstance().getMessage("lblChosenCards") + " ");

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

@@ -19,7 +19,6 @@ import forge.game.card.CardCollection;
 import forge.game.card.CardLists;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import forge.util.Aggregates;
 import forge.util.Localizer;
 
@@ -41,9 +40,6 @@ public class ChooseCardNameEffect extends SpellAbilityEffect {
     public void resolve(SpellAbility sa) {
         final Card host = sa.getHostCard();
 
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
-        final List<Player> tgtPlayers = getTargetPlayers(sa);
-
         String valid = "Card";
         String validDesc = null;
         String message = null;
@@ -68,8 +64,10 @@ public class ChooseCardNameEffect extends SpellAbilityEffect {
             }
         }
 
-        for (final Player p : tgtPlayers) {
+        for (final Player p : getTargetPlayers(sa)) {
-            if ((tgt == null) || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
             String chosen = "";
             //This section was used for Momir Avatar, which no longer uses it - commented out 7/28/2021
             //if (randomChoice) {
@@ -166,6 +164,5 @@ public class ChooseCardNameEffect extends SpellAbilityEffect {
             }
         }
     }
-    }
 
 }

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

@@ -6,7 +6,6 @@ import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import forge.util.Aggregates;
 import forge.util.Lang;
 import forge.util.Localizer;
@@ -49,12 +48,10 @@ public class ChooseColorEffect extends SpellAbilityEffect {
             }
         }
 
-        final List<Player> tgtPlayers = getTargetPlayers(sa);
+        for (Player p : getTargetPlayers(sa)) {
-
+            if (!p.isInGame()) {
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
+                p = getNewChooser(sa, sa.getActivatingPlayer(), p);
-
+            }
-        for (final Player p : tgtPlayers) {
-            if ((tgt == null) || p.canBeTargetedBy(sa)) {
             List<String> chosenColors = new ArrayList<>();
             int cntMin = sa.hasParam("TwoColors") ? 2 : 1;
             int cntMax = sa.hasParam("TwoColors") ? 2 : sa.hasParam("OrColors") ? colorChoices.size() : 1;
@@ -92,5 +89,4 @@ public class ChooseColorEffect extends SpellAbilityEffect {
             p.getGame().getAction().notifyOfValue(sa, p, Lang.joinHomogenous(chosenColors), noNotify);
         }
     }
-    }
 }

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

@@ -30,14 +30,15 @@ public class ChooseEvenOddEffect extends SpellAbilityEffect {
         final Card card = sa.getHostCard();
 
         for (final Player p : getTargetPlayers(sa)) {
-            if ((!sa.usesTargeting()) || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
             EvenOdd chosen = p.getController().chooseBinary(sa, "odd or even", BinaryChoiceType.OddsOrEvens) ? EvenOdd.Odd : EvenOdd.Even;
             card.setChosenEvenOdd(chosen);
             if (sa.hasParam("Notify")) {
                 p.getGame().getAction().notifyOfValue(sa, card, Localizer.getInstance().getMessage("lblPlayerPickedChosen", p.getName(), chosen), p);
             }
         }
-        }
         card.updateStateForView();
     }
 }

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

@@ -43,9 +43,10 @@ public class ChooseGenericEffect extends SpellAbilityEffect {
         final SpellAbility fallback = sa.getAdditionalAbility("FallbackAbility");
         final int amount = AbilityUtils.calculateAmount(host, sa.getParamOrDefault("ChoiceAmount", "1"), sa);
 
-        final List<Player> tgtPlayers = getDefinedPlayersOrTargeted(sa);
+        for (Player p : getDefinedPlayersOrTargeted(sa)) {
-
+            if (!p.isInGame()) {
-        for (final Player p : tgtPlayers) {
+                p = getNewChooser(sa, sa.getActivatingPlayer(), p);
+            }
             // determine if any of the choices are not valid
             List<SpellAbility> saToRemove = Lists.newArrayList();
             
@@ -62,10 +63,6 @@ public class ChooseGenericEffect extends SpellAbilityEffect {
             }
             abilities.removeAll(saToRemove);
 
-            if (sa.usesTargeting() && sa.getTargets().contains(p) && !p.canBeTargetedBy(sa)) {
-                continue;
-            }
-
             List<SpellAbility> chosenSAs = Lists.newArrayList();
             String prompt = sa.getParamOrDefault("ChoicePrompt", "Choose");
             boolean random = false;

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

@@ -12,7 +12,6 @@ import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import forge.util.Localizer;
 import forge.util.MyRandom;
 
@@ -46,12 +45,12 @@ public class ChooseNumberEffect extends SpellAbilityEffect {
         final String sMax = sa.getParamOrDefault("Max", "99");
         final int max = AbilityUtils.calculateAmount(card, sMax, sa); 
 
-        final List<Player> tgtPlayers = getTargetPlayers(sa);
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
         final Map<Player, Integer> chooseMap = Maps.newHashMap(); 
 
-        for (final Player p : tgtPlayers) {
+        for (final Player p : getTargetPlayers(sa)) {
-            if ((tgt == null) || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
             int chosen;
             if (random) {
                 chosen = MyRandom.getRandom().nextInt((max - min) + 1) + min;
@@ -76,7 +75,6 @@ public class ChooseNumberEffect extends SpellAbilityEffect {
                 p.getGame().getAction().notifyOfValue(sa, card, Localizer.getInstance().getMessage("lblPlayerPickedChosen", p.getName(), chosen), p);
             }
         }
-        }
         if (secretlyChoose) {
             StringBuilder sb = new StringBuilder();
             List<Player> highestNum = Lists.newArrayList();

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

@@ -1,14 +1,12 @@
 package forge.game.ability.effects;
 
-import java.util.List;
-
 import forge.game.ability.AbilityUtils;
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import forge.util.Aggregates;
+import forge.util.Lang;
 import forge.util.Localizer;
 import forge.util.collect.FCollectionView;
 
@@ -18,9 +16,8 @@ public class ChoosePlayerEffect extends SpellAbilityEffect {
     protected String getStackDescription(SpellAbility sa) {
         final StringBuilder sb = new StringBuilder();
 
-        for (final Player p : getTargetPlayers(sa)) {
+        sb.append(Lang.joinHomogenous(getTargetPlayers(sa)));
-            sb.append(p).append(" ");
+
-        }
         sb.append("chooses a player.");
 
         return sb.toString();
@@ -30,18 +27,16 @@ public class ChoosePlayerEffect extends SpellAbilityEffect {
     public void resolve(SpellAbility sa) {
         final Card card = sa.getHostCard();
 
-        final List<Player> tgtPlayers = getTargetPlayers(sa);
-
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
-
         final FCollectionView<Player> choices = sa.hasParam("Choices") ? AbilityUtils.getDefinedPlayers(
                 card, sa.getParam("Choices"), sa) : sa.getActivatingPlayer().getGame().getPlayersInTurnOrder();
 
         final String choiceDesc = sa.hasParam("ChoiceTitle") ? sa.getParam("ChoiceTitle") : Localizer.getInstance().getMessage("lblChoosePlayer");
         final boolean random = sa.hasParam("Random");
 
-        for (final Player p : tgtPlayers) {
+        for (final Player p : getTargetPlayers(sa)) {
-            if ((tgt == null) || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
             Player chosen;
             if (random) {
                 chosen = choices.isEmpty() ? null : Aggregates.random(choices);
@@ -83,5 +78,4 @@ public class ChoosePlayerEffect extends SpellAbilityEffect {
             }
         }
     }
-    }
 }

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

@@ -15,6 +15,7 @@ import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.game.spellability.SpellAbilityStackInstance;
 import forge.game.zone.ZoneType;
+import forge.util.Lang;
 import forge.util.Localizer;
 
 public class ChooseSourceEffect extends SpellAbilityEffect {
@@ -22,9 +23,8 @@ public class ChooseSourceEffect extends SpellAbilityEffect {
     protected String getStackDescription(SpellAbility sa) {
         final StringBuilder sb = new StringBuilder();
 
-        for (final Player p : getTargetPlayers(sa)) {
+        sb.append(Lang.joinHomogenous(getTargetPlayers(sa)));
-            sb.append(p).append(" ");
+
-        }
         sb.append("chooses a source.");
 
         return sb.toString();
@@ -129,8 +129,10 @@ public class ChooseSourceEffect extends SpellAbilityEffect {
         final int validAmount = StringUtils.isNumeric(numericAmount) ? Integer.parseInt(numericAmount) : AbilityUtils.calculateAmount(host, numericAmount, sa);
 
         for (final Player p : tgtPlayers) {
+            if (!p.isInGame()) {
+                continue;
+            }
             final CardCollection chosen = new CardCollection();
-            if (!sa.usesTargeting() || p.canBeTargetedBy(sa)) {
             for (int i = 0; i < validAmount; i++) {
                 final String choiceTitle = sa.hasParam("ChoiceTitle") ? sa.getParam("ChoiceTitle") : Localizer.getInstance().getMessage("lblChooseSource") + " ";
                 Card o = null;
@@ -146,5 +148,4 @@ public class ChooseSourceEffect extends SpellAbilityEffect {
             }
         }
     }
-    }
 }

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

@@ -13,9 +13,9 @@ import forge.game.card.CardCollectionView;
 import forge.game.card.CardFactoryUtil;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import forge.game.zone.ZoneType;
 import forge.util.Aggregates;
+import forge.util.Lang;
 
 public class ChooseTypeEffect extends SpellAbilityEffect {
 
@@ -24,9 +24,7 @@ public class ChooseTypeEffect extends SpellAbilityEffect {
         final StringBuilder sb = new StringBuilder();
 
         if (!sa.usesTargeting()) {
-            for (final Player p : getTargetPlayers(sa)) {
+            sb.append(Lang.joinHomogenous(getTargetPlayers(sa)));
-                sb.append(p);
-            }
             sb.append(" chooses a ").append(sa.getParam("Type").toLowerCase()).append(" type.");
         } else {
             sb.append("Please improve the stack description.");
@@ -103,14 +101,11 @@ public class ChooseTypeEffect extends SpellAbilityEffect {
             }
         }
 
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
-
         if (validTypes.isEmpty() && sa.hasParam("Note")) {
             // OK to end up with no choices/have nothing new to note
         } else if (!validTypes.isEmpty()) {
             for (final Player p : tgtPlayers) {
                 String choice;
-                if ((tgt == null) || p.canBeTargetedBy(sa)) {
                 Player noNotify = p;
                 if (sa.hasParam("AtRandom")) {
                     choice = Aggregates.random(validTypes);
@@ -127,7 +122,6 @@ public class ChooseTypeEffect extends SpellAbilityEffect {
                 }
                 p.getGame().getAction().notifyOfValue(sa, p, choice, noNotify);
             }
-            }
         } else {
             throw new InvalidParameterException(sa.getHostCard() + "'s ability resulted in no types to choose from");
         }

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

@@ -37,7 +37,7 @@ public class ControlPlayerEffect extends SpellAbilityEffect {
                 @Override
                 public void run() {
                     // CR 800.4b
-                    if (controller.hasLost()) {
+                    if (!controller.isInGame()) {
                         return;
                     }
 

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

@@ -135,6 +135,9 @@ public class CopyPermanentEffect extends TokenEffectBase {
         }
 
         for (final Player controller : controllers) {
+            if (!controller.isInGame()) {
+                continue;
+            }
             List<Card> tgtCards = Lists.newArrayList();
 
             if (sa.hasParam("ValidSupportedCopy")) {
@@ -231,10 +234,8 @@ public class CopyPermanentEffect extends TokenEffectBase {
                     continue;
                 }
 
-                // if it only targets player, it already got all needed cards from defined
+                // because copy should be able to copy LKI values, don't handle target and timestamp there
-                if (sa.usesTargeting() && !sa.getTargetRestrictions().canTgtPlayer() && !c.canBeTargetedBy(sa)) {
+
-                    continue;
-                }
                 if (sa.hasParam("ForEach")) {
                     for (Player p : AbilityUtils.getDefinedPlayers(host, sa.getParam("ForEach"), sa)) {
                         Card proto = getProtoType(sa, c, controller);

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

@@ -453,7 +453,6 @@ public class CountersPutEffect extends SpellAbilityEffect {
                     }
                     counterAmount = sa.usesTargeting() && sa.isDividedAsYouChoose() ? sa.getDividedValue(gameCard)
                             : counterAmount;
-                    if (!sa.usesTargeting() || gameCard.canBeTargetedBy(sa)) {
                     if (max != -1) {
                         counterAmount = Math.max(Math.min(max - gameCard.getCounters(counterType), counterAmount),
                                 0);
@@ -570,7 +569,6 @@ public class CountersPutEffect extends SpellAbilityEffect {
                     if (sa.isDividedAsYouChoose() && !sa.usesTargeting()) {
                         counterRemain = counterRemain - counterAmount;
                     }
-                    }
                 } else if (obj instanceof Player) {
                     // Add Counters to players!
                     Player pl = (Player) obj;

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

@@ -79,7 +79,6 @@ public class CountersPutOrRemoveEffect extends SpellAbilityEffect {
                             CardTranslation.getTranslatedName(gameCard.getName())), null)) {
                 continue;
             }
-            if (!sa.usesTargeting() || gameCard.canBeTargetedBy(sa)) {
             if (gameCard.hasCounters()) {
                 if (eachExisting) {
                     for (CounterType listType : Lists.newArrayList(gameCard.getCounters().keySet())) {
@@ -92,7 +91,6 @@ public class CountersPutOrRemoveEffect extends SpellAbilityEffect {
                 gameCard.addCounter(ctype, counterAmount, pl, table);
             }
         }
-        }
         table.replaceCounterEffect(game, sa, true);
     }
 

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

@@ -103,8 +103,10 @@ public class CountersRemoveEffect extends SpellAbilityEffect {
         boolean rememberAmount = sa.hasParam("RememberAmount");
 
         for (final Player tgtPlayer : getTargetPlayers(sa)) {
+            if (!tgtPlayer.isInGame()) {
+                continue;
+            }
             // Removing energy
-            if (!sa.usesTargeting() || tgtPlayer.canBeTargetedBy(sa)) {
             if (type.equals("All")) {
                 for (Map.Entry<CounterType, Integer> e : Lists.newArrayList(tgtPlayer.getCounters().entrySet())) {
                     tgtPlayer.subtractCounter(e.getKey(), e.getValue());
@@ -120,7 +122,6 @@ public class CountersRemoveEffect extends SpellAbilityEffect {
                 }
             }
         }
-        }
 
         CardCollectionView srcCards = null;
 
@@ -160,7 +161,6 @@ public class CountersRemoveEffect extends SpellAbilityEffect {
             if (gameCard == null || !tgtCard.equalsWithTimestamp(gameCard)) {
                 continue;
             }
-            if (!sa.usesTargeting() || gameCard.canBeTargetedBy(sa)) {
             final Zone zone = game.getZoneOf(gameCard);
             if (type.equals("All")) {
                 for (Map.Entry<CounterType, Integer> e : Lists.newArrayList(gameCard.getCounters().entrySet())) {
@@ -200,7 +200,6 @@ public class CountersRemoveEffect extends SpellAbilityEffect {
                 }
             }
         }
-        }
 
         if (totalRemoved > 0 && rememberAmount) {
             // TODO use SpellAbility Remember later

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

@@ -272,16 +272,12 @@ public class DamageDealEffect extends DamageBaseEffect {
                     if (c.isPhasedOut()) {
                         continue;
                     }
-                    if (!sa.usesTargeting() || gc.canBeTargetedBy(sa)) {
                     internalDamageDeal(sa, sourceLKI, gc, dmg, damageMap);
-                    }
                 } else if (o instanceof Player) {
                     final Player p = (Player) o;
-                    if (!sa.usesTargeting() || p.canBeTargetedBy(sa)) {
                     damageMap.put(sourceLKI, p, dmg);
                 }
             }
-            }
             for (final Card unTgtC : untargetedCards) {
                 if (unTgtC.isInPlay()) {
                     internalDamageDeal(sa, sourceLKI, unTgtC, dmg, damageMap);

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

@@ -1,10 +1,7 @@
 package forge.game.ability.effects;
 
-import java.util.List;
-
 import forge.game.Game;
 import forge.game.GameEntityCounterTable;
-import forge.game.GameObject;
 import forge.game.ability.AbilityUtils;
 import forge.game.card.Card;
 import forge.game.card.CardDamageMap;
@@ -68,10 +65,6 @@ public class DamageEachEffect extends DamageBaseEffect {
             sources = CardLists.getValidCards(sources, sa.getParam("ValidCards"), sa.getActivatingPlayer(), card, sa);
         }
 
-        final List<GameObject> tgts = getTargets(sa, "DefinedPlayers");
-
-        final boolean targeted = sa.usesTargeting();
-
         boolean usedDamageMap = true;
         CardDamageMap damageMap = sa.getDamageMap();
         CardDamageMap preventMap = sa.getPreventMap();
@@ -85,7 +78,7 @@ public class DamageEachEffect extends DamageBaseEffect {
             usedDamageMap = false;
         }
 
-        for (final Object o : tgts) {
+        for (final Object o : getTargetEntities(sa, "DefinedPlayers")) {
             for (final Card source : sources) {
                 final Card sourceLKI = game.getChangeZoneLKIInfo(source);
 
@@ -94,15 +87,12 @@ public class DamageEachEffect extends DamageBaseEffect {
 
                 if (o instanceof Card) {
                     final Card c = (Card) o;
-                    if (c.isInPlay() && (!targeted || c.canBeTargetedBy(sa))) {
+                    if (c.isInPlay()) {
                         damageMap.put(sourceLKI, c, dmg);
                     }
 
                 } else if (o instanceof Player) {
-                    final Player p = (Player) o;
+                    damageMap.put(sourceLKI, (Player) o, dmg);
-                    if (!targeted || p.canBeTargetedBy(sa)) {
-                        damageMap.put(sourceLKI, p, dmg);
-                    }
                 }
             }
         }

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

@@ -93,27 +93,22 @@ public class DamagePreventEffect extends DamagePreventEffectBase {
 
         final CardCollection untargetedCards = CardUtil.getRadiance(sa);
 
-        final boolean targeted = sa.usesTargeting();
-
         for (final GameObject o : tgts) {
-            numDam = targeted && sa.isDividedAsYouChoose() ? sa.getDividedValue(o) : numDam;
+            numDam = sa.usesTargeting() && sa.isDividedAsYouChoose() ? sa.getDividedValue(o) : numDam;
             if (o instanceof Card) {
                 final Card c = (Card) o;
-                if (c.isInPlay() && (!targeted || c.canBeTargetedBy(sa))) {
+                if (c.isInPlay()) {
                     addPreventNextDamage(sa, o, numDam);
                 }
             } else if (o instanceof Player) {
-                final Player p = (Player) o;
-                if (!targeted || p.canBeTargetedBy(sa)) {
                 addPreventNextDamage(sa, o, numDam);
             }
         }
-        }
 
         for (final Card c : untargetedCards) {
             if (c.isInPlay()) {
                 addPreventNextDamage(sa, c, numDam);
             }
         }
-    } // preventDamageResolve
+    }
 }

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

@@ -69,13 +69,25 @@ public class DebuffEffect extends SpellAbilityEffect {
         final long timestamp = game.getNextTimestamp();
 
         for (final Card tgtC : getTargetCards(sa)) {
+            if (!tgtC.isInPlay()) {
+                continue;
+            }
             if (tgtC.isPhasedOut()) {
                 continue;
             }
 
+            // check if the object is still in game or if it was moved
+            Card gameCard = game.getCardState(tgtC, null);
+            // gameCard is LKI in that case, the card is not in game anymore
+            // or the timestamp did change
+            // this should check Self too
+            if (gameCard == null || !tgtC.equalsWithTimestamp(gameCard)) {
+                continue;
+            }
+
             final List<String> addedKW = Lists.newArrayList();
             final List<String> removedKW = Lists.newArrayList();
-            if (tgtC.isInPlay() && (!sa.usesTargeting() || tgtC.canBeTargetedBy(sa))) {
+
             if (sa.hasParam("AllSuffixKeywords")) {
                 // this only for walk abilities, may to try better
                 if (sa.getParam("AllSuffixKeywords").equals("walk")) {
@@ -142,7 +154,7 @@ public class DebuffEffect extends SpellAbilityEffect {
 
             removedKW.addAll(kws);
             tgtC.addChangedCardKeywords(addedKW, removedKW, false, timestamp, 0);
-            }
+
             if (!"Permanent".equals(sa.getParam("Duration"))) {
                 final GameCommand until = new GameCommand() {
                     private static final long serialVersionUID = 5387486776282932314L;
@@ -155,6 +167,6 @@ public class DebuffEffect extends SpellAbilityEffect {
                 addUntilCommand(sa, until);
             }
         }
-    } // debuffResolve
+    }
 
 }

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

@@ -64,7 +64,9 @@ public class DestroyEffect extends SpellAbilityEffect {
         CardZoneTable table = new CardZoneTable();
         Map<Integer, Card> cachedMap = Maps.newHashMap();
         for (final Card tgtC : tgtCards) {
-            if (tgtC.isInPlay() && (!sa.usesTargeting() || tgtC.canBeTargetedBy(sa))) {
+            if (!tgtC.isInPlay()) {
+                continue;
+            }
             Card gameCard = game.getCardState(tgtC, null);
             // gameCard is LKI in that case, the card is not in game anymore
             // or the timestamp did change
@@ -74,7 +76,6 @@ public class DestroyEffect extends SpellAbilityEffect {
             }
             internalDestroy(gameCard, sa, table, cachedMap, params);
         }
-        }
 
         if (untargetedCards.size() > 1) {
             untargetedCards = GameActionUtil.orderCardsByTheirOwners(game, untargetedCards, ZoneType.Graveyard, sa);

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

@@ -15,7 +15,6 @@ import forge.game.player.DelayedReveal;
 import forge.game.player.Player;
 import forge.game.player.PlayerView;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import forge.game.zone.PlayerZone;
 import forge.game.zone.ZoneType;
 import forge.util.CardTranslation;
@@ -169,19 +168,17 @@ public class DigEffect extends SpellAbilityEffect {
             }
         }
 
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
-        final List<Player> tgtPlayers = getDefinedPlayersOrTargeted(sa);
-
         CardZoneTable table = new CardZoneTable();
         GameEntityCounterTable counterTable = new GameEntityCounterTable();
         boolean combatChanged = false;
         CardCollectionView lastStateBattlefield = game.copyLastStateBattlefield();
         CardCollectionView lastStateGraveyard = game.copyLastStateGraveyard();
 
-        for (final Player p : tgtPlayers) {
+        for (final Player p : getDefinedPlayersOrTargeted(sa)) {
-            if (tgt != null && !p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
                 continue;
             }
+
             final CardCollection top = new CardCollection();
             final CardCollection rest = new CardCollection();
             CardCollection all = new CardCollection(p.getCardsIn(srcZone));

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

@@ -40,7 +40,7 @@ public class DigMultipleEffect extends SpellAbilityEffect {
 
         CardZoneTable table = new CardZoneTable();
         for (final Player chooser : getDefinedPlayersOrTargeted(sa)) {
-            if (sa.usesTargeting() && !chooser.canBeTargetedBy(sa)) {
+            if (!chooser.isInGame()) {
                 continue;
             }
             final CardCollection top = new CardCollection();

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

@@ -122,10 +122,9 @@ public class DigUntilEffect extends SpellAbilityEffect {
         CardCollectionView lastStateGraveyard = game.copyLastStateGraveyard();
 
         for (final Player p : getTargetPlayers(sa)) {
-            if (p == null) {
+            if (p == null || !p.isInGame()) {
                 continue;
             }
-            if (!sa.usesTargeting() || p.canBeTargetedBy(sa)) {
             if (optional && !p.getController().confirmAction(sa, null, Localizer.getInstance().getMessage("lblDoYouWantDigYourLibrary"), null)) {
                 continue;
             }
@@ -261,7 +260,6 @@ public class DigUntilEffect extends SpellAbilityEffect {
                 p.shuffle(sa);
             }
         } // end foreach player
-        }
         if (combatChanged) {
             game.updateCombatForView();
             game.fireEvent(new GameEventCombatChanged());

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

@@ -134,9 +134,6 @@ public class DiscardEffect extends SpellAbilityEffect {
             // In this case the target need not be the discarding player
             discarders = getDefinedPlayersOrTargeted(sa);
             firstTarget = Iterables.getFirst(targets, null);
-            if (sa.usesTargeting() && !firstTarget.canBeTargetedBy(sa)) {
-                firstTarget = null;
-            }
         } else {
             discarders = targets;
         }
@@ -144,6 +141,10 @@ public class DiscardEffect extends SpellAbilityEffect {
         final CardZoneTable table = new CardZoneTable();
         Map<Player, CardCollectionView> discardedMap = Maps.newHashMap();
         for (final Player p : discarders) {
+            if (!p.isInGame()) {
+                continue;
+            }
+
             CardCollectionView toBeDiscarded = new CardCollection();
             if ((mode.equals("RevealTgtChoose") && firstTarget != null) || !sa.usesTargeting() || p.canBeTargetedBy(sa)) {
                 final int numCardsInHand = p.getCardsIn(ZoneType.Hand).size();
@@ -268,9 +269,6 @@ public class DiscardEffect extends SpellAbilityEffect {
                         dPHand = p.getController().chooseCardsToRevealFromHand(amount, amount, dPHand);
                     }
 
-                    final String valid = sa.getParamOrDefault("DiscardValid", "Card");
-                    CardCollection validCards = CardLists.getValidCards(dPHand, valid, source.getController(), source, sa);
-
                     Player chooser = p;
                     if (mode.endsWith("YouChoose")) {
                         chooser = source.getController();
@@ -289,6 +287,9 @@ public class DiscardEffect extends SpellAbilityEffect {
                         continue;
                     }
 
+                    final String valid = sa.getParamOrDefault("DiscardValid", "Card");
+                    CardCollection validCards = CardLists.getValidCards(dPHand, valid, source.getController(), source, sa);
+
                     int min = sa.hasParam("AnyNumber") || sa.hasParam("Optional") ? 0 : Math.min(validCards.size(), numCards);
                     int max = sa.hasParam("AnyNumber") ? validCards.size() : Math.min(validCards.size(), numCards);
 

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

@@ -3,22 +3,19 @@ package forge.game.ability.effects;
 import java.util.ArrayList;
 import java.util.List;
 
-import org.apache.commons.lang3.StringUtils;
-
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.mana.Mana;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
+import forge.util.Lang;
 
 public class DrainManaEffect extends SpellAbilityEffect {
     @Override
     protected String getStackDescription(SpellAbility sa) {
         final StringBuilder sb = new StringBuilder();
 
-        final List<Player> tgtPlayers = getTargetPlayers(sa);
+        sb.append(Lang.joinHomogenous(getTargetPlayers(sa)));
 
-        sb.append(StringUtils.join(tgtPlayers, ", "));
         sb.append(" loses all unspent mana.");
 
         return sb.toString();
@@ -26,13 +23,13 @@ public class DrainManaEffect extends SpellAbilityEffect {
 
     @Override
     public void resolve(SpellAbility sa) {
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
         List<Mana> drained = new ArrayList<>();
 
         for (final Player p : getTargetPlayers(sa)) {
-            if ((tgt == null) || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
-                drained.addAll(p.getManaPool().clearPool(false));
+                continue;
             }
+            drained.addAll(p.getManaPool().clearPool(false));
         }
 
         if (sa.hasParam("DrainMana")) {

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

@@ -62,8 +62,7 @@ public class DrawEffect extends SpellAbilityEffect {
         moveParams.put(AbilityKey.LastStateGraveyard, sa.getLastStateGraveyard());
 
         for (final Player p : getDefinedPlayersOrTargeted(sa)) {
-            // TODO can this be removed?
+            if (!p.isInGame()) {
-            if (sa.usesTargeting() && !p.canBeTargetedBy(sa)) {
                 continue;
             }
 

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

@@ -86,7 +86,6 @@ public class FlipOntoBattlefieldEffect extends SpellAbilityEffect {
     protected String getStackDescription(SpellAbility sa) {
         final StringBuilder sb = new StringBuilder();
         final Card host = sa.getHostCard();
-        final Player p = sa.getActivatingPlayer();
 
         sb.append("Flip ");
         sb.append(host.toString());

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

@@ -6,7 +6,6 @@ import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.zone.ZoneType;
 import forge.util.Lang;
 
 import java.util.List;
@@ -36,12 +35,7 @@ public class GoadEffect extends SpellAbilityEffect {
 
         for (final Card tgtC : getDefinedCardsOrTargeted(sa)) {
             // only goad things on the battlefield
-            if (!game.getCardsIn(ZoneType.Battlefield).contains(tgtC)) {
+            if (!tgtC.isInPlay()) {
-                continue;
-            }
-
-            // make sure we can still target now if using targeting
-            if (sa.usesTargeting() && !sa.getTargetRestrictions().canTgtPlayer() && !tgtC.canBeTargetedBy(sa)) {
                 continue;
             }
 

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

@@ -58,7 +58,9 @@ public class LifeGainEffect extends SpellAbilityEffect {
         }
 
         for (final Player p : tgtPlayers) {
-            if (!sa.usesTargeting() || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
             if (variableAmount) {
                 sa.setSVar("AFNotDrawnNum", sa.getSVar("AFNotDrawnNum_" + p.getId()));
                 lifeAmount = AbilityUtils.calculateAmount(sa.getHostCard(), amount, sa);
@@ -66,6 +68,5 @@ public class LifeGainEffect extends SpellAbilityEffect {
             p.gainLife(lifeAmount, sa.getHostCard(), sa);
         }
     }
-    }
 
 }

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

@@ -1,6 +1,5 @@
 package forge.game.ability.effects;
 
-
 import forge.game.ability.AbilityUtils;
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.player.Player;
@@ -43,9 +42,10 @@ public class LifeLoseEffect extends SpellAbilityEffect {
         final int lifeAmount = AbilityUtils.calculateAmount(sa.getHostCard(), sa.getParam("LifeAmount"), sa);
 
         for (final Player p : getTargetPlayers(sa)) {
-            if (!sa.usesTargeting() || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
-                lifeLost += p.loseLife(lifeAmount, false, false);
+                continue;
             }
+            lifeLost += p.loseLife(lifeAmount, false, false);
         }
         sa.setSVar("AFLifeLost", "Number$" + lifeLost);
     }

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

@@ -10,7 +10,6 @@ import forge.game.ability.SpellAbilityEffect;
 import forge.game.player.Player;
 import forge.game.player.PlayerCollection;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import forge.util.Localizer;
 
 public class LifeSetEffect extends SpellAbilityEffect {
@@ -22,20 +21,22 @@ public class LifeSetEffect extends SpellAbilityEffect {
     public void resolve(SpellAbility sa) {
         final boolean redistribute = sa.hasParam("Redistribute");
         final int lifeAmount = redistribute ? 0 : AbilityUtils.calculateAmount(sa.getHostCard(), sa.getParam("LifeAmount"), sa);
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
         final List<Integer> lifetotals = new ArrayList<>();
         final PlayerCollection players = getTargetPlayers(sa);
 
         if (redistribute) {
             for (final Player p : players) {
-                if (tgt == null || p.canBeTargetedBy(sa)) {
+                if (!p.isInGame()) {
-                    lifetotals.add(p.getLife());
+                    continue;
                 }
+                lifetotals.add(p.getLife());
             }
         }
 
         for (final Player p : players.threadSafeIterable()) {
-            if (tgt == null || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
             if (!redistribute) {
                 p.setLife(lifeAmount, sa);
             } else {
@@ -47,7 +48,6 @@ public class LifeSetEffect extends SpellAbilityEffect {
             }
         }
     }
-    }
 
     private static List<Integer> getDistribution(PlayerCollection players, boolean top, List<Integer> remainingChoices) {
         // distribution was successful
@@ -87,7 +87,7 @@ public class LifeSetEffect extends SpellAbilityEffect {
                 return validChoices;
             }
         }
-        return new ArrayList<Integer>();
+        return new ArrayList<>();
     }
 
     /* (non-Javadoc)

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

@@ -2,30 +2,18 @@ package forge.game.ability.effects;
 
 import forge.game.Game;
 import forge.game.ability.SpellAbilityEffect;
-import forge.game.card.Card;
-import forge.game.card.CardCollection;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import forge.util.Lang;
 
 public class LookAtEffect extends SpellAbilityEffect {
 
     @Override
     public void resolve(final SpellAbility sa) {
-        final Card host = sa.getHostCard();
+        final Game game = sa.getHostCard().getGame();
-        final Game game = host.getGame();
         final Player activator = sa.getActivatingPlayer();
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
 
-        final CardCollection targets = new CardCollection();
+        game.getAction().revealTo(getTargetCards(sa), activator);
-        for (final Card tgtCard : getTargetCards(sa)) {
-            if (tgt == null || tgtCard.canBeTargetedBy(sa)) {
-                targets.add(tgtCard);
-            }
-        }
-
-        game.getAction().revealTo(targets, activator);
     }
 
     @Override

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

@@ -48,8 +48,7 @@ public class ManaEffect extends SpellAbilityEffect {
         final StringBuilder producedMana = new StringBuilder();
 
         for (Player p : tgtPlayers) {
-            if (sa.usesTargeting() && !p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
-                // Illegal target. Skip.
                 continue;
             }
 

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

@@ -37,7 +37,6 @@ public class ManifestEffect extends SpellAbilityEffect {
         moveParams.put(AbilityKey.LastStateGraveyard, lastStateGraveyard);
 
         for (final Player p : getTargetPlayers(sa, "DefinedPlayer")) {
-            if (sa.usesTargeting() || p.canBeTargetedBy(sa)) {
             CardCollection tgtCards;
             if (sa.hasParam("Choices") || sa.hasParam("ChoiceZone")) {
                 ZoneType choiceZone = ZoneType.Hand;
@@ -73,5 +72,4 @@ public class ManifestEffect extends SpellAbilityEffect {
             }
         }
     }
-    }
 }

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

@@ -44,7 +44,10 @@ public class MillEffect extends SpellAbilityEffect {
         moveParams.put(AbilityKey.LastStateGraveyard, sa.getLastStateGraveyard());
 
         for (final Player p : getTargetPlayers(sa)) {
-            if (!sa.usesTargeting() || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
+
             if (sa.hasParam("Optional")) {
                 final String prompt = TextUtil.concatWithSpace(Localizer.getInstance().getMessage("lblDoYouWantPutLibraryCardsTo", destination.getTranslatedName()));
                 // CR 701.13b
@@ -83,7 +86,6 @@ public class MillEffect extends SpellAbilityEffect {
                 source.addImprintedCards(milled);
             }
         }
-        }
 
         // run trigger if something got milled
         table.triggerChangesZoneAll(game, sa);

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

@@ -16,7 +16,6 @@ import forge.game.card.CardCollectionView;
 import forge.game.card.CardLists;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import forge.game.zone.ZoneType;
 import forge.util.Aggregates;
 import forge.util.Localizer;
@@ -56,7 +55,6 @@ public class MultiplePilesEffect extends SpellAbilityEffect {
 
         final String valid = sa.getParamOrDefault("ValidCards", "");
 
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
         final List<Player> tgtPlayers = getTargetPlayers(sa);
         // starting with the activator
         int pSize = tgtPlayers.size();
@@ -66,7 +64,10 @@ public class MultiplePilesEffect extends SpellAbilityEffect {
         }
 
         for (final Player p : tgtPlayers) {
-            if ((tgt == null) || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
+ 
             CardCollection pool;
             if (sa.hasParam("DefinedCards")) {
                 pool = AbilityUtils.getDefinedCards(source, sa.getParam("DefinedCards"), sa);
@@ -88,7 +89,6 @@ public class MultiplePilesEffect extends SpellAbilityEffect {
             p.getGame().getAction().notifyOfValue(sa, p, pileList.toString(), p);
             record.put(p, pileList);
         }
-        }
         if (randomChosen) {
             for (Entry<Player, List<CardCollectionView>> ev : record.entrySet()) {
                 CardCollectionView chosen = Aggregates.random(ev.getValue());

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

@@ -62,7 +62,6 @@ public class MustBlockEffect extends SpellAbilityEffect {
         long ts = game.getNextTimestamp();
 
         for (final Card c : tgtCards) {
-            if ((!sa.usesTargeting()) || c.canBeTargetedBy(sa)) {
             if (mustBlockAll) {
                 c.addMustBlockCards(ts, cards);
             } else {
@@ -70,7 +69,6 @@ public class MustBlockEffect extends SpellAbilityEffect {
                 c.addMustBlockCard(ts, attacker);
             }
         }
-        }
 
         if (sa.hasParam("Duration")) {
             final GameCommand removeBlockingRequirements = new GameCommand() {

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

@@ -29,15 +29,18 @@ public class PoisonEffect extends SpellAbilityEffect {
         final int amount = AbilityUtils.calculateAmount(host, sa.getParam("Num"), sa);
 
         GameEntityCounterTable table = new GameEntityCounterTable();
+
         for (final Player p : getTargetPlayers(sa)) {
-            if ((!sa.usesTargeting()) || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
+
             if (amount >= 0) {
                 p.addPoisonCounters(amount, sa.getActivatingPlayer(), table);
             } else {
                 p.removePoisonCounters(-amount, sa.getActivatingPlayer());
             }
         }
-        }
         table.replaceCounterEffect(game, sa, true);
     }
 

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

@@ -39,7 +39,7 @@ public class ProtectAllEffect extends SpellAbilityEffect {
         }
 
         return sb.toString();
-    } // protectStackDescription()
+    }
 
     @Override
     public void resolve(SpellAbility sa) {
@@ -83,11 +83,9 @@ public class ProtectAllEffect extends SpellAbilityEffect {
         // Deal with permanents
         final String valid = sa.getParamOrDefault("ValidCards", "");
         if (!valid.equals("")) {
-            CardCollectionView list = game.getCardsIn(ZoneType.Battlefield);
+            CardCollectionView list = CardLists.getValidCards(game.getCardsIn(ZoneType.Battlefield), valid, sa.getActivatingPlayer(), host, sa);
-            list = CardLists.getValidCards(list, valid, sa.getActivatingPlayer(), host, sa);
 
             for (final Card tgtC : list) {
-                if (tgtC.isInPlay()) {
                 tgtC.addChangedCardKeywords(gainsKWList, null, false, timestamp, 0, true);
 
                 if (!"Permanent".equals(sa.getParam("Duration"))) {
@@ -106,16 +104,12 @@ public class ProtectAllEffect extends SpellAbilityEffect {
                 }
             }
         }
-        }
 
         // Deal with Players
         final String players = sa.getParamOrDefault("ValidPlayers", "");
         if (!players.equals("")) {
-            final List<Player> playerList = AbilityUtils.getDefinedPlayers(host, players, sa);
+            for (final Player player : AbilityUtils.getDefinedPlayers(host, players, sa)) {
-            for (final Player player : playerList) {
+                player.addChangedKeywords(gainsKWList, ImmutableList.of(), timestamp, 0);
-                for (final String gain : gains) {
-                    player.addChangedKeywords(ImmutableList.of("Protection from " + gain), ImmutableList.of(), timestamp, 0);
-                }
 
                 if (!"Permanent".equals(sa.getParam("Duration"))) {
                     // If not Permanent, remove protection at EOT

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

@@ -90,7 +90,7 @@ public class ProtectEffect extends SpellAbilityEffect {
         }
 
         return sb.toString();
-    } // protectStackDescription()
+    }
 
     @Override
     public void resolve(SpellAbility sa) {
@@ -140,11 +140,6 @@ public class ProtectEffect extends SpellAbilityEffect {
                 continue;
             }
 
-            // if this is a target, make sure we can still target now
-            if (sa.usesTargeting() && !tgtC.canBeTargetedBy(sa)) {
-                continue;
-            }
-
             tgtC.addChangedCardKeywords(gainsKWList, null, false, timestamp, 0, true);
 
             if (!"Permanent".equals(sa.getParam("Duration"))) {
@@ -186,7 +181,7 @@ public class ProtectEffect extends SpellAbilityEffect {
                 addUntilCommand(sa, untilEOT);
             }
         }
-    } // protectResolve()
+    }
     
     public static List<String> getProtectionList(final SpellAbility sa) {
         final List<String> gains = new ArrayList<>();

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

@@ -199,7 +199,7 @@ public class PumpEffect extends SpellAbilityEffect {
                 sb.append(" ");
             }
 
-            if (sa instanceof AbilitySub & sa.getRootAbility().getTargets().containsAll(tgts)) {
+            if (sa instanceof AbilitySub && sa.getRootAbility().getTargets().containsAll(tgts)) {
                 //try to avoid having the same long list of targets twice in a StackDescription
                 sb.append(tgts.size() == 1 && tgts.get(0) instanceof Card ? "It " : "They ");
             } else {
@@ -266,7 +266,7 @@ public class PumpEffect extends SpellAbilityEffect {
         }
 
         return sb.toString();
-    } // pumpStackDescription()
+    }
 
     @Override
     public void resolve(final SpellAbility sa) {
@@ -274,7 +274,6 @@ public class PumpEffect extends SpellAbilityEffect {
         final Game game = activator.getGame();
         final Card host = sa.getHostCard();
         final long timestamp = game.getNextTimestamp();
-        List<GameEntity> tgts = Lists.newArrayList();
         List<Card> tgtCards = getCardsfromTargets(sa);
         List<Player> tgtPlayers = getTargetPlayers(sa);
 
@@ -296,8 +295,6 @@ public class PumpEffect extends SpellAbilityEffect {
             keywords = CardFactoryUtil.sharedKeywords(keywords, restrictions, zones, host, sa);
         }
 
-        tgts.addAll(tgtCards);
-        tgts.addAll(tgtPlayers);
         final CardCollection untargetedCards = CardUtil.getRadiance(sa);
 
         if (sa.hasParam("DefinedKW")) {
@@ -433,11 +430,6 @@ public class PumpEffect extends SpellAbilityEffect {
                 continue;
             }
 
-            // if pump is a target, make sure we can still target now
-            if (sa.usesTargeting() && !tgtC.canBeTargetedBy(sa)) {
-                continue;
-            }
-
             // substitute specific tgtC mana cost for keyword placeholder CardManaCost
             List<String> affectedKeywords = Lists.newArrayList(keywords);
 
@@ -486,7 +478,7 @@ public class PumpEffect extends SpellAbilityEffect {
         }
 
         for (Player p : tgtPlayers) {
-            if (!p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
                 continue;
             }
 
@@ -494,5 +486,5 @@ public class PumpEffect extends SpellAbilityEffect {
         }
 
         replaceDying(sa);
-    } // pumpResolve()
+    }
 }

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

@@ -11,7 +11,6 @@ import forge.game.card.CardCollection;
 import forge.game.card.CardCollectionView;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import forge.game.zone.ZoneType;
 import forge.util.Lang;
 import forge.util.Localizer;
@@ -70,19 +69,15 @@ public class RearrangeTopOfLibraryEffect extends SpellAbilityEffect {
 
     @Override
     public void resolve(SpellAbility sa) {
-        int numCards = 0;
         Card host = sa.getHostCard();
-        boolean shuffle = false;
+        int numCards = AbilityUtils.calculateAmount(host, sa.getParam("NumCards"), sa);
-
+        boolean shuffle = sa.hasParam("MayShuffle");
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
-
-        numCards = AbilityUtils.calculateAmount(host, sa.getParam("NumCards"), sa);
-        shuffle = sa.hasParam("MayShuffle");
 
         for (final Player p : getTargetPlayers(sa)) {
-            if ((tgt == null) || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
-                rearrangeTopOfLibrary(host, p, numCards, shuffle, sa);
+                continue;
             }
+            rearrangeTopOfLibrary(host, p, numCards, shuffle, sa);
         }
     }
 

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

@@ -12,7 +12,6 @@ import forge.game.card.CardCollection;
 import forge.game.combat.Combat;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 
 public class RemoveFromCombatEffect extends SpellAbilityEffect {
 
@@ -34,11 +33,13 @@ public class RemoveFromCombatEffect extends SpellAbilityEffect {
         final Player activator = sa.getActivatingPlayer();
         final Game game = activator.getGame();
         final boolean rem = sa.hasParam("RememberRemovedFromCombat");
-
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
-        for (final Card c : getTargetCards(sa)) {
         final Combat combat = game.getPhaseHandler().getCombat();
-            if (combat != null && (tgt == null || c.canBeTargetedBy(sa))) {
+
+        for (final Card c : getTargetCards(sa)) {
+            if (combat == null || !c.isInPlay()) {
+                continue;
+            }
+
             // Unblock creatures that were blocked only by this card (e.g. Ydwen Efreet)
             if (sa.hasParam("UnblockCreaturesBlockedOnlyBy")) {
                 CardCollection attackers = AbilityUtils.getDefinedCards(sa.getHostCard(), sa.getParam("UnblockCreaturesBlockedOnlyBy"), sa);
@@ -67,5 +68,4 @@ public class RemoveFromCombatEffect extends SpellAbilityEffect {
             }
         }
     }
-    }
 }

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

@@ -7,7 +7,6 @@ import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.CardCollection;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import forge.game.zone.ZoneType;
 import forge.util.Lang;
 import forge.util.MyRandom;
@@ -26,10 +25,12 @@ public class ReorderZoneEffect extends SpellAbilityEffect {
     public void resolve(SpellAbility sa) {
         final ZoneType zone = ZoneType.smartValueOf(sa.getParam("Zone"));
         boolean shuffle = sa.hasParam("Random");
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
 
         for (final Player p : getTargetPlayers(sa)) {
-            if ((tgt == null) || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
+
             CardCollection list = new CardCollection(p.getCardsIn(zone));
             if (shuffle) {
                 Collections.shuffle(list, MyRandom.getRandom());
@@ -39,5 +40,4 @@ public class ReorderZoneEffect extends SpellAbilityEffect {
             }
         }
     }
-    }
 }

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

@@ -29,7 +29,9 @@ public class RevealEffect extends SpellAbilityEffect {
         int cnt = sa.hasParam("NumCards") ? AbilityUtils.calculateAmount(host, sa.getParam("NumCards"), sa) : 1;
 
         for (final Player p : getTargetPlayers(sa)) {
-            if (!sa.usesTargeting() || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
             final CardCollectionView cardsInHand = p.getZone(ZoneType.Hand).getCards();
             if (cardsInHand.isEmpty()) {
                 continue;
@@ -102,7 +104,6 @@ public class RevealEffect extends SpellAbilityEffect {
             }
         }
     }
-    }
 
     @Override
     protected String getStackDescription(SpellAbility sa) {

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

@@ -6,7 +6,6 @@ import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.*;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import forge.game.zone.ZoneType;
 import forge.util.Lang;
 import forge.util.Localizer;
@@ -36,12 +35,12 @@ public class RevealHandEffect extends SpellAbilityEffect {
     @Override
     public void resolve(SpellAbility sa) {
         final Card host = sa.getHostCard();
-
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
         final boolean optional = sa.hasParam("Optional");
 
         for (final Player p : getTargetPlayers(sa)) {
-            if ((tgt == null) || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
             if (optional && !p.getController().confirmAction(sa, null, Localizer.getInstance().getMessage("lblDoYouWantRevealYourHand"), null)) {
                 continue;
             }
@@ -65,5 +64,4 @@ public class RevealHandEffect extends SpellAbilityEffect {
             }
         }
     }
-    }
 }

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

@@ -43,8 +43,10 @@ public class ScryEffect extends SpellAbilityEffect {
 
         // Optional here for spells that have optional multi-player scrying
         for (final Player p : getTargetPlayers(sa)) {
-            if ( (!sa.usesTargeting() || p.canBeTargetedBy(sa)) &&
+            if (!p.isInGame()) {
-                    (!isOptional || p.getController().confirmAction(sa, null, Localizer.getInstance().getMessage("lblDoYouWanttoScry"), null)) ) {
+                continue;
+            }
+            if (!isOptional || p.getController().confirmAction(sa, null, Localizer.getInstance().getMessage("lblDoYouWanttoScry"), null)) {
                 players.add(p);
             }
         }

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

@@ -93,10 +93,6 @@ public class SetStateEffect extends SpellAbilityEffect {
                 continue;
             }
 
-            if (sa.usesTargeting() && !gameCard.canBeTargetedBy(sa)) {
-                continue;
-            }
-
             // Cards which are not on the battlefield should not be able to transform.
             // TurnFace should be allowed in other zones like Exile too
             // Specialize and Unspecialize are allowed in other zones

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

@@ -6,7 +6,6 @@ import java.util.List;
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import forge.util.Localizer;
 
 public class ShuffleEffect extends SpellAbilityEffect {
@@ -15,18 +14,15 @@ public class ShuffleEffect extends SpellAbilityEffect {
     public void resolve(SpellAbility sa) {
         final boolean optional = sa.hasParam("Optional");
 
-        final List<Player> tgtPlayers = getTargetPlayers(sa);
+        for (final Player p : getTargetPlayers(sa)) {
-
+            if (!p.isInGame()) {
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
+                continue;
-
+            }
-        for (final Player p : tgtPlayers) {
-            if ((tgt == null) || p.canBeTargetedBy(sa)) {
             boolean mustShuffle = !optional || sa.getActivatingPlayer().getController().confirmAction(sa, null, Localizer.getInstance().getMessage("lblHaveTargetShuffle", p.getName()), null); 
             if (mustShuffle) 
                 p.shuffle(sa);
         }
     }
-    }
 
     @Override
     protected String getStackDescription(SpellAbility sa) {

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

@@ -42,14 +42,15 @@ public class SurveilEffect extends SpellAbilityEffect {
         moveParams.put(AbilityKey.LastStateGraveyard, sa.getLastStateGraveyard());
 
         for (final Player p : getTargetPlayers(sa)) {
-            if (!sa.usesTargeting() || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
             if (isOptional && !p.getController().confirmAction(sa, null, Localizer.getInstance().getMessage("lblDoYouWantSurveil"), null)) {
                 continue;
             }
 
             p.surveil(num, sa, table, moveParams);
         }
-        }
         table.triggerChangesZoneAll(sa.getHostCard().getGame(), sa);
     }
 

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

@@ -27,9 +27,11 @@ public class TakeInitiativeEffect extends SpellAbilityEffect {
         final String set = sa.getHostCard().getSetCode();
 
         for (final Player p : getTargetPlayers(sa)) {
-            if (!sa.usesTargeting() || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
+
             p.getGame().getAction().takeInitiative(p, set);
         }
     }
-    }
 }

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

@@ -1,12 +1,9 @@
 package forge.game.ability.effects;
 
-import java.util.List;
-
 import forge.game.Game;
 import forge.game.ability.AbilityUtils;
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
-import forge.game.card.CardCollection;
 import forge.game.card.CardCollectionView;
 import forge.game.player.Player;
 import forge.game.spellability.AbilitySub;
@@ -34,17 +31,10 @@ public class TapAllEffect extends SpellAbilityEffect {
         }
 
         CardCollectionView cards;
-
-        final List<Player> tgtPlayers = getTargetPlayers(sa);
-
         if (!sa.usesTargeting() && !sa.hasParam("Defined")) {
             cards = game.getCardsIn(ZoneType.Battlefield);
         } else {
-            CardCollection cards2 = new CardCollection();
+            cards = getTargetPlayers(sa).getCardsIn(ZoneType.Battlefield);
-            for (final Player p : tgtPlayers) {
-                cards2.addAll(p.getCardsIn(ZoneType.Battlefield));
-            }
-            cards = cards2;
         }
 
         cards = AbilityUtils.filterListByType(cards, sa.getParam("ValidCards"), sa);

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

@@ -23,9 +23,6 @@ public class TapEffect extends SpellAbilityEffect {
             if (tgtC.isPhasedOut()) {
                 continue;
             }
-            if (sa.usesTargeting() && !tgtC.canBeTargetedBy(sa)) {
-                continue;
-            }
             if (tgtC.isInPlay()) {
                 if (tgtC.isUntapped() && remTapped || alwaysRem) {
                     card.addRemembered(tgtC);

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

@@ -1,8 +1,5 @@
 package forge.game.ability.effects;
 
-import java.util.List;
-
-import org.apache.commons.lang3.StringUtils;
 
 import forge.game.Game;
 import forge.game.ability.AbilityUtils;
@@ -14,9 +11,8 @@ import forge.game.player.Player;
 import forge.game.player.PlayerController;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.ZoneType;
+import forge.util.Lang;
 import forge.util.Localizer;
-import forge.util.collect.FCollection;
-
 
 public class TapOrUntapAllEffect extends SpellAbilityEffect {
 
@@ -29,8 +25,7 @@ public class TapOrUntapAllEffect extends SpellAbilityEffect {
         if (sa.hasParam("ValidMessage")) {
             sb.append(sa.getParam("ValidMessage"));
         } else {
-            final List<Card> tgtCards = getTargetCards(sa);
+            sb.append(Lang.joinHomogenous(getTargetCards(sa)));
-            sb.append(StringUtils.join(tgtCards, ", "));
         }
         sb.append(".");
         return sb.toString();
@@ -38,19 +33,18 @@ public class TapOrUntapAllEffect extends SpellAbilityEffect {
 
     @Override
     public void resolve(SpellAbility sa) {
-        CardCollectionView validCards = getTargetCards(sa);
         final Player activator = sa.getActivatingPlayer();
         final Game game = activator.getGame();
 
-        FCollection<Player> targetedPlayers = getTargetPlayers(sa);
+        CardCollectionView validCards;
-
         if (sa.hasParam("ValidCards")) {
-            validCards = game.getCardsIn(ZoneType.Battlefield);
+            validCards = AbilityUtils.filterListByType(game.getCardsIn(ZoneType.Battlefield), sa.getParam("ValidCards"), sa);
-            validCards = AbilityUtils.filterListByType(validCards, sa.getParam("ValidCards"), sa);
+        } else {
+            validCards = getTargetCards(sa);
         }
         
         if (sa.usesTargeting() || sa.hasParam("Defined")) {
-            validCards = CardLists.filterControlledBy(validCards, targetedPlayers);
+            validCards = CardLists.filterControlledBy(validCards, getTargetPlayers(sa));
         }
 
         // Default to tapping for AI
@@ -66,13 +60,14 @@ public class TapOrUntapAllEffect extends SpellAbilityEffect {
 
         toTap = sa.getActivatingPlayer().getController().chooseBinary(sa, sb.toString(), PlayerController.BinaryChoiceType.TapOrUntap);
 
-        for (final Card cad : validCards) {
+        for (final Card tgtC : validCards) {
-            if (cad.isInPlay()) {
+            if (!tgtC.isInPlay()) {
-                if (toTap) {
+                continue;
-                    cad.tap(true);
-                } else {
-                    cad.untap(true);
             }
+            if (toTap) {
+                tgtC.tap(true);
+            } else {
+                tgtC.untap(true);
             }
         }
     }

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

@@ -2,14 +2,12 @@ package forge.game.ability.effects;
 
 import java.util.List;
 
-import org.apache.commons.lang3.StringUtils;
-
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
 import forge.game.player.PlayerController;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import forge.util.CardTranslation;
+import forge.util.Lang;
 import forge.util.Localizer;
 
 public class TapOrUntapEffect extends SpellAbilityEffect {
@@ -24,8 +22,7 @@ public class TapOrUntapEffect extends SpellAbilityEffect {
 
         sb.append("Tap or untap ");
 
-        final List<Card> tgtCards = getTargetCards(sa);
+        sb.append(Lang.joinHomogenous(getTargetCards(sa)));
-        sb.append(StringUtils.join(tgtCards, ", "));
         sb.append(".");
         return sb.toString();
     }
@@ -33,15 +30,16 @@ public class TapOrUntapEffect extends SpellAbilityEffect {
     @Override
     public void resolve(SpellAbility sa) {
         final List<Card> tgtCards = getTargetCards(sa);
-
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
         PlayerController pc = sa.getActivatingPlayer().getController();
         
         for (final Card tgtC : tgtCards) {
+            if (!tgtC.isInPlay()) {
+                continue;
+            }
             if (tgtC.isPhasedOut()) {
                 continue;
             }
-            if (tgtC.isInPlay() && ((tgt == null) || tgtC.canBeTargetedBy(sa))) {
+
             // If the effected card is controlled by the same controller of the SA, default to untap.
             boolean tap = pc.chooseBinary(sa, Localizer.getInstance().getMessage("lblTapOrUntapTarget", CardTranslation.getTranslatedName(tgtC.getName())), PlayerController.BinaryChoiceType.TapOrUntap,
                     !tgtC.getController().equals(sa.getActivatingPlayer()) );
@@ -53,6 +51,5 @@ public class TapOrUntapEffect extends SpellAbilityEffect {
             }
         }
     }
-    }
 
 }

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

@@ -11,7 +11,6 @@ import forge.game.card.CardCollectionView;
 import forge.game.card.CardLists;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import forge.game.zone.ZoneType;
 import forge.util.Lang;
 import forge.util.Localizer;
@@ -26,15 +25,11 @@ public class TwoPilesEffect extends SpellAbilityEffect {
     protected String getStackDescription(SpellAbility sa) {
         final StringBuilder sb = new StringBuilder();
 
-        final List<Player> tgtPlayers = getTargetPlayers(sa);
-
         final String valid = sa.getParamOrDefault("ValidCards", "");
 
         sb.append("Separate all ").append(valid).append(" cards ");
 
-        for (final Player p : tgtPlayers) {
+        sb.append(Lang.joinHomogenous(getTargetPlayers(sa)));
-            sb.append(p).append(" ");
-        }
         sb.append("controls into two piles.");
         return sb.toString();
     }
@@ -55,7 +50,6 @@ public class TwoPilesEffect extends SpellAbilityEffect {
 
         final String valid = sa.getParamOrDefault("ValidCards", "Card");
 
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
         final List<Player> tgtPlayers = getTargetPlayers(sa);
 
         Player separator = card.getController();
@@ -75,7 +69,10 @@ public class TwoPilesEffect extends SpellAbilityEffect {
         }
 
         for (final Player p : tgtPlayers) {
-            if ((tgt == null) || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
+ 
             CardCollectionView pool0;
             if (sa.hasParam("DefinedCards")) {
                 pool0 = AbilityUtils.getDefinedCards(card, sa.getParam("DefinedCards"), sa);
@@ -186,7 +183,6 @@ public class TwoPilesEffect extends SpellAbilityEffect {
                 card.addRemembered(tempRemembered);
             }
         }
-        }
         if (!sa.hasParam("KeepRemembered")) {
             // prior to addition of "DefinedPiles" param, TwoPilesEffect cleared remembered objects in the
             // Chosen/Unchosen subability resolutions, so this preserves that

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

@@ -1,18 +1,14 @@
 package forge.game.ability.effects;
 
-import java.util.List;
-
-import org.apache.commons.lang3.StringUtils;
-
 import forge.game.Game;
 import forge.game.GameEntity;
-import forge.game.GameObject;
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
 import forge.game.card.CardCollectionView;
 import forge.game.card.CardLists;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.ZoneType;
+import forge.util.Lang;
 
 public class UnattachAllEffect extends SpellAbilityEffect {
     private static void handleUnattachment(final GameEntity o, final Card cardToUnattach) {
@@ -121,8 +117,7 @@ public class UnattachAllEffect extends SpellAbilityEffect {
     protected String getStackDescription(final SpellAbility sa) {
         final StringBuilder sb = new StringBuilder();
         sb.append("Unattach all valid Equipment and Auras from ");
-        final List<GameObject> targets = getTargets(sa);
+        sb.append(Lang.joinHomogenous(getTargets(sa)));
-        sb.append(StringUtils.join(targets, " "));
         return sb.toString();
     }
 
@@ -130,19 +125,14 @@ public class UnattachAllEffect extends SpellAbilityEffect {
     public void resolve(final SpellAbility sa) {
         Card source = sa.getHostCard();
         final Game game = sa.getActivatingPlayer().getGame();
-        final List<GameObject> targets = getTargets(sa);
 
         // If Cast Targets will be checked on the Stack
-        for (final Object o : targets) {
+        for (final GameEntity ge : getTargetEntities(sa)) {
-            if (!(o instanceof GameEntity)) {
-                continue;
-            }
-
             String valid = sa.getParam("UnattachValid");
             CardCollectionView unattachList = game.getCardsIn(ZoneType.Battlefield);
             unattachList = CardLists.getValidCards(unattachList, valid, source.getController(), source, sa);
             for (final Card c : unattachList) {
-                handleUnattachment((GameEntity) o, c);
+                handleUnattachment(ge, c);
             }
         }
     }

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

@@ -1,12 +1,9 @@
 package forge.game.ability.effects;
 
-import java.util.List;
-
-import org.apache.commons.lang3.StringUtils;
-
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
 import forge.game.spellability.SpellAbility;
+import forge.util.Lang;
 
 public class UnattachEffect extends SpellAbilityEffect {
     /* (non-Javadoc)
@@ -16,8 +13,7 @@ public class UnattachEffect extends SpellAbilityEffect {
     protected String getStackDescription(SpellAbility sa) {
         final StringBuilder sb = new StringBuilder();
         sb.append("Unattach ");
-        final List<Card> targets = getTargetCards(sa);
+        sb.append(Lang.joinHomogenous(getTargetCards(sa)));
-        sb.append(StringUtils.join(targets, " "));
         return sb.toString();
     }
 
@@ -26,16 +22,10 @@ public class UnattachEffect extends SpellAbilityEffect {
      */
     @Override
     public void resolve(SpellAbility sa) {
-        final List<Card> unattachList = getTargetCards(sa);
+        for (final Card cardToUnattach : getTargetCards(sa)) {
-        for (final Card cardToUnattach : unattachList) {
+            if (cardToUnattach.isAttachment() && cardToUnattach.isAttachedToEntity()) {
-            if (cardToUnattach.isAura()) {
-                //final boolean gainControl = "GainControl".equals(af.parseParams().get("AILogic"));
-                //AbilityFactoryAttach.handleUnattachAura(cardToUnattach, c, gainControl);
-            } else if (cardToUnattach.isAttachment()) {
-                if (cardToUnattach.isAttachedToEntity()) {
                 cardToUnattach.unattachFromEntity(cardToUnattach.getEntityAttachedTo());
             }
         }
     }
-    }
 }

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

@@ -1,13 +1,9 @@
 package forge.game.ability.effects;
 
-import java.util.List;
-
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
-import forge.game.card.CardCollection;
 import forge.game.card.CardCollectionView;
 import forge.game.card.CardLists;
-import forge.game.player.Player;
 import forge.game.spellability.AbilitySub;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.ZoneType;
@@ -25,17 +21,12 @@ public class UntapAllEffect extends SpellAbilityEffect {
     public void resolve(SpellAbility sa) {
         final Card card = sa.getHostCard();
         CardCollectionView list;
-        List<Player> tgtPlayers = getTargetPlayers(sa);
         final String valid = sa.getParamOrDefault("ValidCards", "");
 
         if (!sa.usesTargeting() && !sa.hasParam("Defined")) {
             list = sa.getActivatingPlayer().getGame().getCardsIn(ZoneType.Battlefield);
         } else {
-            CardCollection list2 = new CardCollection();
+            list = getTargetPlayers(sa).getCardsIn(ZoneType.Battlefield);
-            for (final Player p : tgtPlayers) {
-                list2.addAll(p.getCardsIn(ZoneType.Battlefield));
-            }
-            list = list2;
         }
         list = CardLists.getValidCards(list, valid, sa.getActivatingPlayer(), card, sa);
 

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

@@ -48,9 +48,6 @@ public class UntapEffect extends SpellAbilityEffect {
                 if (tgtC.isPhasedOut()) {
                     continue;
                 }
-                if (sa.usesTargeting() && !tgtC.canBeTargetedBy(sa)) {
-                    continue;
-                }
                 if (tgtC.isInPlay()) {
                     tgtC.untap(true);
                 }
@@ -86,6 +83,10 @@ public class UntapEffect extends SpellAbilityEffect {
         final String valid = sa.getParam("UntapType");
 
         for (final Player p : AbilityUtils.getDefinedPlayers(sa.getHostCard(), sa.getParam("Defined"), sa)) {
+            if (!p.isInGame()) {
+                continue;
+            }
+
             CardCollectionView list = CardLists.getValidCards(p.getGame().getCardsIn(ZoneType.Battlefield),
                     valid, sa.getActivatingPlayer(), sa.getHostCard(), sa);
             list = CardLists.filter(list, Presets.TAPPED);

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

@@ -132,9 +132,10 @@ public class VentureEffect  extends SpellAbilityEffect {
         moveParams.put(AbilityKey.LastStateGraveyard, sa.getLastStateGraveyard());
 
         for (final Player p : getTargetPlayers(sa)) {
-            if (!sa.usesTargeting() || p.canBeTargetedBy(sa)) {
+            if (!p.isInGame()) {
-                ventureIntoDungeon(sa, p, moveParams);
+                continue; 
             }
+            ventureIntoDungeon(sa, p, moveParams);
         }
     }
 

forge-game/src/main/java/forge/game/card/Card.java

@@ -717,8 +717,6 @@ public class Card extends GameEntity implements Comparable<Card>, IHasSVars {
         if (!turnFaceDown(true) && !isFaceDown()) {
             return null;
         }
-        // Move to p's battlefield
-        Game game = p.getGame();
 
         // Just in case you aren't the controller, now you are!
         setController(p, game.getNextTimestamp());
@@ -726,6 +724,7 @@ public class Card extends GameEntity implements Comparable<Card>, IHasSVars {
         // Mark this card as "manifested"
         setManifested(true);
 
+        // Move to p's battlefield
         Card c = game.getAction().moveToPlay(this, p, sa, params);
         if (c.isInPlay()) {
             c.setManifested(true);
@@ -3656,7 +3655,8 @@ public class Card extends GameEntity implements Comparable<Card>, IHasSVars {
         if (hasKeyword(Keyword.RECONFIGURE)) {
             // need extra time stamp so it doesn't collide with existing ones
             long ts = getGame().getNextTimestamp();
-            // TODO make it use a Static Layer Effect instead
+            // 702.151b Attaching an Equipment with reconfigure to another creature causes the Equipment to stop being a creature until it becomes unattached from that creature.
+            // it is not a Static Ability
             addChangedCardTypes(null, CardType.parse("Creature", true), false, false, false, false, false, false, false, false, ts, 0, true, false);
 
             GameCommand unattach = new GameCommand() {
@@ -6107,7 +6107,7 @@ public class Card extends GameEntity implements Comparable<Card>, IHasSVars {
 
     @Override
     public final boolean canBeTargetedBy(final SpellAbility sa) {
-        if (getOwner().hasLost()) {
+        if (!getOwner().isInGame()) {
             return false;
         }
 

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

@@ -5,10 +5,10 @@ PT:1/3
 K:Reach
 S:Mode$ Continuous | Affected$ Card.Self | AddKeyword$ Hexproof | IsPresent$ Card.Self+notattacking | Description$ Teleport — CARDNAME has hexproof unless he's attacking.
 T:Mode$ Attacks | ValidCard$ Creature.YouCtrl+withReach | TriggerZones$ Battlefield | Execute$ TrigUntap | TriggerDescription$ Whenever a creature you control with reach attacks, untap it and it can't be blocked by creatures with greater power this combat.
-SVar:TrigUntap:DB$ Untap | Defined$ TriggeredAttackerLKICopy | SubAbility$ DBAnimate
+SVar:TrigUntap:DB$ Untap | Defined$ TriggeredAttackerLKICopy | SubAbility$ DBEffect
-SVar:DBAnimate:DB$ Animate | Defined$ TriggeredAttackerLKICopy | staticAbilities$ CantBeBlockedPow | Duration$ UntilEndOfCombat
+SVar:DBEffect:DB$ Effect | RememberObjects$ TriggeredAttacker | StaticAbilities$ CantBeBlockedPow | ForgetOnMoved$ Battlefield | Duration$ UntilEndOfCombat
-SVar:CantBeBlockedPow:Mode$ CantBlockBy | ValidAttacker$ Card.Self | ValidBlocker$ Creature.powerGTX | Description$ CARDNAME can't be blocked by creatures with greater power this combat.
+SVar:CantBeBlockedPow:Mode$ CantBlockBy | ValidAttacker$ Card.IsRemembered | ValidBlocker$ Creature.powerGTX | Description$ CARDNAME can't be blocked by creatures with greater power this combat.
-SVar:X:Count$CardPower
+SVar:X:Remembered$CardPower
 T:Mode$ DamageDoneOnce | CombatDamage$ True | ValidSource$ Creature.YouCtrl | TriggerZones$ Battlefield | ValidTarget$ Player | Execute$ TrigDraw | TriggerDescription$ Fierce Punch — Whenever one or more creatures you control deal combat damage to a player, draw a card.
 SVar:TrigDraw:DB$ Draw
 DeckHints:Keyword$Reach

forge-gui/res/cardsfolder/f/fog_patch.txt

@@ -2,7 +2,6 @@ Name:Fog Patch
 ManaCost:1 G
 Types:Instant
 Text:Cast this spell only during the declare blockers step.
-A:SP$ RepeatEach | Cost$ 1 G | ActivationPhases$ Declare Blockers | RepeatSubAbility$ DBBecomeBlocked | RepeatCards$ Creature.attacking | SpellDescription$ Attacking creatures become blocked. (This spell works on creatures that can't be blocked.)
+A:SP$ BecomesBlocked | Cost$ 1 G | Defined$ Valid Creature.attacking | ActivationPhases$ Declare Blockers | SpellDescription$ Attacking creatures become blocked. (This spell works on creatures that can't be blocked.)
-SVar:DBBecomeBlocked:DB$ BecomesBlocked | Defined$ Remembered
 AI:RemoveDeck:All
 Oracle:Cast this spell only during the declare blockers step.\nAttacking creatures become blocked. (This spell works on creatures that can't be blocked.)

forge-gui/res/cardsfolder/m/mist_of_stagnation.txt

@@ -2,10 +2,8 @@ Name:Mist of Stagnation
 ManaCost:3 U U
 Types:Enchantment
 S:Mode$ Continuous | Affected$ Permanent | AddHiddenKeyword$ CARDNAME doesn't untap during your untap step. | Description$ Permanents don't untap during their controllers' untap steps.
-T:Mode$ Phase | Phase$ Upkeep | ValidPlayer$ Player | Execute$ TrigChoose | TriggerZones$ Battlefield | TriggerDescription$ At the beginning of each player's upkeep, that player chooses a permanent for each card in their graveyard, then untaps those permanents.
+T:Mode$ Phase | Phase$ Upkeep | ValidPlayer$ Player | Execute$ DBUntap | TriggerZones$ Battlefield | TriggerDescription$ At the beginning of each player's upkeep, that player chooses a permanent for each card in their graveyard, then untaps those permanents.
-SVar:TrigChoose:DB$ ChooseCard | Defined$ TriggeredPlayer | Amount$ X | Mandatory$ True | Choices$ Permanent.ActivePlayerCtrl | ChoiceTitle$ Choose a permanent to untap | AILogic$ Untap | SubAbility$ DBUntap
+SVar:DBUntap:DB$ Untap | UntapExactly$ True | Defined$ TriggeredPlayer | Amount$ X | UntapType$ Card
-SVar:DBUntap:DB$ UntapAll | ValidCards$ Permanent.ChosenCard | SubAbility$ DBCleanup
-SVar:DBCleanup:DB$ Cleanup | ClearChosenCard$ True
 SVar:X:Count$ValidGraveyard Card.ActivePlayerCtrl
 AI:RemoveDeck:Random
 SVar:NonStackingEffect:True

forge-gui/res/cardsfolder/t/triumphant_adventurer.txt

@@ -4,7 +4,7 @@ Types:Creature Human Knight
 PT:1/1
 K:Deathtouch
 S:Mode$ Continuous | Affected$ Card.Self | AddKeyword$ First Strike | Condition$ PlayerTurn | Description$ As long as it's your turn, CARDNAME has first strike.
-T:Mode$ Attacks | ValidCard$ Card.Self | Execute$ DBVenture | Secondary$ True | TriggerDescription$ Whenever CARDNAME attacks, venture into the dungeon. (Enter the first room or advance to the next room.)
+T:Mode$ Attacks | ValidCard$ Card.Self | Execute$ DBVenture | TriggerDescription$ Whenever CARDNAME attacks, venture into the dungeon. (Enter the first room or advance to the next room.)
 SVar:DBVenture:DB$ Venture | Defined$ You
 SVar:HasAttackEffect:TRUE
 Oracle:Deathtouch\nAs long as it's your turn, Triumphant Adventurer has first strike.\nWhenever Triumphant Adventurer attacks, venture into the dungeon. (Enter the first room or advance to the next room.)