Repository: boardgameio/boardgame.io (ts)

Issue:
Keep track of players' connection status
It would be useful to keep track of all players' connections status and broadcast that info to other players. This would allow displaying connection status next to player info and let other players know that they are currently unavailable due to e.g. network issues.

I think the best place to store that info is `matchData` 
```
[
  { id: 0, name: 'Alice', isConnected: true },
  { id: 1, name: 'Bob', isConnected: false }
]
```

It could use server-side SocketIO connect/disconnect events to modify `matchData` and broadcast that info independently of game state.

If you think it's a nice-to-have feature in the framework I'd be happy to work on it and prepare a PR.

Pull request:
feat: Track players connection status (#834)
Closes #834 

- Server now keeps track of all players connection status and stores this information in `matchData` alongside `id` and `name`
- Every time player connects or disconnects from the SocketIO server, `onConnectionChange` method is called on `Master` to update players metadata in `DB` and broadcast that info to all the players in the match as `matchData` message

#### 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).


Patch:
diff --git a/docs/documentation/api/Client.md b/docs/documentation/api/Client.md
index 6183d12..aee95fe 100644
--- a/docs/documentation/api/Client.md
+++ b/docs/documentation/api/Client.md
@@ -108,7 +108,7 @@ The following properties are available on a client instance:
   ```js
   [
     { id: 0, name: 'Alice' },
-    { id: 1, name: 'Bob' }
+    { id: 1, name: 'Bob', isConnected: true }
   ]
   ```
 
@@ -322,7 +322,7 @@ following as `props`:
     ```js
     [
       { id: 0, name: 'Alice' },
-      { id: 1, name: 'Bob' }
+      { id: 1, name: 'Bob', isConnected: true }
     ]
     ```
 
diff --git a/src/client/client.ts b/src/client/client.ts
index 60a1ec9..952cfad 100644
--- a/src/client/client.ts
+++ b/src/client/client.ts
@@ -344,6 +344,7 @@ export class _ClientImpl<G extends any = any> {
 
     this.transport.subscribeMatchData(metadata => {
       this.matchData = metadata;
+      this.notifySubscribers();
     });
   }
 
diff --git a/src/client/transport/socketio.ts b/src/client/transport/socketio.ts
index 74026cb..7844563 100644
--- a/src/client/transport/socketio.ts
+++ b/src/client/transport/socketio.ts
@@ -13,6 +13,7 @@ import * as ActionCreators from '../../core/action-creators';
 import { Transport, TransportOpts, MetadataCallback } from './transport';
 import {
   CredentialedActionShape,
+  FilteredMetadata,
   LogEntry,
   PlayerID,
   State,
@@ -133,6 +134,17 @@ export class SocketIOTransport extends Transport {
       }
     });
 
