Repository: boardgameio/boardgame.io
Pull request #871 (2020-12-15 16:35:21)

Chat messages
So I read issue #460 about out of the box chat support and this is WIP for a very basic, non-persistent, chat messages passed between socket-io clients. If you guys think this is a good idea I will polish it and add tests etc. For now I just did something to see if my theory would work in practice. I tested it using the chess example and added some basic UI for it.

@delucis @nicolodavis 

<img width="1166" alt="Screenshot 2020-12-15 at 17 34 00" src="https://user-images.githubusercontent.com/1790340/102243710-c0f61c00-3efb-11eb-81be-82990187bdc2.png">


#### Checklist

* [x] Use a separate branch in your local repo (not `master`).
* [x] Test coverage is 100% (or you have a story for why it's ok).


nicolodavis: I do like the idea of an ephemeral chat that utilizes the boardgame.io transport layer. Nice work @TomasVidhall.

Can you add some unit tests for the code that you've added?
TomasVidhall: Hey @nicolodavis. Added some tests for the code now, not sure if I missed anything important since I don't fully grasp the codebase, so please point out if I did. 

I really wanted to add a test that actually used the client-server connection of SocketIO, but couldn't make it work. If you see a way to achieve that please tell 😄 

Patch:
diff --git a/examples/react-web/src/chess/board.js b/examples/react-web/src/chess/board.js
index 1a29419..88b6f24 100644
--- a/examples/react-web/src/chess/board.js
+++ b/examples/react-web/src/chess/board.js
@@ -11,6 +11,7 @@ import PropTypes from 'prop-types';
 import { Chess } from 'chess.js';
 import { Checkerboard, cartesianToAlgebraic } from './checkerboard';
 import { Token } from './token';
+import Chat from './chat';
 import Bishop from './pieces/bishop';
 import King from './pieces/king';
 import Knight from './pieces/knight';
@@ -31,6 +32,8 @@ class Board extends React.Component {
     isActive: PropTypes.bool,
     isMultiplayer: PropTypes.bool,
     isConnected: PropTypes.bool,
+    sendChatMessage: PropTypes.func,
+    chatMessages: PropTypes.array,
   };
 
   constructor(props) {
@@ -64,6 +67,12 @@ class Board extends React.Component {
 
         {this._getStatus()}
         {disconnected}
+        {this.props.sendChatMessage && this.props.chatMessages && (
+          <Chat
+            onSend={this.props.sendChatMessage}
+            messages={this.props.chatMessages}
+          />
+        )}
       </div>
     );
   }
diff --git a/examples/react-web/src/chess/chat.js b/examples/react-web/src/chess/chat.js
new file mode 100644
index 0000000..9366ff7
--- /dev/null
+++ b/examples/react-web/src/chess/chat.js
@@ -0,0 +1,38 @@
+import React, { useState } from 'react';
+
+const Chat = ({ onSend, messages }) => {
+  const [message, setMessage] = useState('');
+
+  const onChange = event => {
+    setMessage(event.target.value);
+  };
+
+  const triggerSend = () => {
+    onSend(message);
+    setMessage('');
+  };
+
+  return (
+    <div>
+      <div
+        style={{
+          height: 200,
+          maxWidth: 400,
+          overflow: 'scroll',
+          border: '1px solid black',
+        }}
+      >
+        {messages.map(message => (
+          <div key={message.id}>
+            <div>{message.sender}</div>
+            <div>{JSON.stringify(message.payload)}</div>
+          </div>
+        ))}
+      </div>
+      <input onChange={onChange} value={message} />
+      <button onClick={triggerSend}>Send</button>
+    </div>
+  );
+};
+
+export default Chat;
diff --git a/src/client/client.ts b/src/client/client.ts
index c112a0f..d49ddd1 100644
--- a/src/client/client.ts
+++ b/src/client/client.ts
@@ -6,6 +6,7 @@
  * https://opensource.org/licenses/MIT.
  */
 
+import shortid from 'shortid';
 import 'svelte';
 import {
   Dispatch,
@@ -35,6 +36,7 @@ import {
   State,
   Store,
   Ctx,
+  ChatMessage,
 } from '../types';
 
 type ClientAction = ActionShape.Reset | ActionShape.Sync | ActionShape.Update;
@@ -160,6 +162,8 @@ export class _ClientImpl<G extends any = any> {
   reset: () => void;
   undo: () => void;
   redo: () => void;
+  sendChatMessage: (message: ChatMessage) => void;
+  chatMessages: ChatMessage[];
 
   constructor({
     game,
@@ -322,6 +326,7 @@ export class _ClientImpl<G extends any = any> {
       onAction: () => {},
       subscribe: () => {},
       subscribeMatchData: () => {},
+      subscribeChatMessage: () => {},
       connect: () => {},
       disconnect: () => {},
       updateMatchID: () => {},
@@ -348,6 +353,21 @@ export class _ClientImpl<G extends any = any> {
       this.matchData = metadata;
       this.notifySubscribers();
     });
+
+    if (this.transport.onChatMessage) {
+      this.chatMessages = [];
+      this.sendChatMessage = payload => {
+        this.transport.onChatMessage(this.matchID, {
+          id: shortid(),
+          sender: this.playerID,
+          payload: payload,
+        });
+      };
+      this.transport.subscribeChatMessage(message => {
+        this.chatMessages = [...this.chatMessages, message];
+        this.notifySubscribers();
+      });
+    }
   }
 
   private notifySubscribers() {
diff --git a/src/client/react-native.js b/src/client/react-native.js
index ebebe57..2f5d22d 100644
--- a/src/client/react-native.js
+++ b/src/client/react-native.js
@@ -112,6 +112,8 @@ export function Client(opts) {
           undo: this.client.undo,
           redo: this.client.redo,
           matchData: this.client.matchData,
+          sendChatMessage: this.client.sendChatMessage,
+          chatMessages: this.client.chatMessages,
         });
       }
 
diff --git a/src/client/react.tsx b/src/client/react.tsx
index 1c50070..0c2be8f 100644
--- a/src/client/react.tsx
+++ b/src/client/react.tsx
@@ -33,6 +33,7 @@ type ExposedClientProps<G extends any = any> = Pick<
   | 'playerID'
   | 'matchID'
   | 'matchData'
+  | 'sendChatMessage'
 >;
 
 export type BoardProps<G extends any = any> = ClientState<G> &
@@ -180,6 +181,8 @@ export function Client<
           redo: this.client.redo,
           log: this.client.log,
           matchData: this.client.matchData,
+          sendChatMessage: this.client.sendChatMessage,
+          chatMessages: this.client.chatMessages
         });
       }
 
