Etapa 3: infraestructura para publicar mapas en vivo sin reiniciar

[xusuxiang8]

Implementa infraestructura para publicar mapas en vivo sin reiniciar el servidor.

Cambios

1. server/src/loadMaps.ts — Nuevo método reloadMapByNumber(mapNum):

   • Recarga un mapa individual desde disco
   • Reconstruye el runtime (tiles, exits, objetos, NPCs, triggers, metadata)
   • Reubica jugadores que quedan sobre tiles bloqueados (mapa 1, 50, 50)
   • Notifica a los clientes conectados en ese mapa vía socket

2. api/src/server.ts — Nuevo endpoint:

   • POST /internal/game-data/maps/publish/:mapId — Activa la publicación de un mapa
   • Restringido a admins de game data

3. Seguridad de jugadores:

   • Si un jugador está parado sobre un tile que pasó a ser bloqueado,
     se lo reubica automáticamente en la posición segura de fallback

Pendiente (depende de issues de mutación previas)

• Integración con gameDataSync.ts para polling periódico
• Invalidación de caché del cliente (requiere cambios en el frontend)

Closes #11

[gitar-bot]

🚨 Bug: reloadMapByNumber defined outside the class body — won't compile

The LoadMaps class already closes with } at line 289. The new reloadMapByNumber method is added at lines 291-398 outside the class, using object-literal method syntax terminated by a trailing comma (}, at line 398). This is a TypeScript syntax error: a top-level async reloadMapByNumber(...) {...}, is not valid, so the whole module fails to compile, and this.mapFilesExist/this.getMapDirectory would not resolve even if it did. Move the method inside the class: delete the } at line 289, place the method (with no trailing comma) before the class-closing brace, and keep module.exports = LoadMaps; after the class.

Move the method inside the class and remove the stray trailing comma.:

        });
    }

    /** Recarga un mapa individual desde disco y actualiza el runtime. */
    async reloadMapByNumber(mapNum: number): Promise<{ ok: boolean; playersAffected: number }> {
        // ... body unchanged ...
        return { ok: true, playersAffected: movedPlayers };
    }
}

module.exports = LoadMaps;

• Apply fix

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

[gitar-bot]

⚠️ Edge Case: Fallback tile (1,50,50) is not validated as walkable

Players standing on a newly-blocked tile are teleported to the hardcoded fallback (map 1, 50, 50), but there is no check that this destination tile exists and is not itself blocked. If map 1's tile (50,50) is blocked or out of bounds after a reload, players are relocated into an invalid/blocked position, potentially trapping them. Validate the fallback tile (vars.mapa[1]?.[50]?.[50] exists and !blocked) or choose a known-safe spawn, and handle the case where it is unavailable.

Guard the fallback destination.:

const FallbackMap = 1, FallbackX = 50, FallbackY = 50;
const fbTile = vars.mapa[FallbackMap]?.[FallbackY]?.[FallbackX];
if (fbTile && !fbTile.blocked) {
    player.map = FallbackMap;
    player.pos = { x: FallbackX, y: FallbackY };
    player.posX = FallbackX;
    player.posY = FallbackY;
    movedPlayers++;
}

• Apply fix

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

[gitar-bot]

💡 Bug: Relocated players still notified about the old map reload

playersOnMap is captured before repositioning, then reused for the broadcast loop. Players who were moved to the fallback map still receive emit("mapReloaded", { mapNum }) for a map they are no longer on, while their actual new position/map is never communicated to the client. Broadcast the reload only to players who remain on mapNum, and send an explicit teleport/position update to relocated players.

Only emit reload to players still on the map.:

for (const player of playersOnMap) {
    if (player.map === mapNum && player.connection && player.connection.emit) {
        player.connection.emit("mapReloaded", { mapNum: mapNum });
    }
}

• Apply fix

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

[gitar-bot]

💡 Quality: Unused socket require in reloadMapByNumber

const socket = require("./socket"); is declared but never used (the broadcast uses player.connection.emit directly). Remove the dead require to avoid confusion and an unnecessary module load.

Delete the unused line.:

// remove: const socket = require("./socket");

• Apply fix

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

[gitar-bot]

Code Review 🚫 Blocked 0 resolved / 4 findings

Adds live map reloading infrastructure and an internal publish endpoint, but fails to compile because reloadMapByNumber is defined outside the LoadMaps class body and the fallback tile lacks walkability validation.

🚨 Bug: reloadMapByNumber defined outside the class body — won't compile

📄 server/src/loadMaps.ts:288-291 📄 server/src/loadMaps.ts:398-400

The LoadMaps class already closes with } at line 289. The new reloadMapByNumber method is added at lines 291-398 outside the class, using object-literal method syntax terminated by a trailing comma (}, at line 398). This is a TypeScript syntax error: a top-level async reloadMapByNumber(...) {...}, is not valid, so the whole module fails to compile, and this.mapFilesExist/this.getMapDirectory would not resolve even if it did. Move the method inside the class: delete the } at line 289, place the method (with no trailing comma) before the class-closing brace, and keep module.exports = LoadMaps; after the class.

Move the method inside the class and remove the stray trailing comma.

        });
    }

    /** Recarga un mapa individual desde disco y actualiza el runtime. */
    async reloadMapByNumber(mapNum: number): Promise<{ ok: boolean; playersAffected: number }> {
        // ... body unchanged ...
        return { ok: true, playersAffected: movedPlayers };
    }
}

module.exports = LoadMaps;

