diff --git a/src/game/engine.ts b/src/game/engine.ts index e61bd54..92e039d 100644 --- a/src/game/engine.ts +++ b/src/game/engine.ts @@ -69,7 +69,7 @@ export class GameEngine { castRng: Rng skillCooldown = 0 selectedSkill = 0 - npcEntities: { def: NpcDef; x: number; y: number }[] = [] + npcEntities: NpcEntity[] = [] dialog: string[] = [] metrics: GameEngineMetrics = { @@ -93,7 +93,9 @@ export class GameEngine { if (this.terrain.overlap(x, y) > 0) continue const def = this.opts.npcDefs[this.npcEntities.length] if (def === undefined) break - this.npcEntities.push({ def, x, y }) + // These are the synthetic ring placements used when a scene carries no + // prebaked NPC art, so they never have a sprite of their own to draw. + this.npcEntities.push({ def, x, y, hasSprite: false }) } } @@ -243,7 +245,19 @@ export class GameEngine { } } -export interface NpcEntity { def: NpcDef; x: number; y: number } +export interface NpcEntity { + def: NpcDef + x: number + y: number + /** + * Whether this NPC's own art is already drawn by the scene's sprite layer. + * + * The renderer falls back to a coloured placeholder box for NPCs without art. + * Once real art exists the box would sit on top of the sprite, so the two + * paths are mutually exclusive and the renderer needs to know which applies. + */ + hasSprite: boolean +} export interface EngineViewState { monstersSpawned?: number diff --git a/src/game/npc.ts b/src/game/npc.ts index 38f585c..3ee94a4 100644 --- a/src/game/npc.ts +++ b/src/game/npc.ts @@ -20,6 +20,23 @@ export function resolveNpcName(token: string, fallbackName: string): string { return fallbackName } +/** + * Whether a packed scene entry carries its own sprite frame. + * + * Two render passes branch on this: the object/sprite layer draws the entry only + * when it is true, and the placeholder-box layer draws only when it is false. + * They must agree exactly or an NPC ends up with a box painted over its art, so + * the test lives here once instead of being copied into both call sites. + * + * Declared as a type guard so the sprite layer gets `frame` narrowed to + * non-null from the same call that gates it, leaving no second check to drift. + */ +export function hasPackedSprite( + entry: { readonly frame?: TFrame | null } +): entry is { readonly frame: TFrame } { + return entry.frame !== undefined && entry.frame !== null +} + export function buildNpcDef(token: string, id: number, fallbackName: string): NpcDef { const name = resolveNpcName(token, fallbackName) return { diff --git a/src/scene/act-scene.ts b/src/scene/act-scene.ts index 16f0e71..e6b9510 100644 --- a/src/scene/act-scene.ts +++ b/src/scene/act-scene.ts @@ -34,7 +34,7 @@ import { depthInsertIndex } from '../game/map.ts' import { GameEngine, syncEngineState } from "../game/engine.ts" import type { NpcEntity } from "../game/engine.ts" import { DEMO_MONSTERS, DEMO_EXPERIENCE, DEMO_BASES, DEMO_AFFIXES, DEMO_SKILLS, DEMO_NPCS, DEMO_QUESTS } from "../game/demo-data.ts" -import { buildNpcDef } from '../game/npc.ts' +import { buildNpcDef, hasPackedSprite } from '../game/npc.ts' import type { NpcDef } from '../game/quests.ts' import { buildAtlas } from '../render/atlas.ts' import type { AtlasFrame, SpriteAtlas } from '../render/atlas.ts' @@ -360,7 +360,7 @@ async function loadPackRuntime( const objectDrawables: ObjectDrawable[] = [] const packObjects = [...scene.objects, ...(scene.npcs ?? [])] for (const obj of packObjects) { - if (obj.frame) { + if (hasPackedSprite(obj)) { objectDrawables.push({ frame: { x: obj.frame.x, @@ -763,7 +763,9 @@ function runScene(runtime: MapRuntime, renderer: SpriteRenderer, started: number engine.npcEntities.length = 0 for (const n of runtime.npcs) { const def = npcDefs.find(d => d.id === `npc-${String(n.id)}`)! - engine.npcEntities.push({ def, x: n.x, y: n.y }) + // Same predicate the sprite layer gated on above, so the box and the art + // can never both be drawn for one NPC. + engine.npcEntities.push({ def, x: n.x, y: n.y, hasSprite: hasPackedSprite(n) }) } const loop = new GameLoop({ @@ -896,6 +898,10 @@ function runScene(runtime: MapRuntime, renderer: SpriteRenderer, started: number } for (const npc of engine.npcEntities) { + // Sprite-backed NPCs are already painted by the object layer; the box + // is only here so an art-less NPC still shows up and stays clickable. + // The name label below is drawn for both kinds. + if (npc.hasSprite) continue pushEntity(npc.x, npc.y, () => { renderer.drawSolid(npc.x - MARKER_WIDTH / 2, npc.y - MARKER_HEIGHT, MARKER_WIDTH, MARKER_HEIGHT, [0.2, 0.9, 0.2, 1]) }) diff --git a/tests/npc-placeholder.test.ts b/tests/npc-placeholder.test.ts new file mode 100644 index 0000000..f041c9b --- /dev/null +++ b/tests/npc-placeholder.test.ts @@ -0,0 +1,91 @@ +import { test, expect } from 'vitest' +import { existsSync, readFileSync } from 'node:fs' +import { join } from 'node:path' +import { hasPackedSprite } from '../src/game/npc.ts' + +const PACK_ROOT = 'samples/d2-packs' +const PACK_INDEX = join(PACK_ROOT, 'index.json') +/** Guard on the exact file these tests open, not on the directory above it. */ +const noPack = !existsSync(PACK_INDEX) + +interface PackedNpc { + readonly token?: string + readonly frame?: unknown +} + +interface IndexLevel { + readonly path: string + readonly label: string + readonly npcs: number +} + +function readLevels(): readonly IndexLevel[] { + const index = JSON.parse(readFileSync(PACK_INDEX, 'utf8')) as { readonly levels: readonly IndexLevel[] } + return index.levels +} + +function readNpcs(levelPath: string): readonly PackedNpc[] { + const scene = JSON.parse(readFileSync(join(PACK_ROOT, levelPath, 'scene.json'), 'utf8')) as { + readonly npcs?: readonly PackedNpc[] + } + return scene.npcs ?? [] +} + +test('hasPackedSprite treats a missing or null frame as art-less', () => { + expect(hasPackedSprite({ frame: { page: 0 } })).toBe(true) + expect(hasPackedSprite({ frame: null })).toBe(false) + expect(hasPackedSprite({})).toBe(false) +}) + +test('hasPackedSprite narrows frame so callers need no second check', () => { + const entry: { readonly frame?: { readonly page: number } | null } = { frame: { page: 7 } } + if (!hasPackedSprite(entry)) throw new Error('expected the guard to admit this entry') + // Reading .page without a null check is the whole point of the guard: if the + // predicate ever stops narrowing, this line stops compiling. + expect(entry.frame.page).toBe(7) +}) + +/** + * The placeholder box and the sprite are drawn by two separate passes that both + * branch on `hasPackedSprite`. Running every real packed NPC through it proves + * the partition is total — each NPC lands in exactly one pass, never both and + * never neither — which is what keeps a green box off finished art. + */ +test.skipIf(noPack)('every packed NPC is classified into exactly one render pass', () => { + let total = 0 + let withSprite = 0 + const artless: string[] = [] + + for (const level of readLevels()) { + if (level.npcs === 0) continue + for (const npc of readNpcs(level.path)) { + total += 1 + if (hasPackedSprite(npc)) withSprite += 1 + else artless.push(`${level.path}:${npc.token ?? '?'}`) + } + } + + expect(total).toBeGreaterThan(0) + // The packer already refuses to bake an art-less NPC, so a survivor here would + // mean a placeholder box is being drawn where real art was expected. + expect(artless).toEqual([]) + expect(withSprite).toBe(total) +}) + +test.skipIf(noPack)('act 1 town quadrants carry their full NPC roster', () => { + const townLevels = readLevels().filter(l => l.label.includes('act-1-town')) + expect(townLevels.length).toBe(4) + + const tokensSeen = new Set() + for (const level of townLevels) { + for (const npc of readNpcs(level.path)) { + if (npc.token !== undefined) tokensSeen.add(npc.token) + } + } + + // Regression guard for the reported 'npc不全': before DS1 type-1 entries were + // resolved through MonPreset, only Cain (DC) survived in the Rogue Encampment. + for (const token of ['WA', 'RC', 'PS', 'CI', 'GH', 'DC']) { + expect(tokensSeen.has(token)).toBe(true) + } +})