Better handling the numPlayers

Introduced a function to calculate the number of players in a players array.
Calculating numPlayers from the players array. Also, linting.

src/server/api.test.ts

@@ -449,14 +449,60 @@ describe('.configureRouter', () => {
 
         describe('when playerID is omitted', () => {
           beforeEach(async () => {
-            const app = createApiServer({ db, auth, games });
+            const app = createApiServer({
+              db,
+              auth: new Auth({ generateCredentials: () => credentials }),
+              games,
+              uuid: () => 'matchID',
+            });
             response = await request(app.callback())
               .post('/games/foo/1/join')
-              .send('playerName=1');
+              .send('playerName=alice');
           });
 
-          test('throws error 403', async () => {
-            expect(response.status).toEqual(403);
+          describe('numPlayers is reached in match', () => {
+            beforeEach(async () => {
+              db = new AsyncStorage({
+                fetch: async () => {
+                  return {
+                    metadata: {
+                      players: {
+                        '0': { name: 'alice' },
+                      },
+                    },
+                  };
+                },
+              });
+              const app = createApiServer({ db, auth, games });
+              response = await request(app.callback())
+                .post('/games/foo/1/join')
+                .send('playerName=bob');
+            });
+
+            test('throws error 409', async () => {
+              expect(response.status).toEqual(409);
+            });
+          });
+
+          test('is successful', async () => {
+            expect(response.status).toEqual(200);
+          });
+
+          test('returns the player credentials', async () => {
+            expect(response.body.playerCredentials).toEqual(credentials);
+          });
+
+          test('updates the player name', async () => {
+            expect(db.mocks.setMetadata).toHaveBeenCalledWith(
+              '1',
+              expect.objectContaining({
+                players: expect.objectContaining({
+                  '0': expect.objectContaining({
+                    name: 'alice',
+                  }),
+                }),
+              })
+            );
           });
         });
 

src/server/api.ts

@@ -12,7 +12,11 @@ import koaBody from 'koa-body';
 import { nanoid } from 'nanoid';
 import cors from '@koa/cors';
 import type IOTypes from 'socket.io';
-import { createMatch, getFirstAvailablePlayerIndex } from './util';
+import {
+  createMatch,
+  getFirstAvailablePlayerIndex,
+  getNumPlayers,
+} from './util';
 import type { Auth } from './auth';
 import type { Server, LobbyAPI, Game, StorageAPI } from '../types';
 
@@ -223,27 +227,34 @@ export const configureRouter = ({
     const playerName = ctx.request.body.playerName;
     const data = ctx.request.body.data;
     const matchID = ctx.params.id;
-
     if (!playerName) {
       ctx.throw(403, 'playerName is required');
     }
 
     // Fetch matchdata early for determining playerID
-    const { metadata } : { metadata: Server.MatchData } = await (db as StorageAPI.Async).fetch(matchID, {
+    const { metadata }: { metadata: Server.MatchData } = await (
+      db as StorageAPI.Async
+    ).fetch(matchID, {
       metadata: true,
     });
     if (!metadata) {
       ctx.throw(404, 'Match ' + matchID + ' not found');
     }
 
     if (typeof playerID === 'undefined' || playerID === null) {
-      playerID = getFirstAvailablePlayerIndex(metadata);
+      playerID = getFirstAvailablePlayerIndex(metadata.players);
       if (playerID === -1) {
-        ctx.throw(409, 'Match ' + matchID + ' reached maximum number of players (' + metadata.numPlayers + ')');
+        ctx.throw(
+          409,
+          'Match ' +
+            matchID +
+            ' reached maximum number of players (' +
+            getNumPlayers(metadata.players) +
+            ')'
+        );
       }
     }
 
-
     if (!metadata.players[playerID]) {
       ctx.throw(404, 'Player ' + playerID + ' not found');
     }

src/server/util.ts

@@ -21,7 +21,6 @@ export const createMetadata = ({
     players: {},
     createdAt: Date.now(),
     updatedAt: Date.now(),
-    numPlayers: numPlayers
   };
 
   if (setupData !== undefined) metadata.setupData = setupData;
@@ -64,16 +63,27 @@ export const createMatch = ({
 };
 
 /**
- * Given a match, tries to find the first index of player that can be joined
+ * Given players, returns the count of players.
  */
-export const getFirstAvailablePlayerIndex = (match: Server.MatchData): number => {
+export const getNumPlayers = (players: {
+  [id: number]: Server.PlayerMetadata;
+}): number =>
+  Math.max(...Object.keys(players).map((k) => Number.parseInt(k))) + 1;
+
+/**
+ * Given players, tries to find the first index of player that can be joined. Returns -1 if there's no available index.
+ */
+export const getFirstAvailablePlayerIndex = (players: {
+  [id: number]: Server.PlayerMetadata;
+}): number => {
+  const numPlayers = getNumPlayers(players);
   let playerID = -1;
   // Try to get the first index available
-  for (const i of Array(match.numPlayers)) {
-    if (typeof match.players[i].name === 'undefined' || match.players[i].name === null) {
+  for (let i = 0; i < numPlayers; i++) {
+    if (typeof players[i].name === 'undefined' || players[i].name === null) {
       playerID = i;
       break;
     }
   }
   return playerID;
-}
+};

src/types.ts

@@ -385,7 +385,6 @@ export namespace Server {
     unlisted?: boolean;
     createdAt: number;
     updatedAt: number;
-    numPlayers: number;
   }
 
   export type AppCtx = Koa.DefaultContext & {