refactor(Game): handle player action errors solely in GameRoom

Author: xeruf

plugin/src/server/sc/plugin2021/Game.kt

@@ -137,19 +137,15 @@ class Game: AbstractGame<Player>(GamePlugin.PLUGIN_UUID) {
 
     @Throws(InvalidMoveException::class)
     override fun onRoundBasedAction(fromPlayer: Player, data: ProtocolMessage) {
-        try {
-            if (data !is Move)
-                throw InvalidMoveException(MoveMistake.INVALID_FORMAT)
-            
-            logger.debug("Current State: $currentState")
-            logger.debug("Performing Move $data")
-            GameRuleLogic.performMove(currentState, data)
-            GameRuleLogic.removeInvalidColors(currentState)
-            next(if (isGameOver) null else currentState.currentPlayer)
-            logger.debug("Current Board:\n${currentState.board}")
-        } catch(e: InvalidMoveException) {
-            handleInvalidMove(e, fromPlayer)
-        }
+        if (data !is Move)
+            throw InvalidMoveException(MoveMistake.INVALID_FORMAT)
+        
+        logger.debug("Current State: $currentState")
+        logger.debug("Performing Move $data")
+        GameRuleLogic.performMove(currentState, data)
+        GameRuleLogic.removeInvalidColors(currentState)
+        next(if (isGameOver) null else currentState.currentPlayer)
+        logger.debug("Current Board:\n${currentState.board}")
     }
 
     override fun toString(): String =

sdk/src/server-api/sc/api/plugins/exceptions/NotYourTurnException.kt

@@ -0,0 +1,10 @@
+package sc.api.plugins.exceptions
+
+import sc.framework.plugins.Player
+import sc.protocol.responses.ProtocolMessage
+
+data class NotYourTurnException(
+        val expected: Player?,
+        val actual: Player,
+        val data: ProtocolMessage):
+        GameLogicException("It's not your turn yet; expected: $expected, got $actual (msg was $data).")

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

@@ -4,8 +4,8 @@ import org.slf4j.LoggerFactory
 import sc.api.plugins.IGameInstance
 import sc.api.plugins.IGameState
 import sc.api.plugins.exceptions.GameLogicException
+import sc.api.plugins.exceptions.NotYourTurnException
 import sc.api.plugins.host.IGameListener
-import sc.protocol.responses.ProtocolErrorMessage
 import sc.protocol.responses.ProtocolMessage
 import sc.shared.*
 
@@ -39,34 +39,25 @@ abstract class AbstractGame<P : Player>(override val pluginUUID: String) : IGame
      *
      * @throws GameLogicException if any invalid action is done, i.e. game rule violation
      */
-    @Throws(GameLogicException::class)
+    @Throws(GameLogicException::class, InvalidMoveException::class)
     override fun onAction(fromPlayer: Player, data: ProtocolMessage) {
-        var error: String? = null
-        if (fromPlayer == activePlayer) {
-            moveRequestTimeout?.let { timer ->
-                timer.stop()
-                logger.info("Time needed for move:" + timer.timeDiff)
-                if (timer.didTimeout()) {
-                    logger.warn("Client hit soft-timeout.")
-                    fromPlayer.softTimeout = true
-                    onPlayerLeft(fromPlayer, ScoreCause.SOFT_TIMEOUT)
-                } else {
-                    onRoundBasedAction(fromPlayer, data)
-                }
-            } ?: run {
-                error = "We didn't request a move from you yet."
+        if (fromPlayer != activePlayer)
+            throw NotYourTurnException(activePlayer, fromPlayer, data)
+        moveRequestTimeout?.let { timer ->
+            timer.stop()
+            logger.info("Time needed for move: " + timer.timeDiff)
+            if (timer.didTimeout()) {
+                logger.warn("Client hit soft-timeout.")
+                fromPlayer.softTimeout = true
+                onPlayerLeft(fromPlayer, ScoreCause.SOFT_TIMEOUT)
+            } else {
+                onRoundBasedAction(fromPlayer, data)
             }
-        } else {
-            error = "It's not your turn yet; expected: $activePlayer, got $fromPlayer (msg was $data)."
-        }
-        error?.let {
-            fromPlayer.notifyListeners(ProtocolErrorMessage(data, it))
-            throw GameLogicException(it)
-        }
+        } ?: throw GameLogicException("We didn't request a move from you yet.")
     }
 
-    /** Called by [onAction] to execute a move from a Player. */
-    @Throws(GameLogicException::class, InvalidMoveException::class)
+    /** Called by [onAction] to execute a move of a Player. */
+    @Throws(InvalidMoveException::class)
     abstract fun onRoundBasedAction(fromPlayer: Player, data: ProtocolMessage)
 
     /**
@@ -214,22 +205,4 @@ abstract class AbstractGame<P : Player>(override val pluginUUID: String) : IGame
             }
         }
     }
-
-    /**
-     * For use in a catch block when an invalid move was performed.
-     * Logs and notifies listeners about the violation.
-     *
-     * @param e      catched Exception, rethrown at the end
-     * @param author player who caused the exception
-     *
-     * @throws InvalidMoveException Always thrown
-     */
-    @Throws(InvalidMoveException::class)
-    fun handleInvalidMove(e: InvalidMoveException, author: Player): Nothing {
-        val error = "Ungueltiger Zug von '${author.displayName}'.\n$e"
-        logger.error(error)
-        author.violationReason = e.message
-        author.notifyListeners(ProtocolErrorMessage(e.move, error))
-        throw e
-    }
 }

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

@@ -5,9 +5,7 @@
 import org.slf4j.LoggerFactory;
 import sc.api.plugins.IGameInstance;
 import sc.api.plugins.IGameState;
-import sc.api.plugins.exceptions.GameLogicException;
-import sc.api.plugins.exceptions.RescuableClientException;
-import sc.api.plugins.exceptions.TooManyPlayersException;
+import sc.api.plugins.exceptions.*;
 import sc.api.plugins.host.IGameListener;
 import sc.framework.plugins.AbstractGame;
 import sc.framework.plugins.Player;
@@ -263,7 +261,7 @@ public String getId() {
    *
    * @return true if successfully joined
    */
-  public synchronized boolean join(Client client) throws RescuableClientException {
+  public synchronized boolean join(Client client) throws GameRoomException {
     PlayerSlot openSlot = null;
 
     for (PlayerSlot slot : this.playerSlots) {
@@ -295,7 +293,7 @@ public synchronized boolean join(Client client) throws RescuableClientException
    * @param openSlot PlayerSlot to fill
    * @param client   Client to fill PlayerSlot
    */
-  synchronized void fillSlot(PlayerSlot openSlot, Client client) throws RescuableClientException {
+  synchronized void fillSlot(PlayerSlot openSlot, Client client) throws GameRoomException {
     openSlot.setClient(client); // set role of Slot as PlayerRole
 
     if (!this.prepared) // is set when game is game is created or prepared
@@ -313,7 +311,7 @@ synchronized void fillSlot(PlayerSlot openSlot, Client client) throws RescuableC
    * Registers player to role in given slot.
    * Sends JoinGameProtocolMessage when successful.
    */
-  private void syncSlot(PlayerSlot slot) throws RescuableClientException {
+  private void syncSlot(PlayerSlot slot) throws GameRoomException {
     // create new player in gameState of game
     Player player = game.onPlayerJoined();
     // set attributes for player
@@ -347,7 +345,7 @@ private boolean isReady() {
   }
 
   /** Starts game if ready and not over. */
-  private void startIfReady() throws RescuableClientException {
+  private void startIfReady() throws GameRoomException {
     logger.debug("startIfReady called");
     if (isOver()) {
       logger.warn("Game already over: {}", game);
@@ -364,7 +362,7 @@ private void startIfReady() throws RescuableClientException {
   }
 
   /** If the Game is prepared, sync all slots. */
-  private synchronized void start() throws RescuableClientException {
+  private synchronized void start() throws GameRoomException {
     if (this.prepared) // sync slots for prepared game. This was already called for PlayerSlots in a game created by a join
     {
       for (PlayerSlot slot : this.playerSlots) {
@@ -427,34 +425,43 @@ public synchronized List<String> reserveAllSlots() {
    * @param source Client which caused the event
    * @param data   ProtocolMessage containing the action
    */
-  public synchronized void onEvent(Client source, ProtocolMessage data) throws RescuableClientException, InvalidGameStateException {
+  public synchronized void onEvent(Client source, ProtocolMessage data) throws GameRoomException {
     if (isOver())
-      throw new RescuableClientException("Game is already over, but got message: " + data.getClass());
+      throw new GameException("Game is already over, but got " + data);
 
+    Player player = resolvePlayer(source);
     try {
-      this.game.onAction(resolvePlayer(source), data);
+      game.onAction(player, data);
     } catch (InvalidMoveException e) {
-      this.observerBroadcast(new RoomPacket(this.id, new ProtocolErrorMessage(e.move, e.getMessage())));
-      this.game.onPlayerLeft(resolvePlayer(source), ScoreCause.RULE_VIOLATION);
-      throw new GameLogicException(e.toString());
+      final String error = String.format("Ungueltiger Zug von '%s'.\n%s", player.getDisplayName(), e);
+      logger.error(error);
+      player.setViolationReason(e.getMessage());
+      ProtocolErrorMessage errorMessage = new ProtocolErrorMessage(e.move, error);
+      player.notifyListeners(errorMessage);
+      observerBroadcast(new RoomPacket(id, errorMessage));
+      game.onPlayerLeft(player, ScoreCause.RULE_VIOLATION);
+      throw new GameLogicException(e.toString(), e);
+    } catch (GameLogicException e) {
+      player.notifyListeners(new ProtocolErrorMessage(data, e.getMessage()));
+      throw e;
     }
   }
 
   /** Finds player matching the given client. */
-  private Player resolvePlayer(Client client) throws RescuableClientException {
+  private Player resolvePlayer(Client client) throws GameRoomException {
     for (PlayerRole role : getPlayers()) {
       if (role.getClient().equals(client)) {
         Player resolvedPlayer = role.getPlayer();
 
         if (resolvedPlayer == null) {
-          throw new RescuableClientException("Game isn't ready. Please wait before sending messages.");
+          throw new GameException("Game isn't ready. Please wait before sending messages.");
         }
 
         return resolvedPlayer;
       }
     }
 
-    throw new RescuableClientException("Client is not a member of game " + this.id);
+    throw new GameRoomException("Client is not a member of game " + this.id);
   }
 
   /** Get {@link PlayerRole Players} that occupy a slot. */
@@ -519,7 +526,7 @@ public synchronized void pause(boolean pause) {
    * @param forced If true, the game will be forcibly started if starting
    *               conditions are not met. This should result in a GameOver.
    */
-  public synchronized void step(boolean forced) throws RescuableClientException {
+  public synchronized void step(boolean forced) throws GameRoomException {
     if (this.status == GameStatus.CREATED) {
       if (forced) {
         logger.warn("Forcing game start for {}", game);