Skip to content

StaticAbilityManaRestriction: better ability for Myr Superion - #11700

Open
Hanmac wants to merge 5 commits into
masterfrom
spellManaRestriction
Open

StaticAbilityManaRestriction: better ability for Myr Superion#11700
Hanmac wants to merge 5 commits into
masterfrom
spellManaRestriction

Conversation

@Hanmac

@Hanmac Hanmac commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Closes #11689

@tool4ever tool4ever left a comment

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.

no sorry, I think that's a terrible idea:
a) just introduces a secondary way since you can't even remove the ManaRestriction$
b) more importantly we're spending hours thinking about CardTraits caching to speed up AI turns - this just introduces another expensive check in a place like allowsPayingWithShard which will get called multiple times per mana source and per SA every priority window...all that for a tiny refactor of 3 cards?! 👎

@tool4ever
tool4ever requested a review from tehdiplomat August 26, 2026 06:44
@Hanmac

Hanmac commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

@tool4ever
You caused the problem with "Text" on NoAbilities

@tool4ever

Copy link
Copy Markdown
Contributor

Yea whatever, I can assure you the old logic was way worse before my refactor here: eb6d168
I worked with that was available at that time and I certainly didn't introduce these hardscripted spell versions: a80396a
🤷‍♂️

@Hanmac

Hanmac commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

@tool4ever this should reduce the amount of StaticAbility calls

a) just introduces a secondary way since you can't even remove the ManaRestriction$

This is because these 3 are done as Static Ability, while the other are part of Activated Ability or Alternative Cost.
Different things so they need to be implemented differently.

@tool4ever

Copy link
Copy Markdown
Contributor

Hmn, I need to think if we can find a way to support Throne of Eldraine too?
Like mapping the affected to a specific ability, might be other cases like that? 🤔
maybe reuse logic of #8464?

@Hanmac

Hanmac commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Throne of Eldraine
Draw two cards. Spend only mana of the chosen color to activate this ability.

This is part as ONE Ability, so I don't think we should split it.
This matters if something else gets this activated ability.

@tool4ever

Copy link
Copy Markdown
Contributor

I still want to check some alternatives, maybe storing it inside ManaConversionMatrix would be better since that gets filled for each SA anyway...

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.

Myr Superion and similar cause problems with noAbility

2 participants