Implement revert, use array for _prevActivePlayers & remove activePla…

…yersDone (#449) * feat: Implement `revert` flag and use array for `_prevActivePlayers` Contributes towards #445. `_prevActivePlayers` is now always an array, which will be empty unless `setActivePlayers` (or `turn.activePlayers`) is passed `{ revert: true }` at which point the previous state will be stored. If `activePlayers` becomes empty and there is something saved in `_prevActivePlayers`, the last item will be used to restore `activePlayers`. * feat: Remove `activePlayersDone`
boardgameio · Sep 10, 2019 · d7a0c05 · d7a0c05
1 parent ec15ad2
commit d7a0c05
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 92 deletions.
diff --git a/examples/react-web/src/turnorder/example-all-once.js b/examples/react-web/src/turnorder/example-all-once.js
@@ -15,7 +15,7 @@ const code = `{
       start: true,
       next: 'B',
       turn: { activePlayers: ActivePlayers.ALL_ONCE },
-      endIf: (G, ctx) => ctx.activePlayersDone,
+      endIf: (G, ctx) => ctx.numMoves && ctx.activePlayers === null,
     },
     B: {},
   }
@@ -45,7 +45,7 @@ export default {
         start: true,
         next: 'B',
         turn: { activePlayers: ActivePlayers.ALL_ONCE },
-        endIf: (G, ctx) => ctx.activePlayersDone,
+        endIf: (G, ctx) => ctx.numMoves && ctx.activePlayers === null,
       },
       B: {},
     },

diff --git a/src/core/flow.js b/src/core/flow.js
@@ -415,7 +415,7 @@ export function Flow({ moves, phases, endIf, turn, events, plugins }) {
 
     // Initialize the turn order state.
     if (currentPlayer) {
-      ctx = { ...ctx, currentPlayer, activePlayersDone: null };
+      ctx = { ...ctx, currentPlayer };
       if (conf.turn.activePlayers) {
         ctx = SetActivePlayers(ctx, conf.turn.activePlayers);
       }
@@ -428,7 +428,7 @@ export function Flow({ moves, phases, endIf, turn, events, plugins }) {
     G = conf.turn.onBegin(G, ctx);
 
     const turn = ctx.turn + 1;
-    ctx = { ...ctx, turn, numMoves: 0, _prevActivePlayers: null };
+    ctx = { ...ctx, turn, numMoves: 0, _prevActivePlayers: [] };
 
     const plainCtx = ContextEnhancer.detachAllFromContext(ctx);
     const _undo = [{ G, ctx: plainCtx }];
@@ -672,12 +672,7 @@ export function Flow({ moves, phases, endIf, turn, events, plugins }) {
   function ProcessMove(state, action) {
     let conf = GetPhase(state.ctx);
 
-    let {
-      activePlayers,
-      _activePlayersOnce,
-      _prevActivePlayers,
-      activePlayersDone,
-    } = state.ctx;
+    let { activePlayers, _activePlayersOnce, _prevActivePlayers } = state.ctx;
 
     if (_activePlayersOnce) {
       const playerID = action.playerID;
@@ -687,12 +682,16 @@ export function Flow({ moves, phases, endIf, turn, events, plugins }) {
           obj[key] = activePlayers[key];
           return obj;
         }, {});
+    }
 
-      if (Object.keys(activePlayers).length == 0) {
-        activePlayers = state.ctx._prevActivePlayers;
+    if (activePlayers && Object.keys(activePlayers).length == 0) {
+      if (state.ctx._prevActivePlayers.length > 0) {
+        const lastIndex = state.ctx._prevActivePlayers.length - 1;
+        activePlayers = state.ctx._prevActivePlayers[lastIndex];
+        _prevActivePlayers = state.ctx._prevActivePlayers.slice(0, lastIndex);
+      } else {
+        activePlayers = null;
         _activePlayersOnce = false;
-        _prevActivePlayers = null;
-        activePlayersDone = true;
       }
     }
 
@@ -706,7 +705,6 @@ export function Flow({ moves, phases, endIf, turn, events, plugins }) {
       ctx: {
         ...state.ctx,
         activePlayers,
-        activePlayersDone,
         _activePlayersOnce,
         _prevActivePlayers,
         numMoves,

diff --git a/src/core/flow.test.js b/src/core/flow.test.js
@@ -10,7 +10,6 @@ import { makeMove, gameEvent } from './action-creators';
 import { Client } from '../client/client';
 import { FlowInternal, Flow } from './flow';
 import { error } from '../core/logger';
-import { Stage } from './turn-order';
 
 jest.mock('../core/logger', () => ({
   info: jest.fn(),
@@ -760,64 +759,4 @@ describe('activePlayers', () => {
       '2': 'B',
     });
   });
-
-  test('activePlayersDone', () => {
-    const spec = {
-      numPlayers: 3,
-      multiplayer: { local: true },
-      game: {
-        moves: {
-          moveA: (G, ctx) => {
-            ctx.events.setActivePlayers({ all: Stage.NULL, 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': Stage.NULL,
-      '1': Stage.NULL,
-      '2': Stage.NULL,
-    });
-
-    p0.moves.moveB();
-
-    expect(p0.getState().ctx.activePlayersDone).toBe(false);
-    expect(p0.getState().ctx.activePlayers).toEqual({
-      '1': Stage.NULL,
-      '2': Stage.NULL,
-    });
-
-    p1.moves.moveB();
-
-    expect(p0.getState().ctx.activePlayersDone).toBe(false);
-    expect(p0.getState().ctx.activePlayers).toEqual({
-      '2': Stage.NULL,
-    });
-
-    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);
-  });
 });
diff --git a/src/core/turn-order.js b/src/core/turn-order.js
@@ -39,9 +39,15 @@ export function SetActivePlayersEvent(state, arg) {
 }
 
 export function SetActivePlayers(ctx, arg) {
-  let _prevActivePlayers = ctx.activePlayers || null;
+  let { _prevActivePlayers } = ctx;
+
+  if (arg.revert) {
+    _prevActivePlayers = _prevActivePlayers.concat(ctx.activePlayers);
+  } else {
+    _prevActivePlayers = [];
+  }
+
   let activePlayers = {};
-  let activePlayersDone = null;
   let _activePlayersOnce = false;
 
   if (arg.value) {
@@ -76,14 +82,9 @@ export function SetActivePlayers(ctx, arg) {
     activePlayers = null;
   }
 
-  if (arg.once && Object.keys(activePlayers).length > 0) {
-    activePlayersDone = false;
-  }
-
   return {
     ...ctx,
     activePlayers,
-    activePlayersDone,
     _activePlayersOnce,
     _prevActivePlayers,
   };

diff --git a/src/core/turn-order.test.js b/src/core/turn-order.test.js
@@ -414,7 +414,6 @@ describe('setActivePlayers', () => {
       '0': Stage.NULL,
       '1': Stage.NULL,
     });
-    expect(newState.ctx.activePlayersDone).toBeNull();
   });
 
   test('once', () => {
@@ -436,13 +435,10 @@ describe('setActivePlayers', () => {
     let state = InitializeGame({ game });
     state = reducer(state, makeMove('B', null, '0'));
     expect(Object.keys(state.ctx.activePlayers)).toEqual(['0', '1']);
-    expect(state.ctx.activePlayersDone).toBe(false);
     state = reducer(state, makeMove('A', null, '0'));
     expect(Object.keys(state.ctx.activePlayers)).toEqual(['1']);
-    expect(state.ctx.activePlayersDone).toBe(false);
     state = reducer(state, makeMove('A', null, '1'));
     expect(state.ctx.activePlayers).toBeNull();
-    expect(state.ctx.activePlayersDone).toBe(true);
   });
 
   test('others', () => {
@@ -493,14 +489,14 @@ describe('setActivePlayers', () => {
 
       expect(state.ctx).toMatchObject({
         activePlayers: { '0': 'stage' },
-        _prevActivePlayers: null,
+        _prevActivePlayers: [],
       });
 
       state = reducer(state, makeMove('A', null, '0'));
 
       expect(state.ctx).toMatchObject({
         activePlayers: null,
-        _prevActivePlayers: null,
+        _prevActivePlayers: [],
       });
     });
 
@@ -511,6 +507,7 @@ describe('setActivePlayers', () => {
             ctx.events.setActivePlayers({
               currentPlayer: 'stage2',
               once: true,
+              revert: true,
             });
           },
           B: () => {},
@@ -526,21 +523,21 @@ describe('setActivePlayers', () => {
 
       expect(state.ctx).toMatchObject({
         activePlayers: { '0': 'stage1' },
-        _prevActivePlayers: null,
+        _prevActivePlayers: [],
       });
 
       state = reducer(state, makeMove('A', null, '0'));
 
       expect(state.ctx).toMatchObject({
         activePlayers: { '0': 'stage2' },
-        _prevActivePlayers: { '0': 'stage1' },
+        _prevActivePlayers: [{ '0': 'stage1' }],
       });
 
       state = reducer(state, makeMove('B', null, '0'));
 
       expect(state.ctx).toMatchObject({
         activePlayers: { '0': 'stage1' },
-        _prevActivePlayers: null,
+        _prevActivePlayers: [],
       });
     });
   });
@@ -560,6 +557,7 @@ describe('setActivePlayers', () => {
                   ctx.events.setActivePlayers({
                     others: 'B',
                     once: true,
+                    revert: true,
                   });
                 },
               },