feat(world-builder): register uploaded graphics and extend palette schemas (#6)

[Rodrigoue9]

Summary

Resolves #6 by implementing paletteEntrySchema and validatePaletteEntry to validate multi-layer palette definitions, enforce non-colliding graphic index allocations (UPLOADED_GRAPHIC_INDEX_START = 1_000_000), and verify graphic existence across engine and uploaded assets.

Changes

• Implemented paletteEntrySchema and validatePaletteEntry in api/src/repositories/worldBuilder.ts.
• Added unit tests in api/src/repositories/__tests__/paletteValidation.test.ts.

Closes #6

---

Summary by Gitar

• Server Idle Liveness Fixes:
  • Excluded keepalive pings from AFK idle liveness reference calculations in server/src/server.ts.

_{This will update automatically on new commits.}

[gitar-bot]

Code Review ✅ Approved 4 resolved / 4 findings

Implements palette entry validation with paletteEntrySchema and validatePaletteEntry to enforce multi-layer definitions and non-colliding graphic index allocations starting at 1,000,000. Resolved validation gaps including untested validatePaletteEntry, missing engine graphic existence checks, dangling gracefulShutdown imports breaking the build, and ping packets preventing AFK idle detection. No issues found.

✅ 4 resolved

✅ Quality: validatePaletteEntry is never called and untested

📄 api/src/repositories/worldBuilder.ts:231-245
The PR's stated goal is to validate multi-layer palette definitions, but validatePaletteEntry is not imported or invoked by any route or caller in the codebase (only its definition exists), and the new test file only exercises paletteEntrySchema, never validatePaletteEntry. As a result no palette entry is actually validated against existing/uploaded graphics at runtime and the collision/existence logic has zero test coverage. Wire the function into the palette-writing route(s) and add a unit test covering the uploaded-index existence branch (mocking pool.query).

✅ Bug: Engine graphic indices are accepted without existence check

📄 api/src/repositories/worldBuilder.ts:234-248
The function's docstring claims it verifies graphics exist "originales o subidos", but for any positive index below UPLOADED_GRAPHIC_INDEX_START (1,000,000) it performs no validation at all — including indices between the real engine max (320151) and 1,000,000, which are non-existent yet accepted as valid. This lets invalid engine indices pass validation. Either validate engine indices against the known upper bound (e.g. reject grhIndex > MAX_ENGINE_GRAPHIC_INDEX) or correct the docstring to reflect that only uploaded indices are checked.

✅ Bug: Dangling import of non-exported gracefulShutdown symbols breaks build

📄 server/src/server.ts:22-26 📄 server/src/server.ts:216
server/src/server.ts lines 22-26 import gracefulShutdown, GracefulShutdownDependencies, and ShutdownClient from ./gracefulShutdown, but that module only exports ShutdownSignal, withTimeout, and createGracefulShutdown — so these are missing exports (TS2305). Worse, the imported name gracefulShutdown collides with the local const gracefulShutdown = createGracefulShutdown(...) declared at line 216 (cannot redeclare block-scoped variable). This is a leftover from the merge that deleted the old shutdown handlers; the code using these symbols was removed. Delete the entire import block at lines 22-26 to fix compilation.

✅ Bug: Ping packets keep AFK players from ever being idle-kicked

📄 server/src/server.ts:437-441 📄 server/src/server.ts:795-807 📄 server/src/server.ts:819-825
trackClientActivity deliberately stores keepalive pings in lastPingAt (not lastActivityAt) with a comment stating keepalive traffic must not contaminate AFK/gameplay metrics. However, the non-scout idle path uses getClientLivenessReferenceAt, which returns Math.max(lastActivityAt, lastPingAt, connectedAt), so lastPingAt is folded right back into the idle-timeout comparison. Any client that emits automatic keepalive pings will have its idle reference refreshed every ping and can never be disconnected by processIdleCharactersTick, directly defeating the AFK-kick behavior the comment intends. If the intent is to detect only dead transport, drop lastPingAt from the reference; if it is to detect AFK players, exclude ping from liveness as the comment says.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

---

Important

Your trial ends in 1 day — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

_{Was this helpful? React with 👍 / 👎 | Gitar}

[Rodrigoue9]

Addressed the liveness review finding in commit 09c069b:

• Updated getClientLivenessReferenceAt in server/src/server.ts to exclude lastPingAt from the AFK idle reference, ensuring transport keepalive pings no longer prevent idle characters from being disconnected.

All CI checks passing and ready for merge! 🚀