Skirmish Resolution Overhaul

- Overhauled the skirmish resolution code to involve far less repetitive copy-pasted code.  Eliminated code that emitted a "skirmish lost" trigger for characters removed early from a skirmish (this behavior was superceded in a later CRD)
- Consolidated the separate SkirmishType enums instead of having a separate enum for Overwhelm and Normal skirmish types (that had the exact same contents)
- Fixed Dwarven Axe (and any other "skirmish loss" triggers) incorrectly triggering on a character that has been removed from a skirmish before it resolves.

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

@@ -7,6 +7,7 @@ import com.gempukku.lotro.cards.build.InvalidCardDefinitionException;
 import com.gempukku.lotro.cards.build.field.FieldUtils;
 import com.gempukku.lotro.logic.timing.TriggerConditions;
 import com.gempukku.lotro.logic.timing.results.CharacterLostSkirmishResult;
+import com.gempukku.lotro.logic.timing.results.SkirmishType;
 import org.json.simple.JSONObject;
 
 public class LosesSkirmish implements TriggerCheckerProducer {
@@ -30,7 +31,7 @@ public class LosesSkirmish implements TriggerCheckerProducer {
                         againstFilter.getFilterable(actionContext));
                 if (result) {
                     CharacterLostSkirmishResult lostResult = (CharacterLostSkirmishResult) actionContext.getEffectResult();
-                    if (overwhelm && lostResult.getSkirmishType() != CharacterLostSkirmishResult.SkirmishType.OVERWHELM)
+                    if (overwhelm && lostResult.getSkirmishType() != SkirmishType.OVERWHELM)
                         return false;
 
                     if (memorize != null) {

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

@@ -5,10 +5,7 @@ import com.gempukku.lotro.game.state.LotroGame;
 import com.gempukku.lotro.game.state.Skirmish;
 import com.gempukku.lotro.logic.timing.AbstractEffect;
 import com.gempukku.lotro.logic.timing.RuleUtils;
-import com.gempukku.lotro.logic.timing.results.CharacterLostSkirmishResult;
+import com.gempukku.lotro.logic.timing.results.*;
-import com.gempukku.lotro.logic.timing.results.CharacterWonSkirmishResult;
-import com.gempukku.lotro.logic.timing.results.NormalSkirmishResult;
-import com.gempukku.lotro.logic.timing.results.OverwhelmSkirmishResult;
 
 import java.util.Collections;
 import java.util.HashSet;
@@ -70,51 +67,57 @@ public class ResolveSkirmishEffect extends AbstractEffect {
 
 		Result result = getUpcomingResult(game);
 
+		var shadow = skirmish.getShadowCharacters();
+		var freeps = fpList(skirmish.getFellowshipCharacter());
+
 		Set<PhysicalCard> involving = new HashSet<>();
-		if (skirmish.getFellowshipCharacter() != null)
+		involving.addAll(freeps);
-			involving.add(skirmish.getFellowshipCharacter());
+		involving.addAll(shadow);
-		involving.addAll(skirmish.getShadowCharacters());
 
-		if (result == Result.FELLOWSHIP_LOSES) {
+		String message = null;
-			game.getGameState().sendMessage("Skirmish resolved with a normal win for Shadow");
+		Set<PhysicalCard> winners = null, losers = null;
-			game.getActionsEnvironment().emitEffectResult(new NormalSkirmishResult(skirmish.getShadowCharacters(), fpList(skirmish.getFellowshipCharacter()), skirmish.getRemovedFromSkirmish()));
+		SkirmishType skirmishType = null;
 
-			for (PhysicalCard minion : skirmish.getShadowCharacters())
+		switch (result) {
-				game.getActionsEnvironment().emitEffectResult(new CharacterWonSkirmishResult(CharacterWonSkirmishResult.SkirmishType.NORMAL, minion, involving));
+			case FELLOWSHIP_LOSES -> {
-			if (skirmish.getFellowshipCharacter() != null)
+				message = "Skirmish resolved: Shadow defeated Free Peoples";
-				game.getActionsEnvironment().emitEffectResult(new CharacterLostSkirmishResult(CharacterLostSkirmishResult.SkirmishType.NORMAL, skirmish.getFellowshipCharacter(), involving));
+				winners = shadow;
-			for (PhysicalCard removedCharacter : skirmish.getRemovedFromSkirmish())
+				losers = freeps;
-				game.getActionsEnvironment().emitEffectResult(new CharacterLostSkirmishResult(CharacterLostSkirmishResult.SkirmishType.NORMAL, removedCharacter, involving));
+				skirmishType = SkirmishType.NORMAL;
-		} else if (result == Result.SHADOW_LOSES) {
+			}
-			game.getGameState().sendMessage("Skirmish resolved with a normal win for Free Peoples");
+			case FELLOWSHIP_OVERWHELMED -> {
-			game.getActionsEnvironment().emitEffectResult(new NormalSkirmishResult(fpList(skirmish.getFellowshipCharacter()), skirmish.getShadowCharacters(), skirmish.getRemovedFromSkirmish()));
+				message = "Skirmish resolved: Shadow overwhelmed Free Peoples";
+				winners = shadow;
+				losers = freeps;
+				skirmishType = SkirmishType.OVERWHELM;
+			}
+			case SHADOW_LOSES -> {
+				message = "Skirmish resolved: Free Peoples defeated Shadow";
+				winners = freeps;
+				losers = shadow;
+				skirmishType = SkirmishType.NORMAL;
+			}
+			case SHADOW_OVERWHELMED -> {
+				message = "Skirmish resolved: Free Peoples overwhelmed Shadow";
+				winners = freeps;
+				losers = shadow;
+				skirmishType = SkirmishType.OVERWHELM;
+			}
+		}
 
-			for (PhysicalCard minion : skirmish.getShadowCharacters())
+		game.getGameState().sendMessage(message);
-				game.getActionsEnvironment().emitEffectResult(new CharacterLostSkirmishResult(CharacterLostSkirmishResult.SkirmishType.NORMAL, minion, involving));
+		if(skirmishType == SkirmishType.OVERWHELM) {
-			if (skirmish.getFellowshipCharacter() != null)
+			game.getActionsEnvironment().emitEffectResult(new OverwhelmSkirmishResult(winners, losers, skirmish.getRemovedFromSkirmish()));
-				game.getActionsEnvironment().emitEffectResult(new CharacterWonSkirmishResult(CharacterWonSkirmishResult.SkirmishType.NORMAL, skirmish.getFellowshipCharacter(), involving));
+		}
-			for (PhysicalCard removedCharacter : skirmish.getRemovedFromSkirmish())
+		else {
-				game.getActionsEnvironment().emitEffectResult(new CharacterLostSkirmishResult(CharacterLostSkirmishResult.SkirmishType.NORMAL, removedCharacter, involving));
+			game.getActionsEnvironment().emitEffectResult(new NormalSkirmishResult(winners, losers, skirmish.getRemovedFromSkirmish()));
-		} else if (result == Result.FELLOWSHIP_OVERWHELMED) {
+		}
-			game.getGameState().sendMessage("Skirmish resolved with an overwhelm win for Shadow");
-			game.getActionsEnvironment().emitEffectResult(new OverwhelmSkirmishResult(skirmish.getShadowCharacters(), fpList(skirmish.getFellowshipCharacter()), skirmish.getRemovedFromSkirmish()));
 
-			for (PhysicalCard minion : skirmish.getShadowCharacters())
+		for (PhysicalCard loser : losers) {
-				game.getActionsEnvironment().emitEffectResult(new CharacterWonSkirmishResult(CharacterWonSkirmishResult.SkirmishType.OVERWHELM, minion, involving));
+			game.getActionsEnvironment().emitEffectResult(new CharacterLostSkirmishResult(skirmishType, loser, involving));
-			if (skirmish.getFellowshipCharacter() != null)
+		}
-				game.getActionsEnvironment().emitEffectResult(new CharacterLostSkirmishResult(CharacterLostSkirmishResult.SkirmishType.OVERWHELM, skirmish.getFellowshipCharacter(), involving));
+		for (PhysicalCard winner : winners) {
-			for (PhysicalCard removedCharacter : skirmish.getRemovedFromSkirmish())
+			game.getActionsEnvironment().emitEffectResult(new CharacterWonSkirmishResult(skirmishType, winner, involving));
-				game.getActionsEnvironment().emitEffectResult(new CharacterLostSkirmishResult(CharacterLostSkirmishResult.SkirmishType.OVERWHELM, removedCharacter, involving));
-		} else {
-			game.getGameState().sendMessage("Skirmish resolved with an overwhelm win for Free Peoples");
-			game.getActionsEnvironment().emitEffectResult(new OverwhelmSkirmishResult(fpList(skirmish.getFellowshipCharacter()), skirmish.getShadowCharacters(), skirmish.getRemovedFromSkirmish()));
-
-			for (PhysicalCard minion : skirmish.getShadowCharacters())
-				game.getActionsEnvironment().emitEffectResult(new CharacterLostSkirmishResult(CharacterLostSkirmishResult.SkirmishType.OVERWHELM, minion, involving));
-			if (skirmish.getFellowshipCharacter() != null)
-				game.getActionsEnvironment().emitEffectResult(new CharacterWonSkirmishResult(CharacterWonSkirmishResult.SkirmishType.OVERWHELM, skirmish.getFellowshipCharacter(), involving));
-			for (PhysicalCard removedCharacter : skirmish.getRemovedFromSkirmish())
-				game.getActionsEnvironment().emitEffectResult(new CharacterLostSkirmishResult(CharacterLostSkirmishResult.SkirmishType.OVERWHELM, removedCharacter, involving));
 		}
 
 		return new FullEffectResult(true);

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

@@ -11,10 +11,6 @@ public class CharacterLostSkirmishResult extends EffectResult {
     private final Set<PhysicalCard> _involving;
     private final SkirmishType _type;
 
-    public enum SkirmishType {
-        OVERWHELM, NORMAL
-    }
-
     public CharacterLostSkirmishResult(SkirmishType type, PhysicalCard loser, Set<PhysicalCard> involving) {
         super(Type.CHARACTER_LOST_SKIRMISH);
         _type = type;

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

@@ -11,10 +11,6 @@ public class CharacterWonSkirmishResult extends EffectResult {
     private final Set<PhysicalCard> _involving;
     private final SkirmishType _type;
 
-    public enum SkirmishType {
-        OVERWHELM, NORMAL
-    }
-
     public CharacterWonSkirmishResult(SkirmishType type, PhysicalCard winner, Set<PhysicalCard> involving) {
         super(EffectResult.Type.CHARACTER_WON_SKIRMISH);
         _type = type;

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

@@ -0,0 +1,5 @@
+package com.gempukku.lotro.logic.timing.results;
+
+public enum SkirmishType {
+	OVERWHELM, NORMAL
+}

gemp-lotr/gemp-lotr-server/src/test/java/com/gempukku/lotro/cards/official/set01/Card_01_009_Tests.java

@@ -25,9 +25,12 @@ public class Card_01_009_Tests
 				{{
 					put("axe", "1_9");
 					put("gimli", "1_13");
+					put("preparations", "7_12");
+					put("dcard", "2_10");
 
-					put("runner", "1_178");
+					put("orc1", "1_178");
-					put("runner2", "1_178");
+					put("orc2", "1_181");
+					put("orc3", "1_184");
 				}},
 				GenericCardTestHelper.FellowshipSites,
 				GenericCardTestHelper.FOTRFrodo,
@@ -91,28 +94,28 @@ public class Card_01_009_Tests
 		scn.FreepsMoveCharToTable(gimli);
 		scn.AttachCardsTo(gimli, axe);
 
-		var runner = scn.GetShadowCard("runner");
+		var orc1 = scn.GetShadowCard("orc1");
-		scn.ShadowMoveCharToTable(runner);
+		scn.ShadowMoveCharToTable(orc1);
 
 		scn.StartGame();
 
 		scn.SkipToPhase(Phase.ASSIGNMENT);
 
 		scn.PassCurrentPhaseActions();
-		scn.FreepsAssignToMinions(gimli, runner);
+		scn.FreepsAssignToMinions(gimli, orc1);
 
 		scn.FreepsResolveSkirmish(gimli);
 		scn.PassCurrentPhaseActions();
 
 		var topcard = scn.GetShadowTopOfDeck();
 		assertEquals(Zone.DECK, topcard.getZone());
-		assertEquals(3, scn.GetShadowDeckCount());
+		assertEquals(6, scn.GetShadowDeckCount());
 		assertEquals(0, scn.GetShadowDiscardCount());
 
 		scn.FreepsResolveActionOrder("Axe");
 		assertEquals(Zone.DISCARD, topcard.getZone());
-		assertEquals(2, scn.GetShadowDeckCount());
+		assertEquals(5, scn.GetShadowDeckCount());
-		assertEquals(2, scn.GetShadowDiscardCount());//runner, and a card discarded from the deck
+		assertEquals(2, scn.GetShadowDiscardCount());//orc1, and a card discarded from the deck
 
 	}
 
@@ -126,9 +129,9 @@ public class Card_01_009_Tests
 		scn.FreepsMoveCharToTable(gimli);
 		scn.AttachCardsTo(gimli, axe);
 
-		var runner = scn.GetShadowCard("runner");
+		var orc1 = scn.GetShadowCard("orc1");
-		var runner2 = scn.GetShadowCard("runner2");
+		var orc2 = scn.GetShadowCard("orc2");
-		scn.ShadowMoveCharToTable(runner, runner2);
+		scn.ShadowMoveCharToTable(orc1, orc2);
 
 		scn.StartGame();
 
@@ -136,7 +139,7 @@ public class Card_01_009_Tests
 
 		scn.PassCurrentPhaseActions();
 		scn.FreepsDeclineAssignments();
-		scn.ShadowAssignToMinions(gimli, runner, runner2);
+		scn.ShadowAssignToMinions(gimli, orc1, orc2);
 
 		scn.FreepsResolveSkirmish(gimli);
 		scn.FreepsUseCardAction(gimli);
@@ -148,23 +151,82 @@ public class Card_01_009_Tests
 		//Should only have "resolve skirmish wounds" and one instance of "dwarven axe required trigger".
 		// If there are two axe-triggers, then the card is violating the latest Decipher "clarification":
 		//
+		// https://wiki.lotrtcgpc.net/wiki/Comprehensive_Rules_4.1#Section_Three:_Individual_Card_Rulings
 		//"This card can trigger only once for each Shadow player with a minion in that skirmish,
 		// regardless of how many minions that player had."
 		assertEquals(3, scn.FreepsGetAvailableActions().size());
 
 		var topcard = scn.GetShadowTopOfDeck();
 		assertEquals(Zone.DECK, topcard.getZone());
-		assertEquals(2, scn.GetShadowDeckCount());
+		assertEquals(5, scn.GetShadowDeckCount());
 		assertEquals(0, scn.GetShadowDiscardCount());
 
 		//Ideally we shouldn't have both triggers, but at least we can squelch the results of the second
 		scn.FreepsResolveActionOrder("Axe");
 		scn.FreepsResolveActionOrder("Axe");
 		assertEquals(Zone.DISCARD, topcard.getZone());
-		assertEquals(1, scn.GetShadowDeckCount());
+		assertEquals(4, scn.GetShadowDeckCount());
 		assertEquals(3, scn.GetShadowDiscardCount());//2 dead runners, and a card discarded from the deck
 	}
 
+
+	/**
+	 * https://github.com/PlayersCouncil/gemp-lotr/issues/571
+	 * Basic issue is that based on the last CRD:
+	 * 	 > A losing character is any character on the losing side when a skirmish revolves.
+	 * 	 > If a character is removed from his or her skirmish and there are still one or more characters
+	 * 	 > on each side of that skirmish, the removed character is neither a losing nor a winning character.
+	 * 	 > A character removed from a skirmish is not wounded (or overwhelmed) when that skirmish resolves.
+	 * This supercedes the Comprehensive Rules 4.0, which states:
+	 *   > A losing character is any character on the losing side in a skirmish when it resolves.
+	 *   > Also, any character removed during his or her skirmish is a losing character, even if
+	 *   > that character’s side eventually wins.
+	 *
+	 * From a basic functionality standpoint, killed characters should not stick around as part of the
+	 * tracked losing characters lists, or at the very least the skirmish resolution should check to see
+	 * if they are still in the skirmish and disqualify them for trigger emission if so.
+	 */
+
+	@Test
+	public void AxeDoesNotTriggerOnMinionKilledBeforeSkirmishResolves() throws DecisionResultInvalidException, CardNotFoundException {
+		//Pre-game setup
+		var scn = GetScenario();
+
+		var axe = scn.GetFreepsCard("axe");
+		var gimli = scn.GetFreepsCard("gimli");
+		var preparations = scn.GetFreepsCard("preparations");
+		scn.FreepsMoveCharToTable(gimli);
+		scn.AttachCardsTo(gimli, axe);
+		scn.FreepsMoveCardToSupportArea(preparations);
+		scn.FreepsStackCardsOn(preparations, "dcard");
+
+		var orc1 = scn.GetShadowCard("orc1");
+		var orc2 = scn.GetShadowCard("orc2");
+		var orc3 = scn.GetShadowCard("orc3");
+		scn.ShadowMoveCharToTable(orc1, orc2, orc3);
+
+		scn.StartGame();
+
+		scn.SkipToPhase(Phase.ASSIGNMENT);
+
+		scn.PassCurrentPhaseActions();
+		scn.FreepsDeclineAssignments();
+		scn.ShadowAssignToMinions(gimli, orc1, orc2, orc3);
+
+		scn.FreepsResolveSkirmish(gimli);
+		scn.FreepsUseCardAction(preparations);
+		scn.FreepsChooseCard(orc3);
+		assertEquals(Zone.DISCARD, orc3.getZone());
+
+		scn.ShadowPassCurrentPhaseAction();
+		scn.FreepsPassCurrentPhaseAction();
+
+		//There should not be any axe trigger here; we should go straight to regroup
+
+		assertEquals(1, scn.GetWoundsOn(gimli));
+		assertEquals(Phase.REGROUP, scn.GetCurrentPhase());
+	}
+
 	@Test
 	public void Legacy_dwarvenAxeDoesNotFreeze() throws DecisionResultInvalidException, CardNotFoundException {
 		var legacy = new LegacyAtTest();