From 53add718267f193c2bf4f4745e8898826cfe45be Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Fri, 24 Nov 2023 21:22:16 +0400 Subject: [PATCH] Improved network stability and other related fixes: * server: fixed that a critical errors ignored in user commands threads (now it will be added to the logs); * network: fixed frozen user responses in some use cases; * network: fixed accidental and incorrect user responses (only latest response will be used now); * network: improved freeze logs, added problem method name and code's line number; * cheats: removed outdated deck and card load logic (only init.txt commands supports now); * cheats: fixed wrong priority after add card dialog (closes #11437); * cheats: improved stability and random errors on cheat executes (related to #11437); * docs: added details on network and thread logic, human feedback life cycle, etc (see HumanPlayer, ThreadExecutorImpl); --- .../main/java/mage/client/SessionHandler.java | 4 +- .../java/mage/client/game/PlayAreaPanel.java | 2 +- .../java/mage/client/game/PlayerPanelExt.java | 3 +- .../card/dl/sources/CardImageSource.java | 4 - .../main/java/mage/interfaces/MageServer.java | 4 +- .../main/java/mage/remote/SessionImpl.java | 4 +- .../java/mage/remote/interfaces/Testable.java | 2 +- .../src/main/java/mage/utils}/SystemUtil.java | 46 +-- .../src/mage/player/human/HumanPlayer.java | 264 ++++++++++++------ .../src/mage/player/human/PlayerResponse.java | 161 +++++++---- .../java/mage/server/ChatManagerImpl.java | 2 +- .../main/java/mage/server/MageServerImpl.java | 27 +- .../src/main/java/mage/server/Main.java | 1 + .../src/main/java/mage/server/Session.java | 2 +- .../src/main/java/mage/server/User.java | 2 +- .../java/mage/server/game/GameController.java | 41 +-- .../mage/server/game/GameManagerImpl.java | 13 +- .../mage/server/managers/GameManager.java | 4 +- .../mage/server/util/ThreadExecutorImpl.java | 53 +++- .../java/org/mage/test/player/TestPlayer.java | 5 + .../serverside/base/MageTestPlayerBase.java | 2 +- .../base/impl/CardTestPlayerAPIImpl.java | 2 +- .../serverside/cheats/LoadCheatsTest.java | 4 +- .../java/org/mage/test/stub/PlayerStub.java | 5 + .../org/mage/test/testapi/AddCardApiTest.java | 2 +- .../java/mage/verify/VerifyCardDataTest.java | 2 +- .../java/mage/cards/decks/DeckCardLists.java | 30 +- Mage/src/main/java/mage/game/Game.java | 1 - Mage/src/main/java/mage/game/GameImpl.java | 5 +- Mage/src/main/java/mage/game/GameState.java | 2 +- Mage/src/main/java/mage/players/Player.java | 2 + .../main/java/mage/players/PlayerImpl.java | 3 + Mage/src/main/java/mage/util/DebugUtil.java | 45 +++ 33 files changed, 476 insertions(+), 273 deletions(-) rename {Mage.Server/src/main/java/mage/server/util => Mage.Common/src/main/java/mage/utils}/SystemUtil.java (95%) diff --git a/Mage.Client/src/main/java/mage/client/SessionHandler.java b/Mage.Client/src/main/java/mage/client/SessionHandler.java index c9b876d614b..51cadb7aca3 100644 --- a/Mage.Client/src/main/java/mage/client/SessionHandler.java +++ b/Mage.Client/src/main/java/mage/client/SessionHandler.java @@ -225,8 +225,8 @@ public final class SessionHandler { return session.isTestMode(); } - public static void cheat(UUID gameId, UUID playerId, DeckCardLists deckCardLists) { - session.cheat(gameId, playerId, deckCardLists); + public static void cheatShow(UUID gameId, UUID playerId) { + session.cheatShow(gameId, playerId); } public static String getSessionId() { diff --git a/Mage.Client/src/main/java/mage/client/game/PlayAreaPanel.java b/Mage.Client/src/main/java/mage/client/game/PlayAreaPanel.java index fde259e3271..b767676d2e9 100644 --- a/Mage.Client/src/main/java/mage/client/game/PlayAreaPanel.java +++ b/Mage.Client/src/main/java/mage/client/game/PlayAreaPanel.java @@ -588,7 +588,7 @@ public class PlayAreaPanel extends javax.swing.JPanel { } private void btnCheatActionPerformed(java.awt.event.ActionEvent evt) { - SessionHandler.cheat(gameId, playerId, DeckImporter.importDeckFromFile("cheat.dck", false)); + SessionHandler.cheatShow(gameId, playerId); } public boolean isSmallMode() { diff --git a/Mage.Client/src/main/java/mage/client/game/PlayerPanelExt.java b/Mage.Client/src/main/java/mage/client/game/PlayerPanelExt.java index 2b9690300c3..c36fecddd38 100644 --- a/Mage.Client/src/main/java/mage/client/game/PlayerPanelExt.java +++ b/Mage.Client/src/main/java/mage/client/game/PlayerPanelExt.java @@ -970,8 +970,7 @@ public class PlayerPanelExt extends javax.swing.JPanel { } private void btnCheatActionPerformed(java.awt.event.ActionEvent evt) { - DckDeckImporter deckImporter = new DckDeckImporter(); - SessionHandler.cheat(gameId, playerId, deckImporter.importDeck("cheat.dck", false)); + SessionHandler.cheatShow(gameId, playerId); } private void btnToolHintsHelperActionPerformed(java.awt.event.ActionEvent evt) { diff --git a/Mage.Client/src/main/java/org/mage/plugins/card/dl/sources/CardImageSource.java b/Mage.Client/src/main/java/org/mage/plugins/card/dl/sources/CardImageSource.java index 47615d82699..4b3d024afc4 100644 --- a/Mage.Client/src/main/java/org/mage/plugins/card/dl/sources/CardImageSource.java +++ b/Mage.Client/src/main/java/org/mage/plugins/card/dl/sources/CardImageSource.java @@ -64,8 +64,4 @@ public interface CardImageSource { default boolean isTokenImageProvided(String setCode, String cardName, Integer tokenNumber) { return false; } - - default int getDownloadTimeoutMs() { - return 0; - } } diff --git a/Mage.Common/src/main/java/mage/interfaces/MageServer.java b/Mage.Common/src/main/java/mage/interfaces/MageServer.java index 07c794eb626..8be6214173a 100644 --- a/Mage.Common/src/main/java/mage/interfaces/MageServer.java +++ b/Mage.Common/src/main/java/mage/interfaces/MageServer.java @@ -155,9 +155,7 @@ public interface MageServer { void replaySkipForward(UUID gameId, String sessionId, int moves) throws MageException; - void cheatMultiple(UUID gameId, String sessionId, UUID playerId, DeckCardLists deckList) throws MageException; - - boolean cheatOne(UUID gameId, String sessionId, UUID playerId, String cardName) throws MageException; + void cheatShow(UUID gameId, String sessionId, UUID playerId) throws MageException; List adminGetUsers(String sessionId) throws MageException; diff --git a/Mage.Common/src/main/java/mage/remote/SessionImpl.java b/Mage.Common/src/main/java/mage/remote/SessionImpl.java index c94dc8598c4..3443b3d7e48 100644 --- a/Mage.Common/src/main/java/mage/remote/SessionImpl.java +++ b/Mage.Common/src/main/java/mage/remote/SessionImpl.java @@ -1499,10 +1499,10 @@ public class SessionImpl implements Session { } @Override - public boolean cheat(UUID gameId, UUID playerId, DeckCardLists deckList) { + public boolean cheatShow(UUID gameId, UUID playerId) { try { if (isConnected()) { - server.cheatMultiple(gameId, sessionId, playerId, deckList); + server.cheatShow(gameId, sessionId, playerId); return true; } } catch (MageException ex) { diff --git a/Mage.Common/src/main/java/mage/remote/interfaces/Testable.java b/Mage.Common/src/main/java/mage/remote/interfaces/Testable.java index 07b8b501050..96c362178ca 100644 --- a/Mage.Common/src/main/java/mage/remote/interfaces/Testable.java +++ b/Mage.Common/src/main/java/mage/remote/interfaces/Testable.java @@ -12,5 +12,5 @@ public interface Testable { boolean isTestMode(); - boolean cheat(UUID gameId, UUID playerId, DeckCardLists deckList); + boolean cheatShow(UUID gameId, UUID playerId); } diff --git a/Mage.Server/src/main/java/mage/server/util/SystemUtil.java b/Mage.Common/src/main/java/mage/utils/SystemUtil.java similarity index 95% rename from Mage.Server/src/main/java/mage/server/util/SystemUtil.java rename to Mage.Common/src/main/java/mage/utils/SystemUtil.java index 3ea3bdbb6e8..38f26264894 100644 --- a/Mage.Server/src/main/java/mage/server/util/SystemUtil.java +++ b/Mage.Common/src/main/java/mage/utils/SystemUtil.java @@ -1,4 +1,4 @@ -package mage.server.util; +package mage.utils; import mage.MageObject; import mage.abilities.Ability; @@ -248,23 +248,15 @@ public final class SystemUtil { } /** - * Replaces cards in player's hands by specified in config/init.txt.
- *
- * Implementation note:
- * 1. Read init.txt line by line
- * 2. Parse line using for searching groups like: [group 1] 3. Parse line - * using the following format: line ::= - * :::
- * 4. If zone equals to 'hand', add card to player's library
- * 5a. Then swap added card with any card in player's hand
- * 5b. Parse next line (go to 2.), If EOF go to 4.
- * 6. Log message to all players that cards were added (to prevent unfair - * play).
- * 7. Exit
+ * Execute cheat commands from an init.txt file, for more details see + * wiki page + * about testing tools * * @param game + * @param commandsFilePath file path with commands in init.txt format + * @param feedbackPlayer player to execute that cheats (will see choose dialogs) */ - public static void addCardsForTesting(Game game, String fileSource, Player feedbackPlayer) { + public static void executeCheatCommands(Game game, String commandsFilePath, Player feedbackPlayer) { // fake test ability for triggers and events Ability fakeSourceAbilityTemplate = new SimpleStaticAbility(Zone.OUTSIDE, new InfoEffect("adding testing cards")); @@ -272,7 +264,7 @@ public final class SystemUtil { List errorsList = new ArrayList<>(); try { - String fileName = fileSource; + String fileName = commandsFilePath; if (fileName == null) { fileName = INIT_FILE_PATH; } @@ -755,11 +747,12 @@ public final class SystemUtil { } } } catch (Exception e) { - String mes = String.format("Catch critical error: %s", e.getMessage()); + String mes = String.format("Catch critical error on cheating: %s", e.getMessage()); errorsList.add(mes); logger.error(mes, e); + } finally { + sendCheatCommandsFeedback(game, feedbackPlayer, errorsList); } - sendCheatCommandsFeedback(game, feedbackPlayer, errorsList); } private static void sendCheatCommandsFeedback(Game game, Player feedbackPlayer, List errorsList) { @@ -907,4 +900,21 @@ public final class SystemUtil { } return Arrays.asList(cardSet, cardName); } + + public static void ensureRunInGameThread() { + String name = Thread.currentThread().getName(); + if (!name.startsWith("GAME")) { + // how-to fix: use signal logic to inform a game about new command to execute instead direct execute (see example with WantConcede) + // reason: user responses/commands are received by network/call thread, but must be processed by game thread + throw new IllegalArgumentException("Wrong code usage: game related code must run in GAME thread, but it used in " + name, new Throwable()); + } + } + + public static void ensureRunInCallThread() { + String name = Thread.currentThread().getName(); + if (!name.startsWith("CALL")) { + // how-to fix: something wrong in your code logic + throw new IllegalArgumentException("Wrong code usage: client commands code must run in CALL threads, but used in " + name, new Throwable()); + } + } } 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 1530a0e189d..c5e86c2a7c6 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 @@ -45,6 +45,7 @@ import mage.target.common.TargetAttackingCreature; import mage.target.common.TargetDefender; import mage.target.targetpointer.TargetPointer; import mage.util.*; +import mage.utils.SystemUtil; import org.apache.log4j.Logger; import java.awt.*; @@ -58,7 +59,7 @@ import static mage.constants.PlayerAction.REQUEST_AUTO_ANSWER_RESET_ALL; import static mage.constants.PlayerAction.TRIGGER_AUTO_ORDER_RESET_ALL; /** - * @author BetaSteward_at_googlemail.com + * @author BetaSteward_at_googlemail.com, JayDi85 */ public class HumanPlayer extends PlayerImpl { @@ -66,8 +67,36 @@ public class HumanPlayer extends PlayerImpl { // TODO: all user feedback actions executed and waited in diff threads and can't catch exeptions, e.g. on wrong code usage // must catch and log such errors - private transient Boolean responseOpenedForAnswer = false; // can't get response until prepared target (e.g. until send all fire events to all players) + + // Network and threads logic: + // * starting point: ThreadExecutorImpl.java + // * server executing a game's code by single game thread (named: "GAME xxx", one thread per game) + // * data transfering goes by inner jboss threads (named: "WorkerThread xxx", one thread per client connection) + // - from server to client: sync mode, a single game thread uses jboss networking to send data to each player and viewer/watcher one by one + // - from client to server: async mode, each income command executes by shared call thread + // + // Client commands logic: + // * commands can do anything in server, user, table and game contexts + // * it's income in async mode at any time and in any order by "CALL xxx" threads + // * there are two types of client commands: + // - feedback commands - must be executed and synced by GAME thread (example: answer for choose dialog, priority, etc) + // - other commands - can be executed at any time and don't require sync with a game and a GAME thread (example: user/skip settings, concede/cheat, chat messages) + // + // Feedback commands logic: + // * income by CALL thread + // * must be synced and executed by GAME thread + // * keep only latest income feedback (if user sends multiple clicks/choices) + // * HumanPlayer contains "response" object for threads sync and data exchange + // * so sync logic: + // * - GAME thread: open response for income command and wait (go to sleep by response.wait) + // * - CALL thread: on closed response - waiting open status of player's response object (if it's too long then cancel the answer) + // * - CALL thread: on opened response - save answer to player's response object and notify GAME thread about it by response.notifyAll + // * - GAME thread: on nofify from response - check new answer value and process it (if it bad then repeat and wait the next one); + private transient Boolean responseOpenedForAnswer = false; // GAME thread waiting new answer + private transient long responseLastWaitingThreadId = 0; private final transient PlayerResponse response = new PlayerResponse(); + private final int RESPONSE_WAITING_TIME_SECS = 30; // waiting time before cancel current response + private final int RESPONSE_WAITING_CHECK_MS = 100; // timeout for open status check protected static FilterCreatureForCombatBlock filterCreatureForCombatBlock = new FilterCreatureForCombatBlock(); protected static FilterCreatureForCombat filterCreatureForCombat = new FilterCreatureForCombat(); @@ -141,24 +170,68 @@ public class HumanPlayer extends PlayerImpl { || (actionIterations > 0 && !actionQueueSaved.isEmpty())); } - protected void waitResponseOpen() { - // wait response open for answer process - int numTimesWaiting = 0; + /** + * Waiting for opened response and save new value in it + * Use it in CALL threads only, e.g. for client commands + * + * @return on true result game can use response value, on false result - it's outdated response + */ + protected boolean waitResponseOpen() { + // client send commands in async mode and can come too early + // so if next command come too fast then wait here until game ready + // + // example with too early response: + // * game must send new update to 3 users and ask user 2 for feedback answer, but user 3 is too slow + // +0 secs: start sending data to 3 players + // +0 secs: user 1 getting data + // +1 secs: user 1 done + // +1 secs: user 2 getting data + // +3 secs: user 2 done + // +3 secs: user 3 getting data (it's slow) + // +4 secs: user 2 sent answer 1 (but game closed for it, so waiting) + // +5 secs: user 2 sent answer 2 (he can't see changes after answer 1, so trying again - server must keep only latest answer) + // +8 secs: user 3 done + // +8 secs: game start wating a new answer from user 2 + // +8 secs: game find answer + + int currentTimesWaiting = 0; + int maxTimesWaiting = RESPONSE_WAITING_TIME_SECS * 1000 / RESPONSE_WAITING_CHECK_MS; + long currentThreadId = Thread.currentThread().getId(); + // it's a latest response + responseLastWaitingThreadId = currentThreadId; while (!responseOpenedForAnswer && canRespond()) { - numTimesWaiting++; - if (numTimesWaiting >= 300) { - // game frozen -- need to report about error and continue to execute - String s = "Game frozen in waitResponseOpen for user " + getName() + " (connection problem)"; - logger.warn(s); - break; + if (responseLastWaitingThreadId != currentThreadId) { + // there is another latest response, so cancel current + return false; + } + + // keep waiting + currentTimesWaiting++; + if (currentTimesWaiting > maxTimesWaiting) { + // game frozen, possible reasons: + // * ANOTHER player lost connection and GAME thread trying to send data to him + // * current player send answer, but lost connect after it + String possibleReason; + if (response.getActiveAction() == null) { + possibleReason = "maybe connection problem with another player/watcher"; + } else { + possibleReason = "something wrong with your priority on " + response.getActiveAction(); + } + logger.warn(String.format("Game frozen in waitResponseOpen for %d secs: current user %s, %s", + RESPONSE_WAITING_CHECK_MS * currentTimesWaiting / 1000, + this.getName(), + possibleReason + )); + return false; } try { - Thread.sleep(100); - } catch (InterruptedException e) { - logger.warn("Response waiting interrupted for " + getId()); + Thread.sleep(RESPONSE_WAITING_CHECK_MS); + } catch (InterruptedException ignore) { } } + + return true; // can use new value } protected boolean pullResponseFromQueue(Game game) { @@ -179,7 +252,7 @@ public class HumanPlayer extends PlayerImpl { } //waitResponseOpen(); // it's a macro action, no need it here? synchronized (response) { - response.copy(action); + response.copyFrom(action); response.notifyAll(); macroTriggeredSelectionFlag = false; return true; @@ -188,12 +261,39 @@ public class HumanPlayer extends PlayerImpl { return false; } + /** + * Prepare priority player for new feedback, call it for every choose cycle before waitForResponse + */ protected void prepareForResponse(Game game) { - //logger.info("Prepare waiting " + getId()); + SystemUtil.ensureRunInGameThread(); + + // prepare priority player + // on null - it's a discard in cleanaup and other non-user code, so don't change it here at that moment + // TODO: must research null use case + if (game.getState().getPriorityPlayerId() != null) { + if (getId() == null) { + logger.fatal("Player with no ID: " + name); + this.quit(game); + return; + } + if (logger.isDebugEnabled()) { + logger.debug("Setting game priority for " + getId() + " [" + DebugUtil.getMethodNameWithSource(2) + ']'); + } + game.getState().setPriorityPlayerId(getId()); + } + responseOpenedForAnswer = false; } + /** + * Waiting feedback from priority player + * + * @param game + */ protected void waitForResponse(Game game) { + SystemUtil.ensureRunInGameThread(); + ; + if (isExecutingMacro()) { pullResponseFromQueue(game); // logger.info("MACRO pull from queue: " + response.toString()); @@ -204,36 +304,46 @@ public class HumanPlayer extends PlayerImpl { return; } - // wait player's answer loop boolean loop = true; while (loop) { // start waiting for next answer response.clear(); + response.setActiveAction(DebugUtil.getMethodNameWithSource(2)); game.resumeTimer(getTurnControlledBy()); responseOpenedForAnswer = true; loop = false; - - synchronized (response) { + synchronized (response) { // TODO: synchronized response smells bad here, possible deadlocks? Need research try { - response.wait(); - } catch (InterruptedException ex) { - logger.error("Response error for player " + getName() + " gameId: " + game.getId(), ex); + response.wait(); // start waiting a response.notifyAll command from CALL thread (client answer) + } catch (InterruptedException ignore) { } finally { responseOpenedForAnswer = false; game.pauseTimer(getTurnControlledBy()); } } + // async command: concede by any player // game recived immediately response on OTHER player concede -- need to process end game and continue to wait - if (response.getResponseConcedeCheck()) { + // TODO: is it possible to break choose dialog of current player (check it in multiplayer)? + if (response.getAsyncWantConcede()) { ((GameImpl) game).checkConcede(); if (game.hasEnded()) { return; } - + // wait another answer + if (canRespond()) { + loop = true; + } + } + + // async command: cheat by current player + if (response.getAsyncWantCheat()) { + // execute cheats and continue + SystemUtil.executeCheatCommands(game, null, this); + game.fireUpdatePlayersEvent(); // need force to game update for new possible data + // wait another answer if (canRespond()) { - // wait another answer loop = true; } } @@ -265,7 +375,6 @@ public class HumanPlayer extends PlayerImpl { options.put("UI.left.btn.text", "Mulligan"); options.put("UI.right.btn.text", "Keep"); - updateGameStatePriority("chooseMulligan", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireAskPlayerEvent(playerId, new MessageToClient(message), null, options); @@ -323,7 +432,6 @@ public class HumanPlayer extends PlayerImpl { messageToClient.setSecondMessage(getRelatedObjectName(source, game)); } - updateGameStatePriority("chooseUse", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireAskPlayerEvent(playerId, messageToClient, source, options); @@ -415,7 +523,6 @@ public class HumanPlayer extends PlayerImpl { } while (canRespond()) { - updateGameStatePriority("chooseEffect", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireChooseChoiceEvent(playerId, replacementEffectChoice); @@ -493,7 +600,6 @@ public class HumanPlayer extends PlayerImpl { } while (canRespond()) { - updateGameStatePriority("choose(3)", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireChooseChoiceEvent(playerId, choice); @@ -556,7 +662,6 @@ public class HumanPlayer extends PlayerImpl { List chosenTargets = target.getTargets(); options.put("chosenTargets", (Serializable) chosenTargets); - updateGameStatePriority("choose(5)", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireSelectTargetEvent(getId(), new MessageToClient(target.getMessage(), getRelatedObjectName(source, game)), possibleTargetIds, required, getOptions(target, options)); @@ -658,7 +763,6 @@ public class HumanPlayer extends PlayerImpl { List chosenTargets = target.getTargets(); options.put("chosenTargets", (Serializable) chosenTargets); - updateGameStatePriority("chooseTarget", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireSelectTargetEvent(getId(), new MessageToClient(target.getMessage(), getRelatedObjectName(source, game)), @@ -757,7 +861,6 @@ public class HumanPlayer extends PlayerImpl { options.put("possibleTargets", (Serializable) possibleTargets); } - updateGameStatePriority("choose(4)", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireSelectTargetEvent(playerId, new MessageToClient(target.getMessage()), cards, required, options); @@ -841,7 +944,6 @@ public class HumanPlayer extends PlayerImpl { options.put("possibleTargets", (Serializable) possibleTargets); } - updateGameStatePriority("chooseTarget(5)", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireSelectTargetEvent(playerId, new MessageToClient(target.getMessage(), getRelatedObjectName(source, game)), cards, required, options); @@ -936,7 +1038,6 @@ public class HumanPlayer extends PlayerImpl { options.put("possibleTargets", (Serializable) possibleTargets); } - updateGameStatePriority("chooseTargetAmount", game); prepareForResponse(game); if (!isExecutingMacro()) { String multiType = multiAmountType == MultiAmountType.DAMAGE ? " to divide %d damage" : " to distribute %d counters"; @@ -1204,7 +1305,6 @@ public class HumanPlayer extends PlayerImpl { while (canRespond()) { holdingPriority = false; - updateGameStatePriority("priority", game); prepareForResponse(game); if (!isExecutingMacro()) { game.firePriorityEvent(playerId); @@ -1428,7 +1528,6 @@ 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); @@ -1477,7 +1576,6 @@ public class HumanPlayer extends PlayerImpl { // TODO: make canRespond cycle? if (canRespond()) { Map options = new HashMap<>(); - updateGameStatePriority("playMana", game); prepareForResponse(game); if (!isExecutingMacro()) { game.firePlayManaEvent(playerId, "Pay " + promptText, options); @@ -1486,17 +1584,19 @@ public class HumanPlayer extends PlayerImpl { UUID responseId = getFixedResponseUUID(game); if (response.getBoolean() != null) { + // cancel return false; } else if (responseId != null) { + // pay from mana object playManaAbilities(responseId, abilityToCast, unpaid, game); - } else if (response.getString() != null - && response.getString().equals("special")) { + } else if (response.getString() != null && response.getString().equals("special")) { + // pay from special action if (unpaid instanceof ManaCostsImpl) { activateSpecialAction(game, unpaid); } } else if (response.getManaType() != null) { - // this mana type can be paid once from pool - if (response.getResponseManaTypePlayerId().equals(this.getId())) { + // pay from own mana pool + if (response.getManaPlayerId().equals(this.getId())) { this.getManaPool().unlockManaType(response.getManaType()); } // TODO: Handle if mana pool @@ -1521,7 +1621,6 @@ public class HumanPlayer extends PlayerImpl { int xValue = 0; while (canRespond()) { - updateGameStatePriority("announceRepetitions", game); prepareForResponse(game); game.fireGetAmountEvent(playerId, "How many times do you want to repeat your shortcut?", 0, 999); waitForResponse(game); @@ -1557,7 +1656,6 @@ public class HumanPlayer extends PlayerImpl { int xValue = 0; String extraMessage = (multiplier == 1 ? "" : ", X will be increased by " + multiplier + " times"); while (canRespond()) { - updateGameStatePriority("announceXMana", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireGetAmountEvent(playerId, message + extraMessage, min, max); @@ -1583,7 +1681,6 @@ public class HumanPlayer extends PlayerImpl { int xValue = 0; while (canRespond()) { - updateGameStatePriority("announceXCost", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireGetAmountEvent(playerId, message, min, max); @@ -1602,7 +1699,6 @@ public class HumanPlayer extends PlayerImpl { } protected void playManaAbilities(UUID objectId, Ability abilityToCast, ManaCost unpaid, Game game) { - updateGameStatePriority("playManaAbilities", game); MageObject object = game.getObject(objectId); if (object == null) { return; @@ -1703,7 +1799,6 @@ public class HumanPlayer extends PlayerImpl { options.put(Constants.Option.SPECIAL_BUTTON, "All attack"); } - updateGameStatePriority("selectAttackers", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireSelectEvent(playerId, "Select attackers", options); @@ -1930,7 +2025,6 @@ public class HumanPlayer extends PlayerImpl { } while (canRespond()) { - updateGameStatePriority("selectBlockers", game); prepareForResponse(game); if (!isExecutingMacro()) { Map options = new HashMap<>(); @@ -1974,7 +2068,6 @@ public class HumanPlayer extends PlayerImpl { } while (canRespond()) { - updateGameStatePriority("chooseAttackerOrder", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireSelectTargetEvent(playerId, "Pick attacker", attackers, true); @@ -2000,7 +2093,6 @@ public class HumanPlayer extends PlayerImpl { } while (canRespond()) { - updateGameStatePriority("chooseBlockerOrder", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireSelectTargetEvent(playerId, "Pick blocker", blockers, true); @@ -2032,7 +2124,6 @@ public class HumanPlayer extends PlayerImpl { UUID responseId = null; - updateGameStatePriority("selectCombatGroup", game); prepareForResponse(game); if (!isExecutingMacro()) { // possible attackers to block @@ -2075,7 +2166,6 @@ public class HumanPlayer extends PlayerImpl { @Override public void assignDamage(int damage, java.util.List targets, String singleTargetName, UUID attackerId, Ability source, Game game) { - updateGameStatePriority("assignDamage", game); int remainingDamage = damage; while (remainingDamage > 0 && canRespond()) { Target target = new TargetAnyTarget(); @@ -2083,9 +2173,9 @@ public class HumanPlayer extends PlayerImpl { if (singleTargetName != null) { target.setTargetName(singleTargetName); } - choose(Outcome.Damage, target, source, game); + this.choose(Outcome.Damage, target, source, game); if (targets.isEmpty() || targets.contains(target.getFirstTarget())) { - int damageAmount = getAmount(0, remainingDamage, "Select amount", game); + int damageAmount = this.getAmount(0, remainingDamage, "Select amount", game); Permanent permanent = game.getPermanent(target.getFirstTarget()); if (permanent != null) { permanent.damage(damageAmount, attackerId, source, game, false, true); @@ -2108,7 +2198,6 @@ public class HumanPlayer extends PlayerImpl { } while (canRespond()) { - updateGameStatePriority("getAmount", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireGetAmountEvent(playerId, message, min, max); @@ -2149,7 +2238,6 @@ public class HumanPlayer extends PlayerImpl { List answer = null; while (canRespond()) { - updateGameStatePriority("getMultiAmount", game); prepareForResponse(game); if (!isExecutingMacro()) { Map options = new HashMap<>(2); @@ -2222,8 +2310,6 @@ public class HumanPlayer extends PlayerImpl { Map specialActions = game.getState().getSpecialActions().getControlledBy(playerId, unpaidForManaAction != null); if (!specialActions.isEmpty()) { - - updateGameStatePriority("specialAction", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireGetChoiceEvent(playerId, name, null, new ArrayList<>(specialActions.values())); @@ -2253,8 +2339,6 @@ public class HumanPlayer extends PlayerImpl { return; } - updateGameStatePriority("activateAbility", game); - if (abilities.size() == 1 && suppressAbilityPicker(abilities.values().iterator().next(), game)) { ActivatedAbility ability = abilities.values().iterator().next(); @@ -2281,7 +2365,8 @@ public class HumanPlayer extends PlayerImpl { message = message + "
" + object.getLogName(); } - // TODO: add canRespond cycle? + // it's inner method, parent code already uses while and canRespond cycle, + // so can request one time here prepareForResponse(game); if (!isExecutingMacro()) { game.fireGetChoiceEvent(playerId, message, object, new ArrayList<>(abilities.values())); @@ -2305,6 +2390,8 @@ public class HumanPlayer extends PlayerImpl { */ private boolean suppressAbilityPicker(ActivatedAbility ability, Game game) { if (getControllingPlayersUserData(game).isShowAbilityPickerForced()) { + // TODO: is it bugged on mana payment + under control? + // (if player under control then priority player must use own settings, not controlling) // user activated an ability picker in preferences // force to show ability picker for double faces cards in hand/commander/exile and other zones @@ -2355,7 +2442,6 @@ public class HumanPlayer extends PlayerImpl { } else if (useableAbilities != null && !useableAbilities.isEmpty()) { - updateGameStatePriority("chooseAbilityForCast", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireGetChoiceEvent(playerId, message, object, new ArrayList<>(useableAbilities.values())); @@ -2404,7 +2490,6 @@ public class HumanPlayer extends PlayerImpl { case 1: return useableAbilities.values().iterator().next(); default: - updateGameStatePriority("chooseLandOrSpellAbility", game); prepareForResponse(game); if (!isExecutingMacro()) { String message = "Choose spell or ability to play" + (noMana ? " for FREE" : "") + "
" + object.getLogName(); @@ -2491,7 +2576,6 @@ public class HumanPlayer extends PlayerImpl { message = message + "
" + obj.getLogName(); } - updateGameStatePriority("chooseMode", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireGetModeEvent(playerId, message, modeMap); @@ -2541,7 +2625,6 @@ public class HumanPlayer extends PlayerImpl { } while (canRespond()) { - updateGameStatePriority("choosePile", game); prepareForResponse(game); if (!isExecutingMacro()) { game.fireChoosePileEvent(playerId, message, pile1, pile2); @@ -2562,7 +2645,9 @@ public class HumanPlayer extends PlayerImpl { @Override public void setResponseString(String responseString) { - waitResponseOpen(); + if (!waitResponseOpen()) { + return; + } synchronized (response) { response.setString(responseString); response.notifyAll(); @@ -2572,10 +2657,12 @@ public class HumanPlayer extends PlayerImpl { @Override public void setResponseManaType(UUID manaTypePlayerId, ManaType manaType) { - waitResponseOpen(); + if (!waitResponseOpen()) { + return; + } synchronized (response) { response.setManaType(manaType); - response.setResponseManaTypePlayerId(manaTypePlayerId); + response.setResponseManaPlayerId(manaTypePlayerId); response.notifyAll(); logger.debug("Got response mana type from player: " + getId()); } @@ -2583,7 +2670,9 @@ public class HumanPlayer extends PlayerImpl { @Override public void setResponseUUID(UUID responseUUID) { - waitResponseOpen(); + if (!waitResponseOpen()) { + return; + } synchronized (response) { response.setUUID(responseUUID); response.notifyAll(); @@ -2593,7 +2682,9 @@ public class HumanPlayer extends PlayerImpl { @Override public void setResponseBoolean(Boolean responseBoolean) { - waitResponseOpen(); + if (!waitResponseOpen()) { + return; + } synchronized (response) { response.setBoolean(responseBoolean); response.notifyAll(); @@ -2603,7 +2694,9 @@ public class HumanPlayer extends PlayerImpl { @Override public void setResponseInteger(Integer responseInteger) { - waitResponseOpen(); + if (!waitResponseOpen()) { + return; + } synchronized (response) { response.setInteger(responseInteger); response.notifyAll(); @@ -2613,8 +2706,8 @@ public class HumanPlayer extends PlayerImpl { @Override public void abort() { + // abort must cancel any response and stop waiting immediately abort = true; - waitResponseOpen(); synchronized (response) { response.notifyAll(); logger.debug("Got cancel action from player: " + getId()); @@ -2623,17 +2716,28 @@ public class HumanPlayer extends PlayerImpl { @Override public void signalPlayerConcede() { - //waitResponseOpen(); //concede is direct event, no need to wait it + // waitResponseOpen(); // concede is async event, will be processed on first priority synchronized (response) { - response.setResponseConcedeCheck(); + response.setAsyncWantConcede(); response.notifyAll(); logger.debug("Set check concede for waiting player: " + getId()); } } + @Override + public void signalPlayerCheat() { + // waitResponseOpen(); // cheat is async event, will be processed on first player's priority + synchronized (response) { + response.setAsyncWantCheat(); + response.notifyAll(); + logger.debug("Set cheat for waiting player: " + getId()); + } + } + @Override public void skip() { // waitResponseOpen(); //skip is direct event, no need to wait it + // TODO: can be bugged and must be reworked, see wantConcede as example?! synchronized (response) { response.setInteger(0); response.notifyAll(); @@ -2646,20 +2750,6 @@ public class HumanPlayer extends PlayerImpl { return new HumanPlayer(this); } - protected void updateGameStatePriority(String methodName, Game game) { - // call that for every choose cycle before prepareForResponse - // (some choose logic can asks another question with different game state priority) - if (game.getState().getPriorityPlayerId() != null) { // don't do it if priority was set to null before (e.g. discard in cleanaup) - if (getId() == null) { - logger.fatal("Player with no ID: " + name); - this.quit(game); - return; - } - logger.debug("Setting game priority to " + getId() + " [" + methodName + ']'); - game.getState().setPriorityPlayerId(getId()); - } - } - @Override public void sendPlayerAction(PlayerAction playerAction, Game game, Object data) { switch (playerAction) { diff --git a/Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/PlayerResponse.java b/Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/PlayerResponse.java index d87141c5a55..6fdbe396e08 100644 --- a/Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/PlayerResponse.java +++ b/Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/PlayerResponse.java @@ -1,23 +1,40 @@ - package mage.player.human; +import mage.constants.ManaType; +import mage.util.Copyable; + import java.io.Serializable; import java.util.UUID; -import mage.constants.ManaType; /** + * Network: server side data for waiting a user's response like new choice + *

+ * Details: + * - one response object per user; + * - support multiple data types; + * - waiting and writing response on diff threads; + * - start by response.wait (game thread) and end by response.notifyAll (network/call thread) + * - user's request can income in diff order, so only one latest response allowed (except async commands like concede and cheat) * - * @author BetaSteward_at_googlemail.com + * @author BetaSteward_at_googlemail.com, JayDi85 */ -public class PlayerResponse implements Serializable { +public class PlayerResponse implements Serializable, Copyable { + + private String activeAction; // for debug information private String responseString; private UUID responseUUID; private Boolean responseBoolean; private Integer responseInteger; private ManaType responseManaType; - private UUID responseManaTypePlayerId; - private Boolean responseConcedeCheck; + private UUID responseManaPlayerId; + + // async commands can income any time from network thread as signal, + // must process it in game thread on any priority + // TODO: is concede/hand view confirmation can broken waiting cycle for other choosing/priority player + // (with same response type, with diff response type)??? + private Boolean asyncWantConcede; + private Boolean asyncWantCheat; public PlayerResponse() { clear(); @@ -25,96 +42,122 @@ public class PlayerResponse implements Serializable { @Override public String toString() { - return ((responseString == null) ? "null" : responseString) - + ',' + responseUUID - + ',' + responseBoolean - + ',' + responseInteger - + ',' + responseManaType - + ',' + responseManaTypePlayerId - + ',' + responseConcedeCheck; + return ((this.responseString == null) ? "null" : this.responseString) + + "," + this.responseUUID + + "," + this.responseBoolean + + "," + this.responseInteger + + "," + this.responseManaType + + "," + this.responseManaPlayerId + + "," + this.asyncWantConcede + + "," + this.asyncWantCheat + + " (" + (this.activeAction == null ? "sleep" : this.activeAction) + ")"; } - public PlayerResponse(PlayerResponse other) { - copy(other); + protected PlayerResponse(final PlayerResponse response) { + this.copyFrom(response); } - public void copy(PlayerResponse other) { - responseString = other.responseString; - responseUUID = other.responseUUID; - responseBoolean = other.responseBoolean; - responseInteger = other.responseInteger; - responseManaType = other.responseManaType; - responseManaTypePlayerId = other.responseManaTypePlayerId; - responseConcedeCheck = other.responseConcedeCheck; + @Override + public PlayerResponse copy() { + return new PlayerResponse(this); + } + + public void copyFrom(final PlayerResponse response) { + this.activeAction = response.activeAction; + this.responseString = response.responseString; + this.responseUUID = response.responseUUID; + this.responseBoolean = response.responseBoolean; + this.responseInteger = response.responseInteger; + this.responseManaType = response.responseManaType; + this.responseManaPlayerId = response.responseManaPlayerId; + this.asyncWantConcede = response.asyncWantConcede; + this.asyncWantCheat = response.asyncWantCheat; } public void clear() { - responseString = null; - responseUUID = null; - responseBoolean = null; - responseInteger = null; - responseManaType = null; - responseManaTypePlayerId = null; - responseConcedeCheck = null; + this.activeAction = null; + this.responseString = null; + this.responseUUID = null; + this.responseBoolean = null; + this.responseInteger = null; + this.responseManaType = null; + this.responseManaPlayerId = null; + this.asyncWantConcede = null; + this.asyncWantCheat = null; + } + + public String getActiveAction() { + return this.activeAction; + } + + public void setActiveAction(String activeAction) { + this.activeAction = activeAction; } public String getString() { - return responseString; + return this.responseString; } - public void setString(String responseString) { - this.responseString = responseString; + public void setString(String newString) { + this.responseString = newString; } public UUID getUUID() { - return responseUUID; + return this.responseUUID; } - public void setUUID(UUID responseUUID) { - this.responseUUID = responseUUID; + public void setUUID(UUID newUUID) { + this.responseUUID = newUUID; } public Boolean getBoolean() { - return responseBoolean; + return this.responseBoolean; } - public void setBoolean(Boolean responseBoolean) { - this.responseBoolean = responseBoolean; - } - - public Boolean getResponseConcedeCheck() { - if (responseConcedeCheck == null) { - return false; - } - return responseConcedeCheck; - } - - public void setResponseConcedeCheck() { - this.responseConcedeCheck = true; + public void setBoolean(Boolean newBoolean) { + this.responseBoolean = newBoolean; } public Integer getInteger() { return responseInteger; } - public void setInteger(Integer responseInteger) { - this.responseInteger = responseInteger; + public void setInteger(Integer newInteger) { + this.responseInteger = newInteger; } public ManaType getManaType() { - return responseManaType; + return this.responseManaType; } - public void setManaType(ManaType responseManaType) { - this.responseManaType = responseManaType; + public void setManaType(ManaType newManaType) { + this.responseManaType = newManaType; } - public UUID getResponseManaTypePlayerId() { - return responseManaTypePlayerId; + public UUID getManaPlayerId() { + return this.responseManaPlayerId; } - public void setResponseManaTypePlayerId(UUID responseManaTypePlayerId) { - this.responseManaTypePlayerId = responseManaTypePlayerId; + public void setResponseManaPlayerId(UUID newManaPlayerId) { + this.responseManaPlayerId = newManaPlayerId; } + public Boolean getAsyncWantConcede() { + return this.asyncWantConcede != null && this.asyncWantConcede; + } + + public void setAsyncWantConcede() { + this.asyncWantConcede = true; + } + + public Boolean getAsyncWantCheat() { + return this.asyncWantCheat != null && this.asyncWantCheat; + } + + /** + * Start cheat dialog on next player's priority + */ + public void setAsyncWantCheat() { + this.asyncWantCheat = true; + } } diff --git a/Mage.Server/src/main/java/mage/server/ChatManagerImpl.java b/Mage.Server/src/main/java/mage/server/ChatManagerImpl.java index 4642ef9993d..4b9ef0d4b11 100644 --- a/Mage.Server/src/main/java/mage/server/ChatManagerImpl.java +++ b/Mage.Server/src/main/java/mage/server/ChatManagerImpl.java @@ -7,7 +7,7 @@ import mage.server.exceptions.UserNotFoundException; import mage.server.game.GameController; import mage.server.managers.ChatManager; import mage.server.managers.ManagerFactory; -import mage.server.util.SystemUtil; +import mage.utils.SystemUtil; import mage.view.ChatMessage.MessageColor; import mage.view.ChatMessage.MessageType; import mage.view.ChatMessage.SoundToPlay; diff --git a/Mage.Server/src/main/java/mage/server/MageServerImpl.java b/Mage.Server/src/main/java/mage/server/MageServerImpl.java index 9a4d1c67242..5240865fb29 100644 --- a/Mage.Server/src/main/java/mage/server/MageServerImpl.java +++ b/Mage.Server/src/main/java/mage/server/MageServerImpl.java @@ -30,7 +30,7 @@ import mage.server.managers.ManagerFactory; import mage.server.services.impl.FeedbackServiceImpl; import mage.server.tournament.TournamentFactory; import mage.server.util.ServerMessagesUtil; -import mage.server.util.SystemUtil; +import mage.utils.SystemUtil; import mage.utils.*; import mage.view.*; import mage.view.ChatMessage.MessageColor; @@ -974,36 +974,17 @@ public class MageServerImpl implements MageServer { } @Override - public void cheatMultiple(final UUID gameId, final String sessionId, final UUID playerId, final DeckCardLists deckList) throws MageException { - execute("cheat", sessionId, () -> { + public void cheatShow(final UUID gameId, final String sessionId, final UUID playerId) throws MageException { + execute("cheatShow", sessionId, () -> { if (testMode) { managerFactory.sessionManager().getSession(sessionId).ifPresent(session -> { UUID userId = session.getUserId(); - managerFactory.gameManager().cheat(gameId, userId, playerId, deckList); + managerFactory.gameManager().cheatShow(gameId, userId, playerId); }); } }); } - @Override - public boolean cheatOne(final UUID gameId, final String sessionId, final UUID playerId, final String cardName) throws MageException { - return executeWithResult("cheatOne", sessionId, new ActionWithBooleanResult() { - @Override - public Boolean execute() { - if (testMode) { - Optional session = managerFactory.sessionManager().getSession(sessionId); - if (!session.isPresent()) { - logger.error("Session not found : " + sessionId); - } else { - UUID userId = session.get().getUserId(); - return managerFactory.gameManager().cheat(gameId, userId, playerId, cardName); - } - } - return false; - } - }); - } - public void handleException(Exception ex) throws MageException { if (!ex.getMessage().equals("No message")) { logger.fatal("", ex); diff --git a/Mage.Server/src/main/java/mage/server/Main.java b/Mage.Server/src/main/java/mage/server/Main.java index 350dd6064ec..e9cf4c17030 100644 --- a/Mage.Server/src/main/java/mage/server/Main.java +++ b/Mage.Server/src/main/java/mage/server/Main.java @@ -22,6 +22,7 @@ import mage.server.util.*; import mage.server.util.config.GamePlugin; import mage.server.util.config.Plugin; import mage.utils.MageVersion; +import mage.utils.SystemUtil; import org.apache.log4j.Logger; import org.jboss.remoting.*; import org.jboss.remoting.callback.InvokerCallbackHandler; diff --git a/Mage.Server/src/main/java/mage/server/Session.java b/Mage.Server/src/main/java/mage/server/Session.java index b35b83c4bd8..37d4d59108e 100644 --- a/Mage.Server/src/main/java/mage/server/Session.java +++ b/Mage.Server/src/main/java/mage/server/Session.java @@ -9,7 +9,7 @@ import mage.players.net.UserGroup; import mage.server.game.GamesRoom; import mage.server.managers.ConfigSettings; import mage.server.managers.ManagerFactory; -import mage.server.util.SystemUtil; +import mage.utils.SystemUtil; import mage.util.RandomUtil; import org.apache.log4j.Logger; import org.jboss.remoting.callback.AsynchInvokerCallbackHandler; diff --git a/Mage.Server/src/main/java/mage/server/User.java b/Mage.Server/src/main/java/mage/server/User.java index e03295420da..77f909c7c9b 100644 --- a/Mage.Server/src/main/java/mage/server/User.java +++ b/Mage.Server/src/main/java/mage/server/User.java @@ -19,7 +19,7 @@ import mage.server.record.UserStatsRepository; import mage.server.tournament.TournamentController; import mage.server.tournament.TournamentSession; import mage.server.util.ServerMessagesUtil; -import mage.server.util.SystemUtil; +import mage.utils.SystemUtil; import mage.view.TableClientMessage; import org.apache.log4j.Logger; diff --git a/Mage.Server/src/main/java/mage/server/game/GameController.java b/Mage.Server/src/main/java/mage/server/game/GameController.java index debcb3218f3..b03261b390e 100644 --- a/Mage.Server/src/main/java/mage/server/game/GameController.java +++ b/Mage.Server/src/main/java/mage/server/game/GameController.java @@ -27,7 +27,6 @@ import mage.server.Main; import mage.server.User; import mage.server.managers.ManagerFactory; import mage.server.util.Splitter; -import mage.server.util.SystemUtil; import mage.util.MultiAmountMessage; import mage.utils.StreamUtils; import mage.utils.timer.PriorityTimer; @@ -502,6 +501,10 @@ public class GameController implements GameCallback { } public void sendPlayerAction(PlayerAction playerAction, UUID userId, Object data) { + // TODO: critical bug, must be enabled and research/rework due: + // * game change commands must be executed by game thread (example: undo) + // * user change commands can be executed by network thread??? (example: change skip settings) + //SystemUtil.ensureRunInGameThread(); switch (playerAction) { case UNDO: game.undo(getPlayerId(userId)); @@ -705,31 +708,10 @@ public class GameController implements GameCallback { } } - public void cheat(UUID userId, UUID playerId, DeckCardLists deckList) { - try { - Deck deck = Deck.load(deckList, false, false); - game.loadCards(deck.getCards(), playerId); - for (Card card : deck.getCards()) { - card.putOntoBattlefield(game, Zone.OUTSIDE, null, playerId); - } - } catch (GameException ex) { - logger.warn(ex.getMessage()); - } - addCardsForTesting(game, playerId); - updateGame(); - } - - public boolean cheat(UUID userId, UUID playerId, String cardName) { - CardInfo cardInfo = CardRepository.instance.findCard(cardName); - Card card = cardInfo != null ? cardInfo.getCard() : null; - if (card != null) { - Set cards = new HashSet<>(); - cards.add(card); - game.loadCards(cards, playerId); - card.moveToZone(Zone.HAND, null, game, false); - return true; - } else { - return false; + public void cheatShow(UUID playerId) { + Player player = game.getPlayer(playerId); + if (player != null) { + player.signalPlayerCheat(); } } @@ -985,13 +967,6 @@ public class GameController implements GameCallback { return false; } - /** - * Adds cards in player's hands that are specified in config/init.txt. - */ - private void addCardsForTesting(Game game, UUID playerId) { - SystemUtil.addCardsForTesting(game, null, game.getPlayer(playerId)); - } - /** * Performs a request to a player * diff --git a/Mage.Server/src/main/java/mage/server/game/GameManagerImpl.java b/Mage.Server/src/main/java/mage/server/game/GameManagerImpl.java index 3121bfdbdb6..08fb5d2af6c 100644 --- a/Mage.Server/src/main/java/mage/server/game/GameManagerImpl.java +++ b/Mage.Server/src/main/java/mage/server/game/GameManagerImpl.java @@ -146,22 +146,13 @@ public class GameManagerImpl implements GameManager { } @Override - public void cheat(UUID gameId, UUID userId, UUID playerId, DeckCardLists deckList) { + public void cheatShow(UUID gameId, UUID userId, UUID playerId) { GameController gameController = getGameControllerSafe(gameId); if (gameController != null) { - gameController.cheat(userId, playerId, deckList); + gameController.cheatShow(playerId); } } - @Override - public boolean cheat(UUID gameId, UUID userId, UUID playerId, String cardName) { - GameController gameController = getGameControllerSafe(gameId); - if (gameController != null) { - return gameController.cheat(userId, playerId, cardName); - } - return false; - } - @Override public void removeGame(UUID gameId) { GameController gameController = getGameControllerSafe(gameId); diff --git a/Mage.Server/src/main/java/mage/server/managers/GameManager.java b/Mage.Server/src/main/java/mage/server/managers/GameManager.java index 4531a537428..852e0e677ce 100644 --- a/Mage.Server/src/main/java/mage/server/managers/GameManager.java +++ b/Mage.Server/src/main/java/mage/server/managers/GameManager.java @@ -38,9 +38,7 @@ public interface GameManager { void stopWatching(UUID gameId, UUID userId); - void cheat(UUID gameId, UUID userId, UUID playerId, DeckCardLists deckList); - - boolean cheat(UUID gameId, UUID userId, UUID playerId, String cardName); + void cheatShow(UUID gameId, UUID userId, UUID playerId); void removeGame(UUID gameId); diff --git a/Mage.Server/src/main/java/mage/server/util/ThreadExecutorImpl.java b/Mage.Server/src/main/java/mage/server/util/ThreadExecutorImpl.java index 85ca2f8b1e6..fa51ea1da57 100644 --- a/Mage.Server/src/main/java/mage/server/util/ThreadExecutorImpl.java +++ b/Mage.Server/src/main/java/mage/server/util/ThreadExecutorImpl.java @@ -2,15 +2,19 @@ package mage.server.util; import mage.server.managers.ConfigSettings; import mage.server.managers.ThreadExecutor; +import org.apache.log4j.Logger; import java.util.concurrent.*; /** - * @author BetaSteward_at_googlemail.com + * @author BetaSteward_at_googlemail.com, JayDi85 */ public class ThreadExecutorImpl implements ThreadExecutor { - private final ExecutorService callExecutor; - private final ExecutorService gameExecutor; + + private static final Logger logger = Logger.getLogger(ThreadExecutorImpl.class); + + private final ExecutorService callExecutor; // shareable threads to run single task (example: save new game settings from a user, send chat message, etc) + private final ExecutorService gameExecutor; // game threads to run long tasks, one per game (example: run game and wait user's feedback) private final ScheduledExecutorService timeoutExecutor; private final ScheduledExecutorService timeoutIdleExecutor; @@ -26,8 +30,10 @@ public class ThreadExecutorImpl implements ThreadExecutor { */ public ThreadExecutorImpl(ConfigSettings config) { - callExecutor = Executors.newCachedThreadPool(); - gameExecutor = Executors.newFixedThreadPool(config.getMaxGameThreads()); + //callExecutor = Executors.newCachedThreadPool(); + callExecutor = new CachedThreadPoolWithException(); + //gameExecutor = Executors.newFixedThreadPool(config.getMaxGameThreads()); + gameExecutor = new FixedThreadPoolWithException(config.getMaxGameThreads()); timeoutExecutor = Executors.newScheduledThreadPool(4); timeoutIdleExecutor = Executors.newScheduledThreadPool(4); @@ -45,6 +51,42 @@ public class ThreadExecutorImpl implements ThreadExecutor { ((ThreadPoolExecutor) timeoutIdleExecutor).setThreadFactory(new XMageThreadFactory("TIMEOUT_IDLE")); } + static class CachedThreadPoolWithException extends ThreadPoolExecutor { + + CachedThreadPoolWithException() { + // use same params as Executors.newCachedThreadPool() + super(0, Integer.MAX_VALUE,60L, TimeUnit.SECONDS, new SynchronousQueue()); + } + + @Override + protected void afterExecute(Runnable r, Throwable t) { + super.afterExecute(r, t); + + // catch errors in CALL threads (from client commands) + if (t != null) { + logger.error("Catch unhandled error in CALL thread: " + t.getMessage(), t); + } + } + } + + static class FixedThreadPoolWithException extends ThreadPoolExecutor { + + FixedThreadPoolWithException(int nThreads) { + // use same params as Executors.newFixedThreadPool() + super(nThreads, nThreads,0L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue()); + } + + @Override + protected void afterExecute(Runnable r, Throwable t) { + super.afterExecute(r, t); + + // catch errors in GAME threads (from game processing) + if (t != null) { + // it's impossible to brake game thread in normal use case, so each bad use case must be researched + logger.error("Catch unhandled error in GAME thread: " + t.getMessage(), t); + } + } + } @Override public int getActiveThreads(ExecutorService executerService) { @@ -90,5 +132,4 @@ class XMageThreadFactory implements ThreadFactory { thread.setName(prefix + ' ' + thread.getThreadGroup().getName() + '-' + thread.getId()); return thread; } - } 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 150debbcce3..e8870353692 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 @@ -2931,6 +2931,11 @@ public class TestPlayer implements Player { computerPlayer.signalPlayerConcede(); } + @Override + public void signalPlayerCheat() { + computerPlayer.signalPlayerCheat(); + } + @Override public void abortReset() { computerPlayer.abortReset(); diff --git a/Mage.Tests/src/test/java/org/mage/test/serverside/base/MageTestPlayerBase.java b/Mage.Tests/src/test/java/org/mage/test/serverside/base/MageTestPlayerBase.java index 1632f5187e4..cddbaafabb7 100644 --- a/Mage.Tests/src/test/java/org/mage/test/serverside/base/MageTestPlayerBase.java +++ b/Mage.Tests/src/test/java/org/mage/test/serverside/base/MageTestPlayerBase.java @@ -35,7 +35,7 @@ import mage.server.managers.ConfigSettings; import mage.server.util.ConfigFactory; import mage.server.util.ConfigWrapper; import mage.server.util.PluginClassLoader; -import mage.server.util.SystemUtil; +import mage.utils.SystemUtil; import mage.server.util.config.GamePlugin; import mage.server.util.config.Plugin; import mage.target.TargetPermanent; diff --git a/Mage.Tests/src/test/java/org/mage/test/serverside/base/impl/CardTestPlayerAPIImpl.java b/Mage.Tests/src/test/java/org/mage/test/serverside/base/impl/CardTestPlayerAPIImpl.java index a636118d61f..3abe7817ce9 100644 --- a/Mage.Tests/src/test/java/org/mage/test/serverside/base/impl/CardTestPlayerAPIImpl.java +++ b/Mage.Tests/src/test/java/org/mage/test/serverside/base/impl/CardTestPlayerAPIImpl.java @@ -28,7 +28,7 @@ import mage.player.ai.ComputerPlayerMCTS; import mage.players.ManaPool; import mage.players.Player; import mage.server.game.GameSessionPlayer; -import mage.server.util.SystemUtil; +import mage.utils.SystemUtil; import mage.util.CardUtil; import mage.view.GameView; import org.junit.Assert; diff --git a/Mage.Tests/src/test/java/org/mage/test/serverside/cheats/LoadCheatsTest.java b/Mage.Tests/src/test/java/org/mage/test/serverside/cheats/LoadCheatsTest.java index f042338d75a..4f72de91e3c 100644 --- a/Mage.Tests/src/test/java/org/mage/test/serverside/cheats/LoadCheatsTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/serverside/cheats/LoadCheatsTest.java @@ -1,7 +1,7 @@ package org.mage.test.serverside.cheats; import mage.constants.*; -import mage.server.util.SystemUtil; +import mage.utils.SystemUtil; import org.junit.Assert; import org.junit.Test; import org.mage.test.serverside.base.CardTestPlayerBase; @@ -56,7 +56,7 @@ public class LoadCheatsTest extends CardTestPlayerBase { execute(); setChoice(playerA, "5"); // choose [group 3]: 5 = 2 default menus + 3 group - SystemUtil.addCardsForTesting(currentGame, commandsFile, playerA); + SystemUtil.executeCheatCommands(currentGame, commandsFile, playerA); assertHandCount(playerA, "Razorclaw Bear", 1); assertPermanentCount(playerA, "Mountain", 3); diff --git a/Mage.Tests/src/test/java/org/mage/test/stub/PlayerStub.java b/Mage.Tests/src/test/java/org/mage/test/stub/PlayerStub.java index fe0d5232ae0..708be58c98a 100644 --- a/Mage.Tests/src/test/java/org/mage/test/stub/PlayerStub.java +++ b/Mage.Tests/src/test/java/org/mage/test/stub/PlayerStub.java @@ -724,6 +724,11 @@ public class PlayerStub implements Player { } + @Override + public void signalPlayerCheat() { + + } + @Override public void abortReset() { diff --git a/Mage.Tests/src/test/java/org/mage/test/testapi/AddCardApiTest.java b/Mage.Tests/src/test/java/org/mage/test/testapi/AddCardApiTest.java index 42147d4fc8e..62a79cd6954 100644 --- a/Mage.Tests/src/test/java/org/mage/test/testapi/AddCardApiTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/testapi/AddCardApiTest.java @@ -2,7 +2,7 @@ package org.mage.test.testapi; import mage.constants.PhaseStep; import mage.constants.Zone; -import mage.server.util.SystemUtil; +import mage.utils.SystemUtil; import org.junit.Assert; import org.junit.Test; import org.mage.test.serverside.base.CardTestPlayerBase; diff --git a/Mage.Verify/src/test/java/mage/verify/VerifyCardDataTest.java b/Mage.Verify/src/test/java/mage/verify/VerifyCardDataTest.java index b148c7ef87f..9a746d7009b 100644 --- a/Mage.Verify/src/test/java/mage/verify/VerifyCardDataTest.java +++ b/Mage.Verify/src/test/java/mage/verify/VerifyCardDataTest.java @@ -35,7 +35,7 @@ import mage.game.draft.DraftCube; import mage.game.permanent.token.Token; import mage.game.permanent.token.TokenImpl; import mage.game.permanent.token.custom.CreatureToken; -import mage.server.util.SystemUtil; +import mage.utils.SystemUtil; import mage.sets.TherosBeyondDeath; import mage.target.targetpointer.TargetPointer; import mage.util.CardUtil; diff --git a/Mage/src/main/java/mage/cards/decks/DeckCardLists.java b/Mage/src/main/java/mage/cards/decks/DeckCardLists.java index 6d59a686873..d9021e322e2 100644 --- a/Mage/src/main/java/mage/cards/decks/DeckCardLists.java +++ b/Mage/src/main/java/mage/cards/decks/DeckCardLists.java @@ -1,18 +1,21 @@ - package mage.cards.decks; +import mage.util.CardUtil; +import mage.util.Copyable; + import java.io.Serializable; import java.util.ArrayList; import java.util.List; +import java.util.stream.Collectors; /** * - * @author BetaSteward_at_googlemail.com + * @author BetaSteward_at_googlemail.com, JayDi85 */ -public class DeckCardLists implements Serializable { +public class DeckCardLists implements Serializable, Copyable { - private String name; - private String author; + private String name = null; + private String author = null; private List cards = new ArrayList<>(); private List sideboard = new ArrayList<>(); @@ -20,6 +23,23 @@ public class DeckCardLists implements Serializable { private DeckCardLayout cardLayout = null; private DeckCardLayout sideboardLayout = null; + public DeckCardLists() { + } + + protected DeckCardLists(final DeckCardLists deck) { + this.name = deck.name; + this.author = deck.author; + this.cards = CardUtil.deepCopyObject(deck.cards); + this.sideboard = CardUtil.deepCopyObject(deck.sideboard); + this.cardLayout = CardUtil.deepCopyObject(deck.cardLayout); + this.sideboardLayout = CardUtil.deepCopyObject(deck.sideboardLayout); + } + + @Override + public DeckCardLists copy() { + return new DeckCardLists(this); + } + /** * @return The layout of the cards */ diff --git a/Mage/src/main/java/mage/game/Game.java b/Mage/src/main/java/mage/game/Game.java index 2eaa7e37ca5..5e3089d7973 100644 --- a/Mage/src/main/java/mage/game/Game.java +++ b/Mage/src/main/java/mage/game/Game.java @@ -534,7 +534,6 @@ public interface Game extends MageItem, Serializable, Copyable { // game cheats (for tests only) void cheat(UUID ownerId, Map commands); - void cheat(UUID ownerId, List library, List hand, List battlefield, List graveyard, List command); // controlling the behaviour of replacement effects while permanents entering the battlefield diff --git a/Mage/src/main/java/mage/game/GameImpl.java b/Mage/src/main/java/mage/game/GameImpl.java index 02ab84e4b6a..22e2e56f407 100644 --- a/Mage/src/main/java/mage/game/GameImpl.java +++ b/Mage/src/main/java/mage/game/GameImpl.java @@ -794,7 +794,7 @@ public abstract class GameImpl implements Game { if (!concedingPlayers.contains(playerId)) { logger.debug("Game over for player Id: " + playerId + " gameId " + getId()); concedingPlayers.add(playerId); - player.signalPlayerConcede(); + player.signalPlayerConcede(); // will be executed on next priority } } else { // no asynchronous action so check directly @@ -3799,8 +3799,9 @@ public abstract class GameImpl implements Game { for (Player playerObject : getPlayers().values()) { if (playerObject.isHuman() && playerObject.canRespond()) { playerObject.resetStoredBookmark(this); - playerObject.abort(); playerObject.resetPlayerPassedActions(); + playerObject.abort(); + } } fireUpdatePlayersEvent(); diff --git a/Mage/src/main/java/mage/game/GameState.java b/Mage/src/main/java/mage/game/GameState.java index 9d467e965f9..3985e08a724 100644 --- a/Mage/src/main/java/mage/game/GameState.java +++ b/Mage/src/main/java/mage/game/GameState.java @@ -77,7 +77,7 @@ public class GameState implements Serializable, Copyable { private Turn turn; private TurnMods turnMods; // one time turn modifications (turn, phase or step) private UUID activePlayerId; // playerId which turn it is - private UUID priorityPlayerId; // player that has currently priority + private UUID priorityPlayerId; // player that has currently priority (setup before any choose) private UUID playerByOrderId; // player that has currently priority private UUID monarchId; // player that is the monarch private UUID initiativeId; // player that has the initiative diff --git a/Mage/src/main/java/mage/players/Player.java b/Mage/src/main/java/mage/players/Player.java index 363a44ab517..6856f7f1ae3 100644 --- a/Mage/src/main/java/mage/players/Player.java +++ b/Mage/src/main/java/mage/players/Player.java @@ -550,6 +550,8 @@ public interface Player extends MageItem, Copyable { void signalPlayerConcede(); + void signalPlayerCheat(); + void skip(); // priority, undo, ... diff --git a/Mage/src/main/java/mage/players/PlayerImpl.java b/Mage/src/main/java/mage/players/PlayerImpl.java index 006f4c5fddf..b3b939d3295 100644 --- a/Mage/src/main/java/mage/players/PlayerImpl.java +++ b/Mage/src/main/java/mage/players/PlayerImpl.java @@ -5088,7 +5088,10 @@ public abstract class PlayerImpl implements Player, Serializable { @Override public void signalPlayerConcede() { + } + @Override + public void signalPlayerCheat() { } @Override diff --git a/Mage/src/main/java/mage/util/DebugUtil.java b/Mage/src/main/java/mage/util/DebugUtil.java index 634d46ade46..28017431d34 100644 --- a/Mage/src/main/java/mage/util/DebugUtil.java +++ b/Mage/src/main/java/mage/util/DebugUtil.java @@ -1,5 +1,7 @@ package mage.util; +import java.lang.reflect.Method; + /** * Devs only: enable or disable debug features *

@@ -28,4 +30,47 @@ public class DebugUtil { public static boolean GUI_GAME_DRAW_BATTLEFIELD_BORDER = false; public static boolean GUI_GAME_DRAW_HAND_AND_STACK_BORDER = false; public static boolean GUI_GAME_DIALOGS_DRAW_CARDS_AREA_BORDER = false; + + public static String getMethodNameWithSource(final int depth) { + return TraceHelper.getMethodNameWithSource(depth); + } + } + +/** + * Debug: allows to find a caller's method name + * Original code + */ +class TraceHelper { + + private static Method m; + + static { + try { + m = Throwable.class.getDeclaredMethod("getStackTraceElement", int.class); + m.setAccessible(true); + } catch (Exception e) { + e.printStackTrace(); + } + } + + public static String getMethodName(final int depth) { + try { + StackTraceElement element = (StackTraceElement) m.invoke(new Throwable(), depth + 1); + return element.getMethodName(); + } catch (Exception e) { + e.printStackTrace(); + return null; + } + } + + public static String getMethodNameWithSource(final int depth) { + try { + StackTraceElement element = (StackTraceElement) m.invoke(new Throwable(), depth + 1); + return String.format("%s - %s:%d", element.getMethodName(), element.getFileName(), element.getLineNumber()); + } catch (Exception e) { + e.printStackTrace(); + return null; + } + } +} \ No newline at end of file