Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Rage Extractor's Phyrexian mana symbol letter code #12612

Merged
merged 2 commits into from
Jul 30, 2024

Conversation

jimga150
Copy link
Contributor

case "P":
symbol = "p";
case "H":
symbol = "h";
Copy link
Contributor

Choose a reason for hiding this comment

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

is this actually a valid symbol in gatherer now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A couple people here said we should be good to do this before wizards gets this symbol updated. As of now, {P} leads to the pawprint symbol, and {H} is not a valid link.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, this change doesn't depend on gatherer because scryfall has it, but if the gatherer part isn't working, then best to add a comment here

@@ -23,7 +23,7 @@
*/
public final class RageExtractor extends CardImpl {

private static final FilterSpell filter = new FilterSpell("a spell with {P} in its mana cost");
private static final FilterSpell filter = new FilterSpell("a spell with {H} (<i>a phyrexian mana symbol</i>) in its mana cost");
Copy link
Contributor

Choose a reason for hiding this comment

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

I see the merit in this, but I'm not convinced we should be adding reminder text that doesn't exist. At least put the parentheses inside the italics

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe i'll take it out--not worth tracking so closely for one card i suppose

@xenohedron xenohedron merged commit 82a7769 into magefree:master Jul 30, 2024
4 checks passed
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.

2 participants