Continue refactoring

client/src/Trade.tsx

@@ -43,7 +43,7 @@ const empty: Resources = {
 };
 
 const Trade: React.FC = () => {
-  const { ws, sendJsonMessage } = useContext(GlobalContext);
+  const { sendJsonMessage, lastJsonMessage } = useContext(GlobalContext);
   const [gives, setGives] = useState<Resources>(Object.assign({}, empty));
   const [gets, setGets] = useState<Resources>(Object.assign({}, empty));
   const [turn, setTurn] = useState<any>(undefined);
@@ -53,8 +53,11 @@ const Trade: React.FC = () => {
 
   const fields = useMemo(() => ["turn", "players", "private", "color"], []);
 
-  const onWsMessage = (event: MessageEvent) => {
+  useEffect(() => {
-    const data = JSON.parse(event.data);
+    if (!lastJsonMessage) {
+      return;
+    }
+    const data = lastJsonMessage;
     switch (data.type) {
       case "game-update":
         console.log(`trade - game-update: `, data.update);
@@ -74,21 +77,8 @@ const Trade: React.FC = () => {
       default:
         break;
     }
-  };
+  }, [lastJsonMessage, turn, players, priv, color]);
-  const refWsMessage = useRef(onWsMessage);
+
-  useEffect(() => {
-    refWsMessage.current = onWsMessage;
-  });
-  useEffect(() => {
-    if (!ws) {
-      return;
-    }
-    const cbMessage = (e: MessageEvent) => refWsMessage.current(e);
-    ws.addEventListener("message", cbMessage);
-    return () => {
-      ws.removeEventListener("message", cbMessage);
-    };
-  }, [ws, refWsMessage]);
   useEffect(() => {
     if (!sendJsonMessage) {
       return;
@@ -98,6 +88,7 @@ const Trade: React.FC = () => {
       fields,
     });
   }, [sendJsonMessage, fields]);
+
   const transfer = useCallback(
     (type: string, direction: string) => {
       if (direction === "give") {

server/routes/games.ts

@@ -7,7 +7,6 @@ import basePath from "../basepath";
 import {
   MAX_SETTLEMENTS,
   MAX_CITIES,
-  MAX_ROADS,
   types,
   debug,
   all,
@@ -18,6 +17,7 @@ import {
 
 import { getValidRoads, getValidCorners, isRuleEnabled } from "../util/validLocations";
 import { Player, Game, Session, CornerPlacement, RoadPlacement } from "./games/types";
+import { newPlayer } from "./games/playerFactory";
 import { normalizeIncoming, shuffleArray } from "./games/utils";
 // import type { GameState } from './games/state'; // unused import removed during typing pass
 
@@ -607,31 +607,7 @@ const processRoll = (game: Game, session: Session, dice: number[]): any => {
   });
 };
 
-const newPlayer = (color: string) => {
+// newPlayer is provided by ./games/playerFactory
-  return {
-    roads: MAX_ROADS,
-    cities: MAX_CITIES,
-    settlements: MAX_SETTLEMENTS,
-    points: 0,
-    status: "Not active",
-    lastActive: 0,
-    resources: 0,
-    order: 0,
-    stone: 0,
-    wheat: 0,
-    sheep: 0,
-    wood: 0,
-    brick: 0,
-    army: 0,
-    development: [],
-    color: color,
-    name: "",
-    totalTime: 0,
-    turnStart: 0,
-    ports: 0,
-    developmentCards: 0,
-  };
-};
 
 const getSession = (game: Game, id: string) => {
   if (!game.sessions) {
@@ -993,6 +969,10 @@ const adminCommands = (game: any, action: string, value: string, query: any): an
     case "pass":
       let name = game.turn.name;
       const next = getNextPlayerSession(game, name);
+      if (!next) {
+        addChatMessage(game, null, `Admin attempted to skip turn but no next player was found.`);
+        break;
+      }
       game.turn = {
         name: next.player,
         color: next.color,
@@ -1193,10 +1173,11 @@ const colorToWord = (color: string): string => {
   }
 };
 
-const getActiveCount = (game: any): number => {
+const getActiveCount = (game: Game): number => {
   let active = 0;
   for (let color in game.players) {
-    if (!game.players[color].name) {
+    const p = game.players[color];
+    if (!p || !p.name) {
       continue;
     }
     active++;
@@ -1204,7 +1185,7 @@ const getActiveCount = (game: any): number => {
   return active;
 };
 
-const setPlayerColor = (game: any, session: any, color: string): string | undefined => {
+const setPlayerColor = (game: Game, session: Session, color: string): string | undefined => {
   /* Selecting the same color is a NO-OP */
   if (session.color === color) {
     return;
@@ -1225,8 +1206,14 @@ const setPlayerColor = (game: any, session: any, color: string): string | undefi
   }
 
   /* Verify selection is not already taken */
-  if (color && game.players[color].status !== "Not active") {
+  if (color) {
-    return `${game.players[color].name} already has ${colorToWord(color)}`;
+    const candidate = game.players[color];
+    if (!candidate) {
+      return `An invalid player selection was attempted.`;
+    }
+    if (candidate.status !== "Not active") {
+      return `${candidate.name} already has ${colorToWord(color)}`;
+    }
   }
 
   let active = getActiveCount(game);
@@ -1234,14 +1221,17 @@ const setPlayerColor = (game: any, session: any, color: string): string | undefi
   if (session.player) {
     /* Deselect currently active player for this session */
     clearPlayer(session.player);
-    session.player = undefined;
+    // remove the player association
+    delete (session as any).player;
     const old_color = session.color;
     session.color = "";
     active--;
 
     /* If the player is not selecting a color, then return */
     if (!color) {
-      addChatMessage(game, null, `${session.name} is no longer ${colorToWord(old_color)}.`);
+      const msg = String(session.name || "") + " is no longer " + String(colorToWord(String(old_color)));
+      addChatMessage(game, null, msg);
+  if (!game.unselected) game.unselected = [] as any[];
       game.unselected.push(session);
       game.active = active;
       if (active === 1) {
@@ -1267,11 +1257,14 @@ const setPlayerColor = (game: any, session: any, color: string): string | undefi
   active++;
   session.color = color;
   session.live = true;
-  session.player = game.players[color];
+  const picked = game.players[color];
-  session.player.name = session.name;
+  if (picked) {
-  session.player.status = `Active`;
+    (session as any).player = picked;
-  session.player.lastActive = Date.now();
+    picked.name = session.name;
-  session.player.live = true;
+    picked.status = `Active`;
+    picked.lastActive = Date.now();
+    picked.live = true;
+  }
   addChatMessage(game, session, `${session.name} has chosen to play as ${colorToWord(color)}.`);
 
   const update: any = {
@@ -1282,15 +1275,19 @@ const setPlayerColor = (game: any, session: any, color: string): string | undefi
   /* Rebuild the unselected list */
   const unselected = [];
   for (let id in game.sessions) {
-    if (!game.sessions[id].color && game.sessions[id].name) {
+    const s = game.sessions[id];
-      unselected.push(game.sessions[id]);
+    if (!s) continue;
+    if (!s.color && s.name) {
+      unselected.push(s);
     }
   }
+  if (!game.unselected) game.unselected = [] as any[];
   if (unselected.length !== game.unselected.length) {
     game.unselected = unselected;
     update.unselected = getFilteredUnselected(game);
   }
 
+  if (!game.active) game.active = 0;
   if (game.active !== active) {
     if (game.active < 2 && active >= 2) {
       addChatMessage(game, null, `There are now enough players to start the game.`);
@@ -1794,8 +1791,6 @@ const offerToString = (offer: any): string => {
   );
 };
 
- 
-
 router.put("/:id/:action/:value?", async (req, res) => {
   const { action, id } = req.params,
     value = req.params.value ? req.params.value : "";
@@ -2140,13 +2135,18 @@ const pass = (game: any, session: any): string | undefined => {
   }
 
   const next = getNextPlayerSession(game, session.name);
+  if (!next) {
+    return `Unable to find the next player to pass to.`;
+  }
   session.player.totalTime += Date.now() - session.player.turnStart;
   session.player.turnNotice = "";
   game.turn = {
     name: next.name,
     color: next.color,
   };
-  next.player.turnStart = Date.now();
+  if (next.player) {
+    (next.player as any)["turnStart"] = Date.now();
+  }
   startTurnTimer(game, next);
   game.turns++;
   addActivity(game, session, `${name} passed their turn.`);
@@ -2528,8 +2528,9 @@ const playCard = (game: any, session: any, card: any): string | undefined => {
   return undefined;
 };
 
-const placeSettlement = (game: any, session: any, index: any): string | undefined => {
+const placeSettlement = (game: Game, session: Session, index: number | string): string | undefined => {
-  const player = session.player;
+  if (!session.player) return `You are not playing a player.`;
+  const player: any = session.player;
   if (typeof index === "string") index = parseInt(index);
 
   if (game.state !== "initial-placement" && game.state !== "normal") {
@@ -2560,26 +2561,26 @@ const placeSettlement = (game: any, session: any, index: any): string | undefine
 
   if (game.state === "normal") {
     if (!game.turn.free) {
-      if (player.brick < 1 || player.wood < 1 || player.wheat < 1 || player.sheep < 1) {
+      if ((player.brick || 0) < 1 || (player.wood || 0) < 1 || (player.wheat || 0) < 1 || (player.sheep || 0) < 1) {
         return `You have insufficient resources to build a settlement.`;
       }
     }
 
-    if (player.settlements < 1) {
+    if ((player.settlements || 0) < 1) {
       return `You have already built all of your settlements.`;
     }
 
-    player.settlements--;
+    player.settlements = (player.settlements || 0) - 1;
 
     if (!game.turn.free) {
       addChatMessage(game, session, `${session.name} spent 1 brick, 1 wood, 1 sheep, 1 wheat to purchase a settlement.`);
-      player.brick--;
+      player.brick = (player.brick || 0) - 1;
-      player.wood--;
+      player.wood = (player.wood || 0) - 1;
-      player.wheat--;
+      player.wheat = (player.wheat || 0) - 1;
-      player.sheep--;
+      player.sheep = (player.sheep || 0) - 1;
       player.resources = 0;
       ["wheat", "brick", "sheep", "stone", "wood"].forEach((resource) => {
-        player.resources += player[resource];
+        player.resources += player[resource] || 0;
       });
     }
     delete game.turn.free;
@@ -2648,7 +2649,7 @@ const placeSettlement = (game: any, session: any, index: any): string | undefine
         player.ports++;
       });
     }
-    player.settlements--;
+  player.settlements = (player.settlements || 0) - 1;
     if (bankType) {
       addActivity(
         game,
@@ -5140,6 +5141,17 @@ const createGame = async (id: any) => {
     },
     sessions: {},
     unselected: [],
+    placements: {
+      corners: [],
+      roads: [],
+    },
+    turn: {
+      name: "",
+      color: "",
+      actions: [],
+      limits: {},
+      roll: 0,
+    },
     rules: {
       "victory-points": {
         points: 10,

server/routes/games/helpers.ts

@@ -1,4 +1,5 @@
 import type { Game, Session, Player } from "./types";
+import { newPlayer } from "./playerFactory";
 
 export const addActivity = (game: Game, session: Session | null, message: string): void => {
   let date = Date.now();
@@ -138,32 +139,8 @@ export const clearPlayer = (player: Player) => {
     delete (player as any)[key];
   }
 
-  // Inline minimal newPlayer factory to avoid circular import at runtime
+  // Use shared factory to ensure a single source of defaults
-  const base = {
+  Object.assign(player, newPlayer(color || ""));
-    roads: 15,
-    cities: 4,
-    settlements: 5,
-    points: 0,
-    status: "Not active",
-    lastActive: 0,
-    resources: 0,
-    order: 0,
-    stone: 0,
-    wheat: 0,
-    sheep: 0,
-    wood: 0,
-    brick: 0,
-    army: 0,
-    development: [],
-    color: color,
-    name: "",
-    totalTime: 0,
-    turnStart: 0,
-    ports: 0,
-    developmentCards: 0,
-  } as Player;
-
-  Object.assign(player, base);
 };
 
 export const canGiveBuilding = (game: Game): string | undefined => {

server/routes/games/playerFactory.ts

@@ -0,0 +1,30 @@
+import { MAX_ROADS, MAX_CITIES, MAX_SETTLEMENTS } from "./constants";
+import type { Player } from "./types";
+
+export const newPlayer = (color: string): Player => {
+  return {
+    roads: MAX_ROADS,
+    cities: MAX_CITIES,
+    settlements: MAX_SETTLEMENTS,
+    points: 0,
+    status: "Not active",
+    lastActive: 0,
+    resources: 0,
+    order: 0,
+    stone: 0,
+    wheat: 0,
+    sheep: 0,
+    wood: 0,
+    brick: 0,
+    army: 0,
+    development: [],
+    color: color,
+    name: "",
+    totalTime: 0,
+    turnStart: 0,
+    ports: 0,
+    developmentCards: 0,
+  } as Player;
+};
+
+export default newPlayer;