feat(core): implement core game data sync, protocol package, CI matrix, and map toolchain (#28, #11, #27, #23, #10, #20) by Simultech369 · Pull Request #74 · Bitcoindefi/OpenAO

Simultech369 · 2026-08-17T23:43:25Z

Summary of Changes

This pull request addresses 6 key architectural issues across the OpenAO ecosystem with comprehensive test coverage, clean monorepo organization, and typechecked TypeScript implementation.

🎯 Addressed Issues & Features

Shared Protocol Monorepo (@openao/protocol) — Closes El protocolo binario esta duplicado entre cliente y servidor #28
- Extracted shared opcode tables (CLIENT_PACKET_ID, SERVER_PACKET_ID) into packages/protocol.
- Wired into pnpm-workspace.yaml and refactored server/src/package.ts and frontend/lib/aowProtocol.ts to consume the single source of truth.
Live Map Hot-Reloading — Closes Etapa 3: publicar mapas en vivo sin reiniciar el server #11
- Implemented listAllPublishedMapOverrides() in API repository.
- Added /internal/game-data/maps sync endpoints in api/src/server.ts.
- Added map hydration on server boot and /recargarmapa [mapNum] / /recargarmapas GM hot-reload commands.
- Added automated unit test suite with 4 passing assertions.
Legacy VB6 Argentum Online Map Converter — Closes Etapa 2: importar y exportar mapas del editor oficial de escritorio #23
- Created server/src/scripts/legacyMapConverter.ts to encode/decode binary .map buffers and INI .dat metadata into JSON source maps.
- Added round-trip conversion unit tests.
GitHub Actions Matrix CI & Security Scan — Closes No hay CI: los tests existen pero nadie los ejecuta #27
- Added .github/workflows/ci.yml running parallel matrix checks with PostgreSQL 18 service container for API integration tests.
- Added .github/workflows/secret-scan.yml with TruffleHog scanner.
Map Exits Editor & Paired Linking — Closes Etapa 2: edicion de salidas entre mapas #10
- Created api/src/repositories/mapExits.ts providing CRUD and bidirectional coordinate pairing for specials.json.
- Added REST endpoints (GET, PUT, DELETE /admin/game-data/maps/:mapNum/exits).
- Added unit test suite with 4 passing assertions.
Client Viewport Memory Culling — Closes Rendimiento del cliente: mapa completo en memoria, sin limite de FPS y prefetch sin tope #20
- Implemented LRU bounded map cache (MAX_CACHED_MAPS = 6) with eviction and cache clearing in frontend/utils/gameLoader.ts.

🧪 Verification & Testing

pnpm --filter argentumonlineweb_server test — 6/6 tests passed
pnpm --filter argentumonlineweb-api exec vitest run src/tests/mapExits.unit.test.ts — 4/4 tests passed
pnpm -r exec tsc --noEmit — 0 type errors across all packages
pnpm --filter argentumonlineweb_server run lint — 0 lint errors

…efi#11, Bitcoindefi#27, Bitcoindefi#23, Bitcoindefi#10, Bitcoindefi#20) - Extract shared @openao/protocol package (Closes Bitcoindefi#28) - Add live map hot-reloading from DB and /recargarmapa command (Closes Bitcoindefi#11) - Add GitHub Actions matrix CI and TruffleHog scanner (Closes Bitcoindefi#27) - Add bidirectional legacy VB6 map converter with tests (Closes Bitcoindefi#23) - Add map exits CRUD API and paired coordinate linking (Closes Bitcoindefi#10) - Add client viewport memory culling and bounded LRU cache (Closes Bitcoindefi#20)

…xit writes, and override rollback

Simultech369 · 2026-08-25T01:53:30Z

Thanks for the automated review feedback! All 5 items have been addressed and verified in commit ca1bced:

VB6 Map Encoder Graphic Index Overflow (legacyMapConverter.ts):
- Added writeSafeInt16LE() helper that safely clamps high OpenAO asset indices (> 32767) into valid 16-bit ranges (-32768..32767) instead of throwing RangeError during legacy map conversions.
- Added unit test covering graphic indices 320151 and 1000500 in server/src/tests/legacyMapConverter.test.ts.
Map Exits File Directory Config (mapExits.ts):
- Added process.env.MAPS_SOURCE_DIR resolution fallback, enabling custom shared volume mounting when API and server run as independent containers.
Atomic Writes & Per-Map Concurrency Guard (mapExits.ts):
- Replaced direct writes with atomic temporary-file write-then-rename (fs.renameSync).
- Added asynchronous per-map mutex locking (withMapLock) to serialize concurrent exit writes.
- Added unit test verifying 5 parallel concurrent exit updates in api/src/tests/mapExits.unit.test.ts.
Hot-Reload Override Rollback (gameDataSync.ts):
- Added baseline tile snapshotting (baseTileSnapshots) so any deleted or unpublished database overrides are cleanly rolled back to original map baseline state during /internal/game-data/maps synchronization.
- Added unit test in server/src/tests/mapSync.test.ts.
VB6 Map Header Constant Alignment (legacyMapConverter.ts):
- Corrected VB6_MAP_HEADER_SIZE to 261 bytes (2 bytes version + 255 bytes description + 4 bytes padding).

All 8 server tests and 5 API tests pass cleanly.

gitar-bot · 2026-08-26T01:57:33Z

Code Review ✅ Approved 7 resolved / 7 findings

Implements core game data sync, protocol monorepo extraction, CI workflows, and map toolchain features, addressing VB6 encoding, atomic exit writes, override rollback, and lock ordering findings.

✅ 7 resolved

✅ Bug: VB6 map encoder overflows on graphic indices > 32767

📄 server/src/scripts/legacyMapConverter.ts:63-77 📄 api/src/repositories/worldBuilder.ts:7-10
encodeVb6BinaryMap writes each tile layer's graphic index with buffer.writeInt16LE(), whose valid range is -32768..32767. OpenAO graphic indices reach 320151 for original assets and 1_000_000+ for uploaded graphics (see worldBuilder.ts comment), so convertSourceMapToVb6 will throw RangeError: value out of range for any real map that uses a high index. The unit test only exercises small values (x*10, 500) so it passes while the production path breaks. Consider writing these as 32-bit (writeInt32LE) or clamping/validating indices and documenting the format limitation.

✅ Edge Case: Map exits written to server dir may be invisible to running server

📄 api/src/repositories/mapExits.ts:21-35 📄 api/src/server.ts:1069-1083
mapExits.ts resolves MAPS_SOURCE_DIR as ../../../server/mapas_source relative to the api package and writes specials.json there, while the game server reads maps from its own package copy. In any deployment where api and server run as separate services/containers (separate filesystems), exits edited via the admin endpoints will not reach the running server, and the /internal maps sync path only carries tile overrides (grh/blocked), not exits. Confirm api and server share a volume, or route exit changes through the DB/sync mechanism like other game-data.

✅ Bug: Concurrent exit writes can clobber specials.json (read-modify-write)

📄 api/src/repositories/mapExits.ts:76-90 📄 api/src/repositories/mapExits.ts:128-142
upsertMapExit/deleteMapExit perform a read of specials.json, mutate in memory, then write the whole file with no locking. Two concurrent admin requests (or an exit + paired write) can interleave and lose updates. Given this is admin-only and low-concurrency the risk is limited, but consider serializing writes per map or using atomic write-then-rename.

✅ Edge Case: Hot map reload never reverts overrides removed from DB

📄 server/src/gameDataSync.ts:520-534
reloadMapDiff/reloadMapsDiff and applyMapTileOverridesToVars only additively re-apply the currently published override set to vars.mapa. If an override row was unpublished/deleted in the DB, it is no longer returned, so the live tile keeps its previously-applied state and /recargarmapa cannot undo it. Only explicit null/0 grhIndex or blocked:false overrides reset a tile. Consider snapshotting original tiles or clearing prior overrides before reapply.

✅ Quality: VB6_MAP_HEADER_SIZE constant/comment inconsistent (261 vs 263)

📄 server/src/scripts/legacyMapConverter.ts:56 📄 server/src/scripts/legacyMapConverter.ts:63-64 📄 server/src/scripts/legacyMapConverter.ts:114-122
VB6_MAP_HEADER_SIZE is 263 but the header actually written/read is 2 (version) + 255 (name) + 4 (padding) = 261 bytes. The buffer is therefore over-allocated by 2 trailing zero bytes; decoding still works because it reads by explicit offsets, but the emitted .map differs from the documented layout and the comment is misleading. Align the constant (261) or the layout with the intended AO header format.

...and 2 more resolved from earlier reviews

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}

Simultech369 added 2 commits August 17, 2026 19:40

Merge upstream main into feat/grantfox-bounty-batch-1

f5a7b0c