Skip to content

feat(map-tiles): rectangle paint, region query, and isolation detection APIs - #139

Open
JemimahEkong wants to merge 1 commit into
Bitcoindefi:mainfrom
JemimahEkong:feature/map-tile-paint-api
Open

feat(map-tiles): rectangle paint, region query, and isolation detection APIs#139
JemimahEkong wants to merge 1 commit into
Bitcoindefi:mainfrom
JemimahEkong:feature/map-tile-paint-api

Conversation

@JemimahEkong

Copy link
Copy Markdown

feat(map-tiles): rectangle paint, region query & isolation detection APIs

Closes #7

What

Three new admin endpoints for map tile editing:

  • POST /admin/game-data/maps/:mapNum/tiles/paint-rectangle — Paint a filled rectangle of tiles in a single atomic operation (max 500 tiles, auto-swaps min/max coordinates)
  • GET /admin/game-data/maps/:mapNum/tiles/region?startX=..&startY=..&endX=..&endY=.. — Query all tile overrides within a bounding box (drafts take priority over published)
  • POST /admin/game-data/maps/:mapNum/tiles/blocked-check — Run an in-memory BFS from the first walkable tile to detect isolated unreachable regions

The existing PUT /admin/game-data/maps/:mapNum/tiles and the new rectangle endpoint both return an isolatedRegions boolean in the response when any tile in the batch sets blocked: true/false, giving immediate feedback on whether the edit fractured the map.

Why

The current tile-painting workflow requires one HTTP request per tile. For large edits (walls, terrain zones, blocked corridors) this is slow and error-prone. The rectangle endpoint lets editors paint an entire area in one call. The isolation check prevents accidentally sealing off parts of the map from players — a bug that's only discoverable by walking every path manually.

Changes

File What
api/src/repositories/worldBuilder.ts paintRectangle(), queryRegion(), checkIsolatedRegions() + Zod schemas
api/src/server.ts 3 new routes, enhanced PUT tiles response
api/src/tests/map-tiles.integration.test.ts 12 integration tests
.github/workflows/ci.yml Admin auth env vars for integration tests

Implementation details

  • Rectangle paint expands coordinates into a TilePaint[] array and delegates to the existing paintTiles() — no new transaction logic, full atomicity via BEGIN/COMMIT/ROLLBACK.
  • Region query uses DISTINCT ON (x, y, layer) with status ASC ordering so drafts override published entries for the same coordinate.
  • Isolation detection loads terrain.json from disk, applies all overrides (drafts preferred via queryRegion), then runs a 4-directional BFS in memory. If any walkable tile is unreachable from the first walkable tile found, isolated: true is returned with the exact count of unreachable tiles.
  • The blocked-check endpoint operates on the full 100×100 map. The BFS processes at most 10,000 cells — negligible cost.

Tests

# Case Validates
1 Paint single tile Basic override persistence
2 Paint 20×20 rectangle 400 tiles applied atomically
3 Out-of-range coordinates (×4) Returns 400, no data corruption
4 Invalid graphic index Returns 400 with graphic ID in error
5 Blocked flag persists Region query confirms blocked state
6 Failed batch rollback Partial write doesn't leak
7 Region query bounds filtering Tiles outside region excluded
8 Rectangle with blocked: true Response includes isolatedRegions
9 blocked-check on clean map Returns valid result
10 blocked-check after ring paint Detects isolated interior (40-tile ring at center)
11 Array size limit (501 tiles) Rejects with 400
12 Rectangle size limit (25×25 = 625) Rejects with 400

Out of scope

  • No changes to server/ (game server) — server-side loading of published tile overrides for movement validation is a separate issue, since the game server currently reads base maps from terrain.json at startup rather than syncing overrides from the API.
  • No changes to frontend/ — the client already consumes GET /maps/:mapNum/overrides.
  • No database schema changes — the existing game_map_tile_overrides table is sufficient.

