Added new method for discarding cards to handle batch triggers (ready for review) (#6489)

* added new discard method

* started refactoring to use new discard method

* refactored A through I

* fixed some issues

* separated balance effect into its own class

* refactored J through R

* refactored S through Z

* applied requested changes

Mage/src/main/java/mage/abilities/costs/common/DiscardHandCost.java

@@ -1,27 +1,23 @@
-
-
 package mage.abilities.costs.common;
 
-import java.util.UUID;
 import mage.abilities.Ability;
 import mage.abilities.costs.Cost;
 import mage.abilities.costs.CostImpl;
-import mage.cards.Card;
 import mage.game.Game;
 import mage.players.Player;
 
+import java.util.UUID;
+
 /**
- *
  * @author LevelX2
  */
 
 public class DiscardHandCost extends CostImpl {
 
     public DiscardHandCost() {
-    
     }
 
-    public DiscardHandCost(final DiscardHandCost cost) {
+    private DiscardHandCost(final DiscardHandCost cost) {
         super(cost);
     }
 
@@ -38,12 +34,11 @@ public class DiscardHandCost extends CostImpl {
     @Override
     public boolean pay(Ability ability, Game game, UUID sourceId, UUID controllerId, boolean noMana, Cost costToPay) {
         Player player = game.getPlayer(controllerId);
-        if (player != null) {
-            for (Card card : player.getHand().getCards(game)) {
-                player.discard(card, ability, game);
-            }
-            paid = true;
+        if (player == null) {
+            return paid;
         }
+        player.discard(player.getHand(), ability, game);
+        paid = true;
         return paid;
     }
 

Mage/src/main/java/mage/abilities/costs/common/DiscardTargetCost.java

@@ -1,20 +1,21 @@
-
 package mage.abilities.costs.common;
 
-import java.util.ArrayList;
-import java.util.List;
-import java.util.UUID;
 import mage.abilities.Ability;
 import mage.abilities.costs.Cost;
 import mage.abilities.costs.CostImpl;
 import mage.cards.Card;
+import mage.cards.Cards;
+import mage.cards.CardsImpl;
 import mage.constants.Outcome;
 import mage.game.Game;
 import mage.players.Player;
 import mage.target.common.TargetCardInHand;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+
 /**
- *
  * @author BetaSteward_at_googlemail.com
  */
 public class DiscardTargetCost extends CostImpl {
@@ -50,13 +51,11 @@ public class DiscardTargetCost extends CostImpl {
         if (randomDiscard) {
             this.cards.addAll(player.discard(amount, true, ability, game).getCards(game));
         } else if (targets.choose(Outcome.Discard, controllerId, sourceId, game)) {
-            for (UUID targetId : targets.get(0).getTargets()) {
-                Card card = player.getHand().get(targetId, game);
-                if (card == null) {
-                    return false;
-                }
-                player.discard(card, ability, game);
-                this.cards.add(card);
+            Cards toDiscard = new CardsImpl();
+            toDiscard.addAll(targets.get(0).getTargets());
+            Cards discarded = player.discard(toDiscard, ability, game);
+            if (!discarded.isEmpty()) {
+                cards.addAll(discarded.getCards(game));
             }
         }
         paid = cards.size() >= amount;

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

@@ -0,0 +1,168 @@
+package mage.abilities.effects.common;
+
+import mage.abilities.Ability;
+import mage.abilities.effects.OneShotEffect;
+import mage.cards.Card;
+import mage.cards.Cards;
+import mage.cards.CardsImpl;
+import mage.constants.Outcome;
+import mage.filter.FilterCard;
+import mage.filter.StaticFilters;
+import mage.filter.common.FilterControlledCreaturePermanent;
+import mage.filter.common.FilterControlledLandPermanent;
+import mage.filter.common.FilterControlledPermanent;
+import mage.game.Game;
+import mage.game.permanent.Permanent;
+import mage.players.Player;
+import mage.target.common.TargetCardInHand;
+import mage.target.common.TargetControlledPermanent;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+/**
+ * @author emerald000
+ */
+public class BalanceEffect extends OneShotEffect {
+
+    private static final FilterControlledPermanent filter = new FilterControlledLandPermanent("lands to keep");
+    private static final FilterControlledPermanent filter2 = new FilterControlledCreaturePermanent("creatures to keep");
+    private static final FilterCard filter3 = new FilterCard("cards to keep");
+
+    public BalanceEffect() {
+        super(Outcome.Sacrifice);
+        staticText = "each player chooses a number of lands they control "
+                + "equal to the number of lands controlled by the player "
+                + "who controls the fewest, then sacrifices the rest. "
+                + "Players discard cards and sacrifice creatures the same way";
+    }
+
+    private BalanceEffect(final BalanceEffect effect) {
+        super(effect);
+    }
+
+    @Override
+    public BalanceEffect copy() {
+        return new BalanceEffect(this);
+    }
+
+    @Override
+    public boolean apply(Game game, Ability source) {
+        Player controller = game.getPlayer(source.getControllerId());
+        if (controller == null) {
+            return false;
+        }
+        //Lands
+        int minLand = Integer.MAX_VALUE;
+        Cards landsToSacrifice = new CardsImpl();
+        for (UUID playerId : game.getState().getPlayersInRange(controller.getId(), game)) {
+            Player player = game.getPlayer(playerId);
+            if (player == null) {
+                continue;
+            }
+            int count = game.getBattlefield().countAll(StaticFilters.FILTER_CONTROLLED_PERMANENT_LAND, player.getId(), game);
+            if (count < minLand) {
+                minLand = count;
+            }
+        }
+
+        for (UUID playerId : game.getState().getPlayersInRange(controller.getId(), game)) {
+            Player player = game.getPlayer(playerId);
+            if (player == null) {
+                continue;
+            }
+            TargetControlledPermanent target = new TargetControlledPermanent(minLand, minLand, filter, true);
+            if (!target.choose(Outcome.Sacrifice, player.getId(), source.getSourceId(), game)) {
+                continue;
+            }
+            for (Permanent permanent : game.getBattlefield().getActivePermanents(StaticFilters.FILTER_CONTROLLED_PERMANENT_LAND, player.getId(), source.getSourceId(), game)) {
+                if (permanent != null && !target.getTargets().contains(permanent.getId())) {
+                    landsToSacrifice.add(permanent);
+                }
+            }
+        }
+
+        for (UUID cardId : landsToSacrifice) {
+            Permanent permanent = game.getPermanent(cardId);
+            if (permanent != null) {
+                permanent.sacrifice(source.getSourceId(), game);
+            }
+        }
+
+        //Creatures
+        int minCreature = Integer.MAX_VALUE;
+        Cards creaturesToSacrifice = new CardsImpl();
+        for (UUID playerId : game.getState().getPlayersInRange(controller.getId(), game)) {
+            Player player = game.getPlayer(playerId);
+            if (player == null) {
+            }
+            int count = game.getBattlefield().countAll(StaticFilters.FILTER_CONTROLLED_PERMANENT_LAND, player.getId(), game);
+            if (count < minCreature) {
+                minCreature = count;
+            }
+        }
+
+        for (UUID playerId : game.getState().getPlayersInRange(controller.getId(), game)) {
+            Player player = game.getPlayer(playerId);
+            if (player == null) {
+                continue;
+            }
+            TargetControlledPermanent target = new TargetControlledPermanent(minCreature, minCreature, filter2, true);
+            if (!target.choose(Outcome.Sacrifice, player.getId(), source.getSourceId(), game)) {
+                continue;
+            }
+            for (Permanent permanent : game.getBattlefield().getActivePermanents(StaticFilters.FILTER_CONTROLLED_CREATURE, player.getId(), source.getSourceId(), game)) {
+                if (permanent != null && !target.getTargets().contains(permanent.getId())) {
+                    creaturesToSacrifice.add(permanent);
+                }
+            }
+        }
+
+        for (UUID cardId : creaturesToSacrifice) {
+            Permanent permanent = game.getPermanent(cardId);
+            if (permanent != null) {
+                permanent.sacrifice(source.getSourceId(), game);
+            }
+        }
+
+        //Cards in hand
+        int minCard = Integer.MAX_VALUE;
+        Map<UUID, Cards> cardsToDiscard = new HashMap<>(2);
+        for (UUID playerId : game.getState().getPlayersInRange(controller.getId(), game)) {
+            Player player = game.getPlayer(playerId);
+            if (player == null) {
+            }
+            int count = player.getHand().size();
+            if (count < minCard) {
+                minCard = count;
+            }
+        }
+
+        for (UUID playerId : game.getState().getPlayersInRange(controller.getId(), game)) {
+            Player player = game.getPlayer(playerId);
+            if (player == null) {
+                continue;
+            }
+            Cards cards = new CardsImpl();
+            TargetCardInHand target = new TargetCardInHand(minCard, filter3);
+            if (!target.choose(Outcome.Discard, player.getId(), source.getSourceId(), game)) {
+                continue;
+            }
+            for (Card card : player.getHand().getCards(game)) {
+                if (card != null && !target.getTargets().contains(card.getId())) {
+                    cards.add(card);
+                }
+            }
+            cardsToDiscard.put(playerId, cards);
+        }
+
+        for (UUID playerId : game.getState().getPlayersInRange(controller.getId(), game)) {
+            Player player = game.getPlayer(playerId);
+            if (player != null && cardsToDiscard.get(playerId) != null) {
+                player.discard(cardsToDiscard.get(playerId), source, game);
+            }
+        }
+        return true;
+    }
+}

Mage/src/main/java/mage/abilities/effects/common/discard/DiscardCardYouChooseTargetEffect.java

@@ -144,12 +144,7 @@ public class DiscardCardYouChooseTargetEffect extends OneShotEffect {
         if (!controller.choose(Outcome.Benefit, revealedCards, target, game)) {
             return result;
         }
-        for (UUID targetId : target.getTargets()) {
-            Card card = revealedCards.get(targetId, game);
-            if (!player.discard(card, source, game)) {
-                result = false;
-            }
-        }
+        result=!player.discard(new CardsImpl(target.getTargets()),source,game).isEmpty();
         return result;
     }
 

Mage/src/main/java/mage/abilities/effects/common/discard/DiscardControllerEffect.java

@@ -1,11 +1,9 @@
-
 package mage.abilities.effects.common.discard;
 
 import mage.abilities.Ability;
 import mage.abilities.dynamicvalue.DynamicValue;
 import mage.abilities.dynamicvalue.common.StaticValue;
 import mage.abilities.effects.OneShotEffect;
-import mage.cards.Card;
 import mage.constants.Outcome;
 import mage.game.Game;
 import mage.players.Player;
@@ -16,8 +14,8 @@ import mage.util.CardUtil;
  */
 public class DiscardControllerEffect extends OneShotEffect {
 
-    protected DynamicValue amount;
-    protected boolean randomDiscard;
+    protected final DynamicValue amount;
+    protected final boolean randomDiscard;
 
     public DiscardControllerEffect(int amount) {
         this(StaticValue.get(amount));
@@ -51,22 +49,8 @@ public class DiscardControllerEffect extends OneShotEffect {
 
     @Override
     public boolean apply(Game game, Ability source) {
-        boolean result = false;
         Player player = game.getPlayer(source.getControllerId());
-        if (player != null) {
-            if (randomDiscard) {
-                int maxAmount = Math.min(amount.calculate(game, source, this), player.getHand().size());
-                for (int i = 0; i < maxAmount; i++) {
-                    Card card = player.getHand().getRandom(game);
-                    result |= player.discard(card, source, game);
-
-                }
-            } else {
-                player.discard(amount.calculate(game, source, this), false, source, game);
-                result = true;
-            }
-        }
-        return result;
+        return player != null && !player.discard(amount.calculate(game, source, this), randomDiscard, source, game).isEmpty();
     }
 
     private void setText() {

Mage/src/main/java/mage/abilities/effects/common/discard/DiscardEachPlayerEffect.java

@@ -5,12 +5,11 @@ import mage.abilities.Mode;
 import mage.abilities.dynamicvalue.DynamicValue;
 import mage.abilities.dynamicvalue.common.StaticValue;
 import mage.abilities.effects.OneShotEffect;
-import mage.cards.Card;
 import mage.cards.Cards;
 import mage.cards.CardsImpl;
 import mage.constants.Outcome;
 import mage.constants.TargetController;
-import mage.filter.FilterCard;
+import mage.filter.StaticFilters;
 import mage.game.Game;
 import mage.players.Player;
 import mage.target.Target;
@@ -62,54 +61,50 @@ public class DiscardEachPlayerEffect extends OneShotEffect {
         Player controller = game.getPlayer(source.getControllerId());
         // Store for each player the cards to discard, that's important because all discard shall happen at the same time
         Map<UUID, Cards> cardsToDiscard = new HashMap<>();
-        if (controller != null) {
-            // choose cards to discard
-            for (UUID playerId : game.getState().getPlayersInRange(controller.getId(), game)) {
-                Player player = game.getPlayer(playerId);
-                if (player != null) {
-                    switch (targetController) {
-                        case NOT_YOU:
-                            if (playerId.equals(source.getControllerId())) {
-                                continue;
-                            }
-                            break;
-                        case OPPONENT:
-                            if (!game.getOpponents(source.getControllerId()).contains(playerId)) {
-                                continue;
-                            }
-                            break;
-                    }
-                    int numberOfCardsToDiscard = Math.min(amount.calculate(game, source, this), player.getHand().size());
-                    Cards cards = new CardsImpl();
-                    if (randomDiscard) {
-                        while (player.isInGame() && cards.size() < numberOfCardsToDiscard) {
-                            Card card = player.getHand().getRandom(game);
-                            if (!cards.contains(card.getId())) {
-                                cards.add(card);
-                            }
-                        }
-                    } else {
-                        Target target = new TargetDiscard(numberOfCardsToDiscard, numberOfCardsToDiscard, new FilterCard(), playerId);
-                        player.chooseTarget(outcome, target, source, game);
-                        cards.addAll(target.getTargets());
-                    }
-                    cardsToDiscard.put(playerId, cards);
-                }
+        if (controller == null) {
+            return true;
+        }
+        int toDiscard = amount.calculate(game, source, this);
+        // choose cards to discard
+        for (UUID playerId : game.getState().getPlayersInRange(controller.getId(), game)) {
+            Player player = game.getPlayer(playerId);
+            if (player == null) {
+                continue;
             }
-            // discard all choosen cards
-            for (UUID playerId : game.getState().getPlayersInRange(controller.getId(), game)) {
-                Player player = game.getPlayer(playerId);
-                if (player != null) {
-                    Cards cardsPlayer = cardsToDiscard.get(playerId);
-                    if (cardsPlayer != null) {
-                        for (UUID cardId : cardsPlayer) {
-                            Card card = game.getCard(cardId);
-                            player.discard(card, source, game);
-
-                        }
+            switch (targetController) {
+                case NOT_YOU:
+                    if (playerId.equals(source.getControllerId())) {
+                        continue;
                     }
-                }
+                    break;
+                case OPPONENT:
+                    if (!game.getOpponents(source.getControllerId()).contains(playerId)) {
+                        continue;
+                    }
+                    break;
             }
+            if (randomDiscard) {
+                player.discard(toDiscard, true, source, game);
+                continue;
+            }
+            int numberOfCardsToDiscard = Math.min(toDiscard, player.getHand().size());
+            Cards cards = new CardsImpl();
+            Target target = new TargetDiscard(numberOfCardsToDiscard, numberOfCardsToDiscard, StaticFilters.FILTER_CARD, playerId);
+            player.chooseTarget(outcome, target, source, game);
+            cards.addAll(target.getTargets());
+            cardsToDiscard.put(playerId, cards);
+        }
+        if (randomDiscard) {
+            return true;
+        }
+        // discard all choosen cards
+        for (UUID playerId : game.getState().getPlayersInRange(controller.getId(), game)) {
+            Player player = game.getPlayer(playerId);
+            if (player == null) {
+                continue;
+            }
+            Cards cardsPlayer = cardsToDiscard.get(playerId);
+            player.discard(cardsPlayer, source, game);
         }
         return true;
     }

Mage/src/main/java/mage/abilities/effects/common/discard/DiscardHandAllEffect.java

@@ -2,12 +2,10 @@ package mage.abilities.effects.common.discard;
 
 import mage.abilities.Ability;
 import mage.abilities.effects.OneShotEffect;
-import mage.cards.Card;
 import mage.constants.Outcome;
 import mage.game.Game;
 import mage.players.Player;
 
-import java.util.Set;
 import java.util.UUID;
 
 /**
@@ -33,12 +31,10 @@ public class DiscardHandAllEffect extends OneShotEffect {
     public boolean apply(Game game, Ability source) {
         for (UUID playerId : game.getState().getPlayersInRange(source.getControllerId(), game)) {
             Player player = game.getPlayer(playerId);
-            if (player != null) {
-                Set<Card> cards = player.getHand().getCards(game);
-                for (Card card : cards) {
-                    player.discard(card, source, game);
-                }
+            if (player == null) {
+                continue;
             }
+            player.discard(player.getHand(), source, game);
         }
         return true;
     }

Mage/src/main/java/mage/players/Player.java

@@ -412,6 +412,8 @@ public interface Player extends MageItem, Copyable<Player> {
 
     Cards discard(int amount, boolean random, Ability source, Game game);
 
+    Cards discard(Cards cards, Ability source, Game game);
+
     void discardToMax(Game game);
 
     boolean discard(Card card, Ability source, Game game);

Mage/src/main/java/mage/players/PlayerImpl.java

@@ -718,6 +718,24 @@ public abstract class PlayerImpl implements Player, Serializable {
         return discardedCards;
     }
 
+    @Override
+    public Cards discard(Cards cards, Ability source, Game game) {
+        Cards discardedCards = new CardsImpl();
+        for (Card card : cards.getCards(game)) {
+            if (doDiscard(card, source, game, false)) {
+                discardedCards.add(card);
+            }
+        }
+        if (!discardedCards.isEmpty()) {
+            UUID sourceId = source == null ? null : source.getSourceId();
+            game.fireEvent(GameEvent.getEvent(
+                    GameEvent.EventType.DISCARDED_CARDS, sourceId,
+                    sourceId, playerId, discardedCards.size()
+            ));
+        }
+        return discardedCards;
+    }
+
     private Cards doDiscard(int amount, boolean random, Ability source, Game game) {
         Cards discardedCards = new CardsImpl();
         if (amount <= 0) {