Skip to content

Commit abc516b

Browse files
larry801delucis
andauthored
feat: Add an option to use JSON patches for state updates (#920)
* Add patch transmit * fix stale state id * Add test and comment * add reducer test for coverage * fix reducer.test.ts * fix warning and coverage * add document * Fix deltalog handling on client * style: clean up Co-authored-by: Chris Swithinbank <[email protected]>
1 parent e94d476 commit abc516b

17 files changed

Lines changed: 461 additions & 39 deletions

File tree

docs/documentation/api/Game.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,5 +119,8 @@
119119

120120
// Disable undo feature for all the moves in the game
121121
disableUndo: true,
122+
123+
// Transfer delta state with JSON Patch in multiplayer
124+
deltaState: true,
122125
}
123126
```

package-lock.json

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@
162162
"prop-types": "^15.5.10",
163163
"react-cookies": "^0.1.0",
164164
"redux": "^4.0.0",
165+
"rfc6902": "^4.0.1",
165166
"socket.io": "^2.4.1",
166167
"socket.io-client": "^2.3.0",
167168
"svelte": "^3.24.0",

src/client/client.test.ts

Lines changed: 58 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,17 @@ import { ProcessGameConfig } from '../core/game';
1414
import type { Transport } from './transport/transport';
1515
import { LocalTransport, Local } from './transport/local';
1616
import { SocketIOTransport, SocketIO } from './transport/socketio';
17-
import { update, sync, makeMove, gameEvent } from '../core/action-creators';
17+
import {
18+
update,
19+
sync,
20+
makeMove,
21+
gameEvent,
22+
patch,
23+
} from '../core/action-creators';
1824
import Debug from './debug/Debug.svelte';
1925
import { error } from '../core/logger';
2026
import type { LogEntry, State, SyncInfo } from '../types';
27+
import type { Operation } from 'rfc6902';
2128

2229
jest.mock('../core/logger', () => ({
2330
info: jest.fn(),
@@ -370,19 +377,20 @@ test('accepts enhancer for store', () => {
370377
});
371378

372379
describe('event dispatchers', () => {
380+
const clientEvents = [
381+
'endTurn',
382+
'pass',
383+
'endPhase',
384+
'setPhase',
385+
'endGame',
386+
'setActivePlayers',
387+
'endStage',
388+
'setStage',
389+
];
373390
test('default', () => {
374391
const game = {};
375392
const client = Client({ game });
376-
expect(Object.keys(client.events)).toEqual([
377-
'endTurn',
378-
'pass',
379-
'endPhase',
380-
'setPhase',
381-
'endGame',
382-
'setActivePlayers',
383-
'endStage',
384-
'setStage',
385-
]);
393+
expect(Object.keys(client.events)).toEqual(clientEvents);
386394
expect(client.getState().ctx.turn).toBe(1);
387395
client.events.endTurn();
388396
expect(client.getState().ctx.turn).toBe(2);
@@ -396,16 +404,7 @@ describe('event dispatchers', () => {
396404
},
397405
};
398406
const client = Client({ game });
399-
expect(Object.keys(client.events)).toEqual([
400-
'endTurn',
401-
'pass',
402-
'endPhase',
403-
'setPhase',
404-
'endGame',
405-
'setActivePlayers',
406-
'endStage',
407-
'setStage',
408-
]);
407+
expect(Object.keys(client.events)).toEqual(clientEvents);
409408
expect(client.getState().ctx.turn).toBe(1);
410409
client.events.endTurn();
411410
expect(client.getState().ctx.turn).toBe(2);
@@ -464,7 +463,7 @@ describe('move dispatchers', () => {
464463
expect(store.getState().G).toMatchObject({ victory: true });
465464
});
466465

467-
test('with undefined playerID - singleplayer mode', () => {
466+
test('with undefined playerID - single player mode', () => {
468467
const store = createStore(reducer, initialState);
469468
const api = createMoveDispatchers(game.moveNames, store);
470469
api.B();
@@ -484,7 +483,7 @@ describe('move dispatchers', () => {
484483
expect(store.getState().G).toMatchObject({ moved: undefined });
485484
});
486485

487-
test('with null playerID - singleplayer mode', () => {
486+
test('with null playerID - single player mode', () => {
488487
const store = createStore(reducer, initialState);
489488
const api = createMoveDispatchers(game.moveNames, store, null);
490489
api.B();
@@ -552,6 +551,26 @@ describe('log handling', () => {
552551
expect(client.log).toEqual(deltalog);
553552
});
554553

554+
test('patch', () => {
555+
const patches = [
556+
{ op: 'replace', path: '/_stateID', value: 1 },
557+
] as Operation[];
558+
const deltalog = [
559+
{
560+
action: {},
561+
_stateID: 0,
562+
},
563+
{
564+
action: {},
565+
_stateID: 1,
566+
},
567+
] as LogEntry[];
568+
const action = patch(0, 1, patches, deltalog);
569+
client.store.dispatch(action);
570+
571+
expect(client.log).toEqual(deltalog);
572+
});
573+
555574
test('sync', () => {
556575
const state = { restore: true };
557576
const log = ['0', '1'];
@@ -756,6 +775,10 @@ describe('start / stop', () => {
756775
const client = Client({ game: {}, debug: { target: el } });
757776
client.start();
758777
client.stop();
778+
expect(error).toHaveBeenCalledTimes(3);
779+
expect(error).toHaveBeenCalledWith('disallowed move: B');
780+
expect(error).toHaveBeenCalledWith('disallowed move: C');
781+
expect(error).toHaveBeenLastCalledWith('disallowed move: A');
759782
});
760783

761784
test('no error when mounting on null element', () => {
@@ -769,17 +792,29 @@ describe('start / stop', () => {
769792
const client = Client({ game: {}, debug: { impl: Debug } });
770793
client.start();
771794
client.stop();
795+
expect(error).toHaveBeenCalledTimes(3);
796+
expect(error).toHaveBeenCalledWith('disallowed move: B');
797+
expect(error).toHaveBeenCalledWith('disallowed move: C');
798+
expect(error).toHaveBeenLastCalledWith('disallowed move: A');
772799
});
773800

774801
test('production mode', () => {
775802
process.env.NODE_ENV = 'production';
776803
const client = Client({ game: {} });
777804
client.start();
778805
client.stop();
806+
expect(error).toHaveBeenCalledTimes(3);
807+
expect(error).toHaveBeenCalledWith('disallowed move: B');
808+
expect(error).toHaveBeenCalledWith('disallowed move: C');
809+
expect(error).toHaveBeenLastCalledWith('disallowed move: A');
779810
});
780811

781812
test('try to stop without starting', () => {
782813
const client = Client({ game: {} });
783814
client.stop();
815+
expect(error).toHaveBeenCalledTimes(3);
816+
expect(error).toHaveBeenCalledWith('disallowed move: B');
817+
expect(error).toHaveBeenCalledWith('disallowed move: C');
818+
expect(error).toHaveBeenLastCalledWith('disallowed move: A');
784819
});
785820
});

src/client/client.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,11 @@ import type {
3535
ChatMessage,
3636
} from '../types';
3737

38-
type ClientAction = ActionShape.Reset | ActionShape.Sync | ActionShape.Update;
38+
type ClientAction =
39+
| ActionShape.Reset
40+
| ActionShape.Sync
41+
| ActionShape.Update
42+
| ActionShape.Patch;
3943
type Action = CredentialedActionShape.Any | ClientAction;
4044

4145
export interface DebugOpt {
@@ -241,7 +245,7 @@ export class _ClientImpl<G extends any = any> {
241245
this.log = [];
242246
break;
243247
}
244-
248+
case Actions.PATCH:
245249
case Actions.UPDATE: {
246250
let id = -1;
247251
if (this.log.length > 0) {

src/client/transport/socketio.test.ts

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,12 @@ import { InitializeGame } from '../../core/initialize';
1414
import * as Actions from '../../core/action-types';
1515
import type { Master } from '../../master/master';
1616
import type { ChatMessage, State, Store } from '../../types';
17+
import { error } from '../../core/logger';
18+
19+
jest.mock('../../core/logger', () => ({
20+
info: jest.fn(),
21+
error: jest.fn(),
22+
}));
1723

1824
type UpdateArgs = Parameters<Master['onUpdate']>;
1925
type SyncArgs = Parameters<Master['onSync']>;
@@ -27,8 +33,8 @@ class MockSocket {
2733
this.emit = jest.fn();
2834
}
2935

30-
receive(type: string, arg0?: any, arg1?: any) {
31-
this.callbacks[type](arg0, arg1);
36+
receive(type: string, ...args) {
37+
this.callbacks[type](...args);
3238
}
3339

3440
on(type: string, callback: (arg0?: any, arg1?: any) => void) {
@@ -219,6 +225,61 @@ describe('multiplayer', () => {
219225
});
220226
});
221227

228+
describe('multiplayer delta state', () => {
229+
const mockSocket = new MockSocket();
230+
const m = new TransportAdapter({ socket: mockSocket });
231+
m.connect();
232+
const game = { deltaState: true };
233+
let store = null;
234+
235+
beforeEach(() => {
236+
const reducer = CreateGameReducer({ game });
237+
const initialState = InitializeGame({ game });
238+
store = createStore(reducer, initialState);
239+
m.setStore(store);
240+
});
241+
242+
test('returns a valid store', () => {
243+
expect(store).not.toBe(undefined);
244+
});
245+
246+
test('receive patch', () => {
247+
const originalState = JSON.parse(JSON.stringify(store.getState()));
248+
mockSocket.receive(
249+
'patch',
250+
'unknown matchID',
251+
0,
252+
1,
253+
[{ op: 'replace', path: '/_stateID', value: 1 }],
254+
[]
255+
);
256+
expect(store.getState()).toMatchObject(originalState);
257+
mockSocket.receive(
258+
'patch',
259+
'default',
260+
0,
261+
1,
262+
[{ op: 'replace', path: '/_stateID', value: 1 }],
263+
[]
264+
);
265+
expect(store.getState()._stateID).toBe(1);
266+
mockSocket.receive(
267+
'patch',
268+
'default',
269+
1,
270+
2,
271+
[{ op: 'replace', path: '/_stateIDD', value: 3 }],
272+
[]
273+
);
274+
expect(store.getState()._stateID).toBe(1);
275+
const args: SyncArgs = ['default', null, undefined, 2];
276+
expect(mockSocket.emit).lastCalledWith('sync', ...args);
277+
expect(error).lastCalledWith(
278+
'Patch [{"op":"replace","path":"/_stateIDD","value":3}] apply failed'
279+
);
280+
});
281+
});
282+
222283
describe('server option', () => {
223284
const hostname = 'host';
224285
const port = '1234';

src/client/transport/socketio.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@
77
*/
88

99
import * as ioNamespace from 'socket.io-client';
10+
1011
const io = ioNamespace.default;
1112

1213
import * as ActionCreators from '../../core/action-creators';
1314
import type { Master } from '../../master/master';
1415
import { Transport } from './transport';
16+
import type { Operation } from 'rfc6902';
1517
import type {
1618
TransportOpts,
1719
MetadataCallback,
@@ -51,11 +53,13 @@ export class SocketIOTransport extends Transport {
5153
chatMessageCallback: ChatCallback;
5254

5355
/**
54-
* Creates a new Mutiplayer instance.
56+
* Creates a new Multiplayer instance.
5557
* @param {object} socket - Override for unit tests.
5658
* @param {object} socketOpts - Options to pass to socket.io.
59+
* @param {object} store - Redux store
5760
* @param {string} matchID - The game ID to connect to.
5861
* @param {string} playerID - The player ID associated with this client.
62+
* @param {string} credentials - Authentication credentials
5963
* @param {string} gameName - The game type (the `name` field in `Game`).
6064
* @param {string} numPlayers - The number of players.
6165
* @param {string} server - The game server in the form of 'hostname:port'. Defaults to the server serving the client if not provided.
@@ -120,6 +124,35 @@ export class SocketIOTransport extends Transport {
120124
}
121125
}
122126

127+
// Called when another player makes a move and the
128+
// master broadcasts the update as a patch to other clients (including
129+
// this one).
130+
this.socket.on(
131+
'patch',
132+
(
133+
matchID: string,
134+
prevStateID: number,
135+
stateID: number,
136+
patch: Operation[],
137+
deltalog: LogEntry[]
138+
) => {
139+
const currentStateID = this.store.getState()._stateID;
140+
if (matchID === this.matchID && prevStateID === currentStateID) {
141+
const action = ActionCreators.patch(
142+
prevStateID,
143+
stateID,
144+
patch,
145+
deltalog
146+
);
147+
this.store.dispatch(action);
148+
// emit sync if patch apply failed
149+
if (this.store.getState()._stateID === currentStateID) {
150+
this.sync();
151+
}
152+
}
153+
}
154+
);
155+
123156
// Called when another player makes a move and the
124157
// master broadcasts the update to other clients (including
125158
// this one).

src/core/action-creators.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import * as Actions from './action-types';
1010
import type { SyncInfo, State, LogEntry } from '../types';
11+
import type { Operation } from 'rfc6902';
1112

1213
/**
1314
* Generate a move to be dispatched to the game move reducer.
@@ -71,6 +72,28 @@ export const sync = (info: SyncInfo) => ({
7172
clientOnly: true as const,
7273
});
7374

75+
/**
76+
* Used to update the Redux store's state with patch in response to
77+
* an action coming from another player.
78+
* @param prevStateID previous stateID
79+
* @param stateID stateID after this patch
80+
* @param {Operation[]} patch - The patch to apply.
81+
* @param {LogEntry[]} deltalog - A log delta.
82+
*/
83+
export const patch = (
84+
prevStateID: number,
85+
stateID: number,
86+
patch: Operation[],
87+
deltalog: LogEntry[]
88+
) => ({
89+
type: Actions.PATCH as typeof Actions.PATCH,
90+
prevStateID,
91+
stateID,
92+
patch,
93+
deltalog,
94+
clientOnly: true as const,
95+
});
96+
7497
/**
7598
* Used to update the Redux store's state in response to
7699
* an action coming from another player.

src/core/action-types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,5 @@ export const RESET = 'RESET';
1313
export const SYNC = 'SYNC';
1414
export const UNDO = 'UNDO';
1515
export const UPDATE = 'UPDATE';
16+
export const PATCH = 'PATCH';
1617
export const PLUGIN = 'PLUGIN';

0 commit comments

Comments
 (0)