minifying calls to isPlayer/isComputer: bond (done), discard (wip)

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

@@ -6,10 +6,8 @@ import forge.Card;
 import forge.Singletons;
 import forge.card.ability.AbilityUtils;
 import forge.card.ability.SpellAbilityEffect;
-import forge.card.cardfactory.CardFactoryUtil;
 import forge.card.spellability.SpellAbility;
 import forge.game.zone.ZoneType;
-import forge.gui.GuiChoose;
 
 public class BondEffect extends SpellAbilityEffect {
     @Override
@@ -32,15 +30,7 @@ public class BondEffect extends SpellAbilityEffect {
         Card partner = cards.get(0);
         // skip choice if only one card on list
         if (cards.size() > 1) {
-            if (sa.getActivatingPlayer().isHuman()) {
+            sa.getActivatingPlayer().getController().chooseSingleCardForEffect(cards, sa, "Select a card to pair with");
-                Card o = GuiChoose.one("Select a card to pair with", cards);
-                if (o != null) {
-                    partner = o;
-                }
-            } else {
-                // TODO - Pick best creature instead of just the first on the list
-                partner = CardFactoryUtil.getBestCreatureAI(cards);
-            }
         }
 
         // pair choices together

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

@@ -90,68 +90,66 @@ public class DiscardEffect extends RevealEffectBase {
         final Card source = sa.getSourceCard();
         List<Card> dPChHand = new ArrayList<Card>(victim.getCardsIn(ZoneType.Hand));
         dPChHand = CardLists.getValidCards(dPChHand, dValid, source.getController(), source);
-        final List<Card> discarded = new ArrayList<Card>();
+        final List<Card> toDiscard = new ArrayList<Card>();
 
-        if (victim.isComputer()) { // discard AI cards
+        int max = Math.min(dPChHand.size(), numCards);
-            int max = dPChHand.size();
+        List<Card> list = new ArrayList<Card>();
-            max = Math.min(max, numCards);
-            List<Card> list = ((AIPlayer) victim).getAi().getCardsToDiscard(max, dValid, sa);
-            if (isReveal) {
-                GuiChoose.oneOrNone("Computer has chosen", list);
-            }
-            if (list != null) {
-                discarded.addAll(list);
-                for (Card card : list) {
-                    victim.discard(card, sa);
-                }
-            }
-            return discarded;
-        }
         
-        // discard human cards
+        if (!victim.isOpponentOf(chooser) && victim instanceof AIPlayer) { // discard AI cards
-        for (int i = 0; i < numCards; i++) {
+            list = ((AIPlayer) victim).getAi().getCardsToDiscard(max, dValid, sa);
-            if (dPChHand.size() > 0) {
+        } else {
-                List<Card> goodChoices = CardLists.filter(dPChHand, new Predicate<Card>() {
+            // discard hostile or human opponent
-                    @Override
+            for (int i = 0; i < max; i++) {
-                    public boolean apply(final Card c) {
+                Card dC = chooseCardToDiscardFromOpponent(sa, dPChHand);
-                        if (!c.getSVar("DiscardMeByOpp").equals("") || !c.getSVar("DiscardMe").equals("")) {
-                            return false;
-                        }
-                        return true;
-                    }
-                });
-                if (goodChoices.isEmpty()) {
-                    goodChoices = dPChHand;
-                }
-                final List<Card> dChoices = new ArrayList<Card>();
-                if (sa.hasParam("DiscardValid")) {
-                    final String validString = sa.getParam("DiscardValid");
-                    if (validString.contains("Creature") && !validString.contains("nonCreature")) {
-                        final Card c = CardFactoryUtil.getBestCreatureAI(goodChoices);
-                        if (c != null) {
-                            dChoices.add(CardFactoryUtil.getBestCreatureAI(goodChoices));
-                        }
-                    }
-                }
-
-                Collections.sort(goodChoices, CardLists.TextLenReverseComparator);
-
-                CardLists.sortCMC(goodChoices);
-                dChoices.add(goodChoices.get(0));
-
-                final Card dC = Aggregates.random(goodChoices);
                 dPChHand.remove(dC);
-
+                list.add(dC);
-                if (isReveal) {
-                    final List<Card> dCs = new ArrayList<Card>();
-                    dCs.add(dC);
-                    GuiChoose.oneOrNone("Computer has chosen", dCs);
-                }
-                discarded.add(dC);
-                victim.discard(dC, sa);
             }
         }
-        return discarded;
+        
+        if (isReveal) {
+            GuiChoose.oneOrNone("Computer has chosen", list);
+        }
+        
+        return toDiscard;
+
+    }
+
+    /**
+     * TODO: Write javadoc for this method.
+     * @param sa
+     * @param opponentHand
+     * @param list
+     */
+    private Card chooseCardToDiscardFromOpponent(SpellAbility sa, List<Card> opponentHand) {
+        List<Card> goodChoices = CardLists.filter(opponentHand, new Predicate<Card>() {
+            @Override
+            public boolean apply(final Card c) {
+                if (!c.getSVar("DiscardMeByOpp").equals("") || !c.getSVar("DiscardMe").equals("")) {
+                    return false;
+                }
+                return true;
+            }
+        });
+        if (goodChoices.isEmpty()) {
+            goodChoices = opponentHand;
+        }
+        final List<Card> dChoices = new ArrayList<Card>();
+        if (sa.hasParam("DiscardValid")) {
+            final String validString = sa.getParam("DiscardValid");
+            if (validString.contains("Creature") && !validString.contains("nonCreature")) {
+                final Card c = CardFactoryUtil.getBestCreatureAI(goodChoices);
+                if (c != null) {
+                    dChoices.add(CardFactoryUtil.getBestCreatureAI(goodChoices));
+                }
+            }
+        }
+
+        Collections.sort(goodChoices, CardLists.TextLenReverseComparator);
+
+        CardLists.sortCMC(goodChoices);
+        dChoices.add(goodChoices.get(0));
+
+        return Aggregates.random(goodChoices);
     }
 
     @Override
@@ -275,34 +273,47 @@ public class DiscardEffect extends RevealEffectBase {
                             chooser = source.getController().getOpponent();
                         }
 
+                        List<Card> toBeDiscarded = new ArrayList<Card>(); 
                         if (chooser.isComputer()) {
-                            discarded.addAll(discardComputerChooses(sa, p, chooser, numCards, dValid, mode.startsWith("Reveal")));
+                            toBeDiscarded = discardComputerChooses(sa, p, chooser, numCards, dValid, mode.startsWith("Reveal"));
                         } else {
                             // human
                             if (mode.startsWith("Reveal")) {
                                 GuiChoose.oneOrNone("Revealed " + p + "  hand", dPHand);
                             }
 
-                            for (int i = 0; i < numCards; i++) {
+                            if (sa.hasParam("AnyNumber")) {
-                                if (dPChHand.size() > 0) {
+                                List<Card> chosen = getDiscardedList(p, dPChHand, dPChHand.size(), true);
-                                    Card dC = null;
+
-                                    if (sa.hasParam("AnyNumber")) {
+                                for (Card c : chosen) {
-                                        dPChHand = getDiscardedList(p, dPChHand, dPChHand.size(), true);
+                                    dPChHand.remove(chosen);
-                                        for (Card c : dPChHand) {
+                                    toBeDiscarded.add(c);
-                                            discarded.add(c);
-                                            p.discard(c, sa);
-                                        }
-                                    }
-                                    else if (sa.hasParam("Optional")) {
-                                        dC = GuiChoose.oneOrNone("Choose a card to be discarded", dPChHand);
-                                    } else {
-                                        dC = GuiChoose.one("Choose a card to be discarded", dPChHand);
-                                    } if (dC != null) {
-                                        dPChHand.remove(dC);
-                                        discarded.add(dC);
-                                        p.discard(dC, sa);
-                                    }
                                 }
+                            } else 
+                                for (int i = 0; i < numCards; i++) {
+                                if (dPChHand.isEmpty()) {
+                                    break;
+                                }
+                                Card dC = null;
+                                if (sa.hasParam("Optional")) {
+                                    dC = GuiChoose.oneOrNone("Choose a card to be discarded", dPChHand);
+                                } else {
+                                    dC = GuiChoose.one("Choose a card to be discarded", dPChHand);
+                                } 
+                                
+                                if (dC != null) {
+                                    dPChHand.remove(dC);
+                                    toBeDiscarded.add(dC);
+                                }
+                                else break;
+                            }
+                        }
+
+                        if (toBeDiscarded != null) {
+                            for (Card card : toBeDiscarded) {
+                                if ( null == card ) continue;
+                                p.discard(card, sa);
+                                discarded.add(card);
                             }
                         }
                     }

src/main/java/forge/game/phase/CombatUtil.java

@@ -36,6 +36,7 @@ import forge.Command;
 import forge.Constant;
 import forge.GameEntity;
 import forge.Singletons;
+import forge.card.ability.ApiType;
 import forge.card.cardfactory.CardFactoryUtil;
 import forge.card.cost.Cost;
 import forge.card.cost.CostUtil;
@@ -1254,6 +1255,7 @@ public class CombatUtil {
                 final Ability ability = new Ability(c, ManaCost.ZERO) {
                     @Override
                     public void resolve() {
+                        this.api = ApiType.Sacrifice;
                         final Player opponent = Singletons.getModel().getGame().getCombat().getDefendingPlayerRelatedTo(c);
                         //List<Card> list = AbilityUtils.filterListByType(opponent.getCardsIn(ZoneType.Battlefield), "Permanent", this);
                         final List<Card> list = opponent.getCardsIn(ZoneType.Battlefield);

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

@@ -91,4 +91,7 @@ public abstract class PlayerController {
     public abstract String announceRequirements(SpellAbility ability, String announce);
 
     public abstract List<Card> choosePermanentsToSacrifice(List<Card> validTargets, int amount, SpellAbility sa, boolean destroy, boolean isOptional);
+
+    public Card chooseSingleCardForEffect(List<Card> sourceList, SpellAbility sa, String title) { return chooseSingleCardForEffect(sourceList, sa, title); }
+    public abstract Card chooseSingleCardForEffect(List<Card> sourceList, SpellAbility sa, String title, boolean isOptional);
 }

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

@@ -1,10 +1,13 @@
 package forge.game.player;
 
+import java.security.InvalidParameterException;
 import java.util.List;
 import java.util.Map;
 
 import forge.Card;
 import forge.GameEntity;
+import forge.card.ability.ApiType;
+import forge.card.cardfactory.CardFactoryUtil;
 import forge.card.spellability.Spell;
 import forge.card.spellability.SpellAbility;
 import forge.control.input.Input;
@@ -188,5 +191,17 @@ public class PlayerControllerAi extends PlayerController {
         return ComputerUtil.choosePermanentsToSacrifice(player, validTargets, amount, sa, destroy, isOptional);
     }
 
+    @Override
+    public Card chooseSingleCardForEffect(List<Card> options, SpellAbility sa, String title, boolean isOptional) {
+        ApiType api = sa.getApi();
+        if ( null == api ) {
+            throw new InvalidParameterException("SA is not api-based, this is not supported yet");
+        }
+        
+        switch(api) {
+            case Bond: return CardFactoryUtil.getBestCreatureAI(options);
+            default: throw new InvalidParameterException("AI chooseSingleCard does not know how to choose card for " + api);
+        }
+    }
 
 }

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

@@ -228,5 +228,14 @@ public class PlayerControllerHuman extends PlayerController {
         return result;
     }
 
+    @Override
+    public Card chooseSingleCardForEffect(List<Card> options, SpellAbility sa, String title, boolean isOptional) {
+        // Human is supposed to read the message and understand from it what to choose
+        if ( isOptional )
+            return GuiChoose.oneOrNone(title, options);
+        else 
+            return GuiChoose.one(title, options);
+    }
+
 
 }

src/main/java/forge/gui/match/nonsingleton/CField.java

@@ -426,7 +426,7 @@ public class CField implements ICDoc {
                     Singletons.getModel().getGame().getCombat().removeFromCombat(c);
                     CombatUtil.showCombat();
                 } else if (input instanceof InputBlock) {
-                    if (c.getController().isHuman()) {
+                    if (c.getController() == Singletons.getControl().getPlayer() ) {
                         Singletons.getModel().getGame().getCombat().removeFromCombat(c);
                     }
                     ((InputBlock) input).removeFromAllBlocking(c);