Skip to content

Commit

Permalink
Fix “once” and rename stage to activePlayers (#442)
Browse files Browse the repository at this point in the history
* test: Add failing test for ANY_ONCE stage default

If `ctx._stageOnce` is `true`, `flow.js` fetches keys from the 
`ctx.stage` object, but when it’s `null` (for example when ANY_ONCE has 
completed) there are no keys and a TypeError is thrown.

* test: Add failing test for OTHERS_ONCE stage default

* fix: Set _stageOnce to false once all players have played in stage

* feat: Add `ctx.stageCompleted` property

Adds a `stageCompleted` property, which is `null` when `once` is not 
active, `false` when it is, and `true` once all players have played 
during the stage.

* refactor: Rename `stage` API to `activePlayers`

`ctx.stage` ⇒ `ctx.activePlayers`

`ctx._stageOnce` ⇒ `ctx._activePlayersOnce`

`ctx.stageCompleted` ⇒ `ctx.activePlayersDone`

`events.setStage` ⇒ `events.setActivePlayers`

`turn.setStage` ⇒ `turn.activePlayers`

`turn.stages` remains unchanged

import { `Stage` ⇒ `ActivePlayers` } from 'boardgame.io/core'


Internal:

`SetStageEvent` ⇒ `SetActivePlayersEvent`

`SetStage` ⇒ `SetActivePlayers`

* refactor: `ANY` & `ANY_ONCE` ⇒ `ALL` & `ALL_ONCE` in `ActivePlayers`

* fix: Fix comment to match rename

* add test for actionPlayersDone

* reset activePlayersDone at the start of each turn
  • Loading branch information
delucis authored and nicolodavis committed Aug 23, 2019
1 parent 22c45cd commit b85f61a
Show file tree
Hide file tree
Showing 10 changed files with 217 additions and 118 deletions.
4 changes: 2 additions & 2 deletions examples/react-web/src/turnorder/simulator.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ class Board extends React.Component {
let current = false;
let onClick = () => {};

if (this.props.ctx.stage) {
if (this.props.playerID in this.props.ctx.stage) {
if (this.props.ctx.activePlayers) {
if (this.props.playerID in this.props.ctx.activePlayers) {
className += ' active';
active = true;
}
Expand Down
4 changes: 2 additions & 2 deletions packages/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import { INVALID_MOVE } from '../src/core/reducer.js';
import { Pass, Stage, TurnOrder } from '../src/core/turn-order.js';
import { Pass, ActivePlayers, TurnOrder } from '../src/core/turn-order.js';
import { PlayerView } from '../src/core/player-view.js';

export { Stage, TurnOrder, Pass, PlayerView, INVALID_MOVE };
export { ActivePlayers, TurnOrder, Pass, PlayerView, INVALID_MOVE };
12 changes: 6 additions & 6 deletions src/ai/bot.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ export function Simulate({ game, bots, state, depth }) {
let iter = 0;
while (state.ctx.gameover === undefined && iter < depth) {
let playerID = state.ctx.currentPlayer;
if (state.ctx.stage) {
playerID = Object.keys(state.ctx.stage)[0];
if (state.ctx.activePlayers) {
playerID = Object.keys(state.ctx.activePlayers)[0];
}

const bot = bots instanceof Bot ? bots : bots[playerID];
Expand Down Expand Up @@ -128,8 +128,8 @@ export class MCTSBot extends Bot {
if (playerID !== undefined) {
actions = this.enumerate(G, ctx, playerID);
objectives = this.objectives(G, ctx, playerID);
} else if (ctx.stage) {
for (let playerID in ctx.stage) {
} else if (ctx.activePlayers) {
for (let playerID in ctx.activePlayers) {
actions = actions.concat(this.enumerate(G, ctx, playerID));
objectives = objectives.concat(this.objectives(G, ctx, playerID));
}
Expand Down Expand Up @@ -218,8 +218,8 @@ export class MCTSBot extends Bot {
) {
const { G, ctx } = state;
let playerID = ctx.currentPlayer;
if (ctx.stage) {
playerID = Object.keys(ctx.stage)[0];
if (ctx.activePlayers) {
playerID = Object.keys(ctx.activePlayers)[0];
}
const moves = this.enumerate(G, ctx, playerID);

Expand Down
4 changes: 2 additions & 2 deletions src/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ class _ClientImpl {
const state = this.store.getState();

let playerID = state.ctx.currentPlayer;
if (state.ctx.stage) {
playerID = Object.keys(state.ctx.stage)[0];
if (state.ctx.activePlayers) {
playerID = Object.keys(state.ctx.activePlayers)[0];
}

const { action, metadata } = await bot.play(state, playerID);
Expand Down
6 changes: 3 additions & 3 deletions src/client/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ describe('event dispatchers', () => {
test('default', () => {
const game = {};
const client = Client({ game });
expect(Object.keys(client.events)).toEqual(['endTurn', 'setStage']);
expect(Object.keys(client.events)).toEqual(['endTurn', 'setActivePlayers']);
expect(client.getState().ctx.turn).toBe(1);
client.events.endTurn();
expect(client.getState().ctx.turn).toBe(2);
Expand All @@ -328,7 +328,7 @@ describe('event dispatchers', () => {
'endTurn',
'endPhase',
'endGame',
'setStage',
'setActivePlayers',
]);
expect(client.getState().ctx.turn).toBe(1);
client.events.endTurn();
Expand All @@ -340,7 +340,7 @@ describe('event dispatchers', () => {
events: {
endPhase: false,
endTurn: false,
setStage: false,
setActivePlayers: false,
},
};
const client = Client({ game });
Expand Down
62 changes: 34 additions & 28 deletions src/core/flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
*/

import {
SetStageEvent,
SetStage,
SetActivePlayersEvent,
SetActivePlayers,
InitTurnOrderState,
UpdateTurnOrderState,
TurnOrder,
Expand Down Expand Up @@ -87,8 +87,8 @@ export function FlowInternal({

canPlayerCallEvent: (_G, ctx, playerID) => {
const isCurrentPlayer = ctx.currentPlayer == playerID;
if (ctx.stage) {
return isCurrentPlayer && ctx.currentPlayer in ctx.stage;
if (ctx.activePlayers) {
return isCurrentPlayer && ctx.currentPlayer in ctx.activePlayers;
}
return isCurrentPlayer;
},
Expand All @@ -101,16 +101,16 @@ export function FlowInternal({
return false;
}

if (ctx.stage === null && ctx.currentPlayer !== playerID) {
if (ctx.activePlayers === null && ctx.currentPlayer !== playerID) {
return false;
}

return true;
},

canPlayerMakeAnyMove: (_G, ctx, playerID) => {
if (ctx.stage) {
return playerID in ctx.stage;
if (ctx.activePlayers) {
return playerID in ctx.activePlayers;
}
return ctx.currentPlayer === playerID;
},
Expand Down Expand Up @@ -198,8 +198,8 @@ export function Flow({ moves, phases, endIf, turn, events, plugins }) {
if (events === undefined) {
events = {};
}
if (events.setStage === undefined) {
events.setStage = true;
if (events.setActivePlayers === undefined) {
events.setActivePlayers = true;
}
if (events.endPhase === undefined && phases) {
events.endPhase = true;
Expand Down Expand Up @@ -416,9 +416,9 @@ export function Flow({ moves, phases, endIf, turn, events, plugins }) {

// Initialize the turn order state.
if (currentPlayer) {
ctx = { ...ctx, currentPlayer };
if (conf.turn.setStage) {
ctx = SetStage(ctx, conf.turn.setStage);
ctx = { ...ctx, currentPlayer, activePlayersDone: null };
if (conf.turn.activePlayers) {
ctx = SetActivePlayers(ctx, conf.turn.activePlayers);
}
} else {
// This is only called at the beginning of the phase
Expand Down Expand Up @@ -611,8 +611,8 @@ export function Flow({ moves, phases, endIf, turn, events, plugins }) {
next.push({ fn: UpdateTurn, arg, currentPlayer: ctx.currentPlayer });
}

// Reset currentPlayer and stages.
ctx = { ...ctx, currentPlayer: null, stage: null };
// Reset currentPlayer and activePlayers.
ctx = { ...ctx, currentPlayer: null, activePlayers: null };

// Add log entry.
const action = gameEvent('endTurn', arg);
Expand All @@ -635,7 +635,7 @@ export function Flow({ moves, phases, endIf, turn, events, plugins }) {
/**
* Retrieves the relevant move that can be played by playerID.
*
* If ctx.stage is set (i.e. one or more players are in some stage),
* If ctx.activePlayers is set (i.e. one or more players are in some stage),
* then it attempts to find the move inside the stages config for
* that turn. If the stage for a player is '', then the player is
* allowed to make a move (as determined by the phase config), but
Expand All @@ -652,10 +652,11 @@ export function Flow({ moves, phases, endIf, turn, events, plugins }) {
function GetMove(ctx, name, playerID) {
const conf = GetPhase(ctx);
const stages = conf.turn.stages;
const { activePlayers } = ctx;

if (ctx.stage && stages[ctx.stage[playerID]]) {
if (activePlayers && stages[activePlayers[playerID]]) {
// Check if moves are defined for the player's stage.
const stage = stages[ctx.stage[playerID]];
const stage = stages[activePlayers[playerID]];
if (stage) {
const moves = stage.moves;
if (name in moves) {
Expand All @@ -678,18 +679,21 @@ export function Flow({ moves, phases, endIf, turn, events, plugins }) {
function ProcessMove(state, action) {
let conf = GetPhase(state.ctx);

let stage = state.ctx.stage;
if (state.ctx._stageOnce) {
let { activePlayers, _activePlayersOnce, activePlayersDone } = state.ctx;

if (_activePlayersOnce) {
const playerID = action.playerID;
stage = Object.keys(stage)
activePlayers = Object.keys(activePlayers)
.filter(id => id !== playerID)
.reduce((obj, key) => {
obj[key] = stage[key];
obj[key] = activePlayers[key];
return obj;
}, {});

if (Object.keys(stage).length == 0) {
stage = null;
if (Object.keys(activePlayers).length == 0) {
activePlayers = null;
_activePlayersOnce = false;
activePlayersDone = true;
}
}

Expand All @@ -702,7 +706,9 @@ export function Flow({ moves, phases, endIf, turn, events, plugins }) {
...state,
ctx: {
...state.ctx,
stage,
activePlayers,
activePlayersDone,
_activePlayersOnce,
numMoves,
},
};
Expand Down Expand Up @@ -749,7 +755,7 @@ export function Flow({ moves, phases, endIf, turn, events, plugins }) {
endTurn: EndTurnEvent,
endPhase: EndPhaseEvent,
endGame: EndGameEvent,
setStage: SetStageEvent,
setActivePlayers: SetActivePlayersEvent,
};

let enabledEvents = {};
Expand All @@ -762,8 +768,8 @@ export function Flow({ moves, phases, endIf, turn, events, plugins }) {
if (events.endGame) {
enabledEvents['endGame'] = true;
}
if (events.setStage) {
enabledEvents['setStage'] = true;
if (events.setActivePlayers) {
enabledEvents['setActivePlayers'] = true;
}

return FlowInternal({
Expand All @@ -774,7 +780,7 @@ export function Flow({ moves, phases, endIf, turn, events, plugins }) {
playOrder: [...new Array(numPlayers)].map((d, i) => i + ''),
playOrderPos: 0,
phase: startingPhase,
stage: null,
activePlayers: null,
}),
init: state => {
return ProcessEvents(state, [{ fn: StartGame }]);
Expand Down
88 changes: 75 additions & 13 deletions src/core/flow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -437,17 +437,19 @@ test('canPlayerMakeAnyMove', () => {
let flow = FlowInternal({});
expect(flow.canPlayerMakeAnyMove({}, {}, playerID)).toBe(false);

expect(flow.canPlayerMakeAnyMove({}, { stage: { '1': '' } }, playerID)).toBe(
false
);
expect(flow.canPlayerMakeAnyMove({}, { stage: { '0': '' } }, playerID)).toBe(
true
);
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({}, { stage: null }, playerID)).toBe(false);
expect(flow.canPlayerMakeAnyMove({}, { activePlayers: null }, playerID)).toBe(
false
);
expect(flow.canPlayerMakeAnyMove({}, {}, '5')).toBe(false);
});

Expand All @@ -459,7 +461,7 @@ test('canPlayerCallEvent', () => {
expect(
flow.canPlayerCallEvent(
{},
{ currentPlayer: '0', stage: { '1': '' } },
{ currentPlayer: '0', activePlayers: { '1': '' } },
playerID
)
).toBe(false);
Expand Down Expand Up @@ -662,12 +664,12 @@ describe('infinite loops', () => {
});
});

describe('setStage', () => {
test('sets stages at each turn', () => {
describe('activePlayers', () => {
test('sets activePlayers at each turn', () => {
const game = {
turn: {
stages: { A: {}, B: {} },
setStage: {
activePlayers: {
currentPlayer: 'A',
others: 'B',
},
Expand All @@ -677,7 +679,7 @@ describe('setStage', () => {
const client = Client({ game, numPlayers: 3 });

expect(client.getState().ctx.currentPlayer).toBe('0');
expect(client.getState().ctx.stage).toEqual({
expect(client.getState().ctx.activePlayers).toEqual({
'0': 'A',
'1': 'B',
'2': 'B',
Expand All @@ -686,10 +688,70 @@ describe('setStage', () => {
client.events.endTurn();

expect(client.getState().ctx.currentPlayer).toBe('1');
expect(client.getState().ctx.stage).toEqual({
expect(client.getState().ctx.activePlayers).toEqual({
'0': 'B',
'1': 'A',
'2': 'B',
});
});

test('activePlayersDone', () => {
const spec = {
numPlayers: 3,
multiplayer: { local: true },
game: {
moves: {
moveA: (G, ctx) => {
ctx.events.setActivePlayers({ all: '', once: true });
},
moveB: G => G,
},
},
};

const p0 = Client({ ...spec, playerID: '0' });
const p1 = Client({ ...spec, playerID: '1' });
const p2 = Client({ ...spec, playerID: '2' });

p0.connect();
p1.connect();
p2.connect();

expect(p0.getState().ctx.currentPlayer).toBe('0');

expect(p0.getState().ctx.activePlayersDone).toBe(null);
p0.moves.moveA();
expect(p0.getState().ctx.activePlayersDone).toBe(false);

expect(p0.getState().ctx.activePlayers).toEqual({
'0': '',
'1': '',
'2': '',
});

p0.moves.moveB();

expect(p0.getState().ctx.activePlayersDone).toBe(false);
expect(p0.getState().ctx.activePlayers).toEqual({
'1': '',
'2': '',
});

p1.moves.moveB();

expect(p0.getState().ctx.activePlayersDone).toBe(false);
expect(p0.getState().ctx.activePlayers).toEqual({
'2': '',
});

p2.moves.moveB();

expect(p0.getState().ctx.activePlayersDone).toBe(true);
expect(p0.getState().ctx.activePlayers).toEqual(null);

p0.events.endTurn();

expect(p0.getState().ctx.activePlayersDone).toBe(null);
expect(p0.getState().ctx.activePlayers).toEqual(null);
});
});
Loading

0 comments on commit b85f61a

Please sign in to comment.