Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 19 additions & 13 deletions package-lock.json

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
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
"babel-plugin-external-helpers": "^6.22.0",
"babel-plugin-module-resolver": "^3.0.0",
"babel-plugin-transform-es2015-modules-commonjs": "^6.26.0",
"babel-polyfill": "^6.26.0",
Copy link
Copy Markdown
Member

@nicolodavis nicolodavis Feb 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to do this without babel-polyfill? Recent versions of Node support async / await out of the box.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After running examples I got regeneratorRuntime error.
I tried to solve it by testing all options from babel docs and finally this one worked.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

babel-polyfill shouldn't be used in a library, so we should find another way to do this. Have you tried babel-preset-env?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other option would be to not use Babel on anything in src/server and write it without ES5 features. I can take a look at this later.

"babel-preset-es2015": "^6.24.1",
"babel-preset-react": "^6.24.1",
"babel-preset-stage-0": "^6.24.1",
Expand Down
20 changes: 11 additions & 9 deletions src/server/db.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// solve runtime issue with async fumctions
import 'babel-polyfill';

/*
* Copyright 2017 The boardgame.io Authors
*
Expand All @@ -20,28 +23,27 @@ export class InMemory {
/**
* Write the game state to the in-memory object.
* @param {string} id - The game id.
* @param {object} store - A Redux store to persist.
* @param {object} store - A game state to persist.
*/
set(id, store) {
this.games.set(id, store);
async set(id, state) {
return await this.games.set(id, state);
}

/**
* Read the game state from the in-memory object.
* @param {string} id - The game id.
* @returns {object} - A Redux store with the game state, or undefined
* @returns {object} - A game state, or undefined
* if no game is found with this id.
*/
get(id) {
return this.games.get(id);
async get(id) {
return await this.games.get(id);
}

/**
* Read the game state from the in-memory object.
* @param {string} id - The game id.
* @returns {boolean} - True if a game with this id exists.
*/
has(id) {
return this.games.has(id);
async has(id) {
return await this.games.has(id);
}
}
14 changes: 9 additions & 5 deletions src/server/db.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,22 @@
import { InMemory } from './db';
import * as Redux from 'redux';

test('basic', () => {
test('basic', async () => {
const db = new InMemory();
const reducer = () => {};
const store = Redux.createStore(reducer);

// Must return undefined when no game exists.
expect(db.get('gameID')).toEqual(undefined);
let state = await db.get('gameID');
expect(state).toEqual(undefined);

// Create game.
db.set('gameID', store);
await db.set('gameID', store.getState());
// Must return created game.
expect(db.get('gameID')).toEqual(store);
state = await db.get('gameID');
expect(state).toEqual(store.getState());

// Must return true if game exists
expect(db.has('gameID')).toEqual(true);
let has = await db.has('gameID');
expect(has).toEqual(true);
});
33 changes: 19 additions & 14 deletions src/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,18 @@ function Server({ games, db }) {
const nsp = app._io.of(game.name);

nsp.on('connection', socket => {
socket.on('action', (action, stateID, gameID, playerID) => {
const store = db.get(gameID);
socket.on('action', async (action, stateID, gameID, playerID) => {
let state = await db.get(gameID);

if (store === undefined) {
if (state === undefined) {
return { error: 'game not found' };
}

const state = store.getState();
const reducer = createGameReducer({
game,
numPlayers: state.ctx.numPlayers,
});
const store = Redux.createStore(reducer, state);

// The null player is a view-only player.
if (playerID == null) {
Expand All @@ -55,7 +59,7 @@ function Server({ games, db }) {
if (state._id == stateID) {
// Update server's version of the store.
store.dispatch(action);
const state = store.getState();
state = store.getState();

// Get clients connected to this current game.
const roomClients = roomInfo.get(gameID);
Expand All @@ -75,13 +79,13 @@ function Server({ games, db }) {
}
}

db.set(gameID, store);
db.set(gameID, store.getState());
}
});

socket.on('sync', (gameID, playerID, numPlayers) => {
socket.on('sync', async (gameID, playerID, numPlayers) => {
socket.join(gameID);

const reducer = createGameReducer({ game, numPlayers });
let roomClients = roomInfo.get(gameID);
if (roomClients === undefined) {
roomClients = new Set();
Expand All @@ -91,18 +95,19 @@ function Server({ games, db }) {

clientInfo.set(socket.id, { gameID, playerID });

let store = db.get(gameID);
if (store === undefined) {
const reducer = createGameReducer({ game, numPlayers });
store = Redux.createStore(reducer);
db.set(gameID, store);
let state = await db.get(gameID);
if (state === undefined) {
const store = Redux.createStore(reducer);
state = store.getState();
await db.set(gameID, state);
}

const state = store.getState();
socket.emit('sync', gameID, {
...state,
G: game.playerView(state.G, state.ctx, playerID),
});

return;
});

socket.on('disconnect', () => {
Expand Down
52 changes: 31 additions & 21 deletions src/server/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ jest.mock('koa-socket', () => {
this.broadcast = { emit: jest.fn() };
}

receive(type, ...args) {
this.callbacks[type](args[0], args[1], args[2], args[3], args[4]);
async receive(type, ...args) {
await this.callbacks[type](args[0], args[1], args[2], args[3], args[4]);
return;
}

on(type, callback) {
Expand Down Expand Up @@ -65,7 +66,7 @@ test('basic', () => {
io.socket.receive('disconnect');
});

test('sync', () => {
test('sync', async () => {
const server = Server({ games: [game] });
const io = server.context.io;
expect(server).not.toBe(undefined);
Expand All @@ -74,40 +75,42 @@ test('sync', () => {

// Sync causes the server to respond.
expect(io.socket.emit).toHaveBeenCalledTimes(0);
io.socket.receive('sync', 'gameID');
await io.socket.receive('sync', 'gameID');

expect(io.socket.emit).toHaveBeenCalledTimes(1);
Copy link
Copy Markdown
Contributor Author

@saeidalidadi saeidalidadi Jan 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @nicolodavis sorry for my delayed reply
Wrapping this assertion inside a timeout works fine but I think there should be a better solution for async behavior of socket callbacks

Copy link
Copy Markdown
Member

@nicolodavis nicolodavis Feb 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use await in the tests. The trick is to make the function explicitly have a return statement. Then you can use await to wait for its execution and then test what it did. I wrapped receive in this way to make the tests to work.

expect(spy).toHaveBeenCalled();

// Sync a second time does not create a game.
spy.mockReset();
io.socket.receive('sync', 'gameID');
await io.socket.receive('sync', 'gameID');

expect(io.socket.emit).toHaveBeenCalledTimes(2);
expect(spy).not.toHaveBeenCalled();

spy.mockRestore();
});

test('action', () => {
test('action', async () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

async not needed, because not await exist

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, but I forgot to remove it after I clean await s when it didn't work

const server = Server({ games: [game] });
const io = server.context.io;
const action = ActionCreators.gameEvent('endTurn');

io.socket.receive('action', action);
await io.socket.receive('action', action);
expect(io.socket.emit).toHaveBeenCalledTimes(0);
io.socket.emit.mockReset();

io.socket.receive('sync', 'gameID');
await io.socket.receive('sync', 'gameID');
io.socket.id = 'second';
io.socket.receive('sync', 'gameID');
await io.socket.receive('sync', 'gameID');
io.socket.emit.mockReset();

// View-only players cannot send actions.
io.socket.receive('action', action, 0, 'gameID', null);
await io.socket.receive('action', action, 0, 'gameID', null);
expect(io.socket.emit).not.toHaveBeenCalled();

// Actions are broadcasted as state updates.
// The playerID parameter is necessary to account for view-only players.
io.socket.receive('action', action, 0, 'gameID', '0');
await io.socket.receive('action', action, 0, 'gameID', '0');
expect(io.socket.emit).lastCalledWith('sync', 'gameID', {
G: {},
_id: 1,
Expand Down Expand Up @@ -141,23 +144,23 @@ test('action', () => {
io.socket.emit.mockReset();

// ... but not if the gameID is not known.
io.socket.receive('action', action, 1, 'unknown', '1');
await io.socket.receive('action', action, 1, 'unknown', '1');
expect(io.socket.emit).toHaveBeenCalledTimes(0);

// ... and not if the _id doesn't match the internal state.
io.socket.receive('action', action, 100, 'gameID', '1');
await io.socket.receive('action', action, 100, 'gameID', '1');
expect(io.socket.emit).toHaveBeenCalledTimes(0);

// ... and not if player != currentPlayer
io.socket.receive('action', action, 1, 'gameID', '100');
await io.socket.receive('action', action, 1, 'gameID', '100');
expect(io.socket.emit).toHaveBeenCalledTimes(0);

// Another broadcasted action.
io.socket.receive('action', action, 1, 'gameID', '1');
await io.socket.receive('action', action, 1, 'gameID', '1');
expect(io.socket.emit).toHaveBeenCalledTimes(2);
});

test('playerView', () => {
test('playerView', async () => {
// Write the player into G.
const game = Game({
playerView: (G, ctx, player) => {
Expand All @@ -168,7 +171,7 @@ test('playerView', () => {
const server = Server({ games: [game] });
const io = server.context.io;

io.socket.receive('sync', 'gameID', 0);
await io.socket.receive('sync', 'gameID', 0);
expect(io.socket.emit).lastCalledWith('sync', 'gameID', {
G: { player: 0 },
_id: 0,
Expand Down Expand Up @@ -196,19 +199,26 @@ test('playerView', () => {
});
});

test('custom db implementation', () => {
test('custom db implementation', async () => {
let getId = null;

class Custom {
get(id) {
constructor() {
this.games = new Map();
}
async get(id) {
getId = id;
return await this.games.get(id);
}
async set(id, state) {
return await this.games.set(id, state);
}
set() {}
}

const game = Game({});
const server = Server({ games: [game], db: new Custom() });
const io = server.context.io;

io.socket.receive('sync', 'gameID');
await io.socket.receive('sync', 'gameID');
expect(getId).toBe('gameID');
});