diff --git a/src/client/transport/local.ts b/src/client/transport/local.ts
index b029178..894e305 100644
--- a/src/client/transport/local.ts
+++ b/src/client/transport/local.ts
@@ -10,7 +10,7 @@ import * as ActionCreators from '../../core/action-creators';
 import { InMemory } from '../../server/db/inmemory';
 import { LocalStorage } from '../../server/db/localstorage';
 import { Master, TransportAPI } from '../../master/master';
-import { Transport, TransportOpts } from './transport';
+import { Transport, TransportOpts, ChatCallback } from './transport';
 import {
   CredentialedActionShape,
   Game,
@@ -136,6 +136,7 @@ type LocalTransportOpts = TransportOpts & {
  */
 export class LocalTransport extends Transport {
   master: LocalMaster;
+  chatMessageCallback: ChatCallback;
 
   /**
    * Creates a new Mutiplayer instance.
@@ -158,6 +159,10 @@ export class LocalTransport extends Transport {
     this.isConnected = true;
   }
 
+  onChatMessage(matchID, chatMessage) {
+    this.master.onChatMessage(matchID, chatMessage);
+  }
+
   /**
    * Called when another player makes a move and the
    * master broadcasts the update to other clients (including
@@ -202,6 +207,10 @@ export class LocalTransport extends Transport {
       if (type == 'update') {
         this.onUpdate.apply(this, args);
       }
+      if (type == 'chat') {
+        const [matchID, message] = args;
+        this.chatMessageCallback.apply(this, [message]);
+      }
     });
     this.master.onSync(
       this.matchID,
@@ -223,6 +232,10 @@ export class LocalTransport extends Transport {
 
   subscribeMatchData() {}
 
+  subscribeChatMessage(fn: ChatCallback) {
+    this.chatMessageCallback = fn;
+  }
+
   /**
    * Dispatches a reset action, then requests a fresh sync from the master.
    */
diff --git a/src/client/transport/socketio.ts b/src/client/transport/socketio.ts
index df86f93..b220009 100644
--- a/src/client/transport/socketio.ts
+++ b/src/client/transport/socketio.ts
@@ -11,7 +11,12 @@ const io = ioNamespace.default;
 
 import * as ActionCreators from '../../core/action-creators';
 import { Master } from '../../master/master';
-import { Transport, TransportOpts, MetadataCallback } from './transport';
+import {
+  Transport,
+  TransportOpts,
+  MetadataCallback,
+  ChatCallback,
+} from './transport';
 import {
   CredentialedActionShape,
   FilteredMetadata,
@@ -19,6 +24,7 @@ import {
   PlayerID,
   State,
   SyncInfo,
+  ChatMessage,
 } from '../../types';
 
 interface SocketIOOpts {
@@ -42,6 +48,7 @@ export class SocketIOTransport extends Transport {
   socketOpts: SocketIOClient.ConnectOpts;
   callback: () => void;
   matchDataCallback: MetadataCallback;
+  chatMessageCallback: ChatCallback;
 
   /**
    * Creates a new Mutiplayer instance.
@@ -72,6 +79,7 @@ export class SocketIOTransport extends Transport {
     this.isConnected = false;
     this.callback = () => {};
     this.matchDataCallback = () => {};
+    this.chatMessageCallback = () => {};
   }
 
   /**
@@ -88,6 +96,10 @@ export class SocketIOTransport extends Transport {
     this.socket.emit('update', ...args);
   }
 
+  onChatMessage(matchID, chatMessage) {
+    this.socket.emit('chat', matchID, chatMessage);
+  }
+
   /**
    * Connect to the server.
    */
@@ -147,6 +159,12 @@ export class SocketIOTransport extends Transport {
       }
     );
 
+    this.socket.on('chat', (matchID: string, chatMessage: ChatMessage) => {
+      if (matchID === this.matchID) {
+        this.chatMessageCallback(chatMessage);
+      }
+    });
+
     // Keep track of connection status.
     this.socket.on('connect', () => {
       // Initial sync to get game state.
@@ -181,6 +199,10 @@ export class SocketIOTransport extends Transport {
     this.matchDataCallback = fn;
   }
 
+  subscribeChatMessage(fn: ChatCallback) {
+    this.chatMessageCallback = fn;
+  }
+
   /**
    * Send a “sync” event to the server.
    */
diff --git a/src/client/transport/transport.ts b/src/client/transport/transport.ts
index cad5ea0..a9f6c92 100644
--- a/src/client/transport/transport.ts
+++ b/src/client/transport/transport.ts
@@ -14,10 +14,13 @@ import {
   State,
   Store,
   SyncInfo,
+  ChatMessage,
 } from '../../types';
 
 export type MetadataCallback = (metadata: SyncInfo['filteredMetadata']) => void;
 
+export type ChatCallback = (message: ChatMessage) => void;
+
 export interface TransportOpts {
   store?: Store;
   gameName?: string;
@@ -62,4 +65,6 @@ export abstract class Transport {
   abstract updateMatchID(id: string): void;
   abstract updatePlayerID(id: PlayerID): void;
   abstract updateCredentials(credentials?: string): void;
+  abstract onChatMessage(matchID: string, chatMessage: ChatMessage): void;
+  abstract subscribeChatMessage(fn: ChatCallback): void;
 }
diff --git a/src/master/master.ts b/src/master/master.ts
index 3cac00f..c801639 100644
--- a/src/master/master.ts
+++ b/src/master/master.ts
@@ -23,6 +23,7 @@ import {
   CredentialedActionShape,
   LogEntry,
   PlayerID,
+  ChatMessage,
 } from '../types';
 import { createMetadata } from '../server/util';
 import { Auth } from '../server/auth';
@@ -101,6 +102,10 @@ type TransportData =
   | {
       type: 'matchData';
       args: [string, FilteredMetadata];
+    }
+  | {
+      type: 'chat';
+      args: [string, ChatMessage];
     };
 
 export interface TransportAPI {
@@ -458,4 +463,11 @@ export class Master {
       await this.storageAPI.setMetadata(key, metadata);
     }
   }
+
+  async onChatMessage(matchID, chatMessage) {
+    this.transportAPI.sendAll(() => ({
+      type: 'chat',
+      args: [matchID, chatMessage],
+    }));
+  }
 }
diff --git a/src/server/transport/socketio.ts b/src/server/transport/socketio.ts
index 7f485b4..d9fb4a7 100644
--- a/src/server/transport/socketio.ts
+++ b/src/server/transport/socketio.ts
@@ -184,6 +184,15 @@ export class SocketIO {
             );
           }
         });
+        socket.on('chat', async (matchID, chatMessage) => {
+          const master = new Master(
+            game,
+            app.context.db,
+            TransportAPI(matchID, socket, this.clientInfo, this.roomInfo),
+            app.context.auth
+          );
+          master.onChatMessage(matchID, chatMessage);
+        });
       });
     }
   }
