Merge branch 'svarsStackInstance' into 'master'

StackInstance only store direct SVars

See merge request core-developers/forge!4528

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

@@ -1,6 +1,5 @@
 package forge.game;
 
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -531,11 +530,16 @@ public abstract class CardTraitBase extends GameObject implements IHasCardView,
 
     @Override
     public Map<String, String> getSVars() {
-        Map<String, String> res = new HashMap<>(getSVarFallback().getSVars());
+        Map<String, String> res = Maps.newHashMap(getSVarFallback().getSVars());
         res.putAll(sVars);
         return res;
     }
 
+    @Override
+    public Map<String, String> getDirectSVars() {
+        return sVars;
+    }
+
     @Override
     public void setSVars(Map<String, String> newSVars) {
         sVars = Maps.newTreeMap();

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

@@ -15,6 +15,7 @@ public interface IHasSVars {
     //public Set<String> getSVars();
 
     public Map<String, String> getSVars();
+    public Map<String, String> getDirectSVars();
 
     public void removeSVar(final String var);
 }

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

@@ -1589,10 +1589,16 @@ public class Card extends GameEntity implements Comparable<Card>, IHasSVars {
         currentState.setSVar(var, str);
     }
 
+    @Override
     public final Map<String, String> getSVars() {
         return currentState.getSVars();
     }
 
+    @Override
+    public Map<String, String> getDirectSVars() {
+        return ImmutableMap.of();
+    }
+
     public final void setSVars(final Map<String, String> newSVars) {
         currentState.setSVars(newSVars);
     }

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

@@ -480,30 +480,46 @@ public class CardState extends GameObject implements IHasSVars {
         return null;
     }
 
+    @Override
     public final Map<String, String> getSVars() {
         return sVars;
     }
+
+    @Override
+    public Map<String, String> getDirectSVars() {
+        return sVars;
+    }
+
+    @Override
     public final String getSVar(final String var) {
         if (sVars.containsKey(var)) {
             return sVars.get(var);
         }
         return "";
     }
+
+    @Override
     public final boolean hasSVar(final String var) {
         if (var == null) {
             return false;
         }
         return sVars.containsKey(var);
     }
+
+    @Override
     public final void setSVar(final String var, final String str) {
         sVars.put(var, str);
         view.updateFoilIndex(card.getState(CardStateName.Original));
     }
+
+    @Override
     public final void setSVars(final Map<String, String> newSVars) {
         sVars = Maps.newTreeMap();
         sVars.putAll(newSVars);
         view.updateFoilIndex(card.getState(CardStateName.Original));
     }
+
+    @Override
     public final void removeSVar(final String var) {
         sVars.remove(var);
     }

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

@@ -115,8 +115,6 @@ public class CostRemoveAnyCounter extends CostPart {
 
     @Override
     public boolean payAsDecided(Player ai, PaymentDecision decision, SpellAbility ability) {
-        final Card source = ability.getHostCard();
-
         int removed = 0;
         for (Entry<GameEntity, Map<CounterType, Integer>> e : decision.counterTable.row(Optional.absent()).entrySet()) {
             for (Entry<CounterType, Integer> v : e.getValue().entrySet()) {
@@ -128,7 +126,7 @@ public class CostRemoveAnyCounter extends CostPart {
             }
         }
 
-        source.setSVar("CostCountersRemoved", Integer.toString(removed));
+        ability.setSVar("CostCountersRemoved", Integer.toString(removed));
         return true;
     }
 

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

@@ -172,8 +172,6 @@ public class CostRemoveCounter extends CostPart {
 
     @Override
     public boolean payAsDecided(Player ai, PaymentDecision decision, SpellAbility ability) {
-        Card source = ability.getHostCard();
-
         int removed = 0;
         final int toRemove = decision.c;
 
@@ -184,7 +182,7 @@ public class CostRemoveCounter extends CostPart {
             c.getGame().updateLastStateForCard(c);
         }
 
-        source.setSVar("CostCountersRemoved", Integer.toString(removed));
+        ability.setSVar("CostCountersRemoved", Integer.toString(removed));
         return true;
     }
 

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

@@ -27,6 +27,7 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 
+import org.apache.commons.lang3.ObjectUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.Pair;
 
@@ -45,6 +46,7 @@ import forge.game.Game;
 import forge.game.GameActionUtil;
 import forge.game.GameEntity;
 import forge.game.GameObject;
+import forge.game.IHasSVars;
 import forge.game.IIdentifiable;
 import forge.game.ability.AbilityFactory;
 import forge.game.ability.AbilityKey;
@@ -59,7 +61,6 @@ import forge.game.card.CardPredicates;
 import forge.game.card.CardZoneTable;
 import forge.game.cost.Cost;
 import forge.game.cost.CostPart;
-import forge.game.cost.CostRemoveCounter;
 import forge.game.event.GameEventCardStatsChanged;
 import forge.game.keyword.Keyword;
 import forge.game.mana.Mana;
@@ -1471,6 +1472,10 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
         return null;
     }
 
+    protected IHasSVars getSVarFallback() {
+        return ObjectUtils.firstNonNull(this.getParent(), super.getSVarFallback());
+    }
+
     public boolean isUndoable() {
         return undoable && payCosts.isUndoable() && getHostCard().isInPlay();
     }
@@ -1664,20 +1669,8 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
         if (!isMinTargetChosen()) {
             return false;
         }
-        int maxTargets = getMaxTargets();
 
-        if (maxTargets == 0 && getPayCosts().hasSpecificCostType(CostRemoveCounter.class)
+        return getMaxTargets() >= getTargets().size();
-                && hasSVar(getParam("TargetMax"))
-                && getSVar(getParam("TargetMax")).startsWith("Count$CardCounters")
-                && getHostCard() != null && getHostCard().hasSVar("CostCountersRemoved")) {
-            // TODO: Current AI implementation removes the counters during payment before the
-            // ability is added to stack, resulting in maxTargets=0 at this point. We are
-            // assuming here that the AI logic specified a legal number, and that number ended
-            // up being in CostCountersRemoved that is created on the card during payment.
-            maxTargets = Integer.parseInt(getHostCard().getSVar("CostCountersRemoved"));
-        }
-
-        return maxTargets >= getTargets().size();
     }
     /**
      * <p>

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

@@ -132,7 +132,7 @@ public class SpellAbilityStackInstance implements IIdentifiable, IHasCardView {
 
         // We probably should be storing SA svars too right?
         if (!sa.isWrapper()) {
-            for (final Entry<String, String> e : sa.getSVars().entrySet()) {
+            for (final Entry<String, String> e : sa.getDirectSVars().entrySet()) {
                 final String value = e.getValue();
                 if (!StringUtils.isEmpty(value)) {
                     storedSVars.put(e.getKey(), value);