Kill Trigger Overhaul

- Reworked the Killed trigger to more robustly handle lethal wounds without doubling triggers
- Migrated AT for More Yet To Come
- Fixed Mind and Body for good this time
- Fixed various cards with kill triggers triggering twice when killed by a wound

gemp-lotr/gemp-lotr-cards/src/main/resources/cards/official/set10/set10-Dwarven.hjson

@@ -135,7 +135,7 @@
 				inSkirmish: true
 				memorize: killedMinion
 			}
-			text: Play for {killedMinion}
+			text: Play More Yet to Come for {killedMinion}
 			requires: {
 				type: killedWithSurplusDamage
 				memorize: surplusDamage

gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/cards/build/DefaultActionContext.java

@@ -92,7 +92,7 @@ public class DefaultActionContext implements ActionContext {
             memory = memory.toLowerCase();
         }
         final Collection<PhysicalCard> physicalCards = cardMemory.get(memory);
-        if (physicalCards.size() == 0)
+        if (physicalCards.isEmpty())
             return null;
         if (physicalCards.size() != 1)
             throw new RuntimeException("Unable to retrieve one card from memory: " + memory);

gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/cards/build/field/effect/requirement/KilledWithSurplusDamage.java

@@ -21,8 +21,7 @@ public class KilledWithSurplusDamage implements RequirementProducer {
             public boolean accepts(ActionContext actionContext) {
                 LotroGame game = actionContext.getGame();
                 final ResolveSkirmishDamageAction resolveSkirmishDamageAction = game.getActionsEnvironment().findTopmostActionOfType(ResolveSkirmishDamageAction.class);
-                if (resolveSkirmishDamageAction != null
+                if (resolveSkirmishDamageAction != null && resolveSkirmishDamageAction.getRemainingDamage() > 0) {
-                        && resolveSkirmishDamageAction.getRemainingDamage() > 0) {
                     if (memory != null) {
                         actionContext.setValueToMemory(memory, String.valueOf(resolveSkirmishDamageAction.getRemainingDamage()));
                     }

gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/cards/build/field/effect/trigger/Killed.java

@@ -6,10 +6,12 @@ import com.gempukku.lotro.cards.build.FilterableSource;
 import com.gempukku.lotro.cards.build.InvalidCardDefinitionException;
 import com.gempukku.lotro.cards.build.field.FieldUtils;
 import com.gempukku.lotro.common.Filterable;
+import com.gempukku.lotro.filters.Filters;
 import com.gempukku.lotro.game.PhysicalCard;
 import com.gempukku.lotro.logic.effects.KillEffect;
 import com.gempukku.lotro.logic.timing.TriggerConditions;
 import com.gempukku.lotro.logic.timing.results.ForEachKilledResult;
+import com.gempukku.lotro.logic.timing.results.WoundResult;
 import org.json.simple.JSONObject;
 
 public class Killed implements TriggerCheckerProducer {
@@ -18,13 +20,13 @@ public class Killed implements TriggerCheckerProducer {
         FieldUtils.validateAllowedFields(value, "filter", "killer", "inSkirmish", "cause", "memorize");
 
         final String filter = FieldUtils.getString(value.get("filter"), "filter", "any");
-        final String byFilter = FieldUtils.getString(value.get("killer"), "killer", "any");
+        final String killer = FieldUtils.getString(value.get("killer"), "killer", "any");
         final String memorize = FieldUtils.getString(value.get("memorize"), "memorize");
         final boolean inSkirmish = FieldUtils.getBoolean(value.get("inSkirmish"), "inSkirmish", false);
         final KillEffect.Cause cause = FieldUtils.getEnum(KillEffect.Cause.class, value.get("cause"), "cause");
 
         final FilterableSource filterableSource = environment.getFilterFactory().generateFilter(filter, environment);
-        final FilterableSource byFilterableSource = environment.getFilterFactory().generateFilter(byFilter, environment);
+        final FilterableSource killerFilterableSource = environment.getFilterFactory().generateFilter(killer, environment);
 
         return new TriggerChecker() {
             @Override
@@ -35,20 +37,29 @@ public class Killed implements TriggerCheckerProducer {
             @Override
             public boolean accepts(ActionContext actionContext) {
                 final Filterable filterable = filterableSource.getFilterable(actionContext);
-                Filterable byFilterable = byFilterableSource.getFilterable(actionContext);
+                Filterable killerFilterable = killerFilterableSource.getFilterable(actionContext);
                 boolean result = false;
 
-                if(inSkirmish) {
+                if(killerFilterable == Filters.any) {
-                    result = TriggerConditions.forEachKilledInASkirmish(actionContext.getGame(), actionContext.getEffectResult(), byFilterable, cause, filterable);
+                    result = TriggerConditions.forEachKilled(actionContext.getGame(), actionContext.getEffectResult(), inSkirmish, cause, filterable);
                 }
                 else {
-                    result = TriggerConditions.forEachKilledBy(actionContext.getGame(), actionContext.getEffectResult(), byFilterable, cause, filterable);
+                    result = TriggerConditions.forEachKilledBy(actionContext.getGame(), actionContext.getEffectResult(), inSkirmish, cause, killerFilterable, filterable);
                 }
 
                 if (result && memorize != null) {
-                    final PhysicalCard killedCard = ((ForEachKilledResult) actionContext.getEffectResult()).getKilledCard();
+                    PhysicalCard killedCard = null;
+                    if(actionContext.getEffectResult() instanceof ForEachKilledResult killedResult) {
+                        killedCard = killedResult.getKilledCard();
+                    }
+                    if(actionContext.getEffectResult() instanceof WoundResult woundResult) {
+                        killedCard = woundResult.getWoundedCard();
+                    }
+
+                    if(killedCard != null) {
                         actionContext.setCardMemory(memorize, killedCard);
                     }
+                }
                 return result;
             }
         };

gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/logic/effects/KillEffect.java

@@ -121,7 +121,7 @@ public class KillEffect extends AbstractSuccessfulEffect {
         for (PhysicalCard discardedCard : toAddToDiscard)
             gameState.addCardToZone(game, discardedCard, Zone.DISCARD);
 
-        if (killedCards.size() > 0)
+        if (!killedCards.isEmpty())
             game.getActionsEnvironment().emitEffectResult(new KilledResult(killedCards, new HashSet<>(_killers), _cause));
         for (PhysicalCard killedCard : killedCards)
             game.getActionsEnvironment().emitEffectResult(new ForEachKilledResult(killedCard, _killers, _cause));

gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/logic/timing/TriggerConditions.java

@@ -176,10 +176,8 @@ public class TriggerConditions {
             return Filters.acceptsAny(game, woundResult.getSources(), woundedBy) &&
                     Filters.accepts(game, woundResult.getWoundedCard(), filters) &&
                     (
-                            (Filters.or(CardType.ALLY, CardType.COMPANION).accepts(game, wounded) &&
+                            (Filters.or(CardType.ALLY, CardType.COMPANION).accepts(game, wounded) && wounded.getZone() == Zone.DEAD)
-                                    wounded.getZone() == Zone.DEAD) ||
+                                    || (Filters.accepts(game, wounded, CardType.MINION) && wounded.getZone() != Zone.SHADOW_CHARACTERS)
-                                    (Filters.accepts(game, wounded, CardType.MINION) &&
-                                            wounded.getZone() != Zone.SHADOW_CHARACTERS)
                     );
         }
         return false;
@@ -296,39 +294,55 @@ public class TriggerConditions {
         return false;
     }
 
-    public static boolean forEachKilled(LotroGame game, EffectResult effectResult, Filterable... filters) {
+    public static boolean forEachKilled(LotroGame game, EffectResult effectResult, boolean inSkirmish, KillEffect.Cause cause, Filterable... killedFilter) {
-        return forEachKilledBy(game, effectResult, Filters.any, null, filters);
+        if (effectResult.getType() == EffectResult.Type.FOR_EACH_KILLED) {
-    }
+            var killResult = (ForEachKilledResult) effectResult;
-
-    public static boolean forEachKilledBy(LotroGame game, EffectResult effectResult, Filterable killedBy, KillEffect.Cause cause, Filterable... killed) {
-        if (effectResult.getType() == EffectResult.Type.FOR_EACH_WOUNDED) {
-            return forEachMortallyWoundedBy(game, effectResult, killedBy, killed);
-        } else if (effectResult.getType() == EffectResult.Type.FOR_EACH_KILLED) {
-            if (forEachKilledInASkirmish(game, effectResult, killedBy, cause, killed))
-                return true;
-
-            ForEachKilledResult killResult = (ForEachKilledResult) effectResult;
-            var killers = killResult.getKillers();
-
-            if (killedBy != Filters.any && (killers == null || killers.isEmpty() || killers.stream().allMatch(Objects::isNull)))
-                return false;
-
-            return Filters.acceptsAny(game, killers, killedBy) &&
-                    Filters.and(killed).accepts(game, killResult.getKilledCard());
-        }
-        return false;
-    }
-
-    public static boolean forEachKilledInASkirmish(LotroGame game, EffectResult effectResult, Filterable killedBy, KillEffect.Cause cause, Filterable... killed) {
-        if (effectResult.getType() == EffectResult.Type.FOR_EACH_KILLED
-                && game.getGameState().getCurrentPhase() == Phase.SKIRMISH
-                && Filters.countActive(game, Filters.inSkirmish, killedBy) > 0) {
-            ForEachKilledResult killResult = (ForEachKilledResult) effectResult;
 
             if(cause != null && killResult.getCause() != cause)
                 return false;
 
-            return Filters.and(killed).accepts(game, killResult.getKilledCard());
+            if(inSkirmish) {
+                if(game.getGameState().getCurrentPhase() != Phase.SKIRMISH || Filters.countActive(game, Filters.inSkirmish) == 0)
+                    return false;
+            }
+
+            return Filters.and(killedFilter).accepts(game, killResult.getKilledCard());
+        }
+        return false;
+    }
+
+    /**
+     * Standard deaths that come about via vitality being reduced to zero are invoked by the CharacterDeathRule
+     * that is checked in between every effect.  As this is a contextless action, such deaths will never have their
+     * killer properly recorded.  Thus, standard Killed triggers which do not define a trigger simply use the standard
+     * FOR_EACH_KILLED trigger in the method above, while Killed triggers which must be aware of their killer have
+     * to split attention here between FOR_EACH_KILLED (for overwhelms and direct kill effects) and FOR_EACH_WOUNDED (for
+     * mortal wounds and direct vitality subtraction).  To avoid triggering twice when mortally wounded, we abort early
+     * if we detect the killer information has not been provided at all (as it will be when fired by a zero-vitality
+     * check).
+     */
+    public static boolean forEachKilledBy(LotroGame game, EffectResult effectResult, boolean inSkirmish, KillEffect.Cause cause, Filterable killedBy, Filterable... killed) {
+        if (effectResult.getType() == EffectResult.Type.FOR_EACH_WOUNDED) {
+            return forEachMortallyWoundedBy(game, effectResult, killedBy, killed);
+        } else
+        if (effectResult.getType() == EffectResult.Type.FOR_EACH_KILLED) {
+            ForEachKilledResult killResult = (ForEachKilledResult) effectResult;
+            var killers = killResult.getKillers();
+
+            //If killers is not provided at all, then this event was triggered by the CharacterDeathRule
+            // and we must skip this evaluation, as we would have triggered already on the mortal wound above.
+            if (killers == null || killers.isEmpty() || killers.stream().allMatch(Objects::isNull))
+                return false;
+
+            if(cause != null && killResult.getCause() != cause)
+                return false;
+
+            if(inSkirmish) {
+                if(game.getGameState().getCurrentPhase() != Phase.SKIRMISH || Filters.countActive(game, Filters.inSkirmish) == 0)
+                    return false;
+            }
+
+            return Filters.acceptsAny(game, killers, killedBy) && Filters.accepts(game, killResult.getKilledCard(), killed);
         }
         return false;
     }

gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/logic/timing/rules/CharacterDeathRule.java

@@ -60,7 +60,7 @@ public class CharacterDeathRule {
                 if (!_charactersAlreadyOnWayToDeath.contains(character) && game.getModifiersQuerying().getVitality(game, character) <= 0)
                     deadChars.add(character);
 
-            if (deadChars.size() > 0) {
+            if (!deadChars.isEmpty()) {
                 _charactersAlreadyOnWayToDeath.addAll(deadChars);
                 game.getActionsEnvironment().emitEffectResult(new ZeroVitalityResult(deadChars));
             }

gemp-lotr/gemp-lotr-server/src/test/java/com/gempukku/lotro/at/IndividualCardAtTest.java

@@ -515,62 +515,6 @@ public class IndividualCardAtTest extends AbstractAtTest {
         assertEquals(1, _game.getGameState().getTokenCount(corsairWarGalley, Token.RAIDER));
     }
 
-
-    @Test
-    public void moreYetToComeWorks() throws DecisionResultInvalidException, CardNotFoundException {
-        initializeSimplestGame();
-
-        PhysicalCardImpl gimli = new PhysicalCardImpl(100, "1_12", P1, _cardLibrary.getLotroCardBlueprint("1_12"));
-        PhysicalCardImpl moreYetToCome = new PhysicalCardImpl(101, "10_3", P1, _cardLibrary.getLotroCardBlueprint("10_3"));
-        PhysicalCardImpl goblinRunner = new PhysicalCardImpl(102, "1_178", P2, _cardLibrary.getLotroCardBlueprint("1_178"));
-        PhysicalCardImpl goblinRunner2 = new PhysicalCardImpl(103, "1_178", P2, _cardLibrary.getLotroCardBlueprint("1_178"));
-
-        _game.getGameState().addCardToZone(_game, moreYetToCome, Zone.HAND);
-
-        skipMulligans();
-
-        _game.getGameState().addCardToZone(_game, gimli, Zone.FREE_CHARACTERS);
-
-        // End fellowship
-        playerDecided(P1, "");
-
-        _game.getGameState().addCardToZone(_game, goblinRunner, Zone.SHADOW_CHARACTERS);
-        _game.getGameState().addCardToZone(_game, goblinRunner2, Zone.SHADOW_CHARACTERS);
-
-        // End shadow
-        playerDecided(P2, "");
-
-        // End maneuver
-        playerDecided(P1, "");
-        playerDecided(P2, "");
-
-        // End archery
-        playerDecided(P1, "");
-        playerDecided(P2, "");
-
-        // End assignment
-        playerDecided(P1, "");
-        playerDecided(P2, "");
-
-        // Assign Gimli to goblin runner
-        playerDecided(P1, gimli.getCardId() + " " + goblinRunner.getCardId());
-
-        playerDecided(P2, "");
-
-        // Choose skirmish to start
-        playerDecided(P1, gimli.getCardId() + "");
-
-        // End skirmish
-        playerDecided(P1, "");
-        playerDecided(P2, "");
-
-        assertEquals(Zone.DISCARD, goblinRunner.getZone());
-        AwaitingDecision playMoreYetToCome = _userFeedback.getAwaitingDecision(P1);
-        playerDecided(P1, getCardActionId(playMoreYetToCome, moreYetToCome));
-
-        assertEquals(Zone.DISCARD, goblinRunner2.getZone());
-    }
-
     @Test
     public void treebeardEarthborn() throws DecisionResultInvalidException, CardNotFoundException {
         initializeSimplestGame();

gemp-lotr/gemp-lotr-server/src/test/java/com/gempukku/lotro/cards/official/set07/Card_07_183_Tests.java

@@ -123,6 +123,55 @@ public class Card_07_183_Tests
 
     }
 
+    @Test
+    public void MindAndBodyTriggersOnlyOnceOnNormalSkirmishNazgulKill() throws DecisionResultInvalidException, CardNotFoundException {
+        //Pre-game setup
+        var scn = GetScenario();
+
+        var frodo = scn.GetRingBearer();
+        var sam = scn.GetFreepsCard("sam");
+        var merry = scn.GetFreepsCard("merry");
+        var pippin = scn.GetFreepsCard("pippin");
+        var greenleaf = scn.GetFreepsCard("greenleaf");
+        scn.FreepsMoveCharToTable(sam, merry, pippin, greenleaf);
+
+        var mind = scn.GetShadowCard("mind");
+        var shotgun = scn.GetShadowCard("shotgun");
+        var breath1 = scn.GetShadowCard("breath1");
+        var breath2 = scn.GetShadowCard("breath2");
+        var breath3 = scn.GetShadowCard("breath3");
+        scn.ShadowMoveCharToTable(shotgun);
+        scn.AttachCardsTo(merry, breath1);
+        scn.AttachCardsTo(pippin, breath2);
+        scn.AttachCardsTo(pippin, breath3);
+        scn.ShadowMoveCardToHand(mind);
+
+        scn.StartGame();
+
+        scn.AddWoundsToChar(greenleaf, 2);
+        scn.SkipToAssignments();
+        scn.FreepsAssignToMinions(greenleaf, shotgun);
+        scn.FreepsResolveSkirmish(greenleaf);
+
+        assertEquals(1, scn.GetVitality(greenleaf));
+        assertEquals(11, scn.GetStrength(shotgun));
+        assertEquals(6, scn.GetStrength(greenleaf));
+        assertEquals(Zone.FREE_CHARACTERS, greenleaf.getZone());
+        assertEquals(Zone.HAND, mind.getZone());
+
+        scn.PassCurrentPhaseActions();
+
+        assertEquals(Zone.DEAD, greenleaf.getZone());
+
+        assertTrue(scn.ShadowHasOptionalTriggerAvailable());
+        assertTrue(scn.ShadowPlayAvailable(mind));
+
+        //Ensure we're not double-dipping
+        scn.ShadowDeclineOptionalTrigger();
+        assertFalse(scn.ShadowHasOptionalTriggerAvailable());
+
+    }
+
     @Test
     public void MindAndBodyTriggersOnNazgulOverwhelm() throws DecisionResultInvalidException, CardNotFoundException {
         //Pre-game setup
@@ -155,6 +204,10 @@ public class Card_07_183_Tests
         assertEquals(Zone.DEAD, sam.getZone());
         assertTrue(scn.ShadowHasOptionalTriggerAvailable());
         assertTrue(scn.ShadowPlayAvailable(mind));
+
+        //Ensure we're not double-dipping
+        scn.ShadowDeclineOptionalTrigger();
+        assertFalse(scn.ShadowHasOptionalTriggerAvailable());
     }
 
     @Test
@@ -190,6 +243,10 @@ public class Card_07_183_Tests
         assertEquals(Zone.DEAD, bounder.getZone());
         assertTrue(scn.ShadowHasOptionalTriggerAvailable());
         assertTrue(scn.ShadowPlayAvailable(mind));
+
+        //Ensure we're not double-dipping
+        scn.ShadowDeclineOptionalTrigger();
+        assertFalse(scn.ShadowHasOptionalTriggerAvailable());
     }
 
     @Test
@@ -224,6 +281,10 @@ public class Card_07_183_Tests
         assertEquals(Zone.DEAD, sam.getZone());
         assertTrue(scn.ShadowHasOptionalTriggerAvailable());
         assertTrue(scn.ShadowPlayAvailable(mind));
+
+        //Ensure we're not double-dipping
+        scn.ShadowDeclineOptionalTrigger();
+        assertFalse(scn.ShadowHasOptionalTriggerAvailable());
     }
 
     @Test

gemp-lotr/gemp-lotr-server/src/test/java/com/gempukku/lotro/cards/official/set10/Card_10_003_Tests.java

@@ -1,10 +1,7 @@
 package com.gempukku.lotro.cards.official.set10;
 
 import com.gempukku.lotro.cards.GenericCardTestHelper;
-import com.gempukku.lotro.common.CardType;
+import com.gempukku.lotro.common.*;
-import com.gempukku.lotro.common.Culture;
-import com.gempukku.lotro.common.Side;
-import com.gempukku.lotro.common.Timeword;
 import com.gempukku.lotro.game.CardNotFoundException;
 import com.gempukku.lotro.logic.decisions.DecisionResultInvalidException;
 import org.junit.Test;
@@ -20,8 +17,10 @@ public class Card_10_003_Tests
 		return new GenericCardTestHelper(
 				new HashMap<>()
 				{{
-					put("card", "10_3");
+					put("more", "10_3");
-					// put other cards in here as needed for the test case
+					put("gimli", "1_12");
+					put("runner", "1_178");
+					put("savage", "1_151");
 				}},
 				GenericCardTestHelper.FellowshipSites,
 				GenericCardTestHelper.FOTRFrodo,
@@ -46,7 +45,7 @@ public class Card_10_003_Tests
 
 		var scn = GetScenario();
 
-		var card = scn.GetFreepsCard("card");
+		var card = scn.GetFreepsCard("more");
 
 		assertEquals("More Yet to Come", card.getBlueprint().getTitle());
 		assertNull(card.getBlueprint().getSubtitle());
@@ -58,18 +57,37 @@ public class Card_10_003_Tests
 		assertEquals(0, card.getBlueprint().getTwilightCost());
 	}
 
-	// Uncomment any @Test markers below once this is ready to be used
+	//Migrated from legacy AT
-	//@Test
+	@Test
-	public void MoreYettoComeTest1() throws DecisionResultInvalidException, CardNotFoundException {
+	public void MoreYetToComeAppliesExtraDamageToMinion() throws DecisionResultInvalidException, CardNotFoundException {
-		//Pre-game setup
 		var scn = GetScenario();
 
-		var card = scn.GetFreepsCard("card");
+		var gimli = scn.GetFreepsCard("gimli");
-		scn.FreepsMoveCardToHand(card);
+		var more = scn.GetFreepsCard("more");
+		scn.FreepsMoveCharToTable(gimli);
+		scn.FreepsMoveCardToHand(more);
+
+		var runner = scn.GetShadowCard("runner");
+		var savage = scn.GetShadowCard("savage");
+		scn.ShadowMoveCharToTable(runner, savage);
 
 		scn.StartGame();
-		scn.FreepsPlayCard(card);
 
-		assertEquals(0, scn.GetTwilight());
+		scn.SkipToAssignments();
+		scn.FreepsAssignToMinions(gimli, runner);
+		scn.ShadowDeclineAssignments();
+
+		scn.FreepsResolveSkirmish(gimli);
+		assertEquals(Zone.SHADOW_CHARACTERS, runner.getZone());
+		assertEquals(1, scn.GetKeywordCount(gimli, Keyword.DAMAGE));
+		assertEquals(0, scn.GetWoundsOn(savage));
+
+		scn.PassCurrentPhaseActions();
+
+		assertEquals(Zone.DISCARD, runner.getZone());
+		assertTrue(scn.FreepsHasOptionalTriggerAvailable());
+		scn.FreepsAcceptOptionalTrigger();
+
+		assertEquals(1, scn.GetWoundsOn(savage));
 	}
 }