Card-Forge / forge

An unofficial rules engine for the world's greatest card game.
https://card-forge.github.io/forge/
GNU General Public License v3.0
935 stars 547 forks source link

Svars for Card should use Timestamp #362

Closed Hanmac closed 2 years ago

Hanmac commented 2 years ago

Because setting SVar to Cards should not be needed anymore except for AI, it would cleanup better to use a timestamp instead of setting them directly.

All Effects that set SVar should try to set it to the CardTrait instead and let it handle instead via SpellAbility parent for SubAbilities

Hanmac commented 2 years ago
Hanmac commented 2 years ago

need to check if this part is still needed, or if that can be removed:

} else if (mutableKeys.contains(key)) {
    // follow SVar and change it
    final String originalSVarValue = hostCard.getSVar(value);
    hostCard.changeSVar(value, AbilityUtils.applyAbilityTextChangeEffects(originalSVarValue, this));
    newValue = null;
}

this shows it is only for AddAbility:

private static final ImmutableList<String> mutableKeys = ImmutableList.<String>builder()
            .add("AddAbility").build();

and that is only used for StaticAbilities, and that does go through AbilityUtils.getSVar, which does call applyAbilityTextChangeEffects anyway.

the whole Params for StaticAbilities (or Pump or Animate) what does refer to Add Card Traits from SVars should probably be all to the noChangeKeys list

if Svars would be changed by CardTextChanges, probably use timestamp table too?