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 Captain Vargus Wrath to use CommanderCastFromCommandZoneValue #12324

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

grimreap124
Copy link
Contributor

No description provided.

@github-actions github-actions bot added the cards label May 28, 2024
@xenohedron
Copy link
Contributor

Subtle difference between "cast a commander from the command zone this game" and "cast your commander from the command zone this game" - does it matter for anything?

@grimreap124
Copy link
Contributor Author

CommanderCastFromCommandZoneValue tracks all commanders so maybe should be worded "cast a commander from the command zone this game"

@xenohedron
Copy link
Contributor

Well you can't adjust the wording to fit the effect logic. You have to look at the wording for each card used and match that.

If there is a distinction, need to maintain logic for each.
If there is no functional difference, then just need a param for text (e.g. two enum types)

@grimreap124
Copy link
Contributor Author

I can't find a difference in the rules so I guess we need the param.
Do you know of an example I can look at to add the 2 wording options?

@JayDi85
Copy link
Member

JayDi85 commented Jun 3, 2024

It's ok and used in other cards:
shot_240603_061042

@grimreap124
Copy link
Contributor Author

Looks like in xmage its only used in these cards

src/mage/cards/t/ThunderclapDrake.java
src/mage/cards/j/JyotiMoagAncient.java
src/mage/cards/c/CommandersInsignia.java

@JayDi85
Copy link
Member

JayDi85 commented Jun 3, 2024

Hmm. CommanderCastFromCommandZoneValue is duplicated and must be replaced/combined with standard CommanderCastCountValue:

shot_240603_061834

@grimreap124
Copy link
Contributor Author

Did you want me to merge them here or just leave this for now?

@JayDi85
Copy link
Member

JayDi85 commented Jun 3, 2024

Yes, you can do it here or other PR (just make sure it’s really same - I can see only card hints text diff, but it’s fine).

@github-actions github-actions bot added the engine label Jun 7, 2024
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