Amt/server changes#5
Merged
Merged
Conversation
Owner
Author
|
LGTMMM WOOOO |
There was a problem hiding this comment.
Pull request overview
This PR advances BoxGame’s multi-server architecture by modernizing the game server (TypeScript), improving gateway/server-manager allocation flows, and adding container/Kubernetes deployment assets to support orchestrated deployments.
Changes:
- Migrates the game server to TypeScript and adds self-registration to server-manager.
- Updates gateway/server-manager room allocation and server assignment plumbing.
- Adds Dockerfiles and Kubernetes manifests to deploy services in-cluster.
Reviewed changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| server/server.ts | TypeScript migration, room/socket logic updates, and server-manager registration on startup. |
| server/package.json | Adds TypeScript and type packages to support the TS server build. |
| server/package-lock.json | Locks newly added TS/type dependencies. |
| server/Dockerfile | Multi-stage build to compile TS to JS, then run the server in a slimmer runtime image. |
| server/config.yaml | Example pod config for game-server env injection (HOST_IP / SERVER_MANAGER_URL). |
| server/bun.lock | Adds Bun lockfile for server dependencies (in addition to npm lock). |
| server-manager/routes/handleRegister.ts | Adjusts registration payload shape to match game-server’s flat JSON body. |
| server-manager/helper/heap.ts | Adds clarifying comments around heap operations. |
| server-manager/Dockerfile | Containerizes server-manager using Bun to run TS directly. |
| k8s.yaml | Adds manifests for namespaces, RBAC, deployments/services, and game-server pod template. |
| gateway/types/server.ts | Introduces a shared Server type for gateway/server-manager integration. |
| gateway/routes/createRoom.ts | Refactors room creation to request server assignment from server-manager and set cookies. |
| gateway/routes/addToRoom.ts | Implements join flow using the in-memory room map and sets connection cookies. |
| gateway/index.ts | Removes Redis usage and switches to an in-memory room mapping with health endpoint. |
| gateway/helper/generateRoom.ts | Adds helper to generate room codes. |
| gateway/Dockerfile | Containerizes gateway using Bun to run TS directly. |
Files not reviewed (1)
- server/package-lock.json: Generated file
Comments suppressed due to low confidence (6)
server/server.ts:177
Targetis declared twice in this module (interface Target extends Rectangle {}earlier andinterface Target { ... }here). TypeScript will error on duplicate identifiers, so this won’t compile.
server/server.ts:197Playeris declared twice in this module (interface Player extends Rectangle { ... }earlier and anotherinterface Player { ... }here). This causes a duplicate-identifier TypeScript compile error.
server/server.ts:466createRoomhandler usesrooms[code]without ever initializingrooms[code], so this will throw at runtime (Cannot read properties of undefined) and rooms will never be created.
server/server.ts:616/statusdoes not send any HTTP response, which will cause Kubernetes probes/clients to hang until they time out.
server/server.ts:632- The server always binds to an ephemeral port (
listen(0)) and unconditionally attempts registration. This breaks the “single-server” use case (can’t bind a known PORT), and ifSERVER_MANAGER_URL/HOST_IPare unset the startup will fail with an unhandled rejection.
server-manager/routes/handleRegister.ts:22 handleRegistertrustsreq.bodytypes; ifportarrives as a string (common with JSON clients) it will still pass the truthy check but store a non-numeric value in the heap. Normalizing/validating fields avoids subtle heap ordering and header-setting bugs later.
const server = req.body; // server.ts sends flat: { hostIp, port, connections }
if (!server.hostIp || !server.port) {
res.status(401).send("Invalid server info");
return;
}
if (!server.connections) {
server.connections = 0;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
13
to
21
| "dependencies": { | ||
| "@types/express": "^5.0.6", | ||
| "@types/socket.io": "^3.0.2", | ||
| "dotenv": "^17.3.1", | ||
| "express": "^5.2.1", | ||
| "nodemon": "^3.1.11", | ||
| "socket.io": "^4.8.3" | ||
| "socket.io": "^4.8.3", | ||
| "typescript": "^6.0.3" | ||
| } |
| // Kubernetes health probe | ||
| app.get("/healthz", (_, res) => res.status(200).send("ok")); | ||
| // Adds a client to a given room | ||
| app.get("/add", addToRoom); |
Comment on lines
+4
to
+17
| if (!req.body) { | ||
| res.status(400).send("No body"); | ||
| return; | ||
| } | ||
| const roomId = req.body.roomId; | ||
| if (roomId == undefined || roomMaps[roomId] == undefined) { | ||
| res.status(400).send("Invalid room id"); | ||
| return; | ||
| } | ||
|
|
||
| res.cookie("hostip", roomMaps[roomId]["hostIp"]); | ||
| res.cookie("port", roomMaps[roomId]["port"]); | ||
| res.status(200).send("here have your server"); | ||
| return; |
Comment on lines
+12
to
+17
| const serverManager = process.env.SERVER_MANAGER_URL; | ||
|
|
||
| try { | ||
| // Get assigned a server | ||
| const response = await fetch(`${serverManager}/assign`); | ||
|
|
| labels: | ||
| app: gateway | ||
| spec: | ||
| replicas: 2 |
Comment on lines
+318
to
+324
| readinessProbe: | ||
| httpGet: | ||
| path: /status | ||
| port: 6000 # initial probe port; actual port is random — remove if causing issues | ||
| initialDelaySeconds: 3 | ||
| periodSeconds: 10 | ||
| failureThreshold: 3 |
Comment on lines
+21
to
+22
| - name: SERVER_MANAGER_URL | ||
| value: http://localhost:4289 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces several improvements across the BoxGame infrastructure, including gateway updates, server manager enhancements, migration of the game server to TypeScript/Bun, and Kubernetes deployment support.
Changes
Gateway
addToRoom)Server Manager
Game Server
bun.lock)server.jsKubernetes
Why
These changes prepare the project for containerized and orchestrated deployments while improving the server allocation pipeline and modernizing the game server implementation.
Testing
Closes #4