Merge branch 'removeSpellAbilityCache' into 'master'

Game: remove SpellAbilityCache

See merge request core-developers/forge!2763

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

@@ -125,8 +125,6 @@ public class GameCopier {
                 }
             }
         }
-        origGame.validateSpabCache();
-        newGame.validateSpabCache();
 
         // Undo effects first before calculating them below, to avoid them applying twice.
         for (StaticEffect effect : origGame.getStaticEffects().getEffects()) {

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

@@ -40,7 +40,6 @@ import forge.game.player.*;
 import forge.game.replacement.ReplacementHandler;
 import forge.game.spellability.SpellAbility;
 import forge.game.spellability.SpellAbilityStackInstance;
-import forge.game.spellability.SpellAbilityView;
 import forge.game.trigger.TriggerHandler;
 import forge.game.trigger.TriggerType;
 import forge.game.zone.CostPaymentStack;
@@ -209,27 +208,6 @@ public class Game {
         changeZoneLKIInfo.clear();
     }
 
-    private final GameEntityCache<SpellAbility, SpellAbilityView> spabCache = new GameEntityCache<>();
-    public SpellAbility getSpellAbility(final SpellAbilityView view) {
-        return spabCache.get(view);
-    }
-    public void addSpellAbility(SpellAbility spellAbility) {
-        spabCache.put(spellAbility.getId(), spellAbility);
-    }
-    public void removeSpellAbility(SpellAbility spellAbility) {
-        spabCache.remove(spellAbility.getId());
-    }
-    public void validateSpabCache() {
-        for (SpellAbility sa : spabCache.getValues()) {
-            if (sa.getHostCard() != null && sa.getHostCard().getGame() != this) {
-                throw new RuntimeException();
-            }
-            if (sa.getActivatingPlayer() != null && sa.getActivatingPlayer().getGame() != this) {
-                throw new RuntimeException();
-            }
-        }
-    }
-
     public Game(List<RegisteredPlayer> players0, GameRules rules0, Match match0) { /* no more zones to map here */
         rules = rules0;
         match = match0;
@@ -896,7 +874,6 @@ public class Game {
     }
 
     public void clearCaches() {
-        spabCache.clear();
         cardCache.clear();
 
         lastStateBattlefield.clear();

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

@@ -192,9 +192,6 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
             view0 = new SpellAbilityView(this);
         }
         view = view0;
-        if (hostCard != null && hostCard.getGame() != null) {
-            hostCard.getGame().addSpellAbility(this);
-        }
     }
 
     @Override
@@ -213,13 +210,7 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     @Override
     public void setHostCard(final Card c) {
         if (hostCard == c) { return; }
-        Game oldGame = hostCard != null ? hostCard.getGame() : null;
-        Game newGame = c != null ? c.getGame() : null;
         super.setHostCard(c);
-        if (oldGame != newGame) {
-            if (oldGame != null) { oldGame.removeSpellAbility(this); }
-            if (newGame != null) { newGame.addSpellAbility(this); }
-        }
 
         if (manaPart != null) {
             manaPart.setSourceCard(c);
@@ -892,9 +883,6 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
             // dont use setHostCard to not trigger the not copied parts yet
 
             copyHelper(clone, host);
-            if (!lki && host != null && host.getGame() != null) {
-                host.getGame().addSpellAbility(clone);
-            }
 
             clone.triggeringObjects = AbilityKey.newMap(this.triggeringObjects);
 
@@ -1996,24 +1984,4 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     public void setXManaCostPaid(final Integer n) {
         xManaCostPaid = n;
     }
-
-    public void removeFromGame() {
-        if (getHostCard() == null) {
-            return;
-        }
-
-        getHostCard().getGame().removeSpellAbility(this);
-
-        if (subAbility != null) {
-            subAbility.removeFromGame();
-        }
-        for (AbilitySub sa : additionalAbilities.values()) {
-            sa.removeFromGame();
-        }
-        for (List<AbilitySub> list : additionalAbilityLists.values()) {
-            for (AbilitySub sa : list) {
-                sa.removeFromGame();
-            }
-        }
-    }
 }

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

@@ -1,8 +1,11 @@
 package forge.game.spellability;
 
+import java.util.Map;
+
+import com.google.common.collect.Maps;
+
 import forge.game.card.CardView;
 import forge.game.card.IHasCardView;
-import forge.trackable.TrackableCollection;
 import forge.trackable.TrackableObject;
 import forge.trackable.TrackableProperty;
 
@@ -13,15 +16,12 @@ public class SpellAbilityView extends TrackableObject implements IHasCardView {
         return spab == null ? null : spab.getView();
     }
 
-    public static TrackableCollection<SpellAbilityView> getCollection(Iterable<SpellAbility> spabs) {
+    public static <T extends SpellAbility>  Map<SpellAbilityView, T> getMap(Iterable<T> spabs) {
-        if (spabs == null) {
+        Map<SpellAbilityView, T> spellViewCache = Maps.newLinkedHashMap();
-            return null;
+        for (T spellAbility : spabs) {
+            spellViewCache.put(spellAbility.getView(), spellAbility);
         }
-        TrackableCollection<SpellAbilityView> collection = new TrackableCollection<>();
+        return spellViewCache;
-        for (SpellAbility spab : spabs) {
-            collection.add(spab.getView());
-        }
-        return collection;
     }
 
     SpellAbilityView(final SpellAbility sa) {

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

@@ -563,10 +563,4 @@ public class WrappedAbility extends Ability {
     public void setXManaCostPaid(final Integer n) {
         sa.setXManaCostPaid(n);
     }
-
-    @Override
-    public void removeFromGame() {
-        super.removeFromGame();
-        getHostCard().getGame().removeSpellAbility(this.getWrappedAbility());
-    }
 }

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

@@ -640,10 +640,6 @@ public class MagicStack /* extends MyObservable */ implements Iterable<SpellAbil
         sa.setLastStateBattlefield(CardCollection.EMPTY);
         sa.setLastStateGraveyard(CardCollection.EMPTY);
         game.fireEvent(new GameEventSpellRemovedFromStack(sa));
-
-        if (sa.isTrigger()) {
-            sa.removeFromGame();
-        }
     }
 
     public final void remove(final Card c) {

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

@@ -56,7 +56,6 @@ import forge.match.input.*;
 import forge.model.FModel;
 import forge.properties.ForgeConstants;
 import forge.properties.ForgePreferences.FPref;
-import forge.trackable.TrackableObject;
 import forge.util.ITriggerEvent;
 import forge.util.Lang;
 import forge.util.Localizer;
@@ -96,6 +95,8 @@ public class PlayerControllerHuman extends PlayerController implements IGameCont
 
     private final Localizer localizer = Localizer.getInstance();
 
+    protected Map<SpellAbilityView, SpellAbility> spellViewCache = null;
+
     public PlayerControllerHuman(final Game game0, final Player p, final LobbyPlayer lp) {
         super(game0, p, lp);
         inputProxy = new InputProxy(this);
@@ -200,9 +201,10 @@ public class PlayerControllerHuman extends PlayerController implements IGameCont
     @Override
     public SpellAbility getAbilityToPlay(final Card hostCard, final List<SpellAbility> abilities,
             final ITriggerEvent triggerEvent) {
+        spellViewCache = SpellAbilityView.getMap(abilities);
         final SpellAbilityView resultView = getGui().getAbilityToPlay(CardView.get(hostCard),
-                SpellAbilityView.getCollection(abilities), triggerEvent);
+                Lists.newArrayList(spellViewCache.keySet()), triggerEvent);
-        return getGame().getSpellAbility(resultView);
+        return resultView == null ? null : spellViewCache.get(resultView);
     }
 
     @Override
@@ -547,12 +549,8 @@ public class PlayerControllerHuman extends PlayerController implements IGameCont
         getGui().setCard(CardView.get(sa.getHostCard()));
 
         // create a mapping between a spell's view and the spell itself
-        HashMap<SpellAbilityView, SpellAbility> spellViewCache = new HashMap<>();
+        Map<SpellAbilityView, SpellAbility> spellViewCache = SpellAbilityView.getMap(spells);
-        for (SpellAbility spellAbility : spells) {
+        Object choice = getGui().one(title, Lists.newArrayList(spellViewCache.keySet()));
-            spellViewCache.put(spellAbility.getView(), spellAbility);
-        }
-        List<TrackableObject> choices = new ArrayList<>(spellViewCache.keySet());
-        Object choice = getGui().one(title, choices);
 
         // Human is supposed to read the message and understand from it what to
         // choose
@@ -1489,14 +1487,11 @@ public class PlayerControllerHuman extends PlayerController implements IGameCont
             game.getTracker().unfreeze();
         }
         final List<AbilitySub> possible = CharmEffect.makePossibleOptions(sa);
-        LinkedHashMap<SpellAbilityView, AbilitySub> spellViewCache = new LinkedHashMap<>();
+        Map<SpellAbilityView, AbilitySub> spellViewCache = SpellAbilityView.getMap(possible);
-        for (AbilitySub spellAbility : possible) {
-            spellViewCache.put(spellAbility.getView(), spellAbility);
-        }
         if (trackerFrozen) {
             game.getTracker().freeze(); // refreeze if the tracker was frozen prior to this update
         }
-        final List<SpellAbilityView> choices = new ArrayList<>(spellViewCache.keySet());
+        final List<SpellAbilityView> choices = Lists.newArrayList(spellViewCache.keySet());
         final String modeTitle = localizer.getMessage("lblPlayerActivatedCardChooseMode", sa.getActivatingPlayer().toString(), CardTranslation.getTranslatedName(sa.getHostCard().getName()));
         final List<AbilitySub> chosen = Lists.newArrayListWithCapacity(num);
         for (int i = 0; i < num; i++) {
@@ -1666,10 +1661,7 @@ public class PlayerControllerHuman extends PlayerController implements IGameCont
                 List<SpellAbilityView> orderedSAVs = Lists.newArrayList();
 
                 // create a mapping between a spell's view and the spell itself
-                HashMap<SpellAbilityView, SpellAbility> spellViewCache = new HashMap<>();
+                Map<SpellAbilityView, SpellAbility> spellViewCache = SpellAbilityView.getMap(orderedSAs);
-                for (SpellAbility spellAbility : orderedSAs) {
-                    spellViewCache.put(spellAbility.getView(), spellAbility);
-                }
 
                 if (savedOrder != null) {
                     orderedSAVs = Lists.newArrayList();
@@ -1937,7 +1929,10 @@ public class PlayerControllerHuman extends PlayerController implements IGameCont
 
     @Override
     public void selectAbility(final SpellAbilityView sa) {
-        inputProxy.selectAbility(getGame().getSpellAbility(sa));
+        if (spellViewCache == null || spellViewCache.isEmpty()) {
+            return;
+        }
+        inputProxy.selectAbility(spellViewCache.get(sa));
     }
 
     @Override