Inputs from PlayerUtil.java - either moved closer to (single) usage - not changed to sync yet, or replaced with InputSelectCards

Removed different invocations of discard, moved meaningful code to DiscardEffect.java where its callers are.

.gitattributes

@@ -13955,7 +13955,6 @@ src/main/java/forge/game/player/PlayerControllerHuman.java -text
 src/main/java/forge/game/player/PlayerOutcome.java -text
 src/main/java/forge/game/player/PlayerStatistics.java -text
 src/main/java/forge/game/player/PlayerType.java svneol=native#text/plain
-src/main/java/forge/game/player/PlayerUtil.java svneol=native#text/plain
 src/main/java/forge/game/player/package-info.java svneol=native#text/plain
 src/main/java/forge/game/zone/IZone.java svneol=native#text/plain
 src/main/java/forge/game/zone/MagicStack.java svneol=native#text/plain

src/main/java/forge/card/ability/effects/DiscardEffect.java

@@ -4,6 +4,7 @@ import java.util.ArrayList;
 import java.util.List;
 
 import org.apache.commons.lang.StringUtils;
+import org.testng.collections.Lists;
 
 import forge.Card;
 import forge.CardLists;
@@ -13,6 +14,7 @@ import forge.card.spellability.SpellAbility;
 import forge.card.spellability.Target;
 import forge.game.player.Player;
 import forge.game.zone.ZoneType;
