fix: pet obedience pacing, stale AI wake rescheduling, and Guard order persistence through combat (#2594)

Closes #2593. Closes #2595.

Two related pet-AI fixes: the post-#2591 pacing/wake regression (#2593), and the guard order silently converting to Attack during combat (#2595). Root-cause analyses are in the issues.

## #2593 — pets follow slowly; stale AITimer wakes

**Why pets slowed:**
- The per-step budget grew from **half a think interval** (`CurrentSpeed * 500`) to the full RunUO-parity move table (`CurrentMoveSpeed * 1000`). Medium-bucket pets (Horse, Dog, most tamables): passiveMove **1.05s/step**.
- Pet order speed depended on stale `Warmode`: `HandleGuardOrder` set it once, but `OnCombatantChange` clears it whenever the combatant drops, so obedience ran active or passive **by combat history** — usually passive. Net: Guard/Come at ~1.05s/step (~2.1x slower than pre-#2591), vs a player running at 0.1–0.2s/step.
- The AITimer never rescheduled its pending wheel entry: the wheel reads `Interval` only after the next fire, so a speed-up or a fresh order (`Activate()` no-ops while running) waited out the stale wake — up to a full passive think, stacked on the residual move budget on Guard → Follow.

**What changed:**
- **Order handlers own obedience speed** (RunUO `OnCurrentOrderChanged`/`DoOrder*` parity, re-derived continuously): issuing a movement order (Come/Follow/Guard/Attack) sets the **active** think clock, resting orders (Stay/None/Transfer) set passive, and the guard/follow peaceful branches write **RunUO's AOS `CurrentSpeed = 0.1` sprint** — RunUO's guard else-branch had the identical write as follow. The bespoke 0.1 fuses to both clocks through #2591's existing classification, so `CurrentMoveSpeed` stays **pure herding + classification** with no obedience special case, and `DoMoveImpl`'s per-step flip skips obeying pets (their handler owns the pace) and loses its old follow-only 0.1 write. Combat still re-derives organically via warmode/combatant.
- **`AITimer`**: tracks the pending wake and reschedules (`Stop`, `Delay` = remaining, `Start`) when a speed-up or fresh order moves the earliest deadline up; changes inside a tick still flow through `ScheduleNext`. New `Prod()` wakes the AI immediately on player commands — including from a stopped timer, so stable claims no longer wait out the random construction stagger. Sector/spawn wakes keep the stagger. Spam-safe: a prodded think grants reaction, never action — steps/swings/casts/abilities are gated by their own budgets and timers.

The residual move budget is deliberately **not** cleared on order change — that would let order-spam macros grant free steps. Deadline changes reschedule the timer; rate changes take effect at the next deadline computation.

## #2595 — Guard order converts to Attack during combat

**Why:** `FindCombatant()` set `ControlOrder = OrderType.Attack` when engaging, so a guarding pet left the Guard order for the whole fight: OPL tags wiped (pet `1080078` + master `501129`), no retargeting (`DoOrderAttack` locks its target), `TeleportPets` left the pet behind on recall/gate, and every engage→kill→resume cycle replayed the guard flourish.

**What changed:**
- **`FindGuardTarget()`** (was `FindCombatant`): a pure selector — prefers the aggressor **closest to the master** (RunUO guard parity, dynamic retargeting to protect the owner), keeps the current combatant unless a strictly closer one exists, and never mutates order state. `DoOrderGuard` engages through it while **staying in Guard** the whole fight.
- **Persistent-order semantics** (the ModernUO improvement over RunUO): an explicit `all attack` completes → `ResumePersistentOrder()` returns to Guard → the guard scan engages remaining threats in-order. The Attack-chaining fallback (`FightMode.Closest/Aggressor`) now applies only to non-guard persistent orders. Resuming Guard no longer replays the sound/"is now guarding you" message.
- **Peaceful guard stands down deterministically** (`Warmode`/`Combatant`/`FocusMob` cleared) and returns to the master at the RunUO sprint (see above); at the master's side it stays organically active.
- **`WalkMobileRange` honors the caller's run flag** (the internal hardcoded `dist > 5` gate silently overrode it). Run is animation-only server-side; the only callers passing anything but `false` — follow, guard, clone — gate on their own thresholds.

## Resulting behavior (Medium-bucket pet)

| Scenario | Broken | This PR |
|---|---|---|
| Guard trailing master (AOS) | ~1.05s/step, think-grid quantized | 0.1s/step sprint (RunUO parity), smooth move wakes |
| Guard during combat | order flips to Attack; tags lost; no retarget; left behind on recall | stays Guard; retargets to master's closest aggressor; teleports with master |
| `all attack` while guarding | resume spams guard flourish per kill; chains into Attack | resumes Guard silently; guard scan takes over |
| Come / friend-follow | 1.05s/step | activeMove 0.45s/step (≈ pre-#2591 feel) |
| Guard → Follow reaction | up to ~1.5s dead time | think within one wheel turn |
| Follow master (AOS sprint) | 0.1s/step | 0.1s/step (unchanged) |
| Wild creature chase | RunUO-parity move table | unchanged |

Also documents two contracts this work leaned on: the `ControlOrder` setter deliberately fires on every assignment (a reissued order is a command — retarget/break-off/re-anchor), and `OnThink`/`MonsterAbility` must be excess-call tolerant (`dev-docs/content-patterns.md` § OnThink: the excess-call contract).

## Testing

- Full suite passes (1570: 837 Server + 733 UOContent).
- `PetPacingTests`: order-issue think-clock parity, follow-master sprint via Obey, guard organically active at the master's side, combat-chase and herding boundaries, plus two deterministic timer-wheel tests (8ms-lockstep slicing) proving a fresh order and a mid-wait speed-up wake the AI promptly.
- `GuardOrderTests`: engage keeps the Guard order; retargets to the aggressor closest to the master; explicit attack resumes Guard without chaining into Attack; peaceful guard stands down. Setup self-validates LOS/terrain.
- `GuardFollowTests`: guard-following registers a move intent, steps toward the master, sprints at 0.1 under AOS (per-step flip must not undo it), and runs active pre-AOS.
- All behavioral tests were written first and failed for the documented reasons.
This commit is contained in:
Kamron Batman 2026-08-30 16:39:29 -07:00 • committed by GitHub
parent 38c74a968b
commit 4420872b22
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 712 additions and 73 deletions

View file

@ -283,6 +283,61 @@ ClearMoveSpeed(); // back to inheriting the think clock
All four are `[props`-tunable per instance (move values: set `0` to re-inherit); per-instance
move overrides serialize. Being badly hurt slows steps, never decisions (RunUO parity).
### OnThink: the excess-call contract
`OnThink()` is a scheduler pass, not an action. The AI timer calls it *at least* at the
think cadence (`CurrentSpeed`), but it can and does fire more often: a player command
wakes the AI immediately (`AITimer.Prod()`), a speed-up reschedules the pending wake, and
players run command macros that drive extra thinks deliberately (order spam is spam-safe
by design — reaction, never action). RunUO had the same property (its timer restarted
with a random delay on every speed change), so this has never been a fixed-rate callback.
**Every `OnThink` override must be excess-call tolerant.** An extra call must never grant
an extra action:
- Gate consequential work on its own deadline field, compared in subtraction form
(`Core.TickCount - _nextX >= 0` — see `tick-counts.md`), or make it idempotent.
- Never pace a consequential action with a bare per-call `Utility.RandomDouble()` roll —
its frequency then scales with think rate, which players can influence. Per-call rolls
are acceptable only for pure cosmetics (idle animations, flavor sounds).
- The engine already gates the expensive things: steps (the `NextMove` budget), weapon
swings, spell casts, detect-hidden, and the base `BaseCreature.OnThink` actions (heal,
rummage, aura) all carry their own clocks. Follow that pattern.
```csharp
private long _nextSpecial;
public override void OnThink()
{
base.OnThink();
if (Core.TickCount - _nextSpecial >= 0)
{
DoSpecial();
_nextSpecial = Core.TickCount + 5000; // the real rate limit lives here
}
}
```
### MonsterAbility: same contract
`MonsterAbility.CanTrigger` is sampled once per think for `Think`- and
`CombatAction`-triggered abilities, so abilities live under the same rule:
- **`MinTriggerCooldown`/`MaxTriggerCooldown` is the real rate limit** — the floor holds
no matter how often thinks fire. Always give a triggered ability a real cooldown.
- **`ChanceToTrigger` is a per-sample roll**: above the cooldown floor, the expected
trigger delay shrinks as think rate rises. Treat the chance as flavor jitter, never as
the rate limiter, and keep cooldowns long relative to the think interval so the jitter
stays negligible (fire breath — chance 0.5, cooldown 30–45s — varies under 1% between
natural and spammed think rates).
- A **zero-cooldown ability records no cooldown at all** and triggers on every sampled
think that passes its chance — only ever correct for passive alteration hooks, never
for `Think`/`CombatAction` triggers.
- An ability that breaks pet orders (fear-style effects) must own its duration explicitly
(a hold state, or a "refuses orders until" deadline checked in the order handlers) —
pets react to re-issued commands immediately, so think latency is not a hold.
---
## New Spell