Merge branch '1153-multiple-staticability-pump-on-same-creature-does-conflict' into 'master'

Resolve "multiple StaticAbility Pump on same creature does conflict"

Closes #1153 and #1156

See merge request core-developers/forge!2119

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

@@ -1601,8 +1601,8 @@ public class ComputerUtilCard {
         }
 
         pumped.addNewPT(c.getCurrentPower(), c.getCurrentToughness(), timestamp);
-        pumped.setPTBoost(c.getPTBoostMap());
+        pumped.setPTBoost(c.getPTBoostTable());
-        pumped.addPTBoost(power + berserkPower, toughness, timestamp);
+        pumped.addPTBoost(power + berserkPower, toughness, timestamp, null);
         pumped.addChangedCardKeywords(kws, null, false, false, timestamp);
         Set<CounterType> types = c.getCounters().keySet();
         for(CounterType ct : types) {
@@ -1649,8 +1649,8 @@ public class ComputerUtilCard {
         list.add(vCard); // account for the static abilities that may be present on the card itself
         for (final Card c : list) {
             // remove old boost that might be copied
-            vCard.removePTBoost(c.getTimestamp());
             for (final StaticAbility stAb : c.getStaticAbilities()) {
+                vCard.removePTBoost(c.getTimestamp(), stAb.getId());
                 final Map<String, String> params = stAb.getMapParams();
                 if (!params.get("Mode").equals("Continuous")) {
                     continue;
@@ -1683,7 +1683,7 @@ public class ComputerUtilCard {
                         def = AbilityUtils.calculateAmount(c, addT, stAb);
                     }
                 }
-                vCard.addPTBoost(att, def, c.getTimestamp());
+                vCard.addPTBoost(att, def, c.getTimestamp(), stAb.getId());
             }
         }
     }

forge-ai/src/main/java/forge/ai/simulation/GameCopier.java

@@ -246,7 +246,7 @@ public class GameCopier {
             newCard.addType(type);
         }
         for (StaticAbility stAb : c.getStaticAbilities()) {
-            newCard.addStaticAbility(stAb);
+            newCard.addStaticAbility(stAb.copy(newCard, true));
         }
         for (SpellAbility sa : c.getSpellAbilities()) {
             SpellAbility saCopy = sa.copy(newCard, true);
@@ -277,7 +277,7 @@ public class GameCopier {
                 // TODO: Copy the full list with timestamps.
                 newCard.addNewPT(setPower, setToughness, newGame.getNextTimestamp());
             }
-            newCard.setPTBoost(c.getPTBoostMap());
+            newCard.setPTBoost(c.getPTBoostTable());
             newCard.setDamage(c.getDamage());
             
             newCard.setChangedCardTypes(c.getChangedCardTypesMap());

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

@@ -798,7 +798,7 @@ public class StaticEffect {
             }
 
             // remove P/T bonus
-            affectedCard.removePTBoost(getTimestamp());
+            affectedCard.removePTBoost(getTimestamp(), ability.getId());
 
             // the view is updated in GameAction#checkStaticAbilities to avoid flickering
 

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

@@ -52,7 +52,7 @@ public class PumpAllEffect extends SpellAbilityEffect {
             boolean redrawPT = false;
 
             if (a != 0 || d != 0) {
-                tgtC.addPTBoost(a, d, timestamp);
+                tgtC.addPTBoost(a, d, timestamp, 0);
                 redrawPT = true;
             }
 
@@ -76,7 +76,7 @@ public class PumpAllEffect extends SpellAbilityEffect {
 
                     @Override
                     public void run() {
-                        tgtC.removePTBoost(timestamp);
+                        tgtC.removePTBoost(timestamp, 0);
                         tgtC.removeChangedCardKeywords(timestamp);
 
                         for (String kw : hiddenkws) {

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

@@ -56,7 +56,7 @@ public class PumpEffect extends SpellAbilityEffect {
         }
 
         if (a != 0 || d != 0) {
-            gameCard.addPTBoost(a, d, timestamp);
+            gameCard.addPTBoost(a, d, timestamp, 0);
             redrawPT = true;
         }
 
@@ -84,7 +84,7 @@ public class PumpEffect extends SpellAbilityEffect {
 
                 @Override
                 public void run() {
-                    gameCard.removePTBoost(timestamp);
+                    gameCard.removePTBoost(timestamp, 0);
                     boolean updateText = false;
                     updateText = gameCard.removeCanBlockAny(timestamp) || updateText;
                     updateText = gameCard.removeCanBlockAdditional(timestamp) || updateText;

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

@@ -198,7 +198,9 @@ public class Card extends GameEntity implements Comparable<Card> {
     // stack of set power/toughness
     private Map<Long, Pair<Integer,Integer>> newPT = Maps.newTreeMap();
     private Map<Long, Pair<Integer,Integer>> newPTCharacterDefining = Maps.newTreeMap();
-    private Map<Long, Pair<Integer,Integer>> boostPT = Maps.newTreeMap();
+
+    // x=Static Avility id or 0, y=timestamp
+    private Table<Integer, Long, Pair<Integer,Integer>> boostPT = TreeBasedTable.create();
 
     private String basePowerString = null;
     private String baseToughnessString = null;
@@ -3474,23 +3476,21 @@ public class Card extends GameEntity implements Comparable<Card> {
         return result;
     }
 
-    public void addPTBoost(final Integer power, final Integer toughness, final long timestamp) {
+    public void addPTBoost(final Integer power, final Integer toughness, final long timestamp, final Integer staticId) {
-        boostPT.put(timestamp, Pair.of(power, toughness));
+        boostPT.put(staticId == null ? 0 : staticId, timestamp, Pair.of(power, toughness));
     }
 
-    public void removePTBoost(final long timestamp) {
+    public void removePTBoost(final long timestamp, final Integer staticId) {
-        boostPT.remove(timestamp);
+        boostPT.remove(staticId, timestamp);
     }
 
-    public Map<Long, Pair<Integer, Integer>> getPTBoostMap() {
+    public Table<Integer, Long, Pair<Integer, Integer>> getPTBoostTable() {
-        return ImmutableMap.copyOf(boostPT);
+        return ImmutableTable.copyOf(boostPT);
     }
 
-    public void setPTBoost(Map<Long, Pair<Integer, Integer>> map) {
+    public void setPTBoost(Table<Integer, Long, Pair<Integer, Integer>> table) {
         this.boostPT.clear();
-        for (Map.Entry<Long, Pair<Integer,Integer>> e : map.entrySet()) {
+        boostPT.putAll(table);
-            this.boostPT.put(e.getKey(), Pair.of(e.getValue().getLeft(), e.getValue().getRight()));
-        }
     }
 
     public final boolean isUntapped() {
@@ -3965,12 +3965,8 @@ public class Card extends GameEntity implements Comparable<Card> {
         return null;
     }
     public final StaticAbility addStaticAbility(final StaticAbility stAb) {
-        return addStaticAbility(stAb, false);
+        currentState.addStaticAbility(stAb);
-    }
+        return stAb;
-    public final StaticAbility addStaticAbility(final StaticAbility stAb, boolean intrinsic) {
-        final StaticAbility stAbCopy = new StaticAbility(stAb, this);
-        currentState.addStaticAbility(stAbCopy);
-        return stAbCopy;
     }
     public final void removeStaticAbility(StaticAbility stAb) {
         currentState.removeStaticAbility(stAb);

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

@@ -529,7 +529,7 @@ public class CardState extends GameObject {
         staticAbilities.clear();
         for (StaticAbility sa : source.staticAbilities) {
             if (sa.isIntrinsic()) {
-                staticAbilities.add(new StaticAbility(sa, this.card));
+                staticAbilities.add(sa.copy(card, lki));
             }
         }
     }

forge-game/src/main/java/forge/game/keyword/KeywordInstance.java

@@ -228,7 +228,7 @@ public abstract class KeywordInstance<T extends KeywordInstance<?>> implements K
             
             result.staticAbilities = Lists.newArrayList();
             for (StaticAbility sa : this.staticAbilities) {
-                result.staticAbilities.add(new StaticAbility(sa, host));
+                result.staticAbilities.add(sa.copy(host, lki));
             }
             
             return result;

forge-game/src/main/java/forge/game/replacement/ReplacementEffect.java

@@ -27,6 +27,7 @@ import forge.util.TextUtil;
 
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 /**
  * TODO: Write javadoc for this type.
@@ -261,6 +262,6 @@ public abstract class ReplacementEffect extends TriggerReplacementBase {
     /** {@inheritDoc} */
     @Override
     public int hashCode() {
-        return 42 * (42 + this.getId());
+        return Objects.hash(ReplacementEffect.class, getId());
     }
 }

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

@@ -205,7 +205,7 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     }
     @Override
     public int hashCode() {
-        return getId();
+        return Objects.hash(SpellAbility.class, getId());
     }
     @Override
     public boolean equals(final Object obj) {

forge-game/src/main/java/forge/game/staticability/StaticAbility.java

@@ -17,6 +17,7 @@
  */
 package forge.game.staticability;
 
+import com.google.common.collect.ComparisonChain;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import forge.card.MagicColor;
@@ -24,6 +25,7 @@ import forge.game.CardTraitBase;
 import forge.game.Game;
 import forge.game.GameEntity;
 import forge.game.GameStage;
+import forge.game.IIdentifiable;
 import forge.game.ability.AbilityUtils;
 import forge.game.card.Card;
 import forge.game.card.CardCollection;
@@ -43,18 +45,38 @@ import forge.util.TextUtil;
 import java.util.EnumSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 
 /**
  * The Class StaticAbility.
  */
-public class StaticAbility extends CardTraitBase implements Comparable<StaticAbility> {
+public class StaticAbility extends CardTraitBase implements IIdentifiable, Cloneable, Comparable<StaticAbility> {
+    private static int maxId = 0;
+    private static int nextId() { return ++maxId; }
 
-    private final Set<StaticAbilityLayer> layers;
+    private int id;
+
+    private Set<StaticAbilityLayer> layers;
     private CardCollectionView ignoreEffectCards = new CardCollection();
     private final List<Player> ignoreEffectPlayers = Lists.newArrayList();
     private int mayPlayTurn = 0;
 
+    @Override
+    public final int getId() {
+        return id;
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(StaticAbility.class, getId());
+    }
+
+    @Override
+    public boolean equals(final Object obj) {
+        return obj instanceof StaticAbility && this.id == ((StaticAbility) obj).id;
+    }
+
     /**
      * <p>
      * Getter for the field <code>mapParams</code>.
@@ -228,6 +250,7 @@ public class StaticAbility extends CardTraitBase implements Comparable<StaticAbi
      *            the host
      */
     private StaticAbility(final Map<String, String> params, final Card host) {
+        this.id = nextId();
         this.originalMapParams.putAll(params);
         this.mapParams.putAll(params);
         this.layers = this.generateLayer();
@@ -235,19 +258,6 @@ public class StaticAbility extends CardTraitBase implements Comparable<StaticAbi
         buildCommonAttributes(host);
     }
 
-    public StaticAbility(StaticAbility stAb, Card host) {
-        this.originalMapParams.putAll(stAb.originalMapParams);
-        this.mapParams.putAll(stAb.mapParams);
-        this.layers = this.generateLayer();
-        this.hostCard = host;
-        this.intrinsic = stAb.intrinsic;
-
-        // Copy old sVars
-        this.sVars.putAll(stAb.sVars);
-        // but if they are References use this ones
-        buildCommonAttributes(host);
-    }
-
     public final CardCollectionView applyContinuousAbilityBefore(final StaticAbilityLayer layer, final CardCollectionView preList) {
         if (!shouldApplyContinuousAbility(layer, false)) {
             return null;
@@ -769,6 +779,32 @@ public class StaticAbility extends CardTraitBase implements Comparable<StaticAbi
 
     @Override
     public int compareTo(StaticAbility arg0) {
-        return getHostCard().compareTo(arg0.getHostCard());
+        return ComparisonChain.start()
+        .compare(getHostCard(),arg0.getHostCard())
+        .compare(getId(), arg0.getId())
+        .result();
+    }
+
+    public StaticAbility copy(Card host, final boolean lki) {
+        StaticAbility clone = null;
+        try {
+            clone = (StaticAbility) clone();
+            clone.id = lki ? id : nextId();
+
+            // dont use setHostCard to not trigger the not copied parts yet
+            clone.hostCard = host;
+            // need to clone the maps too so they can be changed
+            clone.originalMapParams = Maps.newHashMap(this.originalMapParams);
+            clone.mapParams = Maps.newHashMap(this.mapParams);
+
+            clone.sVars = Maps.newHashMap(this.sVars);
+
+            clone.layers = this.generateLayer();
+
+            clone.buildCommonAttributes(host);
+        } catch (final CloneNotSupportedException e) {
+            System.err.println(e);
+        }
+        return clone;
     }
 } // end class StaticAbility

forge-game/src/main/java/forge/game/staticability/StaticAbilityContinuous.java

@@ -532,7 +532,7 @@ public final class StaticAbilityContinuous {
                 if (addT.startsWith("AffectedX")) {
                     toughnessBonus = CardFactoryUtil.xCount(affectedCard, AbilityUtils.getSVar(stAb, addT));
                 }
-                affectedCard.addPTBoost(powerBonus, toughnessBonus, se.getTimestamp());
+                affectedCard.addPTBoost(powerBonus, toughnessBonus, se.getTimestamp(), stAb.getId());
             }
 
             // add keywords

forge-game/src/main/java/forge/game/trigger/Trigger.java

@@ -431,7 +431,7 @@ public abstract class Trigger extends TriggerReplacementBase {
     /** {@inheritDoc} */
     @Override
     public int hashCode() {
-        return 41 * (41 + this.getId());
+        return Objects.hash(Trigger.class, getId());
     }
 
     /**

forge-gui-desktop/src/test/java/forge/ai/simulation/GameSimulatorTest.java

@@ -2036,4 +2036,23 @@ public class GameSimulatorTest extends SimulationTestCase {
         assertTrue(dimirdgAfterCopy2.isFlipped());
         assertFalse(dimirdgAfterCopy2.getType().isLegendary());
     }
+
+    public void testStaticMultiPump() {
+        Game game = initAndCreateGame();
+        Player p = game.getPlayers().get(1);
+
+        Card c1 = addCard("Creakwood Liege", p);
+        Card c2 = addCard("Creakwood Liege", p);
+
+        game.getPhaseHandler().devModeSet(PhaseType.MAIN1, p);
+
+        // update stats state
+        game.getAction().checkStateEffects(true);
+
+        assertTrue(c1.getNetPower() == 4);
+        assertTrue(c1.getNetToughness() == 4);
+
+        assertTrue(c2.getNetPower() == 4);
+        assertTrue(c2.getNetToughness() == 4);
+    }
 }