diff --git a/src/types.ts b/src/types.ts
index fa2f32d..9338081 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -426,3 +426,9 @@ export interface SyncInfo {
   initialState: State;
   log: LogEntry[];
 }
+
+export interface ChatMessage {
+  id: string;
+  sender: PlayerID;
+  payload: any;
+}


Test patch:
diff --git a/src/client/client.test.ts b/src/client/client.test.ts
index 4ea0bc3..74ad3c2 100644
--- a/src/client/client.test.ts
+++ b/src/client/client.test.ts
@@ -164,6 +164,21 @@ describe('multiplayer', () => {
       client.moves.A();
       expect(client.transport.onAction).toHaveBeenCalled();
     });
+
+    test('Sends and receives chat messages', () => {
+      const fn = jest.fn();
+      jest.spyOn(client.transport, 'onAction');
+      client.updatePlayerID('0');
+      client.updateMatchID('matchID');
+      jest.spyOn(client.transport, 'onChatMessage');
+
+      client.sendChatMessage({ message: 'foo' });
+
+      expect(client.transport.onChatMessage).toHaveBeenCalledWith(
+        'matchID',
+        expect.objectContaining({ payload: { message: 'foo' }, sender: '0' })
+      );
+    });
   });
 
   describe('multiplayer: SocketIO()', () => {
@@ -232,6 +247,25 @@ describe('multiplayer', () => {
 
       expect(client0.getState().G).toEqual({ A: '1' });
       expect(client1.getState().G).toEqual({ A: '1' });
+
+      client0.sendChatMessage({ message: 'foo' });
+
+      expect(client0.chatMessages).toEqual([
+        expect.objectContaining({ sender: '0', payload: { message: 'foo' } }),
+      ]);
+      expect(client1.chatMessages).toEqual([
+        expect.objectContaining({ sender: '0', payload: { message: 'foo' } }),
+      ]);
+
+      client1.sendChatMessage({ message: 'bar' });
+      expect(client0.chatMessages).toEqual([
+        expect.objectContaining({ sender: '0', payload: { message: 'foo' } }),
+        expect.objectContaining({ sender: '1', payload: { message: 'bar' } }),
+      ]);
+      expect(client1.chatMessages).toEqual([
+        expect.objectContaining({ sender: '0', payload: { message: 'foo' } }),
+        expect.objectContaining({ sender: '1', payload: { message: 'bar' } }),
+      ]);
     });
   });
 
