Fix crash (#4610)

* Fix crash

* Move check earlier so Full Control doesn't ask for non-matches

* Restore fix

* Clean up duration checks

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

@@ -313,6 +313,7 @@ public class PumpAi extends PumpAiBase {
                 attack = root.getXManaCostPaid();
             }
         } else {
+            // TODO add Double
             attack = AbilityUtils.calculateAmount(sa.getHostCard(), numAttack, sa);
             if (numAttack.contains("X") && sa.getSVar("X").equals("Count$CardsInYourHand") && source.isInZone(ZoneType.Hand)) {
                 attack--; // the card will be spent casting the spell, so actual power is 1 less

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

@@ -2733,7 +2733,7 @@ public class AbilityUtils {
 
         // Count$ThisTurnEntered <ZoneDestination> [from <ZoneOrigin>] <Valid>
         if (sq[0].startsWith("ThisTurnEntered") || sq[0].startsWith("LastTurnEntered")) {
-            final String[] workingCopy = paidparts[0].split("_");
+            final String[] workingCopy = paidparts[0].split("_", 5);
             ZoneType destination = ZoneType.smartValueOf(workingCopy[1]);
             final boolean hasFrom = workingCopy[2].equals("from");
             ZoneType origin = hasFrom ? ZoneType.smartValueOf(workingCopy[3]) : null;

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

@@ -930,6 +930,27 @@ public abstract class SpellAbilityEffect {
         }
     }
 
+    protected static boolean checkValidDuration(String duration, SpellAbility sa) {
+        if (duration == null) {
+            return true;
+        }
+        Card hostCard = sa.getHostCard();
+
+        //if host is not on the battlefield don't apply
+        // Suspend should does Affect the Stack
+        if ((duration.startsWith("UntilHostLeavesPlay") || "UntilLoseControlOfHost".equals(duration) || "UntilUntaps".equals(duration))
+                && !(hostCard.isInPlay() || hostCard.isInZone(ZoneType.Stack))) {
+            return false;
+        }
+        if ("UntilLoseControlOfHost".equals(duration) && hostCard.getController() != sa.getActivatingPlayer()) {
+            return false;
+        }
+        if ("UntilUntaps".equals(duration) && !hostCard.isTapped()) {
+            return false;
+        }
+        return true;
+    }
+
     public static Player getNewChooser(final SpellAbility sa, final Player activator, final Player loser) {
         // CR 800.4g
         final PlayerCollection options;

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

@@ -1,6 +1,5 @@
 package forge.game.ability.effects;
 
-import com.google.common.collect.Lists;
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
 import forge.game.card.CardCollection;
@@ -8,13 +7,12 @@ import forge.game.spellability.SpellAbility;
 import forge.util.Lang;
 import forge.util.TextUtil;
 
-import java.util.ArrayList;
 
 public class AlterAttributeEffect extends SpellAbilityEffect {
     @Override
     public void resolve(SpellAbility sa) {
         boolean activate = Boolean.valueOf(sa.getParamOrDefault("Activate", "true"));
-        ArrayList<String> attributes = Lists.newArrayList(sa.getParam("Attributes").split(","));
+        String[] attributes = sa.getParam("Attributes").split(",");
         CardCollection defined = getDefinedCardsOrTargeted(sa, "Defined");
 
         if (sa.hasParam("Optional")) {
@@ -28,9 +26,9 @@ public class AlterAttributeEffect extends SpellAbilityEffect {
             }
         }
 
-        for(Card c : defined) {
+        for (Card c : defined) {
-            for(String attr : attributes) {
+            for (String attr : attributes) {
-                switch(attr.trim()) {
+                switch (attr.trim()) {
                     case "Solve":
                     case "Solved":
                         c.setSolved(activate);

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

@@ -31,12 +31,7 @@ public class AnimateEffect extends AnimateEffectBase {
         String animateRemembered = null;
         String animateImprinted = null;
 
-        //if host is not on the battlefield don't apply
+        if (!checkValidDuration(duration, sa)) {
-        if (("UntilHostLeavesPlay".equals(duration) || "UntilLoseControlOfHost".equals(duration))
-                && !source.isInPlay()) {
-            return;
-        }
-        if ("UntilLoseControlOfHost".equals(duration) && source.getController() != sa.getActivatingPlayer()) {
             return;
         }
 

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

@@ -45,8 +45,7 @@ public class ChangeZoneAllEffect extends SpellAbilityEffect {
     public void resolve(SpellAbility sa) {
         final Card source = sa.getHostCard();
 
-        //if host is not on the battlefield don't apply
+        if (!checkValidDuration(sa.getParam("Duration"), sa)) {
-        if ("UntilHostLeavesPlay".equals(sa.getParam("Duration")) && !source.isInPlay()) {
             return;
         }
 

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

@@ -428,8 +428,7 @@ public class ChangeZoneEffect extends SpellAbilityEffect {
 
     @Override
     public void resolve(SpellAbility sa) {
-        //if host is not on the battlefield don't apply
+        if (!checkValidDuration(sa.getParam("Duration"), sa)) {
-        if ("UntilHostLeavesPlay".equals(sa.getParam("Duration")) && !sa.getHostCard().isInPlay()) {
             return;
         }
 

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

@@ -128,10 +128,9 @@ public class CloneEffect extends SpellAbilityEffect {
 
         for (Card tgtCard : cloneTargets) {
             game.getTriggerHandler().clearActiveTriggers(tgtCard, null);
-            if (sa.hasParam("CloneZone")) {
+            if (sa.hasParam("CloneZone") &&
-                if (!tgtCard.isInZone(ZoneType.smartValueOf(sa.getParam("CloneZone")))) {
+                    !tgtCard.isInZone(ZoneType.smartValueOf(sa.getParam("CloneZone")))) {
-                    continue;
+                continue;
-                }
             }
 
             if (tgtCard.isPhasedOut()) {

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

@@ -55,14 +55,7 @@ public class EffectEffect extends SpellAbilityEffect {
         String noteCounterDefined = null;
         final String duration = sa.getParam("Duration");
 
-        if (((duration != null && duration.startsWith("UntilHostLeavesPlay")) || "UntilLoseControlOfHost".equals(duration) || "UntilUntaps".equals(duration))
+        if (!checkValidDuration(duration, sa)) {
-                && !(hostCard.isInPlay() || hostCard.isInZone(ZoneType.Stack))) {
-            return;
-        }
-        if ("UntilLoseControlOfHost".equals(duration) && hostCard.getController() != sa.getActivatingPlayer()) {
-            return;
-        }
-        if ("UntilUntaps".equals(duration) && !hostCard.isTapped()) {
             return;
         }
 

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

@@ -128,6 +128,10 @@ public class PumpAllEffect extends SpellAbilityEffect {
         final List<ZoneType> affectedZones = Lists.newArrayList();
         final Game game = sa.getActivatingPlayer().getGame();
 
+        if (!checkValidDuration(sa.getParam("Duration"), sa)) {
+            return;
+        }
+
         if (sa.hasParam("PumpZone")) {
             affectedZones.addAll(ZoneType.listValueOf(sa.getParam("PumpZone")));
         } else {

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

@@ -44,16 +44,6 @@ public class PumpEffect extends SpellAbilityEffect {
         final String duration = sa.getParam("Duration");
         final boolean perpetual = ("Perpetual").equals(duration);
 
-        //if host is not on the battlefield don't apply
-        // Suspend should does Affect the Stack
-        if (((duration != null && duration.startsWith("UntilHostLeavesPlay")) || "UntilLoseControlOfHost".equals(duration))
-                && !(host.isInPlay() || host.isInZone(ZoneType.Stack))) {
-            return;
-        }
-        if ("UntilLoseControlOfHost".equals(duration) && host.getController() != sa.getActivatingPlayer()) {
-            return;
-        }
-
         // do Game Check there in case of LKI
         final Card gameCard = game.getCardState(applyTo, null);
         if (gameCard == null || !applyTo.equalsWithTimestamp(gameCard)) {
@@ -152,13 +142,6 @@ public class PumpEffect extends SpellAbilityEffect {
         final Card host = sa.getHostCard();
         final String duration = sa.getParam("Duration");
 
-        //if host is not on the battlefield don't apply
-        // Suspend should does Affect the Stack
-        if (((duration != null && duration.startsWith("UntilHostLeavesPlay")) || "UntilLoseControlOfHost".equals(duration))
-                && !(host.isInPlay() || host.isInZone(ZoneType.Stack))) {
-            return;
-        }
-
         if (!keywords.isEmpty()) {
             p.addChangedKeywords(keywords, ImmutableList.of(), timestamp, 0);
         }
@@ -294,6 +277,10 @@ public class PumpEffect extends SpellAbilityEffect {
 
     @Override
     public void resolve(final SpellAbility sa) {
+        if (!checkValidDuration(sa.getParam("Duration"), sa)) {
+            return;
+        }
+
         final Player activator = sa.getActivatingPlayer();
         final Game game = activator.getGame();
         final Card host = sa.getHostCard();

forge-game/src/main/java/forge/game/cost/CostAdjustment.java

@@ -200,7 +200,7 @@ public class CostAdjustment {
         // Sort abilities to apply them in proper order
         for (Card c : cardsOnBattlefield) {
             for (final StaticAbility stAb : c.getStaticAbilities()) {
-                if (stAb.checkMode("ReduceCost")) {
+                if (stAb.checkMode("ReduceCost") && checkRequirement(sa, stAb)) {
                     reduceAbilities.add(stAb);
                 }
                 else if (stAb.checkMode("SetCost")) {
@@ -401,10 +401,6 @@ public class CostAdjustment {
         final Card card = sa.getHostCard();
         final String amount = staticAbility.getParam("Amount");
 
-        if (!checkRequirement(sa, staticAbility)) {
-            return 0;
-        }
-
         int value;
         if ("AffectedX".equals(amount)) {
             value = AbilityUtils.calculateAmount(card, amount, staticAbility);

forge-game/src/main/java/forge/game/cost/CostPartMana.java

@@ -138,7 +138,7 @@ public class CostPartMana extends CostPart {
         if (isCostPayAnyNumberOfTimes) {
             int timesToPay = AbilityUtils.calculateAmount(sa.getHostCard(), sa.getSVar("NumTimes"), sa);
             if (timesToPay == 0) {
-                return null;
+                return ManaCost.NO_COST;
             }
             ManaCostBeingPaid totalMana = new ManaCostBeingPaid(getMana());
             for (int i = 1; i < timesToPay; i++) {

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

@@ -3,13 +3,11 @@ ManaCost:2 R R
 Types:Enchantment Aura
 K:Enchant creature you control
 A:SP$ Attach | Cost$ 2 R R | ValidTgts$ Creature.YouCtrl | TgtPrompt$ Select target creature you control to attach Breath of Fury to | AILogic$ Pump
-T:Mode$ DamageDone | ValidSource$ Card.AttachedBy | ValidTarget$ Player | CombatDamage$ True | Execute$ NewFuriousLeader | TriggerZones$ Battlefield | TriggerDescription$ When enchanted creature deals combat damage to a player, sacrifice it and attach CARDNAME to a creature you control. If you do, untap all creatures you control and after this phase, there is an additional combat phase.
+T:Mode$ DamageDone | ValidSource$ Card.AttachedBy | ValidTarget$ Player | CombatDamage$ True | Execute$ TrigSacrifice | TriggerZones$ Battlefield | TriggerDescription$ When enchanted creature deals combat damage to a player, sacrifice it and attach CARDNAME to a creature you control. If you do, untap all creatures you control and after this phase, there is an additional combat phase.
-SVar:NewFuriousLeader:DB$ ChooseCard | Defined$ You | Amount$ 1 | Choices$ Creature.NotEnchantedBy+YouCtrl+CanBeEnchantedBy | ChoiceTitle$ Choose another creature you control to attach Breath of Fury to | SubAbility$ TrigSacrifice
+SVar:TrigSacrifice:DB$ SacrificeAll | ValidCards$ Card.EnchantedBy | SubAbility$ StillFurious
-SVar:TrigSacrifice:DB$ SacrificeAll | ValidCards$ Card.EnchantedBy | SubAbility$ StillFurious | RememberSacrificed$ True
+SVar:StillFurious:DB$ Attach | Object$ Self | Choices$ Creature.YouCtrl | ChoiceTitle$ Choose a creature you control to attach Breath of Fury to | RememberAttached$ True | SubAbility$ CatchBreath
-SVar:StillFurious:DB$ Attach | Defined$ ChosenCard | ConditionCheckSVar$ WasSacced | ConditionSVarCompare$ EQ1 | SubAbility$ Cleanup
+SVar:CatchBreath:DB$ UntapAll | ValidCards$ Creature.YouCtrl | SubAbility$ TheFuryContinues | ConditionDefined$ Rememembered | ConditionPresent$ Card
-SVar:Cleanup:DB$ Cleanup | ClearRemembered$ True | SubAbility$ CatchBreath
+SVar:TheFuryContinues:DB$ AddPhase | ExtraPhase$ Combat | AfterPhase$ EndCombat | ConditionDefined$ Remembered | ConditionPresent$ Card | SubAbility$ Cleanup
-SVar:CatchBreath:DB$ UntapAll | ValidCards$ Creature.YouCtrl | SubAbility$ TheFuryContinues
+SVar:Cleanup:DB$ Cleanup | ClearRemembered$ True
-SVar:TheFuryContinues:DB$ AddPhase | ExtraPhase$ Combat | AfterPhase$ EndCombat
-SVar:WasSacced:Remembered$Amount
 AI:RemoveDeck:All
 Oracle:Enchant creature you control\nWhen enchanted creature deals combat damage to a player, sacrifice it and attach Breath of Fury to a creature you control. If you do, untap all creatures you control and after this phase, there is an additional combat phase.

forge-gui/res/cardsfolder/t/the_blackstaff_of_waterdeep.txt

@@ -2,8 +2,5 @@ Name:The Blackstaff of Waterdeep
 ManaCost:U
 Types:Legendary Artifact
 K:You may choose not to untap CARDNAME during your untap step.
-A:AB$ Pump | Cost$ 1 U T | ValidTgts$ Artifact.Other+nonToken+YouCtrl | TgtPrompt$ Select another target nontoken artifact you control | RememberTargets$ True | AILogic$ ContinuousBonus | PrecostDesc$ Animate Walking Statue — | SpellDescription$ Another target nontoken artifact you control becomes a 4/4 artifact creature for as long as CARDNAME remains tapped. Activate only as a sorcery. | SorcerySpeed$ True | StackDescription$ SpellDescription
+A:AB$ Animate | Cost$ 1 U T | ValidTgts$ Artifact.Other+nonToken+YouCtrl | TgtPrompt$ Select another target nontoken artifact you control | Duration$ UntilUntaps | Power$ 4 | Toughness$ 4 | Types$ Creature | PrecostDesc$ Animate Walking Statue — | SpellDescription$ Another target nontoken artifact you control becomes a 4/4 artifact creature for as long as CARDNAME remains tapped. Activate only as a sorcery. | SorcerySpeed$ True | StackDescription$ SpellDescription
-S:Mode$ Continuous | Affected$ Artifact.IsRemembered | SetPower$ 4 | SetToughness$ 4 | AddType$ Creature
-T:Mode$ Untaps | ValidCard$ Card.Self | TriggerZones$ Battlefield | Execute$ ClearRemembered | Static$ True
-SVar:ClearRemembered:DB$ Cleanup | ClearRemembered$ True
 Oracle:You may choose not to untap The Blackstaff of Waterdeep during your untap step.\nAnimate Walking Statue — {1}{U}, {T}: Another target nontoken artifact you control becomes a 4/4 artifact creature for as long as The Blackstaff of Waterdeep remains tapped. Activate only as a sorcery.

forge-gui/res/cardsfolder/t/thran_weaponry.txt

@@ -3,9 +3,6 @@ ManaCost:4
 Types:Artifact
 K:Echo:4
 K:You may choose not to untap CARDNAME during your untap step.
-A:AB$ PumpAll | Cost$ 2 T | ValidCards$ Creature | RememberPumped$ True | SpellDescription$ All creatures get +2/+2 for as long as CARDNAME remains tapped.
+A:AB$ PumpAll | Cost$ 2 T | ValidCards$ Creature | Duration$ UntilUntaps | NumAtt$ 2 | NumDef$ 2 | SpellDescription$ All creatures get +2/+2 for as long as CARDNAME remains tapped.
-S:Mode$ Continuous | Affected$ Creature.IsRemembered | AddPower$ 2 | AddToughness$ 2
-T:Mode$ Untaps | ValidCard$ Card.Self | TriggerZones$ Battlefield | Execute$ ClearRemembered | Static$ True
-SVar:ClearRemembered:DB$ Cleanup | ClearRemembered$ True
 AI:RemoveDeck:All
 Oracle:Echo {4} (At the beginning of your upkeep, if this came under your control since the beginning of your last upkeep, sacrifice it unless you pay its echo cost.)\nYou may choose not to untap Thran Weaponry during your untap step.\n{2}, {T}: All creatures get +2/+2 for as long as Thran Weaponry remains tapped.