feat(combat): fix kill event lifecycle & single-drop gate (Fixes #404)

src/game/combat.ts

@@ -102,6 +102,8 @@ export interface CombatOptions {
   readonly respawnTicks: number
   /** When true, monsters ignore aggro distance and remain standing still (idle). */
   readonly disableMonsterAggro?: boolean
+  /** When true, world.events is preserved across ticks instead of cleared at tick start. */
+  readonly preserveEvents?: boolean | undefined
 }
 
 /** One monster's immutable numbers, read from a table row. */
@@ -185,6 +187,8 @@ export interface Monster {
   chillTicks?: number | undefined
   /** Ticks remaining poisoned (damage over time). */
   poisonTicks?: number | undefined
+  /** Whether drops have already been rolled for this monster instance. */
+  dropRolled?: boolean | undefined
 }
 
 /** The player's combat-relevant state. */
@@ -262,6 +266,8 @@ export interface CombatEvent {
   readonly text?: string
   /** Subject identity (a monster's table id on a kill, for quest counters). */
   readonly subjectId?: string
+  /** Stable monster index for kill and hit events. */
+  readonly monsterIndex?: number | undefined
   /** Monster level when event is 'kill'. */
   readonly monsterLevel?: number | undefined
   /** Monster rank when event is 'kill'. */
@@ -304,6 +310,8 @@ export interface CombatWorld {
   tick: number
   /** Events produced during the last tick (cleared each tick). */
   events: CombatEvent[]
+  /** Kill events pending drop resolution. */
+  pendingKills?: CombatEvent[] | undefined
   /** Monsters killed. */
   kills: number
 }
@@ -389,10 +397,14 @@ export function createWorld(x: number, y: number, maxHp = 60, maxMana = 30): Com
     monsters: [],
     tick: 0,
     events: [],
+    pendingKills: [],
     kills: 0,
   }
 }
 
