Merge branch 'master' into 'master'

fixes for multi-player scry; fix bug when human player scrying entire library

See merge request core-developers/forge!1271

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

@@ -1211,7 +1211,7 @@ public class AiController {
     public boolean confirmAction(SpellAbility sa, PlayerActionConfirmMode mode, String message) {
         ApiType api = sa.getApi();
 
-        // Abilities without api may also use this routine, However they should provide a unique mode value
+        // Abilities without api may also use this routine, However they should provide a unique mode value ?? How could this work?
         if (api == null) {
             String exMsg = String.format("AI confirmAction does not know what to decide about %s mode (api is null).",
                     mode);

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

@@ -1164,4 +1164,10 @@ public class PlayerControllerAi extends PlayerController {
 
         return chosenOptCosts;
     }
+
+    @Override
+    public boolean confirmMulliganScry(Player p) {
+        // Always true?
+        return true;
+    }
 }

forge-game/src/main/java/forge/game/GameAction.java

@@ -50,6 +50,7 @@ import forge.util.collect.FCollection;
 import forge.util.collect.FCollectionView;
 import forge.util.maps.HashMapOfLists;
 import forge.util.maps.MapOfLists;
+import org.apache.commons.lang3.tuple.ImmutablePair;
 
 import java.util.*;
 
@@ -1711,10 +1712,17 @@ public class GameAction {
             mulliganDelta++;
         } while (!allKept);
 
-        //Vancouver Mulligan
+        //Vancouver Mulligan as a scry with the decisions inside
+	List<Player> scryers = Lists.newArrayList();
         for(Player p : whoCanMulligan) {
             if (p.getStartingHandSize() > p.getZone(ZoneType.Hand).size()) {
-                p.scry(1, null);
+                scryers.add(p);
+            }
+        }
+
+        for(Player p : scryers) {
+            if (p.getController().confirmMulliganScry(p)) {
+                scry(ImmutableList.of(p), 1, null);
             }
         }
     }
@@ -1812,4 +1820,68 @@ public class GameAction {
         runParams.put("Player", p);
         game.getTriggerHandler().runTrigger(TriggerType.BecomeMonarch, runParams, false);
     }
+
+    // Make scry an action function so that it can be used for mulligans (with a null cause)
+    // Assumes that the list of players is in APNAP order, which should be the case
+    // Optional here as well to handle the way that mulligans do the choice
+    // 701.17. Scry
+    // 701.17a To “scry N” means to look at the top N cards of your library, then put any number of them
+    // on the bottom of your library in any order and the rest on top of your library in any order.
+    // 701.17b If a player is instructed to scry 0, no scry event occurs. Abilities that trigger whenever a
+    // player scries won’t trigger.
+    // 701.17c If multiple players scry at once, each of those players looks at the top cards of their library
+    // at the same time. Those players decide in APNAP order (see rule 101.4) where to put those
+    // cards, then those cards move at the same time.
+    public void scry(List<Player> players, int numScry, SpellAbility cause) {
+        if (numScry == 0) {
+            return;
+        }
+        // reveal the top N library cards to the player (only)
+        // no real need to separate out the look if
+        // there is only one player scrying
+        if (players.size() > 1) {
+            for (final Player p : players) {
+                final CardCollection topN = new CardCollection(p.getCardsIn(ZoneType.Library, numScry));
+                revealTo(topN, p);
+            }
+        }
+        // make the decisions
+        List<ImmutablePair<CardCollection, CardCollection>> decisions = Lists.newArrayList();
+        for (final Player p : players) {
+            final CardCollection topN = new CardCollection(p.getCardsIn(ZoneType.Library, numScry));
+            ImmutablePair<CardCollection, CardCollection> decision = p.getController().arrangeForScry(topN);
+            decisions.add(decision);
+            int numToTop = decision.getLeft() == null ? 0 : decision.getLeft().size();
+            int numToBottom = decision.getRight() == null ? 0 : decision.getRight().size();
+
+            // publicize the decision
+            game.fireEvent(new GameEventScry(p, numToTop, numToBottom));
+        }
+        // do the moves after all the decisions (maybe not necesssary, but let's
+        // do it the official way)
+        for (int i = 0; i < players.size(); i++) {
+            // no good iterate simultaneously in Java
+            final Player p = players.get(i);
+            final CardCollection toTop = decisions.get(i).getLeft();
+            final CardCollection toBottom = decisions.get(i).getRight();
+            if (toTop != null) {
+                Collections.reverse(toTop); // reverse to get the correct order
+                for (Card c : toTop) {
+                    moveToLibrary(c, cause, null);
+                }
+            }
+            if (toBottom != null) {
+                for (Card c : toBottom) {
+                    moveToBottomOfLibrary(c, cause, null);
+                }
+            }
+
+            if (cause != null) {
+                // set up triggers (but not actually do them until later)
+                final Map<String, Object> runParams = Maps.newHashMap();
+                runParams.put("Player", p);
+                game.getTriggerHandler().runTrigger(TriggerType.Scry, runParams, false);
+            }
+        }
+    }
 }

forge-game/src/main/java/forge/game/ability/effects/ScryEffect.java

@@ -4,17 +4,17 @@ import forge.game.ability.AbilityUtils;
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.TargetRestrictions;
 import java.util.List;
 
