Prevent race condition when selecting multiple abilities using Shift+click

forge-ai/src/main/java/forge/ai/AiController.java

@@ -97,7 +97,6 @@ import forge.util.MyRandom;
  * @version $Id$
  */
 public class AiController {
-
     private final Player player;
     private final Game game;
     private final AiCardMemory memory;
@@ -111,13 +110,11 @@ public class AiController {
         this.bCheatShuffle = canCheatShuffle;
     }
 
-    public Game getGame()
+    public Game getGame() {
-    {
         return game;
     }
 
-    public Player getPlayer()
+    public Player getPlayer() {
-    {
         return player;
     }
 
@@ -125,24 +122,12 @@ public class AiController {
         return memory;
     }
 
-    /**
-     * <p>
-     * Constructor for ComputerAI_General.
-     * </p>
-     */
     public AiController(final Player computerPlayer, final Game game0) {
         player = computerPlayer;
         game = game0;
         memory = new AiCardMemory();
     }
 
-    /**
-     * <p>
-     * getAvailableSpellAbilities.
-     * </p>
-     * 
-     * @return a {@link forge.CardList} object.
-     */
     private CardCollection getAvailableCards() {
         CardCollection all = new CardCollection(player.getCardsIn(ZoneType.Hand));
 
@@ -158,13 +143,6 @@ public class AiController {
         return all;
     }
 
-    /**
-     * <p>
-     * getPossibleETBCounters.
-     * </p>
-     * 
-     * @return a {@link java.util.ArrayList} object.
-     */
     private ArrayList<SpellAbility> getPossibleETBCounters() {
         final Player opp = player.getOpponent();
         CardCollection all = new CardCollection(player.getCardsIn(ZoneType.Hand));
@@ -348,13 +326,10 @@ public class AiController {
                 return false;
             }
         }
-
         return true;
     }
 
-    
+    private ArrayList<SpellAbility> getOriginalAndAltCostAbilities(final ArrayList<SpellAbility> originList) {
-    private ArrayList<SpellAbility> getOriginalAndAltCostAbilities(final ArrayList<SpellAbility> originList)
-    {
         final ArrayList<SpellAbility> newAbilities = new ArrayList<SpellAbility>();
         for (SpellAbility sa : originList) {
             sa.setActivatingPlayer(player);
@@ -371,13 +346,6 @@ public class AiController {
         return result;
     }
 
-    /**
-     * Returns the spellAbilities from the card list.
-     * 
-     * @param l
-     *            a {@link forge.CardList} object.
-     * @return an array of {@link forge.game.spellability.SpellAbility} objects.
-     */
     private ArrayList<SpellAbility> getSpellAbilities(final CardCollectionView l) {
         final ArrayList<SpellAbility> spellAbilities = new ArrayList<SpellAbility>();
         for (final Card c : l) {
@@ -388,15 +356,6 @@ public class AiController {
         return spellAbilities;
     }
 
-    /**
-     * <p>
-     * getPlayableCounters.
-     * </p>
-     * 
-     * @param l
-     *            a {@link forge.CardList} object.
-     * @return a {@link java.util.ArrayList} object.
-     */
     private ArrayList<SpellAbility> getPlayableCounters(final CardCollection l) {
         final ArrayList<SpellAbility> spellAbility = new ArrayList<SpellAbility>();
         for (final Card c : l) {
@@ -407,20 +366,11 @@ public class AiController {
                 }
             }
         }
-
         return spellAbility;
     }
 
     // plays a land if one is available
-    /**
-     * <p>
-     * chooseLandsToPlay.
-     * </p>
-     * 
-     * @return a boolean.
-     */
     public CardCollection getLandsToPlay() {
-    
         final CardCollection hand = new CardCollection(player.getCardsIn(ZoneType.Hand));
         hand.addAll(player.getCardsIn(ZoneType.Exile));
         CardCollection landList = CardLists.filter(hand, Presets.LANDS);
@@ -506,15 +456,13 @@ public class AiController {
                 return true;
             }
         });
-    
         return landList;
     }
 
-    public Card chooseBestLandToPlay(CardCollection landList)
+    public Card chooseBestLandToPlay(CardCollection landList) {
-    {
+        if (landList.isEmpty()) {
-        if (landList.isEmpty())
             return null;
-    
+        }
         //Skip reflected lands.
         CardCollection unreflectedLands = new CardCollection(landList);
         for (Card l : landList) {
@@ -594,23 +542,13 @@ public class AiController {
     }
 
     // if return true, go to next phase
-    /**
-     * <p>
-     * playCounterSpell.
-     * </p>
-     * 
-     * @param possibleCounters
-     *            a {@link java.util.ArrayList} object.
-     * @return a boolean.
-     */
     private SpellAbility chooseCounterSpell(final ArrayList<SpellAbility> possibleCounters) {
-        if (possibleCounters == null || possibleCounters.isEmpty())
+        if (possibleCounters == null || possibleCounters.isEmpty()) {
-            return null;;
+            return null;
-        
+        }
         SpellAbility bestSA = null;
         int bestRestriction = Integer.MIN_VALUE;
 
-
         for (final SpellAbility sa : getOriginalAndAltCostAbilities(possibleCounters)) {
             SpellAbility currentSA = sa;
             sa.setActivatingPlayer(player);
@@ -639,7 +577,7 @@ public class AiController {
         // TODO - "Look" at Targeted SA and "calculate" the threshold
         // if (bestRestriction < targetedThreshold) return false;
         return bestSA;
-    } // playCounterSpell()
+    }
 
     public SpellAbility predictSpellToCastInMain2(ApiType exceptSA) {
         return predictSpellToCastInMain2(exceptSA, true);
@@ -669,7 +607,6 @@ public class AiController {
                 }
             }
         }
-
         return null;
     }
 
@@ -701,9 +638,11 @@ public class AiController {
         }
         if (sa.getApi() != null) {
             boolean canPlay = SpellApiToAi.Converter.get(sa.getApi()).canPlayAIWithSubs(player, sa);
-            if (!canPlay) 
+            if (!canPlay) {
                 return AiPlayDecision.CantPlayAi;
-        } else if (sa.getPayCosts() != null){
+            }
+        }
+        else if (sa.getPayCosts() != null){
             Cost payCosts = sa.getPayCosts();
             ManaCost mana = payCosts.getTotalMana();
             if (mana!= null && mana.countX() > 0) {
@@ -796,9 +735,9 @@ public class AiController {
                     }
                 }
             }
-
             return canPlayFromEffectAI((SpellPermanent)sa, false, true);
-        } else if (sa instanceof Spell) {
+        }
+        if (sa instanceof Spell) {
             return canPlaySpellBasic(card);
         }
         return AiPlayDecision.WillPlay;
@@ -901,22 +840,8 @@ public class AiController {
 
             return p;
         }
-    }; // Comparator
+    };
 
-    /**
-     * <p>
-     * AI_discardNumType.
-     * </p>
-     * 
-     * @param numDiscard
-     *            a int.
-     * @param uTypes
-     *            an array of {@link java.lang.String} objects. May be null for
-     *            no restrictions.
-     * @param sa
-     *            a {@link forge.game.spellability.SpellAbility} object.
-     * @return a CardCollection of discarded cards.
-     */
     public CardCollection getCardsToDiscard(final int numDiscard, final String[] uTypes, final SpellAbility sa) {
         CardCollection hand = new CardCollection(player.getCardsIn(ZoneType.Hand));
         if ((uTypes != null) && (sa != null)) {
@@ -981,7 +906,8 @@ public class AiController {
             if (canDiscardLands) {
                 discardList.add(landsInHand.get(0));
                 validCards.remove(landsInHand.get(0));
-            } else { // Discard other stuff
+            }
+            else { // Discard other stuff
                 CardLists.sortByCmcDesc(validCards);
                 int numLandsAvailable = numLandsInPlay;
                 if (numLandsInHand > 0) {
@@ -991,7 +917,8 @@ public class AiController {
                 if (validCards.get(0).getCMC() > numLandsAvailable) {
                     discardList.add(validCards.get(0));
                     validCards.remove(validCards.get(0));
-                } else { //Discard worst card
+                }
+                else { //Discard worst card
                     Card worst = ComputerUtilCard.getWorstAI(validCards);
                     discardList.add(worst);
                     validCards.remove(worst);
@@ -1006,41 +933,37 @@ public class AiController {
         ApiType api = sa.getApi();
 
         // Abilities without api may also use this routine, However they should provide a unique mode value
-        if (null == api) {
+        if (api == null) {
             if (mode != null) switch (mode) {
                 // case BraidOfFire: return true;
                 // case Ripple: return true;
             }
-
             String exMsg = String.format("AI confirmAction does not know what to decide about %s mode (api is null).", mode);
             throw new IllegalArgumentException(exMsg);
-
+        }
-        } else 
         return SpellApiToAi.Converter.get(api).confirmAction(player, sa, mode, message);
     }
 
-    public boolean confirmBidAction(SpellAbility sa, PlayerActionConfirmMode mode, String message,
+    public boolean confirmBidAction(SpellAbility sa, PlayerActionConfirmMode mode, String message, int bid, Player winner) {
-            int bid, Player winner) {
         if (mode != null) switch (mode) {
             case BidLife:
                 if (sa.hasParam("AIBidMax")) {
                     return !player.equals(winner) && bid < Integer.parseInt(sa.getParam("AIBidMax")) && player.getLife() > bid + 5;
-                } else {
-                    return false;
                 }
+                return false;
             default:
                 return false;
         } 
         return false;
     }
 
-
     public boolean confirmStaticApplication(Card hostCard, GameEntity affected, String logic, String message) {
         if (logic.equalsIgnoreCase("ProtectFriendly")) {
             final Player controller = hostCard.getController();
             if (affected instanceof Player) {
                 return !((Player) affected).isOpponentOf(controller);
-            } else if (affected instanceof Card) {
+            }
+            if (affected instanceof Card) {
                 return !((Card) affected).getController().isOpponentOf(controller);
             }
         }
@@ -1084,28 +1007,30 @@ public class AiController {
                 if (AiPlayDecision.WillPlay != canPlayFromEffectAI((Spell) sa, mandatory, withoutPayingManaCost)) {
                     continue;
                 }
-            } else {
+            }
+            else {
                 if (AiPlayDecision.WillPlay == canPlaySa(sa)) {
                     continue;
                 }
             }
 
-            if (withoutPayingManaCost)
+            if (withoutPayingManaCost) {
                 ComputerUtil.playSpellAbilityWithoutPayingManaCost(player, sa, game);
-            else if (!ComputerUtilCost.canPayCost(sa, player)) 
+            }
+            else if (!ComputerUtilCost.canPayCost(sa, player)) {
                 continue;
-            else
+            }
+            else {
                 ComputerUtil.playStack(sa, player, game);
+            }
             return sa;
         }
         return null;
     }
 
     public AiPlayDecision canPlayFromEffectAI(Spell spell, boolean mandatory, boolean withoutPayingManaCost) {
-        
         final Card card = spell.getHostCard();
-        if (spell instanceof SpellApiBased)
+        if (spell instanceof SpellApiBased) {
-        {
             boolean chance = false;
             if (withoutPayingManaCost) {
                 chance = SpellApiToAi.Converter.get(spell.getApi()).doTriggerNoCostWithSubs(player, spell, mandatory);
@@ -1184,7 +1109,7 @@ public class AiController {
         return canPlaySpellBasic(card);
     }
 
-    public SpellAbility chooseSpellAbilityToPlay() {
+    public List<SpellAbility> chooseSpellAbilityToPlay() {
         final PhaseType phase = game.getPhaseHandler().getPhase();
 
         if (game.getStack().isEmpty() && phase.isMain()) {
@@ -1194,14 +1119,21 @@ public class AiController {
                 Card land = chooseBestLandToPlay(landsWannaPlay);
                 if (ComputerUtil.damageFromETB(player, land) < player.getLife() || !player.canLoseLife()) {
                     Ability.PLAY_LAND_SURROGATE.setHostCard(land);
-                    return Ability.PLAY_LAND_SURROGATE;
+                    final List<SpellAbility> abilities = new ArrayList<SpellAbility>();
+                    abilities.add(Ability.PLAY_LAND_SURROGATE);
+                    return abilities;
                 }
             }
         }
 
         SpellAbility sa = getSpellAbilityToPlay();
+        if (sa == null) { return null; }
+
         // System.out.println("Chosen to play: " + sa);
-        return sa;
+
+        final List<SpellAbility> abilities = new ArrayList<SpellAbility>();
+        abilities.add(sa);
+        return abilities;
     }
 
     // declares blockers for given defender in a given combat

forge-ai/src/main/java/forge/ai/PlayerControllerAi.java

@@ -381,7 +381,7 @@ public class PlayerControllerAi extends PlayerController {
     }
 
     @Override
-    public SpellAbility chooseSpellAbilityToPlay() {
+    public List<SpellAbility> chooseSpellAbilityToPlay() {
         return brains.chooseSpellAbilityToPlay();
     }
     

forge-game/src/main/java/forge/game/phase/PhaseHandler.java

@@ -19,6 +19,7 @@ package forge.game.phase;
 
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Multimap;
+
 import forge.card.mana.ManaCost;
 import forge.game.*;
 import forge.game.ability.AbilityFactory;
@@ -881,7 +882,7 @@ public class PhaseHandler implements java.io.Serializable {
                 }
 
                 game.fireEvent(new GameEventPlayerPriority(playerTurn, phase, getPriorityPlayer()));
-                SpellAbility chosenSa = null;
+                List<SpellAbility> chosenSa = null;
 
                 int loopCount = 0;
                 do {
@@ -913,7 +914,9 @@ public class PhaseHandler implements java.io.Serializable {
                         System.out.print("... " + pPlayerPriority + " plays " + chosenSa);
                     }
                     pFirstPriority = pPlayerPriority; // all opponents have to pass before stack is allowed to resolve
-                    pPlayerPriority.getController().playChosenSpellAbility(chosenSa);
+                    for (SpellAbility sa : chosenSa) {
+                        pPlayerPriority.getController().playChosenSpellAbility(sa);
+                    }
                     loopCount++;
                 } while (chosenSa != null && (loopCount < 999 || !pPlayerPriority.getController().isAI()));
 

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

@@ -233,7 +233,7 @@ public abstract class PlayerController {
 
     public abstract void declareAttackers(Player attacker, Combat combat);
     public abstract void declareBlockers(Player defender, Combat combat);
-    public abstract SpellAbility chooseSpellAbilityToPlay();
+    public abstract List<SpellAbility> chooseSpellAbilityToPlay();
     public abstract void playChosenSpellAbility(SpellAbility sa);
 
     public abstract CardCollection chooseCardsToDiscardToMaximumHandSize(int numDiscard);

forge-gui-desktop/src/main/java/forge/view/arcane/PlayArea.java

@@ -526,7 +526,7 @@ public class PlayArea extends CardPanelContainer implements CardPanelMouseListen
 
     @Override
     public final void mouseRightClicked(final CardPanel panel, final MouseEvent evt) {
-        selectCard(panel, new MouseTriggerEvent(evt), false);
+        selectCard(panel, new MouseTriggerEvent(evt), evt.isShiftDown()); //select entire stack if shift key down
         super.mouseRightClicked(panel, evt);
     }
 

forge-gui-desktop/src/test/java/forge/gamesimulationtests/util/PlayerControllerForTests.java

@@ -378,7 +378,7 @@ public class PlayerControllerForTests extends PlayerController {
 	}
 
 	@Override
-	public SpellAbility chooseSpellAbilityToPlay() {
+	public List<SpellAbility> chooseSpellAbilityToPlay() {
 		//TODO: This method has to return the spellability chosen by player
 	    // It should not play the sa right from here. The code has been left as it is to quickly adapt to changed playercontroller interface 
 		if (playerActions != null) {

forge-gui/src/main/java/forge/match/input/InputPassPriority.java

@@ -45,7 +45,7 @@ public class InputPassPriority extends InputSyncronizedBase {
     private static final long serialVersionUID = -581477682214137181L;
     private final Player player;
 
-    private SpellAbility chosenSa;
+    private List<SpellAbility> chosenSa;
 
     public InputPassPriority(final PlayerControllerHuman controller, final Player human) {
         super(controller);
@@ -122,7 +122,7 @@ public class InputPassPriority extends InputSyncronizedBase {
         runnable.run(); //just pass priority immediately if no mana floating that would be lost
     }
 
-    public SpellAbility getChosenSa() { return chosenSa; }
+    public List<SpellAbility> getChosenSa() { return chosenSa; }
 
     @Override
     protected boolean onCardSelected(final Card card, final List<Card> otherCardsToSelect, final ITriggerEvent triggerEvent) {
@@ -133,30 +133,22 @@ public class InputPassPriority extends InputSyncronizedBase {
     	}
 
         final SpellAbility ability = player.getController().getAbilityToPlay(abilities, triggerEvent);
-    	if (selectAbility(ability)) {
+    	if (ability != null) {
+            chosenSa = new ArrayList<SpellAbility>();
+            chosenSa.add(ability);
             if (otherCardsToSelect != null && ability.isManaAbility()) {
                 //if mana ability activated, activate same ability on other cards to select if possible
                 String abStr = ability.toUnsuppressedString();
-                final List<SpellAbility> otherAbilitiesToPlay = new ArrayList<SpellAbility>();
                 for (Card c : otherCardsToSelect) {
                     for (SpellAbility ab : c.getAllPossibleAbilities(player, true)) {
                         if (ab.toUnsuppressedString().equals(abStr)) {
-                            otherAbilitiesToPlay.add(ab);
+                            chosenSa.add(ab);
                             break;
                         }
                     }
                 }
-                if (otherAbilitiesToPlay.size() > 0) {
-                    ThreadUtil.invokeInGameThread(new Runnable() { //must execute other abilities on game thread
-                        @Override
-                        public void run() {
-                            for (SpellAbility ab : otherAbilitiesToPlay) {
-                                player.getController().playChosenSpellAbility(ab);
-                            }
-                        }
-                    });
-                }
             }
+            stop();
     	    return true;
     	}
     	return false;
@@ -165,7 +157,8 @@ public class InputPassPriority extends InputSyncronizedBase {
     @Override
     public boolean selectAbility(final SpellAbility ab) {
     	if (ab != null) {
-            chosenSa = ab;
+    	    chosenSa = new ArrayList<SpellAbility>();
+            chosenSa.add(ab);
             stop();
             return true;
         }

forge-gui/src/main/java/forge/player/PlayerControllerHuman.java

@@ -834,7 +834,7 @@ public class PlayerControllerHuman extends PlayerController {
     }
 
     @Override
-    public SpellAbility chooseSpellAbilityToPlay() {
+    public List<SpellAbility> chooseSpellAbilityToPlay() {
         MagicStack stack = game.getStack();
 
         if (mayAutoPass()) {