From 29b3a4aebd97c787d87136e69fd42d1046262340 Mon Sep 17 00:00:00 2001 From: troytt <47798984@qq.com> Date: Mon, 14 Sep 2026 23:56:49 +0000 Subject: [PATCH] =?UTF-8?q?fix(render):=20NPC=20=E6=9C=89=E7=AB=8B?= =?UTF-8?q?=E7=BB=98=E6=97=B6=E4=B8=8D=E5=86=8D=E5=8F=A0=E5=8A=A0=E7=BB=BF?= =?UTF-8?q?=E8=89=B2=E5=8D=A0=E4=BD=8D=E6=96=B9=E5=9D=97=20(refs=20#6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 城镇 NPC 补齐之后，罗格营地一个分区会出现 19 个绿色方块糊在正常立绘上面。 起因是两套绘制路径各自独立判断：`buildPackRuntime` 用 `if (obj.frame)` 把 NPC 并入 `objectDrawables` 画出真实立绘，而实体层对 `engine.npcEntities` 里的每一个 NPC **无条件**再画一个 16x26 的纯绿占位方块。在只有凯恩一个 NPC 的年代这看不出问题，NPC 数量从 103 涨到 889 之后就藏不住了。 修复不是在实体层加一个平行的判断——那只会把同一个条件抄成两份，早晚漂移。 改为把判断抽成 `hasPackedSprite` 放进 `src/game/npc.ts`，两条路径共用同一个 函数：立绘层为真时画图，占位层为假时画框，互斥由函数本身保证。 该函数写成类型守卫，立绘层因此可以在同一次调用里把 `frame` 收窄成非空， 省掉原本那个多余的二次判空，也就不存在第二个会漂移的判断了。 `NpcEntity` 增加 `hasSprite` 字段，必填而非可选，这样将来新增放置点必须 表态。`GameEngine` 构造函数里那圈合成放置点显式标记为 `false`——它们本来 就没有预烘焙美术，占位方块对它们仍然是唯一的可见形式。金色名字标签两类 都保留，可交互提示不受影响。 顺带把 `npcEntities` 的行内结构类型换成已导出的 `NpcEntity`，避免同一个 形状在同一个文件里存在两份定义。 新增 `tests/npc-placeholder.test.ts`：除守卫本身的行为与收窄能力外，还把 全部已打包 NPC 过一遍，断言这个划分是完备的（每个 NPC 恰好落入一条路径， 不重不漏），并钉住罗格营地四个分区的 NPC 名册。守卫锚定在它真正读取的 `samples/d2-packs/index.json` 上。 --- src/game/engine.ts | 20 ++++++-- src/game/npc.ts | 17 +++++++ src/scene/act-scene.ts | 12 +++-- tests/npc-placeholder.test.ts | 91 +++++++++++++++++++++++++++++++++++ 4 files changed, 134 insertions(+), 6 deletions(-) create mode 100644 tests/npc-placeholder.test.ts 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) + } +})