Skip to content

Commit

Permalink
feat(simultaneous-moves): add option to long form move to ignore stal…
Browse files Browse the repository at this point in the history 
…e stateID (close #828) (#832)

* fix github install

* Update package.json

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

* remove unused script

* Revert "Update package.json"

This reverts commit 62e98e7.

* feat(stale-state-move): add option to long form move to ignore stale state.
Add ignoreStateState option to LongFormMove.
Export IsLongFormMove function from game.ts.
On master.ts, get the move and check if it does not have the ignoreStateState option thuthy before triggering invalid stateID error.

* Rename ignoreStaleState option  to ignoreStaleStateID.

* Undo merge with nicolodavis/fix-github-install.

* Refactoring usage of getMove on master.ts.
Improved unit test for simultaneous moves.

* Implemented class InMemoryAsync to test simultaneous moves.

* Using PQueue library to create a per-match queue for actions sent to SocketIO onUpdate calls.

* Update src/server/transport/socketio.ts

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

* Update src/server/transport/socketio.ts

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

* Implemented all base methods on InMemoryAsync class.

* Refactor createMatchQueue method to getMatchQueue.

* Fix to install boardgame.io from GitHub.

* Export InMemory in server/db.

* Fix comment text.

* Improve error logging messages on Master.

* Add 300 miliseconds interval to PQueue for sync storages.

* Working on simultaneous moves tests with SocketIO server and client.

* Skipping failing test just to push he code.

* Skipping failing test just to push he code.

* Undoing changes on server and socketio files.

* Working on simultaneous moves tests using mocks.

* Fix unit test for simultaneous moves.

* Adjust error messages on Master.

* Undo changes to fix GitHub install.

* Separating simultaneous moves tests to use sync and async storages.
Reducing PQueue interval to 50ms. It needs to be tested in real world,

* Add logging messages for InMemory setState and setMetatada methods.

* Cleaning code removing PQueue interval, which wasn't needed.
Fixing unit tests.

* Removing unnecessary mock for IsSynchronous.

* Fixing unit tests and doing adjustments from code review.

* test(socketio): Remove unused import

* test: Randomise InMemoryAsync interval

* style(master): Revert style change

* docs(Game): Document `ignoreStaleStateID` move option

Co-authored-by: Nicolo Davis <nicolodavis@gmail.com>
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
Co-authored-by: Evandro Abu Kamel <evandro.costa@maxmilhas.com.br>
  • Loading branch information
@evandroabukamel @nicolodavis @delucis
4 people authored Nov 7, 2020
1 parent aca811b commit 6c4e94f
Show file tree
Hide file tree
Showing 10 changed files with 721 additions and 37 deletions.
22 changes: 15 additions & 7 deletions docs/documentation/api/Game.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,23 @@

moves: {
// short-form move.
A: (G, ctx) => {},
A: (G, ctx, ...args) => {},

// long-form move.
B: {
move: (G, ctx) => {},
undoable: false, // prevents undoing the move.
redact: true, // prevents the move arguments from showing up in the log.
client: false, // prevents the move from running on the client.
noLimit: true, // prevents the move counting towards a player’s number of moves.
// The move function.
move: (G, ctx, ...args) => {},
// Prevents undoing the move.
undoable: false,
// Prevents the move arguments from showing up in the log.
redact: true,
// Prevents the move from running on the client.
client: false,
// Prevents the move counting towards a player’s number of moves.
noLimit: true,
// Processes the move even if it was dispatched from an out-of-date client.
// This can be risky; check the validity of the state update in your move.
ignoreStaleStateID: true,
},
},

Expand Down Expand Up @@ -108,7 +116,7 @@
// Called at the end of the game.
// `ctx.gameover` is available at this point.
onEnd: (G, ctx) => G,

// Disable undo feature for all the moves in the game
disableUndo: true,
}
Expand Down
27 changes: 25 additions & 2 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@
"koa-router": "^7.2.1",
"koa-socket-2": "^1.0.17",
"lru-cache": "^4.1.1",
"p-queue": "^6.6.2",
"prop-types": "^15.5.10",
"react-cookies": "^0.1.0",
"redux": "^4.0.0",
Expand Down
2 changes: 1 addition & 1 deletion src/client/transport/socketio.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type SocketIOTransportOpts = TransportOpts &
*/
export class SocketIOTransport extends Transport {
server: string;
socket;
socket: SocketIOClient.Socket;
socketOpts;
callback: () => void;
matchDataCallback: MetadataCallback;
Expand Down
2 changes: 1 addition & 1 deletion src/core/game.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,6 @@ export function ProcessGameConfig(game: Game | ProcessedGame): ProcessedGame {
};
}

