From b3c55555a17100144878c4869f35b7fb691da53f Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Mon, 8 Jan 2024 04:03:16 +0400 Subject: [PATCH] tests: fixed error on load tests end (related to #11572), improved logs, improved session lifecycle on load tests; tests: added additional test for Mana Maze and deep copy (related to #11572); docs: added more info to network related code; --- .../client/dialog/NewTournamentDialog.java | 2 +- .../java/mage/server/TableController.java | 12 +++--- .../java/mage/server/game/GameCallback.java | 5 +-- .../java/mage/server/game/GameController.java | 24 +++++++---- .../java/mage/server/game/GameWorker.java | 42 +++++++++---------- .../java/mage/server/game/ReplaySession.java | 6 +-- .../mage/server/managers/ThreadExecutor.java | 22 ++++++++-- .../test/cards/single/inv/ManaMazeTest.java | 35 ++++++++++++++-- .../java/org/mage/test/load/LoadTest.java | 17 ++++++-- Mage/src/main/java/mage/game/GameImpl.java | 5 ++- .../java/mage/game/match/MatchOptions.java | 13 +++--- readme.md | 2 +- 12 files changed, 119 insertions(+), 66 deletions(-) diff --git a/Mage.Client/src/main/java/mage/client/dialog/NewTournamentDialog.java b/Mage.Client/src/main/java/mage/client/dialog/NewTournamentDialog.java index 7ccc597b5e8..54d5ab49192 100644 --- a/Mage.Client/src/main/java/mage/client/dialog/NewTournamentDialog.java +++ b/Mage.Client/src/main/java/mage/client/dialog/NewTournamentDialog.java @@ -1329,7 +1329,7 @@ public class NewTournamentDialog extends MageDialog { tOptions.getMatchOptions().setMatchBufferTime((MatchBufferTime) this.cbBufferTime.getSelectedItem()); tOptions.getMatchOptions().setSkillLevel((SkillLevel) this.cbSkillLevel.getSelectedItem()); tOptions.getMatchOptions().setWinsNeeded((Integer) this.spnNumWins.getValue()); - tOptions.getMatchOptions().setAttackOption(MultiplayerAttackOption.LEFT); + tOptions.getMatchOptions().setAttackOption(MultiplayerAttackOption.MULTIPLE); tOptions.getMatchOptions().setRange(RangeOfInfluence.ALL); tOptions.getMatchOptions().setRollbackTurnsAllowed(this.chkRollbackTurnsAllowed.isSelected()); tOptions.getMatchOptions().setRated(this.chkRated.isSelected()); diff --git a/Mage.Server/src/main/java/mage/server/TableController.java b/Mage.Server/src/main/java/mage/server/TableController.java index fa66c0e75d9..5aced7e8d92 100644 --- a/Mage.Server/src/main/java/mage/server/TableController.java +++ b/Mage.Server/src/main/java/mage/server/TableController.java @@ -59,20 +59,20 @@ public class TableController { public TableController(ManagerFactory managerFactory, UUID roomId, UUID userId, MatchOptions options) { this.managerFactory = managerFactory; - timeoutExecutor = managerFactory.threadExecutor().getTimeoutExecutor(); + this.timeoutExecutor = managerFactory.threadExecutor().getTimeoutExecutor(); this.userId = userId; this.options = options; - match = GameFactory.instance.createMatch(options.getGameType(), options); + this.match = GameFactory.instance.createMatch(options.getGameType(), options); if (userId != null) { Optional user = managerFactory.userManager().getUser(userId); // TODO: Handle if user == null - controllerName = user.map(User::getName).orElse("undefined"); + this.controllerName = user.map(User::getName).orElse("undefined"); } else { - controllerName = "System"; + this.controllerName = "System"; } - table = new Table(roomId, options.getGameType(), options.getName(), controllerName, DeckValidatorFactory.instance.createDeckValidator(options.getDeckType()), + this.table = new Table(roomId, options.getGameType(), options.getName(), controllerName, DeckValidatorFactory.instance.createDeckValidator(options.getDeckType()), options.getPlayerTypes(), new TableRecorderImpl(managerFactory.userManager()), match, options.getBannedUsers(), options.isPlaneChase()); - chatId = managerFactory.chatManager().createChatSession("Match Table " + table.getId()); + this.chatId = managerFactory.chatManager().createChatSession("Match Table " + table.getId()); init(); } diff --git a/Mage.Server/src/main/java/mage/server/game/GameCallback.java b/Mage.Server/src/main/java/mage/server/game/GameCallback.java index 5d6d3e4166a..31a949ee574 100644 --- a/Mage.Server/src/main/java/mage/server/game/GameCallback.java +++ b/Mage.Server/src/main/java/mage/server/game/GameCallback.java @@ -1,15 +1,12 @@ - - package mage.server.game; import mage.MageException; /** - * * @author BetaSteward_at_googlemail.com */ @FunctionalInterface public interface GameCallback { - void gameResult(String result) throws MageException; + void endGameWithResult(String result) throws MageException; } 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 b69eb84e4e8..4d29300840e 100644 --- a/Mage.Server/src/main/java/mage/server/game/GameController.java +++ b/Mage.Server/src/main/java/mage/server/game/GameController.java @@ -245,7 +245,7 @@ public class GameController implements GameCallback { logger.fatal("Send info about player not joined yet:", ex); } }, GAME_TIMEOUTS_CHECK_JOINING_STATUS_EVERY_SECS, GAME_TIMEOUTS_CHECK_JOINING_STATUS_EVERY_SECS, TimeUnit.SECONDS); - checkStart(); + checkJoinAndStart(); } /** @@ -316,7 +316,7 @@ public class GameController implements GameCallback { user.get().addGame(playerId, gameSession); logger.debug("Player " + player.getName() + ' ' + playerId + " has " + joinType + " gameId: " + game.getId()); managerFactory.chatManager().broadcast(chatId, "", game.getPlayer(playerId).getLogName() + " has " + joinType + " the game", MessageColor.ORANGE, true, game, MessageType.GAME, null); - checkStart(); + checkJoinAndStart(); } private synchronized void startGame() { @@ -326,16 +326,19 @@ public class GameController implements GameCallback { player.updateRange(game); } + // send first info to users for (GameSessionPlayer gameSessionPlayer : getGameSessions()) { gameSessionPlayer.init(); } + // real game start GameWorker worker = new GameWorker(game, choosingPlayerId, this); gameFuture = gameExecutor.submit(worker); try { Thread.sleep(1000); } catch (InterruptedException ignore) { } + if (game.getState().getChoosingPlayerId() != null) { // start timer to force player to choose starting player otherwise loosing by being idle setupTimeout(game.getState().getChoosingPlayerId()); @@ -395,7 +398,7 @@ public class GameController implements GameCallback { } } } - checkStart(); + checkJoinAndStart(); } private Optional getUserByPlayerId(UUID playerId) { @@ -407,14 +410,14 @@ public class GameController implements GameCallback { return Optional.empty(); } - private void checkStart() { - if (allJoined()) { + private void checkJoinAndStart() { + if (isAllJoined()) { joinWaitingExecutor.shutdownNow(); managerFactory.threadExecutor().getCallExecutor().execute(this::startGame); } } - private boolean allJoined() { + private boolean isAllJoined() { for (Player player : game.getPlayers().values()) { if (!player.hasLeft()) { Optional user = getUserByPlayerId(player.getId()); @@ -480,7 +483,7 @@ public class GameController implements GameCallback { public void quitMatch(UUID userId) { UUID playerId = getPlayerId(userId); if (playerId != null) { - if (allJoined()) { + if (isAllJoined()) { GameSessionPlayer gameSessionPlayer = gameSessions.get(playerId); if (gameSessionPlayer != null) { gameSessionPlayer.quitGame(); @@ -492,7 +495,7 @@ public class GameController implements GameCallback { Player player = game.getPlayer(playerId); if (player != null) { player.leave(); - checkStart(); // => So the game starts and gets an result or multiplayer game starts with active players + checkJoinAndStart(); // => So the game starts and gets an result or multiplayer game starts with active players } } } @@ -734,6 +737,7 @@ public class GameController implements GameCallback { } public void endGame(final String message) throws MageException { + // send end game message/dialog for (final GameSessionPlayer gameSession : getGameSessions()) { gameSession.gameOver(message); gameSession.removeGame(); @@ -741,6 +745,8 @@ public class GameController implements GameCallback { for (final GameSessionWatcher gameWatcher : getGameSessionWatchers()) { gameWatcher.gameOver(message); } + + // start next game or close finished table managerFactory.tableManager().endGame(tableId); } @@ -944,7 +950,7 @@ public class GameController implements GameCallback { } @Override - public void gameResult(String result) { + public void endGameWithResult(String result) { try { endGame(result); } catch (MageException ex) { diff --git a/Mage.Server/src/main/java/mage/server/game/GameWorker.java b/Mage.Server/src/main/java/mage/server/game/GameWorker.java index e52f9d7cc8a..169fad24673 100644 --- a/Mage.Server/src/main/java/mage/server/game/GameWorker.java +++ b/Mage.Server/src/main/java/mage/server/game/GameWorker.java @@ -1,18 +1,18 @@ - package mage.server.game; -import java.util.UUID; -import java.util.concurrent.Callable; - import mage.MageException; import mage.game.Game; import org.apache.log4j.Logger; +import java.util.UUID; +import java.util.concurrent.Callable; + /** - * @param - * @author BetaSteward_at_googlemail.com + * Game: main thread to process full game (one thread per game) + * + * @author BetaSteward_at_googlemail.com, JayDi85 */ -public class GameWorker implements Callable { +public class GameWorker implements Callable { private static final Logger LOGGER = Logger.getLogger(GameWorker.class); @@ -27,25 +27,23 @@ public class GameWorker implements Callable { } @Override - public Object call() { + public Boolean call() { try { - LOGGER.debug("GAME WORKER started gameId " + game.getId()); + // play game Thread.currentThread().setName("GAME " + game.getId()); game.start(choosingPlayerId); - game.fireUpdatePlayersEvent(); - gameController.gameResult(game.getWinner()); - game.cleanUp(); - } catch (MageException ex) { - LOGGER.fatal("GameWorker mage error [" + game.getId() + "] " + ex, ex); - } catch (Exception e) { - LOGGER.fatal("GameWorker general exception [" + game.getId() + "] " + e.getMessage(), e); - if (e instanceof NullPointerException) { - LOGGER.info(e.getStackTrace()); - } - } catch (Error err) { - LOGGER.fatal("GameWorker general error [" + game.getId() + "] " + err, err); + + // save result and start next game or close finished table + game.fireUpdatePlayersEvent(); // TODO: no needs in update event (gameController.endGameWithResult already send game end dialog)? + gameController.endGameWithResult(game.getWinner()); + + // clear resources + game.cleanUp();// TODO: no needs in cleanup code (cards list are useless for memory optimization, game states are more important)? + } catch (MageException e) { + LOGGER.fatal("GameWorker mage error [" + game.getId() + " - " + game + "]: " + e, e); + } catch (Throwable e) { + LOGGER.fatal("GameWorker system error [" + game.getId() + " - " + game + "]: " + e, e); } return null; } - } diff --git a/Mage.Server/src/main/java/mage/server/game/ReplaySession.java b/Mage.Server/src/main/java/mage/server/game/ReplaySession.java index ac6592cd7ab..922b6bd17a1 100644 --- a/Mage.Server/src/main/java/mage/server/game/ReplaySession.java +++ b/Mage.Server/src/main/java/mage/server/game/ReplaySession.java @@ -32,7 +32,7 @@ public class ReplaySession implements GameCallback { } public void stop() { - gameResult("stopped replay"); + endGameWithResult("stopped replay"); } public synchronized void next() { @@ -51,7 +51,7 @@ public class ReplaySession implements GameCallback { } @Override - public void gameResult(final String result) { + public void endGameWithResult(final String result) { managerFactory.userManager().getUser(userId).ifPresent(user -> user.fireCallback(new ClientCallback(ClientCallbackMethod.REPLAY_DONE, replay.getGame().getId(), result))); @@ -60,7 +60,7 @@ public class ReplaySession implements GameCallback { private void updateGame(final GameState state, Game game) { if (state == null) { - gameResult("game ended"); + endGameWithResult("game ended"); } else { managerFactory.userManager().getUser(userId).ifPresent(user -> user.fireCallback(new ClientCallback(ClientCallbackMethod.REPLAY_UPDATE, replay.getGame().getId(), new GameView(state, game, null, null)))); diff --git a/Mage.Server/src/main/java/mage/server/managers/ThreadExecutor.java b/Mage.Server/src/main/java/mage/server/managers/ThreadExecutor.java index 6683ec8ecf9..79c1c3e1f22 100644 --- a/Mage.Server/src/main/java/mage/server/managers/ThreadExecutor.java +++ b/Mage.Server/src/main/java/mage/server/managers/ThreadExecutor.java @@ -1,20 +1,34 @@ package mage.server.managers; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ThreadFactory; +/** + * Server side threads to execute some code/tasks + */ public interface ThreadExecutor { + int getActiveThreads(ExecutorService executerService); - ExecutorService getCallExecutor(); - + /** + * Main game thread (one per game) + */ ExecutorService getGameExecutor(); + /** + * Helper threads to execute async commands for game and server related tasks (example: process income command from a client) + */ + ExecutorService getCallExecutor(); + + /** + * Helper threads to execute async timers and time related tasks + */ ScheduledExecutorService getTimeoutExecutor(); ScheduledExecutorService getTimeoutIdleExecutor(); + /** + * Helper thread to execute inner server tasks + */ ScheduledExecutorService getServerHealthExecutor(); } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/single/inv/ManaMazeTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/single/inv/ManaMazeTest.java index e7bad3b866b..5b96860a953 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/single/inv/ManaMazeTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/single/inv/ManaMazeTest.java @@ -2,19 +2,48 @@ package org.mage.test.cards.single.inv; import mage.constants.PhaseStep; import mage.constants.Zone; +import mage.util.CardUtil; +import org.junit.Assert; import org.junit.Ignore; import org.junit.Test; import org.mage.test.serverside.base.CardTestPlayerBase; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + /** * @author JayDi85 */ - +@Ignore // TODO: enable after deep copy fix public class ManaMazeTest extends CardTestPlayerBase { @Test - @Ignore // TODO: enable after deep copy fix - public void test_DeepCopyWithWatcherAndSelfReference() { + public void test_DeepCopy_WithSelfReference() { + // stack overflow bug: https://github.com/magefree/mage/issues/11572 + + // list + List sourceList = new ArrayList<>(Arrays.asList("val1", "val2", "val3")); + List copyList = CardUtil.deepCopyObject(sourceList); + Assert.assertNotSame(sourceList, copyList); + Assert.assertEquals(sourceList.size(), copyList.size()); + Assert.assertEquals(sourceList.toString(), copyList.toString()); + + // list with self ref + List> sourceObjectList = new ArrayList<>(); + sourceObjectList.add(new ArrayList<>(Arrays.asList("val1", "val2", "val3"))); + sourceObjectList.add(new ArrayList<>(Arrays.asList(sourceObjectList))); + List> copyObjectList = CardUtil.deepCopyObject(sourceObjectList); + Assert.assertNotSame(sourceObjectList, copyObjectList); + Assert.assertEquals(sourceObjectList.size(), copyObjectList.size()); + Assert.assertEquals(sourceObjectList.get(0).size(), copyObjectList.get(0).size()); + Assert.assertEquals(sourceObjectList.get(0).toString(), copyObjectList.get(0).toString()); + Assert.assertEquals(sourceObjectList.get(1).size(), copyObjectList.get(1).size()); + Assert.assertEquals(sourceObjectList.get(1).toString(), copyObjectList.get(1).toString()); + } + + @Test + public void test_DeepCopy_WatcherWithSelfReference() { // stack overflow bug: https://github.com/magefree/mage/issues/11572 // card's watcher can have spell's ref to itself, so deep copy must be able to process it diff --git a/Mage.Tests/src/test/java/org/mage/test/load/LoadTest.java b/Mage.Tests/src/test/java/org/mage/test/load/LoadTest.java index a2b783f3c12..e2d966f8d14 100644 --- a/Mage.Tests/src/test/java/org/mage/test/load/LoadTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/load/LoadTest.java @@ -200,7 +200,7 @@ public class LoadTest { // playing until game over while (!player1.client.isGameOver() && !player2.client.isGameOver()) { checkGame = monitor.getTable(tableId); - logger.warn(checkGame.get().getTableState()); + logger.info(checkGame.get().getTableState()); try { Thread.sleep(1000); } catch (InterruptedException e) { @@ -236,6 +236,7 @@ public class LoadTest { // playing until game over gameResult.start(); boolean startToWatching = false; + Date lastActivity = new Date(); while (true) { GameView gameView = monitor.client.getLastGameView(); @@ -243,8 +244,8 @@ public class LoadTest { TableState state = (checkGame == null ? null : checkGame.getTableState()); if (gameView != null && checkGame != null) { - logger.warn(checkGame.getTableName() + ": ---"); - logger.warn(String.format("%s: turn %d, step %s, state %s", + logger.info(checkGame.getTableName() + ": ---"); + logger.info(String.format("%s: turn %d, step %s, state %s", checkGame.getTableName(), gameView.getTurn(), gameView.getStep().toString(), @@ -279,6 +280,13 @@ public class LoadTest { activeInfo )); }); + logger.info(checkGame.getTableName() + ": ---"); + } + + // ping to keep active session + if ((new Date().getTime() - lastActivity.getTime()) / 1000 > 10) { + monitor.session.ping(); + lastActivity = new Date(); } try { @@ -287,6 +295,9 @@ public class LoadTest { logger.error(e.getMessage(), e); } } + + // all done, can disconnect now + monitor.session.connectStop(false, false); } @Test diff --git a/Mage/src/main/java/mage/game/GameImpl.java b/Mage/src/main/java/mage/game/GameImpl.java index 96f8e7732b0..28616f354ae 100644 --- a/Mage/src/main/java/mage/game/GameImpl.java +++ b/Mage/src/main/java/mage/game/GameImpl.java @@ -3973,10 +3973,11 @@ public abstract class GameImpl implements Game { Player activePayer = this.getPlayer(this.getActivePlayerId()); StringBuilder sb = new StringBuilder() .append(this.isSimulation() ? "!!!SIMULATION!!! " : "") - .append(this.getGameType().toString()) + .append("; ").append(this.getGameType().toString()) .append("; ").append(CardUtil.getTurnInfo(this)) .append("; active: ").append((activePayer == null ? "none" : activePayer.getName())) - .append("; stack: ").append(this.getStack().toString()); + .append("; stack: ").append(this.getStack().toString()) + .append(this.getState().isGameOver() ? "; FINISHED: " + this.getWinner() : ""); return sb.toString(); } } diff --git a/Mage/src/main/java/mage/game/match/MatchOptions.java b/Mage/src/main/java/mage/game/match/MatchOptions.java index 42b97dde716..a878a911f35 100644 --- a/Mage/src/main/java/mage/game/match/MatchOptions.java +++ b/Mage/src/main/java/mage/game/match/MatchOptions.java @@ -1,4 +1,3 @@ - package mage.game.match; import mage.cards.decks.DeckCardInfo; @@ -20,8 +19,8 @@ import java.util.*; public class MatchOptions implements Serializable { protected String name; - protected MultiplayerAttackOption attackOption; - protected RangeOfInfluence range; + protected MultiplayerAttackOption attackOption = MultiplayerAttackOption.LEFT; + protected RangeOfInfluence range = RangeOfInfluence.ALL; protected int winsNeeded; protected int freeMulligans; protected boolean customStartLifeEnabled; @@ -35,7 +34,7 @@ public class MatchOptions implements Serializable { protected boolean multiPlayer; protected int numSeats; protected String password; - protected SkillLevel skillLevel; + protected SkillLevel skillLevel = SkillLevel.CASUAL; protected boolean rollbackTurnsAllowed; protected boolean spectatorsAllowed; protected boolean planeChase; @@ -49,8 +48,8 @@ public class MatchOptions implements Serializable { protected MatchBufferTime matchBufferTime = MatchBufferTime.NONE; // additional/buffer time limit for each priority before real time ticking starts protected MulliganType mulliganType = MulliganType.GAME_DEFAULT; - protected Collection perPlayerEmblemCards; - protected Collection globalEmblemCards; + protected Collection perPlayerEmblemCards = Collections.emptySet(); + protected Collection globalEmblemCards = Collections.emptySet(); public MatchOptions(String name, String gameType, boolean multiPlayer, int numSeats) { this.name = name; @@ -58,8 +57,6 @@ public class MatchOptions implements Serializable { this.password = ""; this.multiPlayer = multiPlayer; this.numSeats = numSeats; - this.perPlayerEmblemCards = Collections.emptySet(); - this.globalEmblemCards = Collections.emptySet(); } public void setNumSeats(int numSeats) { diff --git a/readme.md b/readme.md index b376bacee8e..c51dccbb1ed 100644 --- a/readme.md +++ b/readme.md @@ -75,7 +75,7 @@ Github issues page contain [popular problems and fixes](https://github.com/magef * [Any: can't run client, could not open ...jvm.cfg](https://github.com/magefree/mage/issues/1272#issuecomment-529789018); * [Any: no texts or small buttons in launcher](https://github.com/magefree/mage/issues/4126); * [Windows: ugly cards, buttons or other GUI drawing artifacts](https://github.com/magefree/mage/issues/4626#issuecomment-374640070); -* [MacOS: can't open launcher](https://www.reddit.com/r/XMage/comments/kf8l34/updated_java_on_osx_xmage_not_working/ggej8cq/) +* [MacOS: can't open launcher](https://www.reddit.com/r/XMage/comments/kf8l34/updated_java_on_osx_xmage_not_working/ggej8cq/); * [MacOS: client freezes in GUI (on connect dialog, on new match)](https://github.com/magefree/mage/issues/4920#issuecomment-517944308); * [Linux: run on non-standard OS or hardware like Raspberry Pi](https://github.com/magefree/mage/issues/11611#issuecomment-1879385151); * [Linux: ugly GUI and drawing artifacts](https://github.com/magefree/mage/issues/11611#issuecomment-1879396921);