game, refactor: improved player related code, fixed miss reset fields between games like commanderIds (#11081, #11628)

Mage.Server/src/main/java/mage/server/TableController.java

@@ -44,7 +44,7 @@ public class TableController {
     private static final Logger logger = Logger.getLogger(TableController.class);
 
     private final ManagerFactory managerFactory;
-    private final UUID userId;
+    private final UUID userId; // table owner/creator (null in tourney's table)
     private final UUID chatId;
     private final String controllerName;
     private final Table table;
@@ -604,17 +604,17 @@ public class TableController {
         if (table.getState() == TableState.STARTING) {
             try {
                 if (table.isTournamentSubTable()) {
-                    logger.info("Tourn. match started id:" + match.getId() + " tournId: " + table.getTournament().getId());
+                    logger.info("Tourney MATCH started id:" + match.getId() + " tournId: " + table.getTournament().getId());
                 } else {
                     managerFactory.userManager().getUser(userId).ifPresent(user -> {
-                        logger.info("MATCH started [" + match.getName() + "] " + match.getId() + " (" + user.getName() + ')');
+                        logger.info("Single MATCH started [" + match.getName() + "] " + match.getId() + " (" + user.getName() + ')');
                         logger.debug("- " + match.getOptions().getGameType() + " - " + match.getOptions().getDeckType());
                     });
                 }
                 match.startMatch();
                 startGame(null);
-            } catch (GameException ex) {
+            } catch (GameException e) {
-                logger.fatal("Error starting match ", ex);
+                logger.fatal("Error starting match: " + e, e);
                 match.endGame();
             }
         }

Mage.Server/src/main/java/mage/server/game/GameController.java

@@ -102,13 +102,14 @@ public class GameController implements GameCallback {
 
     public void cleanUp() {
         cancelTimeout();
-        for (GameSessionPlayer gameSessionPlayer : getGameSessions()) {
-            gameSessionPlayer.cleanUp();
-        }
-        managerFactory.chatManager().destroyChatSession(chatId);
         for (PriorityTimer priorityTimer : timers.values()) {
             priorityTimer.cancel();
         }
+
+        getGameSessions().forEach(GameSessionPlayer::cleanUp);
+        getGameSessionWatchers().forEach(GameSessionWatcher::cleanUp);
+
+        managerFactory.chatManager().destroyChatSession(chatId);
     }
 
     private void init() {

Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java

@@ -2962,12 +2962,6 @@ public class TestPlayer implements Player {
         computerPlayer.init(game);
     }
 
-    @Override
-    public void init(Game game, boolean testMode) {
-        initialTurns = 0;
-        computerPlayer.init(game, testMode);
-    }
-
     @Override
     public void reset() {
         computerPlayer.reset();

Mage.Tests/src/test/java/org/mage/test/stub/PlayerStub.java

@@ -525,11 +525,6 @@ public class PlayerStub implements Player {
 
     }
 
-    @Override
-    public void init(Game game, boolean testMode) {
-
-    }
-
     @Override
     public void useDeck(Deck deck, Game game) {
 

Mage/src/main/java/mage/game/match/MatchPlayer.java

@@ -150,11 +150,6 @@ public class MatchPlayer implements Serializable {
         // this.player = null;
     }
 
-    public void cleanUp() {
-        // Free resources that are not needed after match end
-        this.player = null;
-    }
-
     public String getName() {
         return name;
     }

Mage/src/main/java/mage/players/Player.java

@@ -370,10 +370,11 @@ public interface Player extends MageItem, Copyable<Player> {
 
     void setAllowBadMoves(boolean allowBadMoves);
 
+    /**
+     * Reset values before new game, e.g. for next game
+     */
     void init(Game game);
 
-    void init(Game game, boolean testMode);
-
     void useDeck(Deck deck, Game game);
 
     /**

Mage/src/main/java/mage/players/PlayerImpl.java

@@ -70,10 +70,21 @@ import java.util.*;
 import java.util.Map.Entry;
 import java.util.stream.Collectors;
 
+/**
+ * Server: basic player implementation, shared for human and AI
+ * <p>
+ * WARNING, if you add new fields then sync it with constructor, restore, reset and init methods
+ */
 public abstract class PlayerImpl implements Player, Serializable {
 
     private static final Logger logger = Logger.getLogger(PlayerImpl.class);
 
+    /**
+     * During some steps we can't play anything
+     */
+    final static Map<PhaseStep, Step.StepPart> SILENT_PHASES_STEPS = ImmutableMap.<PhaseStep, Step.StepPart>builder().
+            put(PhaseStep.DECLARE_ATTACKERS, Step.StepPart.PRE).build();
+
     /**
      * Used to cancel waiting requests send to the player
      */
@@ -82,15 +93,18 @@ public abstract class PlayerImpl implements Player, Serializable {
     protected final UUID playerId;
     protected String name;
     protected boolean human;
+
     protected int life;
     protected boolean wins;
     protected boolean draws;
     protected boolean loses;
+
     protected Library library;
     protected Cards sideboard;
     protected Cards hand;
     protected Graveyard graveyard;
     protected Set<UUID> commandersIds = new HashSet<>(0);
+
     protected Abilities<Ability> abilities;
     protected Counters counters;
     protected int landsPlayed;
@@ -98,6 +112,7 @@ public abstract class PlayerImpl implements Player, Serializable {
     protected int maxHandSize = 7;
     protected int maxAttackedBy = Integer.MAX_VALUE;
     protected ManaPool manaPool;
+
     // priority control
     protected boolean passed; // player passed priority
     protected boolean passedTurn; // F4
@@ -142,7 +157,6 @@ public abstract class PlayerImpl implements Player, Serializable {
     protected boolean drawsOnOpponentsTurn = false;
 
     protected FilterPermanent sacrificeCostFilter;
-
     protected final List<AlternativeSourceCosts> alternativeSourceCosts = new ArrayList<>();
 
     protected boolean isGameUnderControl = true;
@@ -151,9 +165,7 @@ public abstract class PlayerImpl implements Player, Serializable {
     protected Set<UUID> playersUnderYourControl = new HashSet<>();
 
     protected Set<UUID> usersAllowedToSeeHandCards = new HashSet<>();
-
     protected List<UUID> attachments = new ArrayList<>();
-
     protected boolean topCardRevealed = false;
 
     // 800.4i When a player leaves the game, any continuous effects with durations that last until that player's next turn
@@ -184,12 +196,6 @@ public abstract class PlayerImpl implements Player, Serializable {
     // Used during available mana calculation to give back possible available net mana from triggered mana abilities (No need to copy)
     protected final List<List<Mana>> availableTriggeredManaList = new ArrayList<>();
 
-    /**
-     * During some steps we can't play anything
-     */
-    protected final Map<PhaseStep, Step.StepPart> silentPhaseSteps = ImmutableMap.<PhaseStep, Step.StepPart>builder().
-            put(PhaseStep.DECLARE_ATTACKERS, Step.StepPart.PRE).build();
-
     protected PlayerImpl(String name, RangeOfInfluence range) {
         this(UUID.randomUUID());
         this.name = name;
@@ -205,7 +211,7 @@ public abstract class PlayerImpl implements Player, Serializable {
     }
 
     protected PlayerImpl(UUID id) {
-        this.playerId = id;
+        this.playerId = id; // TODO: miss another fields init?
     }
 
     protected PlayerImpl(final PlayerImpl player) {
@@ -291,6 +297,11 @@ public abstract class PlayerImpl implements Player, Serializable {
         this.designations = CardUtil.deepCopyObject(player.designations);
     }
 
+    /**
+     * Restore on rollback
+     *
+     * @param player
+     */
     @Override
     public void restore(Player player) {
         this.name = player.getName();
@@ -398,43 +409,35 @@ public abstract class PlayerImpl implements Player, Serializable {
         for (Card card : deck.getSideboard()) {
             sideboard.add(card);
         }
-        //TODO ARTI initialize extra decks here!
     }
 
-    /**
-     * Cast e.g. from Karn Liberated to restart the current game
-     *
-     * @param game
-     */
     @Override
     public void init(Game game) {
-        init(game, false);
-    }
-
-    @Override
-    public void init(Game game, boolean testMode) {
         this.abort = false;
-        if (!testMode) {
+
-            this.hand.clear();
+        // keep old
-            this.graveyard.clear();
+        //this.playerId;
-        }
+        //this.name;
-        this.library.reset();
+        //this.human;
-        this.abilities.clear();
+
-        this.counters.clear();
+        this.life = game.getStartingLife();
         this.wins = false;
         this.draws = false;
         this.loses = false;
-        this.left = false;
-        // reset is necessary because in tournament player will be used for each round
-        this.quit = false;
-        this.timerTimeout = false;
-        this.idleTimeout = false;
 
-        this.turns = 0;
+        this.library.reset();
-        this.isGameUnderControl = true;
+        this.sideboard.clear();
-        this.turnController = this.getId();
+        this.hand.clear();
-        this.turnControllers.clear();
+        this.graveyard.clear();
-        this.playersUnderYourControl.clear();
+        this.commandersIds.clear();
+
+        this.abilities.clear();
+        this.counters.clear();
+        this.landsPlayed = 0;
+        this.landsPerTurn = 1;
+        this.maxHandSize = 7;
+        this.maxAttackedBy = Integer.MAX_VALUE;
+        this.getManaPool().init(); // needed to remove mana that not empties on step change from previous game if left
 
         this.passed = false;
         this.passedTurn = false;
@@ -448,19 +451,51 @@ public abstract class PlayerImpl implements Player, Serializable {
         this.passedAllTurns = false;
         this.justActivatedType = null;
 
+        this.turns = 0;
+        this.storedBookmark = -1;
+        this.priorityTimeLeft = Integer.MAX_VALUE;
+        this.bufferTimeLeft = 0;
+
+        // reset is necessary because in tournament player will be used for each round
+        this.left = false;
+        this.quit = false;
+        this.timerTimeout = false;
+        this.idleTimeout = false;
+
+        //this.range; // must keep
+        this.inRange.clear();
+
+        //this.isTestMode // must keep
         this.canGainLife = true;
         this.canLoseLife = true;
+        this.payLifeCostLevel = PayLifeCostLevel.allAbilities;
+        this.loseByZeroOrLessLife = true;
+        this.canPlayCardsFromGraveyard = true;
+        this.drawsOnOpponentsTurn = false;
+
+        this.sacrificeCostFilter = null;
+        this.alternativeSourceCosts.clear();
+
+        this.isGameUnderControl = true;
+        this.turnController = null;
+        this.turnControllers.clear();
+        this.playersUnderYourControl.clear();
+
+        //this.usersAllowedToSeeHandCards; // must keep
+        this.attachments.clear();
         this.topCardRevealed = false;
-        this.payManaMode = false;
-        this.setLife(game.getStartingLife(), game, null);
-        this.setReachedNextTurnAfterLeaving(false);
 
+        this.reachedNextTurnAfterLeaving = false;
         this.clearCastSourceIdManaCosts();
+        this.payManaMode = false;
 
-        this.getManaPool().init(); // needed to remove mana that not empties on step change from previous game if left
+        // must keep
-        this.phyrexianColors = null;
+        //this.userData;
+        //this.matchPlayer;
 
         this.designations.clear();
+        this.phyrexianColors = null;
+        this.availableTriggeredManaList.clear();
     }
 
     /**
@@ -4246,7 +4281,7 @@ public abstract class PlayerImpl implements Player, Serializable {
         if (game.getStep() == null) { // happens at the start of the game
             return true;
         }
-        for (Entry<PhaseStep, Step.StepPart> phaseStep : silentPhaseSteps.entrySet()) {
+        for (Entry<PhaseStep, Step.StepPart> phaseStep : SILENT_PHASES_STEPS.entrySet()) {
             if (game.getPhase() != null
                     && game.getPhase().getStep() != null
                     && phaseStep.getKey() == game.getPhase().getStep().getType()) {