Fixed weird errors with server lost of player priority on lagged games (see #4448)

Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/HumanPlayer.java

@@ -74,6 +74,7 @@ import mage.target.common.TargetDefender;
 import mage.util.GameLog;
 import mage.util.ManaUtil;
 import mage.util.MessageToClient;
+import mage.util.RandomUtil;
 import org.apache.log4j.Logger;
 
 /**
@@ -82,6 +83,7 @@ import org.apache.log4j.Logger;
  */
 public class HumanPlayer extends PlayerImpl {
 
+    private transient Boolean responseOpenedForAnswer = false; // can't get response until prepared target (e.g. until send all fire events to all players)
     private final transient PlayerResponse response = new PlayerResponse();
 
     protected static FilterCreatureForCombatBlock filterCreatureForCombatBlock = new FilterCreatureForCombatBlock();
@@ -148,6 +150,17 @@ public class HumanPlayer extends PlayerImpl {
                 || (actionIterations > 0 && !actionQueueSaved.isEmpty()));
     }
 
+    protected void waitResponseOpen() {
+        // wait response open for answer process
+        while(!responseOpenedForAnswer && canRespond()) {
+            try {
+                Thread.sleep(100);
+            } catch (InterruptedException e) {
+                logger.warn("Response waiting interrapted for " + getId());
+            }
+        }
+    }
+
     protected boolean pullResponseFromQueue(Game game) {
         if (actionQueue.isEmpty() && actionIterations > 0 && !actionQueueSaved.isEmpty()) {
             actionQueue = new LinkedList(actionQueueSaved);
@@ -164,6 +177,7 @@ public class HumanPlayer extends PlayerImpl {
                 }
                 sendPlayerAction(PlayerAction.PASS_PRIORITY_UNTIL_STACK_RESOLVED, game, null);
             }
+            waitResponseOpen();
             synchronized (response) {
                 response.copy(action);
                 response.notifyAll();
@@ -174,6 +188,11 @@ public class HumanPlayer extends PlayerImpl {
         return false;
     }
 
+    protected void prepareForResponse(Game game) {
+        //logger.info("Prepare waiting " + getId());
+        responseOpenedForAnswer = false;
+    }
+
     protected void waitForResponse(Game game) {
         if (isExecutingMacro()) {
             pullResponseFromQueue(game);
@@ -184,21 +203,27 @@ public class HumanPlayer extends PlayerImpl {
 //            }
             return;
         }
-        response.clear();
+        response.clear(); // TODO: only one response for all games (can play only one game per session)?!
-        logger.debug("Waiting response from player: " + getId());
+        //logger.info("Waiting response from player: " + getId());
         game.resumeTimer(getTurnControlledBy());
+        responseOpenedForAnswer = true; // start waiting for next response
         boolean loop = true;
         while (loop) {
             loop = false;
+
             synchronized (response) {
                 try {
+                    //logger.info("wait start: " + getId());
                     response.wait();
+                    //logger.info("wait end: " + getId());
                 } catch (InterruptedException ex) {
                     logger.error("Response error for player " + getName() + " gameId: " + game.getId(), ex);
                 } finally {
+                    responseOpenedForAnswer = false;
                     game.pauseTimer(getTurnControlledBy());
                 }
             }
+
             if (response.getResponseConcedeCheck()) {
                 ((GameImpl) game).checkConcede();
                 if (game.hasEnded()) {
@@ -210,6 +235,7 @@ public class HumanPlayer extends PlayerImpl {
                 }
             }
         }
+        //logger.info("Waiting response DONE (res queue " + actionQueueSaved.size() + "): " + getId() + ", res: " + response.toString());
         if (recordingMacro && !macroTriggeredSelectionFlag) {
 //            logger.info("Adding an action " + response);
             actionQueueSaved.add(new PlayerResponse(response));
@@ -227,6 +253,7 @@ public class HumanPlayer extends PlayerImpl {
             Map<String, Serializable> options = new HashMap<>();
             options.put("UI.left.btn.text", "Mulligan");
             options.put("UI.right.btn.text", "Keep");
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireAskPlayerEvent(playerId, new MessageToClient(message), null, options);
             }
@@ -269,6 +296,7 @@ public class HumanPlayer extends PlayerImpl {
             if (messageToClient.getSecondMessage() == null) {
                 messageToClient.setSecondMessage(getRelatedObjectName(source, game));
             }
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireAskPlayerEvent(playerId, messageToClient, source, options);
             }
@@ -330,10 +358,11 @@ public class HumanPlayer extends PlayerImpl {
         replacementEffectChoice.setKeyChoices(rEffects);
 
         while (!abort) {
+            updateGameStatePriority("chooseEffect", game);
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireChooseChoiceEvent(playerId, replacementEffectChoice);
             }
-            updateGameStatePriority("chooseEffect", game);
             waitForResponse(game);
             logger.debug("Choose effect: " + response.getString());
             if (response.getString() != null) {
@@ -367,6 +396,7 @@ public class HumanPlayer extends PlayerImpl {
         }
         updateGameStatePriority("choose(3)", game);
         while (!abort) {
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireChooseChoiceEvent(playerId, choice);
             }
@@ -416,6 +446,7 @@ public class HumanPlayer extends PlayerImpl {
             List<UUID> chosen = target.getTargets();
             options.put("chosen", (Serializable) chosen);
 
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireSelectTargetEvent(getId(), new MessageToClient(target.getMessage(), getRelatedObjectName(sourceId, game)), targetIds, required, getOptions(target, options));
             }
@@ -483,6 +514,7 @@ public class HumanPlayer extends PlayerImpl {
                 required = false;
             }
 
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireSelectTargetEvent(getId(), new MessageToClient(target.getMessage(), getRelatedObjectName(source, game)), possibleTargets, required, getOptions(target, null));
             }
@@ -553,6 +585,7 @@ public class HumanPlayer extends PlayerImpl {
                 options.put("choosable", (Serializable) choosable);
             }
 
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireSelectTargetEvent(playerId, new MessageToClient(target.getMessage()), cards, required, options);
             }
@@ -617,6 +650,7 @@ public class HumanPlayer extends PlayerImpl {
             if (!isExecutingMacro()) {
                 game.fireSelectTargetEvent(playerId, new MessageToClient(target.getMessage(), getRelatedObjectName(source, game)), cards, required, options);
             }
+            prepareForResponse(game);
             waitForResponse(game);
             if (response.getUUID() != null) {
                 if (target.getTargets().contains(response.getUUID())) { // if already included remove it
@@ -643,6 +677,7 @@ public class HumanPlayer extends PlayerImpl {
     public boolean chooseTargetAmount(Outcome outcome, TargetAmount target, Ability source, Game game) {
         updateGameStatePriority("chooseTargetAmount", game);
         while (!abort) {
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireSelectTargetEvent(playerId, new MessageToClient(target.getMessage() + "\n Amount remaining:" + target.getAmountRemaining(), getRelatedObjectName(source, game)),
                         target.possibleTargets(source == null ? null : source.getSourceId(), playerId, game),
@@ -786,6 +821,7 @@ public class HumanPlayer extends PlayerImpl {
             while (canRespond()) {
                 updateGameStatePriority("priority", game);
                 holdingPriority = false;
+                prepareForResponse(game);
                 if (!isExecutingMacro()) {
                     game.firePriorityEvent(playerId);
                 }
@@ -920,6 +956,7 @@ public class HumanPlayer extends PlayerImpl {
             }
             macroTriggeredSelectionFlag = true;
             updateGameStatePriority("chooseTriggeredAbility", game);
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireSelectTargetTriggeredAbilityEvent(playerId, "Pick triggered ability (goes to the stack first)", abilitiesWithNoOrderSet);
             }
@@ -956,6 +993,7 @@ public class HumanPlayer extends PlayerImpl {
     protected boolean playManaHandling(Ability abilityToCast, ManaCost unpaid, String promptText, Game game) {
         updateGameStatePriority("playMana", game);
         Map<String, Serializable> options = new HashMap<>();
+        prepareForResponse(game);
         if (!isExecutingMacro()) {
             game.firePlayManaEvent(playerId, "Pay " + promptText, options);
         }
@@ -992,6 +1030,7 @@ public class HumanPlayer extends PlayerImpl {
         int xValue = 0;
         updateGameStatePriority("announceRepetitions", game);
         do {
+            prepareForResponse(game);
             game.fireGetAmountEvent(playerId, "How many times do you want to repeat your shortcut?", 0, 999);
             waitForResponse(game);
         } while (response.getInteger() == null
@@ -1018,6 +1057,7 @@ public class HumanPlayer extends PlayerImpl {
         int xValue = 0;
         updateGameStatePriority("announceXMana", game);
         do {
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireGetAmountEvent(playerId, message, min, max);
             }
@@ -1036,6 +1076,7 @@ public class HumanPlayer extends PlayerImpl {
         int xValue = 0;
         updateGameStatePriority("announceXCost", game);
         do {
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireGetAmountEvent(playerId, message, min, max);
             }
@@ -1107,6 +1148,7 @@ public class HumanPlayer extends PlayerImpl {
                 options.put(Constants.Option.SPECIAL_BUTTON, (Serializable) "All attack");
             }
 
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireSelectEvent(playerId, "Select attackers", options);
             }
@@ -1326,6 +1368,7 @@ public class HumanPlayer extends PlayerImpl {
             return;
         }
         while (!abort) {
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireSelectEvent(playerId, "Select blockers");
             }
@@ -1358,6 +1401,7 @@ public class HumanPlayer extends PlayerImpl {
     public UUID chooseAttackerOrder(List<Permanent> attackers, Game game) {
         updateGameStatePriority("chooseAttackerOrder", game);
         while (!abort) {
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireSelectTargetEvent(playerId, "Pick attacker", attackers, true);
             }
@@ -1377,6 +1421,7 @@ public class HumanPlayer extends PlayerImpl {
     public UUID chooseBlockerOrder(List<Permanent> blockers, CombatGroup combatGroup, List<UUID> blockerOrder, Game game) {
         updateGameStatePriority("chooseBlockerOrder", game);
         while (!abort) {
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireSelectTargetEvent(playerId, "Pick blocker", blockers, true);
             }
@@ -1395,6 +1440,7 @@ public class HumanPlayer extends PlayerImpl {
     protected void selectCombatGroup(UUID defenderId, UUID blockerId, Game game) {
         updateGameStatePriority("selectCombatGroup", game);
         TargetAttackingCreature target = new TargetAttackingCreature();
+        prepareForResponse(game);
         if (!isExecutingMacro()) {
             game.fireSelectTargetEvent(playerId, new MessageToClient("Select attacker to block", getRelatedObjectName(blockerId, game)),
                     target.possibleTargets(null, playerId, game), false, getOptions(target, null));
@@ -1447,6 +1493,7 @@ public class HumanPlayer extends PlayerImpl {
     public int getAmount(int min, int max, String message, Game game) {
         updateGameStatePriority("getAmount", game);
         do {
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireGetAmountEvent(playerId, message, min, max);
             }
@@ -1478,6 +1525,7 @@ public class HumanPlayer extends PlayerImpl {
         LinkedHashMap<UUID, SpecialAction> specialActions = game.getState().getSpecialActions().getControlledBy(playerId, false);
         if (!specialActions.isEmpty()) {
             updateGameStatePriority("specialAction", game);
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireGetChoiceEvent(playerId, name, null, new ArrayList<>(specialActions.values()));
             }
@@ -1494,6 +1542,7 @@ public class HumanPlayer extends PlayerImpl {
         LinkedHashMap<UUID, SpecialAction> specialActions = game.getState().getSpecialActions().getControlledBy(playerId, true);
         if (!specialActions.isEmpty()) {
             updateGameStatePriority("specialAction", game);
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireGetChoiceEvent(playerId, name, null, new ArrayList<>(specialActions.values()));
             }
@@ -1539,6 +1588,7 @@ public class HumanPlayer extends PlayerImpl {
             }
         }
 
+        prepareForResponse(game);
         if (!isExecutingMacro()) {
             game.fireGetChoiceEvent(playerId, name, object, new ArrayList<>(abilities.values()));
         }
@@ -1579,6 +1629,7 @@ public class HumanPlayer extends PlayerImpl {
                         return (SpellAbility) useableAbilities.values().iterator().next();
                     } else if (useableAbilities != null
                             && !useableAbilities.isEmpty()) {
+                        prepareForResponse(game);
                         if (!isExecutingMacro()) {
                             game.fireGetChoiceEvent(playerId, name, object, new ArrayList<>(useableAbilities.values()));
                         }
@@ -1631,6 +1682,7 @@ public class HumanPlayer extends PlayerImpl {
             if (!modeMap.isEmpty()) {
                 boolean done = false;
                 while (!done) {
+                    prepareForResponse(game);
                     if (!isExecutingMacro()) {
                         game.fireGetModeEvent(playerId, "Choose Mode", modeMap);
                     }
@@ -1660,6 +1712,7 @@ public class HumanPlayer extends PlayerImpl {
     public boolean choosePile(Outcome outcome, String message, List<? extends Card> pile1, List<? extends Card> pile2, Game game) {
         updateGameStatePriority("choosePile", game);
         do {
+            prepareForResponse(game);
             if (!isExecutingMacro()) {
                 game.fireChoosePileEvent(playerId, message, pile1, pile2);
             }
@@ -1673,6 +1726,7 @@ public class HumanPlayer extends PlayerImpl {
 
     @Override
     public void setResponseString(String responseString) {
+        waitResponseOpen();
         synchronized (response) {
             response.setString(responseString);
             response.notifyAll();
@@ -1682,6 +1736,7 @@ public class HumanPlayer extends PlayerImpl {
 
     @Override
     public void setResponseManaType(UUID manaTypePlayerId, ManaType manaType) {
+        waitResponseOpen();
         synchronized (response) {
             response.setManaType(manaType);
             response.setResponseManaTypePlayerId(manaTypePlayerId);
@@ -1692,6 +1747,7 @@ public class HumanPlayer extends PlayerImpl {
 
     @Override
     public void setResponseUUID(UUID responseUUID) {
+        waitResponseOpen();
         synchronized (response) {
             response.setUUID(responseUUID);
             response.notifyAll();
@@ -1701,15 +1757,17 @@ public class HumanPlayer extends PlayerImpl {
 
     @Override
     public void setResponseBoolean(Boolean responseBoolean) {
+        waitResponseOpen();
         synchronized (response) {
             response.setBoolean(responseBoolean);
             response.notifyAll();
-            logger.debug("Got response boolean from player: " + getId());
+            logger.info("Got response boolean from player: " + getId());
         }
     }
 
     @Override
     public void setResponseInteger(Integer responseInteger) {
+        waitResponseOpen();
         synchronized (response) {
             response.setInteger(responseInteger);
             response.notifyAll();
@@ -1720,6 +1778,7 @@ public class HumanPlayer extends PlayerImpl {
     @Override
     public void abort() {
         abort = true;
+        waitResponseOpen();
         synchronized (response) {
             response.notifyAll();
             logger.debug("Got cancel action from player: " + getId());
@@ -1728,6 +1787,7 @@ public class HumanPlayer extends PlayerImpl {
 
     @Override
     public void signalPlayerConcede() {
+        waitResponseOpen();
         synchronized (response) {
             response.setResponseConcedeCheck();
             response.notifyAll();
@@ -1737,6 +1797,7 @@ public class HumanPlayer extends PlayerImpl {
 
     @Override
     public void skip() {
+        waitResponseOpen();
         synchronized (response) {
             response.setInteger(0);
             response.notifyAll();

Mage.Tests/src/test/java/org/mage/test/load/SimpleMageClient.java

@@ -11,7 +11,7 @@ import org.apache.log4j.Logger;
 /**
  * For tests only
  *
- * @author noxx
+ * @author noxx, JayDi85
  */
 public class SimpleMageClient implements MageClient {
 
@@ -20,7 +20,7 @@ public class SimpleMageClient implements MageClient {
 
     private static final Logger log = Logger.getLogger(SimpleMageClient.class);
 
-    private final CallbackClient callbackClient;
+    private final LoadCallbackClient callbackClient;
 
     public SimpleMageClient() {
         clientId = UUID.randomUUID();
@@ -54,7 +54,11 @@ public class SimpleMageClient implements MageClient {
 
     @Override
     public void processCallback(ClientCallback callback) {
-        callbackClient.processCallback(callback);
+        try {
+            callbackClient.processCallback(callback);
+        } catch (Throwable e) {
+            log.error(e.getMessage(), e);
+        }
     }
 
     public void setSession(Session session) {
@@ -64,4 +68,12 @@ public class SimpleMageClient implements MageClient {
     public boolean isGameOver() {
         return ((LoadCallbackClient)callbackClient).isGameOver();
     }
+
+    public void setConcede(boolean needToConcede) {
+        this.callbackClient.setConcede(needToConcede);
+    }
+
+    public String getLastGameResult() {
+        return this.callbackClient.getLastGameResult();
+    }
 }