Merge branch 'tamiyo_discard' into 'master'

Tamiyo: new logic for Discard effects

See merge request core-developers/forge!1540

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

@@ -150,14 +150,17 @@ public class DiscardAi extends SpellAbilityAi {
 
     private boolean discardTargetAI(final Player ai, final SpellAbility sa) {
         final TargetRestrictions tgt = sa.getTargetRestrictions();
-        Player opp = ComputerUtil.getOpponentFor(ai);
+        for (Player opp : ai.getOpponents()) {
-        if (opp.getCardsIn(ZoneType.Hand).isEmpty() && !ComputerUtil.activateForCost(sa, ai)) {
+            if (opp.getCardsIn(ZoneType.Hand).isEmpty() && !ComputerUtil.activateForCost(sa, ai)) {
-            return false;
+                continue;
-        }
+            } else if (!opp.canDiscardBy(sa)) { // e.g. Tamiyo, Collector of Tales
-        if (tgt != null) {
+                continue;
-            if (sa.canTarget(opp)) {
+            }
-                sa.getTargets().add(opp);
+            if (tgt != null) {
-                return true;
+                if (sa.canTarget(opp)) {
+                    sa.getTargets().add(opp);
+                    return true;
+                }
             }
         }
         return false;

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

@@ -48,8 +48,11 @@ public class ChooseGenericEffect extends SpellAbilityEffect {
                     // Sac a permanent in presence of Sigarda, Host of Herons
                     // TODO: generalize this by testing if the unless cost can be paid
                     if (unlessCost.startsWith("Sac<")) {
-                        if (saChoice.getActivatingPlayer().isOpponentOf(p)
+                        if (!p.canSacrificeBy(saChoice)) {
-                            && p.hasKeyword("Spells and abilities your opponents control can't cause you to sacrifice permanents.")) {
+                            saToRemove.add(saChoice);
+                        }
+                    } else if (unlessCost.startsWith("Discard<")) {
+                        if (!p.canDiscardBy(sa)) {
                             saToRemove.add(saChoice);
                         }
                     }

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

@@ -1,5 +1,6 @@
 package forge.game.ability.effects;
 
+import forge.game.Game;
 import forge.game.GameActionUtil;
 import forge.game.ability.AbilityUtils;
 import forge.game.ability.SpellAbilityEffect;
@@ -7,6 +8,7 @@ import forge.game.card.*;
 import forge.game.card.CardPredicates.Presets;
 import forge.game.player.Player;
 import forge.game.player.PlayerActionConfirmMode;
+import forge.game.player.PlayerPredicates;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.ZoneType;
 
@@ -29,9 +31,9 @@ public class DiscardEffect extends SpellAbilityEffect {
         final String mode = sa.getParam("Mode");
         final StringBuilder sb = new StringBuilder();
 
-        final List<Player> tgtPlayers = getTargetPlayers(sa);
+        final Iterable<Player> tgtPlayers = Iterables.filter(getTargetPlayers(sa), PlayerPredicates.canDiscardBy(sa));
 
-        if (!tgtPlayers.isEmpty()) {
+        if (!Iterables.isEmpty(tgtPlayers)) {
             sb.append(Lang.joinHomogenous(tgtPlayers)).append(" ");
 
             if (mode.equals("RevealYouChoose")) {
@@ -103,6 +105,7 @@ public class DiscardEffect extends SpellAbilityEffect {
     public void resolve(SpellAbility sa) {
         final Card source = sa.getHostCard();
         final String mode = sa.getParam("Mode");
+        final Game game = source.getGame();
         //final boolean anyNumber = sa.hasParam("AnyNumber");
 
         final List<Card> discarded = new ArrayList<Card>();
@@ -114,7 +117,7 @@ public class DiscardEffect extends SpellAbilityEffect {
             discarders = getDefinedPlayersOrTargeted(sa);
             firstTarget = Iterables.getFirst(targets, null);
             if (sa.usesTargeting() && !firstTarget.canBeTargetedBy(sa)) {
-            	firstTarget = null;
+                firstTarget = null;
             }
         } else {
             discarders = targets;
@@ -124,23 +127,26 @@ public class DiscardEffect extends SpellAbilityEffect {
         final CardZoneTable table = new CardZoneTable();
         for (final Player p : discarders) {
             if ((mode.equals("RevealTgtChoose") && firstTarget != null) || !sa.usesTargeting() || p.canBeTargetedBy(sa)) {
-            	if (sa.hasParam("RememberDiscarder")) {
+                if (sa.hasParam("RememberDiscarder") && p.canDiscardBy(sa)) {
-            		source.addRemembered(p);
+                    source.addRemembered(p);
-            	}
+                }
-                final int numCardsInHand = p.getCardsIn(ZoneType.Hand).size(); 
+                final int numCardsInHand = p.getCardsIn(ZoneType.Hand).size();
                 if (mode.equals("Defined")) {
-                    boolean runDiscard = !sa.hasParam("Optional") 
+                    if (!p.canDiscardBy(sa)) {
-                    		|| p.getController().confirmAction(sa, PlayerActionConfirmMode.Random, sa.getParam("DiscardMessage"));
+                        continue;
+                    }
+
+                    boolean runDiscard = !sa.hasParam("Optional")
+                            || p.getController().confirmAction(sa, PlayerActionConfirmMode.Random, sa.getParam("DiscardMessage"));
                     if (runDiscard) {
                         CardCollectionView toDiscard = AbilityUtils.getDefinedCards(source, sa.getParam("DefinedCards"), sa);
 
                         if (toDiscard.size() > 1) {
-                            toDiscard = GameActionUtil.orderCardsByTheirOwners(p.getGame(), toDiscard, ZoneType.Graveyard);
+                            toDiscard = GameActionUtil.orderCardsByTheirOwners(game, toDiscard, ZoneType.Graveyard);
                         }
 
                         for (final Card c : toDiscard) {
-                            boolean hasDiscarded = p.discard(c, sa, table) != null;
+                            if (p.discard(c, sa, table) != null) {
-                            if (hasDiscarded) {
                                 discarded.add(c);
                             }
                         }
@@ -155,11 +161,14 @@ public class DiscardEffect extends SpellAbilityEffect {
                 }
 
                 if (mode.equals("Hand")) {
+                    if (!p.canDiscardBy(sa)) {
+                        continue;
+                    }
                     boolean shouldRemember = sa.hasParam("RememberDiscarded");
                     CardCollectionView toDiscard = new CardCollection(Lists.newArrayList(p.getCardsIn(ZoneType.Hand)));
 
                     if (toDiscard.size() > 1) {
-                        toDiscard = GameActionUtil.orderCardsByTheirOwners(p.getGame(), toDiscard, ZoneType.Graveyard);
+                        toDiscard = GameActionUtil.orderCardsByTheirOwners(game, toDiscard, ZoneType.Graveyard);
                     }
 
                     for(Card c : toDiscard) { // without copying will get concurrent modification exception
@@ -171,27 +180,31 @@ public class DiscardEffect extends SpellAbilityEffect {
                 }
 
                 if (mode.equals("NotRemembered")) {
+                    if (!p.canDiscardBy(sa)) {
+                        continue;
+                    }
                     CardCollectionView dPHand = CardLists.getValidCards(p.getCardsIn(ZoneType.Hand), "Card.IsNotRemembered", p, source);
                     if (dPHand.size() > 1) {
-                        dPHand = GameActionUtil.orderCardsByTheirOwners(p.getGame(), dPHand, ZoneType.Graveyard);
+                        dPHand = GameActionUtil.orderCardsByTheirOwners(game, dPHand, ZoneType.Graveyard);
                     }
 
                     for (final Card c : dPHand) {
-                        p.discard(c, sa, table);
+                        if (p.discard(c, sa, table) != null) {
-                        discarded.add(c);
+                            discarded.add(c);
+                        }
                     }
                 }
 
                 int numCards = 1;
                 if (sa.hasParam("NumCards")) {
                     numCards = AbilityUtils.calculateAmount(sa.getHostCard(), sa.getParam("NumCards"), sa);
-                    if (!p.getCardsIn(ZoneType.Hand).isEmpty() && p.getCardsIn(ZoneType.Hand).size() < numCards) {
+                    numCards = Math.min(numCards, numCardsInHand);
-                        // System.out.println("Scale down discard from " + numCards + " to " + p.getCardsIn(ZoneType.Hand).size());
-                        numCards = p.getCardsIn(ZoneType.Hand).size();
-                    }
                 }
 
                 if (mode.equals("Random")) {
+                    if (!p.canDiscardBy(sa)) {
+                        continue;
+                    }
                     String message = "Would you like to discard " + numCards + " random card(s)?";
                     boolean runDiscard = !sa.hasParam("Optional") || p.getController().confirmAction(sa, PlayerActionConfirmMode.Random, message);
 
@@ -211,7 +224,7 @@ public class DiscardEffect extends SpellAbilityEffect {
 
                         CardCollectionView toDiscardView = toDiscard;
                         if (toDiscard.size() > 1) {
-                            toDiscardView = GameActionUtil.orderCardsByTheirOwners(p.getGame(), toDiscard, ZoneType.Graveyard);
+                            toDiscardView = GameActionUtil.orderCardsByTheirOwners(game, toDiscard, ZoneType.Graveyard);
                         }
 
                         for (Card c : toDiscardView) {
@@ -222,13 +235,16 @@ public class DiscardEffect extends SpellAbilityEffect {
                     }
                 }
                 else if (mode.equals("TgtChoose") && sa.hasParam("UnlessType")) {
+                    if (!p.canDiscardBy(sa)) {
+                        continue;
+                    }
                     if( numCardsInHand > 0 ) {
                         CardCollectionView hand = p.getCardsIn(ZoneType.Hand);
                         hand = CardLists.filter(hand, Presets.NON_TOKEN);
                         CardCollectionView toDiscard = p.getController().chooseCardsToDiscardUnlessType(Math.min(numCards, numCardsInHand), hand, sa.getParam("UnlessType"), sa);
 
                         if (toDiscard.size() > 1) {
-                            toDiscard = GameActionUtil.orderCardsByTheirOwners(p.getGame(), toDiscard, ZoneType.Graveyard);
+                            toDiscard = GameActionUtil.orderCardsByTheirOwners(game, toDiscard, ZoneType.Graveyard);
                         }
 
                         for (Card c : toDiscard) {
@@ -240,8 +256,12 @@ public class DiscardEffect extends SpellAbilityEffect {
                     // Reveal
                     final CardCollectionView dPHand = p.getCardsIn(ZoneType.Hand);
 
-                    for (final Player opp : p.getOpponents()) {
+                    for (final Player opp : p.getAllOtherPlayers()) {
-                    	opp.getController().reveal(dPHand, ZoneType.Hand, p, "Reveal ");
+                        opp.getController().reveal(dPHand, ZoneType.Hand, p, "Reveal ");
+                    }
+
+                    if (!p.canDiscardBy(sa)) {
+                        continue;
                     }
 
                     String valid = sa.hasParam("DiscardValid") ? sa.getParam("DiscardValid") : "Card";
@@ -254,13 +274,14 @@ public class DiscardEffect extends SpellAbilityEffect {
                     CardCollectionView dPChHand = CardLists.getValidCards(dPHand, valid.split(","), source.getController(), source, sa);
                     dPChHand = CardLists.filter(dPChHand, Presets.NON_TOKEN);
                     if (dPChHand.size() > 1) {
-                        dPChHand = GameActionUtil.orderCardsByTheirOwners(p.getGame(), dPChHand, ZoneType.Graveyard);
+                        dPChHand = GameActionUtil.orderCardsByTheirOwners(game, dPChHand, ZoneType.Graveyard);
                     }
 
                     // Reveal cards that will be discarded?
                     for (final Card c : dPChHand) {
-                        p.discard(c, sa, table);
+                        if (p.discard(c, sa, table) != null) {
-                        discarded.add(c);
+                            discarded.add(c);
+                        }
                     }
                 } else if (mode.equals("RevealYouChoose") || mode.equals("RevealTgtChoose") || mode.equals("TgtChoose")) {
                     CardCollectionView dPHand = p.getCardsIn(ZoneType.Hand);
@@ -273,6 +294,7 @@ public class DiscardEffect extends SpellAbilityEffect {
                         int amount = StringUtils.isNumeric(amountString) ? Integer.parseInt(amountString) : CardFactoryUtil.xCount(source, source.getSVar(amountString));
                         dPHand = p.getController().chooseCardsToRevealFromHand(amount, amount, dPHand);
                     }
+
                     final String valid = sa.hasParam("DiscardValid") ? sa.getParam("DiscardValid") : "Card";
                     String[] dValid = valid.split(",");
                     CardCollection validCards = CardLists.getValidCards(dPHand, dValid, source.getController(), source, sa);
@@ -284,9 +306,14 @@ public class DiscardEffect extends SpellAbilityEffect {
                         chooser = firstTarget;
                     }
 
-                    if (mode.startsWith("Reveal") && p != chooser)
+                    if (mode.startsWith("Reveal") && p != chooser) {
-                        chooser.getGame().getAction().reveal(dPHand, p);
+                        game.getAction().reveal(dPHand, p);
-                    
+                    }
+
+                    if (!p.canDiscardBy(sa)) {
+                        continue;
+                    }
+
                     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);
 
@@ -294,7 +321,7 @@ public class DiscardEffect extends SpellAbilityEffect {
 
                     if (toBeDiscarded != null) {
                         if (toBeDiscarded.size() > 1) {
-                            toBeDiscarded = GameActionUtil.orderCardsByTheirOwners(p.getGame(), toBeDiscarded, ZoneType.Graveyard);
+                            toBeDiscarded = GameActionUtil.orderCardsByTheirOwners(game, toBeDiscarded, ZoneType.Graveyard);
                         }
 
                         if (mode.startsWith("Reveal") ) {
@@ -303,8 +330,9 @@ public class DiscardEffect extends SpellAbilityEffect {
                         }
                         for (Card card : toBeDiscarded) {
                             if (card == null) { continue; }
-                            p.discard(card, sa, table);
+                            if (p.discard(card, sa, table) != null) {
-                            discarded.add(card);
+                                discarded.add(card);
+                            }
                         }
                     }
                 }

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

@@ -1988,7 +1988,7 @@ public class Card extends GameEntity implements Comparable<Card> {
                     final String[] k = keyword.split(":");
                     final String manacost = k[1];
                     final Cost cost = new Cost(manacost, false);
-    
+
                     StringBuilder sbCost = new StringBuilder(k[0]);
                     if (!cost.isOnlyManaCost()) {
                         sbCost.append("—");
@@ -2038,7 +2038,7 @@ public class Card extends GameEntity implements Comparable<Card> {
                         || keyword.startsWith("Miracle") || keyword.startsWith("Recover")) {
                     final String[] k = keyword.split(":");
                     final Cost cost = new Cost(k[1], false);
-    
+
                     StringBuilder sbCost = new StringBuilder(k[0]);
                     if (!cost.isOnlyManaCost()) {
                         sbCost.append("—");
@@ -2064,13 +2064,13 @@ public class Card extends GameEntity implements Comparable<Card> {
                     sbAfter.append(" (" + inst.getReminderText() + ")").append("\r\n");
                 } else if (keyword.equals("Storm")) {
                     sbAfter.append("Storm (");
-    
+
                     sbAfter.append("When you cast this spell, copy it for each spell cast before it this turn.");
-    
+
                     if (strSpell.contains("Target") || strSpell.contains("target")) {
                         sbAfter.append(" You may choose new targets for the copies.");
                     }
-    
+
                     sbAfter.append(")");
                     sbAfter.append("\r\n");
                 } else if (keyword.startsWith("Replicate")) {
@@ -5480,8 +5480,7 @@ public class Card extends GameEntity implements Comparable<Card> {
             }
         }
 
-        if (getController().isOpponentOf(source.getActivatingPlayer())
+        if (!getController().canSacrificeBy(source)) {
-                && getController().hasKeyword("Spells and abilities your opponents control can't cause you to sacrifice permanents.")) {
             return false;
         }
 
@@ -5921,4 +5920,16 @@ public class Card extends GameEntity implements Comparable<Card> {
             rE.setTemporarilySuppressed(false);
         }
     }
+
+    public boolean canBeDiscardedBy(SpellAbility sa) {
+        if (!isInZone(ZoneType.Hand)) {
+            return false;
+        }
+
+        if (!getOwner().canDiscardBy(sa)) {
+            return false;
+        }
+
+        return true;
+    }
 }

forge-game/src/main/java/forge/game/cost/CostDiscard.java

@@ -18,6 +18,7 @@
 package forge.game.cost;
 
 import forge.game.card.Card;
+import forge.game.card.CardCollection;
 import forge.game.card.CardCollectionView;
 import forge.game.card.CardLists;
 import forge.game.card.CardPredicates;
@@ -106,17 +107,20 @@ public class CostDiscard extends CostPartWithList {
     public final boolean canPay(final SpellAbility ability, final Player payer) {
         final Card source = ability.getHostCard();
 
-        CardCollectionView handList = payer.getCardsIn(ZoneType.Hand);
+        CardCollectionView handList = payer.canDiscardBy(ability) ? payer.getCardsIn(ZoneType.Hand) : CardCollection.EMPTY;
         String type = this.getType();
         final Integer amount = this.convertAmount();
 
         if (this.payCostFromSource()) {
-            if (!source.isInZone(ZoneType.Hand)) {
+            if (!source.canBeDiscardedBy(ability)) {
                 return false;
             }
         }
         else {
             if (type.equals("Hand")) {
+                if (!payer.canDiscardBy(ability)) {
+                    return false;
+                }
                 // this will always work
             }
             else if (type.equals("LastDrawn")) {

forge-game/src/main/java/forge/game/player/Player.java

@@ -455,11 +455,11 @@ public class Player extends GameEntity implements Comparable<Player> {
         }
         return true;
     }
-    
+
     public final int loseLife(final int toLose) {
     	return loseLife(toLose,false);
     }
-    
+
     public final int loseLife(final int toLose, final boolean manaBurn) {
         int lifeLost = 0;
         if (!canLoseLife()) {
@@ -1555,6 +1555,10 @@ public class Player extends GameEntity implements Comparable<Player> {
     }
 
     public final Card discard(final Card c, final SpellAbility sa, CardZoneTable table) {
+        if (!c.canBeDiscardedBy(sa)) {
+            return null;
+        }
+
         // TODO: This line should be moved inside CostPayment somehow
         /*if (sa != null) {
             sa.addCostToHashList(c, "Discarded");
@@ -2935,4 +2939,27 @@ public class Player extends GameEntity implements Comparable<Player> {
         return CardLists.count(attachedCards, CardPredicates.Presets.CURSE) > 0;
     }
 
+    public boolean canDiscardBy(SpellAbility sa) {
+        if (sa == null) {
+            return true;
+        }
+
+        if (isOpponentOf(sa.getActivatingPlayer()) && hasKeyword("Spells and abilities your opponents control can't cause you to discard cards.")) {
+            return false;
+        }
+
+        return true;
+    }
+
+    public boolean canSacrificeBy(SpellAbility sa) {
+        if (sa == null) {
+            return true;
+        }
+
+        if (isOpponentOf(sa.getActivatingPlayer()) && hasKeyword("Spells and abilities your opponents control can't cause you to sacrifice permanents.")) {
+            return false;
+        }
+
+        return true;
+    }
 }

forge-game/src/main/java/forge/game/player/PlayerPredicates.java

@@ -21,7 +21,16 @@ public final class PlayerPredicates {
             }
         };
     }
-    
+
+    public static final Predicate<Player> canDiscardBy(final SpellAbility source) {
+        return new Predicate<Player>() {
+            @Override
+            public boolean apply(final Player p) {
+                return p.canDiscardBy(source);
+            }
+        };
+    }
+
     public static final Predicate<Player> isOpponentOf(final Player player) {
         return new Predicate<Player>() {
             @Override

forge-gui/res/cardsfolder/upcoming/tamiyo_collector_of_tales.txt

@@ -0,0 +1,10 @@
+Name:Tamiyo, Collector of Tales
+ManaCost:2 G U
+Types:Legendary Planeswalker Tamiyo
+Loyalty:5
+S:Mode$ Continuous | Affected$ You | AddKeyword$ Spells and abilities your opponents control can't cause you to discard cards. & Spells and abilities your opponents control can't cause you to sacrifice permanents. | Description$ Spells and abilities your opponents control can't cause you to discard cards or sacrifice permanents.
+A:AB$ NameCard | Cost$ AddCounter<1/LOYALTY> | Planeswalker$ True | Defined$ You | SubAbility$ DBDig | SpellDescription$ Choose a nonland card name, then reveal the top four cards of your library. Put all cards with the chosen name from among them into your hand and the rest into your graveyard.
+SVar:DBDig:DB$Dig | DigNum$ 4 | Reveal$ True | ChangeNum$ All | ChangeValid$ Card.NamedCard | DestinationZone2$ Graveyard
+A:AB$ ChangeZone | Cost$ SubCounter<3/LOYALTY> | Planeswalker$ True | TgtPrompt$ Choose target card in your graveyard | ValidTgts$ Card.YouCtrl | Origin$ Graveyard | Destination$ Hand | SpellDescription$ Return target card from your graveyard to your hand.
+AI:RemoveDeck:All
+Oracle:Spells and abilities your opponents control can't cause you to discard cards or sacrifice permanents.\n[+1]: Choose a nonland card name, then reveal the top four cards of your library. Put all cards with the chosen name from among them into your hand and the rest into your graveyard.\n[-3]: Return target card from your graveyard to your hand.