## 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.
The pure SerializationGenerator conversion of `BaseCreature`, split out of #2592 so it can serve as the reference for converting every other large hand-written class in the Delta Saves project (#7, phase 3). Two behaviour changes from #2592 are deliberately not here and follow in their own PRs on top of this one: `SpeedClass`, and the collapse of `ControlMaster`/`SummonMaster` into one reference (a creature can lose or keep either independently: Blade Spirits and Energy Vortexes are summoned but never controlled, EnragedCreature and talisman summons keep a summon master with neither flag set).
## What this is
- `BaseCreature` becomes `[SerializationGenerator(23, false)]` with a `[SerializableField]` per serialized slot and `[SaveFlag]` elision on nearly every field, so a stock creature serializes to its version plus flags. Field orders run 0..53 with no gaps (54 fields).
- The hand-written reader stays as `private void Deserialize(IGenericReader reader, int version)` for every pre-codegen version (0..22); post-codegen bumps use `MigrateFrom` from here on. `[AfterDeserialization]` carries the post-load fixups main did after reading (stat timers, AI type, followers, reacquire seeding).
- `ControlMaster` and `SummonMaster` stay two independent fields with main's semantics, each elided when null.
- `DamageMin`/`DamageMax`/`ActiveSpeed`/`PassiveSpeed` are no longer virtual (nothing in the tree overrode them); comments swept to the constraints that matter.
- Direct writes to serialized backing fields outside the generated setters (`SetDamage`, `SetResistance`, the move-speed helpers, the loot flag, feed loyalty, the delete timer) call `this.MarkDirty()`, matching the #2609 standard, so the class is ready for delta saves once `Mobile` is audited.
- Schema `Server.Mobiles.BaseCreature.v23.json` regenerated by the tool (a second run produces no diff).
## Deferred to follow-up PRs
SpeedClass: the serialized `_speedClass` field, `DefaultSpeedClass` replacing the type constant, `ApplySpeedClass`/`OnSpeedClassChange`, "None means custom", the four-speeds-as-one-block elision, `NPCSpeeds.FindEntry(SpeedLevel)`, the constructor fallback to Medium, and their tests.
Master references: serializing one `Master` with a `Controlled`/`Summoned` fan-out and the `SetControlMaster` lockstep.
## Tests
UOContent.Tests 776 / Server.Tests 855 green. `BaseCreatureSerializationTests` covers: a default creature elides to version + flags; a populated creature round-trips with exact byte consumption; back-to-back saves are byte-identical; an uncontrolled summon keeps its SummonMaster; byte-authentic v22 legacy streams (replicas of main's `Serialize`) load through the legacy reader for a wild tamable, a controlled pet, a controlled summon with an anchored `SummonEnd`, and a summon-master-only creature (the EnragedCreature shape); a running delete timer round-trips through `[DeserializeTimer]`; `Friends`, `CurrentWayPoint` and `HomeMap` round-trip; a `BaseVendor` stub round-trips the generated BaseVendor v2 → generated BaseCreature v23 chain.
## Behaviour notes for reviewers
- `ActiveMoveSpeed`/`PassiveMoveSpeed` getters return the raw override (0 = inherit); `CurrentMoveSpeed` is the resolved pace.
- Speeds elided as table defaults re-snap to the current `npc-speeds.json` on load, so table edits reach unmodified spawns on restart.
- `GetSpeeds` no longer throws on the save/load path when the table has no entry for the type: saves elide against the creature's own values and loads keep the stream. Construction still throws (`InvalidOperationException`, was `KeyNotFoundException`). An elided load with no table entry would otherwise resume at speed 0, so `[AfterDeserialization]` logs once and paces it at Medium.
- `virtual` removed from `ActiveSpeed`, `PassiveSpeed`, `DamageMin`, `DamageMax` (no overrides in the tree; forks may have some).
- `ControlMaster`, `SummonMaster`, `ControlOrder`, `Tamable`, `IsParagon` are `[SerializableProperty]` over hand-written setters because follower bookkeeping must run before the assignment, which a `fieldChanged` hook cannot express; the wire format is identical.
## Prerequisites for cherry-picking
#2609 (BaseVendor is already generated on top of BaseCreature) and SerializationGenerator 4.1.0.
## Summary
Adopts ModernUO.Serialization 4.0.0 across the engine. Three commits, reviewable independently:
1. **Package + tool bump to 4.0.0** (`Server.csproj`, `UOContent.csproj`, `dotnet-tools.json`).
2. **Timers → `[DeserializeTimer]`** — the 8 drifting timers (BaseLight, TreasureMapChest, MarkContainer, FillableContainer, DeathRobe, DecayedCorpse, Corpse, BaseEscortable) now store their next tick as **anchored time**: server downtime no longer consumes the remaining delay, and idle-world saves are byte-stable. This changes their wire format, so each class bumps its serialization version with a `MigrateFrom` that replays the old delta-time read through the migration schema (the new `vN.json` files carry `@AnchoredTimer`; the old ones keep `@TimerDrift`, which the generator reads forever). The 2 wall-clock timers (Aquarium, FountainOfLife) keep their exact format via `wallClock: true` — no bump. Restart methods drop their `TimeSpan.MinValue` sentinel checks: v4 invokes them **only when a timer was actually running at save**.
3. **Linkage → field-side declarations** — 175 conversions across 25 files: `[SerializableFieldSaveFlag(order)]`/`[SerializableFieldDefault(order)]` become `[SaveFlag(nameof(...), nameof(...))]` on the field, and `[SerializableFieldChanged(order)]` becomes the `fieldChanged:` argument of `[SerializableField]`. **Wire-neutral: zero migration schemas changed.**
## Verification
- Solution builds with **0 errors, 0 warnings**; all three 4.0.0 packages verified indexed on nuget.org (no local feed needed).
- **835 + 708 tests green.**
- Generated output inspected: old-version content structs replay `ReadDeltaTime` (e.g. `V3Content.DecayTimerNext = reader.ReadDeltaTime()`), current versions write/read anchored time with the gated restart, and the wall-clock classes emit byte-identical `Write`/`ReadDateTime` framing.
- Schema tool run is committed (CI's `git diff --exit-code` schema check passes): exactly the 8 expected new `vN.json` files, nothing else touched.
- The conversion was scripted with a class-scoped resolver (order → same-class `[SerializableField(order)]`/`[SerializableProperty(order)]`); it planned 175/175 with zero ambiguities before applying.
## Notes
- New `MigrateFrom`s use the content structs' provided `XxxDelay` property, matching the pre-existing idiom in Corpse's and TreasureMapChest's older migrations.
- Follow-up candidate (separate PR, wire-neutral, any time): fold the ~150 eligible hand-written `[SerializableProperty]` setters (clamps, post-change side effects) down to `[SerializableField]` with `allowFieldChange`/`fieldChanged` hooks.
Follow-up to #2574, which added a `.Migrations.cs` partial to `BaseWeapon`. Pure relocation — no behaviour change.
## The inconsistency
`BaseArmor` and `BaseClothing` already kept their pre-codegen `Deserialize(reader, version)` in a `.Migrations.cs` partial, but left the `OldSaveFlag` enum and the `GetSaveFlag` helper behind in the main class file — even though every call site is in the partial:
| Class | `Deserialize` | `GetSaveFlag` / `OldSaveFlag` | Call sites outside the partial |
|---|---|---|---|
| `BaseArmor` | already in partial | in main file | 0 of 26 |
| `BaseClothing` | already in partial | in main file | 0 of 12 |
| `BaseWeapon` | in main file | in main file | — |
`BaseWeapon` had all three still inline, with its new `.Migrations.cs` holding only a `MigrateFrom`.
## After
All three follow the same layout: `MigrateFrom` newest to oldest, then the pre-codegen `Deserialize`, then `GetSaveFlag`, then `OldSaveFlag`. That moves ~290 lines of legacy read path out of `BaseWeapon.cs` — the file that needed it most at ~3,900 lines — and leaves the main class files describing only how the type behaves today.
## Reviewing this
The diff is large and almost entirely noise, so it is probably not worth reading line by line. Two checks are stronger:
- **Nothing was lost or altered.** Across each `.cs` / `.Migrations.cs` pair, the multiset of non-blank source lines is identical to `main` except for one added comment (below). The relocation was done mechanically and asserted against that invariant rather than by hand.
- **Nothing about serialization moved with the code.** Running `ModernUOSchemaGenerator` after the move emits no new migration files.
The complete set of intentional additions:
- `using System;` in each of the three partials, for the `[Flags]` attribute (implicit usings are not enabled here).
- `// Version 9 (pre-codegen)` above `BaseWeapon`'s moved `Deserialize`, matching the marker `BaseArmor` and `BaseClothing` already carry. Version 9 is correct because `BaseWeapon.v10.json` is its earliest migration schema, so codegen began at 10.
Everything else is blank-line placement.
## Verification
Full solution builds in Release with 0 errors and 0 warnings; 1516 tests pass (815 `Server.Tests`, 701 `UOContent.Tests`).
Follow-up to #2573. That change made `SmallBOD.EndCombine` require a player-crafted item, but it could only read provenance off `BaseArmor`, `BaseWeapon` and `BaseClothing`, because those are the only three classes that track it — hence the hand-enumerated `armor?.PlayerConstructed ?? clothing?.PlayerConstructed ?? weapon?.PlayerConstructed ?? false`.
The gap is structural rather than cosmetic. `PlayerConstructed` is set inside each base's `OnCraft`, so it can only ever reach types implementing `ICraftable`. Most craftables do not — the tinkering catalogue alone is largely plain `Item` subclasses — so any rule keyed on "was this actually crafted" has nothing to key on for those types.
## What changed
Provenance moves to `Item` and is stamped centrally in `CraftItem`, immediately after the item is constructed and before the `ICraftable` dispatch, covering both the AOS and T2A craft paths. The three `OnCraft` overrides drop their now-redundant assignment and inherit `Item`'s property, so no call site outside them changes — `Resmelt` and `SalvageBag` still read `armor.PlayerConstructed` and still compile unchanged. `SmallBOD`'s three-way null-coalescing chain collapses to `item.PlayerConstructed`.
`OnCraft` is only ever invoked from `CraftItem` (the other three call sites are `base.OnCraft` chaining), so removing those assignments has no other reachable effect.
## Storage cost: none
`Item`'s `SaveFlag` word is written as a fixed-width `int`, not an encoded one, so occupying bit `0x08000000` changes no record lengths. Items that are not player-constructed serialize byte for byte as before, and crafted ones differ by a single bit in a field already being written.
`Item` itself needs no version bump: a bare `SaveFlag` bit is self-describing, so records written before it existed lack it and read `false`.
## Version bumps
The three content classes do need one, since removing a serialized field changes their layout:
| Class | Version | Field removed |
|---|---|---|
| `BaseArmor` | 9 → 10 | 24 (was last, nothing renumbered) |
| `BaseClothing` | 7 → 8 | 7 (fields 8–10 shift down) |
| `BaseWeapon` | 10 → 11 | 26 (fields 27–30 shift down) |
Each gets a `MigrateFrom` for its previous version that assigns the old bool to the inherited property, so existing crafted armour, weapons and clothing keep their provenance across the upgrade. `Item.Deserialize` runs first and reads the absent bit as `false`, then the migration overwrites it — the generated `Deserialize` calls `base.Deserialize` before dispatching, so the ordering holds. `BaseWeapon` had no migrations file and gains one.
The renumbering is not stylistic: the generator requires contiguous field ordering and rejects a hole with `SG3005: Expected field 'Crafter' with order 7 but found 8`.
New schema JSONs (`BaseArmor.v10`, `BaseClothing.v8`, `BaseWeapon.v11`) are generated by `ModernUOSchemaGenerator` and committed alongside.
## One thing worth a second opinion
The new property is a plain auto-property on `Item`, so it does not call `this.MarkDirty()` the way the codegen setters it replaces did. `MarkDirty` is currently a no-op (`// TODO: Add dirty tracking back`) and no property in `Item.cs` calls it, so this matches the file as it stands — but it is worth noting if dirty tracking comes back.
## Verification
Full solution builds in Release with 0 errors and 0 warnings; 1516 tests pass (815 `Server.Tests`, 701 `UOContent.Tests`).
Adds the four remaining named Stygian Abyss throwing artifacts as item definitions, completing the throwing artifact set (7/7 now defined in ModernUO).
## Added (stat-for-stat from ServUO)
- **Abyss Reaver** (Cyclone) — random Throwing +5..10 skill bonus, +25..35 damage, Exorcism slayer
- **Storm Caller** (Boomerang) — Hit Lightning / Hit Lower Defense, 20/20/20/20/20 elemental split
- **Banshee's Call** (Cyclone) — Hit Harm / Hit Life Leech, 100% cold, Velocity 35
- **Wind of Corruption** (Cyclone) — Hit Stamina Leech / Hit Lower Defense, 100% chaos, Fey slayer
## Acquisition — deferred (documented)
These are **`[add`-only for now**. Their OSI sources aren't in ModernUO yet:
- Abyss Reaver → the Into the Void quest (Agralem), deferred with the Abyss void-creature content.
- Storm Caller / Banshee's Call / Wind of Corruption → renowned/boss creatures (`WyvernRenowned`, `PrimevalLich`, etc.) that need a `BaseRenowned` framework port.
Per direction, the items are worth defining now; wiring their drops follows later.
## Notes
- Storm Caller's ServUO `WeaponAttributes.BattleLust = 1` is left as a `//TODO Implement BattleLust` — the attribute doesn't exist in ModernUO's `AosWeaponAttribute` set yet.
- The human Bow variant `WindOfCorruptionHuman` is archery, not throwing — intentionally out of scope.
Server builds clean.
Follow-up to the gargoyle Throwing work (#2510/#2512/#2514). Ports two Stygian Abyss creatures from ServUO so two of the throwing artifacts finally have a real OSI drop source instead of being `[add`-only.
## Changes
- **`Raptor`** and **`StoneSlith`** — ported stat-for-stat from ServUO. They **spawn automatically**: `Distribution/Data/Spawns/post-uoml/termur/TerMur.json` already contained Raptor/StoneSlith spawner entries referencing these class names, so no spawn-file edits were needed.
- **`RaptorClaw`** (Boomerang-based) and **`StoneSlithClaw`** (Cyclone-based) artifacts, dropped from each creature's `OnDeath` at ServUO's 0.5% rate (uncontrolled only).
- StoneSlith retains its `GraspingClaw` monster ability; both use `BleedAttack`.
## ModernUO idioms
Default range perception (16) / fight range, derived `GetSpeeds` (no `SetSpeed`), `[SerializationGenerator(0)]` codegen, collection-expression arrays.
## Documented omissions vs ServUO
Flagged as TODO in-code — all are content missing from ModernUO, not silent drops: Raptor's friend-spawn timer + its 25% `AncientPotteryFragments` drop; StoneSlith's `TailSwipe`, `DragonBlood`, and `SlithEye`/`TatteredAncientScroll`/`AncientPotteryFragments` drops; plus `HideType.Horned/Spined` and `PackInstinct.Ostard` (no ModernUO equivalents yet).
## Not included
The other three throwing artifacts (Storm Caller, Banshee's Call, Wind of Corruption) drop from renowned/boss creatures via a `BaseRenowned` artifact-list system that doesn't exist in ModernUO yet — a separate, larger effort. AbyssReaver stays with the (deferred) Into-the-Void quest.
Verified: server builds clean; throwing suite 14/14.
Phase 2 follow-up to #2510 — wires acquisition for the gargoyle Throwing content that Phase 1 deliberately left out.
## Changes
- **SA loot flavor (`IsStygian`)** — completes the dead loot path from the original PR: adds `SAWeaponTypes`/`SARangedWeaponTypes` pools + `isStygian` branches to `Loot.RandomWeapon`/`RandomRangedWeapon`, **and** the missing piece — computes `IsStygian` (`map == Map.TerMur`) in `LootPackEntry.Construct` and threads it through `LootPackItem.Construct`. Ter Mur creatures now roll SA gear + the three throwing weapons (Boomerang/Cyclone/SoulGlaive) from their normal `BaseWeapon`/`BaseRanged` loot entries. Conservative `TerMur`-only (not ServUO's extra `|| RandomBool()`).
- **Valkyrie's Glaive** — re-added as a self-contained Ter Mur stealable artifact at `(843, 665, 27)`, matching ServUO/OSI, with the previously-missing `ArtifactRarity => 5`.
## Deferred (documented)
The 5 host-dependent artifacts (Raptor Claw, Stone Slith Claw, Storm Caller, Banshee's Call, Wind of Corruption) are intentionally NOT included — their OSI drop sources (Raptor, StoneSlith, and the renowned/boss creatures + a `BaseRenowned` artifact-list system) don't exist in ModernUO yet, so re-adding the items now would leave them `[add`-only. They'll follow a dedicated SA-creatures effort.
## Notes
- No new tests: both changes are property/plumbing with RNG-driven output — no deterministic complex logic to assert (consistent with the repo's test-scope conventions).
- Verified: server + test project compile; throwing suite 14/14.
Supersedes #2376 (@jwvalentine). This is a reviewed, corrected, and scoped-down **Phase 1** of Joe Valentine's throwing implementation — his original commits are cherry-picked here with authorship preserved, plus a fix/scoping pass. Phase 1 lands only the **core gargoyle Throwing skill**; the incomplete content is excised for follow-up PRs (see below).
## What's included (core skill)
- `BaseThrown` combat mechanics on top of the existing skeleton: close-quarters penalty, below-min-range penalty, shield penalty, STR-scaled range, overthrow damage penalty.
- Base weapons: **Boomerang, Cyclone, SoulGlaive** (gargoyle-only), + blacksmith crafting (SA-gated).
- Two symmetric hooks on `BaseWeapon` (`ModifyHitChance`, new `ModifyDamage`) that are inert no-ops for every other weapon.
## Fixes over the original
- **Overthrow damage**: was dead code (the swing gate already guarantees you're within `MaxRange`, so the old `ComputeDamage` check never fired). Reimplemented as `finalDamage × 0.53` applied *after* all offensive bonuses via a new `ModifyDamage` hook, firing at the outer range ring.
- **`DefMaxRange`**: clamped to `[MinThrowRange, MaxThrowRange]` (uncapped before → e.g. range 13 at 200 Str) and guarded against a latent divide-by-zero.
- **Close-quarters mitigation** now uses `RawDex` (matches ServUO/OSI; deterministic under stat mods).
- **Return-throw timer** guarded against a deleted/unmapped thrower/target.
- Reverted the `MovingShot` change (it rebalanced archery — belongs in a separate PR).
- Kept only complex-logic tests (hit-chance/range/damage math); dropped property-value assertions.
## Excised for follow-up PRs (Phase 2/3)
7 named artifacts, the Into-the-Void quest + Agralem, GargishOutcast, the Bladeweaver vendor, and the SA loot tables — these were unwired/non-functional (loot never triggered, quest/creature never spawned) and will return properly wired. `StormCaller` also needs its missing Battle Lust, and the quest its correct void-creature target.
## Verification
- Build: 0 warnings / 0 errors.
- `UOContent.Tests`: **501/501** passing (the `ModifyDamage` hook causes zero regressions across all weapons).
- Full whole-branch review completed: no must-fix defects.
## Summary
Consolidates the duplicated inline AOS attribute → `ObjectPropertyList` emission that each item base copy-pastes into per-family `GetProperties(IPropertyList)` methods, mirroring the existing `AosSkillBonuses.GetProperties` precedent.
### Per-family `GetProperties(IPropertyList)`
- **`AosAttributes`** — the 24 common attributes in canonical cliloc-ascending order, with optional `damageBonus` / `hitChanceBonus` / `luckBonus` params so item-computed bonuses (e.g. `GetDamageBonus()`) stay out of the family type.
- **`AosWeaponAttributes`** — `UseBestSkill`, the `Hit*` block (1060416–1060430), `MageWeapon` (`30 - prop`), `SelfRepair`.
- **`AosArmorAttributes`** — `MageArmor`, `SelfRepair` (the always-direct members; `LowerStatReq`/`DurabilityBonus` stay inline since they're item-computed in armor but container-direct in clothing).
### Rewired all 6 `AosAttributes`-emitting item bases
`BaseJewel`, `BaseArmor`, `BaseClothing`, `BaseWeapon`, `BaseTalisman`, `Spellbook` now call the family methods instead of inlining the chain. Net: large dedup in `BaseWeapon`/`BaseArmor`/`BaseClothing`/`BaseTalisman`/`Spellbook`.
## Behavior change: tooltip line **order** (set preserved)
This is **not** a pure no-op refactor, and that's unavoidable. Today the families are emitted **interleaved in cliloc order**, and the relative order differs per item class — e.g. `BonusDex` (1060409) is emitted early in `BaseArmor` but **after** the `Hit*` block in `BaseWeapon`. No single emission order reproduces every class byte-for-byte, so consolidating into contiguous per-family blocks necessarily **de-interleaves**: lines regroup **specific → general** (family-specific, then common `AosAttributes`).
- The **set** of emitted `(cliloc, argument)` lines per item is preserved **exactly** — nothing dropped, added, or value-changed.
- Only the **order** of lines within a tooltip changes for `BaseArmor` / `BaseWeapon` / `BaseClothing`. `BaseJewel` / `BaseTalisman` / `Spellbook` were already canonical, so those are byte-identical.
## Tests
- **Golden set-invariance tests** per item base (`BaseArmor/Clothing/Jewel/Weapon/Talisman/Spellbook PropertiesTests`) — each was written to pass against current `main` **before** the rewire (locking the emitted-line set), then confirmed still passing after, proving no line is lost/added/changed.
- Family-level unit tests for each `GetProperties` (canonical order, computed-bonus folding, the `AosArmorAttributes` exclusions).
- `dotnet build` clean; full `UOContent.Tests` green. (Pre-existing `AccountPacket`/`GumpPacket`/`MobilePacket`/`ClientEnumerator` golden-test failures reproduce on unmodified `main` and are unrelated to this change.)
## Summary
Streamlines the SE-era archery ammo auto-recovery (recovering spent arrows/bolts after a miss). The mechanic was previously spread across four unrelated trigger points and a weapon-scoped timer that was divorced from the recovery state living on `PlayerMobile`. It also contained dead code.
### Problems fixed
- **Dead `!Warmode` gate** — `OnMiss` only runs from `OnSwing`, which requires warmode to fire, so the `if (!pm.Warmode)` branch that started the recovery timer could never trigger.
- **Scattered, divorced state** — banked ammo lived on `PlayerMobile.RecoverableAmmo` while the timer lived on the weapon (`_recoveryTimerToken`), and recovery was kicked off from four different places (`OnWarmodeChanged`, `PlayerMobile.OnDamage` kill, `BaseCreature.OnDamage` kill, `OnBeforeDeath`).
- **Per-player footprint** — every `PlayerMobile` carried a `RecoverableAmmo` field even though ~99% never miss with a bow (and most are offline).
### New design — `AmmoRecovery` side table
- All state (banked ammo + one repeating timer) is keyed by player in a static dictionary, so only players who actually miss carry any state. Transient by design — this was never serialized.
- **One feed point:** `OnMiss` banks the spent ammo type and starts the player's timer.
- **One drain point:** the timer self-gates and gathers ammo into the backpack only once the archer has disengaged — **alive, out of warmode, and not running** — otherwise it retries next tick, so banked ammo is never lost while online.
- **"Not running" allows standing still _or_ walking.** The `Direction.Running` bit is stale after a player stops, so it's paired with movement recency (`LastMoveTime`); only an *actively* running archer is blocked.
- Removed the redundant scattered triggers, the dead `!Warmode` branch, `RecoverableAmmo`, `RecoverAmmo()`, and the now-empty `OnWarmodeChanged` override. `PlayerMobile.OnDelete` calls `AmmoRecovery.Forget`.
### Behavior notes
- On death, banked ammo is **no longer flushed to the corpse** — it stays banked and is recovered after resurrection once the archer settles (player keeps it rather than dropping it to looters).
- `OnHit` immediate recovery (the ~40% arrow-to-defender behavior) is **unchanged**.
## Test plan
- [x] `dotnet build Projects/UOContent/UOContent.csproj -c Release` — succeeds, 0 warnings, 0 errors.
- [ ] In-game (SE era): miss bow shots, then disengage (drop warmode + stop) and confirm the "You recover N arrows/bolts" message and backpack contents; confirm recovery does **not** fire while running and **does** while walking/standing.
## Summary
- **Add a self-referencing `InterpolationHandler` to `ValueStringBuilder`** that writes directly into the builder's buffer — zero intermediate allocation, works with `stackalloc`-backed builders
- **Replace all `System.Text.StringBuilder` usage** across the codebase with `ValueStringBuilder`
- **Convert `ValueStringBuilder.Create()` to `stackalloc`** at 10 sites where output length is provably bounded
- **Convert manual `Dispose()` to `using var`** where possible, and hoist loop-scoped builders outside loops with `Reset()`
- **Convert verbose `Append()` chains to `Append($"...")`** interpolation for readability
- **Add comprehensive documentation** for string handling patterns
## InterpolationHandler Design
`ValueStringBuilder` is a `ref struct`, which creates challenges for C#'s interpolated string handler pattern:
- **`ref` fields to ref structs are not allowed** (CS9050)
- **`[InterpolatedStringHandlerArgument("")]` passes struct receivers by value**, not by ref
- **`ISelfInterpolatedStringHandler` requires boxing** ref structs into interface fields
**Solution: Copy-and-reconcile pattern.** The handler receives a value copy of the builder. The copy shares the same underlying `char` buffer (`Span` points to the same `stackalloc`/pooled memory), so writes go to the original buffer. `Append()` reconciles by `this = handler._builder`, updating `_length` and any buffer references changed by `Grow()`.
This is safe because:
- The game loop is single-threaded — no concurrent access between handler construction and reconciliation
- If `Grow()` occurs in the copy, the original's stale buffer isn't accessed until `Append()` replaces it
- `Dispose()` correctly returns the reconciled buffer to the pool
## Changes by Category
### ValueStringBuilder (`Projects/Server/Buffers/ValueStringBuilder.cs`)
- Added nested `InterpolationHandler` ref struct with copy-and-reconcile pattern
- Added `Append([InterpolatedStringHandlerArgument("")] scoped ref InterpolationHandler)` method
- Removed `RawInterpolatedStringHandler` overloads (new handler replaces them)
- All `AppendFormatted` overloads delegate to existing `Append` methods (no code duplication)
- Alignment support via direct private field access (nested type privilege)
### StringBuilder → ValueStringBuilder (15 files)
Replaced all `new StringBuilder()` with `ValueStringBuilder.Create()` or `stackalloc`:
- ConPVP games: KingOfTheHill, DoubleDom, CTF, BombingRun, TourneyMatch
- ConPVP infrastructure: Tournament, Participant, TourneyParticipant
- ConPVP gumps: ArenaGump, TournamentBracketGump, AcceptTeamGump, ConfirmSignupGump
- Commands: Handlers, Logging, Add
- Other: TownCrier, SpeechLogGump, TestCenter
Key patterns:
- `sb = new StringBuilder()` reassignment → `sb.Reset()`
- `sb.AppendFormat("{0:N0}", value)` → `sb.Append($"{value:N0}")`
- `sb.Append(x).Append(y)` chains → separate statements (VSB returns void)
### Create() → stackalloc (10 files)
Converted heap-allocated builders to stackalloc where output is bounded:
- ClientVersion (32), MapSelection (160), HouseRaffleStone (48)
- HolySense (96), UnholySense (96), ClientVerification (192)
- AcceptTeamGump (64), ConfirmSignupGump (64)
- BaseWeapon (160), BaseArmor (128)
### Loop optimizations (2 files)
Hoisted `ValueStringBuilder` creation outside loops with `Reset()` per iteration:
- TourneyMatch.cs: `using var` inside for loop → stackalloc before loop
- ArenaGump.cs: `Create()` + `Dispose()` per iteration → stackalloc before loop
### Append chain → interpolation (5 files)
Converted multi-line `Append()` chains to `Append($"...")`:
- BountyMessage.cs: title switch (6 cases), paragraph (15→1 Append), description lines, closing
- AcceptTeamGump, ConfirmSignupGump, TournamentBracketGump: tournament type strings
- AdminGump: comment/tag formatting in loops
### Documentation
- `dev-docs/string-handling.md`: Full reference — construction, interpolation, disposal, decision guide
- `dev-docs/claude-skills/modernuo-string-handling.md`: Claude skill with quick reference
- `CLAUDE.md`: Added rule 17 (no StringBuilder), dev-docs table entry, skills table entry
- `dev-docs/code-standards.md`: Updated memory management section
## Test Plan
- [x] `dotnet build` — 0 errors, 0 warnings
- [x] `dotnet test` — 940/940 tests pass
- [x] 28 ValueStringBuilder tests covering all reconciliation scenarios:
- Stackalloc no-grow, stackalloc with grow (→pool transition)
- Heap no-grow, heap with grow, heap double grow
- Pre-existing content with and without grow
- Sequential multiple `Append($"...")` calls
- Mixed plain + interpolated Append
- Empty interpolation, literal-only, format specifiers
- Null string holes, ISpanFormattable types
- Dispose after stackalloc→pool grow
## Summary
Implements T2A-accurate mechanics for the three defensive spells based on UO98 demo scripts. Pre-UOR (`!Core.UOR`) triggers the new behavior; UOR and AOS paths are unchanged.
- **Reactive Armor**: percentage-based melee damage reflection (10-35% based on target Magery), targeted, timed (25-75s). Guards exempt, ranged attacks beyond 1 tile unaffected. Reflected damage is sourceless.
- **Protection**: temporary AC bonus (casterMagery/10, 1-10 AR) via VirtualArmorMod, targeted, timed (12-120s). Shared table with Arch Protection — only one protection buff per mobile.
- **Magic Reflect**: single-use spell reflection using MagicDamageAbsorb as a flag. No timer, no DefensiveSpell lock. Consumed on first reflected spell.
- **Arch Protection**: T2A path applies Protection via shared table with area sound (0x1F7 vs 0x1ED).
- No DefensiveSpell mutual exclusion for T2A — all three spells operate independently.
## Test plan
- [x] Set expansion to T2A
- [x] Cast Reactive Armor on self → verify particles (0x376A) and sound (0x1F2)
- [x] Get hit in melee → verify attacker takes reflected damage with spark effect and sound
- [x] Get hit by ranged weapon from > 1 tile → verify no reflection
- [x] Wait for RA to expire → verify effect ends silently
- [x] Cast RA on target that already has it → verify "This spell is already in effect"
- [x] Cast Protection on another player → verify particles (0x375A) and sound (0x1ED)
- [x] Check target's AR → verify it increased by casterMagery/10
- [x] Wait for Protection to expire → verify AR returns to normal
- [x] Cast Protection on self, then cast Protection on another player → verify both succeed
- [x] Cast Arch Protection on ground near allies → verify each gets AR bonus with sound 0x1F7
- [x] Cast Arch Protection near a target already protected → verify that target is skipped
- [x] Cast Magic Reflect on self → verify particles (0x375A) and sound (0x1E9)
- [x] Have an enemy cast a harmful spell at you → verify spell reflects back, effect consumed
- [x] Cast Magic Reflect again → verify it can be recast after consumption
- [x] Cast Magic Reflect when already active → verify "This spell is already in effect"
- [x] Verify all three spells can be active simultaneously on the same mobile
- [x] Switch expansion to UOR → verify existing UOR behavior unchanged
- [x] Switch expansion to AOS → verify existing AOS behavior unchanged
## Summary
- Refactors the poison system to separate `Index` (globally unique ID) from `Level` (tier within a family), enabling multiple poison families (Standard, Darkglow, Parasitic) to coexist without collisions
- Implements Darkglow and Parasitic poison special effects from Mondain's Legacy: Darkglow boosts damage by 10% when attacker is ranged, Parasitic heals the attacker for damage dealt in melee range
- Fixes several bugs: `Register()` crashing on duplicate `Level` values across families, `IncreaseLevel()` crossing family boundaries, `InfectiousStrike` and `NinjaWeapons` stripping poison family via level-based lookups, and `ArchCure`/`CleansingWinds` using raw `Level + 1` instead of `IncreaseLevel()`
## Changes
**`Projects/Server/Poison.cs`** — Adds `PoisonFamily` enum and abstract `Family` property. Adds `Index` as unique identifier. Fixes `Register()` to check `Index` uniqueness (not `Level`) and validate the new poison's name (not the existing one's). Fixes `IncreaseLevel()` to use `Index + 1`, naturally respecting family boundaries via Index gaps. Replaces linear name lookup with `Dictionary`-based `PoisonsByName`.
**`Projects/UOContent/Misc/Poison.cs`** — Adds `family` parameter to `PoisonImpl`. Implements Darkglow effect (10% damage boost when `From` >1 tile, cliloc 1072850) and Parasitic effect (heals `From` for damage dealt within 1 tile, cliloc 1060203) in `PoisonTimer.OnTick()`. Renames `m_` fields to `_` convention.
**`Projects/UOContent/Misc/PoisonKinds.cs`** — New file. Moves poison registration out of `PoisonImpl` into `PoisonKinds.Configure()`. Adds `PoisonFamily` to Darkglow/Parasitic registrations. Provides extension properties (`Lesser`, `Deadly`, `LesserDarkglow`, etc.), `GetPoison(int level)` (standard-only), `GetPoisonByFamilyAndLevel()`, and `IsDarkglow`/`IsParasitic` instance helpers.
**`Projects/UOContent/Items/Weapons/Abilities/InfectiousStrike.cs`** — Family-aware poison scaling: Darkglow caps at Deadly (Poisoning/33.3), Parasitic caps at Lethal (Poisoning/25), Standard unchanged. Level bump uses `IncreaseLevel()` with family boundary check.
**`Projects/UOContent/Items/Skill Items/Ninjitsu/NinjaWeapons.cs`** — EvilOmen level bump uses `Poison.IncreaseLevel()` instead of `Poison.GetPoison(Level + 1)`.
**`Projects/UOContent/Spells/Fourth/ArchCure.cs`** and **`CleansingWindsSpell.cs`** — Replace `poison.Level + 1` with `Poison.IncreaseLevel(poison).Level` for family-safe cure chance calculation.
**`Projects/Server/Serialization/SerializationExtensions.cs`** — Serializes/deserializes `Index` instead of `Level`.
**`DarkglowPotion.cs`** / **`ParasiticPotion.cs`** — Point to actual Darkglow/Parasitic poisons instead of placeholder `Greater`.
**`PotionKeg.cs`** / **`BasePotion.cs`** — Adds Darkglow, Parasitic, Invisibility, and FlintsPungentBrew to `PotionEffect` enum and keg label support.
## Test plan
- [ ] `dotnet build` compiles cleanly (verified, 0 warnings 0 errors)
- [ ] Verify `PoisonKinds.Configure()` registers all poisons without throwing (Register bug fix)
- [ ] Standard poison behavior unchanged — PoisonField, PoisonSpell, SerpentArrow, SavageShaman, TrappableContainer all use `GetPoison(int level)` which now correctly filters to Standard family
- [ ] Darkglow: poison tick deals +10% damage when attacker is >1 tile away, sends "Darkglow poison increases your damage!" message
- [ ] Parasitic: poison tick heals attacker for damage dealt when within 1 tile, sends heal message
- [ ] InfectiousStrike preserves poison family and respects family-specific skill scaling
- [ ] EvilOmen + NinjaWeapons level bump stays within poison family
- [ ] ArchCure/CleansingWinds cure chance calculations work correctly across all poison families
- [ ] Serialization round-trips correctly using Index
### Summary
Fixes weapons not serializing lesser poison.
> [!NOTE]
> Dev Note: After applying this fix, fix weapons already in-game using the following command
> `[global set poison lesser where baseweapon poison = null poisoncharges > 0`
### Summary
- Fixes timer intervals not continuing
- Fixes `Timer.Index` being off by 1. Should start at 0 for the first OnTick
- Simplifies Gift of Renewal end check
- Fixes force of nature not applying at the proper time and simplifies the timer logic.
## Summary
- Removes allocation of a `List<ContextMenuEntry>` every time a context menu is created.
- Moves packet/context menu creation logic out of the core
- Fixes tame entry
## BREAKING CHANGE
> [!Important]
> **Developer Note**
> ```cs
> public virtual void GetContextMenuEntries(Mobile from, List<ContextMenuEntry> list)
> ```
> and similar functions changed to
> ```cs
> public virtual void GetContextMenuEntries(Mobile from, ref PooledRefList<ContextMenuEntry> list)
> ```
## Summary
### Changes
- Adds `[IgnoreDupe]` and `[SerializedIgnoreDupe]`
- Updates all _known_ classes that need the attribute. Some might be missing, please helps us find them!
- Adds `Item.Dupe()` command and encapsulates `CopyProperties` and `OnAfterDuped`. This is also overridable.
- Updates Dupe command to use the new logic.
- Fixes duping multiple kinds of objects that used to be outright broken.
### Bug Fixes
- Fixes issue with durability after duping
- Fixes issue with hue after duping
> [!Note]
> **Developer Note**
> Customizing how duping an item works now requires two steps:
> 1. Add `[IgnoreDupe]` or `[SerializedIgnoreDupe]` to the property/field
> 2. Add custom logic in an `OnAfterDuped` override
>
> When do you need to do this?
> *When the property being copied is not a primitive, and you need to manually deep-clone the contents of the property such as with Lists, Dictionaries, or sub classes.*
