From c2ae1386ff638d728f6c398b9611127febc6c6f2 Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Fri, 7 Jun 2024 12:52:42 +0400 Subject: [PATCH] server: improved conceding logic with more protection from game errors/freeze (related to #11285, #11460) --- .../src/mage/player/human/HumanPlayer.java | 13 ++- .../java/org/mage/test/player/TestPlayer.java | 4 +- Mage/src/main/java/mage/game/GameImpl.java | 79 ++++++++++++++----- Mage/src/main/java/mage/players/Player.java | 2 +- .../main/java/mage/players/PlayerImpl.java | 2 +- 5 files changed, 72 insertions(+), 28 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 cc3d60a9fa7..16543fb4b62 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 @@ -2725,12 +2725,17 @@ public class HumanPlayer extends PlayerImpl { } @Override - public void signalPlayerConcede() { + public void signalPlayerConcede(boolean stopCurrentChooseDialog) { // waitResponseOpen(); // concede is async event, will be processed on first priority + + // may be executed in CALL, HEALTH, GAME and other threads + // so make sure another player can't break/stop currently choosing player + synchronized (response) { - response.setAsyncWantConcede(); - response.notifyAll(); - logger.debug("Set check concede for waiting player: " + getId()); + response.setAsyncWantConcede(); // tell game that it must check conceding players + if (stopCurrentChooseDialog) { + response.notifyAll(); // will force to stop a current waiting dialog (so game can continue) + } } } diff --git a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java index 7ce11898745..47f322d0da2 100644 --- a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java +++ b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java @@ -2919,8 +2919,8 @@ public class TestPlayer implements Player { } @Override - public void signalPlayerConcede() { - computerPlayer.signalPlayerConcede(); + public void signalPlayerConcede(boolean stopCurrentChooseDialog) { + computerPlayer.signalPlayerConcede(stopCurrentChooseDialog); } @Override diff --git a/Mage/src/main/java/mage/game/GameImpl.java b/Mage/src/main/java/mage/game/GameImpl.java index 6bdea86122c..a7ee3778604 100644 --- a/Mage/src/main/java/mage/game/GameImpl.java +++ b/Mage/src/main/java/mage/game/GameImpl.java @@ -815,33 +815,71 @@ public abstract class GameImpl implements Game { @Override public void setConcedingPlayer(UUID playerId) { - Player player = null; - if (state.getChoosingPlayerId() != null) { - player = getPlayer(state.getChoosingPlayerId()); - } else if (state.getPriorityPlayerId() != null) { - player = getPlayer(state.getPriorityPlayerId()); + // request to concede a player (can be called for any player at any moment by concede button, connection fail, etc) + // warning, it's important to process real concede in game thread only (on priority) + + // concede queue (who requested concede) + if (!concedingPlayers.contains(playerId)) { + concedingPlayers.add(playerId); } - if (player != null) { - if (!player.hasLeft() && player.isHuman()) { - if (!concedingPlayers.contains(playerId)) { - logger.debug("Game over for player Id: " + playerId + " gameId " + getId()); - concedingPlayers.add(playerId); - player.signalPlayerConcede(); // will be executed on next priority - } - } else { - // no asynchronous action so check directly - concedingPlayers.add(playerId); - checkConcede(); + + Player currentPriorityPlayer = null; + if (state.getPriorityPlayerId() != null) { + currentPriorityPlayer = getPlayer(state.getPriorityPlayerId()); // started game + } else if (state.getChoosingPlayerId() != null) { + currentPriorityPlayer = getPlayer(state.getChoosingPlayerId()); // not started game + } + + // if something wrong with a game - it's not started, freeze, etc + if (currentPriorityPlayer == null) { + // try to stop + logger.warn("Game don't have priority player - checking game end: " + this); + if (!ThreadUtils.isRunGameThread()) { + // TODO: if server has that logs then it must be researched and fixed + logger.error("Non-game thread can't concede or end games - someone called it from freeze game?"); } - } else { - checkConcede(); + checkConcede(false); checkIfGameIsOver(); + return; + } + + // if someone requested concede + if (currentPriorityPlayer.getId().equals(playerId)) { + // concede for itself + // stop current player dialog and execute concede + currentPriorityPlayer.signalPlayerConcede(true); + } else { + // concede for another player + // allow current player to continue and check concede on any next priority + currentPriorityPlayer.signalPlayerConcede(false); + } + + // game thread can call concede directly + if (ThreadUtils.isRunGameThread()) { + // TODO: is it normal use case? If yes then remove logs + Player player = this.getPlayer(playerId); + logger.info(String.format("Game thread used concede request for (%s): %s", + player == null ? "null" : player.getName(), + this + )); + checkConcede(); } } public void checkConcede() { - while (!concedingPlayers.isEmpty()) { - leave(concedingPlayers.removeFirst()); + checkConcede(true); + } + + public void checkConcede(boolean mustRunInGameThread) { + // must run in game thread all the time + if (mustRunInGameThread) { + ThreadUtils.ensureRunInGameThread(); + } + + UUID playerId = concedingPlayers.poll(); + while (playerId != null) { + leave(playerId); + playerId = concedingPlayers.poll(); } } @@ -3922,6 +3960,7 @@ public abstract class GameImpl implements Game { @Override public synchronized void rollbackTurns(int turnsToRollback) { + // TODO: need async command if (gameOptions.rollbackTurnsAllowed && !executingRollback) { int turnToGoTo = getTurnNum() - turnsToRollback; if (turnToGoTo < 1 || !gameStatesRollBack.containsKey(turnToGoTo)) { diff --git a/Mage/src/main/java/mage/players/Player.java b/Mage/src/main/java/mage/players/Player.java index a6f42f5a906..5bbf056c2de 100644 --- a/Mage/src/main/java/mage/players/Player.java +++ b/Mage/src/main/java/mage/players/Player.java @@ -563,7 +563,7 @@ public interface Player extends MageItem, Copyable { void abortReset(); - void signalPlayerConcede(); + void signalPlayerConcede(boolean stopCurrentChooseDialog); void signalPlayerCheat(); diff --git a/Mage/src/main/java/mage/players/PlayerImpl.java b/Mage/src/main/java/mage/players/PlayerImpl.java index ac9e166ff28..9b034246e28 100644 --- a/Mage/src/main/java/mage/players/PlayerImpl.java +++ b/Mage/src/main/java/mage/players/PlayerImpl.java @@ -5232,7 +5232,7 @@ public abstract class PlayerImpl implements Player, Serializable { } @Override - public void signalPlayerConcede() { + public void signalPlayerConcede(boolean stopCurrentChooseDialog) { } @Override