Skip to content

Commit

Permalink
Update error types for action_invalid.
Browse files Browse the repository at this point in the history
  • Loading branch information
@shaoster
shaoster committed May 23, 2021
1 parent 0a6c7a5 commit be3bf3f
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 44 deletions.
2 changes: 2 additions & 0 deletions src/core/errors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
}
40 changes: 36 additions & 4 deletions src/core/reducer.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
},
});
});
});

Expand Down Expand Up @@ -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',
},
},
});
});
});

Expand Down Expand Up @@ -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',
},
},
});
});
});

Expand Down
27 changes: 10 additions & 17 deletions src/core/reducer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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.
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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];
Expand All @@ -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);
Expand Down
63 changes: 40 additions & 23 deletions src/master/master.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand Down Expand Up @@ -230,23 +234,23 @@ 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]`
);
});

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(
Expand All @@ -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();
});

Expand Down Expand Up @@ -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 () => {
Expand All @@ -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]`
);
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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');
Expand Down

0 comments on commit be3bf3f

Please sign in to comment.