From: Carl Worth <cworth@cworth.org>
Date: Fri, 5 Jun 2020 20:56:45 +0000 (-0700)
Subject: Move some checks from TicTacToe.add_move to Game.add_move
X-Git-Url: https://git.cworth.org/git?p=empires-server;a=commitdiff_plain;h=041cdbd90052345df5a05779ada61aec53e403d4

Move some checks from TicTacToe.add_move to Game.add_move

Specifically, the checks for whether the player who submitted a move
belongs on a team and belongs to the team that has the next move.

These checks belong in the generic Game class so that future games
won't have to maintain their own copies of these implementations.

Previously, we were using the presence of the "add_move" property on a
game class to determine whether to add the "/move" route. Now that we
are adding add_move to the base class, we have to check specifically
whether the child class has its own add_move (with
hasOwnProperty('add_move')) to know whether the "/move" route should
be added.
---

diff --git a/game.js b/game.js
index 8a0e083..60a22a6 100644
--- a/game.js
+++ b/game.js
@@ -45,6 +45,9 @@ class Game {
     this.players = [];
     this.next_player_id = 1;
     this.teams = [];
+    this.state = {
+      team_to_play: ""
+    };
 
     /* Send a comment to every connected client every 15 seconds. */
     setInterval(() => {this.broadcast_string(":");}, 15000);
@@ -70,6 +73,28 @@ class Game {
     return this._meta;
   }
 
+  /* Just performs some checks for whether a move is definitely not
+   * legal (such as not the player's turn). A child class is expected
+   * to override this (and call super.add_move early!) to implement
+   * the actual logic for a move. */
+  add_move(player, move) {
+    /* Cannot move if you are not on a team. */
+    if (player.team === "")
+    {
+      return { legal: false,
+               message: "You must be on a team to take a turn" };
+    }
+
+    /* Cannot move if it's not this player's team's turn. */
+    if (player.team !== this.state.team_to_play)
+    {
+      return { legal: false,
+               message: "It's not your turn to move" };
+    }
+
+    return { legal: true };
+  }
+
   add_player(session, connection) {
     /* First see if we already have a player object for this session. */
     const existing = this.players[session.id];
diff --git a/lmno.js b/lmno.js
index 18e60ef..baa96f8 100644
--- a/lmno.js
+++ b/lmno.js
@@ -305,7 +305,12 @@ for (let key in engines) {
 
   /* Further, add some routes conditionally depending on whether the
    * engine provides specific, necessary methods for the routes. */
-  if (engine.prototype.add_move) {
+
+  /* Note: We have to use hasOwnProperty here since the base Game
+   * class has a geeric add_move function, and we don't want that to
+   * have any influence on our decision. Only if the child has
+   * overridden that do we want to create a "/move" route. */
+  if (engine.prototype.hasOwnProperty("add_move")) {
     router.post('/move', (request, response) => {
       const game = request.game;
       const move = request.body.move;
diff --git a/tictactoe.js b/tictactoe.js
index 256afec..d123c57 100644
--- a/tictactoe.js
+++ b/tictactoe.js
@@ -15,19 +15,12 @@ class TicTacToe extends Game {
   /* Returns true if move was legal and added, false otherwise. */
   add_move(player, square) {
 
-    /* Cannot move if you are not on a team. */
-    if (player.team === "")
-    {
-      return { legal: false,
-               message: "You must be on a team to take a turn" };
-    }
+    const result = super.add_move(player, square);
 
-    /* Cannot move if it's not this player's team's turn. */
-    if (player.team !== this.state.team_to_play)
-    {
-      return { legal: false,
-               message: "It's not your turn to move" };
-    }
+    /* If the generic Game class can reject this move, then we don't
+     * need to look at it any further. */
+    if (! result.legal)
+      return result;
 
     /* Cannot move to an occupied square. */
     if (this.state.board[square])