retire allowedMoves

Author: nicolodavis

examples/react-web/src/phases/phases.js

@@ -25,11 +25,9 @@ const game = Game({
     phases: {
       take: {
         endPhaseIf: G => G.deck <= 0,
-        allowedMoves: ['takeCard'],
         next: 'play',
       },
       play: {
-        allowedMoves: ['playCard'],
         endPhaseIf: G => G.hand <= 0,
         next: 'take',
       },

examples/react-web/src/turnorder/example-any-once.js

@@ -14,7 +14,7 @@ const code = `{
     startingPhase: 'A',
     phases: {
       A: { turnOrder: TurnOrder.ANY_ONCE, next: 'B' },
-      B: { allowedMoves: [] },
+      B: {},
     }
   },
 }
@@ -40,7 +40,7 @@ export default {
 
       phases: {
         A: { turnOrder: TurnOrder.ANY_ONCE, next: 'B' },
-        B: { allowedMoves: [] },
+        B: {},
       },
     },
   }),

examples/react-web/src/turnorder/example-others-once.js

@@ -14,13 +14,10 @@ const code = `{
     startingPhase: 'play',
 
     phases: {
-      play: {
-        allowedMoves: ['play'],
-      },
+      play: {},
 
       discard: {
         turnOrder: TurnOrder.OTHERS_ONCE,
-        allowedMoves: ['discard'],
       },
     },
   },
@@ -53,13 +50,10 @@ export default {
       startingPhase: 'play',
 
       phases: {
-        play: {
-          allowedMoves: ['play'],
-        },
+        play: {},
 
         discard: {
           turnOrder: TurnOrder.OTHERS_ONCE,
-          allowedMoves: ['discard'],
         },
       },
     },

examples/react-web/src/turnorder/simulator.js

@@ -53,17 +53,11 @@ class Board extends React.Component {
       current = true;
     }
 
-    const moves = Object.entries(this.props.moves)
-      .filter(
-        e =>
-          this.props.ctx.allowedMoves === null ||
-          this.props.ctx.allowedMoves.includes(e[0])
-      )
-      .map(e => (
-        <button key={e[0]} onClick={() => e[1]()}>
-          {e[0]}
-        </button>
-      ));
+    const moves = Object.entries(this.props.moves).map(e => (
+      <button key={e[0]} onClick={() => e[1]()}>
+        {e[0]}
+      </button>
+    ));
 
     const events = Object.entries(this.props.events)
       .filter(() => current && active)

src/client/client.test.js

@@ -41,15 +41,15 @@ describe('namespaced moves', () => {
     client = Client({
       game: Game({
         moves: {
-          A: () => {},
+          A: () => 'A',
         },
 
         flow: {
           phases: {
             PA: {
               moves: {
-                B: () => {},
-                C: () => {},
+                B: () => 'B',
+                C: () => 'C',
               },
             },
           },
@@ -67,6 +67,26 @@ describe('namespaced moves', () => {
     expect(client.moves.PA.C).toBeInstanceOf(Function);
   });
 
+  test('moves are allowed only when phase is active', () => {
+    client.moves.A();
+    expect(client.getState().G).toEqual('A');
+
+    client.moves.PA.B();
+    expect(error).toHaveBeenCalledWith('disallowed move: PA.B');
+    client.moves.PA.C();
+    expect(error).toHaveBeenCalledWith('disallowed move: PA.C');
+
+    client.events.endPhase({ next: 'PA' });
+    expect(client.getState().ctx.phase).toBe('PA');
+
+    client.moves.PA.B();
+    expect(client.getState().G).toEqual('B');
+    client.moves.PA.C();
+    expect(client.getState().G).toEqual('C');
+    client.moves.A();
+    expect(client.getState().G).toEqual('A');
+  });
+
   test('name clash', () => {
     Client({
       game: Game({

src/core/flow.js

@@ -186,12 +186,6 @@ export function Flow({
  *
  * @param {...object} setActionPlayers - Set to true to enable the `setActionPlayers` event.
  *
- * @param {...object} allowedMoves - List of moves that are allowed.
- *                                   This can be either a list of
- *                                   move names or a function with the
- *                                   signature (G, ctx) => [].
- *                                   (default: null, i.e. all moves are allowed).
- *
  * @param {...object} undoableMoves - List of moves that are undoable,
  *                                   (default: null, i.e. all moves are undoable).
  *
@@ -250,10 +244,6 @@ export function Flow({
  *   // A phase-specific movesPerTurn.
  *   movesPerTurn: integer,
  *
- *   // List of moves or a function that returns a list of moves
- *   // that are allowed in this phase.
- *   allowedMoves: (G, ctx) => ['moveA', ...],
- *
  *   // List of moves that are undoable.
  *   undoableMoves: ['moveA', ...],
  * }
@@ -273,7 +263,6 @@ export function FlowWithPhases({
   endGame,
   setActionPlayers,
   undoableMoves,
-  allowedMoves,
   redactedMoves,
   optimisticUpdate,
   game,
@@ -305,7 +294,6 @@ export function FlowWithPhases({
   if (!onTurnEnd) onTurnEnd = G => G;
   if (!onMove) onMove = G => G;
   if (!turnOrder) turnOrder = TurnOrder.DEFAULT;
-  if (allowedMoves === undefined) allowedMoves = null;
   if (undoableMoves === undefined) undoableMoves = null;
 
   const phaseMap = phases;
@@ -365,13 +353,6 @@ export function FlowWithPhases({
     if (conf.undoableMoves === undefined) {
       conf.undoableMoves = undoableMoves;
     }
-    if (conf.allowedMoves === undefined) {
-      conf.allowedMoves = allowedMoves;
-    }
-    if (typeof conf.allowedMoves !== 'function') {
-      const t = conf.allowedMoves;
-      conf.allowedMoves = () => t;
-    }
   }
 
   const shouldEndPhase = ({ G, ctx }) => {
@@ -408,8 +389,7 @@ export function FlowWithPhases({
       },
     };
 
-    const allowedMoves = config.allowedMoves(G, ctx);
-    return { ...state, G, ctx: { ...ctx, allowedMoves } };
+    return { ...state, G, ctx };
   };
 
   const startTurn = function(state, config) {
@@ -419,7 +399,6 @@ export function FlowWithPhases({
     const _undo = [{ G, ctx: plainCtx }];
 
     const ctx = { ...state.ctx };
-    ctx.allowedMoves = config.allowedMoves(G, ctx);
 
     // Reset stats.
     ctx.stats = {
@@ -678,10 +657,6 @@ export function FlowWithPhases({
       return { ...state, ctx: { ...state.ctx, gameover } };
     }
 
-    // Update allowedMoves.
-    const allowedMoves = conf.allowedMoves(state.G, state.ctx);
-    state = { ...state, ctx: { ...state.ctx, allowedMoves } };
-
     // Update undo / redo state.
     if (!endTurn) {
       const undo = state._undo || [];
@@ -700,10 +675,14 @@ export function FlowWithPhases({
   }
 
   const canMakeMove = (G, ctx, moveName) => {
-    const conf = phaseMap[ctx.phase];
-    const moves = conf.allowedMoves(G, ctx);
-    if (!moves) return true;
-    return moves.includes(moveName);
+    // If this is a namespaced move, verify that
+    // we are in the correct phase.
+    if (moveName.includes('.')) {
+      const tokens = moveName.split('.');
+      return tokens[0] == ctx.phase;
+    }
+    // If not, then the move is allowed.
+    return true;
   };
 
   const canUndoMove = (G, ctx, moveName) => {

src/core/flow.test.js

@@ -487,17 +487,6 @@ test('canMakeMove', () => {
       B: () => ({ B: true }),
       C: () => ({ C: true }),
     },
-
-    flow: {
-      allowedMoves: ['A', 'B'],
-      startingPhase: 'A',
-      phases: {
-        A: { allowedMoves: () => ['A'] },
-        B: { allowedMoves: ['B'] },
-        C: {},
-        D: { allowedMoves: null },
-      },
-    },
   });
 
   const reducer = CreateGameReducer({ game });
@@ -510,53 +499,7 @@ test('canMakeMove', () => {
   flow = Flow({ canMakeMove: () => false });
   expect(flow.canMakeMove(state.G, state.ctx)).toBe(false);
 
-  // Phase A (A is allowed).
-  expect(state.ctx.phase).toBe('A');
-
-  state = reducer(state, makeMove('A'));
-  expect(state.G).toMatchObject({ A: true });
-  state = reducer(state, makeMove('B'));
-  expect(state.G).not.toMatchObject({ B: true });
-  state = reducer(state, makeMove('C'));
-  expect(state.G).not.toMatchObject({ C: true });
-
-  // Phase B (B is allowed).
-  state = reducer(state, gameEvent('endPhase', { next: 'B' }));
-  state.G = {};
-  expect(state.ctx.phase).toBe('B');
-
-  state = reducer(state, makeMove('A'));
-  expect(state.G).not.toMatchObject({ A: true });
-  state = reducer(state, makeMove('B'));
-  expect(state.G).toMatchObject({ B: true });
-  state = reducer(state, makeMove('C'));
-  expect(state.G).not.toMatchObject({ C: true });
-
-  // Phase C (A and B allowed).
-  state = reducer(state, gameEvent('endPhase', { next: 'C' }));
-  state.G = {};
-  expect(state.ctx.phase).toBe('C');
-
-  state = reducer(state, makeMove('A'));
-  expect(state.G).toMatchObject({ A: true });
-  state = reducer(state, makeMove('B'));
-  expect(state.G).toMatchObject({ B: true });
-  state = reducer(state, makeMove('C'));
-  expect(state.G).not.toMatchObject({ C: true });
-
-  // Phase D (A, B and C allowed).
-  state = reducer(state, gameEvent('endPhase', { next: 'D' }));
-  state.G = {};
-  expect(state.ctx.phase).toBe('D');
-
-  state = reducer(state, makeMove('A'));
-  expect(state.G).toMatchObject({ A: true });
-  state = reducer(state, makeMove('B'));
-  expect(state.G).toMatchObject({ B: true });
-  state = reducer(state, makeMove('C'));
-  expect(state.G).toMatchObject({ C: true });
-
-  // But not once the game is over.
+  // No moves are allowed after the game is over.
   state.ctx.gameover = true;
   state.G = {};
   state = reducer(state, makeMove('A'));

src/core/reducer.js

@@ -11,6 +11,7 @@ import * as Actions from './action-types';
 import { Random } from './random';
 import { Events } from './events';
 import * as plugins from '../plugins/main';
+import { error } from './logger';
 
 /**
  * Moves can return this when they want to indicate
@@ -235,11 +236,13 @@ export function CreateGameReducer({ game, multiplayer }) {
 
         // Check whether the game knows the move at all.
         if (!game.moveNames.includes(action.payload.type)) {
+          error(`unrecognized move: ${action.payload.type}`);
           return state;
         }
 
         // Ignore the move if it isn't allowed at this point.
         if (!game.flow.canMakeMove(state.G, state.ctx, action.payload.type)) {
+          error(`disallowed move: ${action.payload.type}`);
           return state;
         }
 

src/core/reducer.test.js

@@ -17,6 +17,12 @@ import {
   undo,
   redo,
 } from './action-creators';
+import { error } from '../core/logger';
+
+jest.mock('../core/logger', () => ({
+  info: jest.fn(),
+  error: jest.fn(),
+}));
 
 const game = Game({
   moves: {
@@ -57,6 +63,7 @@ test('makeMove', () => {
   state = reducer(state, makeMove('unknown'));
   expect(state._stateID).toBe(0);
   expect(state.G).not.toMatchObject({ moved: true });
+  expect(error).toHaveBeenCalledWith('unrecognized move: unknown');
 
   state = reducer(state, makeMove('A'));
   expect(state._stateID).toBe(1);