magefree
Bug: Cost Reduction/Permission is using wrong player when card is not cast by its owner
#11165 uncovered a bunch of bugged interaction between cards that allow to cast cards owned by another player (like [[Gonti, Lord of Luxury]]) and cost reduction/permission that is checking/counting something depending on who the caster is.
Some of the impacted cards:
- cost modification (variable or not) depending on who "you" is: [[The Great Henge]] [[Ghalta, Primal Hunger]] and other. But [[Academy Journeymage]] is also not checking if there is a wizard for the caster, but the card's owner instead.
- restriction that care about what you control/own. For instance the legendary sorcery/instant can not currently be cast with Gonti if the card's owner does not control a legend.
A few of other cards that were confirmed bugged [[Animist's Might]] [[Blizzard]] [[Jaya's Immolating Inferno]] A few I did not test, but no reason to suspect they are not bugged too: [[Bedlam Reveler]] [[Dargo, the Shipwrecker]] [[Korvold, Gleeful Glutton]] [[Serra Avenger]]
Gonti, Lord of Luxury - (Gatherer) (Scryfall) (EDHREC)
{2}{B}{B} Legendary Creature — Aetherborn Rogue 2/3 Deathtouch When Gonti, Lord of Luxury enters the battlefield, look at the top four cards of target opponent's library, exile one of them face down, then put the rest on the bottom of that library in a random order. You may cast that card for as long as it remains exiled, and mana of any type can be spent to cast it.
The Great Henge - (Gatherer) (Scryfall) (EDHREC)
{7}{G}{G} Legendary Artifact This spell costs {X} less to cast, where X is the greatest power among creatures you control. {T}: Add {G}{G}. You gain 2 life. Whenever a nontoken creature enters the battlefield under your control, put a +1/+1 counter on it and draw a card.
Ghalta, Primal Hunger - (Gatherer) (Scryfall) (EDHREC)
{10}{G}{G} Legendary Creature — Elder Dinosaur 12/12 This spell costs {X} less to cast, where X is the total power of creatures you control. Trample
Academy Journeymage - (Gatherer) (Scryfall) (EDHREC)
{4}{U} Creature — Human Wizard 3/2 This spell costs {1} less to cast if you control a Wizard. When Academy Journeymage enters the battlefield, return target creature an opponent controls to its owner's hand.
Animist's Might - (Gatherer) (Scryfall) (EDHREC)
{2}{G} Sorcery This spell costs {2} less to cast if it targets a legendary creature you control. Target creature you control deals damage equal to twice its power to target creature or planeswalker you don't control.
Blizzard - (Gatherer) (Scryfall) (EDHREC)
{G}{G} Enchantment Cast this spell only if you control a snow land. Cumulative upkeep {2} (At the beginning of your upkeep, put an age counter on this permanent, then sacrifice it unless you pay its upkeep cost for each age counter on it.) Creatures with flying don't untap during their controllers' untap steps.
Bedlam Reveler - (Gatherer) (Scryfall) (EDHREC)
{6}{R}{R} Creature — Devil Horror 3/4 This spell costs {1} less to cast for each instant and sorcery card in your graveyard. Prowess When Bedlam Reveler enters the battlefield, discard your hand, then draw three cards.
Dargo, the Shipwrecker - (Gatherer) (Scryfall) (EDHREC)
{6}{R} Legendary Creature — Giant Pirate 7/5 As an additional cost to cast this spell, you may sacrifice any number of artifacts and/or creatures. This spell costs {2} less to cast for each permanent sacrificed this way and {2} less to cast for each other artifact or creature you've sacrificed this turn. Trample Partner (You can have two commanders if both have partner.)
Korvold, Gleeful Glutton - (Gatherer) (Scryfall) (EDHREC)
{5}{B}{R}{G} Legendary Creature — Dragon Noble 4/4 This spell costs {1} less to cast for each card type among permanents you've sacrificed this turn. Flying, trample, haste Whenever Korvold deals combat damage to a player, put X +1/+1 counters on Korvold and draw X cards, where X is the number of permanent types among cards in your graveyard.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Serra Avenger - (Gatherer) (Scryfall) (EDHREC)
{W}{W} Creature — Angel 3/3 You can't cast this spell during your first, second, or third turns of the game. Flying, vigilance
Jaya's Immolating Inferno - (Gatherer) (Scryfall) (EDHREC)
{X}{R}{R} Legendary Sorcery (You may cast a legendary sorcery only if you control a legendary creature or planeswalker.) Jaya's Immolating Inferno deals X damage to each of up to three targets.
[[The Skullspore Nexus]] and [[The Great Henge]] will have the correct behavior next update. It is actually a regression on the great henge with V6-beta2 to be wrong. Bug was reintroduced in an attempt to simplify the code, and we lacked an unit test there.
The Skullspore Nexus - (Gatherer) (Scryfall) (EDHREC)
{6}{G}{G} Legendary Artifact This spell costs {X} less to cast, where X is the greatest power among creatures you control. Whenever one or more nontoken creatures you control die, create a green Fungus Dinosaur creature token with base power and toughness each equal to the total power of those creatures. {2}, {T}: Double target creature's power until end of turn.
The Great Henge - (Gatherer) (Scryfall) (EDHREC)
{7}{G}{G} Legendary Artifact This spell costs {X} less to cast, where X is the greatest power among creatures you control. {T}: Add {G}{G}. You gain 2 life. Whenever a nontoken creature enters the battlefield under your control, put a +1/+1 counter on it and draw a card.
Uhm I may have found some origin for those kind of bugs.
On computing the modification in an apply (like here SpellCostReductionSourceEffect), dynamic value should be computed with abilityToModify in the second parameter, not source:
I have not investigated fully, but I think source.getControllerId() is the owner's UUID, while we want to look for the player attempting to cast the card through abilityToModify.getControllerId(). Conditions are similarly applied to the source, not the abilityToModify, so may be wrong too.
Nope, all dynamic value calculations go from source's controller side -- it's correct.
@JayDi85 well something is wrong somewhere.
Try TheSkullsporeNexusTest::test_costreduction_opp_card with a stop in TheSkullsporeNexusReductionEffect::apply ; the second time we go through it, here are the parameters:
source (the controller Id is the card's owner):
abilityToModify (the controller Id is the one that stole the card in the test, and is attempting a cast):
If I wanted to replace the reduction computation by a dynamic value with an inner controlled filter, the test would be broken with source.
It might be an issue with how source is set up above.
Stack trace if that helps:
Okay, thinking more about it, source is right. But there is an issue with cards with cost modification/condition on them, when they are not cast by their owner, as the modifier/condition has not the right controllerId. When they are collected, they are not properly set in ContinuousEffects->costModification:
I am not sure if the engine should copy + set controller of the abilities on the same card that the ability being modified there, or if the issue is upstream in CostModificationEffect maintenance when someone is attempting to cast a card they don't own.
Looks like something wrong with linked abilities -- cause source's controller must be changed too, but it doesn't -- "source" must be also controlled by player A in that example:
Controller changes on cast -- it also try to change subabilities (linked abilities) too:
It must temporary change and restore controller of all source object's abilities, not casting only -- like that:
I'm talking about code like that, but with try/finally and runtime checks for same controllerId in all abilities (to find out bad use cases with diff controllersId in diff abilities of the same card -- is it possible or not -- I know there are possible cards that allow to activate ability by opponent):
That code example works fine in your use case and other tests but need deeper research for same places and potential side effects (can it lost data consistence or it's safe for any code conditions/restore/rollback).
Or maybe you don't need to restore it at all -- e.g. "if player cast card then all card's abilities become under their control too" -- so it can be added in card's cast method like that (also works fine in your use case and current tests and also require some research for side effects/tests):