From c1735a900e2d0a7e2949a45e96ddbd4298e8343c Mon Sep 17 00:00:00 2001 From: liamiak Date: Sat, 22 Aug 2026 20:57:44 -0600 Subject: [PATCH] Cache the per-card trigger and static ability lists getTriggers() and getStaticAbilities() rebuild their result from the layer system on every call, and the rebuilt list is identical to the previous one over 99% of the time. Cache each list on the CardState and drop it whenever an input changes. Co-Authored-By: Claude Opus 5 --- .../src/main/java/forge/game/card/Card.java | 24 ++++++++++++ .../main/java/forge/game/card/CardState.java | 38 +++++++++++++++++++ 2 files changed, 62 insertions(+) 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 5249b18cd2aa..fb1ea002d228 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 58f519285316..aa7cf6737a90 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) {