xmage handling of Auras differs from the CR and is incompatible with rollbacks
- Dominant language
- Java
- Stars
- 2.4k
- Forks
- 940
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 160
Description
The following are the Comprehensive Rules for the enchant keyword: (2022/09/08)
> 702.5. Enchant
> 702.5a Enchant is a static ability, written “Enchant [object or player].” The enchant ability restricts what an Aura spell can target and what an Aura can enchant.
> 702.5b For more information about Auras, see rule 303, “Enchantments.”
> 702.5c If an Aura has multiple instances of enchant, all of them apply. The Aura’s target must follow the restrictions from all the instances of enchant. The Aura can enchant only objects or players that match all of its enchant abilities.
In xmage, the `EnchantAbility` class is completely cosmetic. Every place where the engine wants to know whether an Aura can become attached or remain attached to an object, it does so by extracting a `Target` from the Aura's `SpellAbility`. Besides being inaccurate to the CR, this has a number of practical consequences:
- Some spells have multiple `SpellAbility`s (for alternate casting methods like bestow), and tokens do not normally have one at all (because they are never cast). `GameImpl.checkStateBasedActions` actually has to do a whole lot of ugly blind groping to find the correct ability to extract a `Target` from.
- If a continuous effect wants to turn a permanent into an Aura, or replace an Aura's enchant ability with a different one, it can't simply add or replace the `EnchantAbility`, because that ability *doesn't do anything* except display text. It has to modify the permanent's `SpellAbility`. For reasons that I admit I don't fully understand, this modification is not stable across `GameState` restores (which include but are by no means limited to player-requested rollbacks). One symptom of this is that Animate Dead and Dance of the Dead often spontaneously fall off for no apparent reason (#7228, #7794, #9420). Via debugging I have confirmed that this is caused by `checkStateBasedActions` checking the permanent the Aura is attached to against the spell's original "card in a graveyard" target.
- The only reason the previous bug doesn't affect a lot more cards is that if `checkStateBasedActions` can't find a `Target` at all it logs an error in the server log but allows the Aura to remain attached to whatever it's currently attached to. There are a lot more cards in Magic that start as non-Aura permanents with no target and turn into Auras than that start as one kind of Aura and turn into a different one.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.