From 38c74a968b5935a4a44c5e8265696add1009f520 Mon Sep 17 00:00:00 2001 From: Tald0r <47738492+Tald0r@users.noreply.github.com> Date: Thu, 27 Aug 2026 15:51:42 +0200 Subject: [PATCH 1/9] fix(regions): correct end Z coordinate assignment in InitRectangles (#2597) The `ez` variable was incorrectly assigned `rect.End.X` instead of `rect.End.Z`, causing incorrect rectangle processing in region initialization. --- Projects/UOContent/Regions/BaseRegion.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Projects/UOContent/Regions/BaseRegion.cs b/Projects/UOContent/Regions/BaseRegion.cs index b18c7fa18..776ec269d 100644 --- a/Projects/UOContent/Regions/BaseRegion.cs +++ b/Projects/UOContent/Regions/BaseRegion.cs @@ -113,7 +113,7 @@ public class BaseRegion : Region m_RectBuffer2.RemoveAt(k); var sz = rect.Start.Z; - var ez = rect.End.X; + var ez = rect.End.Z; if (l1 < l2) { From 4420872b22bd9301225335a472ab485941898820 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Sun, 30 Aug 2026 16:39:29 -0700 Subject: [PATCH 2/9] fix: pet obedience pacing, stale AI wake rescheduling, and Guard order persistence through combat (#2594) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Tests/Mobiles/AI/GuardFollowTests.cs | 96 ++++++++ .../Tests/Mobiles/AI/GuardOrderTests.cs | 137 +++++++++++ .../Tests/Mobiles/AI/PetPacingTests.cs | 222 ++++++++++++++++++ .../UOContent/Mobiles/AI/BaseAI/AIMovement.cs | 48 ++-- .../UOContent/Mobiles/AI/BaseAI/AITimer.cs | 70 +++++- .../UOContent/Mobiles/AI/BaseAI/BaseAI.cs | 2 +- .../Mobiles/AI/BaseAI/PetOrderHandlers.cs | 24 +- .../UOContent/Mobiles/AI/BaseAI/PetOrders.cs | 117 +++++---- Projects/UOContent/Mobiles/BaseCreature.cs | 7 +- .../modernuo-content-patterns.md | 7 + dev-docs/content-patterns.md | 55 +++++ 11 files changed, 712 insertions(+), 73 deletions(-) create mode 100644 Projects/UOContent.Tests/Tests/Mobiles/AI/GuardFollowTests.cs create mode 100644 Projects/UOContent.Tests/Tests/Mobiles/AI/GuardOrderTests.cs create mode 100644 Projects/UOContent.Tests/Tests/Mobiles/AI/PetPacingTests.cs diff --git a/Projects/UOContent.Tests/Tests/Mobiles/AI/GuardFollowTests.cs b/Projects/UOContent.Tests/Tests/Mobiles/AI/GuardFollowTests.cs new file mode 100644 index 000000000..db5225359 --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Mobiles/AI/GuardFollowTests.cs @@ -0,0 +1,96 @@ +using System.Collections.Generic; +using Server; +using Server.Mobiles; +using Xunit; + +namespace UOContent.Tests.Mobiles.AI; + +// Guard-following may pathfind, so this shares the pathfinding collection. +[Collection("Sequential Pathfinding Tests")] +public class GuardFollowTests +{ + [Fact] + public void GuardFollow_StepsTowardMaster_AndRegistersMoveIntent() + { + var map = Map.Maps[1]; + Assert.NotNull(map); + map.GetAverageZ(1500, 1600, out _, out var z, out _); + + var master = new PlayerMobile(World.NewMobile); + master.DefaultMobileInit(); + master.MoveToWorld(new Point3D(1494, 1600, (sbyte)z), map); + + var pet = new PetTestStub(); + pet.MoveToWorld(new Point3D(1500, 1600, (sbyte)z), map); // 6 tiles east, open terrain + pet.SetControlMaster(master); + + var ai = pet.AIObject; + ai.AITimer?.Stop(); // drive manually + pet.ControlOrder = OrderType.Guard; + ai.AITimer?.Stop(); // the order change may restart the timer + + var start = pet.Location; + ai.NextMove = 0; + ai.Obey(); + + var moved = pet.Location != start; + var hasIntent = ai.TryGetMoveWake(out _); + var currentSpeed = pet.CurrentSpeed; + var currentMoveSpeed = pet.CurrentMoveSpeed; + + pet.Delete(); + master.Delete(); + + Assert.True(moved, "a guarding pet beyond guard range must step toward its master"); + // Without a move intent, guard-following only steps on the think grid. + Assert.True(hasIntent, "guard-following must register a move intent"); + + // AOS return sprint on both clocks; the per-step speed flip must not undo it. + Assert.Equal(0.1, currentSpeed); + Assert.Equal(0.1, currentMoveSpeed); + } + + [Fact] + public void GuardReturn_PreAOS_RunsActive() + { + var previous = Core.Expansion; + + try + { + Core.Expansion = Expansion.UOR; + + var map = Map.Maps[1]; + Assert.NotNull(map); + map.GetAverageZ(1500, 1600, out _, out var z, out _); + + var master = new PlayerMobile(World.NewMobile); + master.DefaultMobileInit(); + master.MoveToWorld(new Point3D(1494, 1600, (sbyte)z), map); + + var pet = new PetTestStub(); + pet.MoveToWorld(new Point3D(1500, 1600, (sbyte)z), map); + pet.SetControlMaster(master); + + var ai = pet.AIObject; + ai.AITimer?.Stop(); + pet.ControlOrder = OrderType.Guard; + ai.AITimer?.Stop(); + pet.SetCurrentSpeedToPassive(); // a stale passive state must not persist + + ai.NextMove = 0; + ai.Obey(); + + var currentSpeed = pet.CurrentSpeed; + + pet.Delete(); + master.Delete(); + + // No sprint pre-AOS: the return runs active. + Assert.Equal(0.2, currentSpeed); + } + finally + { + Core.Expansion = previous; + } + } +} diff --git a/Projects/UOContent.Tests/Tests/Mobiles/AI/GuardOrderTests.cs b/Projects/UOContent.Tests/Tests/Mobiles/AI/GuardOrderTests.cs new file mode 100644 index 000000000..7d572786a --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Mobiles/AI/GuardOrderTests.cs @@ -0,0 +1,137 @@ +using System; +using System.Collections.Generic; +using Server; +using Server.Mobiles; +using Xunit; + +namespace UOContent.Tests.Mobiles.AI; + +// A guarding pet fights without leaving the Guard order, retargets toward the master's +// closest aggressor, and stands down when nothing threatens. Scene: the open +// (1495..1500, 1600) Trammel segment; targets are adjacent so no pathfinding runs. +[Collection("Sequential UOContent Tests")] +public class GuardOrderTests : IDisposable +{ + private readonly List _created = new(); + + private sealed class AggressorStub : Mobile + { + public AggressorStub() => Body = 0xC9; + } + + public void Dispose() + { + foreach (var m in _created) + { + m?.Delete(); + } + + _created.Clear(); + } + + private (PlayerMobile master, PetTestStub pet) SpawnGuardingPet(out Map map, out int z) + { + map = Map.Maps[1]; + Assert.NotNull(map); + map.GetAverageZ(1500, 1600, out _, out z, out _); + + var master = new PlayerMobile(World.NewMobile); + master.DefaultMobileInit(); + master.MoveToWorld(new Point3D(1500, 1600, (sbyte)z), map); + _created.Add(master); + + var pet = new PetTestStub(); + pet.MoveToWorld(new Point3D(1499, 1600, (sbyte)z), map); + pet.SetControlMaster(master); + _created.Add(pet); + + pet.AIObject.AITimer?.Stop(); // drive manually + pet.ControlOrder = OrderType.Guard; + pet.AIObject.AITimer?.Stop(); // the order change restarts the timer + + return (master, pet); + } + + private AggressorStub SpawnAggressor(PetTestStub pet, Point3D loc, Mobile attacking) + { + var aggr = new AggressorStub(); + aggr.MoveToWorld(loc, pet.Map); + _created.Add(aggr); + + // Setup guard: the scene must stay LOS-clear and the combatant must not be vetoed. + Assert.True(pet.InLOS(aggr), $"no LOS from pet to aggressor at {loc}"); + + if (attacking != null) + { + aggr.Combatant = attacking; + Assert.Same(attacking, aggr.Combatant); + } + + return aggr; + } + + [Fact] + public void GuardEngage_KeepsGuardOrder() + { + var (master, pet) = SpawnGuardingPet(out _, out var z); + var aggr = SpawnAggressor(pet, new Point3D(1498, 1600, (sbyte)z), master); + + pet.AIObject.Obey(); + + Assert.Same(aggr, pet.Combatant); + Assert.Equal(OrderType.Guard, pet.ControlOrder); + Assert.Equal(OrderType.Guard, pet.AIObject.PersistentOrder); + } + + [Fact] + public void Guard_RetargetsToAggressorClosestToMaster() + { + var (master, pet) = SpawnGuardingPet(out _, out var z); + var far = SpawnAggressor(pet, new Point3D(1495, 1600, (sbyte)z), master); + var near = SpawnAggressor(pet, new Point3D(1498, 1600, (sbyte)z), master); + + pet.Combatant = far; // already fighting the far aggressor + + pet.AIObject.Obey(); + + Assert.Same(near, pet.Combatant); // defends the master, not the current fight + Assert.Equal(OrderType.Guard, pet.ControlOrder); + } + + [Fact] + public void ExplicitAttack_ResumesGuard_WithoutChainingIntoAttack() + { + var (master, pet) = SpawnGuardingPet(out _, out var z); + + // Explicit kill order on a target that then becomes invalid. + var victim = SpawnAggressor(pet, new Point3D(1498, 1600, (sbyte)z), null); + pet.ControlTarget = victim; + pet.ControlOrder = OrderType.Attack; + victim.Hidden = true; + + // A second aggressor is still after the master; FightMode.Closest would chain it. + var aggr2 = SpawnAggressor(pet, new Point3D(1497, 1600, (sbyte)z), master); + + pet.AIObject.Obey(); // attack completes -> resume the persistent Guard + + Assert.Equal(OrderType.Guard, pet.ControlOrder); + + pet.AIObject.Obey(); // the guard scan engages the remaining aggressor in-order + + Assert.Same(aggr2, pet.Combatant); + Assert.Equal(OrderType.Guard, pet.ControlOrder); + } + + [Fact] + public void PeacefulGuard_StandsDown() + { + var (_, pet) = SpawnGuardingPet(out _, out _); + Assert.True(pet.Warmode); // the guard order opens in war stance + + pet.AIObject.Obey(); // nothing to guard against + + Assert.False(pet.Warmode); + Assert.Null(pet.Combatant); + Assert.Null(pet.FocusMob); + } +} diff --git a/Projects/UOContent.Tests/Tests/Mobiles/AI/PetPacingTests.cs b/Projects/UOContent.Tests/Tests/Mobiles/AI/PetPacingTests.cs new file mode 100644 index 000000000..217049b3d --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Mobiles/AI/PetPacingTests.cs @@ -0,0 +1,222 @@ +using System; +using System.Collections.Generic; +using Server; +using Server.Mobiles; +using Xunit; + +namespace UOContent.Tests.Mobiles.AI; + +// Pet order handlers own the speed clocks; combat chases and herding keep their own pacing. +[Collection("Sequential UOContent Tests")] +public class PetPacingTests : IDisposable +{ + private readonly List _created = new(); + + private (PlayerMobile master, PetTestStub pet) Spawn(Point3D masterLoc, Point3D petLoc) + { + var pair = PetTestSetup.SpawnControlledPet(masterLoc, petLoc); + _created.Add(pair.master); + _created.Add(pair.pet); + return pair; + } + + public void Dispose() + { + foreach (var m in _created) + { + m?.Delete(); + } + + _created.Clear(); + } + + // Movement orders run active, resting orders run passive; the move clock follows. + [Fact] + public void OrderIssue_SetsThinkClock() + { + var (master, pet) = Spawn(new Point3D(1000, 1000, 0), new Point3D(1001, 1000, 0)); + pet.SetMoveSpeed(0.3, 0.9); + pet.SetCurrentSpeedToPassive(); + + pet.ControlOrder = OrderType.Come; + Assert.Equal(0.2, pet.CurrentSpeed); + Assert.Equal(0.3, pet.CurrentMoveSpeed); // verbatim active -> activeMove + + pet.ControlOrder = OrderType.Stay; + Assert.Equal(0.4, pet.CurrentSpeed); + Assert.Equal(0.9, pet.CurrentMoveSpeed); + + pet.ControlTarget = master; + pet.ControlOrder = OrderType.Follow; + Assert.Equal(0.2, pet.CurrentSpeed); + + pet.ControlOrder = OrderType.Guard; + Assert.Equal(0.2, pet.CurrentSpeed); + } + + // AOS: following the master sprints at a bespoke 0.1 on both clocks. + [Fact] + public void FollowMaster_ObeySprints() + { + var (master, pet) = Spawn(new Point3D(1000, 1000, 0), new Point3D(1001, 1000, 0)); + pet.SetMoveSpeed(0.3, 0.9); + pet.AIObject.AITimer?.Stop(); + + pet.ControlTarget = master; + pet.ControlOrder = OrderType.Follow; // fixture era is EJ + pet.AIObject.Obey(); + + Assert.Equal(0.1, pet.CurrentSpeed); + Assert.Equal(0.1, pet.CurrentMoveSpeed); + } + + // At the master's side a guarding pet stays active: no stale-warmode passive, no sprint. + [Fact] + public void GuardAtMastersSide_IsActive() + { + var (_, pet) = Spawn(new Point3D(1000, 1000, 0), new Point3D(1001, 1000, 0)); + pet.SetMoveSpeed(0.3, 0.9); + pet.AIObject.AITimer?.Stop(); + pet.SetCurrentSpeedToPassive(); + + pet.ControlOrder = OrderType.Guard; + pet.AIObject.Obey(); // nothing to guard against, master adjacent + + Assert.Equal(0.2, pet.CurrentSpeed); + Assert.Equal(0.3, pet.CurrentMoveSpeed); + } + + // A pet chasing a combatant keeps the move table. + [Fact] + public void CombatChasingPet_KeepsMoveTable() + { + var (_, pet) = Spawn(new Point3D(1000, 1000, 0), new Point3D(1001, 1000, 0)); + var target = new PetTestStub(); + target.MoveToWorld(new Point3D(1003, 1000, 0), Map.Felucca); + _created.Add(target); + + pet.SetMoveSpeed(0.3, 0.9); + pet.ControlOrder = OrderType.Guard; + pet.Combatant = target; + pet.SetCurrentSpeedToActive(); + + Assert.Equal(0.3, pet.CurrentMoveSpeed); + } + + // Herding overrides order pacing. + [Fact] + public void HerdedObeyingPet_KeepsHerdingPace() + { + var (_, pet) = Spawn(new Point3D(1000, 1000, 0), new Point3D(1001, 1000, 0)); + pet.SetMoveSpeed(0.45, 0.9); + pet.SetCurrentSpeedToPassive(); + + pet.TargetLocation = new Point2D(1010, 1010); + + Assert.Equal(0.3, pet.CurrentMoveSpeed); // fixed herding pace + } + + private sealed class ThinkProbe : PetTestStub + { + public int Thinks; + + public override void OnThink() + { + Thinks++; + base.OnThink(); + } + } + + private (PlayerMobile master, ThinkProbe pet) SpawnProbe() + { + var master = new PlayerMobile(World.NewMobile); + master.DefaultMobileInit(); + master.MoveToWorld(new Point3D(1000, 1000, 0), Map.Felucca); + _created.Add(master); + + var pet = new ThinkProbe(); + pet.MoveToWorld(new Point3D(1001, 1000, 0), Map.Felucca); + pet.SetControlMaster(master); + _created.Add(pet); + + return (master, pet); + } + + // Advances time in 8ms lockstep so the wheel and Core.TickCount stay in sync. + private static void RunFor(long ms) + { + var deadline = Core._tickCount + ms; + + while (Core._tickCount < deadline) + { + Core._tickCount += 8; + Timer.Slice(Core._tickCount); + } + } + + private static bool RunUntil(Func condition, long maxMs) + { + var deadline = Core._tickCount + maxMs; + + while (Core._tickCount < deadline) + { + if (condition()) + { + return true; + } + + Core._tickCount += 8; + Timer.Slice(Core._tickCount); + } + + return condition(); + } + + // Runs past the spawn stagger; returns right after a think with the next 0.4s away. + private ThinkProbe SettledProbe(out PlayerMobile master) + { + Core._tickCount = 0; + Timer.Init(0); + + var (m, pet) = SpawnProbe(); + master = m; + pet.ForceIdle = true; // no wandering; pure cadence + pet.ControlOrder = OrderType.Stay; + + var settled = RunUntil(() => pet.Thinks >= 2, 8000); + Assert.True(settled, "the AI must reach a steady think cadence"); + + return pet; + } + + [Fact] + public void OrderChange_WakesStaleThinkTimer() + { + var pet = SettledProbe(out var master); + var thinksBefore = pet.Thinks; + + RunFor(200); // mid-wait, next think ~200ms out + Assert.Equal(thinksBefore, pet.Thinks); + + pet.ControlTarget = master; + pet.ControlOrder = OrderType.Follow; + + RunFor(80); + Assert.True(pet.Thinks > thinksBefore, "a fresh order must wake the AI promptly"); + } + + [Fact] + public void SpeedUp_ReschedulesPendingWake() + { + var pet = SettledProbe(out _); + var thinksBefore = pet.Thinks; + + RunFor(200); // mid-wait, next think ~200ms out + Assert.Equal(thinksBefore, pet.Thinks); + + pet.CurrentSpeed = 0.1; + + RunFor(120); + Assert.True(pet.Thinks > thinksBefore, "a speed-up must reschedule the pending wake"); + } +} diff --git a/Projects/UOContent/Mobiles/AI/BaseAI/AIMovement.cs b/Projects/UOContent/Mobiles/AI/BaseAI/AIMovement.cs index ccd1c72b6..5095a4cb4 100644 --- a/Projects/UOContent/Mobiles/AI/BaseAI/AIMovement.cs +++ b/Projects/UOContent/Mobiles/AI/BaseAI/AIMovement.cs @@ -118,18 +118,17 @@ public abstract partial class BaseAI if (TryMove(d)) { - // Writes the think clock only; hurt slowdown applies in ConsumeMoveBudget. - if (Core.AOS && IsFollowingMaster()) + // Obeying pets are paced by their order handlers. + if (!IsObeyingMoveOrder()) { - Mobile.CurrentSpeed = 0.1; - } - else if (Mobile.Warmode || Mobile.Combatant != null) - { - Mobile.SetCurrentSpeedToActive(); - } - else - { - Mobile.SetCurrentSpeedToPassive(); + if (Mobile.Warmode || Mobile.Combatant != null) + { + Mobile.SetCurrentSpeedToActive(); + } + else + { + Mobile.SetCurrentSpeedToPassive(); + } } ConsumeMoveBudget(); @@ -541,8 +540,7 @@ public abstract partial class BaseAI { nextMove = NextMove; - return (_moveIntentTarget != null || _moveIntentPoint != null) && - Core.TickCount - _moveIntentExpire < 0; + return (_moveIntentTarget != null || _moveIntentPoint != null) && Core.TickCount - _moveIntentExpire < 0; } /// @@ -574,9 +572,9 @@ public abstract partial class BaseAI } var distance = (int)Mobile.GetDistanceToSqrt(m); - var distanceThreshold = Core.AOS && IsFollowingMaster() ? 1 : 5; - - var shouldRun = run && distance > distanceThreshold; + //TODO Derive the Running bit from CurrentMoveSpeed in DoMoveImpl and drop the run parameter + var distanceThreshold = Core.AOS && IsFollowingMaster() ? 1 : 3; + var shouldRun = distance > distanceThreshold; if (Mobile.InRange(m, range)) { @@ -599,6 +597,13 @@ public abstract partial class BaseAI Mobile.ControlTarget == Mobile.ControlMaster && Mobile.Combatant == null; + // A pet executing a movement order outside combat; its order handler owns its speed. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public bool IsObeyingMoveOrder() => + Mobile.Controlled && + Mobile.Combatant == null && + Mobile.ControlOrder is OrderType.Come or OrderType.Follow or OrderType.Guard; + private bool MoveToWithCollisionAvoidance(Mobile target, bool run, int range) { var distance = (int)Mobile.GetDistanceToSqrt(target); @@ -649,14 +654,12 @@ public abstract partial class BaseAI { var iCurrDist = (int)Mobile.GetDistanceToSqrt(m); - var shouldRun = run && iCurrDist > 5; - if (iCurrDist >= iWantDistMin && iCurrDist <= iWantDistMax) { return true; } - if (!MoveTowardsOrAwayFrom(m, shouldRun, iCurrDist, iWantDistMax)) + if (!MoveTowardsOrAwayFrom(m, run, iCurrDist, iWantDistMax)) { return false; } @@ -667,18 +670,17 @@ public abstract partial class BaseAI return dist >= iWantDistMin && dist <= iWantDistMax; } + // run only sets the client animation; callers gate it on their own distance thresholds. private bool MoveTowardsOrAwayFrom(Mobile m, bool run, int iCurrDist, int iWantDistMax) { - var shouldRun = run && iCurrDist > 5; - if (iCurrDist > iWantDistMax) { // Too far: approach via the centralized progress-based primitive. - return ApproachTarget(m, shouldRun, iWantDistMax); + return ApproachTarget(m, run, iWantDistMax); } // Too close: back away. Retreat keeps the simple greedy behavior (out of scope). - if (DoMove(m.GetDirectionTo(Mobile, shouldRun), true)) + if (DoMove(m.GetDirectionTo(Mobile, run), true)) { Path = null; return true; diff --git a/Projects/UOContent/Mobiles/AI/BaseAI/AITimer.cs b/Projects/UOContent/Mobiles/AI/BaseAI/AITimer.cs index fe24e5f6d..d5a84f94d 100644 --- a/Projects/UOContent/Mobiles/AI/BaseAI/AITimer.cs +++ b/Projects/UOContent/Mobiles/AI/BaseAI/AITimer.cs @@ -26,6 +26,8 @@ public sealed class AITimer : Timer { private readonly BaseAI _owner; private long _nextThink; + private long _nextWake; // when the pending wheel entry fires + private bool _inTick; private int _detectHiddenMinDelay; private int _detectHiddenMaxDelay; @@ -40,8 +42,30 @@ public sealed class AITimer : Timer public void Activate() { _nextThink = Core.TickCount; - Interval = TimeSpan.FromSeconds(_owner.Mobile.CurrentSpeed); + + if (Running) + { + return; + } + + Start(); // keeps the stagger Delay + _nextWake = Core.TickCount + (long)Delay.TotalMilliseconds; + } + + // Think now. A think grants no action: steps, swings, casts, and abilities keep their own gates. + public void Prod() + { + _nextThink = Core.TickCount; + + if (Running) + { + Reschedule(); + return; + } + + Delay = TimeSpan.Zero; Start(); + _nextWake = Core.TickCount + (long)Delay.TotalMilliseconds; } // A speed-up must not wait out a stale, longer think deadline. @@ -52,12 +76,53 @@ public sealed class AITimer : Timer if (candidate - _nextThink < 0) { _nextThink = candidate; + Reschedule(); + } + } + + // Moves the pending wake earlier. Interval is only read after the next fire, + // so this needs Stop, Delay = remaining, Start. + private void Reschedule() + { + if (_inTick || !Running) + { + return; // ScheduleNext handles it at tick end } - Interval = TimeSpan.FromSeconds(_owner.Mobile.CurrentSpeed); + var now = Core.TickCount; + var deadline = _nextThink; + + if (_owner.TryGetMoveWake(out var nextMove) && nextMove - now > 0 && nextMove - deadline < 0) + { + deadline = nextMove; + } + + if (deadline - _nextWake >= 0) + { + return; // pending wake is already early enough + } + + Stop(); + Delay = TimeSpan.FromMilliseconds(Math.Max(0, deadline - now)); + Start(); + _nextWake = now + (long)Delay.TotalMilliseconds; } protected override void OnTick() + { + _inTick = true; + + try + { + OnTickCore(); + } + finally + { + _inTick = false; + } + } + + private void OnTickCore() { if (ShouldStop()) { @@ -111,6 +176,7 @@ public sealed class AITimer : Timer // The wheel rounds up to its 8ms resolution; a non-positive delay becomes one turn. Interval = TimeSpan.FromMilliseconds(delay); + _nextWake = now + (long)Interval.TotalMilliseconds; } private bool ShouldStop() diff --git a/Projects/UOContent/Mobiles/AI/BaseAI/BaseAI.cs b/Projects/UOContent/Mobiles/AI/BaseAI/BaseAI.cs index cf64cae17..f86ac2bfe 100644 --- a/Projects/UOContent/Mobiles/AI/BaseAI/BaseAI.cs +++ b/Projects/UOContent/Mobiles/AI/BaseAI/BaseAI.cs @@ -64,7 +64,7 @@ public abstract partial class BaseAI if (!m.PlayerRangeSensitive || !World.Loading && m.Map != null && m.Map != Map.Internal && m.Map.GetSector(m.Location).Active) { - AITimer.Start(); + AITimer.Activate(); } if (Action != ActionType.Wander) diff --git a/Projects/UOContent/Mobiles/AI/BaseAI/PetOrderHandlers.cs b/Projects/UOContent/Mobiles/AI/BaseAI/PetOrderHandlers.cs index ecb456d2e..862c78649 100644 --- a/Projects/UOContent/Mobiles/AI/BaseAI/PetOrderHandlers.cs +++ b/Projects/UOContent/Mobiles/AI/BaseAI/PetOrderHandlers.cs @@ -26,7 +26,7 @@ public abstract partial class BaseAI return; } - Activate(); + AITimer.Prod(); switch (Mobile.ControlOrder) { @@ -36,6 +36,10 @@ public abstract partial class BaseAI break; } case OrderType.Come: + { + Mobile.SetCurrentSpeedToActive(); + break; + } case OrderType.Drop: case OrderType.Friend: case OrderType.Unfriend: @@ -135,6 +139,7 @@ public abstract partial class BaseAI Mobile.FocusMob = null; Mobile.Warmode = false; Mobile.Combatant = null; + Mobile.SetCurrentSpeedToPassive(); } private void HandleTransferOrder() @@ -148,6 +153,7 @@ public abstract partial class BaseAI Mobile.FocusMob = null; Mobile.Warmode = false; Mobile.Combatant = null; + Mobile.SetCurrentSpeedToPassive(); Mobile.PlaySound(Mobile.GetIdleSound()); _commandIssuer = null; } @@ -162,9 +168,16 @@ public abstract partial class BaseAI _commandIssuer?.RevealingAction(); Mobile.FocusMob = null; Mobile.Warmode = true; - Mobile.PlaySound(Mobile.GetAttackSound()); - Mobile.ControlMaster?.SendLocalizedMessage(1049671, Mobile.Name); - // ~1_NAME~ is now guarding you. + Mobile.SetCurrentSpeedToActive(); + + // Resuming the persistent order must not replay the flourish. + if (!_resolvingOrder) + { + Mobile.PlaySound(Mobile.GetAttackSound()); + Mobile.ControlMaster?.SendLocalizedMessage(1049671, Mobile.Name); + // ~1_NAME~ is now guarding you. + } + _commandIssuer = null; } @@ -191,6 +204,7 @@ public abstract partial class BaseAI } Mobile.Warmode = true; + Mobile.SetCurrentSpeedToActive(); Mobile.PlaySound(Mobile.GetAttackSound()); _commandIssuer = null; } @@ -206,6 +220,7 @@ public abstract partial class BaseAI Mobile.FocusMob = null; Mobile.Warmode = false; Mobile.Combatant = null; + Mobile.SetCurrentSpeedToActive(); Mobile.PlaySound(Mobile.GetIdleSound()); _commandIssuer = null; } @@ -221,6 +236,7 @@ public abstract partial class BaseAI Mobile.FocusMob = null; Mobile.Warmode = false; Mobile.Combatant = null; + Mobile.SetCurrentSpeedToPassive(); Mobile.PlaySound(Mobile.GetIdleSound()); _commandIssuer = null; // Home (the stay anchor) is owned by SetPersistentOrder, not this handler. diff --git a/Projects/UOContent/Mobiles/AI/BaseAI/PetOrders.cs b/Projects/UOContent/Mobiles/AI/BaseAI/PetOrders.cs index 1b7139a96..54dd5774f 100644 --- a/Projects/UOContent/Mobiles/AI/BaseAI/PetOrders.cs +++ b/Projects/UOContent/Mobiles/AI/BaseAI/PetOrders.cs @@ -128,6 +128,12 @@ public abstract partial class BaseAI this.DebugSayFormatted($"I am ordered to follow {Mobile.ControlTarget?.Name}."); + // AOS: sprint after the master (bespoke 0.1 paces both clocks). + if (Core.AOS && Mobile.ControlTarget == Mobile.ControlMaster && Mobile.Combatant == null) + { + Mobile.CurrentSpeed = 0.1; + } + if (currentDistance > 1) { WalkMobileRange(Mobile.ControlTarget, 1, currentDistance > 2, 1, 2); @@ -291,14 +297,13 @@ public abstract partial class BaseAI return true; } - FindCombatant(); + var combatant = FindGuardTarget(); - if (IsValidCombatant(Mobile.Combatant)) + if (combatant != null) { - var combatant = Mobile.Combatant; - this.DebugSayFormatted($"Attacking target: {combatant.Name}"); + // Engage without leaving the Guard order so tags, recall handling, and retargeting persist. Mobile.Combatant = combatant; Mobile.FocusMob = combatant; Action = ActionType.Combat; @@ -309,16 +314,30 @@ public abstract partial class BaseAI { this.DebugSayFormatted($"Guarding my master, {controlMaster.Name}."); - var guardLocation = controlMaster.Location; + // Stand down; a stale Warmode would skew the return pace. + Mobile.FocusMob = null; + Mobile.Warmode = false; + Mobile.Combatant = null; - var distance = (int)Mobile.GetDistanceToSqrt(guardLocation); + var distance = (int)Mobile.GetDistanceToSqrt(controlMaster); if (distance > 3) { - DoMove(Mobile.GetDirectionTo(guardLocation)); + // AOS: sprint back (bespoke 0.1 paces both clocks); earlier eras run active. + if (Core.AOS) + { + Mobile.CurrentSpeed = 0.1; + } + else + { + Mobile.SetCurrentSpeedToActive(); + } + + WalkMobileRange(controlMaster, 1, true, 1, 3); } else { + Mobile.SetCurrentSpeedToActive(); // alert at the master's side WalkRandom(3, 1, 1); } } @@ -359,67 +378,83 @@ public abstract partial class BaseAI Mobile.ControlTarget = Mobile.ControlMaster; ResumePersistentOrder(); - if (Mobile.FightMode is FightMode.Closest or FightMode.Aggressor) + // A resumed Guard engages through its own scan; other fallbacks chain an explicit Attack. + if (Mobile.ControlOrder == OrderType.Guard || + Mobile.FightMode is not (FightMode.Closest or FightMode.Aggressor)) { - FindCombatant(); + return; + } + + var next = FindGuardTarget(); + + if (next != null) + { + Mobile.ControlTarget = next; + Mobile.ControlOrder = OrderType.Attack; + Mobile.Combatant = next; + + this.DebugSayFormatted($"{next.Name} is still hostile! Engaging..."); + + Think(); } } - private void FindCombatant() + /// + /// Selects the aggressor closest to the master. The current combatant is kept + /// unless a strictly closer one exists. Never mutates order state. + /// + private Mobile FindGuardTarget() { var controlMaster = Mobile.ControlMaster; + var anchor = controlMaster ?? Mobile; + + var current = Mobile.Combatant; + var best = current != controlMaster && IsValidCombatant(current) ? current : null; + var bestDist = best?.GetDistanceToSqrt(anchor) ?? double.MaxValue; foreach (var aggr in Mobile.GetMobilesInRange(Mobile.RangePerception)) { - if (!Mobile.CanSee(aggr) || aggr.IsDeadBondedPet || !aggr.Alive) + if (aggr == best || aggr == Mobile || aggr == controlMaster || + aggr.IsDeadBondedPet || !aggr.Alive || + aggr.Combatant != Mobile && (controlMaster == null || aggr.Combatant != controlMaster)) { continue; } - var isAttackingPet = aggr.Combatant == Mobile; - var isAttackingMaster = controlMaster != null && aggr.Combatant == controlMaster; + var dist = aggr.GetDistanceToSqrt(anchor); - if (isAttackingPet || isAttackingMaster) + if (dist < bestDist && Mobile.CanSee(aggr) && Mobile.InLOS(aggr)) { - if (Mobile.InLOS(aggr)) - { - Mobile.ControlTarget = aggr; - Mobile.ControlOrder = OrderType.Attack; - Mobile.Combatant = aggr; - - var target = isAttackingMaster ? "master" : "me"; - this.DebugSayFormatted($"{aggr.Name} is attacking my {target}! Engaging..."); - - Think(); - return; - } + best = aggr; + bestDist = dist; } } - if (controlMaster?.Aggressors != null) - { - for (var i = 0; i < controlMaster.Aggressors.Count; i++) - { - var aggressor = controlMaster.Aggressors[i].Attacker; + var aggressors = controlMaster?.Aggressors; - if (aggressor?.Deleted != false || !aggressor.Alive || aggressor.IsDeadBondedPet) + if (aggressors != null) + { + for (var i = 0; i < aggressors.Count; i++) + { + var aggressor = aggressors[i].Attacker; + + if (aggressor == best || aggressor?.Deleted != false || !aggressor.Alive || + aggressor.IsDeadBondedPet || !Mobile.InRange(aggressor, Mobile.RangePerception)) { continue; } - if (Mobile.InRange(aggressor, Mobile.RangePerception) && Mobile.CanSee(aggressor) && Mobile.InLOS(aggressor)) + var dist = aggressor.GetDistanceToSqrt(anchor); + + if (dist < bestDist && Mobile.CanSee(aggressor) && Mobile.InLOS(aggressor)) { - Mobile.ControlTarget = aggressor; - Mobile.ControlOrder = OrderType.Attack; - Mobile.Combatant = aggressor; - - this.DebugSayFormatted($"{aggressor.Name} recently attacked my master! Retaliating..."); - - Think(); - return; + best = aggressor; + bestDist = dist; } } } + + return best; } public virtual bool DoOrderRelease() diff --git a/Projects/UOContent/Mobiles/BaseCreature.cs b/Projects/UOContent/Mobiles/BaseCreature.cs index f64a289f4..15d9b4c03 100644 --- a/Projects/UOContent/Mobiles/BaseCreature.cs +++ b/Projects/UOContent/Mobiles/BaseCreature.cs @@ -750,8 +750,9 @@ namespace Server.Mobiles /// /// Resolved seconds per step: a verbatim active/passive - /// maps to the matching movement value; a bespoke pace stays fused to both clocks. - /// A herded creature is always driven at . + /// maps to the matching movement value; a bespoke pace (e.g. the pet-order 0.1 sprint) + /// stays fused to both clocks. A herded creature is always driven at + /// . /// [CommandProperty(AccessLevel.GameMaster)] public double CurrentMoveSpeed @@ -845,6 +846,8 @@ namespace Server.Mobiles [CommandProperty(AccessLevel.GameMaster)] public Point3D ControlDest { get; set; } + // Fires on every assignment, not only changes: a reissued order is a command + // (retarget, break off combat, re-anchor Home). Handlers receive the previous order. [CommandProperty(AccessLevel.GameMaster)] public OrderType ControlOrder { diff --git a/dev-docs/claude-skills/modernuo-content-patterns.md b/dev-docs/claude-skills/modernuo-content-patterns.md index ce7f18acf..cdb3f9e49 100644 --- a/dev-docs/claude-skills/modernuo-content-patterns.md +++ b/dev-docs/claude-skills/modernuo-content-patterns.md @@ -29,6 +29,13 @@ description: > overridden). Prefer `npc-speeds.json` buckets (`SpeedClass`); `SetSpeed()` sets think AND clears move overrides, `SetMoveSpeed()` sets move only -- see `dev-docs/content-patterns.md` § Creature Speeds +8. **`OnThink` overrides must be excess-call tolerant** -- it fires more often than the + think cadence (player commands prod it; speed-ups reschedule it). Gate consequential + work on a tick-count deadline (subtraction form) or make it idempotent; bare per-call + random rolls are cosmetics-only. `MonsterAbility` is under the same contract: the + trigger cooldown is the rate limit, `ChanceToTrigger` is per-sample jitter, and a + zero-cooldown `Think`/`CombatAction` ability triggers every sampled think -- see + `dev-docs/content-patterns.md` § OnThink: the excess-call contract ## New Item Template diff --git a/dev-docs/content-patterns.md b/dev-docs/content-patterns.md index ec56e1bb3..80d925580 100644 --- a/dev-docs/content-patterns.md +++ b/dev-docs/content-patterns.md @@ -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 From e07416902afebad3affbbdc1fbbf8c4a097df4d9 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Sun, 30 Aug 2026 16:48:52 -0700 Subject: [PATCH 3/9] feat: derive the Running bit from the step pace and fix step-pacing bursts (#2599) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on #2594. Fixes jerky creature movement (lich / Fast-bucket melee chases) by choosing the client animation flag from the actual step pace instead of a caller-supplied `run` argument, and fixes three step-pacing defects in the move budget found while verifying it with paired server/client traces. ### Why The `Direction.Running` bit does nothing for creatures server-side (`Mobile.OnMove` reads it only for the player throttle and stealth reveal). Its whole effect is on the client, which animates each step over a fixed time selected by that bit: walk 400 ms / run 200 ms on foot, 200 / 100 ms mounted. ClassicUO queues up to 5 steps and *drops* the sixth, so a creature stepping every 300 ms while flagged as walking backs the queue up until it snaps forward — the observed jerk. The `run` argument never carried the one fact that matters (the step interval). RunUO passed `true` in combat / `false` for pets and gated it on `dist > 5`; #2271 flipped every combat site to `false`; pets passed `currentDistance > 2`. None of that is a coherent signal. ### What **Pace-derived run flag** - `BaseAI.ShouldRun()`: run iff the effective step delay (move clock + badly-hurt inflation) is shorter than `Movement.WalkFootDelay` / `WalkMountDelay` (mounted or flying) — with a continuity rule: an *isolated* step (taken after standing at least a walk interval) goes out as a walk, because the client renders each step alone and a lone run-flagged step is a 200 ms dart. Only a continuing cadence flags run; a true sprinter (pace under the run interpolation) always runs, since a walk-rendered first step would flood the client's 5-step queue. This reproduces RunUO's close-in feel (its `dist > 5` gate) from first principles. - `DoMoveImpl` stamps the bit; it is the single place the flag is set. - `run` removed from `MoveTo`, `WalkMobileRange`, `ApproachTarget`, `MoveToPoint`, `MoveToWithGroup`, `MoveToWithCollisionAvoidance`, the move intent, and `PathFollower.Follow`. All 35 call sites updated. **API change** for custom scripts — documented in the RunUO migration docs (`09-items-mobiles-creatures.md`, `11-api-reference.md`) and `content-patterns.md` § Creature Speeds. **Move-budget pacing fixes** (each confirmed by UTC-aligned server/client step traces) - A stall no longer banks catch-up steps: the budget's snap-to-now released up to three steps in ~300 ms when a creature resumed chasing after standing beside its target — rendered as a teleport. - Debt accrual removed entirely: a step landing sub-period late (think-grid vs budget misalignment during reactive mirroring) kept the remainder and fired a follow-up ~100 ms later — a dart pair. `ConsumeMoveBudget` now paces every step from when it was actually taken; in continuous pursuit the move-wake lands within wheel resolution of the deadline, so the cost is single-digit-ms drift. - Net effect: a creature can never step faster than its pace, verified across a full chase session (zero sub-pace steps; metronomic 350 ms cadence for a 0.3 s lich). - Test fixture now runs `Movement.Configure()` (the walk delays were 0 in tests). ### Accepted trade-off Animal (LOW group) bodies without a run animation slide on their stand frames when flagged as running. Most are slow enough to stay flagged as walking; the client-side fallback is in ClassicUO/ClassicUO#1930. ### Tests `RunFlagTests`: foot thresholds (0.3 / 0.125 run; 0.4 / 0.45 / 1.05 walk), flying uses the mount threshold, badly-hurt inflation flips a 0.35 s creature back to walk, a real `DoMove` stamps the bit, isolated steps drop to walk (sprinters keep running), a stall restarts the cadence with no banked steps, and a late step earns no quicker follow-up. Full suite: 837 Server + 747 UOContent green. --- .../Fixtures/TestServerInitializer.cs | 1 + .../Tests/Mobiles/AI/ApproachTargetTests.cs | 10 +- .../Tests/Mobiles/AI/RunFlagTests.cs | 162 ++++++++++++++++++ .../Factions/Mobiles/Guards/GuardAI.cs | 4 +- .../UOContent/Engines/Pathing/PathFollower.cs | 6 +- Projects/UOContent/Mobiles/AI/AnimalAI.cs | 2 +- Projects/UOContent/Mobiles/AI/ArcherAI.cs | 2 +- .../Mobiles/AI/BaseAI/AIGroupMovement.cs | 6 +- .../UOContent/Mobiles/AI/BaseAI/AIMovement.cs | 94 +++++----- .../UOContent/Mobiles/AI/BaseAI/BaseAI.cs | 8 +- .../UOContent/Mobiles/AI/BaseAI/PetOrders.cs | 6 +- Projects/UOContent/Mobiles/AI/BerserkAI.cs | 2 +- Projects/UOContent/Mobiles/AI/HealerAI.cs | 2 +- Projects/UOContent/Mobiles/AI/MageAI.cs | 10 +- Projects/UOContent/Mobiles/AI/MeleeAI.cs | 2 +- Projects/UOContent/Mobiles/AI/PredatorAI.cs | 4 +- Projects/UOContent/Mobiles/AI/ThiefAI.cs | 2 +- Projects/UOContent/Mobiles/BaseCreature.cs | 2 +- .../Mobiles/Familiars/BaseFamiliar.cs | 2 +- .../Monsters/LBR/Meers/EnragedCreatures.cs | 2 +- .../UOContent/Spells/Ninjitsu/MirrorImage.cs | 5 +- .../migrate-items-mobiles.md | 1 + .../modernuo-content-patterns.md | 5 +- dev-docs/content-patterns.md | 12 ++ .../09-items-mobiles-creatures.md | 23 +++ .../runuo-migration-docs/11-api-reference.md | 3 + 26 files changed, 294 insertions(+), 84 deletions(-) create mode 100644 Projects/UOContent.Tests/Tests/Mobiles/AI/RunFlagTests.cs diff --git a/Projects/UOContent.Tests/Fixtures/TestServerInitializer.cs b/Projects/UOContent.Tests/Fixtures/TestServerInitializer.cs index a38af9c3d..5e4230b7c 100644 --- a/Projects/UOContent.Tests/Fixtures/TestServerInitializer.cs +++ b/Projects/UOContent.Tests/Fixtures/TestServerInitializer.cs @@ -103,6 +103,7 @@ internal static class TestServerInitializer // Registers the Accounts entity persistence; without it no test can construct an Account. Server.Accounting.Accounts.Configure(); RaceDefinitions.Configure(); + Server.Movement.Movement.Configure(); MovementImpl.Configure(); PathFollower.Configure(); World.Load(); diff --git a/Projects/UOContent.Tests/Tests/Mobiles/AI/ApproachTargetTests.cs b/Projects/UOContent.Tests/Tests/Mobiles/AI/ApproachTargetTests.cs index 923f4aff9..a952252a6 100644 --- a/Projects/UOContent.Tests/Tests/Mobiles/AI/ApproachTargetTests.cs +++ b/Projects/UOContent.Tests/Tests/Mobiles/AI/ApproachTargetTests.cs @@ -40,7 +40,7 @@ public class ApproachTargetTests for (var i = 0; i < maxTicks; i++) { ai.NextMove = 0; - ai.WalkMobileRange(target, 1, false, 1, 2); + ai.WalkMobileRange(target, 1, 1, 2); if (bc.InRange(target, arriveDist)) { return true; @@ -123,7 +123,7 @@ public class ApproachTargetTests for (var i = 0; i < 200; i++) { ai.NextMove = 0; - ai.MoveTo(target, false, 1); + ai.MoveTo(target, 1); if (bc.InRange(target, 1)) { arrived = true; @@ -154,7 +154,7 @@ public class ApproachTargetTests for (var i = 0; i < 60; i++) { ai.NextMove = 0; - ai.MoveTo(target, true, 1); + ai.MoveTo(target, 1); // Target walks west every other tick for its first several steps, then stops, // so a same-speed chaser eventually closes the gap. @@ -214,7 +214,7 @@ public class ApproachTargetTests for (var i = 0; i < 120; i++) { ai.NextMove = 0; - ai.MoveTo(target, false, 1); + ai.MoveTo(target, 1); } // After giving up, the creature must idle (not oscillate) while the goal is still. @@ -223,7 +223,7 @@ public class ApproachTargetTests for (var i = 0; i < 20; i++) { ai.NextMove = 0; - ai.MoveTo(target, false, 1); + ai.MoveTo(target, 1); if (bc.Location != idleStart) { stayedIdle = false; diff --git a/Projects/UOContent.Tests/Tests/Mobiles/AI/RunFlagTests.cs b/Projects/UOContent.Tests/Tests/Mobiles/AI/RunFlagTests.cs new file mode 100644 index 000000000..a421aaba2 --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Mobiles/AI/RunFlagTests.cs @@ -0,0 +1,162 @@ +using System.Collections.Generic; +using Server; +using Server.Mobiles; +using Xunit; + +namespace UOContent.Tests.Mobiles.AI; + +// The Running bit is derived from the step pace: a step shorter than the client's walk +// interpolation (400ms on foot, 200ms mounted/flying) is flagged as a run. +[Collection("Sequential Pathfinding Tests")] +public class RunFlagTests : System.IDisposable +{ + private readonly List _created = new(); + + private PetTestStub Spawn(double activeMove) + { + var pet = new PetTestStub(); + pet.MoveToWorld(new Point3D(1000, 1000, 0), Map.Felucca); + pet.AIObject.AITimer?.Stop(); + pet.SetMoveSpeed(activeMove, activeMove * 3); + pet.SetCurrentSpeedToActive(); + pet.LastMoveTime = Core.TickCount; // mid-cadence unless a test says otherwise + _created.Add(pet); + return pet; + } + + public void Dispose() + { + foreach (var m in _created) + { + m?.Delete(); + } + + _created.Clear(); + } + + [Theory] + [InlineData(0.3, true)] + [InlineData(0.125, true)] + [InlineData(0.4, false)] + [InlineData(0.45, false)] + [InlineData(1.05, false)] + public void FootCreature_RunsOnlyWhenFasterThanWalk(double activeMove, bool expected) + { + var pet = Spawn(activeMove); + + Assert.Equal(activeMove, pet.CurrentMoveSpeed); + Assert.Equal(expected, pet.AIObject.ShouldRun()); + } + + [Theory] + [InlineData(0.3, false)] + [InlineData(0.15, true)] + public void FlyingCreature_UsesMountThresholds(double activeMove, bool expected) + { + var pet = Spawn(activeMove); + pet.Flying = true; + + Assert.Equal(expected, pet.AIObject.ShouldRun()); + } + + [Fact] + public void BadlyHurt_SlowsBelowWalk_DropsToWalk() + { + var pet = Spawn(0.35); + Assert.True(pet.AIObject.ShouldRun()); + + // The hurt inflation is on the observed step pace, so the flag follows it. + pet.SetHits(100); + pet.Hits = 5; + pet.SetStam(100); + pet.Stam = 5; + + Assert.False(pet.AIObject.ShouldRun()); + } + + [Theory] + [InlineData(0.3, true)] + [InlineData(0.45, false)] + public void DoMove_StampsRunningBit(double activeMove, bool expected) + { + var map = Map.Maps[1]; + Assert.NotNull(map); + map.GetAverageZ(1500, 1600, out _, out var z, out _); + + var pet = Spawn(activeMove); + pet.MoveToWorld(new Point3D(1500, 1600, (sbyte)z), map); + + var ai = pet.AIObject; + ai.NextMove = 0; + var start = pet.Location; + + Assert.True(ai.DoMove(Direction.West)); + Assert.NotEqual(start, pet.Location); + Assert.Equal(expected, (pet.Direction & Direction.Running) != 0); + } + + // An isolated step (after standing at least a walk interval) renders alone and darts + // if run-flagged, so it walks; continuing cadences and true sprinters keep the flag. + [Fact] + public void IsolatedStep_DropsToWalk() + { + var pet = Spawn(0.3); + pet.LastMoveTime = Core.TickCount - 1000; + + Assert.False(pet.AIObject.ShouldRun()); + } + + [Fact] + public void IsolatedStep_SprinterStillRuns() + { + var pet = Spawn(0.125); + pet.LastMoveTime = Core.TickCount - 1000; + + Assert.True(pet.AIObject.ShouldRun()); + } + + [Fact] + public void StallDoesNotBankCatchUpSteps() + { + var map = Map.Maps[1]; + Assert.NotNull(map); + map.GetAverageZ(1500, 1600, out _, out var z, out _); + + var pet = Spawn(0.3); + pet.MoveToWorld(new Point3D(1500, 1600, (sbyte)z), map); + + var ai = pet.AIObject; + ai.NextMove = Core.TickCount - 1000; + + Assert.True(ai.DoMove(Direction.West)); + + // A stall must restart the cadence at full pace: banked catch-up steps + // release as a burst the client renders as a sprint/teleport. + Assert.False(ai.CanMoveNow(out _)); + Assert.True(ai.NextMove - Core.TickCount > 250); + } + + [Fact] + public void LateStepDoesNotEarnAQuickerFollowUp() + { + var map = Map.Maps[1]; + Assert.NotNull(map); + map.GetAverageZ(1500, 1600, out _, out var z, out _); + + var pet = Spawn(0.3); + pet.MoveToWorld(new Point3D(1500, 1600, (sbyte)z), map); + + pet.Warmode = true; // keep the active move clock through the step + + var ai = pet.AIObject; + // The step lands 200ms past the budget — under one period, the reactive + // mirroring case (think grid vs budget deadline misalignment). + ai.NextMove = Core.TickCount - 200; + + Assert.True(ai.DoMove(Direction.West)); + + // The debt must not be repaid: a sub-period catch-up step follows ~100ms + // behind and renders as a dart pair beside the player. + Assert.True(ai.NextMove - Core.TickCount > 250); + } +} diff --git a/Projects/UOContent/Engines/Factions/Mobiles/Guards/GuardAI.cs b/Projects/UOContent/Engines/Factions/Mobiles/Guards/GuardAI.cs index 32e8566cb..f0fb2386f 100644 --- a/Projects/UOContent/Engines/Factions/Mobiles/Guards/GuardAI.cs +++ b/Projects/UOContent/Engines/Factions/Mobiles/Guards/GuardAI.cs @@ -359,14 +359,14 @@ namespace Server.Factions { if (m_Mobile.InRange( m, 1 )) RunFrom( m ); - else if (!m_Mobile.InRange( m, m_Mobile.RangeFight > 2 ? m_Mobile.RangeFight : 2 ) && !MoveTo( m, true, 1 )) + else if (!m_Mobile.InRange( m, m_Mobile.RangeFight > 2 ? m_Mobile.RangeFight : 2 ) && !MoveTo(m, 1)) OnFailedMove(); } else {*/ if (!Mobile.InRange(m, Mobile.RangeFight)) { - if (!MoveTo(m, true, 1)) + if (!MoveTo(m, 1)) { OnFailedMove(); } diff --git a/Projects/UOContent/Engines/Pathing/PathFollower.cs b/Projects/UOContent/Engines/Pathing/PathFollower.cs index 3a0a56fa5..04aaf0148 100644 --- a/Projects/UOContent/Engines/Pathing/PathFollower.cs +++ b/Projects/UOContent/Engines/Pathing/PathFollower.cs @@ -83,7 +83,7 @@ public class PathFollower public static bool Check(Point3D loc, Point3D goal, int range) => Utility.InRange(loc, goal, range) && (range > 1 || (loc.Z - goal.Z).Abs() < 16); - public bool Follow(bool run, int range) + public bool Follow(int range) { var goal = GetGoalLocation(); Direction d; @@ -97,13 +97,13 @@ public class PathFollower if (!(Enabled && m_Path.Success)) { - d = m_From.GetDirectionTo(goal, run); + d = m_From.GetDirectionTo(goal); m_From.SetDirection(d); return Move(d) is MoveResult.Success or MoveResult.SuccessAutoTurn && Check(m_From.Location, goal, range); } - d = m_From.GetDirectionTo(m_Next, run); + d = m_From.GetDirectionTo(m_Next); m_From.SetDirection(d); var res = Move(d); diff --git a/Projects/UOContent/Mobiles/AI/AnimalAI.cs b/Projects/UOContent/Mobiles/AI/AnimalAI.cs index a9d410483..e5a1670e3 100644 --- a/Projects/UOContent/Mobiles/AI/AnimalAI.cs +++ b/Projects/UOContent/Mobiles/AI/AnimalAI.cs @@ -38,7 +38,7 @@ public class AnimalAI : BaseAI return true; } - if (!WalkMobileRange(combatant, 1, false, Mobile.RangeFight, Mobile.RangeFight)) + if (!WalkMobileRange(combatant, 1, Mobile.RangeFight, Mobile.RangeFight)) { if (Mobile.GetDistanceToSqrt(combatant) > Mobile.RangePerception + 1) { diff --git a/Projects/UOContent/Mobiles/AI/ArcherAI.cs b/Projects/UOContent/Mobiles/AI/ArcherAI.cs index dedd914c9..23ce0549c 100644 --- a/Projects/UOContent/Mobiles/AI/ArcherAI.cs +++ b/Projects/UOContent/Mobiles/AI/ArcherAI.cs @@ -43,7 +43,7 @@ public class ArcherAI : BaseAI return true; } - if (!WalkMobileRange(combatant, 1, false, Mobile.RangeFight, Mobile.Weapon.MaxRange)) + if (!WalkMobileRange(combatant, 1, Mobile.RangeFight, Mobile.Weapon.MaxRange)) { this.DebugSayFormatted($"I am still not in range of {combatant.Name}"); diff --git a/Projects/UOContent/Mobiles/AI/BaseAI/AIGroupMovement.cs b/Projects/UOContent/Mobiles/AI/BaseAI/AIGroupMovement.cs index 548fe0d54..4463bf4de 100644 --- a/Projects/UOContent/Mobiles/AI/BaseAI/AIGroupMovement.cs +++ b/Projects/UOContent/Mobiles/AI/BaseAI/AIGroupMovement.cs @@ -63,7 +63,7 @@ public abstract partial class BaseAI return crowding; } - public static bool MoveToWithGroup(BaseAI ai, Mobile target, bool run, int range) + public static bool MoveToWithGroup(BaseAI ai, Mobile target, int range) { if (Core.TickCount - _lastGroupUpdateTime > 1000) { @@ -79,7 +79,7 @@ public abstract partial class BaseAI if (optimalPosition == Point3D.Zero) { - return ai.MoveToWithCollisionAvoidance(target, run, range); + return ai.MoveToWithCollisionAvoidance(target, range); } _reservedPositions[mobile] = optimalPosition; @@ -99,7 +99,7 @@ public abstract partial class BaseAI } // A blocked or wall-slid step is not progress — route around the obstacle. - return ai.ApproachTarget(target, run, range); + return ai.ApproachTarget(target, range); } finally { diff --git a/Projects/UOContent/Mobiles/AI/BaseAI/AIMovement.cs b/Projects/UOContent/Mobiles/AI/BaseAI/AIMovement.cs index 5095a4cb4..8b2a16cf2 100644 --- a/Projects/UOContent/Mobiles/AI/BaseAI/AIMovement.cs +++ b/Projects/UOContent/Mobiles/AI/BaseAI/AIMovement.cs @@ -18,6 +18,7 @@ using System.Runtime.CompilerServices; using Server.Collections; using Server.Items; using MoveImpl = Server.Movement.MovementImpl; +using Moves = Server.Movement.Movement; namespace Server.Mobiles; @@ -43,7 +44,6 @@ public abstract partial class BaseAI // live, the AITimer wakes at NextMove between think ticks to advance the step. private Mobile _moveIntentTarget; private IPoint3D _moveIntentPoint; - private bool _moveIntentRun; private int _moveIntentRange; private long _moveIntentExpire; @@ -74,23 +74,42 @@ public abstract partial class BaseAI return Core.TickCount - NextMove >= 0; } - // Accumulative full-step budget: long-run pacing averages CurrentMoveSpeed exactly - // regardless of timer-grid jitter; snap-to-now caps stall catch-up at one step. - private void ConsumeMoveBudget() + // Seconds per step as the client observes it: the move clock plus the hurt inflation. + private double EffectiveStepDelay() { var stepDelay = Mobile.CurrentMoveSpeed; - if (!(Core.AOS && IsFollowingMaster())) + return Core.AOS && IsFollowingMaster() ? stepDelay : BadlyHurtMoveDelay(Mobile, stepDelay); + } + + // The Running bit only selects the client's per-step interpolation (walk 400ms / run + // 200ms on foot, 200/100 mounted). A step shorter than the walk time must run or the + // client falls behind and snaps — but an isolated step (after standing at least a walk + // interval) renders alone and darts if run-flagged, so it goes out as a walk. A true + // sprinter always runs: a walk-rendered first step would flood the client's queue. + public bool ShouldRun() + { + var mounted = Mobile.Mounted || Mobile.Flying; + var walkDelay = mounted ? Moves.WalkMountDelay : Moves.WalkFootDelay; + var pace = EffectiveStepDelay() * 1000; + + if (pace >= walkDelay) { - stepDelay = BadlyHurtMoveDelay(Mobile, stepDelay); + return false; } - NextMove += Math.Max(50, (long)(stepDelay * 1000)); + var runDelay = mounted ? Moves.RunMountDelay : Moves.RunFootDelay; - if (Core.TickCount - NextMove > 0) - { - NextMove = Core.TickCount; - } + return pace < runDelay || Core.TickCount - Mobile.LastMoveTime < walkDelay; + } + + // One step per period, paced from the step just taken — no debt accrual: repaying a + // late step with a quicker follow-up puts two steps ~100ms apart, which renders as a + // dart. In continuous pursuit the move-wake lands within wheel resolution of this + // deadline, so the only cost is single-digit-ms drift per step. + private void ConsumeMoveBudget() + { + NextMove = Core.TickCount + Math.Max(50, (long)(EffectiveStepDelay() * 1000)); } public virtual bool CheckMove() => !(Mobile.Deleted || Mobile.DisallowAllMoves); @@ -108,6 +127,8 @@ public abstract partial class BaseAI return MoveResult.BadState; } + d = (d & Direction.Mask) | (ShouldRun() ? Direction.Running : 0); + if ((Mobile.Direction & Direction.Mask) != (d & Direction.Mask)) { Mobile.Direction = d; @@ -334,7 +355,7 @@ public abstract partial class BaseAI /// best-distance stall counter idles the creature if an in-range goal is genuinely /// unreachable, without ever abandoning a real chase or detour. /// - protected bool ApproachTarget(Mobile target, bool run, int range) + protected bool ApproachTarget(Mobile target, int range) { if (Mobile.Deleted || Mobile.DisallowAllMoves || target?.Deleted != false) { @@ -361,7 +382,7 @@ public abstract partial class BaseAI ResetApproach(); // target moved — try again fresh } - RenewMoveIntent(target, null, run, range); + RenewMoveIntent(target, null, range); // FAST PATH: greedy step toward the target, counted as success ONLY when the move // fully succeeded (not an auto-turn sidestep) and actually got us closer. An @@ -373,7 +394,7 @@ public abstract partial class BaseAI if (Path == null && Mobile.InLOS(target)) { var distBefore = Mobile.GetDistanceToSqrt(target); - var res = DoMoveImpl(Mobile.GetDirectionTo(target, run), true); + var res = DoMoveImpl(Mobile.GetDirectionTo(target), true); if (res == MoveResult.BadState) { @@ -402,7 +423,7 @@ public abstract partial class BaseAI var couldMove = CanMoveNow(out _) && !IsInBadState(); var locBefore = Mobile.Location; - if (Path.Follow(run, range)) + if (Path.Follow(range)) { ResetApproach(); return true; @@ -421,7 +442,7 @@ public abstract partial class BaseAI /// Walks toward a fixed point (e.g. a target's last-known position), pathfinding around /// obstacles. Returns false on arrival or when genuinely unable to make progress. /// - public bool MoveToPoint(IPoint3D goal, bool run) + public bool MoveToPoint(IPoint3D goal) { if (Mobile.Deleted || Mobile.DisallowAllMoves || goal == null) { @@ -434,12 +455,12 @@ public abstract partial class BaseAI Path = new PathFollower(Mobile, goal) { Mover = DoMoveImpl }; } - RenewMoveIntent(null, goal, run, 1); + RenewMoveIntent(null, goal, 1); var couldMove = CanMoveNow(out _) && !IsInBadState(); var locBefore = Mobile.Location; - if (Path.Follow(run, 1)) + if (Path.Follow(1)) { Path = null; ClearMoveIntent(); @@ -515,11 +536,10 @@ public abstract partial class BaseAI _approachGaveUp = false; } - private void RenewMoveIntent(Mobile target, IPoint3D point, bool run, int range) + private void RenewMoveIntent(Mobile target, IPoint3D point, int range) { _moveIntentTarget = target; _moveIntentPoint = point; - _moveIntentRun = run; _moveIntentRange = range; // A live pursuit renews every think tick; unrenewed intent dies on its own. @@ -556,26 +576,21 @@ public abstract partial class BaseAI if (_moveIntentTarget != null) { - ApproachTarget(_moveIntentTarget, _moveIntentRun, _moveIntentRange); + ApproachTarget(_moveIntentTarget, _moveIntentRange); } else { - MoveToPoint(_moveIntentPoint, _moveIntentRun); + MoveToPoint(_moveIntentPoint); } } - public virtual bool MoveTo(Mobile m, bool run, int range) + public virtual bool MoveTo(Mobile m, int range) { if (Mobile.Deleted || Mobile.DisallowAllMoves || m?.Deleted != false) { return false; } - var distance = (int)Mobile.GetDistanceToSqrt(m); - //TODO Derive the Running bit from CurrentMoveSpeed in DoMoveImpl and drop the run parameter - var distanceThreshold = Core.AOS && IsFollowingMaster() ? 1 : 3; - var shouldRun = distance > distanceThreshold; - if (Mobile.InRange(m, range)) { ResetApproach(); @@ -584,10 +599,10 @@ public abstract partial class BaseAI if (UseGroupMovement(m, range)) { - return MoveToWithGroup(this, m, shouldRun, range); + return MoveToWithGroup(this, m, range); } - return ApproachTarget(m, shouldRun, range); + return ApproachTarget(m, range); } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -604,12 +619,8 @@ public abstract partial class BaseAI Mobile.Combatant == null && Mobile.ControlOrder is OrderType.Come or OrderType.Follow or OrderType.Guard; - private bool MoveToWithCollisionAvoidance(Mobile target, bool run, int range) + private bool MoveToWithCollisionAvoidance(Mobile target, int range) { - var distance = (int)Mobile.GetDistanceToSqrt(target); - - var shouldRun = run && distance > 5; - var direction = Mobile.GetDirectionTo(target); // Wall-slide auto-turns must not count as progress, or a creature pinned on @@ -640,10 +651,10 @@ public abstract partial class BaseAI // Tactical sidesteps exhausted — route around the obstacle via the centralized // approach primitive (persistent PathFollower, no oscillation). - return ApproachTarget(target, shouldRun, range); + return ApproachTarget(target, range); } - public virtual bool WalkMobileRange(Mobile m, int iSteps, bool run, int iWantDistMin, int iWantDistMax) + public virtual bool WalkMobileRange(Mobile m, int iSteps, int iWantDistMin, int iWantDistMax) { if (Mobile.Deleted || Mobile.DisallowAllMoves || m == null) { @@ -659,7 +670,7 @@ public abstract partial class BaseAI return true; } - if (!MoveTowardsOrAwayFrom(m, run, iCurrDist, iWantDistMax)) + if (!MoveTowardsOrAwayFrom(m, iCurrDist, iWantDistMax)) { return false; } @@ -670,17 +681,16 @@ public abstract partial class BaseAI return dist >= iWantDistMin && dist <= iWantDistMax; } - // run only sets the client animation; callers gate it on their own distance thresholds. - private bool MoveTowardsOrAwayFrom(Mobile m, bool run, int iCurrDist, int iWantDistMax) + private bool MoveTowardsOrAwayFrom(Mobile m, int iCurrDist, int iWantDistMax) { if (iCurrDist > iWantDistMax) { // Too far: approach via the centralized progress-based primitive. - return ApproachTarget(m, run, iWantDistMax); + return ApproachTarget(m, iWantDistMax); } // Too close: back away. Retreat keeps the simple greedy behavior (out of scope). - if (DoMove(m.GetDirectionTo(Mobile, run), true)) + if (DoMove(m.GetDirectionTo(Mobile), true)) { Path = null; return true; diff --git a/Projects/UOContent/Mobiles/AI/BaseAI/BaseAI.cs b/Projects/UOContent/Mobiles/AI/BaseAI/BaseAI.cs index f86ac2bfe..3c2a479e0 100644 --- a/Projects/UOContent/Mobiles/AI/BaseAI/BaseAI.cs +++ b/Projects/UOContent/Mobiles/AI/BaseAI/BaseAI.cs @@ -442,7 +442,7 @@ public abstract partial class BaseAI var master = Mobile.SummonMaster; if (master != null && master.Map == Mobile.Map && master.InRange(Mobile, Mobile.RangePerception)) { - MoveTo(master, false, 1); + MoveTo(master, 1); } } @@ -592,7 +592,7 @@ public abstract partial class BaseAI } _lkpGoal ??= _lkpLocation; - return MoveToPoint(_lkpGoal, false); + return MoveToPoint(_lkpGoal); } private void ClearLastKnown() @@ -644,7 +644,7 @@ public abstract partial class BaseAI _herdGoal = new Point3D(target.X, target.Y, Mobile.Map?.GetAverageZ(target.X, target.Y) ?? Mobile.Z); } - MoveToPoint(_herdGoal, false); + MoveToPoint(_herdGoal); return true; } @@ -798,7 +798,7 @@ public abstract partial class BaseAI { if (AcquireFocusMob(Mobile.RangePerception * 2, FightMode.Closest, true, false, true)) { - if (WalkMobileRange(Mobile.FocusMob, 1, false, Mobile.RangePerception, Mobile.RangePerception * 2)) + if (WalkMobileRange(Mobile.FocusMob, 1, Mobile.RangePerception, Mobile.RangePerception * 2)) { DebugSay("I backed off to safety. Wandering..."); diff --git a/Projects/UOContent/Mobiles/AI/BaseAI/PetOrders.cs b/Projects/UOContent/Mobiles/AI/BaseAI/PetOrders.cs index 54dd5774f..e6c48850d 100644 --- a/Projects/UOContent/Mobiles/AI/BaseAI/PetOrders.cs +++ b/Projects/UOContent/Mobiles/AI/BaseAI/PetOrders.cs @@ -84,7 +84,7 @@ public abstract partial class BaseAI return true; } - WalkMobileRange(Mobile.ControlMaster, 1, false, 1, 2); + WalkMobileRange(Mobile.ControlMaster, 1, 1, 2); if (Mobile.GetDistanceToSqrt(Mobile.ControlMaster) <= 2) { @@ -136,7 +136,7 @@ public abstract partial class BaseAI if (currentDistance > 1) { - WalkMobileRange(Mobile.ControlTarget, 1, currentDistance > 2, 1, 2); + WalkMobileRange(Mobile.ControlTarget, 1, 1, 2); } } @@ -333,7 +333,7 @@ public abstract partial class BaseAI Mobile.SetCurrentSpeedToActive(); } - WalkMobileRange(controlMaster, 1, true, 1, 3); + WalkMobileRange(controlMaster, 1, 1, 3); } else { diff --git a/Projects/UOContent/Mobiles/AI/BerserkAI.cs b/Projects/UOContent/Mobiles/AI/BerserkAI.cs index 4663ae8d0..ff00ec91d 100644 --- a/Projects/UOContent/Mobiles/AI/BerserkAI.cs +++ b/Projects/UOContent/Mobiles/AI/BerserkAI.cs @@ -38,7 +38,7 @@ public class BerserkAI : BaseAI return true; } - if (!WalkMobileRange(combatant, 1, false, Mobile.RangeFight, Mobile.RangeFight)) + if (!WalkMobileRange(combatant, 1, Mobile.RangeFight, Mobile.RangeFight)) { this.DebugSayFormatted($"I am still not in range of {combatant.Name}"); diff --git a/Projects/UOContent/Mobiles/AI/HealerAI.cs b/Projects/UOContent/Mobiles/AI/HealerAI.cs index 2a1127b43..cc54c73c3 100644 --- a/Projects/UOContent/Mobiles/AI/HealerAI.cs +++ b/Projects/UOContent/Mobiles/AI/HealerAI.cs @@ -81,7 +81,7 @@ public class HealerAI : BaseAI return true; } - WalkMobileRange(Mobile.FocusMob, 1, false, 4, 7); + WalkMobileRange(Mobile.FocusMob, 1, 4, 7); // TODO: Should it be able to do this? if (Mobile.TriggerAbility(MonsterAbilityTrigger.CombatAction, Mobile.Combatant)) diff --git a/Projects/UOContent/Mobiles/AI/MageAI.cs b/Projects/UOContent/Mobiles/AI/MageAI.cs index 61be59a1f..1ed8cbdda 100644 --- a/Projects/UOContent/Mobiles/AI/MageAI.cs +++ b/Projects/UOContent/Mobiles/AI/MageAI.cs @@ -171,7 +171,7 @@ public class MageAI : BaseAI { if (!SmartAI) { - if (!MoveTo(m, false, Mobile.RangeFight)) + if (!MoveTo(m, Mobile.RangeFight)) { OnFailedMove(); } @@ -185,14 +185,14 @@ public class MageAI : BaseAI { RunFrom(m); } - else if (!Mobile.InRange(m, Math.Max(Mobile.RangeFight, 2)) && !MoveTo(m, false, 1)) + else if (!Mobile.InRange(m, Math.Max(Mobile.RangeFight, 2)) && !MoveTo(m, 1)) { OnFailedMove(); } } else if (!Mobile.InRange(m, Mobile.RangeFight)) { - if (!MoveTo(m, false, 1)) + if (!MoveTo(m, 1)) { OnFailedMove(); } @@ -701,7 +701,7 @@ public class MageAI : BaseAI { DebugSay("I cannot see my target, moving to regain line of sight"); - if (!MoveTo(c, false, 1)) + if (!MoveTo(c, 1)) { OnFailedMove(); } @@ -1039,7 +1039,7 @@ public class MageAI : BaseAI // target can be invoked. if (!Mobile.InLOS(toTarget)) { - MoveTo(toTarget, true, 1); + MoveTo(toTarget, 1); } else { diff --git a/Projects/UOContent/Mobiles/AI/MeleeAI.cs b/Projects/UOContent/Mobiles/AI/MeleeAI.cs index 544770069..a71d83d5c 100644 --- a/Projects/UOContent/Mobiles/AI/MeleeAI.cs +++ b/Projects/UOContent/Mobiles/AI/MeleeAI.cs @@ -99,7 +99,7 @@ public class MeleeAI : BaseAI private bool AttemptMoveToCombatant(Mobile combatant) { - if (MoveTo(combatant, false, Mobile.RangeFight)) + if (MoveTo(combatant, Mobile.RangeFight)) { return true; } diff --git a/Projects/UOContent/Mobiles/AI/PredatorAI.cs b/Projects/UOContent/Mobiles/AI/PredatorAI.cs index 5e0e01520..b1ea3a638 100644 --- a/Projects/UOContent/Mobiles/AI/PredatorAI.cs +++ b/Projects/UOContent/Mobiles/AI/PredatorAI.cs @@ -41,7 +41,7 @@ public class PredatorAI : BaseAI return true; } - if (!WalkMobileRange(combatant, 1, false, Mobile.RangeFight, Mobile.RangeFight)) + if (!WalkMobileRange(combatant, 1, Mobile.RangeFight, Mobile.RangeFight)) { if (Mobile.GetDistanceToSqrt(combatant) > Mobile.RangePerception + 1) { @@ -70,7 +70,7 @@ public class PredatorAI : BaseAI } else if (AcquireFocusMob(Mobile.RangePerception * 2, FightMode.Closest, true, false, true)) { - if (WalkMobileRange(Mobile.FocusMob, 1, false, Mobile.RangePerception, Mobile.RangePerception * 2)) + if (WalkMobileRange(Mobile.FocusMob, 1, Mobile.RangePerception, Mobile.RangePerception * 2)) { DebugSay("Well, here I am safe"); diff --git a/Projects/UOContent/Mobiles/AI/ThiefAI.cs b/Projects/UOContent/Mobiles/AI/ThiefAI.cs index 0fa37bbc9..a9209dfbb 100644 --- a/Projects/UOContent/Mobiles/AI/ThiefAI.cs +++ b/Projects/UOContent/Mobiles/AI/ThiefAI.cs @@ -43,7 +43,7 @@ public class ThiefAI : BaseAI return true; } - if (!WalkMobileRange(combatant, 1, false, Mobile.RangeFight, Mobile.RangeFight)) + if (!WalkMobileRange(combatant, 1, Mobile.RangeFight, Mobile.RangeFight)) { this.DebugSayFormatted($"I should be closer to {combatant.Name}"); } diff --git a/Projects/UOContent/Mobiles/BaseCreature.cs b/Projects/UOContent/Mobiles/BaseCreature.cs index 15d9b4c03..05c728cf4 100644 --- a/Projects/UOContent/Mobiles/BaseCreature.cs +++ b/Projects/UOContent/Mobiles/BaseCreature.cs @@ -2861,7 +2861,7 @@ namespace Server.Mobiles CanBeHarmful(m) && IsEnemy(m)) { Combatant = FocusMob = m; - AIObject?.MoveTo(m, true, 1); + AIObject?.MoveTo(m, 1); DoHarmful(m); } } diff --git a/Projects/UOContent/Mobiles/Familiars/BaseFamiliar.cs b/Projects/UOContent/Mobiles/Familiars/BaseFamiliar.cs index d3b119523..46d939f3b 100644 --- a/Projects/UOContent/Mobiles/Familiars/BaseFamiliar.cs +++ b/Projects/UOContent/Mobiles/Familiars/BaseFamiliar.cs @@ -92,7 +92,7 @@ public abstract partial class BaseFamiliar : BaseCreature Hidden = m_LastHidden = master.Hidden; } - if (AIObject?.WalkMobileRange(master, 5, false, 1, 1) == true) + if (AIObject?.WalkMobileRange(master, 5, 1, 1) == true) { Warmode = master.Warmode; Combatant = master.Combatant; diff --git a/Projects/UOContent/Mobiles/Monsters/LBR/Meers/EnragedCreatures.cs b/Projects/UOContent/Mobiles/Monsters/LBR/Meers/EnragedCreatures.cs index 91367a5f0..ce4f3526d 100644 --- a/Projects/UOContent/Mobiles/Monsters/LBR/Meers/EnragedCreatures.cs +++ b/Projects/UOContent/Mobiles/Monsters/LBR/Meers/EnragedCreatures.cs @@ -108,7 +108,7 @@ namespace Server.Mobiles */ else if (!Combat(this)) { - AIObject?.MoveTo(SummonMaster, false, 5); + AIObject?.MoveTo(SummonMaster, 5); } /* On OSI, if the summon attacks a mobile, the summoner meer also diff --git a/Projects/UOContent/Spells/Ninjitsu/MirrorImage.cs b/Projects/UOContent/Spells/Ninjitsu/MirrorImage.cs index 6d85b9d73..63788aa69 100644 --- a/Projects/UOContent/Spells/Ninjitsu/MirrorImage.cs +++ b/Projects/UOContent/Spells/Ninjitsu/MirrorImage.cs @@ -238,10 +238,7 @@ namespace Server.Mobiles if (master?.Map == Mobile.Map && master?.InRange(Mobile, Mobile.RangePerception) == true) { - var iCurrDist = (int)Mobile.GetDistanceToSqrt(master); - var bRun = iCurrDist > 5; - - WalkMobileRange(master, 2, bRun, 0, 1); + WalkMobileRange(master, 2, 0, 1); } else { diff --git a/dev-docs/claude-skills/migrate-from-runuo/migrate-items-mobiles.md b/dev-docs/claude-skills/migrate-from-runuo/migrate-items-mobiles.md index 8edf70a72..ce70ad26c 100644 --- a/dev-docs/claude-skills/migrate-from-runuo/migrate-items-mobiles.md +++ b/dev-docs/claude-skills/migrate-from-runuo/migrate-items-mobiles.md @@ -29,6 +29,7 @@ description: > - `BaseCreature(AI, Fight, 10, 1, 0.2, 0.4)` -> `BaseCreature(AI, Fight)` (extra params default) - `Name = "text"` -> `public override string DefaultName => "text";` - Expression-bodied overrides: `public override int Meat { get { return 1; } }` -> `public override int Meat => 1;` +- AI movement calls lose the `run` flag: `MoveTo(m, true, range)` -> `MoveTo(m, range)` (also `WalkMobileRange`, `ApproachTarget`, `MoveToPoint`, `PathFollower.Follow`); the Running bit is derived from step pace -> `dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md` § AI Movement ## Anti-Patterns - Using `_field--` instead of `Property--` (bypasses MarkDirty tracking) diff --git a/dev-docs/claude-skills/modernuo-content-patterns.md b/dev-docs/claude-skills/modernuo-content-patterns.md index cdb3f9e49..2b6df7d55 100644 --- a/dev-docs/claude-skills/modernuo-content-patterns.md +++ b/dev-docs/claude-skills/modernuo-content-patterns.md @@ -27,8 +27,9 @@ description: > (`ActiveSpeed`/`PassiveSpeed`, seconds per AI decision) and move (`ActiveMoveSpeed`/`PassiveMoveSpeed`, seconds per step; inherits think until overridden). Prefer `npc-speeds.json` buckets (`SpeedClass`); `SetSpeed()` sets think - AND clears move overrides, `SetMoveSpeed()` sets move only -- see - `dev-docs/content-patterns.md` § Creature Speeds + AND clears move overrides, `SetMoveSpeed()` sets move only. The client `Running` bit is + derived from the step pace (`BaseAI.ShouldRun`); movement APIs take no run argument -- + see `dev-docs/content-patterns.md` § Creature Speeds 8. **`OnThink` overrides must be excess-call tolerant** -- it fires more often than the think cadence (player commands prod it; speed-ups reschedule it). Gate consequential work on a tick-count deadline (subtraction form) or make it idempotent; bare per-call diff --git a/dev-docs/content-patterns.md b/dev-docs/content-patterns.md index 80d925580..3d11efb16 100644 --- a/dev-docs/content-patterns.md +++ b/dev-docs/content-patterns.md @@ -283,6 +283,18 @@ 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). +The client's `Running` bit is derived from the step pace, never passed by callers +(`BaseAI.ShouldRun`, stamped in `DoMoveImpl`): a step shorter than the client's walk +interpolation — 400 ms on foot, 200 ms mounted/flying (`Movement.WalkFootDelay` / +`WalkMountDelay`) — is flagged as a run, or the client falls behind and snaps. An isolated +step (resuming after at least a walk interval standing) goes out as a walk regardless of +pace — the client renders each step alone, so a run-flagged single step darts — unless the +pace beats the run interpolation (a true sprinter), where a walk-rendered first step would +flood the client's step queue. Movement APIs (`MoveTo`, `WalkMobileRange`, +`ApproachTarget`, `MoveToPoint`) take no run argument; to make a creature run, make it +fast. Creatures step at most once per `CurrentMoveSpeed` period, paced from the step just +taken — a stall never banks catch-up steps, so a resumed chase restarts at full pace. + ### OnThink: the excess-call contract `OnThink()` is a scheduler pass, not an action. The AI timer calls it *at least* at the diff --git a/dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md b/dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md index c5029131c..7d4581b20 100644 --- a/dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md +++ b/dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md @@ -474,6 +474,29 @@ The extra parameters (RangePerception, RangeFight, ActiveSpeed, PassiveSpeed) ha | `Name = "a creature"` in constructor | `public override string DefaultName => "a creature";` | | `get { return value; }` | `=> value;` expression-bodied | +## AI Movement: No `run` Argument + +RunUO's movement calls took a `run` flag that callers set inconsistently (`true` in +combat, `false` for pets, gated by `dist > 5` inside `MoveTo`). The flag only selects the +client's per-step animation time, so ModernUO derives it from the creature's step pace +(`BaseAI.ShouldRun`) and the parameter is gone: + +```csharp +// RunUO +MoveTo(combatant, true, m_Mobile.RangeFight); +WalkMobileRange(m_Mobile.ControlMaster, 1, false, 0, 1); + +// ModernUO +MoveTo(combatant, Mobile.RangeFight); +WalkMobileRange(Mobile.ControlMaster, 1, 0, 1); +``` + +`ApproachTarget`, `MoveToPoint` and `PathFollower.Follow` lose the argument the same way. +To make a creature run, make it fast (`SetMoveSpeed` / `npc-speeds.json`), not flagged. +An isolated step (after the creature stood for at least a walk interval) goes out as a +walk regardless of pace — only a continuing cadence, or a pace faster than the run +interpolation, flags run. + ## Item Name Changes ```csharp diff --git a/dev-docs/runuo-migration-docs/11-api-reference.md b/dev-docs/runuo-migration-docs/11-api-reference.md index 215dd3b06..358bf13c7 100644 --- a/dev-docs/runuo-migration-docs/11-api-reference.md +++ b/dev-docs/runuo-migration-docs/11-api-reference.md @@ -130,6 +130,9 @@ Alphabetical by RunUO API name. Use Ctrl+F / Cmd+F to search. | `writer.WriteEncodedInt(value)` | `writer.WriteEncodedInt(value)` | Same | | `InvalidateProperties()` | `InvalidateProperties()` | Same, or use `[InvalidateProperties]` | | `this.MarkDirty()` | `this.MarkDirty()` | NEW — required in custom setters | +| `MoveTo(m, run, range)` | `MoveTo(m, range)` | `run` removed; the Running bit is derived from the step pace (`BaseAI.ShouldRun`) | +| `WalkMobileRange(m, steps, run, min, max)` | `WalkMobileRange(m, steps, min, max)` | Same | +| `PathFollower.Follow(run, range)` | `Follow(range)` | Same | ## Networking From c9875e7f642ce505a5231e8dcfd58bfc955fc31b Mon Sep 17 00:00:00 2001 From: Sergi Rosell <50594106+srosellj@users.noreply.github.com> Date: Mon, 31 Aug 2026 17:46:20 +0200 Subject: [PATCH 4/9] fix: delete the bonus item, not the primary yield, when the bonus cannot be placed (#2602) --- Projects/UOContent/Engines/Harvest/Core/HarvestSystem.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Projects/UOContent/Engines/Harvest/Core/HarvestSystem.cs b/Projects/UOContent/Engines/Harvest/Core/HarvestSystem.cs index b82040134..b67665d04 100644 --- a/Projects/UOContent/Engines/Harvest/Core/HarvestSystem.cs +++ b/Projects/UOContent/Engines/Harvest/Core/HarvestSystem.cs @@ -219,7 +219,7 @@ namespace Server.Engines.Harvest } else { - item.Delete(); + bonusItem?.Delete(); } } From 547c2ea0fa1acfcc1914e0805f25d1b48977454a Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Tue, 1 Sep 2026 20:25:20 -0700 Subject: [PATCH 5/9] fix: Fixes tick count wrap-around in movement throttle, and eliminates more allocations in NetState (#2603) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Removes the per-tick allocation in the movement throttle, fixes tick-count wrap-around bugs in the throttle and RTT probe state, and trims per-connection allocations and dead fields in `NetState`. ## Movement throttle - **No more per-tick `List` snapshot.** `ProcessAllQueues()` iterates the `HashSet` directly and removes drained or disconnected states in place. `HashSet.Remove` does not invalidate enumerators on .NET Core 3.0+ (verified on 10.0.11); only inserting a *new* member does, and the only `Add` is in the packet handler, which never nests with `Slice()`. The eager `Remove` calls in `RejectAndReset`, `ClearQueue`, and `ProcessMovementQueue` are gone; membership is reconciled once per tick from `_hasQueuedMovements`. - **Debug logging** is now gated solely by the per-connection `NetState.MovementLogging` flag. The global `movementThrottle.debugLogging` setting is removed. - **New settings**: `movementThrottle.maxRttBonus`, `movementThrottle.maxChainGap`, and `movementThrottle.speedHackNotificationCooldown` were fields with no config binding. ## Tick-count wrap-around All comparisons are now in subtraction form and no tick field uses zero as a sentinel: - `now < _nextMovementTime` in the queue drain loop → `now - _nextMovementTime < 0`. - `_lastMovementRecordTime > 0`, `_lastSpeedHackNotification`, `_rttProbeTime > 0`, and `_nextRttProbe == 0` sentinels replaced with `_hasMovementRecord`, `_speedHackNotified`, `_rttProbePending`, and a seeded `_nextRttProbe`. - `_lastQueueDepthCheck` and `_movementWindowStart` are seeded from `Core.TickCount` at construction and on reset instead of zero. User-visible effects of the old code: on hosts with pass-through counters (GCP) movement history never recorded and speed hack detection was silently off; on every host, staff speed hack notifications were suppressed until `Core.TickCount` exceeded the five-minute cooldown. ## NetState - `Instances` returns `HashSet` again so engine-internal `foreach` uses the struct enumerator instead of boxing through `IReadOnlySet`. - Removed `_sustainedQueueDepth` (declared and zeroed since #2266, never read), `_lastRtt` (now derived as `LastRtt` from the newest history slot), and `_rttProbeTimestampHiRes` (only fed one debug log line). 20 bytes per connection. - `HuePickers`, `Menus`, and `Trades` are lazily created instead of allocating three lists per connection, including every login-server connection that dies on shard select. `Trades` is released when it empties. All helpers and the `HuePickerResponse` / `MenuResponse` handlers are null-tolerant; the trade cancel loops keep their `i < Count` guards because `SecureTrade.Cancel()` runs virtual item hooks that can re-enter the same list. ## Testing - `dotnet build -c Release` clean. - All MovementThrottle tests pass (27), plus the Trade / Menu / HuePicker / NetState tests (32). --- Projects/Server/Mobiles/Mobile.cs | 2 +- Projects/Server/Network/MovementThrottle.cs | 138 ++++++++---------- .../Network/NetState/NetState.Movement.cs | 52 +++---- Projects/Server/Network/NetState/NetState.cs | 60 +++++++- .../Network/Packets/IncomingPlayerPackets.cs | 21 ++- 5 files changed, 153 insertions(+), 120 deletions(-) diff --git a/Projects/Server/Mobiles/Mobile.cs b/Projects/Server/Mobiles/Mobile.cs index fb028b9d7..208142225 100644 --- a/Projects/Server/Mobiles/Mobile.cs +++ b/Projects/Server/Mobiles/Mobile.cs @@ -1627,7 +1627,7 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro public virtual bool KeepsItemsOnDeath => m_AccessLevel > AccessLevel.Player; - public bool HasTrade => m_NetState?.Trades.Count > 0; + public bool HasTrade => m_NetState?.Trades?.Count > 0; public bool NoMoveHS { get; set; } diff --git a/Projects/Server/Network/MovementThrottle.cs b/Projects/Server/Network/MovementThrottle.cs index c28ef2228..33ee318fc 100644 --- a/Projects/Server/Network/MovementThrottle.cs +++ b/Projects/Server/Network/MovementThrottle.cs @@ -50,9 +50,6 @@ public static class MovementThrottle private const int ClientMaxUnackedMovements = 5; private const int MaxQueueWithUnmodifiedClient = ClientMaxUnackedMovements - 1; // 4 - // Debug logging - enable for testing speed hack detection - private static bool _debugLogging = false; - // Track NetStates with queued movements for efficient processing private static readonly HashSet _netStatesWithQueuedMovements = new(256); @@ -83,15 +80,9 @@ public static class MovementThrottle public static void Configure() { - _maxCredit = ServerConfiguration.GetOrUpdateSetting( - "movementThrottle.maxCredit", - _maxCredit - ); - - _hardQueueLimit = ServerConfiguration.GetOrUpdateSetting( - "movementThrottle.hardQueueLimit", - _hardQueueLimit - ); + _maxCredit = ServerConfiguration.GetOrUpdateSetting("movementThrottle.maxCredit", _maxCredit); + _maxRttBonus = ServerConfiguration.GetOrUpdateSetting("movementThrottle.maxRttBonus", _maxRttBonus); + _hardQueueLimit = ServerConfiguration.GetOrUpdateSetting("movementThrottle.hardQueueLimit", _hardQueueLimit); _movementHistorySize = ServerConfiguration.GetOrUpdateSetting( "movementThrottle.movementHistorySize", @@ -103,6 +94,13 @@ public static class MovementThrottle _minSamplesForRate ); + _maxChainGap = ServerConfiguration.GetOrUpdateSetting("movementThrottle.maxChainGap", _maxChainGap); + + _speedHackNotificationCooldown = ServerConfiguration.GetOrUpdateSetting( + "movementThrottle.speedHackNotificationCooldown", + _speedHackNotificationCooldown + ); + _suspiciousRateThreshold = (float)ServerConfiguration.GetOrUpdateSetting( "movementThrottle.suspiciousRateThreshold", _suspiciousRateThreshold @@ -112,11 +110,6 @@ public static class MovementThrottle "movementThrottle.definiteRateThreshold", _definiteRateThreshold ); - - _debugLogging = ServerConfiguration.GetOrUpdateSetting( - "movementThrottle.debugLogging", - _debugLogging - ); } /// @@ -191,15 +184,16 @@ public static class MovementThrottle // Credit can go negative up to -dynamicCredit (debt limit) if (ns._movementCredit - earlyAmount >= -dynamicCredit) { - var prevCredit = ns._movementCredit; // Use credit to cover early arrival ns._movementCredit -= earlyAmount; - if (_debugLogging && ns._movementLogging) + if (ns._movementLogging) { + var prevCredit = ns._movementCredit + earlyAmount; + logger.Debug( "[Credit] {Name}: delta={Delta}ms early={Early}ms credit={PrevCredit}->{Credit}/{MaxCredit} action=execute", - mobile.RawName, delta, earlyAmount, prevCredit, ns._movementCredit, dynamicCredit + mobile, delta, earlyAmount, prevCredit, ns._movementCredit, dynamicCredit ); } @@ -208,11 +202,11 @@ public static class MovementThrottle return; } - if (_debugLogging && ns._movementLogging) + if (ns._movementLogging) { logger.Debug( "[Credit] {Name}: delta={Delta}ms early={Early}ms credit={Credit}/{MaxCredit} EXHAUSTED -> queue", - mobile.RawName, delta, earlyAmount, ns._movementCredit, dynamicCredit + mobile, delta, earlyAmount, ns._movementCredit, dynamicCredit ); } @@ -227,11 +221,11 @@ public static class MovementThrottle var prevCredit = ns._movementCredit; ns._movementCredit = Math.Min(ns._movementCredit + delta, dynamicCredit); - if (_debugLogging && ns._movementLogging && ns._movementCredit != prevCredit) + if (ns._movementLogging && ns._movementCredit != prevCredit) { logger.Debug( "[Credit] {Name}: delta=+{Delta}ms credit={PrevCredit}->{Credit}/{MaxCredit} action=execute", - mobile.RawName, delta, prevCredit, ns._movementCredit, dynamicCredit + mobile, delta, prevCredit, ns._movementCredit, dynamicCredit ); } } @@ -247,12 +241,9 @@ public static class MovementThrottle { if (!mobile.Move(dir)) { - if (_debugLogging && ns._movementLogging) + if (ns._movementLogging) { - logger.Debug( - "[Execute] {Name}: Move FAILED dir={Dir} seq={Seq} -> reject+reset", - mobile.RawName, dir, seq - ); + logger.Debug("[Execute] {Name}: Move FAILED dir={Dir} seq={Seq} -> reject+reset", mobile, dir, seq); } // Movement failed (blocked, paralyzed, frozen, etc.) @@ -260,11 +251,11 @@ public static class MovementThrottle return; } - if (_debugLogging && ns._movementLogging) + if (ns._movementLogging) { logger.Debug( "[Execute] {Name}: Move OK dir={Dir} seq={Seq} nextMove={NextMove}ms", - mobile.RawName, dir, seq, ns._nextMovementTime - Core.TickCount + mobile, dir, seq, ns._nextMovementTime - Core.TickCount ); } @@ -304,11 +295,11 @@ public static class MovementThrottle ns._hasQueuedMovements = true; _netStatesWithQueuedMovements.Add(ns); - if (_debugLogging && ns._movementLogging) + if (ns._movementLogging) { logger.Debug( "[Queue] {Name}: enqueued dir={Dir} seq={Seq} (depth={Depth})", - ns.Mobile?.RawName, dir, seq, ns._movementQueue.Count + ns.Mobile, dir, seq, ns._movementQueue.Count ); } } @@ -320,7 +311,6 @@ public static class MovementThrottle { ns.SendMovementRej(seq, mobile); ns.ResetMovementState(); - _netStatesWithQueuedMovements.Remove(ns); } /// @@ -333,20 +323,18 @@ public static class MovementThrottle return; } - // Process each NetState with queued movements - // Use a snapshot to avoid modification during iteration - var toProcess = new List(_netStatesWithQueuedMovements); - - for (var i = 0; i < toProcess.Count; i++) + foreach (var ns in _netStatesWithQueuedMovements) { - var ns = toProcess[i]; - if (!ns.Running) + if (ns.Running) { - _netStatesWithQueuedMovements.Remove(ns); - continue; + ProcessMovementQueue(ns); + if (ns._hasQueuedMovements) + { + continue; + } } - ProcessMovementQueue(ns); + _netStatesWithQueuedMovements.Remove(ns); } } @@ -356,6 +344,7 @@ public static class MovementThrottle public static void ProcessMovementQueue(NetState ns) { var mobile = ns.Mobile; + if (mobile?.Deleted != false) { ClearQueue(ns); @@ -374,7 +363,7 @@ public static class MovementThrottle while (ns._movementQueue?.Count > 0) { // Check if it's time to execute - if (now < ns._nextMovementTime) + if (now - ns._nextMovementTime < 0) { // Not yet - leave remaining items in queue for next Slice break; @@ -394,11 +383,11 @@ public static class MovementThrottle // Execute the move if (!mobile.Move(movement.Direction)) { - if (_debugLogging && ns._movementLogging) + if (ns._movementLogging) { logger.Debug( "[Queue] {Name}: dequeued FAILED dir={Dir} (remaining={Remaining})", - mobile.RawName, movement.Direction, remaining + mobile, movement.Direction, remaining ); } @@ -407,12 +396,12 @@ public static class MovementThrottle return; } - if (_debugLogging && ns._movementLogging) + if (ns._movementLogging) { var waited = now - ns._nextMovementTime; logger.Debug( "[Queue] {Name}: dequeued OK dir={Dir} (remaining={Remaining}, waited={Waited}ms)", - mobile.RawName, movement.Direction, remaining, waited >= 0 ? waited : 0 + mobile, movement.Direction, remaining, waited >= 0 ? waited : 0 ); } @@ -430,10 +419,6 @@ public static class MovementThrottle // Update tracking ns._hasQueuedMovements = ns._movementQueue?.Count > 0; - if (!ns._hasQueuedMovements) - { - _netStatesWithQueuedMovements.Remove(ns); - } } /// @@ -469,7 +454,6 @@ public static class MovementThrottle { ns._movementQueue?.Clear(); ns._hasQueuedMovements = false; - _netStatesWithQueuedMovements.Remove(ns); } // Maximum expected packets per second (mounted running = 100ms = 10/sec, plus tolerance) @@ -484,7 +468,7 @@ public static class MovementThrottle logger.Information( "Movement queue overflow: {Character} ({Account}) | " + "Queue reached hard limit: {Limit} | IP: {IP}", - mobile?.RawName ?? "Unknown", + mobile, ns.Account?.Username ?? "Unknown", _hardQueueLimit, ns.Address @@ -516,7 +500,7 @@ public static class MovementThrottle private static void RecordMovement(NetState ns, long now, int cost, Direction dir, Mobile mobile) { // Calculate interval since last movement - var interval = ns._lastMovementRecordTime > 0 + var interval = ns._hasMovementRecord ? (int)(now - ns._lastMovementRecordTime) : -1; // -1 indicates first movement (no previous time) @@ -525,6 +509,7 @@ public static class MovementThrottle if (interval <= 0 || interval > _maxChainGap) { ns._lastMovementRecordTime = now; + ns._hasMovementRecord = true; // Use RTT to distinguish "stopped moving" vs "lagged" // - Stable low-latency connection with gap >> RTT → player stopped, reset history @@ -544,19 +529,19 @@ public static class MovementThrottle // A large gap followed by a burst of packets = likely lag recovery, not speed hack ns._lastGapDuration = interval; - if (_debugLogging && mobile?.RawName != null) + if (ns._movementLogging) { var action = shouldReset ? "history reset" : "history preserved (possible lag)"; logger.Debug( "[Movement] {Name}: SKIP recording (gap {Gap}ms > {MaxGap}ms, " + "RTT={RTT}ms stable={Stable} → {Action})", - mobile.RawName, interval, _maxChainGap, avgRtt, ns.HasStableConnection, action + mobile, interval, _maxChainGap, avgRtt, ns.HasStableConnection, action ); } } - else if (_debugLogging && mobile?.RawName != null) + else if (ns._movementLogging) { - logger.Debug("[Movement] {Name}: SKIP recording (first in chain)", mobile.RawName); + logger.Debug("[Movement] {Name}: SKIP recording (first in chain)", mobile); } return; @@ -572,12 +557,9 @@ public static class MovementThrottle // the next real move's interval artificially short, inflating rate. if (cost == 0) { - if (_debugLogging && mobile?.RawName != null) + if (ns._movementLogging) { - logger.Debug( - "[Movement] {Name}: SKIP direction-only change (preserves interval measurement)", - mobile.RawName - ); + logger.Debug("[Movement] {Name}: SKIP direction-only change (preserves interval measurement)", mobile); } return; } @@ -613,15 +595,16 @@ public static class MovementThrottle } ns._lastMovementRecordTime = now; + ns._hasMovementRecord = true; // Debug logging - if (_debugLogging && mobile?.RawName != null) + if (ns._movementLogging) { var historyCount = ns._movementHistoryFull ? _movementHistorySize : ns._movementHistoryIndex; logger.Debug( "[Movement] {Name}: interval={Interval}ms target={Target}ms queue={Queue} " + "flags={Flags} history={History}/{MaxHistory} RTT={RTT}ms", - mobile.RawName, interval, cost, record.QueueDepth, + mobile, interval, cost, record.QueueDepth, flags, historyCount, _movementHistorySize, ns.AverageRtt ); } @@ -814,7 +797,7 @@ public static class MovementThrottle var averageRtt = ns.AverageRtt; // Detailed rate breakdown for debugging - if (_debugLogging) + if (ns._movementLogging) { logger.Debug("[MovementAnalysis] Rate={Rate:F3}, Samples={Samples}, RTT={RTT}ms", rate, sampleCount, averageRtt); @@ -977,19 +960,19 @@ public static class MovementThrottle var verdict = AnalyzeMovement(ns, out var rate, out var sampleCount, out var confidence); // Debug logging - if (_debugLogging && ns.Mobile?.RawName != null) + if (ns._movementLogging) { var (burstSize, _) = DetectRecentBurst(ns); - var probeStatus = ns._rttProbeTime > 0 ? "pending" : "idle"; + var probeStatus = ns._rttProbePending ? "pending" : "idle"; var queueDepth = ns._movementQueue?.Count ?? 0; logger.Debug( "[RateCheck] {Name}: rate={Rate:F3} samples={Samples} verdict={Verdict} " + "confidence={Confidence:P0} queue={Queue} burst={Burst} sustained={Sustained}s", - ns.Mobile.RawName, rate, sampleCount, verdict, confidence, queueDepth, burstSize, ns._consecutiveHighRateSeconds + ns.Mobile, rate, sampleCount, verdict, confidence, queueDepth, burstSize, ns._consecutiveHighRateSeconds ); logger.Debug( " RTT: avg={Avg}ms last={Last}ms var={Var} samples={RttSamples} stable={Stable} probe={Probe}", - ns.AverageRtt, ns._lastRtt, ns._rttVariance, ns._rttSampleCount, ns.HasStableConnection, probeStatus + ns.AverageRtt, ns.LastRtt, ns._rttVariance, ns._rttSampleCount, ns.HasStableConnection, probeStatus ); } @@ -1025,11 +1008,11 @@ public static class MovementThrottle if (shouldNotify) { - if (_debugLogging) + if (ns._movementLogging) { logger.Debug( "[ALERT] {Urgency} - {Name}: rate={Rate:F3} verdict={Verdict} confidence={Confidence:P0}", - urgency, ns.Mobile?.RawName, rate, verdict, confidence + urgency, ns.Mobile, rate, verdict, confidence ); } NotifyStaff(ns, rate, sampleCount, confidence, verdict, urgency); @@ -1054,11 +1037,12 @@ public static class MovementThrottle var now = Core.TickCount; // Rate-limit notifications per player - if (now - ns._lastSpeedHackNotification < _speedHackNotificationCooldown) + if (ns._speedHackNotified && now - ns._lastSpeedHackNotification < _speedHackNotificationCooldown) { return; } + ns._speedHackNotified = true; ns._lastSpeedHackNotification = now; var mobile = ns.Mobile; @@ -1070,7 +1054,7 @@ public static class MovementThrottle "PacketRate: {PacketRate}/s (peak: {PeakRate}/s) | RTT: {Rtt}ms (stable: {Stable}) | " + "Sustained: {Sustained}s | Queue: {Queue} | Location: {Location} Map: {Map} | IP: {IP}", urgency, - mobile?.RawName ?? "Unknown", + mobile, ns.Account?.Username ?? "Unknown", rate, sampleCount, @@ -1138,7 +1122,7 @@ public static class MovementThrottle Verdict = verdict, Confidence = confidence, AverageRtt = ns.AverageRtt, - LastRtt = ns._lastRtt, + LastRtt = ns.LastRtt, RttVariance = ns._rttVariance, StableConnection = ns.HasStableConnection, RttSampleCount = ns._rttSampleCount, diff --git a/Projects/Server/Network/NetState/NetState.Movement.cs b/Projects/Server/Network/NetState/NetState.Movement.cs index 1cc79429e..c6f28fcd1 100644 --- a/Projects/Server/Network/NetState/NetState.Movement.cs +++ b/Projects/Server/Network/NetState/NetState.Movement.cs @@ -15,7 +15,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Runtime.InteropServices; using Server.Logging; @@ -70,23 +69,24 @@ public partial class NetState internal Queue _movementQueue; // Lazy initialized internal long _movementCredit; // Credit buffer for timing jitter internal long _nextMovementTime = Core.TickCount; // When next movement is allowed - internal int _sustainedQueueDepth; // Tracks sustained high queue depth - internal long _lastQueueDepthCheck; // Throttle depth check frequency + internal long _lastQueueDepthCheck = Core.TickCount; // Throttle depth check frequency internal bool _hasQueuedMovements; // Fast check for Slice() // Movement history for rate-based speed hack detection (lazy initialized) internal MovementRecord[] _movementHistory; // Circular buffer internal int _movementHistoryIndex; // Next write position (also serves as count until full) internal bool _movementHistoryFull; // True once buffer has wrapped - internal long _lastMovementRecordTime; // For calculating intervals + internal long _lastMovementRecordTime; // For calculating intervals (valid only when _hasMovementRecord) + internal bool _hasMovementRecord; // False until the first movement in a chain is seen // Detection state internal int _consecutiveHighRateSeconds; // Sustained detection counter - internal long _lastSpeedHackNotification; // Rate-limit notifications + internal long _lastSpeedHackNotification; // Rate-limit notifications (valid only when _speedHackNotified) + internal bool _speedHackNotified; // False until the first notification is sent internal int _lastGapDuration; // Duration of last gap > maxChainGap (for burst forgiveness) // Movement packet rate tracking (for speed hack detection) - internal long _movementWindowStart; // Start of current 1-second window + internal long _movementWindowStart = Core.TickCount; // Start of current 1-second window internal int _movementsInWindow; // Count in current window internal int _peakMovementRate; // Highest rate seen (packets/sec) @@ -100,10 +100,9 @@ public partial class NetState _nextMovementTime = Core.TickCount; _movementCredit = 0; _hasQueuedMovements = false; - _sustainedQueueDepth = 0; // Reset movement history - next movement starts a new chain - _lastMovementRecordTime = 0; + _hasMovementRecord = false; _movementHistoryIndex = 0; _movementHistoryFull = false; @@ -113,7 +112,7 @@ public partial class NetState _rttProbeInterval = RttProbeIntervalNormal; // Reset packet rate window - _movementWindowStart = 0; + _movementWindowStart = Core.TickCount; _movementsInWindow = 0; } @@ -165,17 +164,19 @@ public partial class NetState private const long MaxStableLatency = 200; // Max RTT (ms) for "stable" connection // RTT state - internal long _rttProbeTime; // When we sent the probe (0 = not waiting) - internal long _lastRtt; // Most recent RTT measurement + internal bool _rttProbePending; // True while waiting for a probe response + internal long _rttProbeTime; // When we sent the probe (valid only when _rttProbePending) internal long[] _rttHistory; // Rolling history (lazy init) internal int _rttHistoryIndex; // Current position in history internal int _rttSampleCount; // Number of samples collected (saturates at RttHistorySize) internal long _rttVariance; // Calculated variance for stability - internal long _nextRttProbe; // When to send next probe + internal long _nextRttProbe = Core.TickCount; // When to send next probe internal int _rttProbeInterval = RttProbeIntervalNormal; // Current probe interval - // High-resolution timestamp for RTT measurement (Stopwatch ticks, not game loop ticks) - private long _rttProbeTimestampHiRes; + /// + /// Gets the most recent RTT measurement, or 0 if none has been recorded. + /// + public long LastRtt => _rttSampleCount > 0 ? _rttHistory[(_rttHistoryIndex - 1) & (RttHistorySize - 1)] : 0; /// /// Sets the RTT probe interval based on suspicion level. @@ -206,23 +207,22 @@ public partial class NetState var now = Core.TickCount; // Don't send if we're still waiting for a response - if (_rttProbeTime > 0) + if (_rttProbePending) { // Timeout after 10 seconds - connection is probably dead or very laggy if (now - _rttProbeTime > 10000) { - _rttProbeTime = 0; - _rttProbeTimestampHiRes = 0; + _rttProbePending = false; } return; } // First probe: send immediately when player starts moving // Subsequent probes: send when interval has passed - if (_nextRttProbe == 0 || now >= _nextRttProbe) + if (now - _nextRttProbe >= 0) { + _rttProbePending = true; _rttProbeTime = now; - _rttProbeTimestampHiRes = Stopwatch.GetTimestamp(); _nextRttProbe = now + _rttProbeInterval + Utility.Random(RttProbeJitter); if (_movementLogging) @@ -242,10 +242,9 @@ public partial class NetState /// public void RecordRttMeasurement() { - var nowHiRes = Stopwatch.GetTimestamp(); var now = Core.TickCount; - if (_rttProbeTime <= 0) + if (!_rttProbePending) { // Not expecting a response (client-initiated version send) - ignore silently return; @@ -253,19 +252,15 @@ public partial class NetState var rtt = now - _rttProbeTime; - // High-resolution RTT in microseconds - var rttHiResUs = (nowHiRes - _rttProbeTimestampHiRes) * 1_000_000 / Stopwatch.Frequency; - if (_movementLogging) { movementLogger.Debug( - "[RTT-Response] {Account}: {Rtt}ms (HiRes: {RttHiRes:F2}ms)", - Account?.Username ?? _toString, rtt, rttHiResUs / 1000.0 + "[RTT-Response] {Account}: {Rtt}ms", + Account?.Username ?? _toString, rtt ); } - _rttProbeTime = 0; - _rttProbeTimestampHiRes = 0; + _rttProbePending = false; // Sanity check - RTT should be positive and reasonable if (rtt is <= 0 or > 10000) @@ -285,7 +280,6 @@ public partial class NetState // Update history _rttHistory[_rttHistoryIndex++ & (RttHistorySize - 1)] = rtt; - _lastRtt = rtt; // Track sample count (saturates at buffer size) if (_rttSampleCount < RttHistorySize) diff --git a/Projects/Server/Network/NetState/NetState.cs b/Projects/Server/Network/NetState/NetState.cs index 7312603f7..04f1a5a76 100755 --- a/Projects/Server/Network/NetState/NetState.cs +++ b/Projects/Server/Network/NetState/NetState.cs @@ -44,7 +44,7 @@ public partial class NetState : IComparable, IValueLinkListNode _connectingQueue = new(2048); private static readonly HashSet _instances = new(2048); - public static IReadOnlySet Instances => _instances; + public static HashSet Instances => _instances; private readonly string _toString; private ClientVersion _version; @@ -109,9 +109,6 @@ public partial class NetState : IComparable, IValueLinkListNode, IValueLinkListNode Trades { get; } + public List Trades { get; private set; } public bool Seeded { get; set; } @@ -260,8 +257,18 @@ public partial class NetState : IComparable, IValueLinkListNode= 0; --i) { + if (Trades == null) + { + break; + } + if (i >= Trades.Count) { continue; @@ -280,8 +287,18 @@ public partial class NetState : IComparable, IValueLinkListNode= 0; --i) { + if (Trades != null) + { + break; + } + if (i < Trades.Count) { Trades[i].Cancel(); @@ -291,11 +308,21 @@ public partial class NetState : IComparable, IValueLinkListNode, IValueLinkListNode, IValueLinkListNode, IValueLinkListNode Date: Tue, 1 Sep 2026 20:42:15 -0700 Subject: [PATCH 6/9] feat: event-driven target acquisition with a reaction-time gradient (#2601) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes walk-up aggro latency (up to a full 10 s of obliviousness) and hardens the reacquire gate so no state can silence acquisition, while turning `AcquireOnApproach` into the reaction-time knob for future per-creature intelligence tuning. ### Why `AcquireFocusMob` re-armed the 10 s `ReacquireDelay` **before** scanning, success or failure. A creature that scanned an empty room was blind for 10 s to a player walking up — walk-up aggro latency was uniform in 0..10 s. Waking from sector sleep stacked the AI timer's 0–3 s construction stagger on top. And `NextReacquireTime` is not serialized: on hosts whose tick counter starts negative (GCP pass-through), the 0 default blocked **all** acquisition shard-wide after a restart until the counter crossed zero. ### What **Event-driven reaction — `AcquireOnApproachDelay` (the intelligence gradient)** - The paragon `AcquireOnApproach` bool becomes a `TimeSpan` on every creature: an enemy moving inside `AcquireOnApproachRange` (10 for all creatures — on-screen reactive aggro; the periodic scan keeps the wide `RangePerception` sweep) *clamps* the next scan to at most the delay. Repeated steps cannot shorten it further — one scan per delay period, not per step or think. - `Zero` (paragons) also prods the AI timer: the ranked scan engages within a wheel turn — the old snap, minus the special-cased engage path. The target now comes from the normal FightMode ranking instead of whichever mobile happened to move, and the `Combatant == null` guard stops re-engage spam. - The 2 s default reads as "took a beat to notice you"; larger values are dumber; `ReacquireDelay` alone is the oblivious floor. Mover checks are the approach logic's `IsEnemy` + `CanBeHarmful` (so pets count and hidden movers are excluded via `CanSee`), with `IsEnemy` first to cheaply reject same-team wild creatures wandering past. The check rides the `OnMovement` callback every step already pays for — no polling added. **Gate correctness** - Every scan re-arms the full `ReacquireDelay`, success or failure (classic semantics; reaction time is the approach path, not the poll). - Self-healing by construction: a deadline further out than `ReacquireDelay` is an illegal state and reads as open — no wedged or wrapped value can silence acquisition beyond one delay period. - `NextReacquireTime` is seeded from a live tick on deserialize (the GCP negative-tick blackout). **AI timer wake** - Activation (sector wake, spawn, resurrection) starts within a 0–256 ms spread instead of the 0–3 s construction stagger, which read as lag. - The stagger's real job — keeping same-speed cohorts out of lock-step (the RunUO town artifact) — is now a zero-mean ±period/8 jitter on each **idle** think, so phases random-walk apart within seconds and can never re-lock. Instrumentation showed why a one-shot spread can't do this job: the timer wheel fires within ±1 ms, so with 10 creatures on a 500 ms period some pair collides on nearly the same phase ~75% of the time (birthday paradox) and then steps in the same loop iteration *forever*. Jitter is scoped to passive speed: engaged cadence stays exact, since pursuit timing anchors to real step times. **Debug** - The `AcquireFocusMob` scan message no longer re-arms the shared 5 s debug cooldown, which swallowed every AI's "I have detected X" transition line. **API change** for custom scripts: `AcquireOnApproach` (bool) → `AcquireOnApproachDelay` (TimeSpan). Documented in `content-patterns.md` § Target Acquisition, `runuo-migration-docs/09` + `11`, and the migration skill checklist. ### Tests `AcquisitionTests`: both scan outcomes honor `ReacquireDelay`; a 60 s-wedged gate still acquires; enemy movement clamps the deadline (same-team wild movers and out-of-range movers ignored); repeated movement cannot shorten below the delay; `Zero` opens the gate and prods without a direct engage. Full suite: 755 UOContent green. --- .../Tests/Mobiles/AI/AcquisitionTests.cs | 211 ++++++++++++++++++ Projects/UOContent/Mobiles/AI/ArcherAI.cs | 2 +- .../UOContent/Mobiles/AI/BaseAI/AITimer.cs | 23 +- .../UOContent/Mobiles/AI/BaseAI/BaseAI.cs | 24 +- Projects/UOContent/Mobiles/AI/BerserkAI.cs | 2 +- Projects/UOContent/Mobiles/AI/MeleeAI.cs | 16 +- Projects/UOContent/Mobiles/BaseCreature.cs | 61 +++-- .../migrate-items-mobiles.md | 1 + .../modernuo-content-patterns.md | 4 +- dev-docs/content-patterns.md | 18 ++ .../09-items-mobiles-creatures.md | 20 ++ .../runuo-migration-docs/11-api-reference.md | 1 + 12 files changed, 340 insertions(+), 43 deletions(-) create mode 100644 Projects/UOContent.Tests/Tests/Mobiles/AI/AcquisitionTests.cs diff --git a/Projects/UOContent.Tests/Tests/Mobiles/AI/AcquisitionTests.cs b/Projects/UOContent.Tests/Tests/Mobiles/AI/AcquisitionTests.cs new file mode 100644 index 000000000..3ee516dcf --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Mobiles/AI/AcquisitionTests.cs @@ -0,0 +1,211 @@ +using System; +using System.Collections.Generic; +using Server; +using Server.Mobiles; +using Xunit; + +namespace UOContent.Tests.Mobiles.AI; + +// Pins the reacquire gate and the AcquireOnApproachDelay gradient: every scan re-arms the +// full ReacquireDelay; enemy movement clamps the deadline to the approach delay (Zero = +// prodded scan); an illegal deadline self-heals. +[Collection("Sequential Pathfinding Tests")] +public class AcquisitionTests : IDisposable +{ + private readonly List _created = new(); + + public void Dispose() + { + foreach (var m in _created) + { + m?.Delete(); + } + + _created.Clear(); + } + + private sealed class WildStub : BaseCreature + { + public WildStub() : base(AIType.AI_Melee, FightMode.Closest, 16, 1) => Body = 0xC9; + + public override void GetSpeeds(out double activeSpeed, out double passiveSpeed) + { + activeSpeed = 0.3; + passiveSpeed = 0.6; + } + } + + private sealed class TargetStub : Mobile + { + public TargetStub() => Body = 0x190; + } + + private WildStub Spawn(Map map, Point3D loc) + { + var bc = new WildStub(); + bc.MoveToWorld(loc, map); + bc.AIObject.AITimer?.Stop(); + _created.Add(bc); + return bc; + } + + [Fact] + public void EmptyScan_HonorsReacquireDelay() + { + var map = Map.Maps[1]; + Assert.NotNull(map); + map.GetAverageZ(1500, 1600, out _, out var z, out _); + + var bc = Spawn(map, new Point3D(1500, 1600, (sbyte)z)); + bc.NextReacquireTime = Core.TickCount; + + Assert.False(bc.AIObject.AcquireFocusMob(bc.RangePerception, FightMode.Closest, false, false, true)); + Assert.InRange(bc.NextReacquireTime - Core.TickCount, 5000, 10000); + } + + [Fact] + public void WedgedGate_SelfHeals() + { + var map = Map.Maps[1]; + Assert.NotNull(map); + map.GetAverageZ(1500, 1600, out _, out var z, out _); + + var bc = Spawn(map, new Point3D(1500, 1600, (sbyte)z)); + + var target = new TargetStub(); + target.DefaultMobileInit(); + target.MoveToWorld(new Point3D(1497, 1600, (sbyte)z), map); + _created.Add(target); + + // Illegal deadline (beyond ReacquireDelay): must read as open, not block forever. + bc.NextReacquireTime = Core.TickCount + 60000; + + Assert.True(bc.AIObject.AcquireFocusMob(bc.RangePerception, FightMode.Closest, false, false, true)); + Assert.Equal(target, bc.FocusMob); + } + + [Theory] + [InlineData(false, 5, true)] // an enemy moving inside approach range (10) clamps the deadline + [InlineData(true, 5, false)] // a same-team wild creature is not an enemy — ignored + [InlineData(false, 12, false)] // inside RangePerception but outside approach range — poll only + [InlineData(false, 20, false)] // outside approach range (10) is ignored + public void MovementClampsScanDeadlineOnlyForEnemiesInRange(bool wildMover, int distance, bool notices) + { + var map = Map.Maps[1]; + Assert.NotNull(map); + map.GetAverageZ(1500, 1600, out _, out var z, out _); + + var bc = Spawn(map, new Point3D(1500, 1600, (sbyte)z)); + bc.NextReacquireTime = Core.TickCount + 8000; + + Mobile mover; + if (wildMover) + { + mover = Spawn(map, new Point3D(1500 - distance, 1600, (sbyte)z)); + } + else + { + mover = new TargetStub { Player = true }; + mover.DefaultMobileInit(); + mover.MoveToWorld(new Point3D(1500 - distance, 1600, (sbyte)z), map); + _created.Add(mover); + } + + bc.OnMovement(mover, new Point3D(1400, 1600, (sbyte)z)); + + var remaining = bc.NextReacquireTime - Core.TickCount; + + if (notices) + { + // Clamped to the approach delay (2s), never opened outright. + Assert.InRange(remaining, 1, (long)bc.AcquireOnApproachDelay.TotalMilliseconds); + } + else + { + Assert.True(remaining > 5000); + } + } + + private sealed class InstantStub : BaseCreature + { + public InstantStub() : base(AIType.AI_Melee, FightMode.Closest, 16, 1) => Body = 0xC9; + + public override TimeSpan AcquireOnApproachDelay => TimeSpan.Zero; + + public override void GetSpeeds(out double activeSpeed, out double passiveSpeed) + { + activeSpeed = 0.3; + passiveSpeed = 0.6; + } + } + + [Fact] + public void ZeroApproachDelay_OpensGateImmediately() + { + var map = Map.Maps[1]; + Assert.NotNull(map); + map.GetAverageZ(1500, 1600, out _, out var z, out _); + + var bc = new InstantStub(); + bc.MoveToWorld(new Point3D(1500, 1600, (sbyte)z), map); + bc.AIObject.AITimer?.Stop(); + _created.Add(bc); + bc.NextReacquireTime = Core.TickCount + 8000; + + var mover = new TargetStub { Player = true }; + mover.DefaultMobileInit(); + mover.MoveToWorld(new Point3D(1495, 1600, (sbyte)z), map); + _created.Add(mover); + + bc.OnMovement(mover, new Point3D(1400, 1600, (sbyte)z)); + + // Zero = the gate opens and the AI is prodded to think now; no direct engage. + Assert.True(Core.TickCount - bc.NextReacquireTime >= 0); + Assert.Null(bc.Combatant); + Assert.True(bc.AIObject.AITimer.Running); + } + + [Fact] + public void RepeatedMovement_DoesNotShortenBelowApproachDelay() + { + var map = Map.Maps[1]; + Assert.NotNull(map); + map.GetAverageZ(1500, 1600, out _, out var z, out _); + + var bc = Spawn(map, new Point3D(1500, 1600, (sbyte)z)); + bc.NextReacquireTime = Core.TickCount + 8000; + + var mover = new TargetStub { Player = true }; + mover.DefaultMobileInit(); + mover.MoveToWorld(new Point3D(1495, 1600, (sbyte)z), map); + _created.Add(mover); + + bc.OnMovement(mover, new Point3D(1400, 1600, (sbyte)z)); + var afterFirst = bc.NextReacquireTime; + + bc.OnMovement(mover, new Point3D(1496, 1600, (sbyte)z)); + + Assert.Equal(afterFirst, bc.NextReacquireTime); + } + + [Fact] + public void SuccessfulAcquire_HoldsFullDelay() + { + var map = Map.Maps[1]; + Assert.NotNull(map); + map.GetAverageZ(1500, 1600, out _, out var z, out _); + + var bc = Spawn(map, new Point3D(1500, 1600, (sbyte)z)); + + var target = new TargetStub(); + target.DefaultMobileInit(); + target.MoveToWorld(new Point3D(1497, 1600, (sbyte)z), map); + _created.Add(target); + + bc.NextReacquireTime = Core.TickCount; + + Assert.True(bc.AIObject.AcquireFocusMob(bc.RangePerception, FightMode.Closest, false, false, true)); + Assert.Equal(target, bc.FocusMob); + Assert.True(bc.NextReacquireTime - Core.TickCount > 5000); + } +} diff --git a/Projects/UOContent/Mobiles/AI/ArcherAI.cs b/Projects/UOContent/Mobiles/AI/ArcherAI.cs index 23ce0549c..803587ceb 100644 --- a/Projects/UOContent/Mobiles/AI/ArcherAI.cs +++ b/Projects/UOContent/Mobiles/AI/ArcherAI.cs @@ -17,7 +17,7 @@ public class ArcherAI : BaseAI if (AcquireFocusMob(Mobile.RangePerception, Mobile.FightMode, false, false, true)) { - this.DebugSayFormatted($"I have detected {Mobile.FocusMob.Name} and I will attack"); + this.DebugSayFormatted($"I have detected {Mobile.FocusMob.Name}, attacking"); Mobile.Combatant = Mobile.FocusMob; Action = ActionType.Combat; diff --git a/Projects/UOContent/Mobiles/AI/BaseAI/AITimer.cs b/Projects/UOContent/Mobiles/AI/BaseAI/AITimer.cs index d5a84f94d..e11268e87 100644 --- a/Projects/UOContent/Mobiles/AI/BaseAI/AITimer.cs +++ b/Projects/UOContent/Mobiles/AI/BaseAI/AITimer.cs @@ -31,8 +31,8 @@ public sealed class AITimer : Timer private int _detectHiddenMinDelay; private int _detectHiddenMaxDelay; - public AITimer(BaseAI owner) : base(TimeSpan.FromMilliseconds(Utility.Random(3000)), - TimeSpan.FromSeconds(owner.Mobile.CurrentSpeed)) + // The initial delay is irrelevant: Activate is the only start path and sets its own. + public AITimer(BaseAI owner) : base(TimeSpan.Zero, TimeSpan.FromSeconds(owner.Mobile.CurrentSpeed)) { _owner = owner; _owner._nextDetectHidden = Core.TickCount; @@ -48,7 +48,11 @@ public sealed class AITimer : Timer return; } - Start(); // keeps the stagger Delay + // Short random spread: the creature responds within a think while a sector's + // worth of timers avoids a same-tick burst; the idle think jitter keeps the + // cohort apart from there. + Delay = TimeSpan.FromMilliseconds(Utility.Random(256)); + Start(); _nextWake = Core.TickCount + (long)Delay.TotalMilliseconds; } @@ -148,7 +152,18 @@ public sealed class AITimer : Timer } // Cadence from the post-decision speed (decisions may flip active/passive). - _nextThink = Core.TickCount + (long)(_owner.Mobile.CurrentSpeed * 1000); + var period = (long)(_owner.Mobile.CurrentSpeed * 1000); + _nextThink = Core.TickCount + period; + + // Idle cadence drifts: a zero-mean jitter random-walks think phases apart, so + // creatures spawned or woken together cannot stay in lock-step (a one-shot + // spread can collide and identical periods never separate). Engaged cadence + // stays exact — pursuit timing anchors to real step times. + if (_owner.Mobile.CurrentSpeed == _owner.Mobile.PassiveSpeed) + { + var jitter = (int)(period >> 3); + _nextThink += Utility.RandomMinMax(-jitter, jitter); + } } else { diff --git a/Projects/UOContent/Mobiles/AI/BaseAI/BaseAI.cs b/Projects/UOContent/Mobiles/AI/BaseAI/BaseAI.cs index 3c2a479e0..4b8753d10 100644 --- a/Projects/UOContent/Mobiles/AI/BaseAI/BaseAI.cs +++ b/Projects/UOContent/Mobiles/AI/BaseAI/BaseAI.cs @@ -850,22 +850,24 @@ public abstract partial class BaseAI return false; } - if (Core.TickCount - Mobile.NextReacquireTime < 0) + var reacquireDelay = (long)Mobile.ReacquireDelay.TotalMilliseconds; + var gateRemaining = Mobile.NextReacquireTime - Core.TickCount; + + if (gateRemaining > 0 && gateRemaining <= reacquireDelay) { Mobile.FocusMob = null; return false; } - Mobile.NextReacquireTime = Core.TickCount + (int)Mobile.ReacquireDelay.TotalMilliseconds; + DebugSay("Acquiring new target...", 0); - DebugSay("Acquiring new target..."); + var acquired = AcquireNewFocusMob(Mobile.Map, iRange, acqType, bPlayerOnly, bFacFriend, bFacFoe); - if (Mobile.Map == null) - { - return Mobile.FocusMob != null; - } + // Reaction time is the approach path (BaseCreature.ScheduleAcquireOnApproach), + // not this poll — every scan honors the full delay. + Mobile.NextReacquireTime = Core.TickCount + reacquireDelay; - return AcquireNewFocusMob(Mobile.Map, iRange, acqType, bPlayerOnly, bFacFriend, bFacFoe); + return acquired; } private bool HandleBardProvoked() @@ -941,8 +943,10 @@ public abstract partial class BaseAI private bool AcquireNewFocusMob(Map map, int iRange, FightMode acqType, bool bPlayerOnly, bool bFacFriend, bool bFacFoe) { - Mobile newFocusMob = null, enemySummonMob = null; - double val = double.MinValue, enemySummonVal = double.MinValue; + Mobile newFocusMob = null; + Mobile enemySummonMob = null; + var val = double.MinValue; + var enemySummonVal = double.MinValue; foreach (var m in map.GetMobilesInRange(Mobile.Location, iRange)) { diff --git a/Projects/UOContent/Mobiles/AI/BerserkAI.cs b/Projects/UOContent/Mobiles/AI/BerserkAI.cs index ff00ec91d..8a2015f1a 100644 --- a/Projects/UOContent/Mobiles/AI/BerserkAI.cs +++ b/Projects/UOContent/Mobiles/AI/BerserkAI.cs @@ -12,7 +12,7 @@ public class BerserkAI : BaseAI if (AcquireFocusMob(Mobile.RangePerception, FightMode.Closest, false, true, true)) { - this.DebugSayFormatted($"I have detected {Mobile.FocusMob.Name} and I will attack"); + this.DebugSayFormatted($"I have detected {Mobile.FocusMob.Name}, attacking"); Mobile.Combatant = Mobile.FocusMob; Action = ActionType.Combat; diff --git a/Projects/UOContent/Mobiles/AI/MeleeAI.cs b/Projects/UOContent/Mobiles/AI/MeleeAI.cs index a71d83d5c..a6d0a9bf9 100644 --- a/Projects/UOContent/Mobiles/AI/MeleeAI.cs +++ b/Projects/UOContent/Mobiles/AI/MeleeAI.cs @@ -1,3 +1,5 @@ +using System.Runtime.CompilerServices; + namespace Server.Mobiles; public class MeleeAI : BaseAI @@ -14,6 +16,7 @@ public class MeleeAI : BaseAI if (AcquireFocusMob(Mobile.RangePerception, Mobile.FightMode, false, false, true)) { this.DebugSayFormatted($"I have detected {Mobile.FocusMob.Name}, attacking"); + Mobile.Combatant = Mobile.FocusMob; Action = ActionType.Combat; } @@ -65,13 +68,9 @@ public class MeleeAI : BaseAI return true; } - private bool IsValidCombatant(Mobile combatant) - { - return combatant?.Deleted == false - && combatant.Map == Mobile.Map - && combatant.Alive - && !combatant.IsDeadBondedPet; - } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private bool IsValidCombatant(Mobile combatant) => + combatant?.Deleted == false && combatant.Map == Mobile.Map && combatant.Alive && !combatant.IsDeadBondedPet; private bool HandleOutOfRangeCombatant(Mobile combatant) { @@ -127,7 +126,8 @@ public class MeleeAI : BaseAI { if (AcquireFocusMob(Mobile.RangePerception, Mobile.FightMode, false, false, true)) { - this.DebugSayFormatted($"I have detected {Mobile.FocusMob.Name}, attacking."); + this.DebugSayFormatted($"I have detected {Mobile.FocusMob.Name}, attacking"); + Mobile.Combatant = Mobile.FocusMob; Action = ActionType.Combat; } diff --git a/Projects/UOContent/Mobiles/BaseCreature.cs b/Projects/UOContent/Mobiles/BaseCreature.cs index 05c728cf4..581d472e3 100644 --- a/Projects/UOContent/Mobiles/BaseCreature.cs +++ b/Projects/UOContent/Mobiles/BaseCreature.cs @@ -936,21 +936,50 @@ namespace Server.Mobiles public virtual bool GivesMLMinorArtifact => false; - /* To save on cpu usage, RunUO creatures only reacquire creatures under the following circumstances: - * - 10 seconds have elapsed since the last time it tried - * - The creature was attacked - * - Some creatures, like dragons, will reacquire when they see someone move - * - * This functionality appears to be implemented on OSI as well - */ - public long NextReacquireTime { get; set; } public virtual TimeSpan ReacquireDelay => TimeSpan.FromSeconds(10.0); - public virtual bool ReacquireOnMovement => false; - public virtual bool AcquireOnApproach => m_Paragon; + + // Reaction-time gradient: an enemy moving inside AcquireOnApproachRange pulls the + // next scan to at most this far away. Zero (paragons) scans on the very next + // think; larger is dumber; pure ReacquireDelay is the oblivious floor. + public virtual TimeSpan AcquireOnApproachDelay => m_Paragon ? TimeSpan.Zero : TimeSpan.FromSeconds(2.0); + + // Reactive range is tighter than the periodic scan's RangePerception: approach + // aggro starts on-screen; the ReacquireDelay poll keeps the wide ambient sweep. public virtual int AcquireOnApproachRange => 10; + // Clamps the scan deadline rather than opening the gate: repeated steps cannot + // shorten it further, so an armed creature scans once per delay period. + private void ScheduleAcquireOnApproach() + { + var delay = (long)AcquireOnApproachDelay.TotalMilliseconds; + var deadline = Core.TickCount + delay; + + if (deadline - NextReacquireTime < 0) + { + NextReacquireTime = deadline; + } + + if (delay <= 0) + { + // Zero: think now — the ranked scan engages within a wheel turn. Prod is + // spam-safe; the Combatant == null guard stops the prods once engaged. + AIObject?.AITimer?.Prod(); + } + } + + // IsEnemy first — it cheaply rejects the common case (a same-team wild creature + // wandering past); CanBeHarmful covers hidden movers via CanSee. + private bool ShouldAcquireOnApproach(Mobile m) => + Combatant == null && + !Controlled && !Summoned && !BardPacified && + FightMode != FightMode.None && FightMode != FightMode.Aggressor && + InRange(m.Location, AcquireOnApproachRange) && + IsEnemy(m) && CanBeHarmful(m, false); + + public virtual bool ReacquireOnMovement => false; + public static bool Summoning { get; set; } public virtual bool IsDispellable => Summoned && !IsAnimatedDead; @@ -2024,6 +2053,8 @@ namespace Server.Mobiles { base.Deserialize(reader); + NextReacquireTime = Core.TickCount; + var version = reader.ReadInt(); m_CurrentAI = (AIType)reader.ReadInt(); @@ -2855,15 +2886,9 @@ namespace Server.Mobiles public override void OnMovement(Mobile m, Point3D oldLocation) { - if (AcquireOnApproach && !Controlled && !Summoned && !BardPacified && FightMode != FightMode.Aggressor) + if (ShouldAcquireOnApproach(m)) { - if (InRange(m.Location, AcquireOnApproachRange) && !InRange(oldLocation, AcquireOnApproachRange) && - CanBeHarmful(m) && IsEnemy(m)) - { - Combatant = FocusMob = m; - AIObject?.MoveTo(m, 1); - DoHarmful(m); - } + ScheduleAcquireOnApproach(); } else if (ReacquireOnMovement) { diff --git a/dev-docs/claude-skills/migrate-from-runuo/migrate-items-mobiles.md b/dev-docs/claude-skills/migrate-from-runuo/migrate-items-mobiles.md index ce70ad26c..f12f92c55 100644 --- a/dev-docs/claude-skills/migrate-from-runuo/migrate-items-mobiles.md +++ b/dev-docs/claude-skills/migrate-from-runuo/migrate-items-mobiles.md @@ -30,6 +30,7 @@ description: > - `Name = "text"` -> `public override string DefaultName => "text";` - Expression-bodied overrides: `public override int Meat { get { return 1; } }` -> `public override int Meat => 1;` - AI movement calls lose the `run` flag: `MoveTo(m, true, range)` -> `MoveTo(m, range)` (also `WalkMobileRange`, `ApproachTarget`, `MoveToPoint`, `PathFollower.Follow`); the Running bit is derived from step pace -> `dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md` § AI Movement +- `AcquireOnApproach` (bool) -> `AcquireOnApproachDelay` (TimeSpan; `Zero` = old instant behavior) -> same doc § Target Acquisition ## Anti-Patterns - Using `_field--` instead of `Property--` (bypasses MarkDirty tracking) diff --git a/dev-docs/claude-skills/modernuo-content-patterns.md b/dev-docs/claude-skills/modernuo-content-patterns.md index 2b6df7d55..6e0902b64 100644 --- a/dev-docs/claude-skills/modernuo-content-patterns.md +++ b/dev-docs/claude-skills/modernuo-content-patterns.md @@ -29,7 +29,9 @@ description: > overridden). Prefer `npc-speeds.json` buckets (`SpeedClass`); `SetSpeed()` sets think AND clears move overrides, `SetMoveSpeed()` sets move only. The client `Running` bit is derived from the step pace (`BaseAI.ShouldRun`); movement APIs take no run argument -- - see `dev-docs/content-patterns.md` § Creature Speeds + see `dev-docs/content-patterns.md` § Creature Speeds. Reaction time to approaching + enemies is `AcquireOnApproachDelay` (TimeSpan gradient; `Zero` = paragon snap, 2s + default, `ReacquireDelay`-only = oblivious) -- see § Target Acquisition 8. **`OnThink` overrides must be excess-call tolerant** -- it fires more often than the think cadence (player commands prod it; speed-ups reschedule it). Gate consequential work on a tick-count deadline (subtraction form) or make it idempotent; bare per-call diff --git a/dev-docs/content-patterns.md b/dev-docs/content-patterns.md index 3d11efb16..6205ec2d4 100644 --- a/dev-docs/content-patterns.md +++ b/dev-docs/content-patterns.md @@ -295,6 +295,24 @@ flood the client's step queue. Movement APIs (`MoveTo`, `WalkMobileRange`, fast. Creatures step at most once per `CurrentMoveSpeed` period, paced from the step just taken — a stall never banks catch-up steps, so a resumed chase restarts at full pace. +### Target Acquisition: the reaction-time gradient + +Acquisition is event-driven, not polled. The periodic scan (`AcquireFocusMob`) is gated by +`ReacquireDelay` (10 s default) and every scan re-arms it in full, success or failure — it +is target stickiness plus the fallback for what movement cannot signal (reveals, doors, +summons). Reaction time comes from `BaseCreature.OnMovement`: an enemy moving inside +`AcquireOnApproachRange` (10 — on-screen; the periodic scan keeps the wider +`RangePerception`) clamps the next scan to +at most **`AcquireOnApproachDelay`** — the intelligence gradient. `TimeSpan.Zero` +(paragons) also prods the AI, so the ranked scan engages within a timer-wheel turn; the +2 s default reads as "took a beat to notice you"; larger is dumber; a creature that +overrides the delay above `ReacquireDelay` is effectively oblivious to approach. Repeated +steps cannot shorten the clamp, so an armed creature scans once per delay period, not once +per step or think. `ReacquireOnMovement` remains the broader hook (any mover, no enemy +check, scan next think). The gate self-heals: a deadline further out than `ReacquireDelay` +is illegal and reads as open, so no wedged or wrapped value can silence acquisition beyond +one delay period. + ### OnThink: the excess-call contract `OnThink()` is a scheduler pass, not an action. The AI timer calls it *at least* at the diff --git a/dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md b/dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md index 7d4581b20..5b30e7ef9 100644 --- a/dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md +++ b/dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md @@ -497,6 +497,26 @@ An isolated step (after the creature stood for at least a walk interval) goes ou walk regardless of pace — only a continuing cadence, or a pace faster than the run interpolation, flags run. +## Target Acquisition: `AcquireOnApproach` Is a Delay + +RunUO's `AcquireOnApproach` bool (paragon insta-aggro on approach) is now +`AcquireOnApproachDelay`, a `TimeSpan` reaction-time gradient that applies to every +creature — enemy movement inside `AcquireOnApproachRange` schedules a scan within the +delay instead of waiting out the 10 s `ReacquireDelay` poll: + +```csharp +// RunUO +public override bool AcquireOnApproach => true; + +// ModernUO — Zero is the old instant behavior; larger values are dumber +public override TimeSpan AcquireOnApproachDelay => TimeSpan.Zero; +``` + +`AcquireOnApproachRange` stays 10 for all creatures (reactive aggro is on-screen; the +periodic `ReacquireDelay` scan still sweeps the full `RangePerception`). The +acquired target comes from the normal FightMode-ranked scan, not from whichever mobile +happened to move. See `content-patterns.md` § Target Acquisition. + ## Item Name Changes ```csharp diff --git a/dev-docs/runuo-migration-docs/11-api-reference.md b/dev-docs/runuo-migration-docs/11-api-reference.md index 358bf13c7..96db1590d 100644 --- a/dev-docs/runuo-migration-docs/11-api-reference.md +++ b/dev-docs/runuo-migration-docs/11-api-reference.md @@ -133,6 +133,7 @@ Alphabetical by RunUO API name. Use Ctrl+F / Cmd+F to search. | `MoveTo(m, run, range)` | `MoveTo(m, range)` | `run` removed; the Running bit is derived from the step pace (`BaseAI.ShouldRun`) | | `WalkMobileRange(m, steps, run, min, max)` | `WalkMobileRange(m, steps, min, max)` | Same | | `PathFollower.Follow(run, range)` | `Follow(range)` | Same | +| `AcquireOnApproach` (bool) | `AcquireOnApproachDelay` (TimeSpan) | Reaction-time gradient; `Zero` = old instant behavior | ## Networking From 708a35433700152ee407c3acc8e2a7de67c9e800 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Tue, 1 Sep 2026 23:23:32 -0700 Subject: [PATCH 7/9] perf: stop allocating stat/skill mod lists for every mobile (#2604) ## Summary `_statMods` and `_skillMods` are created lazily by `AddStatMod` / `AddSkillMod` and nulled when they empty, and every reader already null-checks. The eager `new List()` in `DefaultMobileInit` and `Deserialize` therefore allocated two dead 32-byte objects for every mobile. On a ~500k-mobile world that is ~32 MB and 1M gen2 objects that hold nothing. - Removes the four eager allocations. - Removes the `StatMods` accessor (no references). - Documents `SkillMods` as `null` when no mods are active (its one caller in `Skills.cs` already checks). First of three PRs from the lazy per-mobile collections design; `DamageEntries` and `Aggressors`/`Aggressed` follow separately. ## Breaking change - `Mobile.SkillMods` may now be `null` (it was never null after construction before). External callers that enumerate it or read `.Count` must null-check. - `Mobile.StatMods` is removed. Use `GetStatMod(name)` / `AddStatMod` / `RemoveStatMod`. Save format is untouched: neither list is serialized. ## Testing - `dotnet build -c Release` clean. - New `MobileLazyModListTests` plus full `Server.Tests` (840) and `UOContent.Tests` (756). --- Projects/Server/Mobiles/Mobile.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Projects/Server/Mobiles/Mobile.cs b/Projects/Server/Mobiles/Mobile.cs index 208142225..0407d7f82 100644 --- a/Projects/Server/Mobiles/Mobile.cs +++ b/Projects/Server/Mobiles/Mobile.cs @@ -6478,9 +6478,6 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro m_DexLock = (StatLockType)reader.ReadByte(); m_IntLock = (StatLockType)reader.ReadByte(); - _statMods = new List(); - _skillMods = new List(); - if (version < 32) { if (reader.ReadBool()) @@ -7813,8 +7810,6 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro m_FollowersMax = 5; Skills = new Skills(this); Items = new List(); - _statMods = new List(); - _skillMods = new List(); Map = Map.Internal; AutoPageNotify = true; Aggressors = new List(); From e52d54b7dacaadca3f4051e6a741c28ce19df1ee Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Tue, 1 Sep 2026 23:25:14 -0700 Subject: [PATCH 8/9] perf: keep damage entries in an inline intrusive list (#2605) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary `Mobile.DamageEntries` was a `List` allocated for every mobile, including the ~99% that never take damage. It is now an inline `ValueLinkList` (24 bytes in the `Mobile` object, no separate allocation) ordered least recent → most recent. - `DamageEntry` implements `IValueLinkListNode`. - `RegisterDamage` moves the entry to the tail in O(1) instead of `Remove` + `Add` on a list. - Expired entries are always a head prefix, so pruning walks from the head and stops at the first live entry. The `DamageEntries` getter prunes on access. - `DamageEntries` is exposed as `ref readonly`; enumerate with `foreach` or `.ByDescending()`. Mutation goes through `RegisterDamage` / `ClearDamageEntries`. - `BaseCreature.GetLootingRights` and `BaseCreature.ComputeBonusDamage` take `in ValueLinkList`; all callers compile unchanged. Files that `foreach` over `DamageEntries` need `using Server.Collections;` for the enumerator extension. - RunUO migration docs (`dev-docs/runuo-migration-docs/09`, `11`) and the `migrate-items-mobiles` skill document the change. Saves one object and 16 bytes per mobile (~8 MB and 500k gen2 objects on a 500k world). Second of three PRs from the lazy per-mobile collections design (first: #2604). Branched from `main`; the two diffs touch disjoint hunks of `Mobile.cs`. ## Breaking change - `Mobile.DamageEntries` is no longer a `List`. Indexing, `.Clear()`, `.Add()`, `.Remove()` no longer compile; use `foreach`, `.ByDescending()`, `.Count`, `ClearDamageEntries()`, and `RegisterDamage`. Calling a `ValueLinkList` mutator on the `ref readonly` property compiles but operates on a copy while still unlinking the real nodes; do not. - `BaseCreature.GetLootingRights` and `BaseCreature.ComputeBonusDamage` signatures changed to `(in ValueLinkList, …)`. Save format is untouched: damage entries are not serialized. ## Behavior Recency order, `allowSelf`, tie-breaking in `FindMostTotal`/`FindLeastTotal` (most recent wins), `Responsible` accounting, and loot-rights ordering are unchanged and covered by the new `DamageEntryTests` and `LootingRightsTests`. ## Testing - `dotnet build -c Release` clean. - New `DamageEntryTests` and `LootingRightsTests` plus full `Server.Tests` and `UOContent.Tests`. --- .../Tests/Mobiles/DamageEntryTests.cs | 311 ++++++++++++++++++ Projects/Server/Mobiles/Mobile.cs | 174 +++++----- .../Tests/Mobiles/LootingRightsTests.cs | 146 ++++++++ .../Engines/CannedEvil/ChampionSpawn.cs | 6 +- Projects/UOContent/Mobiles/BaseCreature.cs | 25 +- .../UOContent/Mobiles/Special/Harrower.cs | 1 + .../migrate-items-mobiles.md | 1 + .../09-items-mobiles-creatures.md | 39 +++ .../runuo-migration-docs/11-api-reference.md | 3 + 9 files changed, 603 insertions(+), 103 deletions(-) create mode 100644 Projects/Server.Tests/Tests/Mobiles/DamageEntryTests.cs create mode 100644 Projects/UOContent.Tests/Tests/Mobiles/LootingRightsTests.cs diff --git a/Projects/Server.Tests/Tests/Mobiles/DamageEntryTests.cs b/Projects/Server.Tests/Tests/Mobiles/DamageEntryTests.cs new file mode 100644 index 000000000..9c3cdfd14 --- /dev/null +++ b/Projects/Server.Tests/Tests/Mobiles/DamageEntryTests.cs @@ -0,0 +1,311 @@ +using System; +using System.Collections.Generic; +using Server.Collections; +using Xunit; + +namespace Server.Tests; + +[Collection("Sequential Server Tests")] +public class DamageEntryTests +{ + private class TestMobile : Mobile + { + } + + private class PetMobile : Mobile + { + public Mobile Master { get; set; } + + public override Mobile GetDamageMaster(Mobile damagee) => Master; + } + + private static List Damagers(Mobile victim) + { + var result = new List(); + foreach (var de in victim.DamageEntries) + { + result.Add(de.Damager); + } + + return result; + } + + [Fact] + public void FreshMobile_HasNoEntries() + { + var m = new TestMobile(); + + try + { + Assert.Equal(0, m.DamageEntries.Count); + Assert.Null(m.FindMostRecentDamageEntry(true)); + Assert.Null(m.FindLeastRecentDamageEntry(true)); + Assert.Null(m.FindMostTotalDamageEntry(true)); + Assert.Null(m.FindLeastTotalDamageEntry(true)); + Assert.Null(m.FindDamageEntryFor(m)); + } + finally + { + m.Delete(); + } + } + + [Fact] + public void RegisterDamage_OrdersLeastRecentToMostRecent() + { + var victim = new TestMobile(); + var a = new TestMobile(); + var b = new TestMobile(); + + try + { + victim.RegisterDamage(10, a); + victim.RegisterDamage(20, b); + victim.RegisterDamage(5, a); // a becomes most recent again + + Assert.Equal(2, victim.DamageEntries.Count); + Assert.Equal(new[] { b, a }, Damagers(victim)); + Assert.Equal(15, victim.FindDamageEntryFor(a).DamageGiven); + Assert.Same(a, victim.FindMostRecentDamager(true)); + Assert.Same(b, victim.FindLeastRecentDamager(true)); + } + finally + { + victim.Delete(); + a.Delete(); + b.Delete(); + } + } + + [Fact] + public void FindRecent_HonorsAllowSelf() + { + var victim = new TestMobile(); + var a = new TestMobile(); + + try + { + victim.RegisterDamage(10, a); + victim.RegisterDamage(10, victim); // self is most recent + + Assert.Same(victim, victim.FindMostRecentDamager(true)); + Assert.Same(a, victim.FindMostRecentDamager(false)); + Assert.Same(a, victim.FindLeastRecentDamager(false)); + } + finally + { + victim.Delete(); + a.Delete(); + } + } + + [Fact] + public void FindLeastRecent_HonorsAllowSelf() + { + var victim = new TestMobile(); + var a = new TestMobile(); + + try + { + victim.RegisterDamage(10, victim); // self is least recent, so the head is the one to skip + victim.RegisterDamage(10, a); + + Assert.Same(victim, victim.FindLeastRecentDamager(true)); + Assert.Same(a, victim.FindLeastRecentDamager(false)); + } + finally + { + victim.Delete(); + a.Delete(); + } + } + + [Fact] + public void FindTotal_PicksByDamage_MostRecentWinsTies() + { + var victim = new TestMobile(); + var a = new TestMobile(); + var b = new TestMobile(); + var c = new TestMobile(); + + try + { + victim.RegisterDamage(30, a); + victim.RegisterDamage(30, b); // ties a; b is more recent + victim.RegisterDamage(1, c); + + Assert.Same(b, victim.FindMostTotalDamager(true)); + Assert.Same(c, victim.FindLeastTotalDamager(true)); + } + finally + { + victim.Delete(); + a.Delete(); + b.Delete(); + c.Delete(); + } + } + + [Fact] + public void FindLeastTotal_MostRecentWinsTies() + { + var victim = new TestMobile(); + var a = new TestMobile(); + var b = new TestMobile(); + var c = new TestMobile(); + + try + { + victim.RegisterDamage(30, a); + victim.RegisterDamage(5, b); + victim.RegisterDamage(5, c); // ties b for the minimum; c is more recent + + Assert.Same(a, victim.FindMostTotalDamager(true)); + Assert.Same(c, victim.FindLeastTotalDamager(true)); + } + finally + { + victim.Delete(); + a.Delete(); + b.Delete(); + c.Delete(); + } + } + + [Fact] + public void Prune_RemovesExpiredPrefix_KeepsOrder() + { + var start = Core._now; + var victim = new TestMobile(); + var a = new TestMobile(); + var b = new TestMobile(); + + try + { + victim.RegisterDamage(10, a); + + Core._now = start + DamageEntry.ExpireDelay + TimeSpan.FromSeconds(1); + victim.RegisterDamage(10, b); // a is now expired, b is live + + Assert.Equal(new[] { b }, Damagers(victim)); + Assert.Null(victim.FindDamageEntryFor(a)); + } + finally + { + Core._now = start; + victim.Delete(); + a.Delete(); + b.Delete(); + } + } + + [Fact] + public void Prune_AllExpired_EmptiesList() + { + var start = Core._now; + var victim = new TestMobile(); + var a = new TestMobile(); + var b = new TestMobile(); + + try + { + victim.RegisterDamage(10, a); + victim.RegisterDamage(10, b); + + Core._now = start + DamageEntry.ExpireDelay + TimeSpan.FromSeconds(1); + + Assert.Equal(0, victim.DamageEntries.Count); + Assert.Null(victim.FindMostRecentDamageEntry(true)); + } + finally + { + Core._now = start; + victim.Delete(); + a.Delete(); + b.Delete(); + } + } + + [Fact] + public void ClearDamageEntries_UnlinksEveryNode() + { + var victim = new TestMobile(); + var a = new TestMobile(); + var b = new TestMobile(); + + try + { + var ea = victim.RegisterDamage(10, a); + var eb = victim.RegisterDamage(10, b); + + victim.ClearDamageEntries(); + + Assert.Equal(0, victim.DamageEntries.Count); + Assert.False(ea.OnLinkList); + Assert.False(eb.OnLinkList); + Assert.Null(ea.Next); + Assert.Null(ea.Previous); + Assert.Null(eb.Next); + Assert.Null(eb.Previous); + } + finally + { + victim.Delete(); + a.Delete(); + b.Delete(); + } + } + + [Fact] + public void FullHitPoints_ClearsEntries() + { + var victim = new TestMobile(); + var a = new TestMobile(); + + try + { + victim.RawStr = 50; // HitsMax follows Str for a base Mobile + victim.Hits = 10; + victim.RegisterDamage(10, a); + Assert.Equal(1, victim.DamageEntries.Count); + + // Also stops the HitsTimer the Hits = 10 write started, so the test leaves no timer behind. + victim.Hits = victim.HitsMax; + + Assert.Equal(0, victim.DamageEntries.Count); + } + finally + { + victim.Delete(); + a.Delete(); + } + } + + [Fact] + public void RegisterDamage_AccumulatesResponsibleMaster() + { + var victim = new TestMobile(); + var master = new TestMobile(); + var pet = new PetMobile { Master = master }; + + try + { + victim.RegisterDamage(10, pet); + var entry = victim.RegisterDamage(5, pet); + + Assert.Same(pet, entry.Damager); + Assert.Equal(15, entry.DamageGiven); + Assert.NotNull(entry.Responsible); + Assert.Single(entry.Responsible); + Assert.Same(master, entry.Responsible[0].Damager); + Assert.Equal(15, entry.Responsible[0].DamageGiven); + Assert.False(entry.Responsible[0].OnLinkList); // sub-entries never join the main list + } + finally + { + victim.Delete(); + master.Delete(); + pet.Delete(); + } + } +} diff --git a/Projects/Server/Mobiles/Mobile.cs b/Projects/Server/Mobiles/Mobile.cs index 0407d7f82..d1db8113b 100644 --- a/Projects/Server/Mobiles/Mobile.cs +++ b/Projects/Server/Mobiles/Mobile.cs @@ -42,7 +42,7 @@ public delegate void PromptCallback(Mobile from, string text); public delegate void PromptStateCallback(Mobile from, string text, T state); -public class DamageEntry +public class DamageEntry : IValueLinkListNode { public DamageEntry(Mobile damager) => Damager = damager; @@ -57,6 +57,11 @@ public class DamageEntry public List Responsible { get; set; } public static TimeSpan ExpireDelay { get; set; } = TimeSpan.FromMinutes(2.0); + + // Intrusive links for Mobile._damageEntries. Sub-entries in Responsible never join a list. + public DamageEntry Next { get; set; } + public DamageEntry Previous { get; set; } + public bool OnLinkList { get; set; } } [Flags] @@ -377,7 +382,6 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro Aggressors = new List(); Aggressed = new List(); NextSkillTime = Core.TickCount; - DamageEntries = new List(); } // Sectors @@ -958,7 +962,23 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro public static VisibleDamageType VisibleDamageType { get; set; } - public List DamageEntries { get; private set; } + private ValueLinkList _damageEntries; + + /// + /// Damage entries ordered least recent (head) to most recent (tail). Expired entries are + /// pruned on access. Enumerate with foreach (ascending) or .ByDescending(). + /// Mutate only through and . + /// Calling a ValueLinkList mutator on this reference compiles, but operates on a defensive copy + /// while still unlinking the real nodes — it silently corrupts the list. + /// + public ref readonly ValueLinkList DamageEntries + { + get + { + PruneExpiredDamageEntries(); + return ref _damageEntries; + } + } [CommandProperty(AccessLevel.GameMaster)] public Mobile LastKiller { get; set; } @@ -2020,10 +2040,7 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro Aggressors[i].CanReportMurder = false; } - if (DamageEntries.Count > 0) - { - DamageEntries.Clear(); // reset damage entries on full HP - } + ClearDamageEntries(); // reset damage entries on full HP } else if (CanRegenHits) { @@ -5745,24 +5762,54 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro } } + // Entries are kept in LastDamage order, so expired entries are always a head prefix. + private void PruneExpiredDamageEntries() + { +#if DEBUG + for (var node = _damageEntries._first; node != null; node = node.Next) + { + Debug.Assert( + node.Next == null || node.Next.LastDamage >= node.LastDamage, + "Damage entries must be ordered by LastDamage ascending." + ); + } +#endif + + var first = _damageEntries._first; + + if (first?.HasExpired != true) + { + return; + } + + var firstLive = first.Next; + + while (firstLive?.HasExpired == true) + { + firstLive = firstLive.Next; + } + + if (firstLive == null) + { + _damageEntries.RemoveAll(); + } + else + { + _damageEntries.RemoveAllBefore(firstLive); + } + } + + public void ClearDamageEntries() => _damageEntries.RemoveAll(); + public Mobile FindMostRecentDamager(bool allowSelf) => FindMostRecentDamageEntry(allowSelf)?.Damager; public DamageEntry FindMostRecentDamageEntry(bool allowSelf) { - for (var i = DamageEntries.Count - 1; i >= 0; --i) + PruneExpiredDamageEntries(); + + for (var de = _damageEntries._last; de != null; de = de.Previous) { - if (i >= DamageEntries.Count) - { - continue; - } - - var de = DamageEntries[i]; - - if (de.HasExpired) - { - DamageEntries.RemoveAt(i); - } - else if (allowSelf || de.Damager != this) + if (allowSelf || de.Damager != this) { return de; } @@ -5775,21 +5822,11 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro public DamageEntry FindLeastRecentDamageEntry(bool allowSelf) { - for (var i = 0; i < DamageEntries.Count; ++i) + PruneExpiredDamageEntries(); + + for (var de = _damageEntries._first; de != null; de = de.Next) { - if (i < 0) - { - continue; - } - - var de = DamageEntries[i]; - - if (de.HasExpired) - { - DamageEntries.RemoveAt(i); - --i; - } - else if (allowSelf || de.Damager != this) + if (allowSelf || de.Damager != this) { return de; } @@ -5800,24 +5837,17 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro public Mobile FindMostTotalDamager(bool allowSelf) => FindMostTotalDamageEntry(allowSelf)?.Damager; + // Walks most recent first with a strict comparison so the most recent entry wins ties, + // matching the previous reverse-indexed loop. public DamageEntry FindMostTotalDamageEntry(bool allowSelf) { + PruneExpiredDamageEntries(); + DamageEntry mostTotal = null; - for (var i = DamageEntries.Count - 1; i >= 0; --i) + for (var de = _damageEntries._last; de != null; de = de.Previous) { - if (i >= DamageEntries.Count) - { - continue; - } - - var de = DamageEntries[i]; - - if (de.HasExpired) - { - DamageEntries.RemoveAt(i); - } - else if ((allowSelf || de.Damager != this) && (mostTotal == null || de.DamageGiven > mostTotal.DamageGiven)) + if ((allowSelf || de.Damager != this) && (mostTotal == null || de.DamageGiven > mostTotal.DamageGiven)) { mostTotal = de; } @@ -5830,46 +5860,28 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro public DamageEntry FindLeastTotalDamageEntry(bool allowSelf) { - DamageEntry mostTotal = null; + PruneExpiredDamageEntries(); - for (var i = DamageEntries.Count - 1; i >= 0; --i) + DamageEntry leastTotal = null; + + for (var de = _damageEntries._last; de != null; de = de.Previous) { - if (i >= DamageEntries.Count) + if ((allowSelf || de.Damager != this) && (leastTotal == null || de.DamageGiven < leastTotal.DamageGiven)) { - continue; - } - - var de = DamageEntries[i]; - - if (de.HasExpired) - { - DamageEntries.RemoveAt(i); - } - else if ((allowSelf || de.Damager != this) && (mostTotal == null || de.DamageGiven < mostTotal.DamageGiven)) - { - mostTotal = de; + leastTotal = de; } } - return mostTotal; + return leastTotal; } public DamageEntry FindDamageEntryFor(Mobile m) { - for (var i = DamageEntries.Count - 1; i >= 0; --i) + PruneExpiredDamageEntries(); + + for (var de = _damageEntries._last; de != null; de = de.Previous) { - if (i >= DamageEntries.Count) - { - continue; - } - - var de = DamageEntries[i]; - - if (de.HasExpired) - { - DamageEntries.RemoveAt(i); - } - else if (de.Damager == m) + if (de.Damager == m) { return de; } @@ -5887,8 +5899,13 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro de.DamageGiven += amount; de.LastDamage = Core.Now; - DamageEntries.Remove(de); - DamageEntries.Add(de); + // Move to the tail so the list stays in LastDamage order. + if (de.OnLinkList) + { + _damageEntries.Remove(de); + } + + _damageEntries.AddLast(de); var master = from.GetDamageMaster(this); @@ -7814,7 +7831,6 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro AutoPageNotify = true; Aggressors = new List(); Aggressed = new List(); - DamageEntries = new List(); NextSkillTime = Core.TickCount; } diff --git a/Projects/UOContent.Tests/Tests/Mobiles/LootingRightsTests.cs b/Projects/UOContent.Tests/Tests/Mobiles/LootingRightsTests.cs new file mode 100644 index 000000000..e02abece7 --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Mobiles/LootingRightsTests.cs @@ -0,0 +1,146 @@ +using System.Collections.Generic; +using Server.Mobiles; +using Xunit; + +namespace Server.Tests; + +/// +/// Pins the looting-rights rules that the inline damage entry list has to keep producing: the +/// returned stores are sorted by damage descending, the first (least recent) damager takes the +/// 1.25x bonus, the hitsMax band decides who clears the threshold, and a pet's damage is credited +/// to its damage master rather than to the pet. +/// +[Collection("Sequential UOContent Tests")] +public class LootingRightsTests +{ + private class TestMobile : Mobile + { + } + + private class PetMobile : Mobile + { + public Mobile Master { get; set; } + + public override Mobile GetDamageMaster(Mobile damagee) => Master; + } + + // GetLootingRights only ever credits mobiles flagged as players. + private static TestMobile NewPlayer() => new() { Player = true }; + + private static DamageStore FindStore(List rights, Mobile m) + { + for (var i = 0; i < rights.Count; i++) + { + if (rights[i].m_Mobile == m) + { + return rights[i]; + } + } + + return null; + } + + [Fact] + public void TwoPlayerDamagers_SortDescending_AndTheFirstDamagerTakesTheBonus() + { + var victim = new TestMobile(); + var first = NewPlayer(); + var second = NewPlayer(); + + try + { + victim.RegisterDamage(100, first); + victim.RegisterDamage(40, second); // second is the most recent, first is the "first damager" + + // hitsMax < 200 puts the bar at topDamage / 2. + var rights = BaseCreature.GetLootingRights(victim.DamageEntries, 100); + + Assert.Equal(2, rights.Count); + + // Sorted by damage descending. + Assert.True(rights[0].m_Damage >= rights[1].m_Damage); + Assert.Same(first, rights[0].m_Mobile); + Assert.Same(second, rights[1].m_Mobile); + + // The first damager - the least recent entry - gets the 1.25x bonus; nobody else does. + Assert.Equal(125, rights[0].m_Damage); + Assert.Equal(40, rights[1].m_Damage); + + // topDamage 125 / 2 = 62, so 40 is below the bar. + Assert.True(rights[0].m_HasRight); + Assert.False(rights[1].m_HasRight); + } + finally + { + victim.Delete(); + first.Delete(); + second.Delete(); + } + } + + [Fact] + public void HitsMaxBand_MovesTheRightsThreshold() + { + var victim = new TestMobile(); + var first = NewPlayer(); + var second = NewPlayer(); + + try + { + victim.RegisterDamage(100, first); + victim.RegisterDamage(40, second); + + // hitsMax >= 200 drops the bar to topDamage / 4 = 31, which 40 clears. + var rights = BaseCreature.GetLootingRights(victim.DamageEntries, 200); + + Assert.Equal(2, rights.Count); + Assert.True(rights[0].m_HasRight); + Assert.True(rights[1].m_HasRight); + Assert.Same(second, rights[1].m_Mobile); + } + finally + { + victim.Delete(); + first.Delete(); + second.Delete(); + } + } + + [Fact] + public void PetDamage_CreditsTheMaster_NotThePet() + { + var victim = new TestMobile(); + var master = NewPlayer(); + var pet = new PetMobile { Master = master }; + var wild = new TestMobile(); // no damage master, and not a player + + try + { + victim.RegisterDamage(50, pet); + victim.RegisterDamage(20, wild); + + var rights = BaseCreature.GetLootingRights(victim.DamageEntries, 100); + + // The master is credited through the entry's Responsible sub-entry, and is the only one. + Assert.Single(rights); + + var masterStore = FindStore(rights, master); + Assert.NotNull(masterStore); + Assert.Equal(62, masterStore.m_Damage); // 50, then the first-damager 1.25x bonus + Assert.True(masterStore.m_HasRight); + + // The pet's own damage was fully handed to the master, so it earns no store. + Assert.Null(FindStore(rights, pet)); + + // A non-player damager earns nothing even when its damage was never reassigned. + Assert.Null(FindStore(rights, wild)); + } + finally + { + victim.Delete(); + master.Delete(); + pet.Delete(); + wild.Delete(); + } + } +} diff --git a/Projects/UOContent/Engines/CannedEvil/ChampionSpawn.cs b/Projects/UOContent/Engines/CannedEvil/ChampionSpawn.cs index 9daea0ba3..3a024576b 100755 --- a/Projects/UOContent/Engines/CannedEvil/ChampionSpawn.cs +++ b/Projects/UOContent/Engines/CannedEvil/ChampionSpawn.cs @@ -18,6 +18,7 @@ using System.Net; using System.Collections.Generic; using System.Runtime.InteropServices; using ModernUO.Serialization; +using Server.Collections; using Server.Engines.Virtues; using Server.Gumps; using Server.Items; @@ -1181,11 +1182,6 @@ public partial class ChampionSpawn : Item foreach (var de in m.DamageEntries) { - if (de.HasExpired) - { - continue; - } - var damager = de.Damager; var master = damager.GetDamageMaster(m); diff --git a/Projects/UOContent/Mobiles/BaseCreature.cs b/Projects/UOContent/Mobiles/BaseCreature.cs index 581d472e3..404aaede8 100644 --- a/Projects/UOContent/Mobiles/BaseCreature.cs +++ b/Projects/UOContent/Mobiles/BaseCreature.cs @@ -3123,14 +3123,12 @@ namespace Server.Mobiles return base.OnBeforeDeath(); } - public int ComputeBonusDamage(List list, Mobile m) + public int ComputeBonusDamage(in ValueLinkList list, Mobile m) { var bonus = 0; - for (var i = list.Count - 1; i >= 0; --i) + foreach (var de in list.ByDescending()) { - var de = list[i]; - if (de.Damager == m || de.Damager is not BaseCreature bc) { continue; @@ -3167,26 +3165,15 @@ namespace Server.Mobiles Combatant is PlayerMobile || Combatant is BaseCreature { Controlled: true } bc && bc.GetMaster() is PlayerMobile; - public static List GetLootingRights(List damageEntries, int hitsMax) + // Iterates most recent first, matching the previous reverse-indexed loop. The list is + // already pruned of expired entries by the Mobile.DamageEntries getter. + public static List GetLootingRights(in ValueLinkList damageEntries, int hitsMax) { var rights = new List(); DamageStore firstDamager = null; - for (var i = damageEntries.Count - 1; i >= 0; --i) + foreach (var de in damageEntries.ByDescending()) { - if (i >= damageEntries.Count) - { - continue; - } - - var de = damageEntries[i]; - - if (de.HasExpired) - { - damageEntries.RemoveAt(i); - continue; - } - var damage = de.DamageGiven; var respList = de.Responsible; diff --git a/Projects/UOContent/Mobiles/Special/Harrower.cs b/Projects/UOContent/Mobiles/Special/Harrower.cs index ee3fa1873..8d87943bc 100644 --- a/Projects/UOContent/Mobiles/Special/Harrower.cs +++ b/Projects/UOContent/Mobiles/Special/Harrower.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using ModernUO.Serialization; +using Server.Collections; using Server.Engines.CannedEvil; using Server.Engines.Virtues; using Server.Items; diff --git a/dev-docs/claude-skills/migrate-from-runuo/migrate-items-mobiles.md b/dev-docs/claude-skills/migrate-from-runuo/migrate-items-mobiles.md index f12f92c55..7310c104e 100644 --- a/dev-docs/claude-skills/migrate-from-runuo/migrate-items-mobiles.md +++ b/dev-docs/claude-skills/migrate-from-runuo/migrate-items-mobiles.md @@ -31,6 +31,7 @@ description: > - Expression-bodied overrides: `public override int Meat { get { return 1; } }` -> `public override int Meat => 1;` - AI movement calls lose the `run` flag: `MoveTo(m, true, range)` -> `MoveTo(m, range)` (also `WalkMobileRange`, `ApproachTarget`, `MoveToPoint`, `PathFollower.Follow`); the Running bit is derived from step pace -> `dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md` § AI Movement - `AcquireOnApproach` (bool) -> `AcquireOnApproachDelay` (TimeSpan; `Zero` = old instant behavior) -> same doc § Target Acquisition +- `DamageEntries` is an inline `ref readonly ValueLinkList`, not a `List`: indexer/`Add`/`Remove`/`Clear` -> `foreach` / `.ByDescending()` (needs `using Server.Collections;`) and `ClearDamageEntries()`; `GetLootingRights` takes it by `in` -> same doc § Damage Entries ## Anti-Patterns - Using `_field--` instead of `Property--` (bypasses MarkDirty tracking) diff --git a/dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md b/dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md index 5b30e7ef9..def2ecf48 100644 --- a/dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md +++ b/dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md @@ -517,6 +517,45 @@ periodic `ReacquireDelay` scan still sweeps the full `RangePerception`). The acquired target comes from the normal FightMode-ranked scan, not from whichever mobile happened to move. See `content-patterns.md` § Target Acquisition. +## Damage Entries: Inline `ValueLinkList`, Not `List` + +RunUO's `Mobile.DamageEntries` was a `List` allocated for every mobile. +ModernUO keeps damage entries in an inline `ValueLinkList` struct held by +the mobile itself, ordered least recent → most recent, so a mobile that never takes +damage owns no list object and `RegisterDamage` relinks in O(1). The property is +`ref readonly`; expired entries are pruned when it is read. + +```csharp +// RunUO +for (var i = m.DamageEntries.Count - 1; i >= 0; --i) +{ + var de = m.DamageEntries[i]; // indexer + ... +} +m.DamageEntries.Clear(); +var rights = BaseCreature.GetLootingRights(m.DamageEntries, m.HitsMax); // List + +// ModernUO — needs `using Server.Collections;` for the enumerator extensions +foreach (var de in m.DamageEntries.ByDescending()) // most recent first +{ + ... +} +foreach (var de in m.DamageEntries) // least recent first +{ + ... +} +m.ClearDamageEntries(); +var rights = BaseCreature.GetLootingRights(m.DamageEntries, m.HitsMax); // in ValueLinkList +``` + +What no longer compiles: the indexer, `.Add`, `.Remove`, `.RemoveAt`, `.Clear`, and +passing the property where a `List` is expected. `.Count`, +`FindDamageEntryFor`, `FindMostRecentDamager` and the other `Find*` methods, and +`RegisterDamage` are unchanged. `DamageEntry` now carries `Next`/`Previous`/`OnLinkList` +link fields; never set them yourself, and never call a `ValueLinkList` mutator on the +`ref readonly` property — it compiles against a copy and corrupts the node's link state. +Mutate only through `RegisterDamage` and `ClearDamageEntries`. + ## Item Name Changes ```csharp diff --git a/dev-docs/runuo-migration-docs/11-api-reference.md b/dev-docs/runuo-migration-docs/11-api-reference.md index 96db1590d..b6c12bfe8 100644 --- a/dev-docs/runuo-migration-docs/11-api-reference.md +++ b/dev-docs/runuo-migration-docs/11-api-reference.md @@ -134,6 +134,9 @@ Alphabetical by RunUO API name. Use Ctrl+F / Cmd+F to search. | `WalkMobileRange(m, steps, run, min, max)` | `WalkMobileRange(m, steps, min, max)` | Same | | `PathFollower.Follow(run, range)` | `Follow(range)` | Same | | `AcquireOnApproach` (bool) | `AcquireOnApproachDelay` (TimeSpan) | Reaction-time gradient; `Zero` = old instant behavior | +| `m.DamageEntries` (`List`) | `m.DamageEntries` (`ref readonly ValueLinkList`) | Inline, least→most recent; `foreach` / `.ByDescending()` only, needs `using Server.Collections;`; no indexer, `Add`, `Remove`, `Clear` | +| `m.DamageEntries.Clear()` | `m.ClearDamageEntries()` | | +| `GetLootingRights(List, int)` | `GetLootingRights(in ValueLinkList, int)` | Callers passing `m.DamageEntries` compile unchanged | ## Networking From 25a2aa03c5cbe608eeed61ceaad02f5e6dc02d7e Mon Sep 17 00:00:00 2001 From: WarrentyExpired Date: Wed, 2 Sep 2026 10:16:24 -0400 Subject: [PATCH 9/9] #W# Update: added Distribution/Data/Files to gitignore. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index d5b26e268..d29ee313a 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ # Distribution Files +/Distribution/Data/Files /Distribution/Logger /Distribution/Logger.* /Distribution/ModernUO