From ba4fee11f66c7e1e7e1baae7a8953ca0eb279613 Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Sat, 27 Jan 2018 13:59:38 +0400 Subject: [PATCH] Fixed weird errors with server lost of player priority on lagged games (see #4448) --- .../src/mage/player/human/HumanPlayer.java | 69 +++++++++++++++++-- .../org/mage/test/load/SimpleMageClient.java | 18 ++++- 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/HumanPlayer.java b/Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/HumanPlayer.java index 96628efb21..9619fff72a 100644 --- a/Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/HumanPlayer.java +++ b/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(); - logger.debug("Waiting response from player: " + getId()); + response.clear(); // TODO: only one response for all games (can play only one game per session)?! + //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 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 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 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 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 blockers, CombatGroup combatGroup, List 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 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 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 pile1, List 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(); diff --git a/Mage.Tests/src/test/java/org/mage/test/load/SimpleMageClient.java b/Mage.Tests/src/test/java/org/mage/test/load/SimpleMageClient.java index 7dcca370c6..369e9671ce 100644 --- a/Mage.Tests/src/test/java/org/mage/test/load/SimpleMageClient.java +++ b/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(); + } }