diff --git a/src/client/transport/local.test.ts b/src/client/transport/local.test.ts
index 6e961f8..33bed9c 100644
--- a/src/client/transport/local.test.ts
+++ b/src/client/transport/local.test.ts
@@ -303,6 +303,7 @@ describe('LocalTransport', () => {
     const master = ({
       onSync: jest.fn(),
       onUpdate: jest.fn(),
+      onChatMessage: jest.fn(),
     } as unknown) as LocalMaster;
     class WrappedLocalTransport extends LocalTransport {
       setStore(store: Store) {
@@ -358,5 +359,12 @@ describe('LocalTransport', () => {
         null
       );
     });
+
+    test('send chat-message', () => {
+      m.onChatMessage('matchID', { message: 'foo' });
+      expect(m.master.onChatMessage).lastCalledWith('matchID', {
+        message: 'foo',
+      });
+    });
   });
 });
diff --git a/src/client/transport/socketio.test.ts b/src/client/transport/socketio.test.ts
index 8cc77b4..0325cfe 100644
--- a/src/client/transport/socketio.test.ts
+++ b/src/client/transport/socketio.test.ts
@@ -192,6 +192,23 @@ describe('multiplayer', () => {
     const args: UpdateArgs = [action, state._stateID, 'default', null];
     expect(mockSocket.emit).lastCalledWith('update', ...args);
   });
