Skip to content

Do not generate a new ID when playing mana abilities#14822

Merged
xenohedron merged 1 commit intomagefree:masterfrom
matoro:tyvar
May 2, 2026
Merged

Do not generate a new ID when playing mana abilities#14822
xenohedron merged 1 commit intomagefree:masterfrom
matoro:tyvar

Conversation

@matoro
Copy link
Copy Markdown
Contributor

@matoro matoro commented May 1, 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

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: magefree@b814837eb8
See: magefree#14382
See: magefree#14381
Copy link
Copy Markdown
Contributor

@Grath Grath left a comment

Choose a reason for hiding this comment

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

LGTM

@xenohedron xenohedron merged commit f1aacb5 into magefree:master May 2, 2026
3 of 5 checks passed
int bookmark = game.bookmarkState();
//20260116 - 109.4a
//The controller of a mana ability is determined as though it were on the stack.
ability.newId();
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 must comment newId and add docs about it like

//do not generate new id cause it can break mana events, see #14822
//ability.newId();

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.

added docs in 39fe545

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.

4 participants