Clean up train continues

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

@@ -1715,12 +1715,12 @@ public class AbilityUtils {
                                 && ZoneType.Battlefield.name().equals(t.getParam("Destination"))) {
                            return doXMath(c.getXManaCostPaid(), expr, c, ctb);
                         } else if (TriggerType.SpellCast.equals(t.getMode())) {
-                            // Cast Trigger like Hydroid Krasis, use SI because Unbound Flourishing might change X
+                            // Cast Trigger like Hydroid Krasis
                             SpellAbilityStackInstance castSI = (SpellAbilityStackInstance) root.getTriggeringObject(AbilityKey.StackInstance);
                             if (castSI == null) {
                                 return doXMath(0, expr, c, ctb);
                             }
-                            return doXMath(castSI.getXManaPaid(), expr, c, ctb);
+                            return doXMath(castSI.getSpellAbility(false).getXManaCostPaid(), expr, c, ctb);
                         } else if (TriggerType.Cycled.equals(t.getMode())) {
                             SpellAbility cycleSA = (SpellAbility) sa.getTriggeringObject(AbilityKey.Cause);
                             if (cycleSA == null || cycleSA.getXManaCostPaid() == null) {

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

@@ -3,10 +3,7 @@ package forge.game.ability.effects;
 import java.util.List;
 
 import forge.game.ability.SpellAbilityEffect;
-import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.spellability.SpellAbilityStackInstance;
-import forge.game.zone.MagicStack;
 
 public class ChangeXEffect extends SpellAbilityEffect {
 
@@ -19,21 +16,13 @@ public class ChangeXEffect extends SpellAbilityEffect {
         // even if they are in the Triggered Objects
         final List<SpellAbility> sas = getTargetSpells(sa);
 
-        final Player activator = sa.getActivatingPlayer();
-
-        final MagicStack stack = activator.getGame().getStack();
         for (final SpellAbility tgtSA : sas) {
             // for Unbound Flourishing, can't go over SpellAbilityStackInstances because the x is in cast SA copy
             SpellAbility castSA = tgtSA.getHostCard().getCastSA();
             if (castSA != null && tgtSA.equals(castSA) && castSA.getXManaCostPaid() != null) {
                 castSA.setXManaCostPaid(castSA.getXManaCostPaid() * 2);
             }
-            // fall back to other potential cards
+            tgtSA.setXManaCostPaid(castSA.getXManaCostPaid() * 2);
-            SpellAbilityStackInstance si = stack.getInstanceMatchingSpellAbilityID(tgtSA);
-            if (si != null) {
-                // currently hard coded, no nicer way to get the xManaPaid from that Spell/Card
-                si.setXManaPaid(si.getXManaPaid() * 2);
-            }
         }
     }
 }

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

@@ -721,7 +721,7 @@ public class CardProperty {
                         if (castSA == null) {
                             return false;
                         }
-                        List<Mana> payingMana = castSA.getPayingMana();
+                        List<Mana> payingMana = castSA.getSpellAbility(false).getPayingMana();
                         // even if the cost was raised, we only care about mana from activation part
                         // since this can only be 1 currently with Protective Sphere, let's just assume it's the first shard spent for easy handling
                         if (payingMana.isEmpty() || !card.getColor().hasAnyColor(payingMana.get(0).getColor())) {

forge-game/src/main/java/forge/game/spellability/SpellAbilityStackInstance.java

@@ -17,27 +17,19 @@
  */
 package forge.game.spellability;
 
-import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Set;
 
-import org.apache.commons.lang3.StringUtils;
-
-import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
-import com.google.common.collect.TreeBasedTable;
 
 import forge.game.GameObject;
 import forge.game.IIdentifiable;
 import forge.game.ability.AbilityKey;
 import forge.game.ability.ApiType;
 import forge.game.card.Card;
-import forge.game.card.CardCollection;
 import forge.game.card.CardView;
 import forge.game.card.IHasCardView;
-import forge.game.mana.Mana;
 import forge.game.player.Player;
 import forge.game.trigger.TriggerType;
 import forge.game.trigger.WrappedAbility;
@@ -71,32 +63,8 @@ public class SpellAbilityStackInstance implements IIdentifiable, IHasCardView {
     private final SpellAbilityStackInstance subInstance;
     private Player activatingPlayer;
 
-    // When going to a SubAbility that SA has a Instance Choice object
-    private TargetChoices tc = new TargetChoices();
-    private CardCollection splicedCards = null;
-
     private String stackDescription = null;
 
-    // Adjusted Mana Cost
-    // private String adjustedManaCost = "";
-
-    // Paid Mana Cost
-    private List<Mana> payingMana;
-    // private ArrayList<AbilityMana> paidAbilities = new ArrayList<AbilityMana>();
-    private Integer xManaPaid = null;
-
-    // Other Paid things
-    private final TreeBasedTable<String, Boolean, CardCollection> paidHash;
-
-    // Additional info
-    // is Kicked, is Buyback
-
-    // Triggers
-    private final Map<AbilityKey, Object> triggeringObjects;
-    private final List<Object> triggerRemembered;
-
-    private final Map<String, String> storedSVars = Maps.newHashMap();
-
     private final Map<Player, Object> playersWithValidTargets;
 
     private final StackItemView view;
@@ -108,46 +76,18 @@ public class SpellAbilityStackInstance implements IIdentifiable, IHasCardView {
         stackDescription = sa.getStackDescription();
         activatingPlayer = sa.getActivatingPlayer();
 
-        // Payment info
-        paidHash = TreeBasedTable.create(ability.getPaidHash());
-        ability.resetPaidHash();
-        splicedCards = sa.getSplicedCards();
-
-        xManaPaid = sa.getXManaCostPaid();
-        payingMana = Lists.newArrayList(sa.getPayingMana());
-
-        // Triggering info
-        triggeringObjects = sa.getTriggeringObjects();
-        triggerRemembered = sa.getTriggerRemembered();
-
         subInstance = ability.getSubAbility() == null ? null : new SpellAbilityStackInstance(ability.getSubAbility());
 
-        // Targeting info -- 29/06/11 Moved to after taking care of SubAbilities
-        // because otherwise AF_DealDamage SubAbilities that use Defined$
-        // Targeted breaks (since it's parents target is reset)
-        if (sa.usesTargeting()) {
-            tc = ability.getTargets();
-            ability.resetTargets();
-        }
-
         final Card source = ability.getHostCard();
 
-        // We probably should be storing SA svars too right?
+        final Map<String, String> sVars = (ability.isWrapper() ? ((WrappedAbility) ability).getWrappedAbility() : ability).getDirectSVars();
-        if (!sa.isWrapper()) {
+        if (ApiType.SetState == sa.getApi() && !sVars.containsKey("StoredTransform")) {
-            for (final Entry<String, String> e : sa.getDirectSVars().entrySet()) {
-                final String value = e.getValue();
-                if (!StringUtils.isEmpty(value)) {
-                    storedSVars.put(e.getKey(), value);
-                }
-            }
-        }
-
-        if (ApiType.SetState == sa.getApi() && !storedSVars.containsKey("StoredTransform")) {
             // Record current state of Transformation if the ability might change state
-            storedSVars.put("StoredTransform", String.valueOf(source.getTransformedTimestamp()));
+            sVars.put("StoredTransform", String.valueOf(source.getTransformedTimestamp()));
         }
 
         //store zones to open and players to open them for at the time the SpellAbility first goes on the stack based on the selected targets
+        TargetChoices tc = ability.getTargets();
         if (tc == null) {
             playersWithValidTargets = null;
         } else {
@@ -172,33 +112,12 @@ public class SpellAbilityStackInstance implements IIdentifiable, IHasCardView {
     //      Perhaps lets move the refresh logic to a separate function called only when necessary
     public final SpellAbility getSpellAbility(boolean refresh) {
         if (refresh) {
-            ability.setTargets(tc);
             ability.setActivatingPlayer(activatingPlayer);
 
             // Saved sub-SA needs to be reset on the way out
             if (subInstance != null) {
                 ability.setSubAbility((AbilitySub) subInstance.getSpellAbility(true));
             }
-
-            // Set Cost specific things here
-            ability.setPaidHash(paidHash);
-            ability.setSplicedCards(splicedCards);
-            ability.setXManaCostPaid(xManaPaid);
-            ability.setPayingMana(payingMana);
-
-            // Triggered
-            ability.setTriggeringObjects(triggeringObjects);
-            ability.setTriggerRemembered(triggerRemembered);
-
-            // Add SVars back in
-            final SpellAbility sa = ability.isWrapper() ? ((WrappedAbility) ability).getWrappedAbility() : ability;
-            for (final String store : storedSVars.keySet()) {
-                final String value = storedSVars.get(store);
-
-                if (!StringUtils.isEmpty(value)) {
-                    sa.setSVar(store, value);
-                }
-            }
         }
         return ability;
     }
@@ -211,13 +130,6 @@ public class SpellAbilityStackInstance implements IIdentifiable, IHasCardView {
     public final Card getSourceCard() {
         return ability.getHostCard();
     }
-    
-    public final int getXManaPaid() {
-    	return xManaPaid == null ? 0 : xManaPaid;
-    }
-    public final void setXManaPaid(int x) {
-        xManaPaid = x;
-    }
 
     public final boolean isSpell() {
         return ability.isSpell();
@@ -244,7 +156,7 @@ public class SpellAbilityStackInstance implements IIdentifiable, IHasCardView {
     }
 
     public final TargetChoices getTargetChoices() {
-        return tc;
+        return ability.getTargets();
     }
 
     public final Map<Player, Object> getPlayersWithValidTargets() {
@@ -253,9 +165,8 @@ public class SpellAbilityStackInstance implements IIdentifiable, IHasCardView {
 
     public void updateTarget(TargetChoices target, Card cause) {
         if (target != null) {
-            TargetChoices oldTarget = tc;
+            TargetChoices oldTarget = ability.getTargets();
-            tc = target;
+            ability.setTargets(target);
-            ability.setTargets(tc);
             stackDescription = ability.getStackDescription();
             view.updateTargetCards(this);
             view.updateTargetPlayers(this);
@@ -293,26 +204,21 @@ public class SpellAbilityStackInstance implements IIdentifiable, IHasCardView {
     }
 
     public boolean addTriggeringObject(AbilityKey trigObj, Object value) {
-        if (!triggeringObjects.containsKey(trigObj)) {
+        if (!ability.hasTriggeringObject(trigObj)) {
-            triggeringObjects.put(trigObj, value);
+            ability.setTriggeringObject(trigObj, value);
             return true;
         }
         return false;
     }
-
     public boolean updateTriggeringObject(AbilityKey trigObj, Object value) {
-        if (triggeringObjects.containsKey(trigObj)) {
+        if (ability.hasTriggeringObject(trigObj)) {
-            triggeringObjects.replace(trigObj, value);
+            ability.setTriggeringObject(trigObj, value);
             return true;
         }
         return false;
     }
-
     public Object getTriggeringObject(AbilityKey trigObj) {
-        if (triggeringObjects.containsKey(trigObj)) {
+        return ability.getTriggeringObject(trigObj);
-            return triggeringObjects.get(trigObj);
-        }
-        return null;
     }
 
     public Player getActivatingPlayer() {
@@ -327,10 +233,6 @@ public class SpellAbilityStackInstance implements IIdentifiable, IHasCardView {
         }
     }
 
-    public List<Mana> getPayingMana() {
-        return payingMana;
-    }
-
     @Override
     public String toString() {
         return TextUtil.concatNoSpace(getSourceCard().toString(), "->", getStackDescription());

forge-game/src/main/java/forge/game/zone/MagicStack.java

@@ -159,8 +159,8 @@ public class MagicStack /* extends MyObservable */ implements Iterable<SpellAbil
 
         // Add all Frozen Abilities onto the stack
         while (!frozenStack.isEmpty()) {
-            final SpellAbility sa = frozenStack.pop().getSpellAbility(true);
+            final SpellAbilityStackInstance si = frozenStack.pop();
-            add(sa);
+            add(si.getSpellAbility(true), si);
         }
         // Add all waiting triggers onto the stack
         game.getTriggerHandler().resetActiveTriggers();
@@ -218,7 +218,9 @@ public class MagicStack /* extends MyObservable */ implements Iterable<SpellAbil
     }
 
     public final void add(SpellAbility sp) {
-        SpellAbilityStackInstance si = null;
+        add(sp, null);
+    }
+    public final void add(SpellAbility sp, SpellAbilityStackInstance si) {
         final Card source = sp.getHostCard();
         Player activator = sp.getActivatingPlayer();
 
@@ -302,19 +304,19 @@ public class MagicStack /* extends MyObservable */ implements Iterable<SpellAbil
             }
         }
 
-        if (frozen && !sp.hasParam("IgnoreFreeze")) {
+        if (si == null && sp.isActivatedAbility() && !sp.isCopied()) {
-            si = new SpellAbilityStackInstance(sp);
-            frozenStack.push(si);
-            return;
-        }
-
-        if (sp.isActivatedAbility() && !sp.isCopied()) {
             // if not already copied use a fresh instance
             SpellAbility original = sp;
             sp = sp.copy();
             sp.setOriginalAbility(original);
         }
 
+        if (frozen && !sp.hasParam("IgnoreFreeze")) {
+            si = new SpellAbilityStackInstance(sp);
+            frozenStack.push(si);
+            return;
+        }
+
         if (sp.isAbility()) {
             source.addAbilityActivated(sp);
         }
@@ -326,7 +328,7 @@ public class MagicStack /* extends MyObservable */ implements Iterable<SpellAbil
         }
 
         // The ability is added to stack HERE
-        si = push(sp);
+        si = push(sp, si);
 
         // Copied spells aren't cast per se so triggers shouldn't run for them.
         Map<AbilityKey, Object> runParams = AbilityKey.mapFromPlayer(sp.getHostCard().getController());
@@ -444,7 +446,7 @@ public class MagicStack /* extends MyObservable */ implements Iterable<SpellAbil
     }
 
     // Push should only be used by add.
-    private SpellAbilityStackInstance push(final SpellAbility sp) {
+    private SpellAbilityStackInstance push(final SpellAbility sp, SpellAbilityStackInstance si) {
         if (null == sp.getActivatingPlayer()) {
             sp.setActivatingPlayer(sp.getHostCard().getController());
             System.out.println(sp.getHostCard().getName() + " - activatingPlayer not set before adding to stack.");
@@ -453,7 +455,7 @@ public class MagicStack /* extends MyObservable */ implements Iterable<SpellAbil
         if (sp.isSpell() && sp.getMayPlay() != null) {
             sp.getMayPlay().incMayPlayTurn();
         }
-        final SpellAbilityStackInstance si = new SpellAbilityStackInstance(sp);
+        si = si == null ? new SpellAbilityStackInstance(sp) : si;
 
         stack.addFirst(si);
         int stackIndex = stack.size() - 1;
@@ -497,7 +499,6 @@ public class MagicStack /* extends MyObservable */ implements Iterable<SpellAbil
         // The SpellAbility isn't removed from the Stack until it finishes resolving
         // temporarily reverted removing SAs after resolution
         final SpellAbility sa = peekAbility();
-        //final SpellAbility sa = pop();
 
         // ActivePlayer gains priority first after Resolve
         game.getPhaseHandler().resetPriority();

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

@@ -20,7 +20,7 @@ Types:Legendary Artifact
 K:Modular:3
 A:AB$ MoveCounter | Cost$ X T | Source$ Self | ValidTgts$ Artifact.Other | TgtPrompt$ Select another target artifact to get counters | CounterType$ P1P1 | CounterNum$ X | SorcerySpeed$ True | SubAbility$ DBPump | SpellDescription$ Move X +1/+1 counters from NICKNAME onto another target artifact. Activate only as a sorcery.
 SVar:DBPump:DB$ Pump | Defined$ Targeted | KW$ Haste | SubAbility$ DBConvert | SpellDescription$ That artifact gains haste until end of turn.
-SVar:DBConvert:DB$ SetState | Mode$ Transform | ConditionPresent$ Card.Self+counters_EQ0_P1P1 | StackDescription$ If NICKNAME has no +1/+1 counters on it, convert it.
+SVar:DBConvert:DB$ SetState | Mode$ Transform | ConditionPresent$ Card.Self+counters_EQ0_P1P1 | SpellDescription$ If NICKNAME has no +1/+1 counters on it, convert it.
 SVar:X:Count$xPaid
 DeckHas:Ability$Counters
 Oracle:Modular 3\n{X}, {T}: Move X +1/+1 counters from Blaster onto another target artifact. That artifact gains haste until end of turn. If Blaster has no +1/+1 counters on it, convert it. Activate only as a sorcery.

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

@@ -3,9 +3,9 @@ ManaCost:2 G
 Types:Legendary Creature Elf Noble
 PT:3/2
 T:Mode$ Scry | ValidPlayer$ You | TriggerZones$ Battlefield | Execute$ TrigCharm | TriggerDescription$ Whenever you scry, ABILITY
-SVar:TrigCharm:DB$ Charm | Choices$ DBMustBeBlocked,DBEffect | SubAbility$ DBPump | AdditionalDescription$ and CARDNAME gets +1/+1 until end of turn. 
+SVar:TrigCharm:DB$ Charm | Choices$ DBMustBeBlocked,DBEffect | AdditionalDescription$ and CARDNAME gets +1/+1 until end of turn. 
-SVar:DBMustBeBlocked:DB$ Pump | Defined$ Self | KW$ HIDDEN CARDNAME must be blocked if able. | SpellDescription$ NICKNAME must be blocked this turn if able.
+SVar:DBMustBeBlocked:DB$ Pump | Defined$ Self | KW$ HIDDEN CARDNAME must be blocked if able. | SubAbility$ DBPump | SpellDescription$ NICKNAME must be blocked this turn if able.
-SVar:DBEffect:DB$ Effect | StaticAbilities$ MinMaxBlocked | ExileOnMoved$ Battlefield | SpellDescription$ NICKNAME can't be blocked by more than one creature each combat this turn.
+SVar:DBEffect:DB$ Effect | StaticAbilities$ MinMaxBlocked | ExileOnMoved$ Battlefield | SubAbility$ DBPump | SpellDescription$ NICKNAME can't be blocked by more than one creature each combat this turn.
 SVar:MinMaxBlocked:Mode$ MinMaxBlocker | ValidCard$ Card.EffectSource | Max$ 1 | Description$ NICKNAME can't be blocked by more than one creature each combat this turn.
 SVar:DBPump:DB$ Pump | Defined$ Self | NumAtt$ 1 | NumDef$ 1
 Oracle:Whenever you scry, choose one and Glorfindel, Dauntless Rescuer gets +1/+1 until end of turn.\n• Glorfindel must be blocked this turn if able.\n• Glorfindel can't be blocked by more than one creature each combat this turn.

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

@@ -206,11 +206,6 @@ public class HumanPlaySpellAbility {
                 game.getStack().addAndUnfreeze(ability);
             }
 
-            // no worries here. The same thread must resolve, and by this moment ability will have been resolved already
-            // Triggers haven't resolved yet ??
-            if (mayChooseTargets && !ability.hasParam("TargetsAtRandom")) {
-                ability.clearTargets();
-            }
             if (manaTypeConversion || manaColorConversion || keywordColor) {
                 manapool.restoreColorReplacements();
             }