+
+  test('receive chat-message', () => {
+    let receivedChatData;
+    m.subscribeChatMessage(data => (receivedChatData = data));
+    const chatData = { message: 'foo' };
+    mockSocket.receive('chat', 'unknown matchID', chatData);
+    expect(receivedChatData).toBe(undefined);
+    mockSocket.receive('chat', 'default', chatData);
+    expect(receivedChatData).toMatchObject(receivedChatData);
+  });
+
+  test('send chat-message', () => {
+    m.onChatMessage('matchID', { message: 'foo' });
+    expect(mockSocket.emit).lastCalledWith('chat', 'matchID', {
+      message: 'foo',
+    });
+  });
 });
 
 describe('server option', () => {
diff --git a/src/master/master.test.ts b/src/master/master.test.ts
index 2a2eee1..0ee0c5c 100644
--- a/src/master/master.test.ts
+++ b/src/master/master.test.ts
@@ -967,3 +967,25 @@ describe('redactLog', () => {
     ]);
   });
 });
+
+describe('chat', () => {
+  let sendAllReturn;
+  const send = jest.fn();
+  const sendAll = jest.fn(arg => {
+    sendAllReturn = arg;
+  });
+  const db = new InMemory();
+  const master = new Master(game, db, TransportAPI(send, sendAll));
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  test('Sends chat messages to all', async () => {
+    master.onChatMessage('matchID', { message: 'foo' });
+    expect(sendAllReturn('0')).toEqual({
+      type: 'chat',
+      args: ['matchID', { message: 'foo' }],
+    });
+  });
+});
diff --git a/src/server/transport/socketio.test.ts b/src/server/transport/socketio.test.ts
index 13836e5..7b5d104 100644
--- a/src/server/transport/socketio.test.ts
+++ b/src/server/transport/socketio.test.ts
@@ -35,11 +35,13 @@ jest.mock('../../master/master', () => {
     onUpdate: jest.Mock<any, any>;
     onSync: jest.Mock<any, any>;
     onConnectionChange: jest.Mock<any, any>;
+    onChatMessage: jest.Mock<any, any>;
 
     constructor() {
       this.onUpdate = jest.fn();
       this.onSync = jest.fn();
       this.onConnectionChange = jest.fn();
+      this.onChatMessage = jest.fn();
     }
   }
 
@@ -205,6 +207,18 @@ describe('sync / update', () => {
   });
 });
 
+describe('chat', () => {
+  const app: any = { context: {} };
+  const games = [ProcessGameConfig({ seed: 0 })];
+  const transport = new SocketIOTestAdapter();
+  transport.init(app, games);
+  const io = app.context.io;
+
+  test('chat message', async () => {
+    await io.socket.receive('chat', 'matchID', { message: 'foo' });
+  });
+});
+
 describe('connect / disconnect', () => {
   const auth = new Auth({ authenticateCredentials: () => true });
   const app: any = { context: { auth } };