Cleaned up the GameState sending warnings, but also fixed tests double-modifiers issue

gemp-lotr/gemp-lotr-common/src/main/java/com/gempukku/lotro/common/Phase.java

@@ -1,23 +1,25 @@
 package com.gempukku.lotro.common;
 
 public enum Phase {
-    PUT_RING_BEARER("Put Ring-bearer", false),
+    PUT_RING_BEARER("Put Ring-bearer", false, false),
-    PLAY_STARTING_FELLOWSHIP("Play starting fellowship", false),
+    PLAY_STARTING_FELLOWSHIP("Play starting fellowship", false, true),
-    FELLOWSHIP("Fellowship", true),
+    FELLOWSHIP("Fellowship", true, true),
-    SHADOW("Shadow", true),
+    SHADOW("Shadow", true, true),
-    MANEUVER("Maneuver", true),
+    MANEUVER("Maneuver", true, true),
-    ARCHERY("Archery", true),
+    ARCHERY("Archery", true, true),
-    ASSIGNMENT("Assignment", true),
+    ASSIGNMENT("Assignment", true, true),
-    SKIRMISH("Skirmish", true),
+    SKIRMISH("Skirmish", true, true),
-    REGROUP("Regroup", true),
+    REGROUP("Regroup", true, true),
-    BETWEEN_TURNS("Between turns", false);
+    BETWEEN_TURNS("Between turns", false, false);
 
     private String humanReadable;
     private boolean realPhase;
+    private boolean cardsAffectGame;
 
-    private Phase(String humanReadable, boolean realPhase) {
+    Phase(String humanReadable, boolean realPhase, boolean cardsAffectGame) {
         this.humanReadable = humanReadable;
         this.realPhase = realPhase;
+        this.cardsAffectGame = cardsAffectGame;
     }
 
     public String getHumanReadable() {
@@ -27,4 +29,8 @@ public enum Phase {
     public boolean isRealPhase() {
         return realPhase;
     }
+
+    public boolean isCardsAffectGame() {
+        return cardsAffectGame;
+    }
 }

gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/communication/GameStateListener.java

@@ -55,4 +55,6 @@ public interface GameStateListener {
     public void cardActivated(String playerPerforming, PhysicalCard card);
 
     public void decisionRequired(String playerId, AwaitingDecision awaitingDecision);
+
+    public void sendWarning(String playerId, String warning);
 }

gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/communication/UserFeedback.java

@@ -5,7 +5,5 @@ import com.gempukku.lotro.logic.decisions.AwaitingDecision;
 public interface UserFeedback {
     public void sendAwaitingDecision(String playerId, AwaitingDecision awaitingDecision);
 
-    public void sendWarning(String playerId, String warning);
-
     public boolean hasPendingDecisions();
 }

gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/game/state/GameCommunicationChannel.java

@@ -187,6 +187,12 @@ public class GameCommunicationChannel implements GameStateListener, LongPollable
             appendEvent(new GameEvent(DECISION).awaitingDecision(decision).participantId(playerId));
     }
 
+    @Override
+    public void sendWarning(String playerId, String warning) {
+        if (playerId.equals(_self))
+            appendEvent(new GameEvent(SEND_WARNING).message(warning));
+    }
+
     public List<GameEvent> consumeGameEvents() {
         updateLastAccess();
         List<GameEvent> result = _events;

gemp-lotr/gemp-lotr-logic/src/main/java/com/gempukku/lotro/game/state/GameState.java

@@ -489,7 +489,7 @@ public class GameState {
                 listener.cardCreated(card);
         }
 
-        if (_currentPhase != Phase.PUT_RING_BEARER) {
+        if (_currentPhase.isCardsAffectGame()) {
             if (zone.isInPlay())
                 if (card.getBlueprint().getCardType() != CardType.SITE || (getCurrentPhase() != Phase.PLAY_STARTING_FELLOWSHIP && getCurrentSite() == card))
                     startAffecting(game, card);
@@ -1041,4 +1041,9 @@ public class GameState {
         for (GameStateListener listener : getAllGameStateListeners())
             listener.sendGameStats(gameStats);
     }
+
+    public void sendWarning(String player, String warning) {
+        for (GameStateListener listener : getAllGameStateListeners())
+            listener.sendWarning(player, warning);
+    }
 }

gemp-lotr/gemp-lotr-server/src/main/java/com/gempukku/lotro/game/DefaultUserFeedback.java

@@ -10,7 +10,6 @@ import java.util.Set;
 
 public class DefaultUserFeedback implements UserFeedback {
     private Map<String, AwaitingDecision> _awaitingDecisionMap = new HashMap<String, AwaitingDecision>();
-    private Map<String, String> _warnings = new HashMap<String, String>();
 
     private LotroGame _game;
 
@@ -33,24 +32,11 @@ public class DefaultUserFeedback implements UserFeedback {
         _game.getGameState().playerDecisionStarted(playerId, awaitingDecision);
     }
 
-    @Override
-    public void sendWarning(String playerId, String warning) {
-        _warnings.put(playerId, warning);
-    }
-
     @Override
     public boolean hasPendingDecisions() {
         return _awaitingDecisionMap.size() > 0;
     }
 
-    public String consumeWarning(String playerId) {
-        return _warnings.remove(playerId);
-    }
-
-    public boolean hasWarning(String playerId) {
-        return _warnings.containsKey(playerId);
-    }
-
     public Set<String> getUsersPendingDecision() {
         return _awaitingDecisionMap.keySet();
     }

gemp-lotr/gemp-lotr-server/src/main/java/com/gempukku/lotro/game/LotroGameMediator.java

@@ -301,8 +301,7 @@ public class LotroGameMediator {
     }
 
     public void cancel(Player player) {
-        if (!_cancellable)
+        _lotroGame.getGameState().sendWarning(player.getName(), "You can't cancel this game");
-            _userFeedback.sendWarning(player.getName(), "You can't cancel this game");
 
         String playerId = player.getName();
         _writeLock.lock();
@@ -336,7 +335,7 @@ public class LotroGameMediator {
 
                             } catch (DecisionResultInvalidException decisionResultInvalidException) {
                                 // Participant provided wrong answer - send a warning message, and ask again for the same decision
-                                _userFeedback.sendWarning(playerName, decisionResultInvalidException.getWarningMessage());
+                                _lotroGame.getGameState().sendWarning(playerName, decisionResultInvalidException.getWarningMessage());
                                 _userFeedback.sendAwaitingDecision(playerName, awaitingDecision);
                             } catch (RuntimeException runtimeException) {
                                 LOG.error("Error processing game decision", runtimeException);
@@ -384,10 +383,6 @@ public class LotroGameMediator {
             for (GameEvent gameEvent : communicationChannel.consumeGameEvents())
                 visitor.visitGameEvent(gameEvent);
 
-            String warning = _userFeedback.consumeWarning(playerName);
-            if (warning != null)
-                visitor.visitGameEvent(new GameEvent(GameEvent.Type.SEND_WARNING).message(warning));
-
             Map<String, Integer> secondsLeft = new HashMap<String, Integer>();
             for (Map.Entry<String, Integer> playerClock : _playerClocks.entrySet()) {
                 String playerClockName = playerClock.getKey();

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

@@ -1410,4 +1410,59 @@ public class IndividualCardAtTest extends AbstractAtTest {
 
         System.out.println();
     }
+
+    @Test
+    public void FarinStrengthBoostWhileSkirmishingOrc() throws DecisionResultInvalidException, CardNotFoundException {
+        //Pre-game setup
+        initializeSimplestGame();
+
+        // Farin, Dwarven Emissary; 5 strength, "while skirmishing an Orc, Farin is strength +2"
+        final PhysicalCardImpl farin = createCard(P1, "1_11");
+        _game.getGameState().addCardToZone(_game, farin, Zone.FREE_CHARACTERS);
+
+        //Orc Scouting Band, 8 strength
+        final PhysicalCardImpl orc = createCard(P2, "1_270");
+        _game.getGameState().addCardToZone(_game, orc, Zone.SHADOW_CHARACTERS);
+
+        skipMulligans();
+
+        _game.getGameState().setTwilight(10);
+
+        playerDecided(P1, "");
+
+        // Pass in shadow
+        playerDecided(P2, "");
+
+        //Pass in maneuver
+        playerDecided(P1, "");
+        playerDecided(P2, "");
+
+        //Pass in archery
+        playerDecided(P1, "");
+        playerDecided(P2, "");
+
+        //Pass in assignment
+        playerDecided(P1, "");
+        playerDecided(P2, "");
+
+        // assign Farin to an Orc
+        playerDecided(P1, farin.getCardId() + " " + orc.getCardId());
+
+        //Farin's strength is still the base of 5
+        assertEquals(5, _game.getModifiersQuerying().getStrength(_game, farin));
+
+        //Choose to resolve Farin's skirmish
+        playerDecided(P1, "" + farin.getCardId());
+
+        //This test fails.  Farin has two instances of the ModifyStrength modifier applied for some reason,
+        // which puts him at 9 strength instead of the expected 7.
+        assertEquals(5 + 2, _game.getModifiersQuerying().getStrength(_game, farin));
+
+        playerDecided(P1, "");
+        playerDecided(P2, "");
+
+        // If the above test is commented out, then Farin wins the skirmish and applies skirmish wounds to the Orc
+        assertEquals(1, _game.getGameState().getWounds(farin));
+        assertEquals(0, _game.getGameState().getWounds(orc));
+    }
 }