Merge pull request #2000 from tool4ever/fixes12

Fix Bishop of Binding

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

@@ -96,7 +96,7 @@ public class AiAttackController {
 
     public AiAttackController(final Player ai, boolean nextTurn) {
         this.ai = ai;
-        defendingOpponent = choosePreferredDefenderPlayer(ai);
+        defendingOpponent = choosePreferredDefenderPlayer(ai, true);
         myList = ai.getCreaturesInPlay();
         this.nextTurn = nextTurn;
         refreshCombatants(defendingOpponent);
@@ -104,7 +104,7 @@ public class AiAttackController {
 
     public AiAttackController(final Player ai, Card attacker) {
         this.ai = ai;
-        defendingOpponent = choosePreferredDefenderPlayer(ai);
+        defendingOpponent = choosePreferredDefenderPlayer(ai, true);
         this.oppList = getOpponentCreatures(defendingOpponent);
         myList = ai.getCreaturesInPlay();
         this.nextTurn = false;
@@ -167,6 +167,9 @@ public class AiAttackController {
      * No strategy to secure a second place instead, since Forge has no variant for that
      */
     public static Player choosePreferredDefenderPlayer(Player ai) {
+        return choosePreferredDefenderPlayer(ai, false);
+    }
+    public static Player choosePreferredDefenderPlayer(Player ai, boolean forCombatDmg) {
         Player defender = ai.getWeakestOpponent(); //Concentrate on opponent within easy kill range
 
         // TODO for multiplayer combat avoid players with cantLose or (if not playing infect) cantLoseForZeroOrLessLife and !canLoseLife
@@ -174,10 +177,24 @@ public class AiAttackController {
         if (defender.getLife() > 8) {
             // TODO connect with evaluateBoardPosition and only fall back to random when no player is the biggest threat by a fair margin
 
+            List<Player> opps = Lists.newArrayList(ai.getOpponents());
+            if (forCombatDmg) {
+                for (Player p : opps) {
+                    if (p.isMonarch() && ai.canBecomeMonarch()) {
+                        // just increase the odds for now instead of being fully predictable
+                        // as it could lead to other too complex factors giving this reasoning negative impact
+                        opps.add(p);
+                    }
+                    if (p.hasInitiative()) {
+                        opps.add(p);
+                    }
+                }
+            }
+
             // TODO should we cache the random for each turn? some functions like shouldPumpCard base their decisions on the assumption who will be attacked
 
             //Otherwise choose a random opponent to ensure no ganging up on players
-            return Aggregates.random(ai.getOpponents());
+            return Aggregates.random(opps);
         }
         return defender;
     }

forge-ai/src/main/java/forge/ai/ability/AmassAi.java

@@ -29,7 +29,7 @@ public class AmassAi extends SpellAbilityAi {
         final Game game = ai.getGame();
 
         if (!aiArmies.isEmpty()) {
-            return CardLists.count(aiArmies, CardPredicates.canReceiveCounters(CounterEnumType.P1P1)) > 0;
+            return Iterables.any(aiArmies, CardPredicates.canReceiveCounters(CounterEnumType.P1P1));
         }
         final String tokenScript = "b_0_0_zombie_army";
         final int amount = AbilityUtils.calculateAmount(host, sa.getParamOrDefault("Num", "1"), sa);

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

@@ -932,7 +932,9 @@ public final class GameActionUtil {
             // add back to where it came from, hopefully old state
             // skip GameAction
             oldCard.getZone().remove(oldCard);
-            fromZone.add(oldCard, zonePosition >= 0 ? Integer.valueOf(zonePosition) : null);
+            // in some rare cases the old position no longer exists (Panglacial Wurm + Selvala)
+            Integer newPosition = zonePosition >= 0 ? Math.min(Integer.valueOf(zonePosition), fromZone.size()) : null;
+            fromZone.add(oldCard, newPosition);
             ability.setHostCard(oldCard);
             ability.setXManaCostPaid(null);
             ability.setSpendPhyrexianMana(false);

forge-game/src/main/java/forge/game/ability/AbilityFactory.java

@@ -110,7 +110,7 @@ public final class AbilityFactory {
     }
 
     public static final SpellAbility getAbility(final String abString, final Card card) {
-        return getAbility(abString, card, card.getCurrentState());
+        return getAbility(abString, card.getCurrentState());
     }
     public static final SpellAbility getAbility(final String abString, final Card card, final IHasSVars sVarHolder) {
         return getAbility(abString, card.getCurrentState(), sVarHolder);

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

@@ -4,6 +4,7 @@ import java.util.Map;
 
 import org.apache.commons.lang3.mutable.MutableBoolean;
 
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
 
 import forge.game.Game;
@@ -53,7 +54,7 @@ public class AmassEffect extends TokenEffectBase {
         final boolean remember = sa.hasParam("RememberAmass");
 
         // create army token if needed
-        if (CardLists.count(activator.getCardsIn(ZoneType.Battlefield), CardPredicates.isType("Army")) == 0) {
+        if (!Iterables.any(activator.getCardsIn(ZoneType.Battlefield), CardPredicates.isType("Army"))) {
             CardZoneTable triggerList = new CardZoneTable();
             MutableBoolean combatChanged = new MutableBoolean(false);
 

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

@@ -61,7 +61,7 @@ public class BalanceEffect extends SpellAbilityEffect {
             if (zone.equals(ZoneType.Hand)) {
                 discardedMap.put(p, p.getController().chooseCardsToDiscardFrom(p, sa, validCards.get(i), numToBalance, numToBalance));
             } else { // Battlefield
-                for (Card card : p.getController().choosePermanentsToSacrifice(sa, numToBalance, numToBalance,  validCards.get(i), valid)) {
+                for (Card card : p.getController().choosePermanentsToSacrifice(sa, numToBalance, numToBalance, validCards.get(i), valid)) {
                     if ( null == card ) continue; 
                     game.getAction().sacrifice(card, sa, true, table, params);
                 }

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

@@ -219,7 +219,6 @@ public class ChooseCardEffect extends SpellAbilityEffect {
                 notTgtPlayerCtrl.removeAll(tgtPlayerCtrl);
                 chosen.addAll(p.getController().chooseCardsForEffect(notTgtPlayerCtrl, sa, title + " " + "you don't control", minAmount, validAmount,
                         !sa.hasParam("Mandatory"), null));
-
             } else if (sa.hasParam("AtRandom") && !choices.isEmpty()) {
                 // don't pass FCollection for direct modification, the Set part would get messed up
                 chosen = new CardCollection(Aggregates.random(choices, validAmount));

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

@@ -8,7 +8,6 @@ import com.google.common.collect.Lists;
 import forge.game.ability.AbilityUtils;
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
-import forge.game.card.CardUtil;
 import forge.game.cost.Cost;
 import forge.game.event.GameEventCardModeChosen;
 import forge.game.player.Player;
@@ -66,24 +65,24 @@ public class ChooseGenericEffect extends SpellAbilityEffect {
             List<SpellAbility> chosenSAs = Lists.newArrayList();
             String prompt = sa.getParamOrDefault("ChoicePrompt", "Choose");
             boolean random = false;
+
             if (sa.hasParam("AtRandom")) {
                 random = true;
-                Aggregates.random(abilities, amount, chosenSAs);
+                chosenSAs = Aggregates.random(abilities, amount);
-            } else if (!abilities.isEmpty()) {
-                chosenSAs = p.getController().chooseSpellAbilitiesForEffect(abilities, sa, prompt, amount, ImmutableMap.of());
-            }
 
-            for (SpellAbility chosenSA : chosenSAs) {
+                int i = 0;
-                if (random && sa.getParam("AtRandom").equals("Urza") && chosenSA.usesTargeting()) {
+                while (sa.getParam("AtRandom").equals("Urza") && i < chosenSAs.size()) {
-                    List<Card> validTargets = CardUtil.getValidCardsToTarget(chosenSA.getTargetRestrictions(), sa);
+                    if (!chosenSAs.get(i).usesTargeting()) {
-                    if (validTargets.isEmpty()) {
+                        i++;
-                        List <SpellAbility> newChosenSAs = Lists.newArrayList();
+                    } else if (sa.getTargetRestrictions().hasCandidates(chosenSAs.get(i))) {
-                        Aggregates.random(abilities, amount, newChosenSAs);
+                        p.getController().chooseTargetsFor(chosenSAs.get(i));
-                        chosenSAs = newChosenSAs;
+                        i++;
                     } else {
-                        p.getController().chooseTargetsFor(chosenSA);
+                        chosenSAs.set(i, Aggregates.random(abilities));
                     }
                 }
+            } else if (!abilities.isEmpty()) {
+                chosenSAs = p.getController().chooseSpellAbilitiesForEffect(abilities, sa, prompt, amount, ImmutableMap.of());
             }
 
             if (!chosenSAs.isEmpty()) {

forge-gui/res/cardsfolder/a/archghoul_of_thraben.txt

@@ -4,7 +4,7 @@ Types:Creature Zombie Cleric
 PT:3/2
 T:Mode$ ChangesZone | Origin$ Battlefield | Destination$ Graveyard | ValidCard$ Card.Self,Zombie.Other+YouCtrl | Execute$ TrigPeek | TriggerDescription$ Whenever CARDNAME or another Zombie you control dies, look at the top card of your library. If it's a Zombie card, you may reveal it and put it into your hand. If you don't put the card into your hand, you may put it into your graveyard.
 SVar:TrigPeek:DB$ PeekAndReveal | PeekAmount$ 1 | RevealValid$ Zombie | RevealOptional$ True | RememberRevealed$ True | SubAbility$ DBToHand
-SVar:DBToHand:DB$ ChangeZone | Defined$ TopOfLibrary | Origin$ Library | Destination$ Hand | ConditionDefined$ Remembered | ConditionPresent$ Card | ConditionCompare$ EQ1 | SubAbility$ DBToGrave
+SVar:DBToHand:DB$ ChangeZone | Defined$ Remembered | Origin$ Library | Destination$ Hand | SubAbility$ DBToGrave
 SVar:DBToGrave:DB$ ChangeZone | Defined$ TopOfLibrary | Origin$ Library | Destination$ Graveyard | Optional$ True | ConditionDefined$ Remembered | ConditionPresent$ Card | ConditionCompare$ EQ0 | SubAbility$ DBCleanup
 SVar:DBCleanup:DB$ Cleanup | ClearRemembered$ True
 DeckHints:Type$Zombie

forge-gui/res/cardsfolder/b/bishop_of_binding.txt

@@ -3,11 +3,11 @@ ManaCost:3 W
 Types:Creature Vampire Cleric
 PT:1/1
 T:Mode$ ChangesZone | Origin$ Any | Destination$ Battlefield | ValidCard$ Card.Self | Execute$ TrigExile | TriggerDescription$ When CARDNAME enters the battlefield, exile target creature an opponent controls until CARDNAME leaves the battlefield.
-SVar:TrigExile:DB$ ChangeZone | Origin$ Battlefield | Destination$ Exile | ValidTgts$ Creature.OppCtrl | TgtPrompt$ Select target creature an opponent controls | ConditionPresent$ Card.Self | Duration$ UntilHostLeavesPlay | RememberChanged$ True
+SVar:TrigExile:DB$ ChangeZone | Origin$ Battlefield | Destination$ Exile | ValidTgts$ Creature.OppCtrl | TgtPrompt$ Select target creature an opponent controls | Duration$ UntilHostLeavesPlay | RememberChanged$ True
 T:Mode$ Attacks | ValidCard$ Card.Self | Execute$ TrigPump | TriggerDescription$ Whenever CARDNAME attacks, target Vampire gets +X/+X until end of turn, where X is the power of the exiled card.
-SVar:TrigPump:DB$ Pump | ValidTgts$ Permanent.Vampire | TgtPrompt$ Select target Vampire | NumAtt$ X | NumDef$ X
+SVar:TrigPump:DB$ Pump | ValidTgts$ Vampire | TgtPrompt$ Select target Vampire | NumAtt$ X | NumDef$ X
-SVar:X:Remembered$CardPower
+SVar:X:Count$ValidExile Card.IsRemembered+ExiledWithSource$CardPower
-// Release notes indicate that this effect should work with Vehicle cards.
+# Release notes indicate that this effect should work with Vehicle cards.
 T:Mode$ ChangesZone | Origin$ Battlefield | Destination$ Any | Static$ True | ValidCard$ Card.Self | Execute$ DBCleanup
 SVar:DBCleanup:DB$ Cleanup | ClearRemembered$ True
 SVar:PlayMain1:TRUE

forge-gui/res/cardsfolder/upcoming/sarinth_steelseeker.txt

@@ -3,10 +3,10 @@ ManaCost:1 G
 Types:Creature Human Artificer Scout
 PT:1/2
 T:Mode$ ChangesZone | Origin$ Any | Destination$ Battlefield | ValidCard$ Artifact.YouCtrl | TriggerZones$ Battlefield | Execute$ TrigDig | TriggerDescription$ Whenever an artifact enters the battlefield under your control, look at the top card of your library. If it's a land card, you may reveal it and put it into your hand. If you don't put the card into your hand, you may put it into your graveyard.
-SVar:TrigDig:DB$ PeekAndReveal | PeekAmount$ 1 | RevealValid$ Land | RevealOptional$ True | RememberPeeked$ True | ImprintRevealed$ True | SubAbility$ DBChangeZone
+SVar:TrigDig:DB$ PeekAndReveal | PeekAmount$ 1 | RevealValid$ Land | RevealOptional$ True | RememberRevealed$ True | SubAbility$ DBToHand
-SVar:DBChangeZone:DB$ ChangeZone | Defined$ Imprinted | Optional$ True | ForgetChanged$ True | Origin$ Library | Destination$ Hand | SubAbility$ DBGrave
+SVar:DBToHand:DB$ ChangeZone | Defined$ Remembered | Origin$ Library | Destination$ Hand | SubAbility$ DBToGrave
-SVar:DBGrave:DB$ ChangeZone | Defined$ Remembered | Origin$ Library | Optional$ True | Destination$ Graveyard | SubAbility$ DBCleanup
+SVar:DBToGrave:DB$ ChangeZone | Defined$ TopOfLibrary | Origin$ Library | Destination$ Graveyard | Optional$ True | ConditionDefined$ Remembered | ConditionPresent$ Card | ConditionCompare$ EQ0 | SubAbility$ DBCleanup
-SVar:DBCleanup:DB$ Cleanup | ClearRemembered$ True | ClearImprinted$ True
+SVar:DBCleanup:DB$ Cleanup | ClearRemembered$ True
 DeckHas:Ability$Graveyard
 DeckHints:Type$Artifact
 Oracle:Whenever an artifact enters the battlefield under your control, look at the top card of your library. If it's a land card, you may reveal it and put it into your hand. If you don't put the card into your hand, you may put it into your graveyard.

forge-gui/res/cardsfolder/upcoming/the_temporal_anchor.txt

@@ -3,7 +3,7 @@ ManaCost:3 U U U
 Types:Legendary Artifact
 T:Mode$ Phase | Phase$ Upkeep | ValidPlayer$ You | TriggerZones$ Battlefield | Execute$ TrigScry | TriggerDescription$ At the beginning of your upkeep, scry 2.
 SVar:TrigScry:DB$ Scry | ScryNum$ 2
-T:Mode$ Scry | ValidPlayer$ You | ToBottom$ True | Execute$ TrigExile | TriggerDescription$ Whenever you choose to put one or more cards on the bottom of your library while scrying, exile that many cards from the bottom of your library.
+T:Mode$ Scry | ValidPlayer$ You | ToBottom$ True | TriggerZones$ Battlefield | Execute$ TrigExile | TriggerDescription$ Whenever you choose to put one or more cards on the bottom of your library while scrying, exile that many cards from the bottom of your library.
 SVar:TrigExile:DB$ Dig | DigNum$ X | ChangeNum$ All | FromBottom$ True | DestinationZone$ Exile | RememberChanged$ True
 SVar:X:TriggerCount$ScryBottom
 S:Mode$ Continuous | Condition$ PlayerTurn | MayPlay$ True | Affected$ Card.ExiledWithSource+IsRemembered | AffectedZone$ Exile | Description$ During your turn, you may play cards exiled with CARDNAME.

forge-gui/src/main/java/forge/gamemodes/puzzle/Puzzle.java

@@ -103,7 +103,7 @@ public class Puzzle extends GameState implements InventoryItem, Comparable<Puzzl
     }
 
     public void setupMaxPlayerHandSize(Game game, int maxHandSize) {
-        for(Player p : game.getPlayers()) {
+        for (Player p : game.getPlayers()) {
             p.setStartingHandSize(maxHandSize);
             p.setMaxHandSize(maxHandSize);
         }
@@ -111,7 +111,7 @@ public class Puzzle extends GameState implements InventoryItem, Comparable<Puzzl
 
     public void addGoalEnforcement(Game game) {
         Player human = null;
-        for(Player p : game.getPlayers()) {
+        for (Player p : game.getPlayers()) {
             if (p.getController().isGuiPlayer()) {
                 human = p;
             }