From baf6b57bb9c615e1776ac303a9f8fa4e736fce64 Mon Sep 17 00:00:00 2001 From: Rodrigoue9 Date: Thu, 27 Aug 2026 18:34:55 -0300 Subject: [PATCH 1/4] feat(npcs): implement map NPC placement, movement, and persistence API (#8) --- api/src/lib/mapNpcStorage.ts | 127 +++++++++++++++++++- api/src/tests/mapNpcPlacement.test.ts | 160 ++++++++++++++++++++++++++ 2 files changed, 285 insertions(+), 2 deletions(-) create mode 100644 api/src/tests/mapNpcPlacement.test.ts diff --git a/api/src/lib/mapNpcStorage.ts b/api/src/lib/mapNpcStorage.ts index c6dfa7ab..1abc9d76 100644 --- a/api/src/lib/mapNpcStorage.ts +++ b/api/src/lib/mapNpcStorage.ts @@ -10,6 +10,9 @@ export type MapNpcPlacement = { movement?: number; }; +export const MAX_NPCS_PER_MAP = 50; +export const MAP_GRID_SIZE = 100; + const MAP_DIR_PATTERN = /^mapa_(\d+)$/i; function toFiniteNumber(value: unknown): number | null { @@ -25,7 +28,7 @@ function toFiniteNumber(value: unknown): number | null { return null; } -function normalizePlacement( +export function normalizePlacement( value: unknown, fallbackMapNum?: number, ): MapNpcPlacement | null { @@ -47,9 +50,11 @@ function normalizePlacement( x === null || !Number.isInteger(x) || x <= 0 || + x > MAP_GRID_SIZE || y === null || !Number.isInteger(y) || y <= 0 || + y > MAP_GRID_SIZE || npcIndex === null || !Number.isInteger(npcIndex) || npcIndex <= 0 @@ -62,7 +67,7 @@ function normalizePlacement( : { mapNum, x, y, npcIndex }; } -function sortPlacements(placements: MapNpcPlacement[]): MapNpcPlacement[] { +export function sortPlacements(placements: MapNpcPlacement[]): MapNpcPlacement[] { return [...placements].sort( (left, right) => left.mapNum - right.mapNum || @@ -122,3 +127,121 @@ export async function loadAllMapNpcPlacements( return sortPlacements(placements.flat()); } + +export async function saveMapNpcPlacements( + mapsSourceDir: string, + mapNum: number, + placements: MapNpcPlacement[], +): Promise { + const mapDir = path.join(mapsSourceDir, `mapa_${mapNum}`); + if (!existsSync(mapDir)) { + await fs.mkdir(mapDir, { recursive: true }); + } + + const filePath = path.join(mapDir, "npcs.json"); + const formatted = sortPlacements(placements).map((p) => ({ + mapNum: p.mapNum, + x: p.x, + y: p.y, + npcIndex: p.npcIndex, + ...(p.movement !== undefined ? { movement: p.movement } : {}), + })); + + await fs.writeFile(filePath, JSON.stringify(formatted, null, 2), "utf8"); +} + +export async function placeMapNpc( + mapsSourceDir: string, + rawPlacement: unknown, + options: { + maxNpcs?: number; + isTileBlocked?: (x: number, y: number) => boolean; + isValidNpcIndex?: (npcIndex: number) => boolean; + } = {}, +): Promise<{ ok: true; placements: MapNpcPlacement[] } | { ok: false; reason: string }> { + const placement = normalizePlacement(rawPlacement); + if (!placement) { + return { ok: false, reason: "Formato de colocación de NPC inválido o coordenadas fuera de límites (1-100)." }; + } + + if (options.isValidNpcIndex && !options.isValidNpcIndex(placement.npcIndex)) { + return { ok: false, reason: `El npcIndex ${placement.npcIndex} no existe en el catálogo.` }; + } + + if (options.isTileBlocked && options.isTileBlocked(placement.x, placement.y)) { + return { ok: false, reason: `La coordenada (${placement.x}, ${placement.y}) es un tile bloqueado.` }; + } + + const currentPlacements = await loadMapNpcPlacements(mapsSourceDir, placement.mapNum); + + const alreadyAtTile = currentPlacements.some((p) => p.x === placement.x && p.y === placement.y); + if (alreadyAtTile) { + return { ok: false, reason: `Ya existe un NPC colocado en la coordenada (${placement.x}, ${placement.y}).` }; + } + + const maxAllowed = options.maxNpcs ?? MAX_NPCS_PER_MAP; + if (currentPlacements.length >= maxAllowed) { + return { ok: false, reason: `Se alcanzó el límite máximo de ${maxAllowed} NPCs para el mapa ${placement.mapNum}.` }; + } + + const updated = [...currentPlacements, placement]; + await saveMapNpcPlacements(mapsSourceDir, placement.mapNum, updated); + + return { ok: true, placements: sortPlacements(updated) }; +} + +export async function moveMapNpc( + mapsSourceDir: string, + mapNum: number, + fromX: number, + fromY: number, + toX: number, + toY: number, + options: { + isTileBlocked?: (x: number, y: number) => boolean; + } = {}, +): Promise<{ ok: true; placements: MapNpcPlacement[] } | { ok: false; reason: string }> { + if (toX < 1 || toX > MAP_GRID_SIZE || toY < 1 || toY > MAP_GRID_SIZE) { + return { ok: false, reason: "Las coordenadas de destino están fuera de la grilla (1-100)." }; + } + + if (options.isTileBlocked && options.isTileBlocked(toX, toY)) { + return { ok: false, reason: `La coordenada de destino (${toX}, ${toY}) es un tile bloqueado.` }; + } + + const currentPlacements = await loadMapNpcPlacements(mapsSourceDir, mapNum); + + const sourceIndex = currentPlacements.findIndex((p) => p.x === fromX && p.y === fromY); + if (sourceIndex === -1) { + return { ok: false, reason: `No se encontró ningún NPC en (${fromX}, ${fromY}) en el mapa ${mapNum}.` }; + } + + const destOccupied = currentPlacements.some((p) => p.x === toX && p.y === toY && !(p.x === fromX && p.y === fromY)); + if (destOccupied) { + return { ok: false, reason: `La coordenada de destino (${toX}, ${toY}) ya está ocupada por otro NPC.` }; + } + + const targetNpc = currentPlacements[sourceIndex]; + const updated = currentPlacements.filter((_, idx) => idx !== sourceIndex); + updated.push({ ...targetNpc, x: toX, y: toY }); + + await saveMapNpcPlacements(mapsSourceDir, mapNum, updated); + return { ok: true, placements: sortPlacements(updated) }; +} + +export async function removeMapNpc( + mapsSourceDir: string, + mapNum: number, + x: number, + y: number, +): Promise<{ ok: true; placements: MapNpcPlacement[] } | { ok: false; reason: string }> { + const currentPlacements = await loadMapNpcPlacements(mapsSourceDir, mapNum); + const filtered = currentPlacements.filter((p) => !(p.x === x && p.y === y)); + + if (filtered.length === currentPlacements.length) { + return { ok: false, reason: `No se encontró ningún NPC en (${x}, ${y}) para remover del mapa ${mapNum}.` }; + } + + await saveMapNpcPlacements(mapsSourceDir, mapNum, filtered); + return { ok: true, placements: sortPlacements(filtered) }; +} diff --git a/api/src/tests/mapNpcPlacement.test.ts b/api/src/tests/mapNpcPlacement.test.ts new file mode 100644 index 00000000..fec900d0 --- /dev/null +++ b/api/src/tests/mapNpcPlacement.test.ts @@ -0,0 +1,160 @@ +import assert from "node:assert/strict"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import { + loadMapNpcPlacements, + moveMapNpc, + placeMapNpc, + removeMapNpc, + saveMapNpcPlacements, + sortPlacements, + type MapNpcPlacement, +} from "../lib/mapNpcStorage"; + +test("mapNpcPlacement - colocación y persistencia de NPCs", async (t) => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openao-npc-test-")); + + t.after(async () => { + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + await t.test("placeMapNpc - coloca un NPC exitosamente y persiste en disco", async () => { + const result = await placeMapNpc(tempDir, { + mapNum: 1, + x: 50, + y: 50, + npcIndex: 1, + movement: 0, + }); + + assert.equal(result.ok, true); + if (result.ok) { + assert.equal(result.placements.length, 1); + assert.deepEqual(result.placements[0], { + mapNum: 1, + x: 50, + y: 50, + npcIndex: 1, + movement: 0, + }); + } + + const fromDisk = await loadMapNpcPlacements(tempDir, 1); + assert.equal(fromDisk.length, 1); + assert.equal(fromDisk[0].x, 50); + assert.equal(fromDisk[0].y, 50); + }); + + await t.test("placeMapNpc - rechaza coordenadas fuera de rango (1-100)", async () => { + const resZero = await placeMapNpc(tempDir, { mapNum: 1, x: 0, y: 50, npcIndex: 1 }); + assert.equal(resZero.ok, false); + + const resOver = await placeMapNpc(tempDir, { mapNum: 1, x: 101, y: 50, npcIndex: 1 }); + assert.equal(resOver.ok, false); + }); + + await t.test("placeMapNpc - rechaza npcIndex inválido o inexistente", async () => { + const resInvalid = await placeMapNpc(tempDir, { mapNum: 1, x: 10, y: 10, npcIndex: -5 }); + assert.equal(resInvalid.ok, false); + + const resCatalog = await placeMapNpc( + tempDir, + { mapNum: 1, x: 10, y: 10, npcIndex: 9999 }, + { isValidNpcIndex: (idx) => idx <= 340 } + ); + assert.equal(resCatalog.ok, false); + if (!resCatalog.ok) { + assert.match(resCatalog.reason, /no existe en el catálogo/); + } + }); + + await t.test("placeMapNpc - rechaza colocación sobre tile bloqueado", async () => { + const blockedTile = (x: number, y: number) => x === 20 && y === 20; + const res = await placeMapNpc( + tempDir, + { mapNum: 1, x: 20, y: 20, npcIndex: 1 }, + { isTileBlocked: blockedTile } + ); + + assert.equal(res.ok, false); + if (!res.ok) { + assert.match(res.reason, /tile bloqueado/); + } + }); + + await t.test("placeMapNpc - rechaza apilar dos NPCs en el mismo tile", async () => { + await placeMapNpc(tempDir, { mapNum: 2, x: 15, y: 15, npcIndex: 1 }); + const resDup = await placeMapNpc(tempDir, { mapNum: 2, x: 15, y: 15, npcIndex: 2 }); + + assert.equal(resDup.ok, false); + if (!resDup.ok) { + assert.match(resDup.reason, /Ya existe un NPC/); + } + }); + + await t.test("placeMapNpc - respeta el límite máximo por mapa", async () => { + const mapNum = 3; + for (let i = 1; i <= 3; i++) { + await placeMapNpc(tempDir, { mapNum, x: i, y: 10, npcIndex: 1 }, { maxNpcs: 3 }); + } + + const resLimit = await placeMapNpc( + tempDir, + { mapNum, x: 4, y: 10, npcIndex: 1 }, + { maxNpcs: 3 } + ); + assert.equal(resLimit.ok, false); + if (!resLimit.ok) { + assert.match(resLimit.reason, /límite máximo/); + } + }); + + await t.test("moveMapNpc - mueve un NPC existente a otra coordenada válida", async () => { + const mapNum = 4; + await placeMapNpc(tempDir, { mapNum, x: 10, y: 10, npcIndex: 5 }); + + const moveRes = await moveMapNpc(tempDir, mapNum, 10, 10, 12, 14); + assert.equal(moveRes.ok, true); + + const loaded = await loadMapNpcPlacements(tempDir, mapNum); + assert.equal(loaded.length, 1); + assert.equal(loaded[0].x, 12); + assert.equal(loaded[0].y, 14); + }); + + await t.test("moveMapNpc - rechaza mover a un tile bloqueado u ocupado", async () => { + const mapNum = 5; + await placeMapNpc(tempDir, { mapNum, x: 10, y: 10, npcIndex: 1 }); + await placeMapNpc(tempDir, { mapNum, x: 11, y: 10, npcIndex: 2 }); + + const resOccupied = await moveMapNpc(tempDir, mapNum, 10, 10, 11, 10); + assert.equal(resOccupied.ok, false); + + const resBlocked = await moveMapNpc( + tempDir, + mapNum, + 10, + 10, + 30, + 30, + { isTileBlocked: (x, y) => x === 30 && y === 30 } + ); + assert.equal(resBlocked.ok, false); + }); + + await t.test("removeMapNpc - quita un NPC y actualiza el archivo", async () => { + const mapNum = 6; + await placeMapNpc(tempDir, { mapNum, x: 5, y: 5, npcIndex: 1 }); + await placeMapNpc(tempDir, { mapNum, x: 6, y: 6, npcIndex: 2 }); + + const removeRes = await removeMapNpc(tempDir, mapNum, 5, 5); + assert.equal(removeRes.ok, true); + + const remaining = await loadMapNpcPlacements(tempDir, mapNum); + assert.equal(remaining.length, 1); + assert.equal(remaining[0].x, 6); + assert.equal(remaining[0].y, 6); + }); +}); From 60ce76f2a9937d7c809b07652dbdfb4f6d4c0284 Mon Sep 17 00:00:00 2001 From: Rodrigoue9 Date: Sat, 29 Aug 2026 07:44:56 -0300 Subject: [PATCH 2/4] feat(world-builder): NPC placement, movement, and map persistence API (Closes #8) --- api/src/server.ts | 124 ++++++++++++++++++++++++++ api/src/tests/mapNpcPlacement.test.ts | 19 +++- 2 files changed, 142 insertions(+), 1 deletion(-) diff --git a/api/src/server.ts b/api/src/server.ts index 2b309610..97cd86bc 100644 --- a/api/src/server.ts +++ b/api/src/server.ts @@ -1,3 +1,11 @@ +import { + loadMapNpcPlacements, + placeMapNpc, + moveMapNpc, + removeMapNpc, + MAX_NPCS_PER_MAP +} from "./lib/mapNpcStorage"; +import { isValidGameNpcIndex } from "./repositories/gameNpcs"; import express from "express"; import config from "./config"; import pool from "./db"; @@ -3066,3 +3074,119 @@ app.get("/user-online-stats", async (request, response) => { }); void start(); + +/** + * Endpoints del Modo Construcción: Gestión de NPCs en mapas (#8) + */ +app.get("/admin/game-data/maps/:mapNum/npcs", async (request, response) => { + try { + const authorized = await requireAdminEmailSession(request, response); + if (!authorized) return; + + const mapNum = Number.parseInt(request.params.mapNum ?? "", 10); + if (!Number.isInteger(mapNum) || mapNum <= 0) { + response.status(400).json({ error: "Número de mapa inválido." }); + return; + } + + const npcs = await loadMapNpcPlacements(config.mapsSourceDir, mapNum); + response.json({ mapNum, npcs }); + } catch (error) { + const message = error instanceof Error ? error.message : "Unexpected error"; + response.status(400).json({ error: message }); + } +}); + +app.post("/admin/game-data/maps/:mapNum/npcs", async (request, response) => { + try { + const authorized = await requireAdminEmailSession(request, response); + if (!authorized) return; + + const mapNum = Number.parseInt(request.params.mapNum ?? "", 10); + if (!Number.isInteger(mapNum) || mapNum <= 0) { + response.status(400).json({ error: "Número de mapa inválido." }); + return; + } + + const result = await placeMapNpc( + config.mapsSourceDir, + { ...request.body, mapNum }, + { + maxNpcs: MAX_NPCS_PER_MAP, + isValidNpcIndex: (idx) => isValidGameNpcIndex(idx) + } + ); + + if (!result.ok) { + response.status(400).json({ error: result.reason }); + return; + } + + response.status(201).json({ ok: true, npcs: result.placements }); + } catch (error) { + const message = error instanceof Error ? error.message : "Unexpected error"; + response.status(400).json({ error: message }); + } +}); + +app.put("/admin/game-data/maps/:mapNum/npcs/move", async (request, response) => { + try { + const authorized = await requireAdminEmailSession(request, response); + if (!authorized) return; + + const mapNum = Number.parseInt(request.params.mapNum ?? "", 10); + const { fromX, fromY, toX, toY } = request.body ?? {}; + + if (![mapNum, fromX, fromY, toX, toY].every(Number.isInteger)) { + response.status(400).json({ error: "Parámetros de coordenadas inválidos." }); + return; + } + + const result = await moveMapNpc( + config.mapsSourceDir, + mapNum, + fromX, + fromY, + toX, + toY + ); + + if (!result.ok) { + response.status(400).json({ error: result.reason }); + return; + } + + response.json({ ok: true, npcs: result.placements }); + } catch (error) { + const message = error instanceof Error ? error.message : "Unexpected error"; + response.status(400).json({ error: message }); + } +}); + +app.delete("/admin/game-data/maps/:mapNum/npcs/:x/:y", async (request, response) => { + try { + const authorized = await requireAdminEmailSession(request, response); + if (!authorized) return; + + const mapNum = Number.parseInt(request.params.mapNum ?? "", 10); + const x = Number.parseInt(request.params.x ?? "", 10); + const y = Number.parseInt(request.params.y ?? "", 10); + + if (![mapNum, x, y].every(Number.isInteger)) { + response.status(400).json({ error: "Parámetros inválidos." }); + return; + } + + const result = await removeMapNpc(config.mapsSourceDir, mapNum, x, y); + + if (!result.ok) { + response.status(400).json({ error: result.reason }); + return; + } + + response.json({ ok: true, npcs: result.placements }); + } catch (error) { + const message = error instanceof Error ? error.message : "Unexpected error"; + response.status(400).json({ error: message }); + } +}); diff --git a/api/src/tests/mapNpcPlacement.test.ts b/api/src/tests/mapNpcPlacement.test.ts index fec900d0..230cea1b 100644 --- a/api/src/tests/mapNpcPlacement.test.ts +++ b/api/src/tests/mapNpcPlacement.test.ts @@ -4,6 +4,7 @@ import os from "node:os"; import path from "node:path"; import test from "node:test"; import { + MAX_NPCS_PER_MAP, loadMapNpcPlacements, moveMapNpc, placeMapNpc, @@ -13,13 +14,18 @@ import { type MapNpcPlacement, } from "../lib/mapNpcStorage"; -test("mapNpcPlacement - colocación y persistencia de NPCs", async (t) => { +test("mapNpcPlacement - colocación y persistencia de NPCs (#8)", async (t) => { const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openao-npc-test-")); t.after(async () => { await fs.rm(tempDir, { recursive: true, force: true }); }); + await t.test("MAX_NPCS_PER_MAP constante nombrada exportada", () => { + assert.equal(typeof MAX_NPCS_PER_MAP, "number"); + assert.equal(MAX_NPCS_PER_MAP, 50); + }); + await t.test("placeMapNpc - coloca un NPC exitosamente y persiste en disco", async () => { const result = await placeMapNpc(tempDir, { mapNum: 1, @@ -124,6 +130,17 @@ test("mapNpcPlacement - colocación y persistencia de NPCs", async (t) => { assert.equal(loaded[0].y, 14); }); + await t.test("moveMapNpc - mover a la misma casilla actual no falla (caso autodesplazamiento)", async () => { + const mapNum = 4; + const selfMoveRes = await moveMapNpc(tempDir, mapNum, 12, 14, 12, 14); + assert.equal(selfMoveRes.ok, true); + + const loaded = await loadMapNpcPlacements(tempDir, mapNum); + assert.equal(loaded.length, 1); + assert.equal(loaded[0].x, 12); + assert.equal(loaded[0].y, 14); + }); + await t.test("moveMapNpc - rechaza mover a un tile bloqueado u ocupado", async () => { const mapNum = 5; await placeMapNpc(tempDir, { mapNum, x: 10, y: 10, npcIndex: 1 }); From b2df5a9fd0938356a109a65101cd7c07197e07a8 Mon Sep 17 00:00:00 2001 From: Rodrigoue9 Date: Sat, 29 Aug 2026 07:56:13 -0300 Subject: [PATCH 3/4] fix(world-builder): add concurrency mutex, export resolveMapsSourceDir and isValidGameNpcIndex, tighten mapNum validation (#8) --- api/src/lib/mapNpcStorage.ts | 109 ++++++++++++++++++--------- api/src/repositories/gameNpcs.ts | 8 ++ api/src/repositories/worldBuilder.ts | 2 +- api/src/server.ts | 26 +++++-- 4 files changed, 101 insertions(+), 44 deletions(-) diff --git a/api/src/lib/mapNpcStorage.ts b/api/src/lib/mapNpcStorage.ts index 1abc9d76..a3465966 100644 --- a/api/src/lib/mapNpcStorage.ts +++ b/api/src/lib/mapNpcStorage.ts @@ -15,6 +15,34 @@ export const MAP_GRID_SIZE = 100; const MAP_DIR_PATTERN = /^mapa_(\d+)$/i; +const mapMutexes = new Map>(); + +export async function withMapLock( + mapNum: number, + operation: () => Promise, +): Promise { + const currentLock = mapMutexes.get(mapNum) ?? Promise.resolve(); + let releaseLock: () => void = () => {}; + const nextLock = new Promise((resolve) => { + releaseLock = resolve; + }); + + mapMutexes.set( + mapNum, + currentLock.then(() => nextLock).catch(() => nextLock), + ); + + try { + await currentLock; + return await operation(); + } finally { + releaseLock(); + if (mapMutexes.get(mapNum) === nextLock) { + mapMutexes.delete(mapNum); + } + } +} + function toFiniteNumber(value: unknown): number | null { if (typeof value === "number" && Number.isFinite(value)) { return value; @@ -156,7 +184,7 @@ export async function placeMapNpc( options: { maxNpcs?: number; isTileBlocked?: (x: number, y: number) => boolean; - isValidNpcIndex?: (npcIndex: number) => boolean; + isValidNpcIndex?: (npcIndex: number) => boolean | Promise; } = {}, ): Promise<{ ok: true; placements: MapNpcPlacement[] } | { ok: false; reason: string }> { const placement = normalizePlacement(rawPlacement); @@ -164,30 +192,35 @@ export async function placeMapNpc( return { ok: false, reason: "Formato de colocación de NPC inválido o coordenadas fuera de límites (1-100)." }; } - if (options.isValidNpcIndex && !options.isValidNpcIndex(placement.npcIndex)) { - return { ok: false, reason: `El npcIndex ${placement.npcIndex} no existe en el catálogo.` }; + if (options.isValidNpcIndex) { + const valid = await options.isValidNpcIndex(placement.npcIndex); + if (!valid) { + return { ok: false, reason: `El npcIndex ${placement.npcIndex} no existe en el catálogo.` }; + } } if (options.isTileBlocked && options.isTileBlocked(placement.x, placement.y)) { return { ok: false, reason: `La coordenada (${placement.x}, ${placement.y}) es un tile bloqueado.` }; } - const currentPlacements = await loadMapNpcPlacements(mapsSourceDir, placement.mapNum); + return withMapLock(placement.mapNum, async () => { + const currentPlacements = await loadMapNpcPlacements(mapsSourceDir, placement.mapNum); - const alreadyAtTile = currentPlacements.some((p) => p.x === placement.x && p.y === placement.y); - if (alreadyAtTile) { - return { ok: false, reason: `Ya existe un NPC colocado en la coordenada (${placement.x}, ${placement.y}).` }; - } + const alreadyAtTile = currentPlacements.some((p) => p.x === placement.x && p.y === placement.y); + if (alreadyAtTile) { + return { ok: false, reason: `Ya existe un NPC colocado en la coordenada (${placement.x}, ${placement.y}).` }; + } - const maxAllowed = options.maxNpcs ?? MAX_NPCS_PER_MAP; - if (currentPlacements.length >= maxAllowed) { - return { ok: false, reason: `Se alcanzó el límite máximo de ${maxAllowed} NPCs para el mapa ${placement.mapNum}.` }; - } + const maxAllowed = options.maxNpcs ?? MAX_NPCS_PER_MAP; + if (currentPlacements.length >= maxAllowed) { + return { ok: false, reason: `Se alcanzó el límite máximo de ${maxAllowed} NPCs para el mapa ${placement.mapNum}.` }; + } - const updated = [...currentPlacements, placement]; - await saveMapNpcPlacements(mapsSourceDir, placement.mapNum, updated); + const updated = [...currentPlacements, placement]; + await saveMapNpcPlacements(mapsSourceDir, placement.mapNum, updated); - return { ok: true, placements: sortPlacements(updated) }; + return { ok: true, placements: sortPlacements(updated) }; + }); } export async function moveMapNpc( @@ -209,24 +242,26 @@ export async function moveMapNpc( return { ok: false, reason: `La coordenada de destino (${toX}, ${toY}) es un tile bloqueado.` }; } - const currentPlacements = await loadMapNpcPlacements(mapsSourceDir, mapNum); + return withMapLock(mapNum, async () => { + const currentPlacements = await loadMapNpcPlacements(mapsSourceDir, mapNum); - const sourceIndex = currentPlacements.findIndex((p) => p.x === fromX && p.y === fromY); - if (sourceIndex === -1) { - return { ok: false, reason: `No se encontró ningún NPC en (${fromX}, ${fromY}) en el mapa ${mapNum}.` }; - } + const sourceIndex = currentPlacements.findIndex((p) => p.x === fromX && p.y === fromY); + if (sourceIndex === -1) { + return { ok: false, reason: `No se encontró ningún NPC en (${fromX}, ${fromY}) en el mapa ${mapNum}.` }; + } - const destOccupied = currentPlacements.some((p) => p.x === toX && p.y === toY && !(p.x === fromX && p.y === fromY)); - if (destOccupied) { - return { ok: false, reason: `La coordenada de destino (${toX}, ${toY}) ya está ocupada por otro NPC.` }; - } + const destOccupied = currentPlacements.some((p) => p.x === toX && p.y === toY && !(p.x === fromX && p.y === fromY)); + if (destOccupied) { + return { ok: false, reason: `La coordenada de destino (${toX}, ${toY}) ya está ocupada por otro NPC.` }; + } - const targetNpc = currentPlacements[sourceIndex]; - const updated = currentPlacements.filter((_, idx) => idx !== sourceIndex); - updated.push({ ...targetNpc, x: toX, y: toY }); + const targetNpc = currentPlacements[sourceIndex]; + const updated = currentPlacements.filter((_, idx) => idx !== sourceIndex); + updated.push({ ...targetNpc, x: toX, y: toY }); - await saveMapNpcPlacements(mapsSourceDir, mapNum, updated); - return { ok: true, placements: sortPlacements(updated) }; + await saveMapNpcPlacements(mapsSourceDir, mapNum, updated); + return { ok: true, placements: sortPlacements(updated) }; + }); } export async function removeMapNpc( @@ -235,13 +270,15 @@ export async function removeMapNpc( x: number, y: number, ): Promise<{ ok: true; placements: MapNpcPlacement[] } | { ok: false; reason: string }> { - const currentPlacements = await loadMapNpcPlacements(mapsSourceDir, mapNum); - const filtered = currentPlacements.filter((p) => !(p.x === x && p.y === y)); + return withMapLock(mapNum, async () => { + const currentPlacements = await loadMapNpcPlacements(mapsSourceDir, mapNum); + const filtered = currentPlacements.filter((p) => !(p.x === x && p.y === y)); - if (filtered.length === currentPlacements.length) { - return { ok: false, reason: `No se encontró ningún NPC en (${x}, ${y}) para remover del mapa ${mapNum}.` }; - } + if (filtered.length === currentPlacements.length) { + return { ok: false, reason: `No se encontró ningún NPC en (${x}, ${y}) para remover del mapa ${mapNum}.` }; + } - await saveMapNpcPlacements(mapsSourceDir, mapNum, filtered); - return { ok: true, placements: sortPlacements(filtered) }; + await saveMapNpcPlacements(mapsSourceDir, mapNum, filtered); + return { ok: true, placements: sortPlacements(filtered) }; + }); } diff --git a/api/src/repositories/gameNpcs.ts b/api/src/repositories/gameNpcs.ts index 8d8a66d7..f2afa681 100644 --- a/api/src/repositories/gameNpcs.ts +++ b/api/src/repositories/gameNpcs.ts @@ -535,3 +535,11 @@ export async function exportFrontendNpcs(): Promise< ]), ); } + +export async function isValidGameNpcIndex(id: number): Promise { + if (!Number.isInteger(id) || id <= 0) { + return false; + } + const npc = await getGameNpcById(id); + return npc !== null; +} diff --git a/api/src/repositories/worldBuilder.ts b/api/src/repositories/worldBuilder.ts index 4402a924..ff6cdae1 100644 --- a/api/src/repositories/worldBuilder.ts +++ b/api/src/repositories/worldBuilder.ts @@ -614,7 +614,7 @@ export async function clearTile( * specials.json, npcs.json, meta.json) y la API los re-exporta para el * frontend. Si la copia fuente no existe, el mapa no es editable. */ -function resolveMapsSourceDir(): string { +export function resolveMapsSourceDir(): string { const candidates = [ path.resolve(__dirname, ".."), path.resolve(__dirname, "..", "..", "src"), diff --git a/api/src/server.ts b/api/src/server.ts index 97cd86bc..26811254 100644 --- a/api/src/server.ts +++ b/api/src/server.ts @@ -5,6 +5,7 @@ import { removeMapNpc, MAX_NPCS_PER_MAP } from "./lib/mapNpcStorage"; +import { resolveMapsSourceDir } from "./repositories/worldBuilder"; import { isValidGameNpcIndex } from "./repositories/gameNpcs"; import express from "express"; import config from "./config"; @@ -3089,7 +3090,8 @@ app.get("/admin/game-data/maps/:mapNum/npcs", async (request, response) => { return; } - const npcs = await loadMapNpcPlacements(config.mapsSourceDir, mapNum); + const mapsSourceDir = resolveMapsSourceDir(); + const npcs = await loadMapNpcPlacements(mapsSourceDir, mapNum); response.json({ mapNum, npcs }); } catch (error) { const message = error instanceof Error ? error.message : "Unexpected error"; @@ -3108,12 +3110,13 @@ app.post("/admin/game-data/maps/:mapNum/npcs", async (request, response) => { return; } + const mapsSourceDir = resolveMapsSourceDir(); const result = await placeMapNpc( - config.mapsSourceDir, + mapsSourceDir, { ...request.body, mapNum }, { maxNpcs: MAX_NPCS_PER_MAP, - isValidNpcIndex: (idx) => isValidGameNpcIndex(idx) + isValidNpcIndex: async (idx) => await isValidGameNpcIndex(idx) } ); @@ -3135,15 +3138,21 @@ app.put("/admin/game-data/maps/:mapNum/npcs/move", async (request, response) => if (!authorized) return; const mapNum = Number.parseInt(request.params.mapNum ?? "", 10); + if (!Number.isInteger(mapNum) || mapNum <= 0) { + response.status(400).json({ error: "Número de mapa inválido." }); + return; + } + const { fromX, fromY, toX, toY } = request.body ?? {}; - if (![mapNum, fromX, fromY, toX, toY].every(Number.isInteger)) { + if (![fromX, fromY, toX, toY].every(Number.isInteger)) { response.status(400).json({ error: "Parámetros de coordenadas inválidos." }); return; } + const mapsSourceDir = resolveMapsSourceDir(); const result = await moveMapNpc( - config.mapsSourceDir, + mapsSourceDir, mapNum, fromX, fromY, @@ -3172,12 +3181,13 @@ app.delete("/admin/game-data/maps/:mapNum/npcs/:x/:y", async (request, response) const x = Number.parseInt(request.params.x ?? "", 10); const y = Number.parseInt(request.params.y ?? "", 10); - if (![mapNum, x, y].every(Number.isInteger)) { + if (!Number.isInteger(mapNum) || mapNum <= 0 || !Number.isInteger(x) || !Number.isInteger(y)) { response.status(400).json({ error: "Parámetros inválidos." }); return; } - const result = await removeMapNpc(config.mapsSourceDir, mapNum, x, y); + const mapsSourceDir = resolveMapsSourceDir(); + const result = await removeMapNpc(mapsSourceDir, mapNum, x, y); if (!result.ok) { response.status(400).json({ error: result.reason }); @@ -3190,3 +3200,5 @@ app.delete("/admin/game-data/maps/:mapNum/npcs/:x/:y", async (request, response) response.status(400).json({ error: message }); } }); + + From 0acad5ab8d1ff55a204ebd7d56d712c0feb54034 Mon Sep 17 00:00:00 2001 From: Rodrigoue9 Date: Sat, 29 Aug 2026 20:52:14 -0300 Subject: [PATCH 4/4] test(world-builder): run NPC persistence coverage in Vitest --- api/src/lib/mapNpcStorage.ts | 16 ++++---- api/src/server.ts | 2 - api/src/tests/mapNpcPlacement.test.ts | 58 ++++++++++++++++++--------- 3 files changed, 48 insertions(+), 28 deletions(-) diff --git a/api/src/lib/mapNpcStorage.ts b/api/src/lib/mapNpcStorage.ts index a3465966..6b669368 100644 --- a/api/src/lib/mapNpcStorage.ts +++ b/api/src/lib/mapNpcStorage.ts @@ -21,23 +21,23 @@ export async function withMapLock( mapNum: number, operation: () => Promise, ): Promise { - const currentLock = mapMutexes.get(mapNum) ?? Promise.resolve(); + const previousLock = mapMutexes.get(mapNum) ?? Promise.resolve(); let releaseLock: () => void = () => {}; - const nextLock = new Promise((resolve) => { + const operationLock = new Promise((resolve) => { releaseLock = resolve; }); + const queuedLock = previousLock + .catch(() => undefined) + .then(() => operationLock); - mapMutexes.set( - mapNum, - currentLock.then(() => nextLock).catch(() => nextLock), - ); + mapMutexes.set(mapNum, queuedLock); try { - await currentLock; + await previousLock.catch(() => undefined); return await operation(); } finally { releaseLock(); - if (mapMutexes.get(mapNum) === nextLock) { + if (mapMutexes.get(mapNum) === queuedLock) { mapMutexes.delete(mapNum); } } diff --git a/api/src/server.ts b/api/src/server.ts index 26811254..b059d75f 100644 --- a/api/src/server.ts +++ b/api/src/server.ts @@ -3200,5 +3200,3 @@ app.delete("/admin/game-data/maps/:mapNum/npcs/:x/:y", async (request, response) response.status(400).json({ error: message }); } }); - - diff --git a/api/src/tests/mapNpcPlacement.test.ts b/api/src/tests/mapNpcPlacement.test.ts index 230cea1b..9218ab22 100644 --- a/api/src/tests/mapNpcPlacement.test.ts +++ b/api/src/tests/mapNpcPlacement.test.ts @@ -2,31 +2,32 @@ import assert from "node:assert/strict"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import test from "node:test"; +import { afterAll, beforeAll, describe, test } from "vitest"; import { MAX_NPCS_PER_MAP, loadMapNpcPlacements, moveMapNpc, placeMapNpc, removeMapNpc, - saveMapNpcPlacements, - sortPlacements, - type MapNpcPlacement, } from "../lib/mapNpcStorage"; -test("mapNpcPlacement - colocación y persistencia de NPCs (#8)", async (t) => { - const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openao-npc-test-")); +describe("mapNpcPlacement - colocación y persistencia de NPCs (#8)", () => { + let tempDir: string; - t.after(async () => { + beforeAll(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openao-npc-test-")); + }); + + afterAll(async () => { await fs.rm(tempDir, { recursive: true, force: true }); }); - await t.test("MAX_NPCS_PER_MAP constante nombrada exportada", () => { + test("MAX_NPCS_PER_MAP constante nombrada exportada", () => { assert.equal(typeof MAX_NPCS_PER_MAP, "number"); assert.equal(MAX_NPCS_PER_MAP, 50); }); - await t.test("placeMapNpc - coloca un NPC exitosamente y persiste en disco", async () => { + test("placeMapNpc - coloca un NPC exitosamente y persiste en disco", async () => { const result = await placeMapNpc(tempDir, { mapNum: 1, x: 50, @@ -53,7 +54,7 @@ test("mapNpcPlacement - colocación y persistencia de NPCs (#8)", async (t) => { assert.equal(fromDisk[0].y, 50); }); - await t.test("placeMapNpc - rechaza coordenadas fuera de rango (1-100)", async () => { + test("placeMapNpc - rechaza coordenadas fuera de rango (1-100)", async () => { const resZero = await placeMapNpc(tempDir, { mapNum: 1, x: 0, y: 50, npcIndex: 1 }); assert.equal(resZero.ok, false); @@ -61,7 +62,7 @@ test("mapNpcPlacement - colocación y persistencia de NPCs (#8)", async (t) => { assert.equal(resOver.ok, false); }); - await t.test("placeMapNpc - rechaza npcIndex inválido o inexistente", async () => { + test("placeMapNpc - rechaza npcIndex inválido o inexistente", async () => { const resInvalid = await placeMapNpc(tempDir, { mapNum: 1, x: 10, y: 10, npcIndex: -5 }); assert.equal(resInvalid.ok, false); @@ -76,7 +77,7 @@ test("mapNpcPlacement - colocación y persistencia de NPCs (#8)", async (t) => { } }); - await t.test("placeMapNpc - rechaza colocación sobre tile bloqueado", async () => { + test("placeMapNpc - rechaza colocación sobre tile bloqueado", async () => { const blockedTile = (x: number, y: number) => x === 20 && y === 20; const res = await placeMapNpc( tempDir, @@ -90,7 +91,7 @@ test("mapNpcPlacement - colocación y persistencia de NPCs (#8)", async (t) => { } }); - await t.test("placeMapNpc - rechaza apilar dos NPCs en el mismo tile", async () => { + test("placeMapNpc - rechaza apilar dos NPCs en el mismo tile", async () => { await placeMapNpc(tempDir, { mapNum: 2, x: 15, y: 15, npcIndex: 1 }); const resDup = await placeMapNpc(tempDir, { mapNum: 2, x: 15, y: 15, npcIndex: 2 }); @@ -100,7 +101,7 @@ test("mapNpcPlacement - colocación y persistencia de NPCs (#8)", async (t) => { } }); - await t.test("placeMapNpc - respeta el límite máximo por mapa", async () => { + test("placeMapNpc - respeta el límite máximo por mapa", async () => { const mapNum = 3; for (let i = 1; i <= 3; i++) { await placeMapNpc(tempDir, { mapNum, x: i, y: 10, npcIndex: 1 }, { maxNpcs: 3 }); @@ -117,7 +118,28 @@ test("mapNpcPlacement - colocación y persistencia de NPCs (#8)", async (t) => { } }); - await t.test("moveMapNpc - mueve un NPC existente a otra coordenada válida", async () => { + test("placeMapNpc - serializa escrituras concurrentes sin perder NPCs", async () => { + const mapNum = 7; + const [first, second] = await Promise.all([ + placeMapNpc(tempDir, { mapNum, x: 10, y: 20, npcIndex: 1 }), + placeMapNpc(tempDir, { mapNum, x: 11, y: 20, npcIndex: 2 }), + ]); + + assert.equal(first.ok, true); + assert.equal(second.ok, true); + + const persisted = await loadMapNpcPlacements(tempDir, mapNum); + assert.equal(persisted.length, 2); + assert.deepEqual( + persisted.map(({ x, y, npcIndex }) => ({ x, y, npcIndex })), + [ + { x: 10, y: 20, npcIndex: 1 }, + { x: 11, y: 20, npcIndex: 2 }, + ], + ); + }); + + test("moveMapNpc - mueve un NPC existente a otra coordenada válida", async () => { const mapNum = 4; await placeMapNpc(tempDir, { mapNum, x: 10, y: 10, npcIndex: 5 }); @@ -130,7 +152,7 @@ test("mapNpcPlacement - colocación y persistencia de NPCs (#8)", async (t) => { assert.equal(loaded[0].y, 14); }); - await t.test("moveMapNpc - mover a la misma casilla actual no falla (caso autodesplazamiento)", async () => { + test("moveMapNpc - mover a la misma casilla actual no falla (caso autodesplazamiento)", async () => { const mapNum = 4; const selfMoveRes = await moveMapNpc(tempDir, mapNum, 12, 14, 12, 14); assert.equal(selfMoveRes.ok, true); @@ -141,7 +163,7 @@ test("mapNpcPlacement - colocación y persistencia de NPCs (#8)", async (t) => { assert.equal(loaded[0].y, 14); }); - await t.test("moveMapNpc - rechaza mover a un tile bloqueado u ocupado", async () => { + test("moveMapNpc - rechaza mover a un tile bloqueado u ocupado", async () => { const mapNum = 5; await placeMapNpc(tempDir, { mapNum, x: 10, y: 10, npcIndex: 1 }); await placeMapNpc(tempDir, { mapNum, x: 11, y: 10, npcIndex: 2 }); @@ -161,7 +183,7 @@ test("mapNpcPlacement - colocación y persistencia de NPCs (#8)", async (t) => { assert.equal(resBlocked.ok, false); }); - await t.test("removeMapNpc - quita un NPC y actualiza el archivo", async () => { + test("removeMapNpc - quita un NPC y actualiza el archivo", async () => { const mapNum = 6; await placeMapNpc(tempDir, { mapNum, x: 5, y: 5, npcIndex: 1 }); await placeMapNpc(tempDir, { mapNum, x: 6, y: 6, npcIndex: 2 });