When an AI debug-message cooldown crosses the signed tick-counter boundary, DebugInterpolatedStringHandler can emit before the deadline or suppress a message after it. Both constructors compare absolute tick values.
Use subtraction-based deadline comparisons in both constructors, following dev-docs/tick-counts.md. This is a two-line production change; message formatting, cooldown duration, and gameplay behavior are unchanged.
Adds 35 regression cases exercising the actual handler and DebugSayFormatted:
- Both constructor overloads, with debugging enabled and disabled.
- Before/at/after deadlines on positive and negative clocks.
- Future and expired deadlines across signed wraparound.
- Cooldown rearming across wraparound and buffer clearing.
- A compiler-generated interpolated call through the public extension method.
Validation on Linux / .NET 10, based on clean upstream 35e3a31b4:
- Release build: 0 warnings, 0 errors.
- New tests against unchanged production code: 28 passed, 7 failed.
- With the fix: 35 passed, including without client map data (no skips).
- Selected AI/pet suites: 134 passed / 7 failed before; 141 passed / 0 failed after.
Tests use synthetic entities; no running shard or world saves. The faulty comparisons were reverified on upstream main before submission. Windows execution remains unverified. The tests exercise explicitly assigned deadlines; deadline initialization is outside this patch.
Addresses only the DebugInterpolatedStringHandler comparison item in #2627; it does not close the other audit items.
Two bugs found while reworking #2653.
**Custom flags throw.** `SyncStaticFlag` is a switch expression with no discard arm, so any key that has no static behind it throws `SwitchExpressionException`. `[FeatureFlag mykey create <category> <desc>` crashes in `CreateOrUpdateFlag`, and once such a key is in `flags.json`, every boot's `SyncAllStaticFlags()` throws mid-iteration — caught and logged as "Failed to load feature flags", but stock flags later in dictionary order never sync. Added `_ => enabled`.
**Removing a flag turns some features on.** `RemoveFlag` hardcoded `SyncStaticFlag(flagKey, true)`. For `speedhack_detection` and `insurance` the real default is `false` / `insurance.enable`, so deleting the flag enabled the feature. It now syncs the removed flag's `DefaultEnabled`, which #2653 seeds from the static.
Verified: `dotnet build Projects/UOContent/UOContent.csproj -c Release`, 0 `CS` diagnostics (post-build copy to `Distribution/` was blocked by a running server instance).
## Summary
Profiling with thousands of creatures in range of each other showed `BaseAI.IsInvalidFactionTarget` checking Ethereal Voyage and active Honor on every acquisition candidate and then calling `BaseCreature.IsEnemy`, which checked both again. This makes `IsEnemy` the single authority and removes the duplicate per-pair work.
## Changes
- **`BaseCreature.IsEnemy`**: the Ethereal Voyage check sat below the `m is not BaseCreature` early return, so it never reached players — the only mobiles that cast it. It is hoisted next to the Honor veto. `GetMaster()` was called three times per creature pair (inside `Ethics.Player.Find(m, true)` and twice at the bottom); it is now computed once.
- **`BaseAI.IsInvalidFactionTarget`**: reduced to `IsFriend` / `IsEnemy` / `CanBeHarmful`. The removed `Combatant != m` Honor exception was dead code — `IsEnemy` vetoed Honor unconditionally on the following line.
- **`Ethics.Player.Find`** / **`TransformationSpellHelper.GetContext`**: small simplifications on the same path.
## Behavior
- `ShouldAcquireOnApproach` and `OnAggressiveAction` only consult `IsEnemy`, so movement-triggered acquisition now respects Ethereal Voyage for players (previously a player under Ethereal Voyage walking past a monster was still acquired on approach).
- `BaseFactionGuard.IsEnemy` does not call base, so faction guards no longer skip honoring/voyaging enemy-faction players. Accepted: both mechanics describe monsters, not guards.
- `HealerAI`/`BerserkAI`/`PredatorAI` (`bFacFriend`) callers are unaffected — `IsFriend` already required a `BaseCreature`, so players were excluded before these checks ran. `MilitiaFighter`/`MilitiaCanoneer` return `false` for all players, so they are unaffected too.
## Testing
- `dotnet build` clean.
- Pure predicate reorder; no new tests.
### Summary
`default-flags.json` was never shipped — `Configuration/` is gitignored — so the predefined-flag loader has been dead since #2328. A fresh shard boots with 0 flags and writes no JSON until an admin changes something, so `[FeatureList` is empty on first run.
Stock flags are now defined in code. `Initialize()` runs `Load()` first, then `LoadDefaultFlags()` seeds any of the 14 stock keys the save is missing, reading each default from the static it syncs (`ServerFeatureFlags` / `ContentFeatureFlags`) rather than a duplicated boolean — so `speedhack_detection` stays off and `insurance` honors `Insurance.Configure` (`insurance.enable`). Existing entries are never overwritten, so admin state survives upgrades and saves predating a flag pick it up. `Save()` runs only when something was seeded, so all five JSON files exist from first boot.
Verified: `dotnet build Projects/UOContent/UOContent.csproj -c Release`, 0 warnings 0 errors.
## Symptom
Since #2614, on any shard with `taming.petsStandDownOnCommand` (default `Core.ML`), every wild creature hits `"I'm being attacked but my master told me not to fight."` when struck. Brigands still chase (acquisition is a separate path in the think loop) but the retaliation path is dead: no `OnAggressiveAction`, no `StopFlee`, no `ForceReacquire`, `Combatant` never set, and `Warmode` forced off on every hit.
## Root cause
#2614 correctly added `OrderType.None` to `BaseAI.IsStandDownOrder` — a stopped pet rests on `None` and Publish 51 says it must not fight back. But the gate in `BaseCreature.AggressiveAction` never asked whether anybody could have given the order. The old predicate (`ct != Follow && ct != Stop && ct != Stay`) had only excluded wild creatures by accident: `None` was not in its set, so nothing ever needed to spell the check out. A wild creature's `_controlOrder` is always `None`.
## Fix
Both halves of `AggressiveAction` now gate on `Controlled && ControlMaster != null && Commandable` — the same predicate every order entry point already uses (`OnSpeech`, context menu, `IsValidTarget`). Only a creature somebody can command has been told to stand down. That excludes, and lets fight back:
| Creature | Why it was standing down |
|---|---|
| Wild creature | rests on `None` |
| Energy vortex, blade spirits, animated weapon, animate dead | `Summoned` with a `SummonMaster` but never `Controlled`; rests on `None` |
| Familiar, talisman summon, escortee, mirror image | `Controlled` with a master but `Commandable => false`; sits on a system-issued `Follow` |
The last row is a deliberate behaviour change from #2614 for ML+ shards: a familiar or escortee on `Follow` no longer stands down when attacked. The publish speaks of commanded pets; a creature that cannot take an order was never told anything, and pre-ML it always fought back.
Commandable summons (Summon Creature, elementals, daemons) are `Controlled` with `ControlMaster == SummonMaster`, so they stand down exactly as pets do.
## Also: stop after stay
Found while testing: `all come` then `all stop` left the pet on `Stay`, ticking *"I have been ordered to stay"*. `come` rests into `Stay` on arrival (deliberate ModernUO divergence, kept), and #2614 had `IssueStop` keep a previous `Stay`. RunUO and ServUO never consult the previous order — `DoOrderStop` is "wander around here" (or `None` pre-ML) — and Publish 51 says a stopped pet *"may wander"*. `Stay` now joins `Follow`/`Guard` in `IssueStop`: stop cancels the standing order and the pet idles anchored where it stands. `Stop_WhileStaying_RemainsStayingAtOriginalPost` is replaced by `Stop_WhileStaying_CancelsToIdleNone` plus the `come, stop` repro.
## Tests
`PetRetaliationTests` gains one case per row above: `WildCreature_`, `UncontrolledSummon_`, `UncommandableCreature_Retaliates_UnderStandDown`. Each fails on main and passes here; full `UOContent.Tests` green (1058 passed).
## Problem
`BaseFamiliar.OnThink` drove its own movement (`WalkMobileRange` toward the master) while the familiar was also a controlled pet running `Obey()`. `Summon → SetControlMaster` issues `Come`; `DoOrderCome` converts it to `Stay` within two tiles and anchors `Home`; from then on `OnThink` walked toward the caster while `DoOrderStay` greedy-stepped back toward the stale post — the backtracking. Combat never approached anything: main only copied `Combatant` while already adjacent to the caster. `CurrentSpeed = 0.01` was a 10 ms think / 50 ms step sprint hack, and `RangeCheck` teleported the familiar to a spot eight tiles *from* the caster.
## Change
A dedicated `FamiliarAI : BaseAI` (registered through `ForcedAI`, like `CloneAI`) owns every familiar decision, for both the controlled (`Obey`) and uncontrolled (`Think`) dispatch:
1. **Lifecycle** — caster gone → drop pack, delete. Caster on another map → stand down and wait for `TeleportPets`.
2. **Herding** (Dark Tides) — stands down, then `CheckHerding()`. Outranks combat.
3. **Assist** — combat-capable familiars (dark wolf, vampire bat, horde minion) engage the caster's target; otherwise anything in a fight with the caster's side — attacked the caster or the familiar, or attacked by the caster (a pet's attack is credited to the caster) — that is still fighting the caster, the familiar, or one of the caster's pets. Leashed to `RangePerception` of the caster; dropped when the caster hides. Shadow wisp and death adder never fight (`AssistsMaster => false`, enforced at the `Combatant` setter so no path can hand them a target).
4. **Follow** — `MoveTo(master, 1)` through the centralized `ApproachTarget` (greedy step / persistent `PathFollower` / stall detection).
5. **Keep-up** — snap to a validated tile beside the caster (on the caster's floor) when outpaced on open ground beyond 10 tiles, or when `ApproachTarget` gave up; never while a detour is working.
**Command immunity** is expressed inside the order machinery rather than around it: `FamiliarAI.IssueOrder` does nothing and rests the order on `Come`, so `TeleportPets` keeps working and no system-issued `Attack` (retaliation on ML's stand-down rule) can strand the familiar. `StandsDownOnCommand => false` so the ML rule never mutes it.
**Visibility** mirrors the caster from the familiar's own state (a step reveals a hidden NPC in `Mobile.OnMove`; the old cache compared the caster's previous state), `RevealingAction` is suppressed while the caster is hidden, and becoming hidden drops Warmode so no swing gives the caster away.
**Speed** is a flat 0.1 (`ReduceSpeedWithDamage => false`).
### Engine-side (all `Projects/UOContent`)
- `ApproachTarget` records which exit it took in `BaseAI.LastApproach` (`ApproachOutcome`: Arrived / Waiting / DirectProgress / Routing / Blocked / GaveUp / InvalidGoal). Callers' booleans are unchanged; keep-up reads this instead of running a second scheduler. `MoveTo`'s arrival return now also clears the move intent, as `ApproachTarget`'s own arrival does.
- `MoveToPoint(goal, range = 1)`; `CheckHerding` passes 0. **Fixes a main regression from #2591:** herding stopped one tile short, never cleared `TargetLocation`, and left the creature pinned to the herding pace — affects the shepherd's crook and the Dark Tides scroll fetch for every herded creature, not just familiars.
- `ChangeAIType` reads `ForcedAI` once. It read it twice, and each `BaseAI` ctor activates its timer for a non-sector-gated creature, so a `ForcedAI` creature with `PlayerRangeSensitive => false` got an orphan AI ticking it.
## Tests
`FamiliarAITests` are timer-wheel driven (the real `AITimer` thinks and moves; `PetPacingTests` style) against live Trammel statics, gated on client map data: follow without backtracking, the five-way assist theory, leash, retaliation, aggressor fallback (caster's own `Combatant` expired; caster's pet in the fight), target dropped when it stops fighting, keep-up on open ground / not while routing / after give-up, hidden mirror across steps, herding priority with a visible fighting caster, stand-down when left behind, no stale move intent. `ApproachOutcomeTests`, `HerdingTests` (fails on main), `ForcedAITests` (fails on main) cover the engine-side pieces.
Against `origin/main` with the familiar tests dropped in: 16/16 fail, including the reported backtracking. On this branch: `UOContent.Tests` 1082 passed / 2 skipped, `Server.Tests` 891/891, solution builds with 0 warnings.
## Symptom
`[set TargetLocation (x, y)` (quoted or not) answers **"That is not properly formatted."**, and in `[props` the `>` next to `TargetLocation` does nothing when the value is null — which is its normal idle state (`BaseAI.cs:650` clears it).
This looked like a `Point3D` parsing regression from #2624/#2625, but `Point3D`/`Point2D`-typed properties (`Location`, etc.) were never affected. The only `[CommandProperty]` in the tree declared as an **interface** is `BaseCreature.TargetLocation : IPoint2D` (`BaseCreature.cs:1111`), and both code paths only knew the structs. `git log -S"IPoint"` over the parser and gump files hits nothing but the initial import — the gap is inherited from RunUO, not recent.
## Root cause
- **`[set`** — `Types.TryParse` has no branch for `IPoint2D`/`IPoint3D`. An interface has no static `Parse`, so `GetParseMethod` returns null and the value falls into `Convert.ChangeType("(x, y)", typeof(IPoint2D))`, which throws → "not properly formatted".
- **Props gump** — `PropsGump` routes on `obj?.GetType() ?? prop.PropertyType` (since #2180). With a null value the type is `IPoint2D`; `Point2D.IsAssignableFrom(IPoint2D)` is false, no branch matches, and the click is inert. It only worked when the slot already held a `Point2D`, because the runtime type is then the struct.
## Fix
- `Types.TryParse`: `IPoint3D`/`IPoint2D` targets resolve to the concrete struct — `Point3D` first, then `Point2D` for an `IPoint2D` target (a 3-tuple is a valid `IPoint2D`). `(-null-)` still clears; the existing null branch runs first.
- `PropsGump`: the interface types route to `SetPoint3DGump`/`SetPoint2DGump`. The entity branch stays ahead of them — `TargetLocation` legitimately holds a Mobile too (`ShepherdsCrook.cs:148`, herding toward the shepherd), and that case still opens `SetObjectGump`.
- `SetPoint2DGump`/`SetPoint3DGump`: seed the text entries from `value is IPoint2D/IPoint3D` rather than a hard cast, so a `Point3D` sitting in an `IPoint2D` slot cannot `InvalidCast`.
## Not covered
`[set TargetLocation 0x40001234` (assigning a mobile by serial) still reports "not properly formatted" — the entity branch keys on the *target* type being `IEntity`, which `IPoint2D` isn't. Real state, but niche; left out to keep this to the reported symptom.
## Testing
Five cases in `InterfacePointParseTests`, watched fail before the change (three returned the error string; two pin existing behaviour that must survive): tuple → `Point3D` for both interfaces, pair → `Point2D`, pair rejected for `IPoint3D`, `(-null-)` clears.
`dotnet build` 0 warnings. **1059 UOContent** and **891 Server** tests pass, 0 failures. The gump routing is a one-line branch with no automated test — needs an in-game check: `[props` a creature with a null `TargetLocation`, press `>`, expect the Point2D editor.
## Summary
Two small additive changes a derived spawner needs.
- **`BaseSpawner.OnTick()` is now `virtual`.** A subclass that gates spawning on external state (time windows, event triggers) must gate *timer* spawns without gating the manual `Spawn()` API, and `OnTick` is the only place the two paths differ: it is the timer callback and `Spawn()` is both what it calls and what commands and scripts call. Cost: one virtual dispatch on the existing timer callback; no change to stock behaviour.
- **`group` in the JSON DTO.** `BaseSpawner.Group` (all dead, then respawn) is binary-persisted but was missing from `SpawnerDto`, so it did not survive export/import. Added to the abstract record after `spawnLocationIsHome`, assigned in `ApplyDto` after `InitSpawn` (which resets it), and exported by the three stock `ToDto` implementations.
## Test plan
- [x] A derived spawner overriding `OnTick` with a closed gate: `OnTick()` spawns nothing; manual `Spawn()` still spawns and does not pass through `OnTick`.
- [x] DTO round trip with `Group = true` carries `group` and restores it on `ToSpawner()`.
- [x] `UOContent.Tests` full suite green.
- [ ] CI
## Summary
Two small additive changes that an external content assembly (ModernSpawner) needs, as separable commits.
**1. `SkillEvents.SkillUsed`** (`Projects/UOContent/Skills/SkillEvents.cs`, namespace `Server.Misc`): a plain C# event `Action<Mobile, Skill, bool success>` raised once per skill attempt from each of the four `Mobile_SkillCheck*` handlers, with the handler's own result. Attempts the handler resolves without a roll (too difficult, no challenge) raise too, so a grandmaster's trivial success and a guaranteed combat roll are observable. Not raised when the mobile lacks the skill. Each handler keeps its logic in a private core method and raises on the way out, so there is exactly one raise per attempt and `CheckSkill` itself is unchanged.
- **Why a plain event and not a `[GeneratedEvent]`:** generated events are compile-time static dispatch inside the UOContent compilation, so a subscriber in another assembly cannot use `[OnEvent]`. Shape follows `HelpEvents`.
- **Why "used", not "gained":** this is the XmlSpawner skill-trigger semantic (it wrapped the same four handlers and passed their result as `success`; its grammar was `Skill[+/-]` for success-only or failure-only). Gains are already observable through the existing skill-change notification on `Mobile`.
- **Cost:** one delegate null-check per attempt when nothing is subscribed; no boxing, no closure, no allocation. The handlers sit on the combat swing path.
- **Exception contract:** subscriber exceptions propagate, matching `EventSink`/`HelpEvents`; no try/catch by design.
**2. `InternalsVisibleTo("ModernSpawner.Tests")`** on `Server.csproj`, beside the existing `Server.Tests`/`UOContent.Tests` entries, so an external test host can seed `Core._now` the way the engine's own test initializers do. Separable; a public test seam on `Core` would serve the same need without naming a downstream assembly.
## Open question
The payload is the `Skill` object plus a positional `bool`. A `readonly struct` args type passed `in` would leave room to add `chance` or the target later without breaking subscribers. Happy to change before merge.
## Test plan
- [x] `UOContent.Tests`: 4 tests — a rolled attempt raises once with the returned outcome; each short-circuit path (no challenge, too difficult, on both the direct and value-window handlers) raises with the handler's result; a direct `CheckSkill` call does not raise; no subscriber does not throw. Full suite green.
- [x] `Server` and `UOContent` build clean with `TreatWarningsAsErrors`.
- [ ] CI