+import forge.util.Aggregates;
 
 public class DiscardEffect extends RevealEffectBase {
     @Override
@@ -94,7 +96,9 @@ public class DiscardEffect extends RevealEffectBase {
                 if (mode.equals("Defined")) {
                     final List<Card> toDiscard = AbilityUtils.getDefinedCards(source, sa.getParam("DefinedCards"), sa);
                     for (final Card c : toDiscard) {
-                        discarded.addAll(p.discard(c, sa));
+                        boolean hasDiscarded = p.discard(c, sa);
+                        if(hasDiscarded)
+                            discarded.add(c);
                     }
                     if (sa.hasParam("RememberDiscarded")) {
                         for (final Card c : discarded) {
@@ -105,12 +109,12 @@ public class DiscardEffect extends RevealEffectBase {
                 }
 
                 if (mode.equals("Hand")) {
-                    final List<Card> list = p.discardHand(sa);
+                    boolean shouldRemember = sa.hasParam("RememberDiscarded");
-                    if (sa.hasParam("RememberDiscarded")) {
+                    for(Card c : Lists.newArrayList(p.getCardsIn(ZoneType.Hand))) { // without copying will get concurrent modification exception
-                        for (final Card c : list) {
+                        boolean hasDiscarded = p.discard(c, sa);
+                        if( hasDiscarded && shouldRemember )
                             source.addRemembered(c);
                     }
-                    }
                     continue;
                 }
 
@@ -137,10 +141,18 @@ public class DiscardEffect extends RevealEffectBase {
                     String message = "Would you like to discard " + numCards + " random card(s)?";
                     boolean runDiscard = !sa.hasParam("Optional") || p.getController().confirmAction(sa, mode, message);
 
-
                     if (runDiscard) {
                         final String valid = sa.hasParam("DiscardValid") ? sa.getParam("DiscardValid") : "Card";
-                        discarded.addAll(p.discardRandom(numCards, sa, valid));
+                        final List<Card> list = CardLists.getValidCards(p.getCardsIn(ZoneType.Hand), valid, sa.getSourceCard().getController(), sa.getSourceCard());
+                        for (int i = 0; i < numCards; i++) {
+                            if (list.isEmpty())
+                                break;
+                                
+                            final Card disc = Aggregates.random(list);
+                            if ( p.discard(disc, sa) ) 
+                                discarded.add(disc);
+                            list.remove(disc);
+                        }
                     }
                 } else if (mode.equals("TgtChoose") && sa.hasParam("UnlessType")) {
                     p.discardUnless(numCards, sa.getParam("UnlessType"), sa);

src/main/java/forge/card/cardfactory/CardFactorySorceries.java

@@ -39,14 +39,20 @@ import forge.card.cost.Cost;
 import forge.card.mana.ManaCost;
 import forge.card.spellability.Spell;
 import forge.card.spellability.SpellAbility;
+import forge.control.input.Input;
+import forge.control.input.InputBase;
 import forge.control.input.InputPayManaExecuteCommands;
+import forge.control.input.InputSelectCards;
+import forge.control.input.InputSelectCardsFromList;
 import forge.game.GameState;
 import forge.game.ai.ComputerUtil;
+import forge.game.player.AIPlayer;
 import forge.game.player.Player;
-import forge.game.player.PlayerUtil;
+import forge.game.zone.Zone;
 import forge.game.zone.ZoneType;
 import forge.gui.GuiChoose;
 import forge.util.Aggregates;
+import forge.view.ButtonUtil;
 
 /**
  * <p>
@@ -269,23 +275,37 @@ public class CardFactorySorceries {
                     Singletons.getModel().getGame().getAction().sacrifice(l.get(i), card);
                 }
             } else {
-                Singletons.getModel().getMatch().getInput().setInput(PlayerUtil.inputSacrificePermanents(sac, "Land"));
+                final List<Card> list = CardLists.getType(p.getCardsIn(ZoneType.Battlefield), "Land");
+                Input inp = inputSacrificePermanentsFromList(sac, list, "Select a land to sacrifice");
+                Singletons.getModel().getMatch().getInput().setInput(inp);
             }
         }
     }
 
-    private static final void balanceHands(Spell card) {
+    private static final void balanceHands(Spell spell) {
         int min = Integer.MAX_VALUE;
         for (Player p : Singletons.getModel().getGame().getPlayers()) {
             min = Math.min(min, p.getZone(ZoneType.Hand).size());
         }
 
         for (Player p : Singletons.getModel().getGame().getPlayers()) {
-            int sac = p.getCardsIn(ZoneType.Hand).size() - min;
+            List<Card> hand = p.getCardsIn(ZoneType.Hand);
+            int sac = hand.size() - min;
             if (sac == 0) {
                 continue;
             }
-            p.discard(sac, card);
+            if (p.isHuman()) {
+                InputSelectCards sc = new InputSelectCardsFromList(sac, sac, hand);
+                sc.setMessage("Select %d more card(s) to discard");
+                FThreads.setInputAndWait(sc);
+                for( Card c : sc.getSelected())
+                    p.discard(c, spell);
+            } else {
+                final List<Card> toDiscard = ((AIPlayer)p).getAi().getCardsToDiscard(sac, (String[])null, spell);
+                for (int i = 0; i < toDiscard.size(); i++) {
+                    p.discard(toDiscard.get(i), spell);
+                }
+            }
         }
     }
 
@@ -316,11 +336,51 @@ public class CardFactorySorceries {
                     Singletons.getModel().getGame().getAction().sacrifice(c.get(i), card);
                 }
             } else {
-                Singletons.getModel().getMatch().getInput().setInput(PlayerUtil.inputSacrificePermanents(sac, "Creature"));
+                final List<Card> list = CardLists.getType(p.getCardsIn(ZoneType.Battlefield), "Creature");
+                Input inp = inputSacrificePermanentsFromList(sac, list, "Select a creature to sacrifice");
+                Singletons.getModel().getMatch().getInput().setInput(inp);
             }
         }
     }
     
+    private static Input inputSacrificePermanentsFromList(final int nCards, final List<Card> list, final String message) {
+        final Input target = new InputBase() {
+            private static final long serialVersionUID = 1981791992623774490L;
+            private int n = 0;
+
+            @Override
+            public void showMessage() {
+                // in case no more {type}s in play
+                if ((this.n == nCards) || (list.size() == 0)) {
+                    this.stop();
+                    return;
+                }
+
+                showMessage(message + " (" + (nCards - this.n) + " left)");
+                ButtonUtil.disableAll();
+            }
+
+            @Override
+            public void selectCard(final Card card) {
+                Zone zone = Singletons.getModel().getGame().getZoneOf(card);
+                if (zone.equals(Singletons.getControl().getPlayer().getZone(ZoneType.Battlefield)) && list.contains(card)) {
+                    Singletons.getModel().getGame().getAction().sacrifice(card, null);
+                    this.n++;
+                    list.remove(card);
+
+                    // in case no more {type}s in play
+                    if ((this.n == nCards) || (list.size() == 0)) {
+                        this.stop();
+                        return;
+                    } else {
+                        this.showMessage();
+                    }
+                }
+            }
+        };
+        return target;
+    } 
+
     private static final SpellAbility getBalance(final Card card) {
         return new Spell(card) {
             private static final long serialVersionUID = -5941893280103164961L;

src/main/java/forge/game/player/AIPlayer.java

@@ -65,16 +65,6 @@ public class AIPlayer extends Player {
     // /
     // //////////////////////////////
 
-    /** {@inheritDoc} */
-    @Override
-    public final void discard(final int num, final SpellAbility sa) {
-        int max = this.getCardsIn(ZoneType.Hand).size();
-        max = Math.min(max, num);
-        final List<Card> toDiscard = this.getAi().getCardsToDiscard(max, (String[])null, sa);
-        for (int i = 0; i < toDiscard.size(); i++) {
-            this.doDiscard(toDiscard.get(i), sa);
-        }
-    } // end discard
 
     /** {@inheritDoc} */
     @Override
@@ -87,7 +77,11 @@ public class AIPlayer extends Player {
             discard(toDiscard, sa); // this got changed to doDiscard basically
             return;
         }
-        this.discard(num, sa);
+        
+        final List<Card> toDiscard = getAi().getCardsToDiscard(num, (String[])null, sa);
+        for (int i = 0; i < toDiscard.size(); i++) {
+            discard(toDiscard.get(i), sa);
+        }
     }
 
     // /////////////////////////

src/main/java/forge/game/player/HumanPlayer.java

@@ -17,10 +17,16 @@
  */
 package forge.game.player;
 
+import forge.Card;
 import forge.Singletons;
 import forge.card.spellability.SpellAbility;
+import forge.control.input.Input;
+import forge.control.input.InputBase;
 import forge.game.GameState;
+import forge.game.zone.Zone;
 import forge.game.zone.ZoneType;
+import forge.gui.match.CMatchUI;
+import forge.view.ButtonUtil;
 
 public class HumanPlayer extends Player {
     private PlayerControllerHuman controller;
@@ -30,20 +36,60 @@ public class HumanPlayer extends Player {
         controller = new PlayerControllerHuman(game, this);
     }
 
-    /** {@inheritDoc} */
-    @Override
-    public final void discard(final int num, final SpellAbility sa) {
-        Singletons.getModel().getMatch().getInput().setInput(PlayerUtil.inputDiscard(num, sa));
-    }
-
     /** {@inheritDoc} */
     @Override
     public final void discardUnless(final int num, final String uType, final SpellAbility sa) {
         if (this.getCardsIn(ZoneType.Hand).size() > 0) {
-            Singletons.getModel().getMatch().getInput().setInput(PlayerUtil.inputDiscardNumUnless(num, uType, sa));
+            Singletons.getModel().getMatch().getInput().setInput(inputDiscardNumUnless(num, uType, sa));
         }
     }
     
+    private static Input inputDiscardNumUnless(final int nCards, final String uType, final SpellAbility sa) {
+        final SpellAbility sp = sa;
+        final Input target = new InputBase() {
+            private static final long serialVersionUID = 8822292413831640944L;
+
+            private int n = 0;
+
+            @Override
+            public void showMessage() {
+                if (Singletons.getControl().getPlayer().getZone(ZoneType.Hand).size() == 0) {
+                    this.stop();
+                }
+                CMatchUI.SINGLETON_INSTANCE.showMessage(
+                        "Select " + (nCards - this.n) + " cards to discard, unless you discard a " + uType + ".");
+                ButtonUtil.disableAll();
+            }
+
+            @Override
+            public void selectButtonCancel() {
+                this.stop();
+            }
+
+            @Override
+            public void selectCard(final Card card) {
+                Zone zone = Singletons.getModel().getGame().getZoneOf(card);
+                if (zone.is(ZoneType.Hand)) {
+                    card.getController().discard(card, sp);
+                    this.n++;
+
+                    if (card.isType(uType.toString())) {
+                        this.stop();
+                    } else {
+                        if ((this.n == nCards) || (Singletons.getControl().getPlayer().getZone(ZoneType.Hand).size() == 0)) {
+                            this.stop();
+                        } else {
+                            this.showMessage();
+                        }
+                    }
+                }
+            }
+        };
+
+        return target;
+    } // input_discardNumUnless
+    
+
     @Override
     public PlayerType getType() {
         return PlayerType.HUMAN;

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

@@ -74,7 +74,6 @@ import forge.game.zone.ZoneType;
 import forge.gui.GuiChoose;
 import forge.gui.GuiDialog;
 import forge.properties.ForgePreferences.FPref;
-import forge.util.Aggregates;
 import forge.util.MyRandom;
 
 /**
@@ -1583,21 +1582,6 @@ public abstract class Player extends GameEntity implements Comparable<Player> {
     // /
     // //////////////////////////////
 
-    /**
-     * <p>
-     * discard.
-     * </p>
-     * 
-     * @param num
-     *            a int.
-     * @param sa
-     *            a {@link forge.card.spellability.SpellAbility} object.
-     * @param duringResolution
-     *            a boolean.
-     * @return a {@link forge.CardList} object.
-     */
-    public abstract void discard(final int num, final SpellAbility sa);
-
     /**
      * <p>
      * discard.
@@ -1609,22 +1593,7 @@ public abstract class Player extends GameEntity implements Comparable<Player> {
      *            a {@link forge.card.spellability.SpellAbility} object.
      * @return a {@link forge.CardList} object.
      */
-    public final List<Card> discard(final Card c, final SpellAbility sa) {
+    public final boolean discard(final Card c, final SpellAbility sa) {
-        this.doDiscard(c, sa);
-        return CardLists.createCardList(c);
-    }
-
-    /**
-     * <p>
-     * doDiscard.
-     * </p>
-     * 
-     * @param c
-     *            a {@link forge.Card} object.
-     * @param sa
-     *            a {@link forge.card.spellability.SpellAbility} object.
-     */
-    protected final void doDiscard(final Card c, final SpellAbility sa) {
         FThreads.checkEDT("Player.doDiscard", false);
         // TODO: This line should be moved inside CostPayment somehow
         /*if (sa != null) {
@@ -1640,7 +1609,7 @@ public abstract class Player extends GameEntity implements Comparable<Player> {
         repRunParams.put("Affected", this);
 
         if (game.getReplacementHandler().run(repRunParams) != ReplacementResult.NotReplaced) {
-            return;
+            return false;
         }
 
         game.getAction().discardMadness(c, this);
@@ -1668,67 +1637,10 @@ public abstract class Player extends GameEntity implements Comparable<Player> {
         runParams.put("Card", c);
         runParams.put("Cause", cause);
         game.getTriggerHandler().runTrigger(TriggerType.Discarded, runParams, false);
+        return true;
 
     } // end doDiscard
 
-    /**
-     * <p>
-     * discardHand.
-     * </p>
-     * 
-     * @param sa
-     *            a {@link forge.card.spellability.SpellAbility} object.
-     * @return the card list
-     */
-    public final List<Card> discardHand(final SpellAbility sa) {
-        final List<Card> list = new ArrayList<Card>(this.getCardsIn(ZoneType.Hand));
-        this.discardRandom(list.size(), sa);
-        return list;
-    }
-
-
-    /**
-     * <p>
-     * discardRandom.
-     * </p>
-     * 
-     * @param num
-     *            a int.
-     * @param sa
-     *            a {@link forge.card.spellability.SpellAbility} object.
-     * @return a List<Card> of cards discarded
-     */
-    public final List<Card> discardRandom(final int num, final SpellAbility sa) {
-        return this.discardRandom(num, sa, "Card");
-    }
-
-    /**
-     * <p>
-     * discardRandom.
-     * </p>
-     * 
-     * @param num
-     *            a int.
-     * @param sa
-     *            a {@link forge.card.spellability.SpellAbility} object.
-     * @param valid
-     *            a valid expression
-     * @return a List<Card> of cards discarded
-     */
-    public final List<Card> discardRandom(final int num, final SpellAbility sa, final String valid) {
-        final List<Card> discarded = new ArrayList<Card>();
-        for (int i = 0; i < num; i++) {
-            final List<Card> list =
-                    CardLists.getValidCards(this.getCardsIn(ZoneType.Hand), valid, sa.getSourceCard().getController(), sa.getSourceCard());
-            if (list.size() != 0) {
-                final Card disc = Aggregates.random(list);
-                discarded.add(disc);
-                this.doDiscard(disc, sa);
-            }
-        }
-        return discarded;
-    }
-
     /**
      * <p>
      * discardUnless.

src/main/java/forge/game/player/PlayerUtil.java

@@ -1,227 +0,0 @@
-/*
- * Forge: Play Magic: the Gathering.
- * Copyright (C) 2011  Forge Team
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- * 
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- * 
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-package forge.game.player;
-
-import java.util.List;
-
-import forge.Card;
-import forge.CardLists;
-import forge.Singletons;
-import forge.card.spellability.SpellAbility;
-import forge.control.input.Input;
-import forge.control.input.InputBase;
-import forge.game.zone.Zone;
-import forge.game.zone.ZoneType;
-import forge.gui.match.CMatchUI;
-import forge.view.ButtonUtil;
-
-/**
- * <p>
- * PlayerUtil class.
- * </p>
- * 
- * @author Forge
- * @version $Id$
- */
-public final class PlayerUtil {
-
-    private PlayerUtil() {
-        throw new AssertionError();
-    }
-
-    /**
-     * <p>
-     * input_discardNumUnless.
-     * </p>
-     * 
-     * @param nCards
-     *            a int.
-     * @param uType
-     *            a {@link java.lang.String} object.
-     * @param sa
-     *            a {@link forge.card.spellability.SpellAbility} object.
-     * @return a {@link forge.control.input.InputBase} object.
-     * @since 1.0.15
-     */
-    public static Input inputDiscardNumUnless(final int nCards, final String uType, final SpellAbility sa) {
-        final SpellAbility sp = sa;
-        final Input target = new InputBase() {
-            private static final long serialVersionUID = 8822292413831640944L;
-
-            private int n = 0;
-
-            @Override
-            public void showMessage() {
-                if (Singletons.getControl().getPlayer().getZone(ZoneType.Hand).size() == 0) {
-                    this.stop();
-                }
-                CMatchUI.SINGLETON_INSTANCE.showMessage(
-                        "Select " + (nCards - this.n) + " cards to discard, unless you discard a " + uType + ".");
-                ButtonUtil.disableAll();
-            }
-
-            @Override
-            public void selectButtonCancel() {
-                this.stop();
-            }
-
-            @Override
-            public void selectCard(final Card card) {
-                Zone zone = Singletons.getModel().getGame().getZoneOf(card);
-                if (zone.is(ZoneType.Hand)) {
-                    card.getController().discard(card, sp);
-                    this.n++;
-
-                    if (card.isType(uType.toString())) {
-                        this.stop();
-                    } else {
-                        if ((this.n == nCards) || (Singletons.getControl().getPlayer().getZone(ZoneType.Hand).size() == 0)) {
-                            this.stop();
-                        } else {
-                            this.showMessage();
-                        }
-                    }
-                }
-            }
-        };
-
-        return target;
-    } // input_discardNumUnless
-
-    /**
-     * <p>
-     * input_discard.
-     * </p>
-     * 
-     * @param nCards
-     *            a int.
-     * @param sa
-     *            a {@link forge.card.spellability.SpellAbility} object.
-     * @return a {@link forge.control.input.InputBase} object.
-     * @since 1.0.15
-     */
-    public static Input inputDiscard(final int nCards, final SpellAbility sa) {
-        final SpellAbility sp = sa;
-        final Input target = new InputBase() {
-            private static final long serialVersionUID = -329993322080934435L;
-
-            private int n = 0;
-
-            @Override
-            public void showMessage() {
-                if (Singletons.getControl().getPlayer().getZone(ZoneType.Hand).size() == 0) {
-                    this.stop();
-                }
-                if (nCards == 0) {
-                    this.stop();
-                }
-
-                CMatchUI.SINGLETON_INSTANCE.showMessage("Select a card to discard");
-                ButtonUtil.disableAll();
-            }
-
-            @Override
-            public void selectCard(final Card card) {
-                Zone zone = Singletons.getModel().getGame().getZoneOf(card);
-                if (zone.is(ZoneType.Hand)) {
-                    card.getController().discard(card, sp);
-                    this.n++;
-
-                    // in case no more cards in hand
-                    if ((this.n == nCards) || (Singletons.getControl().getPlayer().getZone(ZoneType.Hand).size() == 0)) {
-                        this.stop();
-                    } else {
-                        this.showMessage();
-                    }
-                }
-            }
-        };
-        return target;
-    } // input_discard()
-
-    /**
-     * <p>
-     * input_sacrificePermanents.
-     * </p>
-     * 
-     * @param nCards
-     *            a int.
-     * @param type
-     *            a {@link java.lang.String} object.
-     * @return a {@link forge.control.input.InputBase} object.
-     * @since 1.0.15
-     */
-    public static Input inputSacrificePermanents(final int nCards, final String type) {
-        final List<Card> list = CardLists.getType(Singletons.getControl().getPlayer().getCardsIn(ZoneType.Battlefield), type);
-
-        return PlayerUtil.inputSacrificePermanentsFromList(nCards, list, "Select a " + type + " to sacrifice");
-    } // input_sacrificePermanents()
-
-    /**
-     * <p>
-     * input_sacrificePermanentsFromList.
-     * </p>
-     * 
-     * @param nCards
-     *            a int.
-     * @param list
-     *            a {@link forge.CardList} object.
-     * @param message
-     *            a {@link java.lang.String} object.
-     * @return a {@link forge.control.input.InputBase} object.
-     * @since 1.0.15
-     */
-    public static Input inputSacrificePermanentsFromList(final int nCards, final List<Card> list, final String message) {
-        final Input target = new InputBase() {
-            private static final long serialVersionUID = 1981791992623774490L;
-            private int n = 0;
-
-            @Override
-            public void showMessage() {
-                // in case no more {type}s in play
-                if ((this.n == nCards) || (list.size() == 0)) {
-                    this.stop();
-                    return;
-                }
-
-                CMatchUI.SINGLETON_INSTANCE.showMessage(message + " (" + (nCards - this.n) + " left)");
-                ButtonUtil.disableAll();
-            }
-
-            @Override
-            public void selectCard(final Card card) {
-                Zone zone = Singletons.getModel().getGame().getZoneOf(card);
-                if (zone.equals(Singletons.getControl().getPlayer().getZone(ZoneType.Battlefield)) && list.contains(card)) {
-                    Singletons.getModel().getGame().getAction().sacrifice(card, null);
-                    this.n++;
-                    list.remove(card);
-
-                    // in case no more {type}s in play
-                    if ((this.n == nCards) || (list.size() == 0)) {
-                        this.stop();
-                        return;
-                    } else {
-                        this.showMessage();
-                    }
-                }
-            }
-        };
-        return target;
-    } // input_sacrificePermanents()
-
-} // end class PlayerUtil