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

[don't merge] [NCC] Implement Henzie "Toolbox" Torre #12188

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

Conversation

alexander-novo
Copy link
Contributor

Building off the work done in #9463

The biggest issue that I've run into so far is that Commander objects only seem to only keep track of activated abilities that were printed on the card - so any activated abilities (such as blitz) added to the commander during the game aren't respected. I've updated this, but I'm not confident in it. Let me know your opinions.

Currently, the biggest blocker is casting other players' cards - but there is a comment in the code saying that this depends on #9521 which is not currently merged. Does anyone know the status of this?

@alexander-novo
Copy link
Contributor Author

I haven't tried playing with him a bunch, I'll do that a bit and report back if I see anything weird beyond what I've mentioned above.

@JayDi85 JayDi85 changed the title [NCC] Implement Henzie "Toolbox" Torre [don't merge] [NCC] Implement Henzie "Toolbox" Torre Apr 28, 2024
@JayDi85
Copy link
Member

JayDi85 commented Apr 28, 2024

See some problems with wrong abilities usage, will review later with details.

@alexander-novo
Copy link
Contributor Author

Some thoughts:

  • Currently, if you cast a spell for the blitz cost given by Henzie and sacrifice Henzie as part of the cost (such as with [[Phyrexian Tower]]), the resulting spell doesn't have Blitz and won't get haste or the sac trigger. I don't know if this is correct.
  • I haven't tested what happens if you cast a spell for its blitz cost given by Henzie and then sacrifice Henzie before the spell resolves - I suspect that this will also cause the resulting creature to not get haste or a sac trigger.
  • If the above interactions are correct, I'm not sure what would happen if you cast a creature that already has blitz for the blitz cost given to it by Henzie, and then sacrifice Henzie.

I'll write up tests for these later.

Copy link

Phyrexian Tower - (Gatherer) (Scryfall) (EDHREC)

Legendary Land
{T}: Add {C}.
{T}, Sacrifice a creature: Add {B}{B}.

@alexander-novo
Copy link
Contributor Author

Another thought: check if opponents who can play your cards can use blitz cost. If so, we might have to rethink how alternate casting costs are given to spells...

@JayDi85
Copy link
Member

JayDi85 commented May 2, 2024

@Susucre please help alex to rebase or create new PR. Current PR contains dirty commits and can’t be reviewed before merge.

@alexander-novo
Copy link
Contributor Author

Sorry about that - I did a rebase and accidentally pulled instead of force-pushing. Should be better now.

- Give Blitz to spells on the stack, in addition to the cards. This allows creature to come in with proper haste and delayed trigger.
- Fixed cost reduction test (this was always working - the test just didn't have enough mana to cast)
- `Commander` objects now load abilities from the current game when a player is presented options for abilities to activate. This was not previously the case, so any `SpellAbility` added to a commander in the command zone (such as with Henzie) would not be playable by players.
- Added some more tests
@alexander-novo
Copy link
Contributor Author

I also ended up removing the changes to the rad counter test I inadvertently pushed

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.

None yet

5 participants