[ICE] Implement Kjeldoran Guard #11184
Conversation
|
Kjeldoran Guard - (Gatherer) (Scryfall) (EDHREC)
|
| game.addEffect(buffEffect, source); | ||
|
|
||
| // When that creature leaves the battlefield this turn, sacrifice Kjeldoran Guard. | ||
| game.addDelayedTriggeredAbility( |
There was a problem hiding this comment.
Please test- if you flicker Kjeldoran Guard, shouldn't have to sacrifice it.
There was a problem hiding this comment.
oh right that should be using MOR
There was a problem hiding this comment.
Actually that was already correct. SacrificeSourceEffect does sacrifice source.getSourceObjectIfItStillExists(game), so the Kjeldoran Guard with a different zcc will not get sacrificed (there still will be a trigger, although it does nothing).
I did replace the UUID of the target to a MOR, but that should not really matter with
- target being still valid means that it has not leave on resolve
- the
DelayedTriggerbeing set withtriggerOnce=true.
There was a problem hiding this comment.
Oh nevermind, there is indeed an issue if you blink the Guard in response to tapping it.
Will rework with a MOR based SacrificeTargetEffect
Sure I'll do a cleaning pass. Not used to taking over someone else WIP. |
|
Uhm and on re reading it, the condition is not right, the ability should only be usable if the defending player controls no snow land, currently it is only usable if the defending player controls a snow land. |
| private KjeldoranGuard(final KjeldoranGuard card) { super(card); } | ||
|
|
||
| @Override | ||
| public Card copy() { |
| this.addAbility(ability); | ||
| } | ||
|
|
||
| private KjeldoranGuard(final KjeldoranGuard card) { super(card); } |
| super(Outcome.BoostCreature); | ||
| staticText = "Target creature gets +1/+1 until end of turn." | ||
| + "When that creature leaves the battlefield this turn, sacrifice {this}." | ||
| + "Activate only during combat and only if defending player controls no snow lands."; |
There was a problem hiding this comment.
I don't think the activation restriction should be part of the effect text
| KjeldoranGuardEffect() { | ||
| super(Outcome.BoostCreature); | ||
| staticText = "Target creature gets +1/+1 until end of turn." | ||
| + "When that creature leaves the battlefield this turn, sacrifice {this}." |
| } | ||
|
|
||
| @Override | ||
| public String getRule() { return "sacrifice {this}"; } |
There was a problem hiding this comment.
Hm, this doesn't feel quite right but not sure why, can you check the delayed trigger displays with correct text on the stack?
There was a problem hiding this comment.
I'm not sure we are displaying the trigger when those delayed trigger happens, is that was you feel is not right?
There was a problem hiding this comment.
Yeah ideally the trigger phrase should be part of it (but for sure lots of these have wrong text all over the codebase). So not required to fix, just a nice to have.
Also it doesn't get automatically capitalized which is a separate bug.
There was a problem hiding this comment.
I can replace it with "When that creature leaves the battlefield, sacrifice Kjeldoran Guard"?
There was a problem hiding this comment.
Should be "When that creature leaves the battlefield this turn, sacrifice {this}."
There was a problem hiding this comment.
Just for a discussion point, got a comment from theelk801 last month that reflexive triggers can have their trigger left out. #10866 (comment)
So is that the line? Delayed should have trigger text but not reflexive ones? Or should that be a standalone dev discussion outside of that poor Ice Age common implementation.
There was a problem hiding this comment.
That's a good question - my initial reaction is I disagree but I should double check how it works.
There was a problem hiding this comment.
It's more important to have clarity on delayed, the reflexive at least occur in close proximity to their source
Modified slightly the existing PR from #8593