diff --git a/forge-game/src/main/java/forge/game/card/Card.java b/forge-game/src/main/java/forge/game/card/Card.java index 5249b18cd2a..fb1ea002d22 100644 --- a/forge-game/src/main/java/forge/game/card/Card.java +++ b/forge-game/src/main/java/forge/game/card/Card.java @@ -628,6 +628,7 @@ public CardStateName getCurrentStateName() { // use by CopyPermanent public void setStates(Map map) { + invalidateTraitCaches(); states.clear(); states.putAll(map); } @@ -4192,6 +4193,10 @@ public final boolean removeChangedCardTypes(final long timestamp, final long sta } public final void updateTypeCache() { + // updateTypes() only refreshes the current state, so drop every state's cached traits here: + // clone and rollback paths reach this via updateChangedText(), and relying on a non-current + // state's cache happening to match its equally stale type would be far too subtle. + invalidateTraitCaches(); this.getCurrentState().updateTypes(); } @@ -4921,6 +4926,7 @@ public final Table getChangedCardTraitsByText() { return changedCardTraitsByText; } public final void setChangedCardTraitsByText(Table changes) { + invalidateTraitCaches(); changedCardTraitsByText.clear(); for (Table.Cell e : changes.cellSet()) { changedCardTraitsByText.put(e.getRowKey(), e.getColumnKey(), e.getValue().copy(this, true)); @@ -4928,6 +4934,7 @@ public final void setChangedCardTraitsByText(Table } public final void addChangedCardTraitsByText(Collection spells, Collection trigger, Collection replacements, Collection statics, long timestamp, long staticId) { + invalidateTraitCaches(); changedCardTraitsByText.put(timestamp, staticId, new CardTraitChanges( spells, trigger, replacements, statics, e -> true )); @@ -4952,6 +4959,7 @@ public final ICardTraitChanges addChangedCardTraits(Collection spe return addChangedCardTraits(result, timestamp, staticId, updateView); } public final ICardTraitChanges addChangedCardTraits(ICardTraitChanges changes, long timestamp, long staticId, boolean updateView) { + invalidateTraitCaches(); changedCardTraits.put(timestamp, staticId, changes); if (updateView) { updateAbilityTextForView(); @@ -4960,9 +4968,11 @@ public final ICardTraitChanges addChangedCardTraits(ICardTraitChanges changes, l } public final boolean removeChangedCardTraits(long timestamp, long staticId) { + invalidateTraitCaches(); return changedCardTraits.remove(timestamp, staticId) != null; } public final boolean removeChangedCardTraitsByText(long timestamp, long staticId) { + invalidateTraitCaches(); return changedCardTraitsByText.remove(timestamp, staticId) != null; } @@ -4982,6 +4992,7 @@ public final Table getChangedCardTraits() { } public final void setChangedCardTraits(Table changes) { + invalidateTraitCaches(); changedCardTraits.clear(); for (Table.Cell e : changes.cellSet()) { changedCardTraits.put(e.getRowKey(), e.getColumnKey(), e.getValue().copy(this, true)); @@ -4989,6 +5000,7 @@ public final void setChangedCardTraits(Table chan } public boolean clearChangedCardTraits() { + invalidateTraitCaches(); boolean changed = false; if (!changedCardTraitsByText.isEmpty()) { changed = true; @@ -5221,7 +5233,19 @@ public final KeywordCollection getUnhiddenKeywords(CardState state) { public final void updateKeywordsCache() { updateKeywordsCache(getCurrentState()); } + /** + * Drop the cached trait lists (triggers / static abilities) on every state of this card. + * Called from each place an input to those lists can change. + */ + public final void invalidateTraitCaches() { + for (CardState st : states.values()) { + st.invalidateTraitCache(); + } + } + public final void updateKeywordsCache(final CardState state) { + // Keywords contribute triggers and statics, so the trait lists must go too. + state.invalidateTraitCache(); KeywordCollection keywords = new KeywordCollection(); // Layer 1 diff --git a/forge-game/src/main/java/forge/game/card/CardState.java b/forge-game/src/main/java/forge/game/card/CardState.java index 58f51928531..aa7cf6737a9 100644 --- a/forge-game/src/main/java/forge/game/card/CardState.java +++ b/forge-game/src/main/java/forge/game/card/CardState.java @@ -114,6 +114,24 @@ public class CardState implements GameObject, IHasSVars, ITranslatable { // wrapped in a List so it can be reused directly private List landTraitChanges = List.of(new LandTraitChanges(this)); + // Trait caches. getStaticAbilities/getTriggers rebuild their list from the layer system on + // every call - millions of times per game - and the result is identical to the previous one + // over 99.9% of the time. Cache it and drop the cache whenever any input changes; see + // Card.invalidateTraitCaches for the invalidation points. + private FCollectionView cachedStaticAbilities; + private FCollectionView cachedTraitTriggers; + + /** + * Drop every cached trait list; they are rebuilt lazily on next access. + * Note: mutating a split state's raw trait lists must invalidate the whole card, because the + * Original state merges LeftSplit/RightSplit's lists into its own - hence those mutators call + * Card.invalidateTraitCaches() rather than this. + */ + final void invalidateTraitCache() { + cachedStaticAbilities = null; + cachedTraitTriggers = null; + } + public CardState(Card card, CardStateName name) { this(card.getView().createAlternateState(name), card); } @@ -156,6 +174,9 @@ public CardTypeView getTypeWithChanges() { public void updateTypes() { this.changedType = getType().getTypeWithChanges(card.getChangedCardTypes()); + // LandTraitChanges clears both lists when hasRemoveIntrinsic() is set, and that reads + // changedCardTypes - which every type mutation funnels through here to refresh. + invalidateTraitCache(); } public void updateTypesForView() { view.updateType(this); @@ -686,6 +707,9 @@ public final boolean addSpellAbility(final SpellAbility a) { } public final FCollectionView getTriggers() { + if (cachedTraitTriggers != null) { + return cachedTraitTriggers; + } FCollection result = new FCollection<>(triggers); if (getStateName().equals(CardStateName.Original)) { if (getCard().hasState(CardStateName.LeftSplit)) @@ -694,6 +718,7 @@ public final FCollectionView getTriggers() { result.addAll(getCard().getState(CardStateName.RightSplit).triggers); } card.updateTriggers(result, this); + cachedTraitTriggers = result; return result; } @@ -711,10 +736,14 @@ public final boolean hasTrigger(final int id) { } public final boolean addTrigger(final Trigger t) { + card.invalidateTraitCaches(); return triggers.add(t); } public final FCollectionView getStaticAbilities() { + if (cachedStaticAbilities != null) { + return cachedStaticAbilities; + } FCollection result = new FCollection<>(staticAbilities); if (getStateName().equals(CardStateName.Original)) { if (getCard().hasState(CardStateName.LeftSplit)) @@ -723,12 +752,15 @@ public final FCollectionView getStaticAbilities() { result.addAll(getCard().getState(CardStateName.RightSplit).staticAbilities); } card.updateStaticAbilities(result, this); + cachedStaticAbilities = result; return result; } public final boolean addStaticAbility(StaticAbility stab) { + card.invalidateTraitCaches(); return staticAbilities.add(stab); } public final boolean removeStaticAbility(StaticAbility stab) { + card.invalidateTraitCaches(); return staticAbilities.remove(stab); } @@ -963,6 +995,10 @@ public final void copyFrom(final CardState source, final boolean lki, final Card this.landManaAbilities.put(e.getKey(), e.getValue().copy(card, true)); } } + + // after the copy, not before: the trait copies above read back through this card, so an + // earlier drop could be refilled from a half-copied state + card.invalidateTraitCaches(); } public final void addAbilitiesFrom(final CardState source, final boolean lki) { @@ -993,6 +1029,8 @@ public final void addAbilitiesFrom(final CardState source, final boolean lki) { staticAbilities.add(sa.copy(card, lki)); } } + + card.invalidateTraitCaches(); } public CardState copy(final Card host, CardStateName name, final boolean lki) {