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

[SLD] Implement Captain America, First Avenger #13023

Merged
merged 6 commits into from
Oct 24, 2024

Conversation

Grath
Copy link
Contributor

@Grath Grath commented Oct 22, 2024

I made assumptions that WotC is going to fix this card to actually work by adding "choose the equipment you're unattaching with Throw..." (in some way) to rule 601.2b since you have to choose a TargetAnyTargetAmount in steps 601.2c/601.2d long before you actually pay the unattach cost in 601.2h; the way I implemented it, you're technically choosing the equipment at the beginning of 601.2c instead of during 601.2b but it's functionally identical.

I made assumptions that WotC is going to fix the rules by adding "choose the equipment you're unattaching with Throw..." to rule 601.2b so that this card actually functions since you have to choose a TargetAnyTargetAmount in steps 601.2c/601.2d long before you actually pay the unattach cost in 601.2h; the way I implemented it, you're technically choosing the equipment at the beginning of 601.2c instead of during 601.2b but it's functionally identical.
@JayDi85 JayDi85 self-requested a review October 22, 2024 18:48
@xenohedron xenohedron self-requested a review October 22, 2024 18:54
@JayDi85 JayDi85 changed the title [SLD] Implement Captain America, First Avenger [don’t merge] [SLD] Implement Captain America, First Avenger Oct 22, 2024
@magefree magefree deleted a comment from github-actions bot Oct 22, 2024
@JayDi85
Copy link
Member

JayDi85 commented Oct 22, 2024

Targets workaround looks strange. There are dozen cards with same effects: https://scryfall.com/search?q=o%3A%22divided+as+you+choose+among%22&unique=cards&as=grid&order=name

Example: [[Arc Mage]] or [[Infernal Harvest]]

Need some research, looks like game engine need some improves to fully support early choices due rules. Or something wrong with card implementation (cost usage and choice propagation for a target like X values).

Copy link

Arc Mage - (Gatherer) (Scryfall) (EDHREC)

{2}{R}
Creature — Human Spellshaper
2/2
{2}{R}, {T}, Discard a card: Arc Mage deals 2 damage divided as you choose among one or two targets.

Infernal Harvest - (Gatherer) (Scryfall) (EDHREC)

{1}{B}
Sorcery
As an additional cost to cast this spell, return X Swamps you control to their owner's hand.
Infernal Harvest deals X damage divided as you choose among any number of target creatures.

@JayDi85 JayDi85 self-assigned this Oct 22, 2024
@Grath
Copy link
Contributor Author

Grath commented Oct 22, 2024

That's false, there is one card with a similar effect to this one and it's [[Nahiri's Sacrifice]] which uses "declare the value of X in step 601.2b" to be able to know how much damage is being divided and how many targets you can target.

Copy link

Nahiri's Sacrifice - (Gatherer) (Scryfall) (EDHREC)

{1}{R}
Sorcery
As an additional cost to cast this spell, sacrifice an artifact or creature with mana value X.
Nahiri's Sacrifice deals X damage divided as you choose among any number of target creatures.

@Grath
Copy link
Contributor Author

Grath commented Oct 22, 2024

Options we have:

  1. Dig through the guts of AbilityImpl to do a functionally equivalent one-off engine-level workaround to add "choose the equipment you're going to unattach early" if there's a CaptainAmericaFirstAvengerUnattachCost present in the ability.
  2. This target workaround, where instead of doing it during 601.2b handling we do it at the beginning of 601.2c handling.

…t targets early using inheritance to avoid having a horrific brittle list of 'these costs must be paid early'.
@Grath Grath changed the title [don’t merge] [SLD] Implement Captain America, First Avenger [SLD] Implement Captain America, First Avenger Oct 22, 2024
@Grath
Copy link
Contributor Author

Grath commented Oct 22, 2024

Targets workaround looks strange. There are dozen cards with same effects: https://scryfall.com/search?q=o%3A%22divided+as+you+choose+among%22&unique=cards&as=grid&order=name

Example: [[Arc Mage]] or [[Infernal Harvest]]

Need some research, looks like game engine need some improves to fully support early choices due rules. Or something wrong with card implementation (cost usage and choice propagation for a target like X values).

Workaround removed, did the engine improvement to support costs which must be chosen during 601.2b

Copy link
Contributor

@xenohedron xenohedron left a comment

Choose a reason for hiding this comment

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

I have a few minor comments here but the overall direction with EarlyTargetCost looks good to me.

@Grath
Copy link
Contributor Author

Grath commented Oct 24, 2024

@JayDi85 I'm going to merge this once Travis passes, because I have removed the workaround which you objected to when you self-assigned it.

@Grath Grath merged commit f7f2d58 into magefree:master Oct 24, 2024
3 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.

3 participants