Merge branch 'attach' into 'master'

ChangeZoneAi: Fix NPE for attaching cards

See merge request core-developers/forge!5557

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

@@ -699,9 +699,9 @@ public class AiAttackController {
 
         final boolean bAssault = doAssault(ai);
         // TODO: detect Lightmine Field by presence of a card with a specific trigger
-        final boolean lightmineField = ComputerUtilCard.isPresentOnBattlefield(ai.getGame(), "Lightmine Field");
+        final boolean lightmineField = ai.getGame().isCardInPlay("Lightmine Field");
         // TODO: detect Season of the Witch by presence of a card with a specific trigger
-        final boolean seasonOfTheWitch = ComputerUtilCard.isPresentOnBattlefield(ai.getGame(), "Season of the Witch");
+        final boolean seasonOfTheWitch = ai.getGame().isCardInPlay("Season of the Witch");
 
         // Determine who will be attacked
         GameEntity defender = chooseDefender(combat, bAssault);

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

@@ -1834,10 +1834,6 @@ public class ComputerUtilCard {
         return false;
     }
 
-    public static boolean isPresentOnBattlefield(final Game game, final String cardName) {
-        return Iterables.any(game.getCardsIn(ZoneType.Battlefield), CardPredicates.nameEquals(cardName));
-    }
-
     public static int getMaxSAEnergyCostOnBattlefield(final Player ai) {
         // returns the maximum energy cost of an ability that permanents on the battlefield under AI's control have
         CardCollectionView otb = ai.getCardsIn(ZoneType.Battlefield);

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

@@ -459,8 +459,7 @@ public class AttachAi extends SpellAbilityAi {
      *            the mandatory
      * @return the player
      */
-    private static Player attachToPlayerAIPreferences(final Player aiPlayer, final SpellAbility sa,
+    public static Player attachToPlayerAIPreferences(final Player aiPlayer, final SpellAbility sa, final boolean mandatory) {
-            final boolean mandatory) {
         List<Player> targetable = new ArrayList<>();
         for (final Player player : aiPlayer.getGame().getPlayers()) {
             if (sa.canTarget(player)) {

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

@@ -54,7 +54,6 @@ import forge.game.spellability.SpellAbility;
 import forge.game.spellability.TargetRestrictions;
 import forge.game.staticability.StaticAbilityMustTarget;
 import forge.game.zone.ZoneType;
-import forge.util.Aggregates;
 import forge.util.MyRandom;
 
 public class ChangeZoneAi extends SpellAbilityAi {
@@ -1733,7 +1732,7 @@ public class ChangeZoneAi extends SpellAbilityAi {
     @Override
     public Player chooseSinglePlayer(Player ai, SpellAbility sa, Iterable<Player> options, Map<String, Object> params) {
         // Called when attaching Aura to player
-        return Aggregates.random(options);
+        return AttachAi.attachToPlayerAIPreferences(ai, sa, true);
     }
 
     private boolean doSacAndReturnFromGraveLogic(final Player ai, final SpellAbility sa) {

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

@@ -90,12 +90,12 @@ public class CharmAi extends SpellAbilityAi {
             if (AiPlayDecision.WillPlay == aic.canPlaySa(sub)) {
                 chosenList.add(sub);
                 if (chosenList.size() == num) {
-                    return chosenList;  // maximum choices reached
+                    return chosenList; // maximum choices reached
                 }
             }
         }
         if (isTrigger && chosenList.size() < min) {
-            // Second pass using doTrigger(false) to fulfil minimum choice
+            // Second pass using doTrigger(false) to fulfill minimum choice
             choices.removeAll(chosenList);
             for (AbilitySub sub : choices) {
                 sub.setActivatingPlayer(ai);

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

@@ -583,21 +583,11 @@ public class Game {
     }
 
     public boolean isCardInPlay(final String cardName) {
-        for (final Player p : getPlayers()) {
+        return Iterables.any(getCardsIn(ZoneType.Battlefield), CardPredicates.nameEquals(cardName));
-            if (p.isCardInPlay(cardName)) {
-                return true;
-            }
-        }
-        return false;
     }
 
     public boolean isCardInCommand(final String cardName) {
-        for (final Player p : getPlayers()) {
+        return Iterables.any(getCardsIn(ZoneType.Command), CardPredicates.nameEquals(cardName));
-            if (p.isCardInCommand(cardName)) {
-                return true;
-            }
-        }
-        return false;
     }
 
     public CardCollectionView getColoredCardsInPlay(final String color) {

forge-game/src/main/java/forge/game/card/CardFactoryUtil.java

@@ -958,6 +958,7 @@ public class CardFactoryUtil {
             final Trigger conspireTrigger = TriggerHandler.parseTrigger(trigScript, card, intrinsic);
             conspireTrigger.setOverridingAbility(AbilityFactory.getAbility(abString, card));
             conspireTrigger.setSVar("Conspire", "0");
+
             inst.addTrigger(conspireTrigger);
         } else if (keyword.startsWith("Cumulative upkeep")) {
             final String[] k = keyword.split(":");
@@ -995,7 +996,6 @@ public class CardFactoryUtil {
             trigger.setOverridingAbility(AbilityFactory.getAbility(transformEff, card));
 
             inst.addTrigger(trigger);
-
         } else if (keyword.equals("Decayed")) {
             final String attackTrig = "Mode$ Attacks | ValidCard$ Card.Self | Secondary$ True | TriggerDescription$ " +
                     "When a creature with decayed attacks, sacrifice it at end of combat.";
@@ -1542,7 +1542,6 @@ public class CardFactoryUtil {
             trigger.setOverridingAbility(AbilityFactory.getAbility(transformEff, card));
 
             inst.addTrigger(trigger);
-
         } else if (keyword.startsWith("Partner:")) {
             // Partner With
             final String[] k = keyword.split(":");
@@ -1580,6 +1579,7 @@ public class CardFactoryUtil {
 
             final String effect = "DB$ Poison | Defined$ TriggeredTarget | Num$ " + n;
             parsedTrigger.setOverridingAbility(AbilityFactory.getAbility(effect, card));
+
             inst.addTrigger(parsedTrigger);
         } else if (keyword.startsWith("Presence")) {
             final String[] k = keyword.split(":");
@@ -2530,7 +2530,6 @@ public class CardFactoryUtil {
             ReplacementEffect re = ReplacementHandler.parseReplacement(rep, host, intrinsic, card);
             inst.addReplacement(re);
         }
-
         else if (keyword.startsWith("If CARDNAME would be put into a graveyard "
                 + "from anywhere, reveal CARDNAME and shuffle it into its owner's library instead.")) {
 
@@ -2606,7 +2605,6 @@ public class CardFactoryUtil {
                 newSA.setIntrinsic(intrinsic);
 
                 inst.addSpellAbility(newSA);
-
             }
         } else if (keyword.startsWith("Adapt")) {
             final String[] k = keyword.split(":");
@@ -2914,7 +2912,6 @@ public class CardFactoryUtil {
             foretell.getRestrictions().setZone(ZoneType.Hand);
             foretell.setIntrinsic(intrinsic);
             inst.addSpellAbility(foretell);
-
         } else if (keyword.startsWith("Fortify")) {
             String[] k = keyword.split(":");
             // Get cost string
@@ -3091,7 +3088,6 @@ public class CardFactoryUtil {
             sa.setIntrinsic(intrinsic);
             sa.setAlternativeCost(AlternativeCost.Outlast);
             inst.addSpellAbility(sa);
-
         } else if (keyword.startsWith("Prowl")) {
             final String[] k = keyword.split(":");
             final Cost prowlCost = new Cost(k[1], false);
@@ -3148,7 +3144,6 @@ public class CardFactoryUtil {
             sa.setSVar("ScavengeX", "Exiled$CardPower");
             sa.setIntrinsic(intrinsic);
             inst.addSpellAbility(sa);
-
         } else if (keyword.startsWith("Encore")) {
             final String[] k = keyword.split(":");
             final String manacost = k[1];
@@ -3194,7 +3189,6 @@ public class CardFactoryUtil {
 
             AbilitySub cleanupSA = (AbilitySub) AbilityFactory.getAbility(cleanupStr, card);
             delTrigSA.setSubAbility(cleanupSA);
-
         } else if (keyword.startsWith("Spectacle")) {
             final String[] k = keyword.split(":");
             final Cost cost = new Cost(k[1], false);
@@ -3208,7 +3202,6 @@ public class CardFactoryUtil {
 
             newSA.setIntrinsic(intrinsic);
             inst.addSpellAbility(newSA);
-
         } else if (keyword.startsWith("Surge")) {
             final String[] k = keyword.split(":");
             final Cost surgeCost = new Cost(k[1], false);
@@ -3222,7 +3215,6 @@ public class CardFactoryUtil {
 
             newSA.setIntrinsic(intrinsic);
             inst.addSpellAbility(newSA);
-
         } else if (keyword.startsWith("Suspend") && !keyword.equals("Suspend")) {
             // only add it if suspend has counter and cost
             final String[] k = keyword.split(":");
@@ -3320,7 +3312,6 @@ public class CardFactoryUtil {
             final SpellAbility sa = AbilityFactory.getAbility(effect, card);
             sa.setIntrinsic(intrinsic);
             inst.addSpellAbility(sa);
-
         } else if (keyword.endsWith(" offering")) {
             final String offeringType = keyword.split(" ")[0];
             final SpellAbility sa = card.getFirstSpellAbility();
@@ -3337,7 +3328,6 @@ public class CardFactoryUtil {
             newSA.setDescription(sa.getDescription() + " (" + offeringType + " offering)");
             newSA.setIntrinsic(intrinsic);
             inst.addSpellAbility(newSA);
-
         } else if (keyword.startsWith("Crew")) {
             final String[] k = keyword.split(":");
             final String power = k[1];
@@ -3352,7 +3342,6 @@ public class CardFactoryUtil {
             final SpellAbility sa = AbilityFactory.getAbility(effect, card);
             sa.setIntrinsic(intrinsic);
             inst.addSpellAbility(sa);
-
         } else if (keyword.startsWith("Cycling")) {
             final String[] k = keyword.split(":");
             final String manacost = k[1];
@@ -3370,7 +3359,6 @@ public class CardFactoryUtil {
             sa.setAlternativeCost(AlternativeCost.Cycling);
             sa.setIntrinsic(intrinsic);
             inst.addSpellAbility(sa);
-
         } else if (keyword.startsWith("TypeCycling")) {
             final String[] k = keyword.split(":");
             final String type = k[1];
@@ -3396,7 +3384,6 @@ public class CardFactoryUtil {
             sa.setAlternativeCost(AlternativeCost.Cycling);
             sa.setIntrinsic(intrinsic);
             inst.addSpellAbility(sa);
-
         }
     }
 

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

@@ -5,7 +5,7 @@ PT:4/2
 S:Mode$ ReduceCost | ValidTarget$ Card.Self | Activator$ Player.Opponent | Type$ Spell | Amount$ 1 | Description$ Spells your opponents cast that target CARDNAME cost {1} less to cast.
 T:Mode$ ChangesZone | Origin$ Battlefield | Destination$ Graveyard | ValidCard$ Card.Self | Execute$ TrigChoose | TriggerController$ TriggeredCardController | TriggerDescription$ When CARDNAME dies, return it to the battlefield transformed under your control attached to target opponent.
 SVar:TrigChoose:DB$ Pump | ValidTgts$ Opponent | TgtPrompt$ Choose a opponent | IsCurse$ True | SubAbility$ DBChange
-SVar:DBChange:DB$ ChangeZone | Defined$ TriggeredNewCardLKICopy | Origin$ Graveyard | Destination$ Battlefield | AttachedToPlayer$ ParentTarget | Transformed$ True | GainControl$ True
+SVar:DBChange:DB$ ChangeZone | Defined$ TriggeredNewCardLKICopy | Origin$ Graveyard | Destination$ Battlefield | AttachedToPlayer$ ParentTarget | Transformed$ True | GainControl$ True | AILogic$ Curse
 SVar:SacMe:4
 SVar:MustAttack:True
 AlternateMode:DoubleFaced
@@ -18,7 +18,7 @@ ManaCost:no cost
 Colors:black
 Types:Enchantment Aura Curse
 K:Enchant player
-A:SP$ Attach | Cost$ 0 | ValidTgts$ Player | AILogic$ Curse
+A:SP$ Attach | Cost$ 0 | ValidTgts$ Player
 S:Mode$ ReduceCost | ValidTarget$ Player.EnchantedBy | Activator$ You | Type$ Spell | Amount$ 1 | Description$ Spells you cast that target enchanted player cost {1} less to cast.
 T:Mode$ Phase | Phase$ Upkeep | ValidPlayer$ Player.EnchantedBy | TriggerZones$ Battlefield | Execute$ TrigDrain | TriggerDescription$ At the beginning of enchanted player's upkeep, that player loses 1 life and you gain 1 life.
 SVar:TrigDrain:DB$ LoseLife | Defined$ TriggeredPlayer | LifeAmount$ 1 | SubAbility$ DBGainLife

forge-gui/res/cardsfolder/c/curse_of_misfortunes.txt

@@ -4,6 +4,6 @@ Types:Enchantment Aura Curse
 K:Enchant player
 A:SP$ Attach | Cost$ 4 B | ValidTgts$ Player | AILogic$ Curse
 T:Mode$ Phase | Phase$ Upkeep | ValidPlayer$ You | TriggerZones$ Battlefield | OptionalDecider$ You | Execute$ TrigMisfortune | TriggerDescription$ At the beginning of your upkeep, you may search your library for a Curse card that doesn't have the same name as a Curse attached to enchanted player, put it onto the battlefield attached to that player, then shuffle you library.
-SVar:TrigMisfortune:DB$ ChangeZone | Origin$ Library | Destination$ Battlefield | ChangeType$ Aura.Curse+NameNotEnchantingEnchantedPlayer | ChangeNum$ 1 | AttachedToPlayer$ EnchantedPlayer | ShuffleNonMandatory$ True
+SVar:TrigMisfortune:DB$ ChangeZone | Origin$ Library | Destination$ Battlefield | ChangeType$ Aura.Curse+NameNotEnchantingEnchantedPlayer | ChangeNum$ 1 | AttachedToPlayer$ EnchantedPlayer | ShuffleNonMandatory$ True | AILogic$ Curse
 SVar:Picture:http://www.wizards.com/global/images/magic/general/curse_of_misfortunes.jpg
 Oracle:Enchant player\nAt the beginning of your upkeep, you may search your library for a Curse card that doesn't have the same name as a Curse attached to enchanted player, put it onto the battlefield attached to that player, then shuffle.

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

@@ -5,7 +5,7 @@ PT:2/1
 K:CARDNAME can't block.
 T:Mode$ ChangesZone | Origin$ Battlefield | Destination$ Graveyard | ValidCard$ Card.Self | Execute$ TrigChoose | TriggerDescription$ When CARDNAME dies, return it to the battlefield transformed under your control attached to target creature or planeswalker an opponent controls.
 SVar:TrigChoose:DB$ Pump | ValidTgts$ Creature.OppCtrl,Planeswalker.OppCtrl | TgtPrompt$ Select target creature or planeswalker an opponent controls | IsCurse$ True | SubAbility$ DBChange
-SVar:DBChange:DB$ ChangeZone | Defined$ TriggeredNewCardLKICopy | Origin$ Graveyard | Destination$ Battlefield | AttachedTo$ ParentTarget | Transformed$ True | GainControl$ True
+SVar:DBChange:DB$ ChangeZone | Defined$ TriggeredNewCardLKICopy | Origin$ Graveyard | Destination$ Battlefield | AttachedTo$ ParentTarget | Transformed$ True | GainControl$ True | AILogic$ Curse
 SVar:SacMe:1
 AlternateMode:DoubleFaced
 Oracle:Vengeful Strangler can't block.\nWhen Vengeful Strangler dies, return it to the battlefield transformed under your control attached to target creature or planeswalker an opponent controls.
@@ -17,7 +17,7 @@ ManaCost:no cost
 Colors:black
 Types:Enchantment Aura
 K:Enchant creature or planeswalker an opponent controls
-A:SP$ Attach | Cost$ 0 | ValidTgts$ Creature.OppCtrl,Planeswalker.OppCtrl | TgtPrompt$ Select target creature or planeswalker an opponent controls | AILogic$ Curse
+A:SP$ Attach | Cost$ 0 | ValidTgts$ Creature.OppCtrl,Planeswalker.OppCtrl | TgtPrompt$ Select target creature or planeswalker an opponent controls
 T:Mode$ Phase | Phase$ Upkeep | ValidPlayer$ You | TriggerZones$ Battlefield | Execute$ TrigSac | TriggerDescription$ At the beginning of your upkeep, enchanted permanent's controller sacrifices a nonland permanent and loses 1 life.
 SVar:TrigSac:DB$ Sacrifice | Defined$ Player.controlsPermanent.EnchantedBy | SacValid$ Permanent.nonLand | SubAbility$ DBLoseLife
 SVar:DBLoseLife:DB$ LoseLife | LifeAmount$ 1 | Defined$ Player.controlsPermanent.EnchantedBy