fix: simplify Game & GameRoom termination

Author: xeruf

sdk/src/server-api/sc/api/plugins/IGameInstance.kt

@@ -5,13 +5,12 @@ import sc.api.plugins.host.IGameListener
 import sc.framework.plugins.Player
 import sc.shared.InvalidMoveException
 import sc.shared.PlayerScore
-import sc.shared.ScoreCause
+
 import kotlin.jvm.Throws
 
 interface IGameInstance {
     /** @return the player that joined. */
     fun onPlayerJoined(): Player
-    fun onPlayerLeft(player: Player, cause: ScoreCause? = null)
 
     /**
      * Called by the Server upon receiving a Move.
@@ -28,13 +27,13 @@ interface IGameInstance {
 
     /** Server or an administrator requests the game to start now.  */
     fun start()
-
+    
     /**
-     * Destroys the Game.
-     * Might be invoked by the server at any time. Any open handles should be removed.
+     * Stops the Game, removing any open handles.
+     *
      * No events (GameOver etc) should be sent out after this method has been called.
      */
-    fun destroy()
+    fun stop()
 
     /**
      * Returns the players that have won the game, empty if the game has no winners,

sdk/src/server-api/sc/framework/plugins/AbstractGame.kt

@@ -46,7 +46,7 @@ abstract class AbstractGame<P : Player>(override val pluginUUID: String) : IGame
             if (timer.didTimeout()) {
                 logger.warn("Client hit soft-timeout.")
                 fromPlayer.softTimeout = true
-                onPlayerLeft(fromPlayer, ScoreCause.SOFT_TIMEOUT)
+                stop()
             } else {
                 onRoundBasedAction(fromPlayer, move)
             }
@@ -68,15 +68,12 @@ abstract class AbstractGame<P : Player>(override val pluginUUID: String) : IGame
      */
     abstract fun checkWinCondition(): WinCondition?
 
-    /**
-     * At any time this method might be invoked by the server.
-     * Any open handles should be removed.
-     * No events should be sent out (GameOver etc) after this method has been called.
-     */
-    override fun destroy() {
-        logger.info("Destroying Game")
+    /** Stops pending MoveRequests and invokes [notifyOnGameOver]. */
+    override fun stop() {
+        logger.info("Stopping {}", this)
         moveRequestTimeout?.stop()
         moveRequestTimeout = null
+        notifyOnGameOver(generateScoreMap())
     }
 
     /** Starts the game by sending a [WelcomeMessage] to all players and calling [next]. */
@@ -85,28 +82,6 @@ abstract class AbstractGame<P : Player>(override val pluginUUID: String) : IGame
         next(players.first())
     }
 
-    /**
-     * Handle leave of a player.
-     *
-     * @param player the player that left.
-     * @param cause  the cause for the leave. If none is provided, then it will either be {@link ScoreCause#RULE_VIOLATION} or {$link ScoreCause#LEFT}, depending on whether the player has {@link Player#hasViolated()}.
-     */
-    override fun onPlayerLeft(player: Player, cause: ScoreCause?) {
-        if (cause == ScoreCause.REGULAR) return
-        val newCause = cause ?:
-            if (!player.hasViolated()) {
-                player.left = true
-                ScoreCause.LEFT
-            } else {
-                ScoreCause.RULE_VIOLATION
-            }
-        val scores = HashMap(generateScoreMap())
-        scores[player]?.let { score ->
-            scores[player] = PlayerScore(newCause, score.reason, score.parts)
-        }
-        notifyOnGameOver(scores)
-    }
-
     /** Advances the Game to [nextPlayer].
      * - sends out a state update
      * - invokes [notifyOnGameOver] if the game is over
@@ -119,7 +94,7 @@ abstract class AbstractGame<P : Player>(override val pluginUUID: String) : IGame
 
         if (checkWinCondition() != null) {
             logger.debug("Game over")
-            notifyOnGameOver(generateScoreMap())
+            stop()
         } else {
             logger.debug("Next player: $nextPlayer")
 
@@ -155,7 +130,7 @@ abstract class AbstractGame<P : Player>(override val pluginUUID: String) : IGame
         timeout.start {
             logger.warn("Player $player reached the timeout of ${timeout.hardTimeout}ms")
             player.hardTimeout = true
-            onPlayerLeft(player, ScoreCause.HARD_TIMEOUT)
+            stop()
         }
 
         logger.info("Sending MoveRequest to player $activePlayer")

server/src/sc/server/gaming/GameRoom.java

@@ -14,11 +14,8 @@
 import sc.framework.HelperMethods;
 import sc.framework.plugins.AbstractGame;
 import sc.framework.plugins.Player;
-import sc.framework.HelperMethods;
 import sc.networking.XStreamProvider;
 import sc.networking.clients.IClient;
-import sc.networking.clients.LobbyClient;
-import sc.networking.clients.ObservingClient;
 import sc.networking.clients.XStreamClient;
 import sc.protocol.ProtocolPacket;
 import sc.protocol.RemovedFromGame;
@@ -95,8 +92,7 @@ public synchronized void onGameOver(Map<Player, PlayerScore> results) {
       }
     }
 
-    kickAllClients();
-    cancel();
+    destroy();
   }
 
   public File createReplayFile() throws IOException {
@@ -262,8 +258,8 @@ private void startIfReady() {
   }
 
   private synchronized void start() {
-    setStatus(GameStatus.ACTIVE);
     this.game.start();
+    setStatus(GameStatus.ACTIVE);
     logger.info("Started {}", game);
   }
 
@@ -326,7 +322,7 @@ public synchronized void onEvent(Client source, IMove move) throws GameRoomExcep
       player.notifyListeners(errorMessage);
       observerBroadcast(errorMessage);
       history.add(errorMessage);
-      game.onPlayerLeft(player, ScoreCause.RULE_VIOLATION);
+      cancel();
       throw new GameLogicException(e.toString(), e);
     } catch (GameLogicException e) {
       player.notifyListeners(new ErrorMessage(move, e.getMessage()));
@@ -416,11 +412,12 @@ public synchronized void step(boolean forced) throws GameRoomException {
 
   /** Kick all players, destroy the game and remove it from the manager. */
   public void cancel() {
-    if (!isOver()) {
-      kickAllClients();
-      setStatus(GameStatus.OVER);
-    }
-    this.game.destroy();
+    // this will invoked onGameOver and thus stop everything else
+    this.game.stop();
+  }
+
+  private void destroy() {
+    kickAllClients();
     this.gameRoomManager.remove(this);
   }
 
@@ -455,10 +452,12 @@ protected void setStatus(GameStatus status) {
     this.status = status;
   }
 
-  /** Remove a player by calling {@link IGameInstance#onPlayerLeft(Player, ScoreCause) onPlayerLeft}. */
+  /** Remove a player and stop the game. */
   public void removePlayer(Player player, XStreamClient.DisconnectCause cause) {
     logger.info("Removing {} from {}", player, this);
-    this.game.onPlayerLeft(player, cause == XStreamClient.DisconnectCause.DISCONNECTED ? ScoreCause.REGULAR : null);
+    player.setLeft(true);
+    if (!isOver())
+      cancel();
   }
 
   /** Get the saved {@link GameResult result}. */

server/test/sc/server/plugins/TestGame.kt

@@ -44,15 +44,11 @@ data class TestGame(
         throw TooManyPlayersException()
     }
 
-    override fun onPlayerLeft(player: Player, cause: ScoreCause?) {
-        val result = generateScoreMap().toMutableMap()
-        result[player] = PlayerScore(cause ?: ScoreCause.LEFT, "Spieler hat das Spiel verlassen.", 0)
-        notifyOnGameOver(result)
-    }
-
-    override fun getScoreFor(player: Player): PlayerScore {
-        return PlayerScore(true, "Spieler hat gewonnen.")
-    }
+    override fun getScoreFor(player: Player) =
+            if(player.hasLeft())
+                PlayerScore(ScoreCause.LEFT, "Spieler ist rausgeflogen.", 0)
+            else
+                PlayerScore(true, "Spieler hat gewonnen.")
 
     override fun getTimeoutFor(player: Player): ActionTimeout =
             ActionTimeout(false)