From b0aac19d1d2798b2cf22fbf2ff3fcae6d0e0d24b Mon Sep 17 00:00:00 2001 From: Christian 'ketura' McCarty Date: Sun, 30 Mar 2025 14:19:15 -0500 Subject: [PATCH] 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. --- .../field/effect/trigger/LosesSkirmish.java | 3 +- .../logic/effects/ResolveSkirmishEffect.java | 91 ++++++++++--------- .../results/CharacterLostSkirmishResult.java | 4 - .../results/CharacterWonSkirmishResult.java | 4 - .../logic/timing/results/SkirmishType.java | 5 + .../official/set01/Card_01_009_Tests.java | 90 +++++++++++++++--- 6 files changed, 130 insertions(+), 67 deletions(-) create mode 100644 gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/logic/timing/results/SkirmishType.java diff --git a/gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/cards/build/field/effect/trigger/LosesSkirmish.java b/gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/cards/build/field/effect/trigger/LosesSkirmish.java index 6d4e0f724..eeaa78aa0 100644 --- a/gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/cards/build/field/effect/trigger/LosesSkirmish.java +++ b/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) { diff --git a/gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/logic/effects/ResolveSkirmishEffect.java b/gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/logic/effects/ResolveSkirmishEffect.java index 6a1799c18..7eccd708f 100644 --- a/gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/logic/effects/ResolveSkirmishEffect.java +++ b/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.CharacterWonSkirmishResult; -import com.gempukku.lotro.logic.timing.results.NormalSkirmishResult; -import com.gempukku.lotro.logic.timing.results.OverwhelmSkirmishResult; +import com.gempukku.lotro.logic.timing.results.*; 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 involving = new HashSet<>(); - if (skirmish.getFellowshipCharacter() != null) - involving.add(skirmish.getFellowshipCharacter()); - involving.addAll(skirmish.getShadowCharacters()); + involving.addAll(freeps); + involving.addAll(shadow); - if (result == Result.FELLOWSHIP_LOSES) { - game.getGameState().sendMessage("Skirmish resolved with a normal win for Shadow"); - game.getActionsEnvironment().emitEffectResult(new NormalSkirmishResult(skirmish.getShadowCharacters(), fpList(skirmish.getFellowshipCharacter()), skirmish.getRemovedFromSkirmish())); + String message = null; + Set winners = null, losers = null; + SkirmishType skirmishType = null; - for (PhysicalCard minion : skirmish.getShadowCharacters()) - game.getActionsEnvironment().emitEffectResult(new CharacterWonSkirmishResult(CharacterWonSkirmishResult.SkirmishType.NORMAL, minion, involving)); - if (skirmish.getFellowshipCharacter() != null) - game.getActionsEnvironment().emitEffectResult(new CharacterLostSkirmishResult(CharacterLostSkirmishResult.SkirmishType.NORMAL, skirmish.getFellowshipCharacter(), involving)); - for (PhysicalCard removedCharacter : skirmish.getRemovedFromSkirmish()) - game.getActionsEnvironment().emitEffectResult(new CharacterLostSkirmishResult(CharacterLostSkirmishResult.SkirmishType.NORMAL, removedCharacter, involving)); - } else if (result == Result.SHADOW_LOSES) { - game.getGameState().sendMessage("Skirmish resolved with a normal win for Free Peoples"); - game.getActionsEnvironment().emitEffectResult(new NormalSkirmishResult(fpList(skirmish.getFellowshipCharacter()), skirmish.getShadowCharacters(), skirmish.getRemovedFromSkirmish())); + switch (result) { + case FELLOWSHIP_LOSES -> { + message = "Skirmish resolved: Shadow defeated Free Peoples"; + winners = shadow; + losers = freeps; + skirmishType = SkirmishType.NORMAL; + } + case FELLOWSHIP_OVERWHELMED -> { + 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.getActionsEnvironment().emitEffectResult(new CharacterLostSkirmishResult(CharacterLostSkirmishResult.SkirmishType.NORMAL, minion, involving)); - if (skirmish.getFellowshipCharacter() != null) - game.getActionsEnvironment().emitEffectResult(new CharacterWonSkirmishResult(CharacterWonSkirmishResult.SkirmishType.NORMAL, skirmish.getFellowshipCharacter(), involving)); - for (PhysicalCard removedCharacter : skirmish.getRemovedFromSkirmish()) - game.getActionsEnvironment().emitEffectResult(new CharacterLostSkirmishResult(CharacterLostSkirmishResult.SkirmishType.NORMAL, removedCharacter, involving)); - } 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())); + game.getGameState().sendMessage(message); + if(skirmishType == SkirmishType.OVERWHELM) { + game.getActionsEnvironment().emitEffectResult(new OverwhelmSkirmishResult(winners, losers, skirmish.getRemovedFromSkirmish())); + } + else { + game.getActionsEnvironment().emitEffectResult(new NormalSkirmishResult(winners, losers, skirmish.getRemovedFromSkirmish())); + } - for (PhysicalCard minion : skirmish.getShadowCharacters()) - game.getActionsEnvironment().emitEffectResult(new CharacterWonSkirmishResult(CharacterWonSkirmishResult.SkirmishType.OVERWHELM, minion, involving)); - if (skirmish.getFellowshipCharacter() != null) - game.getActionsEnvironment().emitEffectResult(new CharacterLostSkirmishResult(CharacterLostSkirmishResult.SkirmishType.OVERWHELM, skirmish.getFellowshipCharacter(), involving)); - for (PhysicalCard removedCharacter : skirmish.getRemovedFromSkirmish()) - 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)); + for (PhysicalCard loser : losers) { + game.getActionsEnvironment().emitEffectResult(new CharacterLostSkirmishResult(skirmishType, loser, involving)); + } + for (PhysicalCard winner : winners) { + game.getActionsEnvironment().emitEffectResult(new CharacterWonSkirmishResult(skirmishType, winner, involving)); } return new FullEffectResult(true); diff --git a/gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/logic/timing/results/CharacterLostSkirmishResult.java b/gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/logic/timing/results/CharacterLostSkirmishResult.java index 7b0cffdbb..6b57aeff9 100644 --- a/gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/logic/timing/results/CharacterLostSkirmishResult.java +++ b/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 _involving; private final SkirmishType _type; - public enum SkirmishType { - OVERWHELM, NORMAL - } - public CharacterLostSkirmishResult(SkirmishType type, PhysicalCard loser, Set involving) { super(Type.CHARACTER_LOST_SKIRMISH); _type = type; diff --git a/gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/logic/timing/results/CharacterWonSkirmishResult.java b/gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/logic/timing/results/CharacterWonSkirmishResult.java index 246ec3076..201620c09 100644 --- a/gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/logic/timing/results/CharacterWonSkirmishResult.java +++ b/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 _involving; private final SkirmishType _type; - public enum SkirmishType { - OVERWHELM, NORMAL - } - public CharacterWonSkirmishResult(SkirmishType type, PhysicalCard winner, Set involving) { super(EffectResult.Type.CHARACTER_WON_SKIRMISH); _type = type; diff --git a/gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/logic/timing/results/SkirmishType.java b/gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/logic/timing/results/SkirmishType.java new file mode 100644 index 000000000..8d778bdfe --- /dev/null +++ b/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 +} diff --git a/gemp-lotr/gemp-lotr-server/src/test/java/com/gempukku/lotro/cards/official/set01/Card_01_009_Tests.java b/gemp-lotr/gemp-lotr-server/src/test/java/com/gempukku/lotro/cards/official/set01/Card_01_009_Tests.java index 80ad337d5..d93b7aa6a 100644 --- a/gemp-lotr/gemp-lotr-server/src/test/java/com/gempukku/lotro/cards/official/set01/Card_01_009_Tests.java +++ b/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("runner2", "1_178"); + put("orc1", "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"); - scn.ShadowMoveCharToTable(runner); + var orc1 = scn.GetShadowCard("orc1"); + 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(2, scn.GetShadowDiscardCount());//runner, and a card discarded from the deck + assertEquals(5, scn.GetShadowDeckCount()); + 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 runner2 = scn.GetShadowCard("runner2"); - scn.ShadowMoveCharToTable(runner, runner2); + var orc1 = scn.GetShadowCard("orc1"); + var orc2 = scn.GetShadowCard("orc2"); + 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();