Verification

  • Full existing test suite passes alongside the 12 new integration tests.
  • Lint/typecheck clean, no new warnings.
  • CI workflow updated with the admin auth env vars required for the new integration tests to run in CI.

Acceptance criteria

  • Painting a tile changes what the player sees after reloading the map (via override persistence + region query)
  • Painting a 20×20 rectangle is a single atomic operation
  • Out-of-range coordinates return 400 without corrupting data
  • A tile marked as blocked persists correctly at the API layer (end-to-end server-side movement enforcement is tracked separately, per the out-of-scope note above)
  • A failed operation leaves the map exactly as it was (transaction rollback verified by test Etapa 1: registrar PNG subidos como graficos del motor y extender la paleta #6)
  • Integration tests included (12 total)

…on APIs

- POST /admin/game-data/maps/:mapNum/tiles/paint-rectangle: atomic rectangle paint with 500-tile limit
- GET /admin/game-data/maps/:mapNum/tiles/region: query overrides within a bounding box
- POST /admin/game-data/maps/:mapNum/tiles/blocked-check: BFS-based isolated region detection
- Enhanced PUT tiles and rectangle paint to return isolatedRegions when blocked flag changes
- Added 12 integration tests covering paint, validation, rollback, region query, isolation detection, and size limits
- Added CI env vars for admin auth in integration tests
Comment on lines +594 to +608
export async function checkIsolatedRegions(
mapNum: number,
): Promise<{ isolated: boolean; unreachableCount: number }> {
const grid = loadBaseBlockedTiles(mapNum);

if (!grid) {
return { isolated: false, unreachableCount: 0 };
}

const height = grid.length;
const width = grid[0]!.length;

const overrides = await queryRegion(mapNum, {
startX: 1,
startY: 1,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Bug: Isolation check reports false positives on naturally-disconnected maps

checkIsolatedRegions seeds a BFS from the first walkable tile and reports isolated=true whenever total walkable tiles exceed those reachable from that single seed. But base maps already contain multiple disconnected walkable regions — running this on the clean, unedited mapa_1/terrain.json yields 15 walkable components, 6118 walkable tiles, and unreachableCount=368 with isolated=true. So every PUT/paint-rectangle edit that sets blocked will return isolatedRegions=true regardless of whether the edit actually fractured the map, defeating the feature's purpose and misleading editors. Test #9 only asserts the return type, not that a clean map yields isolated=false, so this slips through. Fix by comparing connectivity against a baseline computed on the pre-edit grid (e.g. count connected components before vs. after, or only report regions newly isolated by this edit) rather than comparing a single seed's reach to the global walkable total.

Was this helpful? React with 👍 / 👎

@gitar-bot

gitar-bot Bot commented Aug 25, 2026

Copy link
Copy Markdown
Code Review ⚠️ Changes requested 0 resolved / 2 findings

Adds rectangle paint, region query, and isolation detection admin APIs, but the isolation check reports false positives on naturally-disconnected maps and synchronous file I/O on the request path impacts performance.

⚠️ Bug: Isolation check reports false positives on naturally-disconnected maps

📄 api/src/repositories/worldBuilder.ts:594-608 📄 api/src/server.ts:880-888 📄 api/src/server.ts:957-964

checkIsolatedRegions seeds a BFS from the first walkable tile and reports isolated=true whenever total walkable tiles exceed those reachable from that single seed. But base maps already contain multiple disconnected walkable regions — running this on the clean, unedited mapa_1/terrain.json yields 15 walkable components, 6118 walkable tiles, and unreachableCount=368 with isolated=true. So every PUT/paint-rectangle edit that sets blocked will return isolatedRegions=true regardless of whether the edit actually fractured the map, defeating the feature's purpose and misleading editors. Test #9 only asserts the return type, not that a clean map yields isolated=false, so this slips through. Fix by comparing connectivity against a baseline computed on the pre-edit grid (e.g. count connected components before vs. after, or only report regions newly isolated by this edit) rather than comparing a single seed's reach to the global walkable total.

💡 Performance: BFS uses array.shift() and synchronous file I/O on request path

📄 api/src/repositories/worldBuilder.ts:551-565 📄 api/src/repositories/worldBuilder.ts:656-670

checkIsolatedRegions runs on every blocked-flag edit (PUT tiles and paint-rectangle). It uses queue.shift() which is O(n) per dequeue, making the BFS O(n^2) on up to 10,000 cells, and loadBaseBlockedTiles calls fs.existsSync/fs.readFileSync + JSON.parse synchronously, blocking the event loop for the whole file read on each such request. Both are functionally fine at 100x100 but avoidable: use an index-based queue head pointer (or a deque) instead of shift(), and use fs.promises.readFile to avoid blocking. Optionally cache parsed terrain per map since terrain.json is static.

🤖 Prompt for agents
Code Review: Adds rectangle paint, region query, and isolation detection admin APIs, but the isolation check reports false positives on naturally-disconnected maps and synchronous file I/O on the request path impacts performance.

1. ⚠️ Bug: Isolation check reports false positives on naturally-disconnected maps
   Files: api/src/repositories/worldBuilder.ts:594-608, api/src/server.ts:880-888, api/src/server.ts:957-964

   checkIsolatedRegions seeds a BFS from the first walkable tile and reports isolated=true whenever total walkable tiles exceed those reachable from that single seed. But base maps already contain multiple disconnected walkable regions — running this on the clean, unedited mapa_1/terrain.json yields 15 walkable components, 6118 walkable tiles, and unreachableCount=368 with isolated=true. So every PUT/paint-rectangle edit that sets blocked will return isolatedRegions=true regardless of whether the edit actually fractured the map, defeating the feature's purpose and misleading editors. Test #9 only asserts the return type, not that a clean map yields isolated=false, so this slips through. Fix by comparing connectivity against a baseline computed on the pre-edit grid (e.g. count connected components before vs. after, or only report regions newly isolated by this edit) rather than comparing a single seed's reach to the global walkable total.

2. 💡 Performance: BFS uses array.shift() and synchronous file I/O on request path
   Files: api/src/repositories/worldBuilder.ts:551-565, api/src/repositories/worldBuilder.ts:656-670

   checkIsolatedRegions runs on every blocked-flag edit (PUT tiles and paint-rectangle). It uses queue.shift() which is O(n) per dequeue, making the BFS O(n^2) on up to 10,000 cells, and loadBaseBlockedTiles calls fs.existsSync/fs.readFileSync + JSON.parse synchronously, blocking the event loop for the whole file read on each such request. Both are functionally fine at 100x100 but avoidable: use an index-based queue head pointer (or a deque) instead of shift(), and use fs.promises.readFile to avoid blocking. Optionally cache parsed terrain per map since terrain.json is static.

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

Comment on lines +551 to +565
function loadBaseBlockedTiles(mapNum: number): boolean[][] | null {
const terrainPath = path.join(
MAPAS_SOURCE_DIR,
`mapa_${mapNum}`,
"terrain.json",
);

if (!fs.existsSync(terrainPath)) {
return null;
}

const terrain = JSON.parse(
fs.readFileSync(terrainPath, "utf8"),
) as TerrainJson;
const palette = terrain.palette ?? {};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Performance: BFS uses array.shift() and synchronous file I/O on request path

checkIsolatedRegions runs on every blocked-flag edit (PUT tiles and paint-rectangle). It uses queue.shift() which is O(n) per dequeue, making the BFS O(n^2) on up to 10,000 cells, and loadBaseBlockedTiles calls fs.existsSync/fs.readFileSync + JSON.parse synchronously, blocking the event loop for the whole file read on each such request. Both are functionally fine at 100x100 but avoidable: use an index-based queue head pointer (or a deque) instead of shift(), and use fs.promises.readFile to avoid blocking. Optionally cache parsed terrain per map since terrain.json is static.

Was this helpful? React with 👍 / 👎

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: API para pintar el piso del mapa

1 participant