From 502c3f6aa4be9a87aec71da26fc714fdbce51b2b Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Tue, 22 Sep 2026 18:48:13 -0700 Subject: [PATCH] fix(ai): BaseAI.OnAggressiveAction owns the retaliation policy; familiars refuse cleanly (#2662) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > [!IMPORTANT] > **Breaking change to API to fix NPC AI issues** ## Symptom `FamiliarAITests.HiddenCaster_FamiliarRefusesRetaliation` fails on main at `Assert.False(wolf.Warmode)`: the familiar's `Combatant` stays null as intended, but it is left in war stance. In-game the familiar of a hidden caster flips into warmode for a tick when struck, until `FamiliarAI.StandDown()` clears it on the next think. CI never saw it because the familiar tests are `SkippableFact` (they need client tiledata). ## Root cause #2644 and #2645 landed a day apart and are individually correct but conflict at one seam. Since #2645, a familiar (`Commandable == false`) is no longer "told to stand down", so `BaseCreature.AggressiveAction` continues past `AIObject.OnAggressiveAction(aggressor)` to its generic fallback: ```cs else if (Combatant == null && !BardPacified) { Warmode = true; Combatant = aggressor; } ``` `FamiliarAI.OnAggressiveAction` had already declined (caster hidden), but the hook was `void`, so the creature could not tell. The fallback raised `Warmode` before `BaseFamiliar.Combatant` vetoed the target. ## Fix This is the follow-up agreed in #2645: the retaliation policy moves into a virtual `BaseAI.OnAggressiveAction(aggressor) -> bool`. - **`BaseAI.OnAggressiveAction`** now holds the stand-down verdict, the closer-aggressor swap (`PreferCloserAggressor`), and the `IssueOrder(Attack)` vs `Combatant =` decision. Returns `false` when the creature stands down. - **`BaseCreature.AggressiveAction`** keeps only bookkeeping — `ControlMaster.Aggressors` entry, `StopFlee`, `ForceReacquire`, ethics `FinishShield` — keyed off the return, so an AI that declines is never second-guessed. - **`FamiliarAI`** overrides the whole policy: no orders to stand down on, never fights for a hidden caster or a hidden aggressor. Behaviour for every other AI is unchanged. Moving the fallback ahead of `StopFlee`/`ForceReacquire` is order-safe: both are pure field resets, `IssueAttack` does not `Think()`, and `OnCombatantChange` only derives `Warmode`. Two commits: the first is the minimal `bool` hook that alone fixes the test, the second the policy move. ## Not changed (noted for later) - `Warmode = true; Combatant = x;` (also in `FamiliarAI.Act`, `IssueAttack`) is redundant when the setter accepts — `BaseCreature.OnCombatantChange` derives `Warmode` — and is the dangling-warmode hazard whenever a `Combatant` setter vetoes. - Since #2614 a stood-down pet skips the ethics `FinishShield`; RunUO ran it unconditionally. Preserved as-is. ## Tests `HiddenCaster_FamiliarRefusesRetaliation` fails on main and passes here. Full `UOContent.Tests`: 1126 passed, 0 failed, 2 skipped (spawner fixture capture + benchmark). --- .../UOContent/Mobiles/AI/BaseAI/BaseAI.cs | 46 +++++++++++++++---- Projects/UOContent/Mobiles/AI/FamiliarAI.cs | 12 +++-- Projects/UOContent/Mobiles/BaseCreature.cs | 21 +-------- .../migrate-items-mobiles.md | 1 + dev-docs/content-patterns.md | 3 ++ .../09-items-mobiles-creatures.md | 43 +++++++++++++++++ .../runuo-migration-docs/11-api-reference.md | 1 + 7 files changed, 96 insertions(+), 31 deletions(-) diff --git a/Projects/UOContent/Mobiles/AI/BaseAI/BaseAI.cs b/Projects/UOContent/Mobiles/AI/BaseAI/BaseAI.cs index 9b42136a6..6e5f1c295 100644 --- a/Projects/UOContent/Mobiles/AI/BaseAI/BaseAI.cs +++ b/Projects/UOContent/Mobiles/AI/BaseAI/BaseAI.cs @@ -131,21 +131,51 @@ public abstract partial class BaseAI } } - public virtual void OnAggressiveAction(Mobile aggressor) + /// + /// The retaliation policy: how the creature answers . Returns false when it + /// stands down, and then skips its combat bookkeeping. + /// + public virtual bool OnAggressiveAction(Mobile aggressor) + { + // Only a creature somebody can command has been told anything; wild creatures rest on None. + var toldToStandDown = Mobile.Controlled && Mobile.ControlMaster != null && Mobile.Commandable && + IsStandDownOrder(Mobile.ControlOrder); + + if (Mobile.StandsDownOnCommand && toldToStandDown) + { + DebugSay("I'm being attacked but my master told me not to fight."); + Mobile.Warmode = false; + return false; + } + + PreferCloserAggressor(aggressor); + + if (aggressor.ChangingCombatant && toldToStandDown) + { + // With stand-down off, a resting pet answers a direct attack with a full Attack order. + Mobile.IssueOrder(OrderType.Attack, null, aggressor); + } + else if (Mobile.Combatant == null && !Mobile.BardPacified) + { + Mobile.Warmode = true; + Mobile.Combatant = aggressor; + } + + return true; + } + + // A visible aggressor closer than the current combatant takes over. + protected void PreferCloserAggressor(Mobile aggressor) { if (aggressor.Hidden) { return; } - var currentCombat = Mobile.Combatant; + var current = Mobile.Combatant; - if (currentCombat == null || currentCombat == aggressor) - { - return; - } - - if (Mobile.GetDistanceToSqrt(aggressor) < Mobile.GetDistanceToSqrt(currentCombat)) + if (current != null && current != aggressor && + Mobile.GetDistanceToSqrt(aggressor) < Mobile.GetDistanceToSqrt(current)) { Mobile.Combatant = aggressor; } diff --git a/Projects/UOContent/Mobiles/AI/FamiliarAI.cs b/Projects/UOContent/Mobiles/AI/FamiliarAI.cs index fcaf96c69..2da24dc8d 100644 --- a/Projects/UOContent/Mobiles/AI/FamiliarAI.cs +++ b/Projects/UOContent/Mobiles/AI/FamiliarAI.cs @@ -47,21 +47,25 @@ public class FamiliarAI : BaseAI return Mobile.Controlled ? OrderType.Come : OrderType.None; } - public override void OnAggressiveAction(Mobile aggressor) + // No orders to stand down on; never fights for a hidden caster. + public override bool OnAggressiveAction(Mobile aggressor) { if (!Familiar.AssistsMaster || aggressor.Hidden || Familiar.ControlMaster?.Hidden == true) { - return; + return false; } if (Mobile.Combatant == null) { Mobile.Warmode = true; Mobile.Combatant = aggressor; - return; + } + else + { + PreferCloserAggressor(aggressor); } - base.OnAggressiveAction(aggressor); + return true; } private bool Act() diff --git a/Projects/UOContent/Mobiles/BaseCreature.cs b/Projects/UOContent/Mobiles/BaseCreature.cs index 9d7e77959..f159c70e1 100644 --- a/Projects/UOContent/Mobiles/BaseCreature.cs +++ b/Projects/UOContent/Mobiles/BaseCreature.cs @@ -2735,18 +2735,12 @@ namespace Server.Mobiles aggressor.Aggressors.Add(AggressorInfo.Create(this, aggressor, true)); } - var toldToStandDown = _controlled && _controlMaster != null && Commandable && - BaseAI.IsStandDownOrder(_controlOrder); - - if (StandsDownOnCommand && toldToStandDown) + // The AI owns the retaliation policy; a creature that stands down keeps its flee and acquire state. + if (AIObject?.OnAggressiveAction(aggressor) != true) { - AIObject?.DebugSay("I'm being attacked but my master told me not to fight."); - Warmode = false; return; } - AIObject?.OnAggressiveAction(aggressor); - StopFlee(); ForceReacquire(); @@ -2760,17 +2754,6 @@ namespace Server.Mobiles pl.FinishShield(); } } - - // Only reachable when the pet does not stand down: the orders above returned early. - if (aggressor.ChangingCombatant && toldToStandDown) - { - IssueOrder(OrderType.Attack, null, aggressor); - } - else if (Combatant == null && !BardPacified) - { - Warmode = true; - Combatant = aggressor; - } } public override bool OnMoveOver(Mobile m) 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 7310c104e..9a7c08ab4 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 +- `BaseAI.OnAggressiveAction(aggressor)` is `bool`, not `void`: the AI owns the retaliation policy (`BaseCreature` no longer sets `Combatant` after it); an early `return;` that refused a fight -> `return false;`, otherwise `return true;`; swap-only overrides call `PreferCloserAggressor(aggressor)` instead of `base` -> same doc § Retaliation - `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 diff --git a/dev-docs/content-patterns.md b/dev-docs/content-patterns.md index 85ce6075c..e583364c6 100644 --- a/dev-docs/content-patterns.md +++ b/dev-docs/content-patterns.md @@ -231,6 +231,9 @@ class — `OnThink` is for content extras (see the excess-call contract below). creature is dispatched through `Obey()`, an uncontrolled one through `Think()`; an AI that owns both routes them into one decision (`FamiliarAI.Act`). Overriding `IssueOrder` to return a fixed order is how "command immunity" is expressed without bypassing the order machinery. +`OnAggressiveAction(aggressor) -> bool` is the retaliation policy: `BaseCreature` sets no +combatant of its own and runs `StopFlee`/`ForceReacquire` only on `true`, so an AI that +returns `false` has refused the fight outright. ### Fight Modes | FightMode | Behavior | 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 def2ecf48..389c845e5 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,49 @@ 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. +## Retaliation: `OnAggressiveAction` Returns `bool` + +RunUO's `BaseAI.OnAggressiveAction` was a `void` hook that only swapped to a closer +aggressor; `BaseCreature.AggressiveAction` then decided the response itself and set +`Combatant` regardless of what the AI had done. ModernUO moves the whole retaliation policy +into the hook — the stand-down verdict for commanded pets, the closer-aggressor swap +(`PreferCloserAggressor`), and the `IssueOrder(Attack)` vs `Combatant =` choice — and +`BaseCreature` only runs its bookkeeping (`StopFlee`, `ForceReacquire`, ethics shield) when +the hook returns `true`: + +```csharp +// RunUO — swap only; BaseCreature picks the combatant afterwards +public override void OnAggressiveAction(Mobile aggressor) +{ + if (m_Mobile.Combatant == null) + { + m_Mobile.Combatant = aggressor; + return; + } + + base.OnAggressiveAction(aggressor); +} + +// ModernUO — the AI owns the response; false = stood down, nothing else fires +public override bool OnAggressiveAction(Mobile aggressor) +{ + if (Mobile.Combatant == null) + { + Mobile.Combatant = aggressor; + } + else + { + PreferCloserAggressor(aggressor); + } + + return true; +} +``` + +An override that used to `return;` early to refuse a fight now returns `false`; calling +`base.OnAggressiveAction` runs the full stock policy, including setting `Combatant`, so an AI +that only wants the closer-aggressor swap calls `PreferCloserAggressor` instead. + ## Target Acquisition: `AcquireOnApproach` Is a Delay RunUO's `AcquireOnApproach` bool (paragon insta-aggro on approach) is now diff --git a/dev-docs/runuo-migration-docs/11-api-reference.md b/dev-docs/runuo-migration-docs/11-api-reference.md index b6c12bfe8..788a7cd78 100644 --- a/dev-docs/runuo-migration-docs/11-api-reference.md +++ b/dev-docs/runuo-migration-docs/11-api-reference.md @@ -134,6 +134,7 @@ 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 | +| `BaseAI.OnAggressiveAction(m)` (`void`) | `OnAggressiveAction(m)` (`bool`) | The AI owns the retaliation policy; `false` = stood down; swap-only overrides call `PreferCloserAggressor` | | `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 |