Skip to content

Commit

Permalink
retire allowedMoves
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolodavis committed Sep 10, 2019
1 parent 7a411c9 commit fa58e5b
Show file tree
Hide file tree
Showing 11 changed files with 74 additions and 132 deletions.
2 changes: 0 additions & 2 deletions examples/react-web/src/phases/phases.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
Expand Down
4 changes: 2 additions & 2 deletions examples/react-web/src/turnorder/example-any-once.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const code = `{
startingPhase: 'A',
phases: {
A: { turnOrder: TurnOrder.ANY_ONCE, next: 'B' },
B: { allowedMoves: [] },
B: {},
}
},
}
Expand All @@ -40,7 +40,7 @@ export default {

phases: {
A: { turnOrder: TurnOrder.ANY_ONCE, next: 'B' },
B: { allowedMoves: [] },
B: {},
},
},
}),
Expand Down
10 changes: 2 additions & 8 deletions examples/react-web/src/turnorder/example-others-once.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,10 @@ const code = `{
startingPhase: 'play',
phases: {
play: {
allowedMoves: ['play'],
},
play: {},
discard: {
turnOrder: TurnOrder.OTHERS_ONCE,
allowedMoves: ['discard'],
},
},
},
Expand Down Expand Up @@ -53,13 +50,10 @@ export default {
startingPhase: 'play',

phases: {
play: {
allowedMoves: ['play'],
},
play: {},

discard: {
turnOrder: TurnOrder.OTHERS_ONCE,
allowedMoves: ['discard'],
},
},
},
Expand Down
16 changes: 5 additions & 11 deletions examples/react-web/src/turnorder/simulator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 23 additions & 3 deletions src/client/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
},
},
Expand All @@ -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({
Expand Down
39 changes: 9 additions & 30 deletions src/core/flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -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).
*
Expand Down Expand Up @@ -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', ...],
* }
Expand All @@ -273,7 +263,6 @@ export function FlowWithPhases({
endGame,
setActionPlayers,
undoableMoves,
allowedMoves,
redactedMoves,
optimisticUpdate,
game,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 }) => {
Expand Down Expand Up @@ -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) {
Expand All @@ -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 = {
Expand Down Expand Up @@ -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 || [];
Expand All @@ -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) => {
Expand Down
59 changes: 1 addition & 58 deletions src/core/flow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand All @@ -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'));
Expand Down
3 changes: 3 additions & 0 deletions src/core/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand Down
7 changes: 7 additions & 0 deletions src/core/reducer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit fa58e5b

Please sign in to comment.