allow all players to call events

also simplify canPlayerCallEvent/canPlayerMakeMove

src/client/client.js

@@ -320,21 +320,21 @@ class _ClientImpl {
 
     let isActive = true;
 
-    const canPlayerMakeMove = this.game.flow.canPlayerMakeAnyMove(
+    const isPlayerActive = this.game.flow.isPlayerActive(
       state.G,
       state.ctx,
       this.playerID
     );
 
-    if (this.multiplayer && !canPlayerMakeMove) {
+    if (this.multiplayer && !isPlayerActive) {
       isActive = false;
     }
 
     if (
       !this.multiplayer &&
       this.playerID !== null &&
       this.playerID !== undefined &&
-      !canPlayerMakeMove
+      !isPlayerActive
     ) {
       isActive = false;
     }

src/core/flow.js

@@ -87,36 +87,7 @@ export function FlowInternal({
       return dispatch(state, action);
     },
 
-    canPlayerCallEvent: (_G, ctx, playerID) => {
-      const isCurrentPlayer = ctx.currentPlayer == playerID;
-      if (ctx.activePlayers) {
-        return isCurrentPlayer && ctx.currentPlayer in ctx.activePlayers;
-      }
-      return isCurrentPlayer;
-    },
-
-    canPlayerMakeMove: (_G, ctx, action) => {
-      const playerID = action.payload.playerID;
-      const move = getMove(ctx, action.payload.type, playerID);
-
-      if (move === null) {
-        return false;
-      }
-
-      if (ctx.activePlayers) {
-        if (!(playerID in ctx.activePlayers)) {
-          return false;
-        }
-      } else {
-        if (ctx.currentPlayer !== playerID) {
-          return false;
-        }
-      }
-
-      return true;
-    },
-
-    canPlayerMakeAnyMove: (_G, ctx, playerID) => {
+    isPlayerActive: (_G, ctx, playerID) => {
       if (ctx.activePlayers) {
         return playerID in ctx.activePlayers;
       }

src/core/flow.test.js

@@ -650,43 +650,19 @@ describe('endIf', () => {
   });
 });
 
-test('canPlayerMakeAnyMove', () => {
-  const playerID = '0';
-
-  let flow = FlowInternal({});
-  expect(flow.canPlayerMakeAnyMove({}, {}, playerID)).toBe(false);
-
-  expect(
-    flow.canPlayerMakeAnyMove({}, { activePlayers: { '1': '' } }, playerID)
-  ).toBe(false);
-  expect(
-    flow.canPlayerMakeAnyMove({}, { activePlayers: { '0': '' } }, playerID)
-  ).toBe(true);
-
-  // no one can make a move
-  flow = FlowInternal({ canPlayerMakeAnyMove: () => false });
-  expect(flow.canPlayerMakeAnyMove({}, {}, playerID)).toBe(false);
-  expect(flow.canPlayerMakeAnyMove({}, { activePlayers: null }, playerID)).toBe(
-    false
-  );
-  expect(flow.canPlayerMakeAnyMove({}, {}, '5')).toBe(false);
-});
-
-test('canPlayerCallEvent', () => {
+test('isPlayerActive', () => {
   const playerID = '0';
 
   const flow = FlowInternal({});
-  expect(flow.canPlayerCallEvent({}, {}, playerID)).toBe(false);
+  expect(flow.isPlayerActive({}, {}, playerID)).toBe(false);
   expect(
-    flow.canPlayerCallEvent(
+    flow.isPlayerActive(
       {},
       { currentPlayer: '0', activePlayers: { '1': '' } },
       playerID
     )
   ).toBe(false);
-  expect(flow.canPlayerCallEvent({}, { currentPlayer: '0' }, playerID)).toBe(
-    true
-  );
+  expect(flow.isPlayerActive({}, { currentPlayer: '0' }, playerID)).toBe(true);
 });
 
 describe('endGame', () => {

src/core/reducer.js

@@ -64,16 +64,19 @@ export function CreateGameReducer({ game, multiplayer }) {
           return state;
         }
 
-        // Ignore the event if the player isn't allowed to make it.
+        // Disallow events once the game is over.
+        if (state.ctx.gameover !== undefined) {
+          error(`cannot call event after game end`);
+          return state;
+        }
+
+        // Ignore the event if the player isn't active.
         if (
           action.payload.playerID !== null &&
           action.payload.playerID !== undefined &&
-          !game.flow.canPlayerCallEvent(
-            state.G,
-            state.ctx,
-            action.payload.playerID
-          )
+          !game.flow.isPlayerActive(state.G, state.ctx, action.payload.playerID)
         ) {
+          error(`disallowed event: ${action.payload.type}`);
           return state;
         }
 
@@ -115,11 +118,11 @@ export function CreateGameReducer({ game, multiplayer }) {
           return state;
         }
 
-        // Ignore the move if the player isn't allowed to make it.
+        // Ignore the move if the player isn't active.
         if (
           action.payload.playerID !== null &&
           action.payload.playerID !== undefined &&
-          !game.flow.canPlayerMakeMove(state.G, state.ctx, action)
+          !game.flow.isPlayerActive(state.G, state.ctx, action.payload.playerID)
         ) {
           error(`disallowed move: ${action.payload.type}`);
           return state;

src/core/reducer.test.js

@@ -76,6 +76,10 @@ test('makeMove', () => {
   state = reducer(state, makeMove('B'));
   expect(state._stateID).toBe(2);
   expect(error).toBeCalledWith('cannot make move after game end');
+
+  state = reducer(state, gameEvent('endTurn'));
+  expect(state._stateID).toBe(2);
+  expect(error).toBeCalledWith('cannot call event after game end');
 });
 
 test('disable move by invalid playerIDs', () => {

src/master/master.js

@@ -9,7 +9,7 @@
 import { InitializeGame } from '../core/initialize';
 import { CreateGameReducer } from '../core/reducer';
 import { Game } from '../core/game';
-import { UNDO, REDO, MAKE_MOVE, GAME_EVENT } from '../core/action-types';
+import { UNDO, REDO, MAKE_MOVE } from '../core/action-types';
 import { createStore } from 'redux';
 import * as logging from '../core/logger';
 
@@ -155,6 +155,11 @@ export class Master {
       return { error: 'game not found' };
     }
 
+    if (state.ctx.gameover !== undefined) {
+      logging.error(`game over - gameID=[${key}]`);
+      return;
+    }
+
     const reducer = CreateGameReducer({
       game: this.game,
       numPlayers: state.ctx.numPlayers,
@@ -174,26 +179,23 @@ export class Master {
       }
     }
 
+    // Check whether the player is active.
+    if (!this.game.flow.isPlayerActive(state.G, state.ctx, playerID)) {
+      logging.error(`player not active - playerID=[${playerID}]`);
+      return;
+    }
+
     // Check whether the player is allowed to make the move.
     if (
       action.type == MAKE_MOVE &&
-      !this.game.flow.canPlayerMakeMove(state.G, state.ctx, action)
+      !this.game.flow.getMove(state.ctx, action.payload.type, playerID)
     ) {
       logging.error(
         `move not processed - canPlayerMakeMove=false, playerID=[${playerID}]`
       );
       return;
     }
 
-    // Check whether the player is allowed to call the event.
-    if (
-      action.type == GAME_EVENT &&
-      !this.game.flow.canPlayerCallEvent(state.G, state.ctx, playerID)
-    ) {
-      logging.error(`event not processed - invalid playerID=[${playerID}]`);
-      return;
-    }
-
     if (state._stateID !== stateID) {
       logging.error(
         `invalid stateID, was=[${stateID}], expected=[${state._stateID}]`

src/master/master.test.js

@@ -16,7 +16,7 @@ jest.mock('../core/logger', () => ({
   error: jest.fn(),
 }));
 
-const game = { seed: 0, flow: { setActionPlayers: true } };
+const game = { seed: 0 };
 
 function TransportAPI(send = jest.fn(), sendAll = jest.fn()) {
   return { send, sendAll };
@@ -82,7 +82,8 @@ describe('update', () => {
   const sendAll = jest.fn(arg => {
     sendAllReturn = arg;
   });
-  const master = new Master(game, new InMemory(), TransportAPI(send, sendAll));
+  const db = new InMemory();
+  const master = new Master(game, db, TransportAPI(send, sendAll));
   const action = ActionCreators.gameEvent('endTurn');
 
   beforeAll(async () => {
@@ -171,8 +172,14 @@ describe('update', () => {
     await master.onUpdate(action, 1, 'gameID', '100');
     await master.onUpdate(ActionCreators.makeMove(), 1, 'gameID', '100');
     expect(sendAll).not.toHaveBeenCalled();
+    expect(error).toHaveBeenCalledWith(`player not active - playerID=[100]`);
+  });
+
+  test('invalid move', async () => {
+    await master.onUpdate(ActionCreators.makeMove('move'), 1, 'gameID', '1');
+    expect(sendAll).not.toHaveBeenCalled();
     expect(error).toHaveBeenCalledWith(
-      `event not processed - invalid playerID=[100]`
+      `move not processed - canPlayerMakeMove=false, playerID=[1]`
     );
   });
 
@@ -191,17 +198,12 @@ describe('update', () => {
     );
   });
 
-  test('playerID is not an action player', async () => {
-    const setActionPlayersEvent = ActionCreators.gameEvent('setActionPlayers', [
-      '1',
-    ]);
-    await master.onUpdate(setActionPlayersEvent, 2, 'gameID', '0');
-
-    const move = ActionCreators.makeMove('move');
-    await master.onUpdate(move, 3, 'gameID', '0');
-    expect(error).toHaveBeenCalledWith(
-      `move not processed - canPlayerMakeMove=false, playerID=[0]`
-    );
+  test('game over', async () => {
+    let event = ActionCreators.gameEvent('endGame');
+    await master.onUpdate(event, 2, 'gameID', '0');
+    event = ActionCreators.gameEvent('endTurn');
+    await master.onUpdate(event, 3, 'gameID', '0');
+    expect(error).toHaveBeenCalledWith(`game over - gameID=[gameID]`);
   });
 });