Add life lost event and batch event to implement Ob Nixilis, Captive Kingpin #11974
Conversation
|
Writing the tests. Intended tests i plan to hit (Let me know if there should be more):
|
| .sum(); | ||
| } | ||
|
|
||
| public int getAmount(UUID targetID) { |
| @@ -808,6 +808,25 @@ public boolean hasSimultaneousEvents() { | |||
| return !simultaneousEvents.isEmpty(); | |||
| } | |||
|
|
|||
| public void addSimultaneousLifeLoss(LifeLostEvent lifeLossEvent, Game game) { | |||
There was a problem hiding this comment.
Current naming logic of all batch methods is wrong - it does not add real events to sim queue, only batches. So it must be renamed to addSimultaneousXxxEventToBatches - damaged, loseLife, tapped, etc.
There was a problem hiding this comment.
Going to rename just this function in this PR for now, unless youre good with renaming the rest in here.
| } | ||
|
|
||
| public boolean isCombatDamage() { | ||
| return events.stream().anyMatch(LifeLostEvent::isCombatDamage); |
There was a problem hiding this comment.
Can be wrong usage. Related card must use events list or use method like getLifeLoseByCombatDamage(isCombatDamage::Boolaean) > 0, not isCombatDamage.
|
|
||
| attack(2, playerD, "Memnite", playerA); | ||
|
|
||
| setStopAt(2, PhaseStep.END_TURN); |
There was a problem hiding this comment.
Miss setStrictChooseMode before setstop/execute. It allows to check commands order and missing commands. Must be used all the time.
|
|
||
| public class ObNixilisCaptiveKingpinTest extends CardTestCommander4Players { | ||
|
|
||
| @Test |
There was a problem hiding this comment.
Looks like a details list from your comment about possible use cases can be useful here (just copy as comment).
| // - 1 opponent loses 2 life -> No trigger | ||
| // - 2 opponents lose 1 life each -> Ob Nixilis triggers | ||
| // - 2 opponents lose 2 life each -> No trigger | ||
| // - controller loses 1 life -> No trigger |
There was a problem hiding this comment.
These are a good starting point. I'd like to see a couple more tests:
- opponent loses 1 life and another opponent simultaneously loses 2 life
- opponent loses 1 life and controller simultaneously loses 2 life
| playerId, source, playerId, amount, atCombat); | ||
| } | ||
|
|
||
| public boolean isCombatDamage() { |
There was a problem hiding this comment.
this should be renamed same as the batch event for clarity
(also... is there existing card that cares about it? if not, maybe better to just remove entirely.)
There was a problem hiding this comment.
No, currently no card uses this, but its a short road to walk.
|
nice work though, this is great functionality to have in the engine, hopefully we can use it to fix some other longstanding bugs |
#10020
In order to get this card to work properly, i implemented a
LifeLostEventthat gets batched intoLifeLostBatchEvent, similar toDamagedBatchEventandDamagedEvent. It looks like batching theLOST_LIFEevent is the only thing necessary to ensure this card sees all the triggers it should, since any form of damage, life paying costs, as well as "lose life" effects all fire theLOST_LIFEevent.So far i have playtested all of these with one and two opponents losing 1 and 2 or more life due to
The card seems to work as intended.