magefree / mage

Magic Another Game Engine
http://xmage.today
MIT License
1.91k stars 782 forks source link

NotManaAbilityPredicate must be removed as useless #12827

Open JayDi85 opened 2 months ago

JayDi85 commented 2 months ago

NotManaAbilityPredicate uses stack objects. It's useless cause mana abilities don't use stack by mtg rules and game engine. So it must be removed and used cards must be checked -- is it really work as intended. Original discussion from https://github.com/magefree/mage/pull/12597#discussion_r1727818414

6 affected cards like [[Battlemage's Bracers]]: shot_240908_113226

github-actions[bot] commented 2 months ago

Battlemage's Bracers - (Gatherer) (Scryfall) (EDHREC)

{2}{R} Artifact — Equipment Equipped creature has haste. Whenever an ability of equipped creature is activated, if it isn't a mana ability, you may pay {1}. If you do, copy that ability. You may choose new targets for the copy. Equip {2}

ssk97 commented 1 month ago

Sorry for the delay on this, it led me down a bit of a rabbit hole with [[Magus Lucea Kane]]'s interaction with mana abilities that have an {X} cost, namely [[Wizard's Rockets]]. It seems like the correct behavior would be for the Magus trigger to go on the stack and to successfully copy the mana ability. Obviously this does not currently work in XMage, as the copying is currently designed to only copy stack objects.

NotManaAbilityPredicate is currently useless and can be safely removed, but it seems to me like a rework which would correct the above interaction would likely require re-introducing it (in a somewhat modified form). As such, I'm leaning towards leaving it as-is beyond adding a comment explaining the situation. Is that acceptable, or should I remove it anyway?

github-actions[bot] commented 1 month ago

Magus Lucea Kane - (Gatherer) (Scryfall) (EDHREC)

{1}{G}{U}{R} Legendary Creature — Human Tyranid Wizard 1/1 Spiritual Leader — At the beginning of combat on your turn, put a +1/+1 counter on target creature. Psychic Stimulus — {T}: Add {C}{C}. When you next cast a spell with {X} in its mana cost or activate an ability with {X} in its activation cost this turn, copy that spell or ability. You may choose new targets for the copy. (A copy of a permanent spell becomes a token.)

Wizard's Rockets - (Gatherer) (Scryfall) (EDHREC)

{1} Artifact Wizard's Rockets enters tapped. {X}, {T}, Sacrifice Wizard's Rockets: Add X mana in any combination of colors. When Wizard's Rockets is put into a graveyard from the battlefield, draw a card.

JayDi85 commented 1 month ago

Mana abilities can’t be copied cause it doesn’t use a stack.

From https://www.cranial-insertion.com/article/4160

Q: If I control [[Unbound Flourishing]] and activate [[Wizard's Rockets]], does Unbound Flourishing double the amount of mana I get from the Rockets?

A: No. While activating Wizard's Rockets' ability triggers Unbound Flourishing's triggered ability, the resolution of that triggered ability does nothing. To copy an ability, you make a copy of an object on the stack. Since mana abilities don't use the stack, there is nothing to copy in the first place.

github-actions[bot] commented 1 month ago

Unbound Flourishing - (Gatherer) (Scryfall) (EDHREC)

{2}{G} Enchantment Whenever you cast a permanent spell with a mana cost that contains {X}, double the value of X. Whenever you cast an instant or sorcery spell or activate an ability, if that spell's mana cost or that ability's activation cost contains {X}, copy that spell or ability. You may choose new targets for the copy.

Wizard's Rockets - (Gatherer) (Scryfall) (EDHREC)

{1} Artifact Wizard's Rockets enters tapped. {X}, {T}, Sacrifice Wizard's Rockets: Add X mana in any combination of colors. When Wizard's Rockets is put into a graveyard from the battlefield, draw a card.

ssk97 commented 1 month ago

Q: If I control [[Unbound Flourishing]] and activate [[Wizard's Rockets]], does Unbound Flourishing double the amount of mana I get from the Rockets?

A: No. While activating Wizard's Rockets' ability triggers Unbound Flourishing's triggered ability, the resolution of that triggered ability does nothing. To copy an ability, you make a copy of an object on the stack. Since mana abilities don't use the stack, there is nothing to copy in the first place.

Huh, right. The ability does still trigger though, which means that Magus Lucea Kane's delayed ability should be used up.