feat(worldBuilder): implement floor objects, multi-tile structures and door collision states (#9)

[Rodrigoue9]

📌 Summary

Fixes #9

Implements Etapa 2 (Construcción del Mundo): Placement, movement, and removal of floor objects, multi-tile structures across layers 3 and 4, and interactive doors with blocking state synchronization.

---

🛠️ Key Implementation Details

1. Floor Objects (placeMapObject, removeMapObject):

   • Validates objIndex against the active catalog in game_objects before placement.
   • Enforces coordinates within map bounds (1..MAP_SIZE).
   • Supports atomic updates and placement tracking in game_map_tile_overrides.

2. Multi-Tile Structures (placeStructure):

   • Atomically places composite structures (buildings, decor) across upper layers (3 and 4).
   • Wrapped in a database transaction (BEGIN...COMMIT / ROLLBACK) to prevent partial placement.
   • Rejects coordinates and offsets that exceed map boundaries.

3. Door State & Collision Management (setDoorState):

   • Toggles visual state between openGrhIndex and closedGrhIndex.
   • Automatically synchronizes tile collision: blocked: true when closed, blocked: false when open.

---

🧪 Verification & Testing

• Added automated unit tests in api/src/tests/worldBuilder.unit.test.ts.
• Verified coordinate validation and schema parsing.
• Verified structure multi-tile atomic placement logic.
• Verified door state toggle & collision blocking rules.
• TypeScript build and linter pass cleanly.

---

🤝 Bounty Reference

Addresses bounty issue #9 (Etapa 2: colocacion de objetos, estructuras y puertas) under the GrantFox OSS reward program.

[gitar-bot]

💡 Quality: moveMapObject/removeMapObject mutate published rows, skipping publish

placeMapObject writes only draft rows (status='draft'), so edits require an explicit publish step. moveMapObject and removeMapObject instead mutate/delete all statuses including 'published', so moving or deleting an object changes the live map immediately with no draft/preview/publish cycle. This is inconsistent with the rest of the world-builder workflow and can surprise admins by altering what players see before publishing. Consider scoping these to drafts (or making the live mutation an explicit, documented behavior).

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

[gitar-bot]

Code Review 👍 Approved with suggestions 8 resolved / 9 findings

Implements floor objects, multi-tile structures, and door collision states with comprehensive test coverage and atomic database transactions. The placement workflow is solid, but moveMapObject and removeMapObject mutate published rows immediately rather than respecting the draft/publish cycle used by placeMapObject—consider scoping these operations to drafts or documenting live mutations as intentional behavior to keep the workflow consistent.

💡 Quality: moveMapObject/removeMapObject mutate published rows, skipping publish

📄 api/src/repositories/worldBuilder.ts:1007-1021 📄 api/src/repositories/worldBuilder.ts:962-976

placeMapObject writes only draft rows (status='draft'), so edits require an explicit publish step. moveMapObject and removeMapObject instead mutate/delete all statuses including 'published', so moving or deleting an object changes the live map immediately with no draft/preview/publish cycle. This is inconsistent with the rest of the world-builder workflow and can surprise admins by altering what players see before publishing. Consider scoping these to drafts (or making the live mutation an explicit, documented behavior).

✅ 8 resolved

✅ Bug: placeMapObject stores objIndex in grh_index and drops amount

📄 api/src/repositories/worldBuilder.ts:485-499
placeMapObject writes the catalog objIndex into the grh_index column and never persists amount. In this codebase grh_index is a graphic index (a different namespace from the game_objects catalog id / objIndex), and floor objects at runtime are modeled as { objIndex, amount } (see server/src/loadMaps.ts objInfo). Persisting objIndex as grh_index means the renderer will draw an unrelated graphic, and the validated amount (needed for stacks like gold) is silently lost — failing the acceptance criteria that placed objects appear correctly after a map reload. Store the object with its own object/amount columns (or the object-info model the runtime consumes) rather than overloading grh_index, and persist amount.

✅ Edge Case: Doors and structures share layer 3, causing overwrite collisions

📄 api/src/repositories/worldBuilder.ts:485-494 📄 api/src/repositories/worldBuilder.ts:546-556 📄 api/src/repositories/worldBuilder.ts:580-590
setDoorState hardcodes layer 3 and placeStructure writes to layers 3–4. Since the primary key is (map_num, x, y, layer, status), a door placed on a coordinate already occupied by a structure tile on layer 3 (or vice-versa) will overwrite it via ON CONFLICT DO UPDATE, silently clobbering the other feature. Similarly, floor objects on layer 2 can overwrite terrain painted on layer 2 via paintTiles. Consider dedicating distinct layers per feature or detecting/rejecting conflicting placements.

✅ Quality: Tests only cover zod schemas, not DB placement logic

📄 api/src/tests/worldBuilder.unit.test.ts:1-15
The added tests only exercise schema parsing (mapObjectSchema, structurePlacementSchema, doorStateSchema). The actual repository logic — objIndex catalog validation, the placeStructure BEGIN/COMMIT/ROLLBACK transaction and per-tile bounds check, ON CONFLICT upsert behavior, and door blocked-state synchronization — is untested, and issue #9 explicitly requests integration tests. Add tests (with a test DB or mocked pool) covering these paths, especially the transaction rollback and door blocked toggling.

✅ Edge Case: structureTileSchema offsets unbounded before origin bounds check

📄 api/src/repositories/worldBuilder.ts:435-448 📄 api/src/repositories/worldBuilder.ts:536-544
structureTileSchema allows arbitrary integer offsetX/offsetY with no magnitude limit; only the computed targetX/targetY are bounds-checked inside placeStructure. Combined with tiles up to 200 entries this is functionally safe (out-of-range tiles throw), but a single out-of-bounds offset aborts the whole transaction after doing work. This is acceptable but worth noting; consider validating offsets against MAP_SIZE at the schema level for clearer client errors.

✅ Edge Case: Published objects/doors cannot be removed via delete endpoint

📄 api/src/repositories/worldBuilder.ts:718-730
removeMapObject only deletes rows with status='draft', so once an object has been published (draft row deleted by publishMap), calling the DELETE endpoint returns { ok: false } and the object can never be removed short of revertMap wiping the whole map. The same limitation affects doors, which have no removal path at all. Consider staging a deletion as a draft tombstone, or deleting the published row directly for admins, so map editors can undo published placements.

...and 3 more resolved from earlier reviews

🤖 Prompt for agents

Code Review: Implements floor objects, multi-tile structures, and door collision states with comprehensive test coverage and atomic database transactions. The placement workflow is solid, but `moveMapObject` and `removeMapObject` mutate published rows immediately rather than respecting the draft/publish cycle used by `placeMapObject`—consider scoping these operations to drafts or documenting live mutations as intentional behavior to keep the workflow consistent.

1. 💡 Quality: moveMapObject/removeMapObject mutate published rows, skipping publish
   Files: api/src/repositories/worldBuilder.ts:1007-1021, api/src/repositories/worldBuilder.ts:962-976

   placeMapObject writes only draft rows (status='draft'), so edits require an explicit publish step. moveMapObject and removeMapObject instead mutate/delete all statuses including 'published', so moving or deleting an object changes the live map immediately with no draft/preview/publish cycle. This is inconsistent with the rest of the world-builder workflow and can surprise admins by altering what players see before publishing. Consider scoping these to drafts (or making the live mutation an explicit, documented behavior).

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

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