From be3bf3f76bde4d48b697dd00fc9f076e266cfa0e Mon Sep 17 00:00:00 2001 From: Philip Shao Date: Sun, 23 May 2021 10:32:22 -0400 Subject: [PATCH] Update error types for action_invalid. --- src/core/errors.ts | 2 ++ src/core/reducer.test.ts | 40 ++++++++++++++++++++++--- src/core/reducer.ts | 27 +++++++---------- src/master/master.test.ts | 63 +++++++++++++++++++++++++-------------- 4 files changed, 88 insertions(+), 44 deletions(-) diff --git a/src/core/errors.ts b/src/core/errors.ts index 8fad0ec11..267c8621d 100644 --- a/src/core/errors.ts +++ b/src/core/errors.ts @@ -28,4 +28,6 @@ export enum ActionErrorType { GameOver = 'action/gameover', // The requested action is disabled (e.g. undo/redo, events) ActionDisabled = 'action/action_disabled', + // The requested action is not currently possible + ActionInvalid = 'action/action_invalid', } diff --git a/src/core/reducer.test.ts b/src/core/reducer.test.ts index 3e26754a0..0d2c83917 100644 --- a/src/core/reducer.test.ts +++ b/src/core/reducer.test.ts @@ -438,7 +438,15 @@ describe('undo / redo', () => { test('redo only resets deltalog if nothing to redo', () => { const state = reducer(initialState, makeMove('move', 'A', '0')); - expect(reducer(state, redo())).toEqual({ ...state, deltalog: [] }); + expect(reducer(state, redo())).toMatchObject({ + ...state, + deltalog: [], + transients: { + error: { + type: 'action/action_invalid', + }, + }, + }); }); }); @@ -533,13 +541,29 @@ describe('undo stack', () => { test('can’t undo at the start of a turn', () => { const newState = reducer(state, undo()); - expect(newState).toEqual({ ...state, deltalog: [] }); + expect(newState).toMatchObject({ + ...state, + deltalog: [], + transients: { + error: { + type: 'action/action_invalid', + }, + }, + }); }); test('can’t undo another player’s move', () => { state = reducer(state, makeMove('basic', null, '1')); const newState = reducer(state, undo('0')); - expect(newState).toEqual({ ...state, deltalog: [] }); + expect(newState).toMatchObject({ + ...state, + deltalog: [], + transients: { + error: { + type: 'action/action_invalid', + }, + }, + }); }); }); @@ -595,7 +619,15 @@ describe('redo stack', () => { expect(state._redo).toHaveLength(1); const newState = reducer(state, redo('0')); expect(state._redo).toHaveLength(1); - expect(newState).toEqual({ ...state, deltalog: [] }); + expect(newState).toMatchObject({ + ...state, + deltalog: [], + transients: { + error: { + type: 'action/action_invalid', + }, + }, + }); }); }); diff --git a/src/core/reducer.ts b/src/core/reducer.ts index 9a2960b46..beba12b13 100644 --- a/src/core/reducer.ts +++ b/src/core/reducer.ts @@ -391,10 +391,8 @@ export function CreateGameReducer({ const { _undo, _redo } = state; if (_undo.length < 2) { - // TODO(#723): Add an error case here. - // error(`No moves to undo`); - // return WithError(state, ActionErrorType.ActionDisabled); - return state; + error(`No moves to undo`); + return WithError(state, ActionErrorType.ActionInvalid); } const last = _undo[_undo.length - 1]; @@ -405,10 +403,8 @@ export function CreateGameReducer({ actionHasPlayerID(action) && action.payload.playerID !== last.playerID ) { - // TODO(#723): Add an error case here. - // error(`Cannot undo other players' moves`); - // return WithError(state, ActionErrorType.ActionDisabled); - return state; + error(`Cannot undo other players' moves`); + return WithError(state, ActionErrorType.ActionInvalid); } // If undoing a move, check it is undoable. @@ -419,7 +415,8 @@ export function CreateGameReducer({ last.playerID ); if (!CanUndoMove(state.G, state.ctx, lastMove)) { - return state; + error(`Move cannot be undone`); + return WithError(state, ActionErrorType.ActionInvalid); } } @@ -447,10 +444,8 @@ export function CreateGameReducer({ const { _undo, _redo } = state; if (_redo.length == 0) { - // TODO(#723): Add an error case here. - // error(`No moves to redo`); - // return WithError(state, ActionErrorType.ActionDisabled); - return state; + error(`No moves to redo`); + return WithError(state, ActionErrorType.ActionInvalid); } const first = _redo[0]; @@ -460,10 +455,8 @@ export function CreateGameReducer({ actionHasPlayerID(action) && action.payload.playerID !== first.playerID ) { - // TODO(#723): Add an error case here. - // error(`Cannot redo other players` moves`); - // return WithError(state, ActionErrorType.ActionDisabled); - return state; + error(`Cannot redo other players' moves`); + return WithError(state, ActionErrorType.ActionInvalid); } state = initializeDeltalog(state, action); diff --git a/src/master/master.test.ts b/src/master/master.test.ts index a0e18b21b..fb2021bdf 100644 --- a/src/master/master.test.ts +++ b/src/master/master.test.ts @@ -177,15 +177,19 @@ describe('update', () => { const sendAll = jest.fn((arg) => { sendAllReturn = arg; }); - const db = new InMemory(); - const master = new Master(game, db, TransportAPI(send, sendAll)); + const game = { + moves: { + A: (G) => G, + }, + }; + let db; + let master; const action = ActionCreators.gameEvent('endTurn'); - beforeAll(async () => { + beforeEach(async () => { + db = new InMemory(); + master = new Master(game, db, TransportAPI(send, sendAll)); await master.onSync('matchID', '0', undefined, 2); - }); - - beforeEach(() => { sendAllReturn = undefined; jest.clearAllMocks(); }); @@ -230,7 +234,7 @@ describe('update', () => { }); test('invalid matchID', async () => { - await master.onUpdate(action, 1, 'default:unknown', '1'); + await master.onUpdate(action, 0, 'default:unknown', '1'); expect(sendAll).not.toHaveBeenCalled(); expect(error).toHaveBeenCalledWith( `game not found, matchID=[default:unknown]` @@ -238,15 +242,15 @@ describe('update', () => { }); test('invalid stateID', async () => { - await master.onUpdate(action, 100, 'matchID', '1'); + await master.onUpdate(action, 100, 'matchID', '0'); expect(sendAll).not.toHaveBeenCalled(); expect(error).toHaveBeenCalledWith( - `invalid stateID, was=[100], expected=[1] - playerID=[1] - action[endTurn]` + `invalid stateID, was=[100], expected=[0] - playerID=[0] - action[endTurn]` ); }); test('invalid playerID', async () => { - await master.onUpdate(action, 1, 'matchID', '100'); + await master.onUpdate(action, 0, 'matchID', '100'); await master.onUpdate(ActionCreators.makeMove('move'), 1, 'matchID', '100'); expect(sendAll).not.toHaveBeenCalled(); expect(error).toHaveBeenCalledWith( @@ -255,15 +259,15 @@ describe('update', () => { }); test('invalid move', async () => { - await master.onUpdate(ActionCreators.makeMove('move'), 1, 'matchID', '1'); + await master.onUpdate(ActionCreators.makeMove('move'), 0, 'matchID', '0'); expect(sendAll).not.toHaveBeenCalled(); expect(error).toHaveBeenCalledWith( - `move not processed - canPlayerMakeMove=false - playerID=[1] - action[move]` + `move not processed - canPlayerMakeMove=false - playerID=[0] - action[move]` ); }); test('valid matchID / stateID / playerID', async () => { - await master.onUpdate(action, 1, 'matchID', '1'); + await master.onUpdate(action, 0, 'matchID', '0'); expect(sendAll).toHaveBeenCalled(); }); @@ -361,8 +365,15 @@ describe('update', () => { describe('undo / redo', () => { test('player 0 can undo', async () => { + const move = ActionCreators.makeMove('A', null, '0'); + await master.onUpdate(move, 0, 'matchID', '0'); + expect(error).not.toHaveBeenCalled(); + await master.onUpdate(ActionCreators.undo(), 1, 'matchID', '0'); + expect(error).not.toHaveBeenCalled(); + + // Negative case: All moves already undone. await master.onUpdate(ActionCreators.undo(), 2, 'matchID', '0'); - expect(error).not.toBeCalled(); + expect(error).toHaveBeenCalledWith(`No moves to undo`); }); test('player 1 can’t undo', async () => { @@ -378,30 +389,34 @@ describe('update', () => { [{ all: 'A' }], '0' ); - await master.onUpdate(setActivePlayers, 2, 'matchID', '0'); - await master.onUpdate(ActionCreators.undo('0'), 3, 'matchID', '0'); + await master.onUpdate(setActivePlayers, 0, 'matchID', '0'); + await master.onUpdate(ActionCreators.undo('0'), 1, 'matchID', '0'); expect(error).toHaveBeenCalledWith( `playerID=[0] cannot undo / redo right now` ); }); test('player can undo if they are the only active player', async () => { + const move = ActionCreators.makeMove('A', null, '0'); + await master.onUpdate(move, 0, 'matchID', '0'); + expect(error).not.toHaveBeenCalled(); const endStage = ActionCreators.gameEvent('endStage', undefined, '0'); - await master.onUpdate(endStage, 3, 'matchID', '0'); - await master.onUpdate(ActionCreators.undo('1'), 4, 'matchID', '1'); + await master.onUpdate(endStage, 1, 'matchID', '0'); + expect(error).not.toBeCalled(); + await master.onUpdate(ActionCreators.undo(), 2, 'matchID', '0'); expect(error).not.toBeCalled(); // Clean-up active players. const endStage2 = ActionCreators.gameEvent('endStage', undefined, '1'); - await master.onUpdate(endStage2, 4, 'matchID', '1'); + await master.onUpdate(endStage2, 3, 'matchID', '1'); }); }); test('game over', async () => { let event = ActionCreators.gameEvent('endGame'); - await master.onUpdate(event, 5, 'matchID', '0'); + await master.onUpdate(event, 0, 'matchID', '0'); event = ActionCreators.gameEvent('endTurn'); - await master.onUpdate(event, 6, 'matchID', '0'); + await master.onUpdate(event, 1, 'matchID', '0'); expect(error).toHaveBeenCalledWith( `game over - matchID=[matchID] - playerID=[0] - action[endTurn]` ); @@ -664,7 +679,8 @@ describe('patch', () => { describe('undo / redo', () => { test('player 0 can undo', async () => { await master.onUpdate(ActionCreators.undo(), 2, 'matchID', '1'); - expect(error).not.toBeCalled(); + // The master allows this, but the reducer does not. + expect(error).toHaveBeenCalledWith(`No moves to undo`); }); test('player 1 can’t undo', async () => { @@ -691,7 +707,8 @@ describe('patch', () => { const endStage = ActionCreators.gameEvent('endStage', undefined, '1'); await master.onUpdate(endStage, 2, 'matchID', '1'); await master.onUpdate(ActionCreators.undo('0'), 3, 'matchID', '1'); - expect(error).not.toBeCalled(); + // The master allows this, but the reducer does not. + expect(error).toHaveBeenCalledWith(`Cannot undo other players' moves`); // Clean-up active players. const endStage2 = ActionCreators.gameEvent('endStage', undefined, '1');