function IsLongFormMove(move: Move): move is LongFormMove {
export function IsLongFormMove(move: Move): move is LongFormMove {
return move instanceof Object && (move as LongFormMove).move !== undefined;
}
175 changes: 160 additions & 15 deletions src/master/master.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ import {
isActionFromAuthenticPlayer,
} from './master';
import { error } from '../core/logger';
import { Server, State } from '../types';
import { Server, State, Ctx, LogEntry } from '../types';
import * as StorageAPI from '../server/db/base';
import * as dateMock from 'jest-date-mock';
import { PlayerView } from '../core/player-view';

jest.mock('../core/logger', () => ({
info: jest.fn(),
Expand All @@ -30,9 +31,54 @@ beforeEach(() => {
dateMock.clear();
});

class InMemoryAsync extends InMemory {
type() {
return StorageAPI.Type.ASYNC;
class InMemoryAsync extends StorageAPI.Async {
db: InMemory;

constructor() {
super();
this.db = new InMemory();
}

async connect() {
await this.sleep();
}

private sleep(): Promise<void> {
const interval = Math.round(Math.random() * 50 + 50);
return new Promise(resolve => void setTimeout(resolve, interval));
}

async createMatch(id: string, opts: StorageAPI.CreateMatchOpts) {
await this.sleep();
this.db.createMatch(id, opts);
}

async setMetadata(matchID: string, metadata: Server.MatchData) {
await this.sleep();
this.db.setMetadata(matchID, metadata);
}

async setState(matchID: string, state: State, deltalog?: LogEntry[]) {
await this.sleep();
this.db.setState(matchID, state, deltalog);
}

async fetch<O extends StorageAPI.FetchOpts>(
matchID: string,
opts: O
): Promise<StorageAPI.FetchResult<O>> {
await this.sleep();
return this.db.fetch(matchID, opts);
}

async wipe(matchID: string) {
await this.sleep();
this.db.wipe(matchID);
}

async listMatches(opts?: StorageAPI.ListMatchesOpts): Promise<string[]> {
await this.sleep();
return this.db.listMatches(opts);
}
}

Expand Down Expand Up @@ -178,22 +224,24 @@ describe('update', () => {
await master.onUpdate(action, 100, 'matchID', '1');
expect(sendAll).not.toHaveBeenCalled();
expect(error).toHaveBeenCalledWith(
`invalid stateID, was=[100], expected=[1]`
`invalid stateID, was=[100], expected=[1] - playerID=[1] - action[endTurn]`
);
});

test('invalid playerID', async () => {
await master.onUpdate(action, 1, 'matchID', '100');
await master.onUpdate(ActionCreators.makeMove('move'), 1, 'matchID', '100');
expect(sendAll).not.toHaveBeenCalled();
expect(error).toHaveBeenCalledWith(`player not active - playerID=[100]`);
expect(error).toHaveBeenCalledWith(
`player not active - playerID=[100] - action[move]`
);
});

test('invalid move', async () => {
await master.onUpdate(ActionCreators.makeMove('move'), 1, 'matchID', '1');
expect(sendAll).not.toHaveBeenCalled();
expect(error).toHaveBeenCalledWith(
`move not processed - canPlayerMakeMove=false, playerID=[1]`
`move not processed - canPlayerMakeMove=false - playerID=[1] - action[move]`
);
});

Expand All @@ -202,6 +250,98 @@ describe('update', () => {
expect(sendAll).toHaveBeenCalled();
});

test('allow execution of moves with ignoreStaleStateID truthy', async () => {
const game = {
setup: () => {
const G = {
players: {
'0': {
cards: ['card3'],
},
'1': {
cards: [],
},
},
cards: ['card0', 'card1', 'card2'],
discardedCards: [],
};
return G;
},
playerView: PlayerView.STRIP_SECRETS,
turn: {
activePlayers: { currentPlayer: { stage: 'A' } },
stages: {
A: {
moves: {
A: (G, ctx: Ctx) => {
const card = G.players[ctx.playerID].cards.shift();
G.discardedCards.push(card);
},
B: {
move: (G, ctx: Ctx) => {
const card = G.cards.pop();
G.players[ctx.playerID].cards.push(card);
},
ignoreStaleStateID: true,
},
},
},
},
},
};

const send = jest.fn();
const master = new Master(
game,
new InMemory(),
TransportAPI(send, sendAll)
);

const setActivePlayers = ActionCreators.gameEvent(
'setActivePlayers',
[{ all: 'A' }],
'0'
);
const actionA = ActionCreators.makeMove('A', null, '0');
const actionB = ActionCreators.makeMove('B', null, '1');
const actionC = ActionCreators.makeMove('B', null, '0');

// test: simultaneous moves
await master.onSync('matchID', '0', 2);
await master.onUpdate(actionA, 0, 'matchID', '0');
await master.onUpdate(setActivePlayers, 1, 'matchID', '0');
await Promise.all([
master.onUpdate(actionB, 2, 'matchID', '1'),
master.onUpdate(actionC, 2, 'matchID', '0'),
]);
await Promise.all([
master.onSync('matchID', '0', 2),
master.onSync('matchID', '1', 2),
]);

const G_player0 = sendAllReturn('0').args[1].G;
const G_player1 = sendAllReturn('1').args[1].G;

expect(G_player0).toMatchObject({
players: {
'0': {
cards: ['card1'],
},
},
cards: ['card0'],
discardedCards: ['card3'],
});
expect(G_player1).toMatchObject({
players: {
'1': {
cards: ['card2'],
},
},
cards: ['card0'],
discardedCards: ['card3'],
});
});

describe('undo / redo', () => {
test('player 0 can undo', async () => {
await master.onUpdate(ActionCreators.undo(), 2, 'matchID', '0');
Expand Down Expand Up @@ -245,7 +385,9 @@ describe('update', () => {
await master.onUpdate(event, 5, 'matchID', '0');
event = ActionCreators.gameEvent('endTurn');
await master.onUpdate(event, 6, 'matchID', '0');
expect(error).toHaveBeenCalledWith(`game over - matchID=[matchID]`);
expect(error).toHaveBeenCalledWith(
`game over - matchID=[matchID] - playerID=[0] - action[endTurn]`
);
});

test('writes gameover to metadata', async () => {
Expand Down Expand Up @@ -279,14 +421,14 @@ describe('update', () => {
createdAt: 0,
updatedAt: 0,
};
db.setMetadata(id, dbMetadata);
await db.setMetadata(id, dbMetadata);
const masterWithMetadata = new Master(game, db, TransportAPI(send));
await masterWithMetadata.onSync(id, '0', 2);

const gameOverArg = 'gameOverArg';
const event = ActionCreators.gameEvent('endGame', gameOverArg);
await masterWithMetadata.onUpdate(event, 0, id, '0');
const { metadata } = db.fetch(id, { metadata: true });
const { metadata } = await db.fetch(id, { metadata: true });
expect(metadata.gameover).toEqual(gameOverArg);
});

Expand All @@ -300,15 +442,15 @@ describe('update', () => {
createdAt: 0,
updatedAt: 0,
};
db.setMetadata(id, dbMetadata);
await db.setMetadata(id, dbMetadata);
const masterWithMetadata = new Master(game, db, TransportAPI(send));
await masterWithMetadata.onSync(id, '0', 2);

const updatedAt = new Date(2020, 3, 4, 5, 6, 7);
dateMock.advanceTo(updatedAt);
const event = ActionCreators.gameEvent('endTurn', null, '0');
await masterWithMetadata.onUpdate(event, 0, id, '0');
const { metadata } = db.fetch(id, { metadata: true });
const { metadata } = await db.fetch(id, { metadata: true });
expect(metadata.updatedAt).toEqual(updatedAt.getTime());
});

Expand Down Expand Up @@ -337,13 +479,13 @@ describe('update', () => {
// Store state manually to bypass automatic metadata initialization on sync.
let state = InitializeGame({ game });
expect(state.ctx.turn).toBe(1);
db.setState(id, state);
await db.setState(id, state);
// Dispatch update to end the turn.
const event = ActionCreators.gameEvent('endTurn', null, '0');
await masterWithoutMetadata.onUpdate(event, 0, id, '0');
// Confirm the turn ended.
let metadata: undefined | Server.MatchData;
({ state, metadata } = db.fetch(id, { state: true, metadata: true }));
({ state, metadata } = await db.fetch(id, { state: true, metadata: true }));
expect(state.ctx.turn).toBe(2);
expect(metadata).toBeUndefined();
});
Expand Down Expand Up @@ -430,7 +572,10 @@ describe('connectionChange', () => {
asyncDb,
TransportAPI(send, sendAll)
);
asyncDb.createMatch('matchID', { metadata, initialState: {} as State });
await asyncDb.createMatch('matchID', {
metadata,
initialState: {} as State,
});

await masterWithAsyncDb.onConnectionChange('matchID', '0', true);

Expand Down
Loading

0 comments on commit 6c4e94f

Please sign in to comment.