+    // Called when new player joins the match or changes
+    // it's connection status
+    this.socket.on(
+      'matchData',
+      (matchID: string, matchData: FilteredMetadata) => {
+        if (matchID == this.matchID) {
+          this.matchDataCallback(matchData);
+        }
+      }
+    );
+
     // Keep track of connection status.
     this.socket.on('connect', () => {
       // Initial sync to get game state.
diff --git a/src/master/master.ts b/src/master/master.ts
index bb789ff..e48e39c 100644
--- a/src/master/master.ts
+++ b/src/master/master.ts
@@ -35,6 +35,15 @@ export const getPlayerMetadata = (
   }
 };
 
+/**
+ * Filter match data to get a player metadata object with credentials stripped.
+ */
+const filterMatchData = (matchData: Server.MatchData): FilteredMetadata =>
+  Object.values(matchData.players).map(player => {
+    const { credentials, ...filteredData } = player;
+    return filteredData;
+  });
+
 function IsSynchronous(
   storageAPI: StorageAPI.Sync | StorageAPI.Async
 ): storageAPI is StorageAPI.Sync {
@@ -132,6 +141,10 @@ type TransportData =
   | {
       type: 'sync';
       args: [string, SyncInfo];
+    }
+  | {
+      type: 'matchData';
+      args: [string, FilteredMetadata];
     };
 
 export interface TransportAPI {
@@ -379,13 +392,7 @@ export class Master {
       }
     }
 
-    let filteredMetadata: FilteredMetadata;
-    if (metadata) {
-      filteredMetadata = Object.values(metadata.players).map(player => {
-        const { credentials, ...filteredData } = player;
-        return filteredData;
-      });
-    }
+    const filteredMetadata = metadata ? filterMatchData(metadata) : undefined;
 
     const filteredState = {
       ...state,
@@ -412,4 +419,52 @@ export class Master {
 
     return;
   }
+
+  /**
+   * Called when a client connects or disconnects.
+   * Updates and sends out metadata to reflect the player’s connection status.
+   */
+  async onConnectionChange(
+    matchID: string,
+    playerID: string,
+    connected: boolean
+  ) {
+    const key = matchID;
+
+    let metadata: Server.MatchData | undefined;
+    if (IsSynchronous(this.storageAPI)) {
+      ({ metadata } = this.storageAPI.fetch(matchID, { metadata: true }));
+    } else {
+      ({ metadata } = await this.storageAPI.fetch(matchID, {
+        metadata: true,
+      }));
+    }
+
+    if (metadata === undefined) {
+      logging.error(`metadata not found for matchID=[${key}]`);
+      return { error: 'metadata not found' };
+    }
+
+    if (metadata.players[playerID] === undefined) {
+      logging.error(
+        `Player not in the match, matchID=[${key}] playerID=[${playerID}]`
+      );
+      return { error: 'player not in the match' };
+    }
+
+    metadata.players[playerID].isConnected = connected;
+
+    const filteredMetadata = filterMatchData(metadata);
+
+    this.transportAPI.sendAll(() => ({
+      type: 'matchData',
+      args: [matchID, filteredMetadata],
+    }));
+
+    if (IsSynchronous(this.storageAPI)) {
+      this.storageAPI.setMetadata(key, metadata);
+    } else {
+      await this.storageAPI.setMetadata(key, metadata);
+    }
+  }
 }
diff --git a/src/server/transport/socketio.ts b/src/server/transport/socketio.ts
index 825711e..2fcab56 100644
--- a/src/server/transport/socketio.ts
+++ b/src/server/transport/socketio.ts
@@ -155,13 +155,22 @@ export class SocketIO {
             this.auth
           );
           await master.onSync(matchID, playerID, numPlayers);
+          await master.onConnectionChange(matchID, playerID, true);
         });
 
-        socket.on('disconnect', () => {
+        socket.on('disconnect', async () => {
           if (this.clientInfo.has(socket.id)) {
-            const { matchID } = this.clientInfo.get(socket.id);
+            const { matchID, playerID } = this.clientInfo.get(socket.id);
             this.roomInfo.get(matchID).delete(socket.id);
             this.clientInfo.delete(socket.id);
+
+            const master = new Master(
+              game,
+              app.context.db,
+              TransportAPI(matchID, socket, this.clientInfo, this.roomInfo),
+              this.auth
+            );
+            await master.onConnectionChange(matchID, playerID, false);
           }
         });
       });