+import com.google.common.collect.Lists;
+
+
 public class ScryEffect extends SpellAbilityEffect {
     @Override
     protected String getStackDescription(SpellAbility sa) {
         final StringBuilder sb = new StringBuilder();
 
-        final List<Player> tgtPlayers = getTargetPlayers(sa);
-
-        for (final Player p : tgtPlayers) {
+        for (final Player p : getTargetPlayers(sa)) {
             sb.append(p.toString()).append(" ");
         }
 
@@ -36,19 +36,16 @@ public class ScryEffect extends SpellAbilityEffect {
 
         boolean isOptional = sa.hasParam("Optional");
 
-        final TargetRestrictions tgt = sa.getTargetRestrictions();
-        final List<Player> tgtPlayers = getTargetPlayers(sa);
+	final List<Player> players = Lists.newArrayList(); // players really affected
 
-        for (final Player p : tgtPlayers) {
-            if ((tgt == null) || p.canBeTargetedBy(sa)) {
-                if (isOptional && !p.getController().confirmAction(sa, null, "Do you want to scry?")) {
-                    continue;
-                }
-
-                p.scry(num, sa);
-            }
-        }
+	// Optional here for spells that have optional multi-player scrying
+        for (final Player p : getTargetPlayers(sa)) {
+	    if ( (!sa.usesTargeting() || p.canBeTargetedBy(sa)) &&
+		 (!isOptional || p.getController().confirmAction(sa, null, "Do you want to scry?")) ) {
+		players.add(p);
+	    }
+	}
+	sa.getActivatingPlayer().getGame().getAction().scry(players, num, sa);
     }
 
-
 }

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

@@ -1262,42 +1262,6 @@ public class Player extends GameEntity implements Comparable<Player> {
         return drawCards(1);
     }
 
-    public void scry(final int numScry, SpellAbility cause) {
-        final CardCollection topN = new CardCollection(this.getCardsIn(ZoneType.Library, numScry));
-
-        if (topN.isEmpty()) {
-            return;
-        }
-
-        final ImmutablePair<CardCollection, CardCollection> lists = getController().arrangeForScry(topN);
-        final CardCollection toTop = lists.getLeft();
-        final CardCollection toBottom = lists.getRight();
-
-        int numToBottom = 0;
-        int numToTop = 0;
-
-        if (toBottom != null) {
-            for(Card c : toBottom) {
-                getGame().getAction().moveToBottomOfLibrary(c, cause, null);
-                numToBottom++;
-            }
-        }
-
-        if (toTop != null) {
-            Collections.reverse(toTop); // the last card in list will become topmost in library, have to revert thus.
-            for(Card c : toTop) {
-                getGame().getAction().moveToLibrary(c, cause, null);
-                numToTop++;
-            }
-        }
-
-        getGame().fireEvent(new GameEventScry(this, numToTop, numToBottom));
-
-        final Map<String, Object> runParams = Maps.newHashMap();
-        runParams.put("Player", this);
-        getGame().getTriggerHandler().runTrigger(TriggerType.Scry, runParams, false);
-    }
-
     public void surveil(int num, SpellAbility cause) {
 
         final Map<String, Object> repParams = Maps.newHashMap();

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

@@ -13,8 +13,8 @@ public enum PlayerActionConfirmMode {
     ChangeZoneGeneral,
     BidLife,
     OptionalChoose,
-    Tribute;
+    Tribute,
     // Ripple;
+    ;
     
-    
-}
+}

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

@@ -260,4 +260,6 @@ public abstract class PlayerController {
     }
 
     public abstract List<OptionalCostValue> chooseOptionalCosts(SpellAbility choosen, List<OptionalCostValue> optionalCostValues);
+
+    public abstract boolean confirmMulliganScry(final Player p);
 }

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

@@ -678,4 +678,10 @@ public class PlayerControllerForTests extends PlayerController {
         return null;
     }
 
+    @Override
+    public boolean confirmMulliganScry(Player p) {
+        // TODO Auto-generated method stub
+        return false;
+    }
+
 }

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

@@ -741,11 +741,11 @@ public class PlayerControllerHuman extends PlayerController implements IGameCont
 	List<Card> result = getGui().manipulateCardList("Move cards to top or bottom of library", cards, manipulable, topOK, bottomOK, false);
         CardCollection toBottom = new CardCollection();
         CardCollection toTop = new CardCollection();
-	for (int i = 0; manipulable.contains(result.get(i)) && i<cards.size(); i++ ) {
+	for (int i = 0; i<cards.size() && manipulable.contains(result.get(i)) ; i++ ) {
 	    toTop.add(result.get(i));
 	}
 	if (toTop.size() < cards.size()) { // the top isn't everything
-	    for (int i = result.size()-1; manipulable.contains(result.get(i)); i-- ) {	
+	    for (int i = result.size()-1; i>=0 && manipulable.contains(result.get(i)); i-- ) {	
 		toBottom.add(result.get(i));
 	    }
 	}
@@ -2889,4 +2889,9 @@ public class PlayerControllerHuman extends PlayerController implements IGameCont
                 optionalCost, choosen.getHostCard().getView());
     }
 
+    @Override
+    public boolean confirmMulliganScry(Player p) {
+        return InputConfirm.confirm(this, (SpellAbility)null, "Do you want to scry?");
+    }
+
 }