-
Notifications
You must be signed in to change notification settings - Fork 44
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
Add currently acceptable(DM-07) promo exclusives. Fix Cosmic Nebula and Xeno Mantis. #293
Conversation
game/cards/promo/chimera.go
Outdated
"Destroy one of your opponent's creatures", | ||
1, | ||
1, | ||
false).Map(func(x *match.Card) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This card is a may destroy
. Should allow the select to be canceled as currently it's enforcing destruction.
game/cards/promo/demon_command.go
Outdated
c.ManaCost = 7 | ||
c.ManaRequirement = []string{civ.Darkness} | ||
|
||
c.Use(fx.Creature, fx.Doublebreaker, func(card *match.Card, ctx *match.Context) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be refactored into fx.When(fx.AnotherOwnCreatureDestroyed, fx.MayUntapSelf)
MayUntapSelf already exists
AnotherOwnCreatureDestroyed, doesn't exist, but there is AnotherCreatureDestroyed, so it could use it in it's implementation.
When it come to When
, the idea for the future is that when all cards with trigger effects implement When/WhenAll
func, it will be a good candidate to refactor to better handle multiple trigger effects happening at the same time, by separately handling the validation step and the effect step (which this method already separates). That's also why with all the new cards added I also slowly refactored many of the existing cards to use When.
game/cards/promo/angel_command.go
Outdated
} | ||
event.Blockers = newBlockersList | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't work as expected with cards that give blocker e.g. Full Defensor. I think this requires implementing ConditionalBlocker (akin to ConditionalSlayer). And then when the blockers list is assembled for AttackCreature and AttackPlayer it should handle there the ConditionalBlocker.
If this change is done, it should be applied to Lurking Eel as well, which also has the same issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit tricky. Conditional Blockers are still considered blockers, i.e., are effected by cards like Critical Blade. We'd need to refactor a lot to make it work as expected.
game/cards/promo/demon_command.go
Outdated
} | ||
event.Blockers = newBlockersList | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as for AmnisHolyElemental
No description provided.