diff --git a/src/types.ts b/src/types.ts
index 608e43b..c175dd8 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -307,6 +307,7 @@ export namespace Server {
     name?: string;
     credentials?: string;
     data?: any;
+    isConnected?: boolean;
   };
 
   export interface MatchData {


Test patch:
diff --git a/src/client/transport/socketio.test.js b/src/client/transport/socketio.test.js
index a8e4116..5b365a8 100644
--- a/src/client/transport/socketio.test.js
+++ b/src/client/transport/socketio.test.js
@@ -135,6 +135,16 @@ describe('multiplayer', () => {
     expect(store.getState()).toMatchObject(restored);
   });
 
+  test('receive matchData', () => {
+    let receivedMatchData;
+    m.subscribeMatchData(data => (receivedMatchData = data));
+    const matchData = [{ id: '0', name: 'Alice' }];
+    mockSocket.receive('matchData', 'unknown matchID', matchData);
+    expect(receivedMatchData).toBe(undefined);
+    mockSocket.receive('matchData', 'default', matchData);
+    expect(receivedMatchData).toMatchObject(matchData);
+  });
+
   test('send update', () => {
     const action = makeMove();
     const state = { _stateID: 0 };
diff --git a/src/master/master.test.ts b/src/master/master.test.ts
index 5db608d..b099ea2 100644
--- a/src/master/master.test.ts
+++ b/src/master/master.test.ts
@@ -349,6 +349,95 @@ describe('update', () => {
   });
 });
 
+describe('connectionChange', () => {
+  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));
+
+  const metadata = {
+    gameName: 'tic-tac-toe',
+    setupData: {},
+    players: {
+      '0': {
+        id: 0,
+        credentials: 'qS2m4Ujb_',
+        name: 'Alice',
+      },
+      '1': {
+        id: 1,
+        credentials: 'nIQtXFybDD',
+        name: 'Bob',
+        isConnected: true,
+      },
+    },
+    createdAt: 0,
+    updatedAt: 0,
+  };
+  db.createMatch('matchID', { metadata, initialState: {} as State });
+
+  beforeEach(() => {
+    sendAllReturn = undefined;
+    jest.clearAllMocks();
+  });
+
+  test('changes players metadata', async () => {
+    await master.onConnectionChange('matchID', '0', true);
+
+    const expectedPlayerData = { id: 0, name: 'Alice', isConnected: true };
+    const {
+      metadata: { players },
+    } = db.fetch('matchID', { metadata: true });
+    expect(players['0']).toMatchObject(expectedPlayerData);
+  });
+
+  test('sends metadata to all', async () => {
+    await master.onConnectionChange('matchID', '1', false);
+    const expectedMetadata = [
+      { id: 0, name: 'Alice', isConnected: true },
+      { id: 1, name: 'Bob', isConnected: false },
+    ];
+    const sentMessage = sendAllReturn('0');
+    expect(sentMessage.type).toEqual('matchData');
+    expect(sentMessage.args[1]).toMatchObject(expectedMetadata);
+  });
+
+  test('invalid matchID', async () => {
+    const result = await master.onConnectionChange('invalidMatchID', '0', true);
+    expect(error).toHaveBeenCalledWith(
+      'metadata not found for matchID=[invalidMatchID]'
+    );
+    expect(result.error).toEqual('metadata not found');
+  });
+
+  test('invalid playerID', async () => {
+    const result = await master.onConnectionChange('matchID', '3', true);
+    expect(error).toHaveBeenCalledWith(
+      'Player not in the match, matchID=[matchID] playerID=[3]'
+    );
+    expect(result.error).toEqual('player not in the match');
+  });
+
+  test('processes connection change with an async db', async () => {
+    const asyncDb = new InMemoryAsync();
+    const masterWithAsyncDb = new Master(
+      game,
+      asyncDb,
+      TransportAPI(send, sendAll)
+    );
+    asyncDb.createMatch('matchID', { metadata, initialState: {} as State });
+
+    await masterWithAsyncDb.onConnectionChange('matchID', '0', true);
+
+    expect(sendAll).toHaveBeenCalled();
+  });
+});
+
 describe('playerView', () => {
   let sendAllReturn;
   let sendReturn;
diff --git a/src/server/transport/socketio.test.ts b/src/server/transport/socketio.test.ts
index 56b9dd3..7d12968 100644
--- a/src/server/transport/socketio.test.ts
+++ b/src/server/transport/socketio.test.ts
@@ -30,10 +30,12 @@ jest.mock('../../master/master', () => {
   class Master {
     onUpdate: jest.Mock<any, any>;
     onSync: jest.Mock<any, any>;
+    onConnectionChange: jest.Mock<any, any>;
 
     constructor() {
       this.onUpdate = jest.fn();
       this.onSync = jest.fn();
+      this.onConnectionChange = jest.fn();
     }
   }