Skip to content

Commit

Permalink
refactor(transport): Consolidate transport interface (#1002)
Browse files Browse the repository at this point in the history 
* feat(transport): Initial P2P transport implementation

* docs(examples): Add P2P transport example

* docs(examples): Fix lack of viewport tag on HTML template

* docs(examples): P2P example tweaks

* refactor(transport): Move shared transport logic to base class

* test(transport): Test callback initialisation in base class

* feat(transport): Allow configuration of `Peer`

Pass options object to P2P transport to use when instantiating the `Peer` connection.

* refactor(transport): Centralise update handling in the client

Previously, different transport implementations received a reference to the client’s Redux store and duplicated logic to map update data from the master to an action which they dispatched to the store. This centralises this in the client, keeping the store a client-only concept and exposing a single callback to transport implementations that expects to receive the same `TransportData` emitted by the master. In this way, transports should be “thinner”, focusing on implementing the glue between a client and a master.

* test: Update tests for new transport interface

* refactor(transport): Rename transport methods for clarity

- `onAction` → `sendAction`
- `onChatMessage` → `sendChatMessage`

* refactor(transport): Move `isConnected` initialisation to base class

* refactor(transport): Move responsibility for calling connection callback to base class

* refactor(transport): Rename method for clarity

Rename internal `callback` property to `connectionStatusCallback` to better describe purpose.

* docs(transport): Move method comments to base class

* revert: Remove P2P transport

* refactor(transport): Rename method to `subscribeToConnectionStatus`

* build: Export base `Transport` class in internal package

* refactor(client): Rename private class method

* refactor(client): Insulate client callback in `Transport`

* docs(master): Add comments to clarify types

* fix(types): Include `isConnected` in match metadata interface

* style: Be consistent about specifying return types
  • Loading branch information
@delucis
delucis authored Sep 13, 2021
1 parent e4fe528 commit 510a082
Show file tree
Hide file tree
Showing 13 changed files with 492 additions and 546 deletions.
1 change: 1 addition & 0 deletions examples/react-web/src/index.html
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<!DOCTYPE html>
<html>
<head>
<meta content="width=device-width,initial-scale=1" name="viewport">
<link rel="stylesheet" href="//unpkg.com/docsify/lib/themes/vue.css">
</head>
<body class="ready sticky">
Expand Down
10 changes: 9 additions & 1 deletion packages/internal.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,13 @@ import { InitializeGame } from '../src/core/initialize';
import { ProcessGameConfig } from '../src/core/game';
import { CreateGameReducer } from '../src/core/reducer';
import { Async, Sync } from '../src/server/db/base';
import { Transport } from '../src/client/transport/transport';

export { Async, Sync, ProcessGameConfig, InitializeGame, CreateGameReducer };
export {
Async,
Sync,
Transport,
ProcessGameConfig,
InitializeGame,
CreateGameReducer,
};
154 changes: 132 additions & 22 deletions src/client/client.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { CreateGameReducer } from '../core/reducer';
import { InitializeGame } from '../core/initialize';
import { Client, createMoveDispatchers } from './client';
import { ProcessGameConfig } from '../core/game';
import type { Transport } from './transport/transport';
import { Transport } from './transport/transport';
import { LocalTransport, Local } from './transport/local';
import { SocketIOTransport, SocketIO } from './transport/socketio';
import {
Expand All @@ -27,6 +27,7 @@ import Debug from './debug/Debug.svelte';
import { error } from '../core/logger';
import type { LogEntry, State, SyncInfo } from '../types';
import type { Operation } from 'rfc6902';
import type { TransportData } from '../master/master';

jest.mock('../core/logger', () => ({
info: jest.fn(),
Expand Down Expand Up @@ -166,35 +167,35 @@ describe('multiplayer', () => {
});

test('onAction called', () => {
jest.spyOn(client.transport, 'onAction');
jest.spyOn(client.transport, 'sendAction');
const state = { G: {}, ctx: { phase: '' }, plugins: {} };
const filteredMetadata = [];
client.store.dispatch(sync({ state, filteredMetadata } as SyncInfo));
client.moves.A();
expect(client.transport.onAction).toHaveBeenCalled();
expect(client.transport.sendAction).toHaveBeenCalled();
});

test('strip transients action not sent to transport', () => {
jest.spyOn(client.transport, 'onAction');
jest.spyOn(client.transport, 'sendAction');
const state = { G: {}, ctx: { phase: '' }, plugins: {} };
const filteredMetadata = [];
client.store.dispatch(sync({ state, filteredMetadata } as SyncInfo));
client.moves.Invalid();
expect(client.transport.onAction).not.toHaveBeenCalledWith(
expect(client.transport.sendAction).not.toHaveBeenCalledWith(
expect.any(Object),
{ type: Actions.STRIP_TRANSIENTS }
);
});

test('Sends and receives chat messages', () => {
jest.spyOn(client.transport, 'onAction');
jest.spyOn(client.transport, 'sendAction');
client.updatePlayerID('0');
client.updateMatchID('matchID');
jest.spyOn(client.transport, 'onChatMessage');
jest.spyOn(client.transport, 'sendChatMessage');

client.sendChatMessage({ message: 'foo' });

expect(client.transport.onChatMessage).toHaveBeenCalledWith(
expect(client.transport.sendChatMessage).toHaveBeenCalledWith(
'matchID',
expect.objectContaining({ payload: { message: 'foo' }, sender: '0' })
);
Expand Down Expand Up @@ -290,20 +291,20 @@ describe('multiplayer', () => {
});

describe('custom transport', () => {
class CustomTransport {
callback;

constructor() {
this.callback = null;
}

subscribeMatchData(fn) {
this.callback = fn;
class CustomTransport extends Transport {
connect() {}
disconnect() {}
sendAction() {}
sendChatMessage() {}
requestSync() {}
updateMatchID() {}
updatePlayerID() {}
updateCredentials() {}
setMetadata(metadata) {
this.notifyClient({ type: 'matchData', args: ['default', metadata] });
}

subscribeChatMessage() {}
}
const customTransport = () => new CustomTransport() as unknown as Transport;
const customTransport = (opts) => new CustomTransport(opts);

let client;

Expand All @@ -320,12 +321,121 @@ describe('multiplayer', () => {

test('metadata callback', () => {
const metadata = { m: true };
client.transport.callback(metadata);
client.transport.setMetadata(metadata);
expect(client.matchData).toEqual(metadata);
});
});
});

describe('receiveTransportData', () => {
let sendToClient: (data: TransportData) => void;
let client: ReturnType<typeof Client>;
let requestSync: jest.Mock;

beforeEach(() => {
requestSync = jest.fn();
client = Client({
game: {},
matchID: 'A',
debug: false,
// Use the multiplayer interface to extract the client callback
// and use it to send updates to the client directly.
multiplayer: ({ transportDataCallback }) => {
sendToClient = transportDataCallback;
return {
connect() {},
disconnect() {},
subscribe() {},
requestSync,
} as unknown as Transport;
},
});
client.start();
});

afterEach(() => {
client.stop();
});

test('discards update with wrong matchID', () => {
sendToClient({
type: 'sync',
args: ['wrongID', { state: { G: 'G', ctx: {} } } as SyncInfo],
});
expect(client.getState()).toBeNull();
});

test('applies sync', () => {
const state = { G: 'G', ctx: {} };
sendToClient({ type: 'sync', args: ['A', { state } as SyncInfo] });
expect(client.getState().G).toEqual(state.G);
});

test('applies update', () => {
const state1 = { G: 'G1', _stateID: 1, ctx: {} } as State;
const state2 = { G: 'G2', _stateID: 2, ctx: {} } as State;
sendToClient({ type: 'sync', args: ['A', { state: state1 } as SyncInfo] });
sendToClient({ type: 'update', args: ['A', state2, []] });
expect(client.getState().G).toEqual(state2.G);
});

test('ignores stale update', () => {
const state1 = { G: 'G1', _stateID: 1, ctx: {} } as State;
const state2 = { G: 'G2', _stateID: 0, ctx: {} } as State;
sendToClient({ type: 'sync', args: ['A', { state: state1 } as SyncInfo] });
sendToClient({ type: 'update', args: ['A', state2, []] });
expect(client.getState().G).toEqual(state1.G);
});

test('applies a patch', () => {
const state = { G: 'G1', _stateID: 1, ctx: {} } as State;
sendToClient({ type: 'sync', args: ['A', { state } as SyncInfo] });
sendToClient({
type: 'patch',
args: ['A', 1, 2, [{ op: 'replace', path: '/_stateID', value: 2 }], []],
});
expect(client.getState()._stateID).toBe(2);
});

test('ignores patch for different state ID', () => {
const state = { G: 'G1', _stateID: 1, ctx: {} } as State;
sendToClient({ type: 'sync', args: ['A', { state } as SyncInfo] });
sendToClient({
type: 'patch',
args: ['A', 2, 3, [{ op: 'replace', path: '/_stateID', value: 3 }], []],
});
expect(client.getState()._stateID).toBe(1);
});

test('resyncs after failed patch', () => {
const state = { G: 'G1', _stateID: 1, ctx: {} } as State;
sendToClient({ type: 'sync', args: ['A', { state } as SyncInfo] });
expect(requestSync).not.toHaveBeenCalled();
// Send bad patch.
sendToClient({
type: 'patch',
args: ['A', 1, 2, [{ op: 'replace', path: '/_stateIDD', value: 2 }], []],
});
// State is unchanged and the client requested to resync.
expect(client.getState()._stateID).toBe(1);
expect(requestSync).toHaveBeenCalled();
});

test('updates match metadata', () => {
expect(client.matchData).toBeUndefined();
const matchData = [{ id: 0 }];
sendToClient({ type: 'matchData', args: ['A', matchData] });
expect(client.matchData).toEqual(matchData);
});

test('appends a chat message', () => {
expect(client.chatMessages).toEqual([]);
const message = { id: 'x', sender: '0', payload: 'hi' };
sendToClient({ type: 'chat', args: ['A', message] });
expect(client.chatMessages).toEqual([message]);
});
});

describe('strip secret only on server', () => {
let client0;
let client1;
Expand Down Expand Up @@ -771,7 +881,7 @@ describe('subscribe', () => {
client.subscribe(fn);
client.start();
fn.mockClear();
transport.callback();
(transport as any).connectionStatusCallback();
expect(fn).toHaveBeenCalled();
client.stop();
});
Expand Down
83 changes: 69 additions & 14 deletions src/client/client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { PlayerView } from '../plugins/main';
import type { Transport, TransportOpts } from './transport/transport';
import { DummyTransport } from './transport/dummy';
import { ClientManager } from './manager';
import type { TransportData } from '../master/master';
import type {
ActivePlayersArg,
ActionShape,
Expand Down Expand Up @@ -182,7 +183,7 @@ export class _ClientImpl<G extends any = any> {
}: ClientOpts) {
this.game = ProcessGameConfig(game);
this.playerID = playerID;
this.matchID = matchID;
this.matchID = matchID || 'default';
this.credentials = credentials;
this.multiplayer = multiplayer;
this.debugOpt = debug;
Expand Down Expand Up @@ -291,7 +292,7 @@ export class _ClientImpl<G extends any = any> {
!('clientOnly' in action) &&
action.type !== Actions.STRIP_TRANSIENTS
) {
this.transport.onAction(baseState, action);
this.transport.sendAction(baseState, action);
}

return result;
Expand Down Expand Up @@ -321,9 +322,9 @@ export class _ClientImpl<G extends any = any> {

if (!multiplayer) multiplayer = DummyTransport;
this.transport = multiplayer({
transportDataCallback: (data) => this.receiveTransportData(data),
gameKey: game,
game: this.game,
store: this.store,
matchID,
playerID,
credentials,
Expand All @@ -333,23 +334,77 @@ export class _ClientImpl<G extends any = any> {

this.createDispatchers();

this.transport.subscribeMatchData((metadata) => {
this.matchData = metadata;
this.notifySubscribers();
});

this.chatMessages = [];
this.sendChatMessage = (payload) => {
this.transport.onChatMessage(this.matchID, {
this.transport.sendChatMessage(this.matchID, {
id: nanoid(7),
sender: this.playerID,
payload: payload,
});
};
this.transport.subscribeChatMessage((message) => {
this.chatMessages = [...this.chatMessages, message];
this.notifySubscribers();
});
}

/** Handle incoming match data from a multiplayer transport. */
private receiveMatchData(matchData: FilteredMetadata): void {
this.matchData = matchData;
this.notifySubscribers();
}

/** Handle an incoming chat message from a multiplayer transport. */
private receiveChatMessage(message: ChatMessage): void {
this.chatMessages = [...this.chatMessages, message];
this.notifySubscribers();
}

/** Handle all incoming updates from a multiplayer transport. */
private receiveTransportData(data: TransportData): void {
const [matchID] = data.args;
if (matchID !== this.matchID) return;
switch (data.type) {
case 'sync': {
const [, syncInfo] = data.args;
const action = ActionCreators.sync(syncInfo);
this.receiveMatchData(syncInfo.filteredMetadata);
this.store.dispatch(action);
break;
}
case 'update': {
const [, state, deltalog] = data.args;
const currentState = this.store.getState();
if (state._stateID >= currentState._stateID) {
const action = ActionCreators.update(state, deltalog);
this.store.dispatch(action);
}
break;
}
case 'patch': {
const [, prevStateID, stateID, patch, deltalog] = data.args;
const currentStateID = this.store.getState()._stateID;
if (prevStateID !== currentStateID) break;
const action = ActionCreators.patch(
prevStateID,
stateID,
patch,
deltalog
);
this.store.dispatch(action);
// Emit sync if patch apply failed.
if (this.store.getState()._stateID === currentStateID) {
this.transport.requestSync();
}
break;
}
case 'matchData': {
const [, matchData] = data.args;
this.receiveMatchData(matchData);
break;
}
case 'chat': {
const [, chatMessage] = data.args;
this.receiveChatMessage(chatMessage);
break;
}
}
}

private notifySubscribers() {
Expand All @@ -376,7 +431,7 @@ export class _ClientImpl<G extends any = any> {
subscribe(fn: (state: ClientState<G>) => void) {
const id = Object.keys(this.subscribers).length;
this.subscribers[id] = fn;
this.transport.subscribe(() => this.notifySubscribers());
this.transport.subscribeToConnectionStatus(() => this.notifySubscribers());

if (this._running || !this.multiplayer) {
fn(this.getState());
Expand Down
8 changes: 3 additions & 5 deletions src/client/transport/dummy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@ import type { TransportOpts } from './transport';
class DummyImpl extends Transport {
connect() {}
disconnect() {}
onAction() {}
onChatMessage() {}
subscribe() {}
subscribeChatMessage() {}
subscribeMatchData() {}
sendAction() {}
sendChatMessage() {}
requestSync() {}
updateCredentials() {}
updateMatchID() {}
updatePlayerID() {}
Expand Down
Loading

0 comments on commit 510a082

Please sign in to comment.