Skip to content

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

Open
angelTomo9 wants to merge 7 commits into
Bitcoindefi:mainfrom
angelTomo9:feat-world-builder-graphics-palette-1787657711522
Open

feat(world-builder): register uploaded graphics and extend palette schemas (#6)#130
angelTomo9 wants to merge 7 commits into
Bitcoindefi:mainfrom
angelTomo9:feat-world-builder-graphics-palette-1787657711522

Conversation

@angelTomo9

Copy link
Copy Markdown

Closes #6

Summary of Changes

Bridges uploaded custom PNG assets with the game engine runtime, allowing uploaded graphics to be resolved by client renderers and assigned into dynamic map palettes.

Features & Deliverables

  • Reserved Non-Colliding ID Range: Uploaded graphics use dedicated index space >= 1,000,000, preventing collision with legacy original engine graphics (1..320,151).
  • Graphic Metadata Resolution Endpoint: Added GET /graphics/:grhIndex/metadata returning frame count, dimension, offset, and URL metadata for engine compatibility.
  • Dynamic Palette Overrides: Added game_map_palette_overrides table in api/schema.sql and endpoints PUT /admin/game-data/maps/:mapNum/palette & GET /admin/game-data/maps/:mapNum/palette.
  • Defensive Validation: Strict validation rejects references to unuploaded or out-of-range graphic indexes before they can corrupt tile rendering.
  • Integration Tests: Added unit & integration test coverage in api/src/tests/worldBuilder_palette.integration.test.ts.

Comment thread api/src/server.ts
Comment thread api/src/repositories/worldBuilder.ts Outdated
@gitar-bot

gitar-bot Bot commented Aug 25, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 2 resolved / 2 findings

Registers uploaded custom PNG assets and extends palette schemas for the world builder, addressing missing authentication on the palette endpoint and race conditions in upsertPaletteEntry.

✅ 2 resolved
Security: GET /admin/game-data/maps/:mapNum/palette lacks authentication

📄 api/src/server.ts:962-976
Unlike every sibling admin map endpoint (PUT palette, tiles, publish, discard, revert, permissions), this GET handler never calls requireMapEditSession or requireAdminEmailSession, and there is no global admin auth middleware (confirmed: app.use only does JSON parsing, logging, CORS). Any unauthenticated client can enumerate a map's dynamic palette overrides, including uploaded graphic index assignments. Add an auth guard, e.g. call const authorized = await requireMapEditSession(request, response, mapNum); if (!authorized) return; (or requireAdminEmailSession) before listMapPalette.

Bug: Concurrent upsertPaletteEntry can overwrite existing palette rows

📄 api/src/repositories/worldBuilder.ts:619-633
When entry.paletteId is omitted, the next id is derived from SELECT MAX(palette_id)+1. Two concurrent requests for the same mapNum compute the same next_id; the first INSERT succeeds and the second hits ON CONFLICT (map_num, palette_id) DO UPDATE, silently overwriting the just-created entry instead of allocating a new id. Allocate the id atomically (e.g. a per-map sequence, a SERIAL/IDENTITY column, or wrapping the select+insert in a transaction with row locking) so concurrent new entries don't clobber each other.

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 7 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

Was this helpful? React with 👍 / 👎 | Gitar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Etapa 1: registrar PNG subidos como graficos del motor y extender la paleta

1 participant