CardTrait caching

[Hanmac]

Card Traits are updated multiple times via

getChangedCardTraitsList and their functions like applyReplacementEffect

inside, ICardTraitChanges apply their stuff to all of the four categories. (+1 for Keywords?)

combine the apply function into one

and then store the result as cache like KeywordCache

[Hanmac]

@tool4ever @liamiak @MostCromulent no MR yet, just brainstorm for now

What I might do is adding an Interface, that can be implemented into CardState and CardTraitCache.
With that, if the card has no trait changes, and doesn't have a basic land type change (I'm looking at you, blood moon)
then the methods can just return the original Collections from CardState.

(plus leftState and rightState for split cards)

[tool4ever]

• there are multiple AI-based PR in progress with slightly different approaches
• ideally we can isolate the caches more cleanly somehow, then again interfaces can't have instance fields 🤔

[Hanmac]

there are multiple AI-based PR in progress with slightly different approaches

That's why i made this Issue to talk about the different approaches and what the best way would be.

then again interfaces can't have instance fields

The Cache itself will be a Record class of course, just sharing an Interface with CardState

[Hanmac]

Also, the cache might only apply before special rules apply

see

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

Lines 755 to 771 in 38ce75b

	if (!rulesHost) {
	return result;
	}
	// below are global rules
	if (type.hasSubtype("Adventure")) {
	if (this.adventureRep == null) {
	adventureRep = CardFactoryUtil.setupAdventureAbility(this);
	}
	result.add(adventureRep);
	}
	if (type.hasSubtype("Omen")) {
	if (this.omenRep == null) {
	omenRep = CardFactoryUtil.setupOmenAbility(this);
	}
	result.add(omenRep);
	}

and

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

Lines 7060 to 7065 in 38ce75b

	if (!rulesHost) {
	return;
	}
	// Shield Counter aren't affected by Changed Card Traits
	if (getCounters(CounterEnumType.SHIELD) > 0) {

So for the RulesHost check to apply, the Cache might need to be before these are added.

And these:

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

Lines 733 to 751 in 38ce75b

	CardTypeView type = getTypeWithChanges();
	if (type.isPlaneswalker()) {
	if (loyaltyRep == null) {
	loyaltyRep = CardFactoryUtil.makeEtbCounter("etbCounter:LOYALTY:" + this.baseLoyalty, this, true);
	}
	result.add(loyaltyRep);
	}
	if (type.isBattle()) {
	if (defenseRep == null) {
	defenseRep = CardFactoryUtil.makeEtbCounter("etbCounter:DEFENSE:" + this.baseDefense, this, true);
	}
	result.add(defenseRep);
	}
	if (type.isSaga() && !hasKeyword(Keyword.READ_AHEAD)) {
	if (sagaRep == null) {
	sagaRep = CardFactoryUtil.makeEtbCounter("etbCounter:LORE:1", this, true);
	}
	result.add(sagaRep);
	}

might be added to LandTraitChanges (with changed Name?), but then it can't be a record anymore? 🤔
I mean, yes, the abilities should only be added if the Type is right, but there is nothing that turns something into a Planeswalker, Battle or Saga?

[tool4ever]

hmn? I can always clone a Planeswalker creature...

[Hanmac]

hmn? I can always clone a Planeswalker creature...

Yes, but that will create a new CardState that has always been a Planeswalker (creature)
i might add it to #11389 to show what i mean

Edit:
Kinda an Init Problem, right now, i init LandTraitChanges as property.
but if i want to init the Planeswalker/Battle/Saga replacement effects there too,
i need to access the baseLoyalty and baseDefense after it has been set by the CardFactory/Copy Effect.

Edit:
But Suppliers.memoize could work

Edit:
Suppliers.memoize doesn't work as i want, because these RE are copied as part of LKI:

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

Lines 932 to 940 in cc741e0

	if (source.loyaltyRep != null) {
	loyaltyRep = source.loyaltyRep.copy(card, true);
	}
	if (source.defenseRep != null) {
	defenseRep = source.defenseRep.copy(card, true);
	}
	if (source.sagaRep != null) {
	sagaRep = source.sagaRep.copy(card, true);
	}