Set fromZone for copied spells to stack#14803
Conversation
|
|
||
| // copy spell | ||
| Spell spellCopy = new Spell(copiedPart, this.ability.copySpell(this.card, copiedPart), this.controllerId, this.fromZone, game, true); | ||
| Spell spellCopy = new Spell(copiedPart, this.ability.copySpell(this.card, copiedPart), this.controllerId, Zone.STACK, game, true); |
There was a problem hiding this comment.
PR's use case with [[Silverquill, the Disputant]] and [[Transpose]] depends on different rule part (copied spell is not casted -- it's not related to the zone lookup by paper rules).
The main problem how's xmage track down the cast status. If you search related cards like "this spell was cast" then can find two workarounds in different cards:
spell.isCopy()spell.fromZone
Maybe there are another use cases and track down solutions like cast spell watchers, need better research.
So the real fix must look like:
set fromZone to Zone.STACK with rules comment:
// set fromZone to stack cause it's not casted and cause outside zone can be used in normal cast
// 707.10. To copy a spell, activated ability, or triggered ability means to put a copy of it onto the stack;
// a copy of a spell isn't cast and a copy of an activated ability isn't activated.
Spell spellCopy = new Spell(copiedPart, this.ability.copySpell(this.card, copiedPart), this.controllerId, Zone.STACK, game, true);- search "this spell was cast" and keep only fromZone check, remove spell.isCopy() usage
- search spell.isCopy() and replace by zone lookup
- or better: add spell's method like
wasCast(fromZone = null)and use it all around insteadfromZoneandisCopy
There was a problem hiding this comment.
I am a little bit confused, I implemented option 1 exactly as described here. Do you mean you just want me to add a comment?
There was a problem hiding this comment.
I think that the issue is that some card implementations put in a manual workaround and, if we're fixing the source problem, we should also remove those manual checks for "is not a copy" as well. Like, [[Approach of the Second Sun]] has a specific "is not a copy" check and there's probably others.
There was a problem hiding this comment.
alright, I went ahead and added wasCast() and replaced it in as many cards as I could find
Spells on the stack seem to use a separate `fromZone` member variable to track where they were cast from, independent of the zone of the card or the object. This should be unconditionally set to the stack for copied spells rather than inheriting the zone of the parent spell. This sets it in the constructor rather than `Spell.setZone()` to keep the field `final`.
Adds a clearer `Spell.wasCast()` to check if a spell was cast rather than checking `!StackObject.isCopy()`.
Spells on the stack seem to use a separate
fromZonemember variable to track where they were cast from, independent of the zone of the card or the object. This should be unconditionally set to the stack for copied spells rather than inheriting the zone of the parent spell.This sets it in the constructor rather than
Spell.setZone()to keep the fieldfinal.