+/** Alias for createWorld conforming to combat world nomenclature. */
+export const createCombatWorld = createWorld
+
 /**
  * Create one player body.
  *
@@ -730,6 +742,7 @@ function buildKillEvent(monster: Monster): CombatEvent {
     y: monster.y,
     text: monster.stats.name,
     subjectId: monster.stats.id,
+    monsterIndex: monster.index,
     monsterLevel: monster.stats.level,
     monsterRank: monster.stats.rank,
     monsterType,
@@ -746,8 +759,13 @@ export function damageMonster(world: CombatWorld, monsterIndex: number, amount:
   if (monster.hp > 0) return false
   monster.state = 'dead'
   monster.corpseTicks = 100
+  monster.dropRolled = monster.dropRolled ?? false
   world.kills += 1
-  world.events.push(buildKillEvent(monster))
+  const killEvent = buildKillEvent(monster)
+  world.events.push(killEvent)
+  if (world.pendingKills !== undefined) {
+    world.pendingKills.push(killEvent)
+  }
   attacker.xp += monster.stats.xp
   return true
 }
@@ -796,7 +814,9 @@ export function tickCombatMulti(
   xpTable: readonly number[],
 ): void {
   world.tick += 1
-  world.events = []
+  if (!options.preserveEvents) {
+    world.events = []
+  }
   // Who the monsters may attack is decided at the *start* of the tick. A player
   // who comes back to life during this tick is therefore safe in it, and a world
   // with nobody alive gives the monsters nothing to do: their cooldowns and the
@@ -902,8 +922,13 @@ function tickPlayer(
         if (target.hp <= 0) {
           target.state = 'dead'
           target.corpseTicks = 100
+          target.dropRolled = target.dropRolled ?? false
           world.kills += 1
-          world.events.push(buildKillEvent(target))
+          const killEvent = buildKillEvent(target)
+          world.events.push(killEvent)
+          if (world.pendingKills !== undefined) {
+            world.pendingKills.push(killEvent)
+          }
           player.xp += target.stats.xp
           // Level up while the threshold is crossed; the table is cumulative.
           while (player.level + 1 < xpTable.length && player.xp >= (xpTable[player.level + 1] ?? Number.POSITIVE_INFINITY)) {

src/game/engine.ts

@@ -370,6 +370,9 @@ export class GameEngine {
   }
 
   tick(input: EngineInput): void {
+    if (!this.world.pendingKills || this.world.pendingKills.length === 0) {
+      this.world.events = []
+    }
     const movement = input.movement
     const player = this.world.player
     const beforeX = player.x
@@ -417,7 +420,8 @@ export class GameEngine {
       }
     }
 
-    tickCombat(this.world, { movement, attack: castHappened ? false : input.attacking }, this.opts.combatOptions, this.terrain, this.opts.xpTable)
+    const combatOptions = { ...this.opts.combatOptions, preserveEvents: true }
+    tickCombat(this.world, { movement, attack: castHappened ? false : input.attacking }, combatOptions, this.terrain, this.opts.xpTable)
     
     if (this.world.events.some(event => event.kind === 'monsterHit')) this.metrics.playerHits += 1
 
@@ -857,8 +861,22 @@ export class GameEngine {
       }
     }
 
+    if (this.world.pendingKills && this.world.pendingKills.length > 0) {
+      for (const kill of this.world.pendingKills) {
+        if (!this.world.events.includes(kill)) {
+          this.world.events.push(kill)
+        }
+      }
+      this.world.pendingKills = []
+    }
+
     for (const event of this.world.events) {
       if (event.kind !== 'kill') continue
+      const monster = event.monsterIndex !== undefined
+        ? this.world.monsters.find(m => m.index === event.monsterIndex)
+        : this.world.monsters.find(m => m.stats.id === event.subjectId && Math.hypot(m.x - event.x, m.y - event.y) < 10)
+      if (monster?.dropRolled) continue
+      if (monster) monster.dropRolled = true
       for (const reward of this.questLog.recordKill(event.subjectId ?? '')) {
         player.xp += reward.xp
         this.bag.add(goldItem(reward.gold))

src/scene/act-scene.ts

@@ -5627,6 +5627,9 @@ if (typeof window !== 'undefined') {
   const loop = new GameLoop({
     tickRate: 25,
     onTick: () => {
+      // Clean tick boundary: flush previous tick events before mouseController runs
+      engine.world.events = []
+      if (engine.world.pendingKills) engine.world.pendingKills = []
       const mouseStep = mouseController.tick()
       const effectiveMovement = mouseStep.movement
       if (hudManager) {

tests/combat-kill-lifecycle.test.ts

@@ -0,0 +1,293 @@
+import { describe, it, expect, beforeAll } from 'vitest'
+import * as fs from 'fs'
+import { MountedArchives } from '../src/mpq/mount.ts'
+import { MpqArchive } from '../src/mpq/archive.ts'
+import { fileSource } from '../src/mpq/file-source.ts'
+import { loadDropTables, type DropTables } from '../src/game/drop-pipeline.ts'
+import { GameEngine, type GameEngineOptions } from '../src/game/engine.ts'
+import { damageMonster, type Monster } from '../src/game/combat.ts'
+import { DEMO_EXPERIENCE, DEMO_NPCS, DEMO_QUESTS } from '../src/game/demo-data.ts'
+
+const hasD2 = fs.existsSync('samples/d2/d2data.mpq')
+
+describe('Combat Kill Event Lifecycle & Single-Drop Gate (Issue #404)', () => {
+  let dropTables: DropTables
+
+  beforeAll(async () => {
+    if (!hasD2) {
+      throw new Error('samples/d2 MPQ files required for Combat Kill Lifecycle test')
+    }
+    const archives = new MountedArchives()
+    for (const name of ['d2data.mpq', 'd2exp.mpq', 'Patch_D2.mpq']) {
+      archives.add(name, await MpqArchive.open(await fileSource(`samples/d2/${name}`)))
+    }
+    dropTables = await loadDropTables(archives)
+  })
+
+  const dummyTerrain = {
+    overlap: () => 0,
+    isMissileBlocked: () => false,
+  }
+
+  const DUMMY_INPUT = {
+    movement: { x: 0, y: 0 },
+    attacking: false,
+    pickingUp: false,
+    talking: false,
+    digits: [],
+    saving: false,
+    loading: false,
+  }
+
+  function createMonster(index: number, x: number, y: number, hp = 20): Monster {
+    return {
+      index,
+      stats: {
+        id: 'fallen1',
+        name: 'Fallen',
+        hp,
+        damage: 2,
+        cooldownTicks: 25,
+        reach: 30,
+        aggroRadius: 100,
+        speed: 60,
+        xp: 20,
+        rank: 'normal',
+        level: 5,
+      },
+      x,
+      y,
+      hp,
+      cooldown: 0,
+      state: 'idle',
+      facing: 0,
+      hitFlash: 0,
+      corpseTicks: 0,
+      dropRolled: false,
+    }
+  }
+
+  function createTestEngine(): GameEngine {
+    const engineOpts: GameEngineOptions = {
+      spawn: { x: 100, y: 100 },
+      combatPlayer: { x: 100, y: 100, maxHp: 100, maxMana: 100 },
+      stats: [
+        {
+          id: 'fallen1',
+          name: 'Fallen',
+          hp: 20,
+          damage: 2,
+          cooldownTicks: 25,
+          reach: 30,
+          aggroRadius: 100,
+          speed: 60,
+          xp: 20,
+        },
+      ],
+      xpTable: DEMO_EXPERIENCE,
+      dropTables,
+      difficulty: 'normal',
+      monsterKinds: dropTables.monsterKinds,
+      skills: [
+        {
+          id: 'attack',
+          name: 'Attack',
+          manaCost: 0,
+          cooldownTicks: 1,
+          radius: 48,
+        },
+      ],
+      npcDefs: DEMO_NPCS,
+      questDefs: DEMO_QUESTS,
+      combatOptions: {
+        playerSpeed: 100,
+        playerReach: 60,
+        playerCooldownTicks: 1,
+        playerDamage: 50,
+        playerManaPerAttack: 0,
+        respawnTicks: 100,
+      },
+      talkRadius: 80,
+      pickupRadius: 64,
+      inventoryCols: 10,
+      inventoryRows: 4,
+      monsterCount: 1,
+      monsterSpread: 50,
+    }
+
+    const engine = new GameEngine(dummyTerrain, engineOpts)
+    engine.world.monsters = [createMonster(0, 110, 100, 20)]
+    return engine
+  }
+
+  // --- Case 1: Pre-tick melee attack kill produces drops ---
+  it('Case 1: Pre-tick melee attack kill produces drops', () => {
+    const engine = createTestEngine()
+    const targetMonster = engine.world.monsters[0]!
+    expect(targetMonster.state).toBe('idle')
+    expect(targetMonster.dropRolled).toBe(false)
+
+    // Execute pre-tick lethal melee damage via damageMonster
+    const killed = damageMonster(engine.world, targetMonster.index, 100, engine.world.player)
+    expect(killed).toBe(true)
+    expect(targetMonster.state).toBe('dead')
+    expect(engine.world.pendingKills?.length).toBe(1)
+
+    const initialDropsRolled = engine.metrics.dropsRolled
+
+    // Run tick to process drops
+    engine.tick(DUMMY_INPUT)
+
+    // Drops must have rolled and monster.dropRolled must be true
+    expect(engine.metrics.dropsRolled).toBe(initialDropsRolled + 1)
+    expect(targetMonster.dropRolled).toBe(true)
+    expect(engine.world.events.some(e => e.kind === 'kill' && e.monsterIndex === targetMonster.index)).toBe(true)
+  })
+
+  // --- Case 2: Pre-tick instant AoE skill kill (Frost Nova / Nova) produces drops ---
+  it('Case 2: Pre-tick instant AoE skill kill (Frost Nova / Nova) produces drops', () => {
+    const engine = createTestEngine()
+    // Spawn 3 monsters around player
+    engine.world.monsters = [
+      createMonster(0, 120, 100, 20),
+      createMonster(1, 80, 100, 20),
+      createMonster(2, 100, 120, 20),
+    ]
+
+    // Configure Nova (instant AoE skill)
+    engine.opts.skills.push({
+      id: 'nova',
+      name: 'Nova',
+      manaCost: 0,
+      cooldownTicks: 1,
+      radius: 80,
+      damage: 50, // Lethal to 20 hp monsters
+    })
+    engine.selectedSkill = engine.opts.skills.length - 1
+
+    const initialDropsRolled = engine.metrics.dropsRolled
+
+    // Tick with attacking = true to trigger instant skill cast
+    engine.tick({
+      ...DUMMY_INPUT,
+      attacking: true,
+    })
+
+    // All 3 monsters must be killed and have drops rolled
+    expect(engine.world.monsters[0]!.state).toBe('dead')
+    expect(engine.world.monsters[1]!.state).toBe('dead')
+    expect(engine.world.monsters[2]!.state).toBe('dead')
+
+    expect(engine.world.monsters[0]!.dropRolled).toBe(true)
+    expect(engine.world.monsters[1]!.dropRolled).toBe(true)
+    expect(engine.world.monsters[2]!.dropRolled).toBe(true)
+
+    expect(engine.metrics.dropsRolled).toBe(initialDropsRolled + 3)
+    const killEvents = engine.world.events.filter(e => e.kind === 'kill')
+    expect(killEvents.length).toBe(3)
+  })
+
+  // --- Case 3: Missile projectile kill (direct & splash) produces drops ---
+  it('Case 3: Missile projectile kill (direct & splash) produces drops', () => {
+    const engine = createTestEngine()
+    // Direct target at (150, 100) and splash target nearby at (160, 100)
+    const directTarget = createMonster(0, 150, 100, 20)
+    const splashTarget = createMonster(1, 160, 100, 20)
+    engine.world.monsters = [directTarget, splashTarget]
+
+    // Spawn a projectile with aoeRadius targeting (150, 100)
+    engine.projectiles.push({
+      skillId: '36',
+      x: 148,
+      y: 100,
+      vx: 4,
+      vy: 0,
+      damage: 50,
+      ttl: 10,
+      fromPlayer: true,
+      missileType: 'firebolt',
+      pierce: false,
+      aoeRadius: 40,
+    })
+
+    const initialDropsRolled = engine.metrics.dropsRolled
+
+    // Advance 1 tick: missile collides with directTarget and splashes to splashTarget
+    engine.tick(DUMMY_INPUT)
+
+    expect(directTarget.state).toBe('dead')
+    expect(splashTarget.state).toBe('dead')
+
+    expect(directTarget.dropRolled).toBe(true)
+    expect(splashTarget.dropRolled).toBe(true)
+
+    expect(engine.metrics.dropsRolled).toBe(initialDropsRolled + 2)
+    const killEvents = engine.world.events.filter(e => e.kind === 'kill')
+    expect(killEvents.length).toBe(2)
+  })
+
+  // --- Case 4: Periodic aura pulse kill (Holy Freeze / Holy Fire) produces drops ---
+  it('Case 4: Periodic aura pulse kill (Holy Freeze / Holy Fire) produces drops', () => {
+    const engine = createTestEngine()
+    engine.levelId = 2 // Blood Moor (wilderness, not town)
+    const targetMonster = createMonster(0, 130, 100, 5) // 5 HP
+    engine.world.monsters = [targetMonster]
+
+    // Activate Holy Freeze aura (skill 114) at slvl 20
+    engine.updateActiveAura(114, 20)
+
+    // Advance tick to 49 so next tick is 50 (50 % 50 === 0 aura pulse)
+    engine.world.tick = 49
+
+    const initialDrops = engine.metrics.dropsRolled
+
+    engine.tick(DUMMY_INPUT)
+
+    expect(engine.world.tick).toBe(50)
+    expect(targetMonster.state).toBe('dead')
+    expect(targetMonster.dropRolled).toBe(true)
+    expect(engine.metrics.dropsRolled).toBe(initialDrops + 1)
+    expect(engine.world.events.some(e => e.kind === 'kill' && e.monsterIndex === targetMonster.index)).toBe(true)
+  })
+
+  // --- Case 5: Multi-damage concurrent hits & resurrected monster drop idempotency (drops strictly once) ---
+  it('Case 5: Multi-damage concurrent hits & resurrected monster drop idempotency (drops strictly once)', () => {
+    const engine = createTestEngine()
+    const monster = createMonster(0, 110, 100, 20)
+    engine.world.monsters = [monster]
+
+    const initialDrops = engine.metrics.dropsRolled
+
+    // 1. Concurrent hits: hit 1 kills monster, hit 2 strikes dead monster on same tick
+    const hit1Killed = damageMonster(engine.world, monster.index, 30, engine.world.player)
+    expect(hit1Killed).toBe(true)
+    expect(monster.state).toBe('dead')
+
+    // Hit 2 against already dead monster returns false and does not queue extra kill
+    const hit2Killed = damageMonster(engine.world, monster.index, 30, engine.world.player)
+    expect(hit2Killed).toBe(false)
+
+    // Tick to resolve drops
+    engine.tick(DUMMY_INPUT)
+
+    expect(engine.metrics.dropsRolled).toBe(initialDrops + 1)
+    expect(monster.dropRolled).toBe(true)
+
+    // 2. Resurrect the monster (e.g. by Shaman)
+    monster.state = 'idle'
+    monster.hp = 20
+    // Notice: monster.dropRolled remains true (1.13c UNITFLAG_NOITEM parity)
+    expect(monster.dropRolled).toBe(true)
+
+    // Kill resurrected monster a second time
+    const killedAgain = damageMonster(engine.world, monster.index, 50, engine.world.player)
+    expect(killedAgain).toBe(true)
+    expect(monster.state).toBe('dead')
+
+    // Advance tick: drop loop must skip drops because monster.dropRolled is already true
+    engine.tick(DUMMY_INPUT)
+
+    // dropsRolled must NOT have increased!
+    expect(engine.metrics.dropsRolled).toBe(initialDrops + 1)
+  })
+})