⚠️ Edge Case: Fallback tile (1,50,50) is not validated as walkable

📄 server/src/loadMaps.ts:377-387

Players standing on a newly-blocked tile are teleported to the hardcoded fallback (map 1, 50, 50), but there is no check that this destination tile exists and is not itself blocked. If map 1's tile (50,50) is blocked or out of bounds after a reload, players are relocated into an invalid/blocked position, potentially trapping them. Validate the fallback tile (vars.mapa[1]?.[50]?.[50] exists and !blocked) or choose a known-safe spawn, and handle the case where it is unavailable.

Guard the fallback destination.

const FallbackMap = 1, FallbackX = 50, FallbackY = 50;
const fbTile = vars.mapa[FallbackMap]?.[FallbackY]?.[FallbackX];
if (fbTile && !fbTile.blocked) {
    player.map = FallbackMap;
    player.pos = { x: FallbackX, y: FallbackY };
    player.posX = FallbackX;
    player.posY = FallbackY;
    movedPlayers++;
}

💡 Bug: Relocated players still notified about the old map reload

📄 server/src/loadMaps.ts:377-391

playersOnMap is captured before repositioning, then reused for the broadcast loop. Players who were moved to the fallback map still receive emit("mapReloaded", { mapNum }) for a map they are no longer on, while their actual new position/map is never communicated to the client. Broadcast the reload only to players who remain on mapNum, and send an explicit teleport/position update to relocated players.

Only emit reload to players still on the map.

for (const player of playersOnMap) {
    if (player.map === mapNum && player.connection && player.connection.emit) {
        player.connection.emit("mapReloaded", { mapNum: mapNum });
    }
}

💡 Quality: Unused socket require in reloadMapByNumber

📄 server/src/loadMaps.ts:371

const socket = require("./socket"); is declared but never used (the broadcast uses player.connection.emit directly). Remove the dead require to avoid confusion and an unnecessary module load.

Delete the unused line.

// remove: const socket = require("./socket");

🤖 Prompt for agents

Code Review: Adds live map reloading infrastructure and an internal publish endpoint, but fails to compile because reloadMapByNumber is defined outside the LoadMaps class body and the fallback tile lacks walkability validation.

1. 🚨 Bug: reloadMapByNumber defined outside the class body — won't compile
   Files: server/src/loadMaps.ts:288-291, server/src/loadMaps.ts:398-400

   The `LoadMaps` class already closes with `}` at line 289. The new `reloadMapByNumber` method is added at lines 291-398 *outside* the class, using object-literal method syntax terminated by a trailing comma (`},` at line 398). This is a TypeScript syntax error: a top-level `async reloadMapByNumber(...) {...},` is not valid, so the whole module fails to compile, and `this.mapFilesExist`/`this.getMapDirectory` would not resolve even if it did. Move the method inside the class: delete the `}` at line 289, place the method (with no trailing comma) before the class-closing brace, and keep `module.exports = LoadMaps;` after the class.

   Fix (Move the method inside the class and remove the stray trailing comma.):
           });
       }
   
       /** Recarga un mapa individual desde disco y actualiza el runtime. */
       async reloadMapByNumber(mapNum: number): Promise<{ ok: boolean; playersAffected: number }> {
           // ... body unchanged ...
           return { ok: true, playersAffected: movedPlayers };
       }
   }
   
   module.exports = LoadMaps;

2. ⚠️ Edge Case: Fallback tile (1,50,50) is not validated as walkable
   Files: server/src/loadMaps.ts:377-387

   Players standing on a newly-blocked tile are teleported to the hardcoded fallback (map 1, 50, 50), but there is no check that this destination tile exists and is not itself blocked. If map 1's tile (50,50) is blocked or out of bounds after a reload, players are relocated into an invalid/blocked position, potentially trapping them. Validate the fallback tile (`vars.mapa[1]?.[50]?.[50]` exists and `!blocked`) or choose a known-safe spawn, and handle the case where it is unavailable.

   Fix (Guard the fallback destination.):
   const FallbackMap = 1, FallbackX = 50, FallbackY = 50;
   const fbTile = vars.mapa[FallbackMap]?.[FallbackY]?.[FallbackX];
   if (fbTile && !fbTile.blocked) {
       player.map = FallbackMap;
       player.pos = { x: FallbackX, y: FallbackY };
       player.posX = FallbackX;
       player.posY = FallbackY;
       movedPlayers++;
   }

3. 💡 Bug: Relocated players still notified about the old map reload
   Files: server/src/loadMaps.ts:377-391

   `playersOnMap` is captured before repositioning, then reused for the broadcast loop. Players who were moved to the fallback map still receive `emit("mapReloaded", { mapNum })` for a map they are no longer on, while their actual new position/map is never communicated to the client. Broadcast the reload only to players who remain on `mapNum`, and send an explicit teleport/position update to relocated players.

   Fix (Only emit reload to players still on the map.):
   for (const player of playersOnMap) {
       if (player.map === mapNum && player.connection && player.connection.emit) {
           player.connection.emit("mapReloaded", { mapNum: mapNum });
       }
   }

4. 💡 Quality: Unused `socket` require in reloadMapByNumber
   Files: server/src/loadMaps.ts:371

   `const socket = require("./socket");` is declared but never used (the broadcast uses `player.connection.emit` directly). Remove the dead require to avoid confusion and an unnecessary module load.

   Fix (Delete the unused line.):
   // remove: const socket = require("./socket");

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

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