Skip to content

Mana abilities should be controlled by their activator#14382

Merged
xenohedron merged 1 commit intomagefree:masterfrom
matoro:manacache
Feb 26, 2026
Merged

Mana abilities should be controlled by their activator#14382
xenohedron merged 1 commit intomagefree:masterfrom
matoro:manacache

Conversation

@matoro
Copy link
Copy Markdown
Contributor

@matoro matoro commented Feb 4, 2026

Per rule 113.8, the controller of an activated ability on the stack is the player who activated it. This matters for cards with activated abilities which specify they can be activated by players other than the controller of the permanent, e.g. [[Mana Cache]] which will add mana to its controller's mana pool on activation.

@matoro
Copy link
Copy Markdown
Contributor Author

matoro commented Feb 4, 2026

Fixes #14381

@matoro matoro marked this pull request as ready for review February 4, 2026 20:02

protected Player getPlayer(Game game, Ability source) {
//20260116 - 113.8
// The controller of an activated ability on the stack is the player who activated it.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this code should be part of ManaEffect rather than ActivatedAbilityImpl. Seems like you've uncovered a deeper bug in the engine?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very strange. It's not a fix. XMage setup activation player before playable check, ability resolve or effect creation. There are other abilities with opponent's activation and it works fine. Need research, maybe original bug somewhere else (if you can't choose something then it's targeting code bug -- some of it can miss a code to find real player).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are other effects that try similar workaround. It may not be the right fix, but there's likely an existing underlying problem. Example:

if (source instanceof ActivatedAbilityImpl) {
playerId = ((ActivatedAbilityImpl) source).getActivatorId(); // for Lethal Vapors
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then, isn't the problem ultimately that getControllerId() in Ability interface doesn't take a Game parameter, when according to the rules the controller of an ability may not necessarily be the same as the controller of the card in some scenarios?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would game param help?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

basically, the controller of an ability is fundamentally dependent on the game state

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xenohedron look at player's triggerAbility -- maybe it can have same setController logic as playAbility above -- and it can fix problems with triggers and old cards/topic.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shot_260207_212552

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, I see what you're talking about. it looks like this is an issue because Mana Cache goes through playManaAbility() instead of playAbility(), which doesn't have the same stack logic there, presumably because of the "mana abilities don't use the stack" rule.

That makes sense, but in that case, what rule is is that justifies Mana Cache giving mana to the player that activated it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found this in the rules here:

109.4a The controller of a mana ability is determined as though it were on the stack.

So, we should have that same logic in playManaAbility(). However, that does not explain why the same workaround is necessary for other non-mana abilities like [[Lethal Vapors]].

Per rule 113.8, the controller of an activated ability on the stack is
the player who activated it.  Combined with rule 109.4a which states
that the controller of a mana ability is determined as if it was on the
stack, this means that we should apply all logic to mana abilities in
the same way as we do activated abilities on the stack, such as setting
their controller.
@matoro matoro changed the title Activated abilities should be controlled by their activator Mana abilities should be controlled by their activator Feb 12, 2026
@matoro matoro requested review from JayDi85 and xenohedron February 12, 2026 19:02
@xenohedron xenohedron merged commit b814837 into magefree:master Feb 26, 2026
3 checks passed
xenohedron pushed a commit that referenced this pull request May 2, 2026
It seems that `playManaAbility()` is called at a different time (earier)
than `playAbility()` and thus its events (`MANA_ADDED` etc) get the new
ID in them and not the old one, whereas this is not the case for events
fired by playing non-mana abilities.  Since no stack object is created
for mana abilities, generating a new ID should not be necessary.

Fixes: b814837eb8
See: #14382
See: #14381

Co-authored-by: matoro <matoro@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants