feat: add test for mob aggro

6 files changed, 120 insertions, 64 deletions

diff --git a/internal/engine/tick.go b/internal/engine/tick.go
index 251373c..7b9666b 100644
--- a/internal/engine/tick.go
+++ b/internal/engine/tick.go
@@ -86,17 +86,10 @@ func (e *Engine) Stop() {
 	}
 }
 
-// processTick fires every due subscriber for the current engine tick.
-//
-// Subscribers are invoked in ascending subscription-ID order, i.e. the order in
-// which they were registered with Subscribe. IDs are allocated monotonically
-// (e.nextID++), so subscription order is a stable, deterministic sequence:
-// the bootstrap master subscriber (cmd/thoi/main.go) subscribes at startup as
-// ID 1 and therefore always fires first each tick, followed by each
-// subsequently-registered tick (combat rounds, aggro rolls, scheduled respawns,
-// etc.) in the order they registered. This guarantees predictable per-tick
-// ordering across runs — critical for features whose behaviour depends on the
-// relative ordering of combat resolution vs. movement vs. aggro checks.
+// Tick fires every due subscriber synchronously and returns. Subscribers
+// are invoked in ascending subscription-ID (registration) order.
+func (e *Engine) Tick() { e.processTick() }
+
 func (e *Engine) processTick() {
 	e.mu.Lock()
 	snapshot := make(map[uint64]*subscriber, len(e.subscribers))
diff --git a/internal/game/act_state_test.go b/internal/game/act_state_test.go
new file mode 100644
index 0000000..0ec139f
--- /dev/null
+++ b/internal/game/act_state_test.go
@@ -0,0 +1,22 @@
+package game
+
+import (
+	"testing"
+
+	"thehouseoficarus/internal/player"
+)
+
+func TestPlayerActionDisplayFightingInCombat(t *testing.T) {
+	g := escapeTestGame()
+	inst := spawnTestMob(g, "skeleton", 100)
+	p := player.New("combatant")
+	p.EscapeDir = "north"
+
+	g.Combat.Enter(p.Name, inst.InstanceID)
+
+	display := g.playerActionDisplay(p)
+	want := "fighting a " + inst.Name
+	if display != want {
+		t.Errorf("playerActionDisplay = %q, want %q", display, want)
+	}
+}
diff --git a/internal/game/sys_combat.go b/internal/game/sys_combat.go
index e23d94b..e05cfed 100644
--- a/internal/game/sys_combat.go
+++ b/internal/game/sys_combat.go
@@ -68,14 +68,12 @@ func (g *Game) dropItemsOnDeath(p *player.Player) {
 	}
 }
 
-// aggroChancePerTick is the probability per engine tick that an aggressive mob
-// in the player's room initiates combat against them. The first roll happens
-// inline on arrival (so a player who enters and immediately moves again — the
-// 1-tick single-move case — still risks being caught ~33% of the time); if the
-// initial roll fails, a per-tick retry subscriber keeps rolling for any
-// subsequent tick the player lingers in the room.
+// aggroChancePerTick is the probability per engine tick that an aggressive
+// mob in the player's room initiates combat.
 const aggroChancePerTick = 0.33
 
+var rollAggroRand = rand.Float64
+
 func (g *Game) checkAggro(sess *net.Session) {
 	p := sess.Player
 
@@ -106,13 +104,6 @@ func (g *Game) checkAggro(sess *net.Session) {
 		}
 		aggMob := mob
 
-		// rollAggro attempts to start combat this instant. It returns true
-		// when aggro should stop retrying that mob for this player — either
-		// because it succeeded, because the mob/player is no longer eligible,
-		// or because the roll failed and the caller subscribed a per-tick
-		// retry. The retry subscriber returns the negation of the inline
-		// call's "stop" so it stays subscribed only while the player is
-		// still a valid target the roll hasn't yet hit.
 		rollAggro := func() (stop bool) {
 			if g.Combat.Get(p.Name) != nil {
 				return true
@@ -123,7 +114,7 @@ func (g *Game) checkAggro(sess *net.Session) {
 			if g.Combat.IsMobInCombat(aggMob.InstanceID) {
 				return true
 			}
-			if rand.Float64() >= aggroChancePerTick {
+			if rollAggroRand() >= aggroChancePerTick {
 				return false
 			}
 			attacker := aggMob.Name
diff --git a/internal/game/sys_combat_test.go b/internal/game/sys_combat_test.go
new file mode 100644
index 0000000..788e62c
--- /dev/null
+++ b/internal/game/sys_combat_test.go
@@ -0,0 +1,86 @@
+package game
+
+import (
+	"testing"
+
+	"thehouseoficarus/internal/behavior"
+	"thehouseoficarus/internal/player"
+	"thehouseoficarus/internal/world"
+)
+
+func spawnTestMob(g *Game, defID string, roomID int) *world.MobInstance {
+	def, err := g.MobStore.LoadDef(defID)
+	if err != nil {
+		panic("spawnTestMob: " + err.Error())
+	}
+	cfg := &behavior.SpawnMobConfig{ID: defID}
+	return g.MobStore.SpawnTransient(def, cfg, roomID, "")
+}
+
+func patchAggroRand(val float64) func() {
+	orig := rollAggroRand
+	rollAggroRand = func() float64 { return val }
+	return func() { rollAggroRand = orig }
+}
+
+func TestAggroInlineRollFiresOnArrival(t *testing.T) {
+	defer patchAggroRand(0.0)()
+
+	g := escapeTestGame()
+	spawnTestMob(g, "skeleton", 100)
+	p := player.New("agro_fire")
+	p.RoomID = 100
+	sess := escapeTestSession(p)
+
+	g.checkAggro(sess)
+
+	if g.Combat.Get(p.Name) == nil {
+		t.Error("aggro should have started combat on inline roll")
+	}
+}
+
+func TestAggroPerTickRetrySubscriber(t *testing.T) {
+	t.Run("miss_then_retry_fires", func(t *testing.T) {
+		defer patchAggroRand(0.5)()
+
+		g := escapeTestGame()
+		spawnTestMob(g, "skeleton", 200)
+		p := player.New("agro_retry")
+		p.RoomID = 200
+		sess := escapeTestSession(p)
+
+		g.checkAggro(sess)
+
+		if g.Combat.Get(p.Name) != nil {
+			t.Fatal("inline roll should have missed (val=0.5)")
+		}
+
+		rollAggroRand = func() float64 { return 0.0 }
+		g.Ticks.Tick()
+
+		if g.Combat.Get(p.Name) == nil {
+			t.Error("per-tick retry subscriber should have started combat")
+		}
+	})
+
+	t.Run("miss_then_ineligible_unsubscribes", func(t *testing.T) {
+		defer patchAggroRand(0.5)()
+
+		g := escapeTestGame()
+		spawnTestMob(g, "skeleton", 300)
+		p := player.New("agro_leave")
+		p.RoomID = 300
+		sess := escapeTestSession(p)
+
+		g.checkAggro(sess)
+
+		if g.Combat.Get(p.Name) != nil {
+			t.Fatal("inline roll should have missed (val=0.5)")
+		}
+		p.RoomID = 999
+		g.Ticks.Tick()
+		if g.Combat.Get(p.Name) != nil {
+			t.Error("retry should have self-unsubscribed, no combat started")
+		}
+	})
+}
diff --git a/internal/game/sys_energy_test.go b/internal/game/sys_energy_test.go
index a1a5dab..72ae223 100644
--- a/internal/game/sys_energy_test.go
+++ b/internal/game/sys_energy_test.go
@@ -120,24 +120,6 @@ func TestMoveTicks(t *testing.T) {
 	}
 }
 
-func TestGracefulCountCapeExcludesFullSet(t *testing.T) {
-	// Cape of Agility shares the back slot — 5 graceful pieces + cape = 5,
-	// not 6, so no full-set bonus.
-	p := player.New("h")
-	p.Equipment[item.SlotBack] = capeOfAgilityID
-	p.Equipment[item.SlotHead] = "graceful_hat"
-	p.Equipment[item.SlotTorso] = "graceful_torso"
-	p.Equipment[item.SlotLegs] = "graceful_legs"
-	p.Equipment[item.SlotHands] = "graceful_gloves"
-	p.Equipment[item.SlotFeet] = "graceful_boots"
-	if got := gracefulCount(p); got != 5 {
-		t.Errorf("5 graceful + cape: count=%d want 5", got)
-	}
-	if hasCapeOfAgility(p) != true {
-		t.Error("should detect cape of agility")
-	}
-}
-
 func TestEnergyRegenIdle(t *testing.T) {
 	g := energyTestGame()
 	p := player.New("idle")
diff --git a/internal/game/sys_escape_test.go b/internal/game/sys_escape_test.go
index f8cd2e1..a9842ba 100644
--- a/internal/game/sys_escape_test.go
+++ b/internal/game/sys_escape_test.go
@@ -128,11 +128,6 @@ func TestBeginEscapeMoveWithCapeOfAgility(t *testing.T) {
 	p := player.New("cape")
 	p.Equipment[item.SlotBack] = capeOfAgilityID
 
-	// moveTicks returns 0 for Cape of Agility. beginEscapeMove is only ever
-	// called when an escape has been stashed, and Cape-of-Agility / GodMode
-	// never enter the stash path (doMove's ticks<=0 short-circuit fires
-	// first), so this test just verifies the moveTicks call site compiles
-	// against the two-value signature.
 	ticks, usesEnergy := g.moveTicks(p, false)
 	if ticks != 0 {
 		t.Errorf("Cape of Agility should give 0 ticks, got %d", ticks)
@@ -247,11 +242,8 @@ func TestEscapeClearedOnStop(t *testing.T) {
 	}
 }
 
-// TestPlayerActionDisplayNoPreparingToFlee verifies the (previously-unreachable)
-// "preparing to flee" display branch has been removed. While waiting to flee
-// the player is still in combat, so playerActionDisplay's in-combat branch
-// governs (returning "fighting a X" — exercised indirectly through combat
-// tests). A pure EscapeDir set with no combat engagement now yields "".
+// TestPlayerActionDisplayNoPreparingToFlee verifies the
+// "preparing to flee" display branch has been removed.
 func TestPlayerActionDisplayNoPreparingToFlee(t *testing.T) {
 	g := escapeTestGame()
 	p := player.New("fleer")
@@ -281,9 +273,6 @@ func TestEscapeReliefFiresOnThirdHit(t *testing.T) {
 	g.Combat.Enter(p.Name, "relief_mob")
 	sess := escapeTestSession(p)
 
-	// Three escape-canceling hits. Between hits the test re-arms the escape
-	// (in the real flow the player re-issues a direction). maxHit=1 makes
-	// RollDamage deterministic (always 1) so HP=100 survives cleanly.
 	for i := 0; i < 3; i++ {
 		p.EscapeDir = "north"
 		p.EscapeTarget = 11
@@ -411,14 +400,7 @@ func TestEscapeFailCountResetsOnCombatEntry(t *testing.T) {
 }
 
 // TestEscapeReliefSuppressedOnKillingBlow verifies that when the 3rd hit
-// would simultaneously kill the player (HP drops to 0), the relief is
-// suppressed — the player dies cleanly (the live mob-attack tick subscriber
-// notices HP<=0 and calls endCombat → killPlayer) rather than seeing a
-// misleading "power through" message.
-//
-// We exercise applyMobHit directly (no subscriber), so killPlayer is not
-// invoked here; we instead verify that the relief branch did NOT fire
-// (MoveTicks stays 0).
+// would kill the player, the relief is suppressed.
 func TestEscapeReliefSuppressedOnKillingBlow(t *testing.T) {
 	g := escapeTestGame()
 	p := player.New("doomed")