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 8a0e0834c0a48587e24e6ff3d4589224b87a2a3b..60a22a633ee23e028b0fc94e9ffc9de345d6fe87 100644 (file)
--- 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 18e60efd947349be35218a643ac6ba7a9756971d..baa96f82a5469d66ccd51916005182e4361e0c74 100644 (file)
--- 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 256afecde34d86fd12af215d089c0b6cae9edcee..d123c5718f01b5fb93280447d003f91e4eac1e1a 100644 (file)
--- 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])