PlayerController - added chooseSinglePlayerForEffect , removed more calls to ishuman/iscomputer

src/main/java/forge/card/ability/SpellAbilityAi.java

@@ -118,7 +118,7 @@ public abstract class SpellAbilityAi {
         return options.get(0);
     }
     
-    public Player chooseSinglePlayer(Player ai, SpellAbility sa, List<Player> options, boolean isOptional) {
+    public Player chooseSinglePlayer(Player ai, SpellAbility sa, List<Player> options) {
         System.err.println("Warning: default (ie. inherited from base class) implementation of chooseSinglePlayer is used for " + this.getClass().getName() + ". Consider declaring an overloaded method");
         return options.get(0);
     }

src/main/java/forge/card/ability/ai/AttachAi.java

@@ -29,6 +29,7 @@ import forge.game.phase.CombatUtil;
 import forge.game.phase.PhaseHandler;
 import forge.game.phase.PhaseType;
 import forge.game.player.Player;
+import forge.game.player.PlayerActionConfirmMode;
 import forge.game.zone.ZoneType;
 import forge.util.MyRandom;
 
@@ -675,7 +676,7 @@ public class AttachAi extends SpellAbilityAi {
      *            the mandatory
      * @return true, if successful
      */
-    public static boolean attachPreference(final SpellAbility sa, final Target tgt, final boolean mandatory) {
+    private static boolean attachPreference(final SpellAbility sa, final Target tgt, final boolean mandatory) {
         Object o;
         if (tgt.canTgtPlayer()) {
             o = attachToPlayerAIPreferences(sa.getActivatingPlayer(), sa, mandatory);
@@ -1164,4 +1165,18 @@ public class AttachAi extends SpellAbilityAi {
         return true;
     }
 
+    @Override
+    public boolean confirmAction(Player player, SpellAbility sa, PlayerActionConfirmMode mode, String message) {
+        return true;
+    }
+    
+    @Override
+    public Card chooseSingleCard(Player ai, SpellAbility sa, List<Card> options, boolean isOptional) {
+        return attachToCardAIPreferences(ai, sa, true);
+    }
+    
+    @Override
+    public Player chooseSinglePlayer(Player ai, SpellAbility sa, List<Player> options) {
+        return attachToPlayerAIPreferences(ai, sa, true);
+    }
 }

src/main/java/forge/card/ability/ai/ChangeZoneAi.java

@@ -1389,4 +1389,14 @@ public class ChangeZoneAi extends SpellAbilityAi {
         return ComputerUtilCard.getBestAI(options);
     }
     
+    /* (non-Javadoc)
+     * @see forge.card.ability.SpellAbilityAi#chooseSinglePlayer(forge.game.player.Player, forge.card.spellability.SpellAbility, java.util.List)
+     */
+    @Override
+    public Player chooseSinglePlayer(Player ai, SpellAbility sa, List<Player> options) {
+        // Currently only used by Curse of Misfortunes, so this branch should never get hit
+        // But just in case it does, just select the first option
+        return options.get(0);
+    }
+    
 }

src/main/java/forge/card/ability/ai/ChoosePlayerAi.java

@@ -1,5 +1,7 @@
 package forge.card.ability.ai;
 
+import java.util.List;
+
 import forge.card.ability.SpellAbilityAi;
 import forge.card.spellability.SpellAbility;
 import forge.game.player.Player;
@@ -24,4 +26,29 @@ public class ChoosePlayerAi extends SpellAbilityAi {
         return canPlayAI(ai, sa);
     }
 
+    /* (non-Javadoc)
+     * @see forge.card.ability.SpellAbilityAi#chooseSinglePlayer(forge.game.player.Player, forge.card.spellability.SpellAbility, java.util.List)
+     */
+    @Override
+    public Player chooseSinglePlayer(Player ai, SpellAbility sa, List<Player> choices) {
+        Player chosen = null;
+        if ("Curse".equals(sa.getParam("AILogic"))) {
+            for (Player pc : choices) {
+                if (pc.isOpponentOf(ai)) {
+                    chosen = pc;
+                    break;
+                }
+            }
+            if (chosen == null) {
+                System.out.println("No good curse choices. Picking first available: " + choices.get(0));
+                chosen = choices.get(0);
+            }
+        } else if ("Pump".equals(sa.getParam("AILogic"))) {
+            chosen = choices.contains(ai) ? ai : choices.get(0);
+        } else {
+            System.out.println("Default player choice logic.");
+            chosen = choices.contains(ai) ? ai : choices.get(0);
+        }
+        return chosen;
+    }
 }

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

@@ -10,14 +10,11 @@ import forge.GameEntity;
 import forge.card.ability.AbilityUtils;
 import forge.card.ability.ApiType;
 import forge.card.ability.SpellAbilityEffect;
-import forge.card.ability.ai.AttachAi;
 import forge.card.spellability.SpellAbility;
 import forge.card.spellability.Target;
 import forge.game.Game;
 import forge.game.player.Player;
 import forge.game.zone.ZoneType;
-import forge.gui.GuiChoose;
-import forge.gui.GuiDialog;
 
 public class AttachEffect extends SpellAbilityEffect {
 
@@ -46,12 +43,10 @@ public class AttachEffect extends SpellAbilityEffect {
             card = AbilityUtils.getDefinedCards(source, sa.getParam("Object"), sa).get(0);
         }
 
-        final StringBuilder sb = new StringBuilder();
+        final Player p = sa.getActivatingPlayer();
-        sb.append("Do you want to attach " + card + " to " + targets + "?");
+        String message = "Do you want to attach " + card + " to " + targets + "?";
-        if (sa.getActivatingPlayer().isHuman() && sa.hasParam("Optional")
+        if ( sa.hasParam("Optional") && !p.getController().confirmAction(sa, null, message) )
-                && !GuiDialog.confirm(source, sb.toString())) {
             return;
-        }
 
         // If Cast Targets will be checked on the Stack
         for (final Object o : targets) {
@@ -158,14 +153,12 @@ public class AttachEffect extends SpellAbilityEffect {
      * @return the attach spell ability
      */
     public static SpellAbility getAttachSpellAbility(final Card source) {
-        SpellAbility aura = null;
         for (final SpellAbility sa : source.getSpells()) {
             if (sa.getApi() == ApiType.Attach) {
-                aura = sa;
+                return sa;
-                break;
             }
         }
-        return aura;
+        return null;
     }
 
     /**
@@ -187,50 +180,34 @@ public class AttachEffect extends SpellAbilityEffect {
         final Game game = source.getGame();
         final Target tgt = aura.getTarget();
 
-        if (source.getController().isHuman()) {
+        Player p = source.getController();
-            if (tgt.canTgtPlayer()) {
+        if (tgt.canTgtPlayer()) {
-                final ArrayList<Player> players = new ArrayList<Player>();
+            final ArrayList<Player> players = new ArrayList<Player>();
 
-                for (Player player : game.getPlayers()) {
+            for (Player player : game.getPlayers()) {
-                    if (player.isValid(tgt.getValidTgts(), aura.getActivatingPlayer(), source)) {
+                if (player.isValid(tgt.getValidTgts(), aura.getActivatingPlayer(), source)) {
-                        players.add(player);
+                    players.add(player);
-                    }
-                }
-
-                final Player p = GuiChoose.one(source + " - Select a player to attach to.", players);
-                if (p != null) {
-                    handleAura(source, p);
-                    return true;
-                }
-            } else {
-                List<Card> list = game.getCardsIn(tgt.getZone());
-                list = CardLists.getValidCards(list, tgt.getValidTgts(), aura.getActivatingPlayer(), source);
-                if (list.isEmpty()) {
-                    return false;
-                }
-
-                final Object o = GuiChoose.one(source + " - Select a card to attach to.", list);
-                if (o instanceof Card) {
-                    handleAura(source, (Card) o);
-                    //source.enchantEntity((Card) o);
-                    return true;
                 }
             }
-        }
+            final Player pa = p.getController().chooseSinglePlayerForEffect(players, aura, source + " - Select a player to attach to.");
+            if (pa != null) {
+                handleAura(source, pa);
+                return true;
+            }
+        } else {
+            List<Card> list = game.getCardsIn(tgt.getZone());
+            list = CardLists.getValidCards(list, tgt.getValidTgts(), aura.getActivatingPlayer(), source);
+            if (list.isEmpty()) {
+                return false;
+            }
 
-        else if (AttachAi.attachPreference(aura, tgt, true)) {
+            final Card o = p.getController().chooseSingleCardForEffect(list, aura, source + " - Select a card to attach to.");
-            final Object o = aura.getTarget().getTargets().get(0);
+            if (o != null) {
-            if (o instanceof Card) {
-                //source.enchantEntity((Card) o);
                 handleAura(source, (Card) o);
-                return true;
+                //source.enchantEntity((Card) o);
-            } else if (o instanceof Player) {
-                //source.enchantEntity((Player) o);
-                handleAura(source, (Player) o);
                 return true;
             }
         }
-
         return false;
     }
 }

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

@@ -17,7 +17,6 @@ import forge.card.spellability.SpellAbility;
 import forge.game.Game;
 import forge.game.player.Player;
 import forge.game.zone.ZoneType;
-import forge.gui.GuiChoose;
 import forge.util.MyRandom;
 
 public class ChangeZoneAllEffect extends SpellAbilityEffect {
@@ -65,10 +64,10 @@ public class ChangeZoneAllEffect extends SpellAbilityEffect {
                 cards.addAll(p.getCardsIn(origin));
             }
         }
-        if (sa.getActivatingPlayer().isHuman() && origin.contains(ZoneType.Library)
+        
-                && sa.hasParam("Search")) {
+        if (origin.contains(ZoneType.Library) && sa.hasParam("Search")) {
-            GuiChoose.oneOrNone("Looking at the Library",
+            List<Card> libCards = CardLists.getValidCards(cards, "Card.inZoneLibrary", sa.getActivatingPlayer(), sa.getSourceCard());
-                    CardLists.getValidCards(cards, "Card.inZoneLibrary", sa.getActivatingPlayer(), sa.getSourceCard()));
+            sa.getActivatingPlayer().getController().reveal("Looking at the Library", libCards, ZoneType.Library, sa.getActivatingPlayer());
         }
         cards = AbilityUtils.filterListByType(cards, sa.getParam("ChangeType"), sa);
 

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

@@ -813,19 +813,7 @@ public class ChangeZoneEffect extends SpellAbilityEffect {
                         List<Player> list = AbilityUtils.getDefinedPlayers(card,
                                 sa.getParam("AttachedToPlayer"), sa);
                         if (!list.isEmpty()) {
-                            Player attachedTo = null;
+                            Player attachedTo = player.getController().chooseSinglePlayerForEffect(list, sa, c + " - Select a player to attach to.");
-
-                            if (list.size() == 1) {
-                                attachedTo = list.get(0);
-                            } else {
-                                if (player.isHuman()) {
-                                    attachedTo = GuiChoose.one(c + " - Select a player to attach to.", list);
-                                } else { // AI player
-                                    // Currently only used by Curse of Misfortunes, so this branch should never get hit
-                                    // But just in case it does, just select the first option
-                                    attachedTo = list.get(0);
-                                }
-                            }
                             if (c.isAura()) {
                                 if (c.isEnchanting()) {
                                     // If this Card is already Enchanting something, need

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

@@ -8,7 +8,6 @@ import forge.card.ability.SpellAbilityEffect;
 import forge.card.spellability.SpellAbility;
 import forge.card.spellability.Target;
 import forge.game.player.Player;
-import forge.gui.GuiChoose;
 
 public class ChoosePlayerEffect extends SpellAbilityEffect {
 
@@ -39,31 +38,12 @@ public class ChoosePlayerEffect extends SpellAbilityEffect {
 
         for (final Player p : tgtPlayers) {
             if ((tgt == null) || p.canBeTargetedBy(sa)) {
-                Player chosen = null;
+
-                if (p.isHuman()) {
+                // Was if (sa.getActivatingPlayer().isHuman()) but defined player was being
-                    // Was if (sa.getActivatingPlayer().isHuman()) but defined player was being
+                // overwritten by activatingPlayer (or controller if no activator was set).
-                    // overwritten by activatingPlayer (or controller if no activator was set).
+                // Revert if it causes issues and remove Goblin Festival from card database.
-                    // Revert if it causes issues and remove Goblin Festival from card database.
+
-                    chosen = GuiChoose.one(choiceDesc, choices);
+                Player chosen = choices.isEmpty() ? null : p.getController().chooseSinglePlayerForEffect(choices, sa, choiceDesc);
-                } else {
-                    if ("Curse".equals(sa.getParam("AILogic"))) {
-                        for (Player pc : choices) {
-                            if (pc.isOpponentOf(p)) {
-                                chosen = pc;
-                                break;
-                            }
-                        }
-                        if (chosen == null) {
-                            System.out.println("No good curse choices. Picking first available: " + choices.get(0));
-                            chosen = choices.get(0);
-                        }
-                    } else if ("Pump".equals(sa.getParam("AILogic"))) {
-                        chosen = choices.contains(p) ? p : choices.get(0);
-                    } else {
-                        System.out.println("Default player choice logic.");
-                        chosen = choices.contains(p) ? p : choices.get(0);
-                    }
-                }
 
                 if( null != chosen )
                     card.setChosenPlayer(chosen);

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

@@ -93,8 +93,6 @@ public abstract class PlayerController {
     public abstract boolean playCascade(Card cascadedCard, Card sourceCard);
     public abstract void playSpellAbilityForFree(SpellAbility copySA);
 
-   
-    
     public abstract Deck sideboard(final Deck deck, GameType gameType);
 
 
@@ -107,6 +105,7 @@ public abstract class PlayerController {
 
     public Card chooseSingleCardForEffect(List<Card> sourceList, SpellAbility sa, String title) { return chooseSingleCardForEffect(sourceList, sa, title, false); }
     public abstract Card chooseSingleCardForEffect(List<Card> sourceList, SpellAbility sa, String title, boolean isOptional);
+    public abstract Player chooseSinglePlayerForEffect(List<Player> options, SpellAbility sa, String title);
     public abstract boolean confirmAction(SpellAbility sa, PlayerActionConfirmMode mode, String message);
     public abstract boolean getWillPlayOnFirstTurn(boolean isFirstGame);
     public abstract boolean confirmStaticApplication(Card hostCard, GameEntity affected, String logic, String message);
@@ -142,4 +141,5 @@ public abstract class PlayerController {
     
     public abstract List<Card> chooseCardsToDiscardToMaximumHandSize(int numDiscard);
     public abstract boolean payManaOptional(Card card, Cost cost, String prompt, ManaPaymentPurpose purpose);
+
 }

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

@@ -140,10 +140,19 @@ public class PlayerControllerAi extends PlayerController {
         if ( null == api ) {
             throw new InvalidParameterException("SA is not api-based, this is not supported yet");
         }
-        
         return api.getAi().chooseSingleCard(player, sa, options, isOptional);
     }
 
+    @Override
+    public Player chooseSinglePlayerForEffect(List<Player> options, SpellAbility sa, String title) {
+        ApiType api = sa.getApi();
+        if ( null == api ) {
+            throw new InvalidParameterException("SA is not api-based, this is not supported yet");
+        }
+        return api.getAi().chooseSinglePlayer(player, sa, options);
+    }
+
+    
     @Override
     public boolean confirmAction(SpellAbility sa, PlayerActionConfirmMode mode, String message) {
         return getAi().confirmAction(sa, mode, message);

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

@@ -269,6 +269,16 @@ public class PlayerControllerHuman extends PlayerController {
             return options.get(0);
     }
 
+    @Override
+    public Player chooseSinglePlayerForEffect(List<Player> options, SpellAbility sa, String title) {
+        // Human is supposed to read the message and understand from it what to choose
+        if ( options.size() > 2 )
+            return GuiChoose.one(title, options);
+        else 
+            return options.get(0);
+    }
+
+
     /* (non-Javadoc)
      * @see forge.game.player.PlayerController#confirmAction(forge.card.spellability.SpellAbility, java.lang.String, java.lang.String)
      */