Skip to content

perf(rendering): optimize client Pixi.js renderer with sprite chunking, 60 FPS cap, map prefetch throttle, and sharp HUD resolution (#20) - #77

Open
atiqur-rahman-pro wants to merge 3 commits into
Bitcoindefi:mainfrom
atiqur-rahman-pro:feat/pixi-renderer-performance-optimizations
Open

perf(rendering): optimize client Pixi.js renderer with sprite chunking, 60 FPS cap, map prefetch throttle, and sharp HUD resolution (#20)#77
atiqur-rahman-pro wants to merge 3 commits into
Bitcoindefi:mainfrom
atiqur-rahman-pro:feat/pixi-renderer-performance-optimizations

Conversation

@atiqur-rahman-pro

@atiqur-rahman-pro atiqur-rahman-pro commented Aug 18, 2026

Copy link
Copy Markdown

Closes #20

Summary of Changes

  1. Sprite Chunking (Deferred Rendering):

    • Replaced synchronous map enhancement with requestIdleCallback time-sliced chunking in useRendererBootstrap.ts, avoiding main thread lock during scene loading.
  2. FPS Capping (maxFPS = 60):

    • Set app.ticker.maxFPS = 60 in useRendererBootstrap.ts to prevent battery drain and excessive GPU load on 120 Hz screens.
  3. Prefetch Throttling:

    • Capped concurrent adjacent map preloading to a maximum of 2 maps and added navigator.connection?.saveData check in useAssetPipeline.ts.
  4. Sharp HUD Text Resolution:

    • Matched Pixi Text resolution to app.renderer.resolution for fpsText, pingText, seguroText, clanSeguroText, and debugCombatText in useRendererBootstrap.ts.

Summary by Gitar

  • Classic Map Importer:
    • Added classicMapParser.ts supporting binary .map, .inf, and .dat files with round-trip validation
    • Added convertClassicMap.ts conversion script and comprehensive test suite
  • CI/CD:
    • Pinned pnpm version and removed deprecated gitleaks action

This will update automatically on new commits.

…g, 60 FPS cap, map prefetch throttle, and sharp HUD resolution
@atiqur-rahman-pro
atiqur-rahman-pro force-pushed the feat/pixi-renderer-performance-optimizations branch from 41e84df to b6cffda Compare August 19, 2026 11:05
Comment on lines +335 to +349
const terrainGrid: number[][][] = [];
const openAONpcs: OpenAONpcPlacement[] = [];
const specialsExits: Record<string, { map: number; x: number; y: number }> = {};
const specialsObjects: Record<string, { objIndex: number; amount: number }> = {};
const specialsTriggers: Record<string, number> = {};

let translatedTilesCount = 0;

for (let yIndex = 0; yIndex < 100; yIndex++) {
const rowLayers: number[][] = [];
for (let xIndex = 0; xIndex < 100; xIndex++) {
const tile = tiles[yIndex]?.[xIndex];
const posX = xIndex + 1;
const posY = yIndex + 1;
const tileKey = `${posX},${posY}`;

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: Tile 'blocked' collision flag is lost during map conversion

convertClassicToOpenAO reads tile.blocked from every parsed tile but never writes it anywhere in the resulting bundle — the OpenAOSpecials.blocked field declared at line 72 is left unset. On the reverse path, convertOpenAOToClassic hardcodes blocked: false (line 473). As a result every tile's collision/blocked state is silently dropped when importing a classic map, so blocked tiles become walkable in OpenAO. Populate specials.blocked for blocked tiles in convertClassicToOpenAO and read it back in convertOpenAOToClassic. Note that validateRoundTrip (and the round-trip test) never compares blocked, so this loss is currently masked; add blocked to the comparison and test fixtures.

Persist and restore per-tile blocked state; also add blocked to validateRoundTrip comparison.:

// in convertClassicToOpenAO tile loop:
const specialsBlocked: Record<string, boolean> = {};
// ... inside loop, after computing tile:
if (tile.blocked) specialsBlocked[tileKey] = true;
// ... when building specials:
blocked: Object.keys(specialsBlocked).length > 0 ? specialsBlocked : undefined,

// in convertOpenAOToClassic tile loop:
blocked: specials?.blocked?.[tileKey] === true,
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

@gitar-bot

gitar-bot Bot commented Aug 27, 2026

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

Sprite chunking, FPS capping, prefetch throttling, and HUD text resolution improvements are well-implemented. However, the classic map importer loses tile 'blocked' collision flags during conversion — convertClassicToOpenAO reads tile.blocked but never writes it to specials.blocked, and the reverse path hardcodes blocked: false, causing blocked tiles to become walkable in OpenAO. The round-trip validation also skips comparing this field. Populate specials.blocked for blocked tiles in both conversion directions and add blocked to the round-trip comparison and test fixtures before merge.

⚠️ Bug: Tile 'blocked' collision flag is lost during map conversion

📄 api/src/lib/classicMapParser.ts:335-349 📄 api/src/lib/classicMapParser.ts:466-480 📄 api/src/lib/classicMapParser.ts:537-551 📄 api/src/tests/classicMapParser.test.ts:158-172

convertClassicToOpenAO reads tile.blocked from every parsed tile but never writes it anywhere in the resulting bundle — the OpenAOSpecials.blocked field declared at line 72 is left unset. On the reverse path, convertOpenAOToClassic hardcodes blocked: false (line 473). As a result every tile's collision/blocked state is silently dropped when importing a classic map, so blocked tiles become walkable in OpenAO. Populate specials.blocked for blocked tiles in convertClassicToOpenAO and read it back in convertOpenAOToClassic. Note that validateRoundTrip (and the round-trip test) never compares blocked, so this loss is currently masked; add blocked to the comparison and test fixtures.

Persist and restore per-tile blocked state; also add blocked to validateRoundTrip comparison.
// in convertClassicToOpenAO tile loop:
const specialsBlocked: Record<string, boolean> = {};
// ... inside loop, after computing tile:
if (tile.blocked) specialsBlocked[tileKey] = true;
// ... when building specials:
blocked: Object.keys(specialsBlocked).length > 0 ? specialsBlocked : undefined,

// in convertOpenAOToClassic tile loop:
blocked: specials?.blocked?.[tileKey] === true,
🤖 Prompt for agents
Code Review: Sprite chunking, FPS capping, prefetch throttling, and HUD text resolution improvements are well-implemented. However, the classic map importer loses tile 'blocked' collision flags during conversion — `convertClassicToOpenAO` reads `tile.blocked` but never writes it to `specials.blocked`, and the reverse path hardcodes `blocked: false`, causing blocked tiles to become walkable in OpenAO. The round-trip validation also skips comparing this field. Populate `specials.blocked` for blocked tiles in both conversion directions and add blocked to the round-trip comparison and test fixtures before merge.

1. ⚠️ Bug: Tile 'blocked' collision flag is lost during map conversion
   Files: api/src/lib/classicMapParser.ts:335-349, api/src/lib/classicMapParser.ts:466-480, api/src/lib/classicMapParser.ts:537-551, api/src/tests/classicMapParser.test.ts:158-172

   `convertClassicToOpenAO` reads `tile.blocked` from every parsed tile but never writes it anywhere in the resulting bundle — the `OpenAOSpecials.blocked` field declared at line 72 is left unset. On the reverse path, `convertOpenAOToClassic` hardcodes `blocked: false` (line 473). As a result every tile's collision/blocked state is silently dropped when importing a classic map, so blocked tiles become walkable in OpenAO. Populate `specials.blocked` for blocked tiles in `convertClassicToOpenAO` and read it back in `convertOpenAOToClassic`. Note that `validateRoundTrip` (and the round-trip test) never compares `blocked`, so this loss is currently masked; add blocked to the comparison and test fixtures.

   Fix (Persist and restore per-tile blocked state; also add blocked to validateRoundTrip comparison.):
   // in convertClassicToOpenAO tile loop:
   const specialsBlocked: Record<string, boolean> = {};
   // ... inside loop, after computing tile:
   if (tile.blocked) specialsBlocked[tileKey] = true;
   // ... when building specials:
   blocked: Object.keys(specialsBlocked).length > 0 ? specialsBlocked : undefined,
   
   // in convertOpenAOToClassic tile loop:
   blocked: specials?.blocked?.[tileKey] === true,

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

Was this helpful? React with 👍 / 👎 | Gitar

@leocagli

Copy link
Copy Markdown
Collaborator

Revisé esta contra el main de hoy, que quedó verde por primera vez. El CI te
pasa en los cuatro jobs y aun así no la puedo mergear
, por dos motivos. Te dejo
los dos reproducidos, no son sospechas.

1. El terrain.json que genera el conversor no lo lee nadie

convertClassicMap.ts:82 vuelca la grilla cruda:

fs.writeFileSync(path.join(targetDir, "terrain.json"), JSON.stringify(bundle.terrain));

y bundle.terrain está tipado number[][][] (classicMapParser.ts:77). Pero el
formato real de terrain.json en api/src/mapas_source/mapa_N/ es un objeto:

{ id, width, height, palette, rows }

donde palette mapea id a { graphics: [l1,l2,l3,l4], blocked } y rows es la
matriz 100x100 de ids de paleta. Lo confirman los dos consumidores del repo:
server/src/loadMaps.ts:191-192 hace terrain.palette ?? {} y
Array.isArray(terrain.rows) ? ... : [], y worldBuilder.ts:674-683 recorre
terrain.palette.

Verifiqué los 294 terrain.json de api/src/mapas_source/: ninguno tiene
forma de array, todos son {id,width,height,palette,rows}.

Lo corrí sobre un mapa con 900 tiles bloqueados:

$ tsx src/scripts/convertClassicMap.ts --map mapa1.map --out /tmp/cm/out
$ head -c 40 /tmp/cm/out/terrain.json
[[[5500,581,0,0],[5500,0,0,0],...
   isArray: true   palette: undefined   rows: undefined   width: undefined

Y simulando loadMaps.readMap con ese archivo: 10000 tiles, 0 con gráficos, 0
bloqueados
. Un mapa 100x100 vacío y enteramente caminable. No tira error ni
warning: el servidor arranca e imprime "Mapas Cargados." como si nada.

Lo peor es que se pierde toda la colisión, no sólo en terrain.json:
convertClassicToOpenAO nunca escribe OpenAOSpecials.blocked, así que el
specials.json generado sale como {"id":7}. La información de bloqueo no está
en otro archivo, directamente no está.

El arreglo: el repo ya tiene el serializador canónico en
server/src/scripts/exportEditableMaps.ts:296-428, que construye paleta y filas.
Reusá eso en vez de volcar la grilla cruda.

Aclaración de encuadre, porque importa: el script es un CLI manual, no está en
api/package.json, no lo importa nada de runtime ni de CI, y escribe donde le
digas con --out. Mergear esto no pone main en rojo. Lo llamo bloqueante
porque el entregable del conversor es inservible para lo que existe, y porque el
layout invita a copiarlo a mapas_source/mapa_N/: emite los mismos cuatro nombres
de archivo, y meta.json, npcs.json y specials.json sí coinciden con el
formato real. O sea que el error se descubre cuando el mapa ya está en el juego.

2. El 97% del diff es de otra issue

El título dice perf(rendering): optimize client Pixi.js renderer y el cuerpo
lista cuatro cambios de cliente. Pero de las 881 líneas agregadas, 855 son tres
archivos nuevos de api/
: classicMapParser.ts (565), convertClassicMap.ts
(93) y classicMapParser.test.ts (197), que implementan un importador y
exportador de mapas binarios del Argentum clásico.

La #20 no menciona api/ ni parseo de mapas: sus archivos relevantes son los tres
del frontend, y es sólo rendimiento del cliente (app.ticker.maxFPS, tope de
prefetch, navigator.connection.saveData, resolución del texto del HUD).

El propio código se delata. classicMapParser.test.ts:14:

describe("Classic Map Importer / Exporter (Issue #23)", ...)

Y la #23 sigue abierta. Como esta PR dice Closes #20, al mergearla se
cerraría la #20 y entraría además una feature entera de la #23 sin review propio,
debajo de un título de performance. Un revisor que lea el título y mire los ~26
renglones de frontend aprueba 855 líneas que nadie evaluó.

3. Y esas 855 líneas no están verificadas

parseClassicMapBuffer no chequea longitud esperada (64 + 10010010 = 100064
bytes), ni magic, ni versión. Cuando el buffer se acaba,
classicMapParser.ts:122-125 empuja un tile vacío y sigue, sin acumular ningún
error. Y validateRoundTrip compara el parseo contra sí mismo re-serializado, con
convertClassicToOpenAO copiando layers sin transformar: estructuralmente no
puede fallar nunca.

Ejecutado:

parseClassicMapBuffer(Buffer.alloc(100, 0x41))    # 100 bytes de "A"
  -> 100x100, mapNum 16705
  -> success: true, translatedTiles: 10000, warnings: []
  -> roundTrip: {"valid":true,"differences":[]}

Cien bytes de basura pasan como mapa válido y el CLI imprimiría
[Round-Trip Validation]: PASSED. TranslationAuditReport declara
warnings: string[], pero success: true está fijo en la línea 424 y warnings
nunca recibe un push.

Qué necesito

Separá la PR en dos. Dejá acá los ~26 renglones de frontend que sí resuelven
la #20, que se mergean solos. El conversor va a su propia PR contra la #23, con el
formato de terrain.json arreglado, la colisión preservada, y una validación que
pueda fallar.

El trabajo del conversor está bien encarado y hace falta. El problema es que
entró de polizón y con un bug que no se nota hasta que el mapa está cargado.

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.

Rendimiento del cliente: mapa completo en memoria, sin limite de FPS y prefetch sin tope

2 participants