- ConsoleInputPump gains an optional ILogger so the reader-fault and
command-dispatch catch blocks log warnings/errors again, matching the
behavior the old ConsoleInputHandler had before the pump refactor.
- Remove ConsoleInputPump.Stop() — dead code, nothing calls it; the reader
thread already terminates on EOF/Core.Closing/finally cleanup.
- GetInputCommand no longer clears the help-description cache on every
lookup; RegisterCommand/UnregisterInputCommand still invalidate it
correctly on the paths that actually change the command set.
Data-gating prompts (delete corrupt entities/files) and first-boot config
prompts (data dir, listeners, server name, expansion, maps) now call
ConsoleInputHandler.ReadLine() instead of Console.ReadLine() directly, so a
headless server crashes fatal with a clear message instead of reading null.
ConsoleInputHandler.Initialize() now checks Core.Headless and skips
starting the input thread entirely when stdin isn't a TTY, logging
instead. When not headless, it delegates the read loop to
ConsoleInputPump (Task 2) rather than running its own async void
loop with a two-AutoResetEvent handshake.
ReadLine() throws HeadlessConsoleInputException when headless, falls
back to a direct Console.ReadLine() during early startup or after the
pump loop has ended, and otherwise blocks on the pump.
Wrap the Run() loop in try/finally so cleanup (_running = false;
ReleasePendingPrompt(null)) always executes, even if the injected
lookup throws. Previously a throwing lookup escaped the loop and left
any pending ReadLine() caller blocked forever. Also fold the lookup
call into the same try/catch as the handler invocation so a bad
command is swallowed without tearing down the reader loop.
Document via XML comments that ReadLine() supports a single sequential
caller (not concurrent callers) and that command handlers dispatched
by Run() execute on the reader thread and must not call ReadLine().
Add a regression test proving a throwing lookup neither hangs a
pending prompt nor kills the reader loop. Also drop an unused
System.Text using from the test file.
## The bug
#2522 rewrote the outgoing huffman table in `NetworkCompression.cs` and transposed symbol `0x19`'s code from `0x1CE` to `0x12E` (both 9 bits, so the length distribution — and the Kraft sum — stayed valid, which is why nothing obvious tripped).
The real damage is that it broke prefix-freeness. `0x12E` is `100101110`, and symbol `0x0D`'s 8-bit code is `10010111` — a proper prefix of it. The client's decoder walks the tree bit by bit, so it hit a valid leaf at `0x0D` after 8 bits, emitted the wrong byte, and then reframed every subsequent code.
That is exactly what the reporter's capture shows. Server sends `BF 00 0C 00 19 02 00 00 00 01 00 00`; the client's post-decompression stream reads `BF 00 0C 00 0D 55 00 00 01 00 00` — the literal `0D` is the mis-decoded `0x19`, and the packet is now one byte short, so framing desyncs from there on.
## Impact
Any outgoing packet with byte `0x19` anywhere in its body (serials, coordinates, hues, lengths, text) corrupted the stream. Because the desync is in framing rather than a single field, the client silently stops applying server updates while still being able to send — no disconnect, no error.
`StatLockInfo` (`0xBF` subcommand `0x19`) is sent during login, so it reproduces on essentially every connection. This is also #2526: "can only walk a few steps, then the client stops responding" is the same desync, not a VPS sizing problem.
## Fix
One entry, restored to the canonical value:
```diff
- 0x9, 0x191, 0x9, 0x12E, 0x7, 0x03F, ...
+ 0x9, 0x191, 0x9, 0x1CE, 0x7, 0x03F, ...
```
## Validation of the whole table
Rather than eyeball 257 entries, I diffed the current table against **every revision of it in this repo's history** — all 34, back through the renames to the original import. All 34 agree with each other, and `0x19` is the sole disagreement with #2522's rewrite. No other entry has ever changed.
I also validated the table structurally: all 257 lengths in `[2,11]`, every value fits its declared bit-length, Kraft–McMillan sum exactly 1, and no code is a prefix of any other. It passes on all counts now, and the prefix check is what located the bug in the first place.
Both checks were one-off validation scripts, not committed — see below.
## Test
A single known-answer test (`~10ms`) that compresses all 256 symbols and asserts the exact output bytes. The expected bytes were generated from the canonical table, *not* from the implementation, so the test isn't circular. Any single wrong table entry changes the output, so it pins all 256 entries plus the terminal code, and it exercises the encoder end to end.
A round-trip test would **not** catch this class of bug — encoder and decoder built from the same table agree with each other even when the table is wrong. The contract being violated is with the client's hard-coded tree, so the expected bytes have to come from outside the implementation.
The structural prefix-free check and a second `StatLockInfo` vector were deliberately dropped after they'd served their purpose: the table is now verified and effectively frozen, so the structural check was guarding a constant, and the `StatLockInfo` vector is a strict subset of the all-symbols one. What remains covers the risk that's still live — `Compress` is a hand-unrolled bit-packing loop that will get optimized again, and this is the guard against that rewrite silently corrupting the wire format, which is precisely what happened here.
Verified the test fails when the bug is reintroduced and passes when fixed. Full `Server.Tests` suite green: 727 passed.
Started as an allocation pass over `StepCache` and grew into a cleanup of the surrounding pathing engine. Four commits, each independently reviewable; net **−560 lines**.
Build clean (0 warnings). All 122 `Server.Tests.Pathfinding` tests pass.
---
## 1. `perf`: pool the strata buffer, cut a hot-path dictionary lookup
**The headline is that `TryGetMask` — the actual hot path — was already allocation-free.** `StepMask` is a readonly struct, `StaticTileEnumerable` is a `ref struct`, `ChunkMissState` is a struct in a `Dictionary`. So most of this is a bake-throughput and GC-churn win, with one exception noted below.
`BuildChunk` accumulated packed multi-Z strata into a `List<byte>` that grew by doubling (256 → 512 → 1024 → …) and then paid a final `ToArray()`. A full map bake runs it ~114k times. It now writes into a `byte[]` rented from `STArrayPool<byte>.Shared` through a span writer, and hands the chunk one exact-size copy.
**This required fixing a latent out-of-bounds guard.** The record-fit check reserved headroom for **8** strata (`StratumByteLength * 8`) while `ComputeStandableSurfaceZs` can return up to **16** — so a cell could write 305 bytes starting from a 65,383-byte offset. Against a `List` that was benign (it just grew past 64 KB, and emitted offsets stayed under the `NoStrata` sentinel). Against a fixed-size rented buffer it is an out-of-bounds write, so tightening it was a *prerequisite* for the pooling, not a drive-by. The guard is now exact, which additionally proves no emitted offset can collide with `NoStrata == ushort.MaxValue`.
**One genuine query-path win:** `ShouldPromoteAfterMiss` did *two* dictionary lookups per miss — a `TryGetValue`, then an indexer assignment that re-hashes and re-probes. It now mutates in place via `CollectionsMarshal.GetValueRefOrNullRef`. This runs on every uncached chunk touch during A* expansion. The window-expiry branch keeps its explicit early return, so `MissPromotionThreshold == 1` still resets rather than promoting.
Also dropped `StepProbe.ComputeStrataAt` / `ComputedStratum` (dead code, zero callers) and collapsed six 18-argument `new StepMask(0, 0, …, kind)` blocks into `Fallthrough(kind)`.
**Considered and rejected:** pooling the `Direction[]` that `Find` returns. It *escapes* the call — `MovementPath` holds it across ticks while `PathFollower` walks `m_Index` through it — so it cannot be rented-and-returned, and it cannot be borrowed from the shared `BitmapAStarAlgorithm.Instance` without one creature clobbering another's in-flight path. `CheckPath` rate-limits repaths to one per 2s per creature, putting this at roughly 60 KB/sec at 1,000 pathing creatures. Not worth a public API break plus a use-after-return footgun.
## 2. `docs`: rewrite the comments for publication
The comments had accumulated as development notes: internal phase jargon (`Tier 4`, `the Phase-2 synthesizer`), change narration aimed at a reviewer (`which the old ComputeStandingZ anchor missed`, `legacy behavior`), benchmark anecdotes (`benchmarked as near-optimal`, `a ~20 ns lookup`), and paragraphs restating the code.
Rewritten to keep the rationale you cannot recover by reading the code — why the source-Z guard cannot be widened, why multis fall through with a halo, why the promotion gate counts Finds rather than calls, why `ComputeFingerprint` must hash the *files* and not the live tile tables — and drop the history that got us there.
Three comments were **factually wrong**, not just wordy:
- `CacheEvictionTimer` and `CacheStats` documented a class called `StaticWalkabilityCache`. No such class exists — it is `StepCache`.
- `StepCacheFile` declared `File layout v8` while `FormatVersion` is 9, and called the current record layout "the v6 layout" in four places. The layout descriptions are now unversioned so they cannot drift again.
- `StepProbe.ComputeStandingZ` claimed `StepCache` uses it to bake `SourceZ`. It has not since the baker moved to the clearance-aware `ComputeStandableSurfaceZs`; only a parity test calls it.
## 3. `refactor`: simplify `StepCacheFile.Write`, consolidate the format tests
`SaveToFile` walked `_keysList` **twice** — once to count the map's chunks, then again through a `ChunkEnumerator` closure to emit them — because `Write` needed the count up front to size its index array. Both loops had the same root cause. Passing a **span** collapses them: the count is just `span.Length`.
That deletes the `ChunkEnumerator` delegate, the closure over the list enumerator, and **both `InvalidOperationException` throws**, which existed only to police the delegate's "yield exactly `chunkCount` chunks" contract — a contract a span makes unrepresentable.
`Write` now patches the header's `IndexOffset` by seeking back to it rather than reaching into the writer's live buffer with `BinaryPrimitives`. That also retires `IndexOffsetFieldPosition`, a hand-maintained byte offset that had to track the header layout, and sidesteps the stale-array hazard that motivated the manual patch (`BufferWriter` reallocates on growth).
**Tests:** `StepCacheFileV6/V7/V8Tests` were named for the format version that introduced each transform — and the format is now **v9**, so all three names described formats the loader rejects outright. Beyond triplicated builders and plumbing, two things were actually broken:
- The three near-identical rejection tests each cited a `MinSupportedVersion` that had since moved (`"version 5 < MinSupportedVersion 6"`, `"6 < 7"`, `"7 < 8"`). They passed for the wrong reason.
- `AssertBaseEqual` (used by V7 and V8) **silently skipped the swim and strata trailers**. A regression dropping either would not have failed those tests.
Now one `StepCacheFileFormatTests`, named for behavior — predictive-Z elision, compression, compact index — with a single `AssertIdentical` that does check both trailers, the three rejection tests folded into one theory that also covers a future version, and a zero-chunk case the delegate-based writer never had coverage for.
## 4. `test`: consolidate the parity and lifecycle tests
Three files tested "parity" and none of the names said *which*. They were three different layers, and the seams are the useful part, so they are now one `StepCacheParityTests` that names them:
| Test | Compares | Answers |
|---|---|---|
| `ProbeMatchesSlowPath` | StepProbe vs MovementImpl | Is the bake right? |
| `CacheMatchesProbe` | StepCache vs StepProbe | Is it stored and returned intact? |
| `CacheServesReachableWalkStates` | StepCache vs MovementImpl | End to end, over the states A* visits |
Merging removed a duplicated stub `Mobile`, duplicated region seeds, and a filename/class mismatch (`StepProbeParityTests.cs` declared `StaticWalkabilityParityTests`). `SwimBake_ProducesWetCells` moved with it — it lived in the cache parity file but never touched the cache.
Tests reached into `StepCache._chunks` via `GetField` in **9 places**, each rebuilding the key encoding and cell-index arithmetic by hand. `StepCache` now exposes `GetResidentChunk` and `ResidentIndexInSync` alongside the internal test hooks it already had (`LazyReaderHasChunk`, `CurrentFindGeneration`), and the shared arithmetic moved to `PathingTestSupport`. All 9 reflection blocks are gone.
`StepCacheLifecycleTests` is regrouped by what it covers — promotion gate, fallthrough routes, strata, swim layer, eviction — with the `Tier4*` names dropped. Removed `Singleton_IsAvailable`, which asserted an inline-initialized static property was not null; that is the entire 123 → 122 test-count delta.
---
## Verification
Tests were mutation-checked rather than just run, since round-trip and parity tests can pass while a transform silently no-ops:
- Injecting an off-by-one into the `IndexOffset` patch fails **15 of 123** — the format tests are load-bearing.
- Offsetting the cache's cell index by one fails **7 of 10** parity cases, and the 3 that stay green are exactly the ones that do not touch the cache. The layering localizes a fault rather than just reporting one.
## Summary
`[AddonGen` currently produces addon scripts that **do not compile**, plus a few
gather-logic and UI bugs. This fixes all of them.
## Compile-breaking (verified)
Every generated addon failed to build because of the item-component emission path:
- **Trailing comma + missing semicolon.** Items were emitted as a multi-line
`AddComponent(\n … ,\n)` — a trailing comma in the argument list and no terminating
`;`, i.e. `CS1525: Invalid expression term ')'` and `CS1002: ; expected`.
- **`Deed` missing `new`.** The template emitted
`public override BaseAddonDeed Deed => {name}AddonDeed();` — invoking the type as a
method (`CS1955: Non-invocable member … cannot be used like a method`).
Both are now fixed; components are emitted on a single line matching the existing
static-tile path:
```csharp
AddComponent(new AddonComponent(3215) { Light = LightType.Circle300, Hue = 5 }, 2, 3, 5);
```
**Verification:** compiled the generator's *output* (a representative two-component addon —
one plain, one hued + light-source) before and after the change against minimal
`BaseAddon`/`AddonComponent` stubs:
- Before: `CS1525` + `CS1002` (item path), and `CS1955` in isolation for the `Deed` line.
- After: **Build succeeded.**
`Projects/UOContent` also builds clean with the source change.
## Gather-logic + UI (reasoned from the code, not runtime-tested)
- **Inverted Z-range guards.** The tile/item scan used `if (range && …)`, so with the range
filter off (the default) map tiles and items were never captured — inconsistent with the
Static pass's `if (!range || …)`. Corrected to match.
- **"Export Items" was dead unless "Export Statics" was also checked** — the items scan was
nested inside `if (statics)`. Items now scan independently. Placed `Static` items are
skipped in this path because they're already captured (with hue/light) by the
`GetItemsInBounds<Static>` pass, which also removes a pre-existing double-count.
- **Swapped gump Min/Max labels** — the "Max" label sat over the Min entry and vice versa.
## Notes
The three gather/UI fixes are reasoned from the code rather than exercised through the
in-game gump, so they're worth a close look in review. The compile fixes are the headline
and are output-verified.
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.
The Throwing skill shipped with `StrScale`/`DexScale`/`StatTotal`/`StrGain`/`DexGain` all `0` in `skills.json`, so training it never raised Str or Dex — unlike every other weapon skill.
Fills those in by mirroring **Archery** (the Dex-primary ranged analog, which matches Throwing's existing `PrimaryStat: Dex` / `SecondaryStat: Str`): `StrScale 0.025` / `DexScale 0.075`, `StatTotal 10`, `StrGain 0.25` / `DexGain 0.75`.
Data-only change.
## Problem
Three coupled issues, each hiding the next:
1. **CI passed despite failing tests, with no test logs.** ([example run](https://github.com/modernuo/ModernUO/actions/runs/28639143286/job/84931544255) — the `Test` step produced zero output and the job went green.)
2. **Two `EmitsLowerStatReqWhenPassed` tests** fail with `KeyNotFoundException: '1060435'`.
3. Once CI actually ran the tests, **~337 UOContent tests failed** with `FileNotFoundException: tiledata.mul was not found` — the test bootstrap force-loaded copyrighted client data that CI doesn't have.
## Root causes & fixes
### 1. CI ran zero tests (`fix(ci)`)
The `Test` step ran `dotnet test --no-restore`, but the `Build` step only restores/builds `Application` — never the test projects. Without a restore, the test projects have no `project.assets.json`, so `Microsoft.NET.Test.Sdk`'s targets aren't imported, they aren't recognized as test projects, and `dotnet test` runs the `VSTest` target against **zero** projects → no output, **exit 0**.
- Both jobs now run `dotnet test --logger trx --results-directory ./TestResults` (test projects restore and run) **plus a guard** that fails the job if no `.trx` is produced — a permanent backstop against silent zero-test passes.
### 2. Impossible OPL tests (`fix(ci)` + `test(opl)`)
#2501 deliberately emits `LowerStatReq` (`1060435`) **inline in each item**, not in `GetProperties`. A follow-up "fix" dropped the `lowerStatReq:` argument to make the tests compile but left the assertions expecting `1060435`.
- Removed the two impossible tests, then removed the **entire `Tests/PropertyList/` OPL attribute set** from #2501: these assert exact cliloc/value/order of OPL emission per item base — a one-time proof of the #2501 rewire, now a permanent tax on modding (any admin reorder/value change/added line reddens the build). The one non-trivial case (LowerStatReq) is what just broke, because the test was wrong. Inline emission stays covered by the `BaseArmor`/`BaseClothing` tests.
### 3. Tile-data-dependent tests crashed CI (`test(uocontent)`)
UOContent.Tests' collection-fixture constructor force-loaded `tiledata.mul` unconditionally. On CI (no client files) it threw, and xUnit failed **every test in the collection** with the same error — mostly collateral (packet/scheduler/spawner tests that don't need tile data).
- Mirror Server.Tests' graceful pattern: `TestServerInitializer` probes for `tiledata.mul` and only loads tile/multi data (and runs the tile-dependent configure steps) when present, exposing `TileDataLoaded` so the fixture no longer throws.
- Add a shared `TileDataRequirement.SkipIfMissing()` guard and apply it to exactly the **31** pathfinding/multi/AI tests that genuinely need real tile data (`[SkippableFact]`/`[SkippableTheory]`).
## Verification (all local)
| Scenario | Server.Tests | UOContent.Tests |
|---|---|---|
| **Client data absent (CI)** | 726 pass, 17 skip, **0 fail** | 469 pass, 32 skip, **0 fail** |
| **Client data present (dev)** | 726 pass, 0 skip, **0 fail** | 501 pass, 0 skip, **0 fail** |
- Full `dotnet test` exits **0**; TRX files produced; the no-test guard trips (exit 1) only when zero `.trx` are produced.
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.
## At a glance
```csharp
public override void GetProperties(IPropertyList list)
{
base.GetProperties(list);
// Accumulate any number of free-text lines. On dispose the block flushes via
// AddChunked, splitting across as many OPL properties as needed so none can
// overflow the legacy 2D client's per-property buffer (which would crash it).
using var block = list.TextBlock();
if (luck > 0)
{
block.Add($"Luck Bonus: +{luck}%"); // zero-alloc interpolation
}
block.Add("Cannot be repaired".AsSpan()); // plain text, no string allocation
// Already holding a '\n'-joined string? Skip the builder and chunk directly:
// list.AddChunked(description);
}
```
## What
Adds a safe path for emitting **variable-length, free-form (non-cliloc) tooltip text**:
- **`ObjectPropertyList.Add(ReadOnlySpan<char>)`** overloads — append raw text with no string allocation. Makes the old single-arg `Add(string)` redundant (a `string` binds to the span overload implicitly), so it's dropped.
- **`AddChunked(ReadOnlySpan<char>)`** on `IPropertyList` — splits newline-joined text at `\n` boundaries across as many passthrough-cliloc properties as needed, so no single property exceeds the cap.
- **`OplTextBlock`** (`ref struct`) + **`IPropertyList.TextBlock()`** — an ergonomic builder that accumulates `\n`-joined lines (with a zero-alloc interpolated `Add($"...")` overload) and flushes via `AddChunked` on dispose. Usage: `using var block = list.TextBlock();`.
- **`MaxArgumentLength` (504)** — per-property cap with a hard backstop that clamps + logs anything that slips through.
## Why
The legacy 2D client copies each OPL property's text into a fixed ~512-char (1024-byte) buffer. A single property longer than that smashes an adjacent world object's vtable on the client heap and crashes the client. `AddChunked`/`OplTextBlock` keep multi-line content safely under the cap instead of risking one oversized `Add`.
## Docs
- `dev-docs/property-lists.md` — new "Multi-Line Free Text" deep-dive section; corrected the stale `IPropertyList` listing.
- `dev-docs/claude-skills/modernuo-property-lists.md` — condensed pattern + anti-pattern.
## Tests
9 tests pass (`OplTextBlockTests`, `ObjectPropertyListSpanAddTests`): line joining, empty-line skipping, no-line no-op, zero-alloc interpolation, and long-content chunking staying under `MaxArgumentLength`. Full `UOContent` build is green, confirming dropping `Add(string)` breaks no call sites.
## 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
`Rectangle3DConverter.Write` corrupts a rectangle's Z range for certain bounds. The omit-z guard was:
```csharp
var writeZ = value.Start.Z is > sbyte.MinValue and < sbyte.MaxValue
|| value.End.Z is > sbyte.MinValue and < sbyte.MaxValue;
```
This drops `z1`/`z2` whenever **both** Z bounds sit at/outside the sbyte extremes — but `Read` reconstructs absent Z as exactly `z1 = -128, z2 = 127` (depth 255). So any rectangle that trips the omit condition without *being* that sentinel round-trips to depth 255 and is corrupted.
The concrete case: a **homeRange-style** bound `Start.Z = -128, End.Z = 128` (depth 256, the full vertical range used by spawners) gets `z1`/`z2` omitted on write, then reads back as depth **255** — silently losing the top z-level on every re-serialize.
(Spotted while working on #2505; spawners now prefer the `homeRange` form so most square bounds avoid this path, but any non-square `spawnBounds` or other `Rectangle3D` JSON is affected.)
## Fix
Omit Z only for the exact sentinel `Read` produces (`z1 == -128 && z2 == 127`); write it for anything else:
```csharp
var writeZ = value.Start.Z != sbyte.MinValue || value.End.Z != sbyte.MaxValue;
```
`Read` is unchanged, so existing `regions.json` rectangles that omit Z keep loading identically.
## Tests
New `Rectangle3DConverterTests`: round-trips the homeRange depth-256 case, the depth-255 sentinel, and ordinary/edge z values; asserts the sentinel omits Z while homeRange bounds write it. Server.Tests 717/717, UOContent.Tests 487/487.
## Summary
Removes the dated `DynamicJson` JSON helper and migrates spawner JSON (de)serialization to a polymorphic `record SpawnerDto` hierarchy. `DynamicJson` was the last remaining consumer (regions moved off it in #1400).
The key correctness improvement: **System.Text.Json deserializes plain DTO records, never a live `Item`.** Previously, deserializing directly into an `Item` meant STJ constructed world-registered objects *before* the data was validated — a malformed/hand-edited spawn file could leave orphaned spawner Items in the world save. Now a parse failure is GC-only; `dto.ToSpawner()` constructs the spawner only from a fully-validated DTO and self-cleans on failure.
## What changed
- **New:** `SpawnerDto` (abstract) + `SpawnerDataDto` / `RegionSpawnerDto` / `ProximitySpawnerDto`, each marked with a reusable `[JsonDiscoverableType]` opt-in attribute. Auto-discovered at the `Configure` phase — no manual registration list (avoids the regions `Register<T>()` footgun), open to custom spawner subtypes.
- **Symmetric mapping:** `BaseSpawner.ToDto()` (export) ⇄ `SpawnerDto.ToSpawner()` (import). `ToSpawner()` deletes-and-rethrows on any failure, so the importer can never orphan an Item.
- **`SpawnerJsonSerializer`** wires `$type` polymorphism on the `SpawnerDto` root with loud collision/constructibility validation.
- **Import/export commands** rewired to the typed DTO path (reflection `FindTypeByName`/`CreateInstance` removed).
- **Data migration:** the 109 `Distribution/Data/Spawns/**` files moved from `"type"`→`"$type"` and legacy `homeRange`→`spawnBounds`. The runtime still *reads* legacy `homeRange` for external files. The `homeRange→spawnBounds` formula is proven equivalent to the runtime conversion (`BoundsEquivalenceTests`, hr=0/1/3/7).
- **Deleted:** `Projects/Server/Json/DynamicJson.cs`.
Sparse export output matches the legacy `ToJson` (nullable DTO properties + `WhenWritingNull`). Binary world-save serialization is untouched.
## Tests
- DTO round-trip per spawner type; sparse-default omission; legacy `homeRange` read; export/import file round-trip.
- `Import_MalformedFile_LeaksNoWorldItems` — proves a mid-array parse failure constructs zero world Items.
- `AllSpawnFilesLoadTests` — every migrated spawn file deserializes and builds.
- Duplicate-discriminator validation.
- UOContent.Tests 485/485, Server.Tests 710/710, build clean.
## Follow-up (not in this PR)
`Rectangle3DConverter.Write` (in `Projects/Server/`) omits `z1/z2` when `Start.Z == -128`, so a future server-side export of a `homeRange`-style spawner round-trips depth 256→255. Pre-existing and out of scope here (Server change); the migrated data reads correctly. Worth a separate small converter PR.
## Problem
A creature equipped with two items that resolve to the **same layer** can crash the legacy EA 2D client (use-after-free). Equipment `Layer` comes from tiledata (`Layer = (Layer)ItemData.Quality`), so a **two-handed weapon and a shield both resolve to `Layer.TwoHanded`**. `Mobile.FindItemOnLayer` even documents the invariant: *"We only allow 1 item per layer. Its an implicit contract."*
## Root cause
- `SendMobileIncoming` (0x78) **dedupes by layer** (the `layers` span) and sends only the first item per slot — so a static creature is fine.
- But the per-item **`SendEquipUpdate` (0x2E)** and the item **OPL** sends do **not** dedupe. On any equip/property delta (`Item.ProcessDelta`) they fire per item and leak the second same-layer item on its own.
- The client then holds two items on one equipment slot; when that slot is torn down (e.g. a large group of such creatures and the player runs out of range → mass remove) the legacy 2D client frees one and dereferences it → UAF. ClassicUO bounds-checks and is unaffected, but the server is emitting an invalid, self-contradictory stream either way.
## Fix
Make per-item equip/OPL sends honor the same first-item-per-layer rule `SendMobileIncoming` already uses:
- `Item.IsDupedEquipLayer()` — `m_Parent is Mobile m && m.FindItemOnLayer(m_Layer) != this` (reuses the existing helper; true when an earlier item already holds this items layer).
- `Item.ProcessDelta`: early-return before the per-client loop for a duped equipped item (skips the EquipUpdate and OPL to everyone).
- `Mobile` lift-reject re-show: skip the EquipUpdate and OPL for the dupe.
No behavioral change for valid equipment (distinct layers → never duped). For the invalid duped-layer case the second item was already omitted by the 0x78 packet; this just stops it leaking back via the per-item paths.
## Summary
House customization rejected ~40% of components — all classic/base tiles such as **sandstone** — for non-staff players. The pieces appeared briefly in the editor, then vanished before commit; only staff (GM+) could add them.
## Root cause
A data-convention mismatch introduced when housing.bin support was added (#2329).
- The OSI `housing.bin` encodes pre-AOS base pieces with the client **T2A** feature bit (`0x1`).
- `walls.txt` (and RunUO) encode the same pieces as `FeatureMask = 0` (always valid).
- `ComponentVerification.CheckValidity` validates against `ExpansionInfo.HousingFlags`, whose enum has no `0x1` bit, so base pieces failed `(HousingFlags & 0x1) != 0`.
- `HouseFoundation.Designer_Build` only enforces `ValidPiece` for `AccessLevel < GameMaster`, so staff bypassed validation while players had placements rejected — the server re-sends the design state and the client rebuilds the house from it, erasing the just-placed piece.
This only affects servers loading `housing.bin` from a UOP client; the old txt-only path (pre-#2329, like RunUO) was unaffected because base pieces are `0` there.
## Fix
Normalize the `housing.bin` feature mask to the housing-tier bits on load (`& HousingFlags.HousingEJ`). Base pieces collapse to `0` (always valid, exactly as `walls.txt` encodes them); `AOS`/`SE`/`ML`/... pass through unchanged. Both data sources now produce an identical validity table, matching RunUO behavior. `CheckValidity` and the `val != -1` anti-cheat guard are unchanged.
## Verification
- Parsed the real `housing.bin` from a 7.0.x client: sandstone (`0x345`) loads as `0x1` → `& HousingEJ` → `0` → valid; tier pieces (`0x40` SE, etc.) pass through; unregistered tiles stay `-1` → rejected.
- Confirmed OSI's own files disagree for the same pieces: `walls.txt` base `FeatureMask = 0` vs `housing.bin` `0x1`.
- `dotnet build` clean (0 warnings, 0 errors).
## 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.
## What
- **BuildTool now has a distinct application icon** — the MUO mark with a three-gear "settings" cluster in the bottom-right, colored by size (azure / steel / teal), wired in via `<ApplicationIcon>` in `BuildTool.csproj`. This differentiates the (now signed) build tool from the server in the taskbar/Explorer.
- **Refreshed `Projects/Application/MUO.ico`** — re-rendered from vector with the full Windows size ladder (16/32/48/64/128/256). The previous icon only carried 128/256 frames, so Windows had nothing proper for small sizes.
- **Added `branding/`** — the source SVGs (`muo.svg`, `gears.svg`, `build-tool.svg`) the `.ico` files are rasterized from.
## How
Both icons are rasterized from the branding SVGs (high-density render → Lanczos downscale per frame → packed into a 6-frame `.ico`). The rasterization script and its node deps are kept local under the gitignored `tools/` dir and intentionally not committed — `branding/` is the source of truth.
## Verification
- `dotnet build Projects/BuildTool/BuildTool.csproj` succeeds (0 warnings/errors).
- The embedded icon resource extracts from the produced `build-tool.exe`.
- Both `.ico`s validate as 6-frame Windows icons.
### Icons
<img width="150" height="150" alt="build-tool" src="https://github.com/user-attachments/assets/9c443947-4c26-46a3-ad9b-5f50630d90ad" />
## Summary
Audit of CI/CD workflows (`.github/workflows/**`, `azure-pipelines.yml`) for outdated actions, focused on the Node 20 → Node 24 runner deprecation. Most actions were already migrated; this PR cleans up the remaining stragglers and closes the gap that let them drift.
## Changes
| Action | File(s) | From | To | Reason |
|---|---|---|---|---|
| `softprops/action-gh-release` | `create-release.yml`, `build-tool-release.yml` | `v2` | `v3` | v2 still runs Node 20; v3 is a pure Node 24 runtime move, inputs unchanged (drop-in) |
| `dotnet/nbgv` | `create-release.yml` | `v0.5.1` | `v0.5.2` | Node 24 runtime bump |
| `SethCohen/github-releases-to-discord` | `post-release-discord.yml` | `v1.19.0` | `v1.20.0` | Latest; adds manual-dispatch test support |
| Dependabot | `dependabot.yml` | nuget only | + `github-actions` (weekly) | Auto-PR future action bumps instead of manual audits |
## Already current (no change)
`actions/checkout@v6`, `actions/setup-dotnet@v5`, `actions/upload-artifact@v7`, `actions/download-artifact@v8`, and `signpath/...@v2` are all on current majors running the Node 24 runtime. The Azure tasks (`UseDotNet@2`, `NuGetAuthenticate@1`) are current as well.
## Notes
- All target versions verified against GitHub's release API.
- `action-gh-release@v3` release notes confirm it's a runtime-only change with no input/behavior changes — safe drop-in for both usages.
## Problem
Houses and boats (multis) were pathed correctly only by **delegation to the slow path**: `StepCache.TryGetMask` returns `Fallthrough_Multi` for any multi-covered cell, and `GetSuccessors` ran `CheckMovement` **8× per cell** (each re-resolving the tile stack via `GetStaticAndMultiTiles`) — a sustained per-step cost near every house/boat. There was also no automated test pinning multi pathfinding.
This branch is the full multi-pathfinding effort in phases on one branch.
## Phase 1 — characterization tests (the oracle)
Implementation-agnostic invariants: a cache-on≡cache-off whole-path invariant, a per-cell sweep vs `CheckMovement` over footprint+halo (incl. destination Z), hand-verified routing (around walls, demolish-reopens, foundation-redesign-honored), classic-house / foundation / boat fixtures, non-vacuity guards. These gate every later phase byte-for-byte.
## Phase 2 — live single-pass synthesizer
`StepProbe.ComputeMultiMaskAt` synthesizes a covered cell's full 8-direction `StepMask` in one pass (the existing surface/step logic over `GetStaticAndMultiTiles` instead of 8× `CheckMovement`). `GetSuccessors` routes `Fallthrough_Multi` cells through it. No new cache, no `.swb` change. **~1.5×**, zero added allocations.
## Phase 3 / 3.1 — warm per-`multiID` interior cache (airtight)
`MultiMaskCache` caches each fixed multi's local-frame `StepMask` for **interior** cells (cell + all 8 neighbours covered → terrain-neighbour-free → position-invariant), keyed by `multiID & 0x3FFF`, built lazily from the MCL. Interior cells become ~20 ns lookups.
The cache is gated on a **per-instance footprint-clean flag** (`BaseMulti.PathInteriorCacheState`): an instance whose whole footprint terrain is below its floor (`maxTerrain < minFloor`) serves from the cache; a **dirty** instance (terrain intrudes — a contrived/GM placement) **degrades to live-synth, never a wrong mask**. This closes a cross-instance soundness gap (the cached mask depends on neighbour terrain too) found in a holistic review. The gate resets whenever the footprint's world-terrain relationship can change — **location, map, or ItemID** (a boat's heading swaps the MCL).
**Boats are cached too.** Their per-`multiID` deck masks are movement-invariant (built once per heading), so a sailing boat never rebuilds them; only the cheap clean-flag rescan repeats per move (and only when pathed near). Narrow existing boats have little interior; wide galleons (`multi.mul`) would gain Castle-class. `HouseFoundation` (per-instance runtime `DesignState`) is the one type that stays on the live path.
## Verification
- `UOContent.Tests` **454/454**, `Server.Tests` **708/708**, 0 failures.
- The Phase-1 oracle (`MultiPathInvariantTests`, cache-on ≡ cache-off) stays **byte-identical** with the synthesizer + interior cache active.
- Tests pin: footprint-cleanliness (clean vs sunk), dirty/cluttered placement degrades to live-synth while still pathing, clean placement serves, and the gate resets on move/ItemID change.
## Performance (modernuo/ModernUO-Benchmarks#8, full-fixture)
Houses at **Green Acres** (flat staff region → clean footprints, the legit-placement case):
| Route | Slow path | Phase 3.1 (interior cache) | Speedup |
|-------|----------:|---------------------------:|--------:|
| `around_a` (29 steps) | 238.3 µs | **49.1 µs** | **4.85×** |
| `around_b` (29 steps) | 224.3 µs | **49.5 µs** | **4.53×** |
~130 of ~167 multi cells/route serve from the cache (~20 ns) vs 37 live-synth. Per-cell, the slow path's 8× `CheckMovement` grows with multi complexity (GuildHouse ~857 ns → Castle ~1,194 ns), the synthesizer is a flat ~780 ns, and the cache serve is ~20 ns — so big/tall multis (and wide galleons) gain most. Identical allocations throughout.
## Summary
`Mobile.SayTo(Mobile to, int number, string args = "")` always sends the localized message using `SpeechHue`. This adds a parallel overload that accepts an explicit `hue`:
```csharp
public void SayTo(Mobile to, int number, int hue, string args = "") =>
to.NetState.SendMessageLocalized(Serial, Body, MessageType.Regular, hue, 3, number, Name, args);
```
It mirrors the existing localized overload exactly, only substituting the caller-provided `hue` for `SpeechHue`, so content can send a cliloc message to a single mobile in a chosen color without dropping down to `NetState.SendMessageLocalized` directly. This restores the hued-cliloc `SayTo` that RunUO/ServUO content commonly relied on (e.g. `SayTo(from, 1042205, 0x3B2)`).
## Notes
- Purely additive; no behavior change to existing call sites.
- No overload ambiguity: `SayTo(m, num)` and `SayTo(m, num, "args")` still bind to the existing overload; `SayTo(m, num, hue)` binds to the new one (the third positional arg is `int` vs `string`).
- Null-safe to the same degree as the existing overload (`SendMessageLocalized` guards via `CannotSendPackets()`).
## Test Plan
- [x] `dotnet build Projects/Server` — 0 warnings, 0 errors.
- One-line additive overload mirroring an existing (untested) method; no existing `Mobile.SayTo` unit tests to extend. Happy to add coverage if preferred.
Fixes#1690. Addresses Blood Oath holistically — three bugs found while researching the spell against RunUO, ServUO, the UODemise/uo.com guides, and the archived UOGuide page.
## Bugs fixed
### 1. Expiry timing (the filed issue)
The `ExpireTimer` polled every 1s, so expiry and death/delete cleanup lagged up to ~1s. Replaced with a **single-shot** timer plus centralized `[OnEvent]` handlers on `PlayerDeathEvent`/`PlayerDeletedEvent`/`CreatureDeathEvent`/`CreatureDeletedEvent` — the oath now breaks immediately on death/delete of either party.
### 2. Duration formula
Used `/80` (the bugged in-game tooltip value) instead of the real OSI formula `((SpiritSpeak - Resist) / 8) + 8`. Confirmed by RunUO, ServUO, the emulator guides, and the code's own fixed-point comment. At GM Spirit Speak this changes duration from ~9.5s to 23s and makes Spirit Speak actually affect duration.
### 3. Damage reflection (`BaseCreature.Damage` vs `PlayerMobile.Damage`)
`BaseCreature.Damage` diverged: it attributed the reflected hit to the attacker itself (`from.Damage(amount, from)`) instead of the caster, reflected the bonused (not original) amount, used `×1.1` vs `×1.2`, lacked the caster-survival guard, and had no Publish 48 resist mitigation.
Unified both paths: reflect the **original** damage attributed to the **caster** at `×1.2`. Publish 48 resist mitigation now applies only to creature casters and is gated behind `Core.SA`.
## Internals
- Collapsed the parallel `_oathTable` into a single `_table` keyed by both participants → shared timer, so `RemoveCurse` resolves from either side (required by the event handlers).
- Extracted `GetDurationSeconds` and `ComputeReflectedDamage` as testable statics.
## Tests
13 new tests (duration formula, reflection mitigation, oath lifecycle, end-to-end event-driven removal). Full suite: **436/436 pass**.
## Summary
Fixes the pathfinding step-cache (`.swb`) prebake so it bakes **once** and skips when a valid cache already exists, instead of re-baking on every boot. The root cause was the staleness fingerprint hashing mutable in-memory tile data rather than the on-disk files. This PR makes the fingerprint a pure function of the client data files and separates dynamic multis (houses/boats) from the static cache.
> This branch builds on the `ConfigurePrompts` first-boot-prompt unification (commit `5df8d0bd`, also included here) — that commit accounts for the `ServerConfiguration.cs` and `dev-docs/server-lifecycle.md` changes in the diff.
## The bug
With `pathfinding.prebakeMaps` set, the cache re-baked on **every** boot. The `.swb` staleness fingerprint hashed the live `TileData.LandTable`/`ItemTable` flags, which the server patches at runtime (`ItemFixes`, `LOSBlocker`, `PotionKeg`, `CTF`) at nondeterministic lifecycle points (Initialize-phase methods share a priority; static ctors fire lazily). So a fingerprint stamped at runtime (`[PathBake`) never matched the one recomputed during startup `Initialize()`, and the cache rebaked every time.
## Changes
**1. Fingerprint the files, not the in-memory tables** (`fix`)
Hash `tiledata.mul` (cached, computed once) plus the per-map `.mul`/`.uop` files — never the runtime-mutated `TileData` tables. The fingerprint is now lifecycle-stable. Existing `.swb` files rebake once after deploy, then stay stable.
**2. Compute the fingerprint once per boot** (`refactor`)
`Configure()`'s `AutoLoadAtStartup()` already opens and fingerprint-validates a reader for every up-to-date `.swb`. `Initialize()` now skips baking any map that already has an open reader (`StepCache.HasLazyReader`) instead of recomputing the fingerprint a second time.
**3. Bake static-only; route multis to the live path** (`refactor`)
Multis (houses/boats) are dynamic, so they're no longer baked into the static chunk cache — they were tagged with `BuiltMultisVersion`, a non-persisted session counter, which made persisting them unsafe (false matches / wasted re-bakes).
- Chunks bake land + `statics.mul` only.
- At query time, any cell whose sector (or its 1-cell halo) contains a multi routes to `Fallthrough_Multi` → the existing live, multi-aware `CheckMovement` path. The halo prevents a cell proposing a walkable edge into a neighbouring wall; interior (multi-free) cells pay one sector lookup.
- Adds `Sector.HasMultis` (one engine accessor); `.swb` format → v9 (rejects old multi-baked files); new `Fallthrough_Multi` telemetry.
- Behaviour-preserving: multis use the same live path the engine used before the cache existed.
**4. Comment polish** (`style`) — no behaviour change.
## Testing
All green: **92** pathfinding (incl. a new fingerprint-stability test and a multi-halo fallthrough test), **423** UOContent, **708** Server.
## Follow-ups (not in this PR)
- **Background bake worker** — make `[PathBake` and the boot prebake non-blocking (game thread serves tile reads to an off-thread worker).
- **Per-multi MCL cache** — cache walkability in each multi's own frame (keyed by multiID, movement-invariant) so houses/boats get a fast path instead of the live fallback.
- **House-pathfinding equivalence tests** — the one area not yet covered by a dedicated automated test; multi pathing is currently correct by delegation to the live path.
Stacked on #2475 (the `ConfigurePrompts` phase). Base will switch to `main` once #2475 merges.
## What
Move the engine's own first-boot prompts — data directories, listeners, server name, expansion + map selection — out of `ServerConfiguration.Load` and into **`ServerConfiguration.ConfigurePrompts()`** (`[CallPriority(0)]`), so **all** first-boot prompting (engine and content) runs through the single `AssemblyHandler.Invoke("ConfigurePrompts")` phase. `Load` now only reads/creates the config file.
## Why it's safe
- **Assembly loading uses `AssemblyDirectories` (default `./Assemblies`), not `DataDirectories`** — so assemblies load fine before the now-later data-dir prompt. This is the linchpin that makes the move possible.
- **`UOClient.Load()`** (client-file discovery via `Core.FindDataFile`) needs `DataDirectories`, so it moved *with* the data-dir prompt into `ConfigurePrompts`.
- **`Core.Expansion`** is now assigned in `ConfigurePrompts` (every non-mocked boot). Nothing between `LoadAssemblies` and that phase reads it — type initializers run lazily on first use, not during `LoadAssemblies`.
- **`[CallPriority(0)]`** keeps the engine prompts (including map selection) ahead of content prompts such as the pathfinding pre-bake (priority 50), preserving "after map selection".
- `Main.cs` already invokes the phase — **no startup-ordering edit** here.
## Tests
`Server.Tests` **708/708**, `UOContent.Tests` **418/418**, build clean. Fixtures are unaffected: they call `Load(true)` (now just reads config) and set expansion/data dirs directly; `ConfigurePrompts` is gated on `m_Mocked`.
## ⚠️ Needs first-boot runtime verification
`Main.cs` startup ordering is **not** covered by the fixture-based suite (the fixtures bypass `Main`). Please boot once with a fresh `modernuo.json` to confirm the first-boot prompt sequence (data dirs → … → expansion/maps → pathfinding pre-bake) and that `Core.Expansion` resolves correctly. Docs updated in `dev-docs/server-lifecycle.md`.
## Summary
Adds authentic **T2A-era (pre-UO:Third-Dawn) packet-based crafting menus**, enabled via the **`t2aCraftMenus` server setting** (read once at startup; default **`!Core.UOTD`**, so a pre-UO:TD shard gets them automatically). When enabled, double-clicking a crafting tool opens the classic `0x7C`/`0x7D` item-list menu — skill- and material-filtered — instead of the modern gump, covering all 8 tool/skill crafts (blacksmithy, tailoring, tinkering, carpentry, alchemy, bowcraft/fletching, inscription, cartography). It is **not** a runtime/admin-flippable feature flag.
This is the **definitive, reconciled** branch and **supersedes**:
- **#2181** (Delphi — `T2A_CraftingMenus`): the original effort.
- **#2381** (Jack/UOLL — `t2a_crafting_menus`): the research-grounded superset (Delphi's base + 12 corrections), rebased onto current `main`.
Original authorship is preserved across the cherry-picked history: foundation commit **@Delphi79**, mechanic fixes **@jackuoll (Jack Ward)**, reconciliation/fixes/docs mine.
## How it was built
1. Cherry-picked Jack's 13 commits onto current `main` (superset of Delphi's; only 2 trivial FeatureFlags conflicts).
2. Applied targeted fixes (below) with tests.
3. Full convention audit, build, and test pass.
Grounded in independent historical research plus Jack's deep dive. Maintainer reference: `dev-docs/t2a-crafting.md`.
## Mechanics (highlights)
- Double-click tool → target resource → skill/material-filtered menu → craft. Resource pre-selection per skill; make-last by targeting the tool.
- **Stacked-gem jewelry:** target a gem stack → the **full stack** is consumed and the piece is named by count ("a 1000 diamond ring"); count persists (`BaseJewel` serialization **v4 → v5**, new `_gemCount`).
- **Tool-less inscription & cartography** (skill-list invoked; no pen/sextant); inscription consumes reagents+scroll on success and failure, mana only on success.
- **Tailoring matching-hue consumption:** targeting hued cloth/leather consumes only that hue. Crafted items take color from their **`CraftResource`** (not the dyed hue), so dyed leather/cloth don't tint the product; in T2A only colored ingots/ore color items (metal armor/shields).
- **Half-resources on failed non-scroll crafts** (pre-UO:TD).
- **Maker's mark** always prompted for exceptional items, via the shared `QueryMakersMarkGump`.
- Server-side menu infra changes are additive (`ItemListEntry.CraftIndex`, `Entries` setter, `HasSent`).
## Notable changes on top of the cherry-pick
- **Toggle is a startup server setting, not a feature flag.** Removed `ContentFeatureFlags.T2ACraftMenus` (and its admin-flippable plumbing); the value is read once via `ServerConfiguration.GetSetting("t2aCraftMenus", !Core.UOTD)` into `T2ACraftSystem.Enabled`. Since the default tracks the era and it can't be flipped at runtime, there's no incoherent "menus-on / UO:TD-era" state.
- **Stacked-gem consumption (B3a/B3):** consume the full `PendingGemCount` (was deliberately consuming 1 while naming by the stack), null-safe gem type, plain-piece fallback + message. New `T2AJewelGemCraftTests`.
- **Convention audit:** `new List<Item>()` → `PooledRefList<Item>` on the hue-aware consume path; removed dead code.
## Decisions & deviations
- `make-last` kept as **QoL** (post-T2A gump-era feature).
- `half-on-failure` (non-scroll) kept as a **reconstruction** (not OSI-confirmed).
- **Stacked-gem** behavior set per shard authority (overrides the "single gem" reconstruction).
- **Cooking** out of scope (no T2A crafting menu existed for it).
- **No colored items from dyed materials:** crafted color comes from the `CraftResource` type. Pre-AOS leather has no colored variant, so leather is always uncolored; weapons retain resource color only in AOS+ (unchanged, intended).
## Test plan
- Automated: `dotnet build ModernUO.slnx -c Debug` clean; `dotnet test Projects/UOContent.Tests` → **421 passed** (incl. 3 new jewelry tests).
- Manual (needs a running T2A shard + client):
- [ ] Each of the 8 skills opens the correct menu; empty-menu guard fires.
- [ ] Make-last repeats the last craft (jewelry re-prompts gem).
- [ ] Jewelry consumes the full targeted gem stack and names by count.
- [ ] Cartography consumes blank maps only with T2A enabled / maps+scrolls when disabled.
- [ ] Tailoring consumes only the targeted-hue material; crafted items are not tinted by dyed cloth/leather.
- [ ] Maker's-mark prompt on exceptional.
- [ ] Failed non-scroll craft consumes half resources.
- [ ] Inscription: reagents+scroll on success/failure, mana only on success.
- [ ] T2A disabled: gump crafting unchanged.
## Credits
Co-authored-by: @Delphi79
Co-authored-by: @jackuoll
## What
On **first boot** (right after map selection), offer to pre-bake the pathfinding `.swb` cache for the selected maps. This removes first-pathfind-after-boot latency and is now cheap — ~18 MB/facet after the v8 format work (the old ~565 MB is gone). The answer persists in `modernuo.json` as **`pathfinding.prebakeMaps`** (default **false**): asked exactly once, and skipped on headless/CI boots (redirected input) where operators can set the flag directly.
## How — a generic startup phase, not pathfinding hardcoded in the engine
The clean-console (pre-Serilog) prompt window is inside the engine startup, but UOContent isn't loaded until after `ServerConfiguration.Load`. So rather than coupling the engine to pathfinding, this adds a generic lifecycle phase:
- **`Main.cs`**: new `AssemblyHandler.Invoke("ConfigurePrompts")` — runs **after** `LoadAssemblies` (so content can participate) but **before** the first `logger.Information` (so console prompts aren't interleaved with the async console sink). The first log line moves below it. Any class can hook in with `public static void ConfigurePrompts()` and self-gate on first-boot state. No `ServerConfiguration` or pathfinding coupling added to the engine.
- **`PathCacheCommands.ConfigurePrompts()`**: the first-boot prompt (interactive-only, flag-absent-only); persists the answer.
- **`PathCacheCommands.Initialize()`** (`Invoke("Initialize")` phase, after the tile matrix loads — which the bake walks): when the flag is set, bakes any map whose `.swb` is **missing or stale** (tile-data fingerprint mismatch, via `StepCache.ComputeLiveFingerprint` / `TryReadFingerprintFromFile`). A fresh cache is a no-op, so only the first boot — or a post-client-update boot — pays the several-minute cost.
## Docs
Fixed the now-stale "~565 MB / ~1.5–2 GB / do not bake by default" section in `dev-docs/pathfinding.md` (it's 17.9 MB for Trammel, tens of MB for all six facets after v8), added a "First-boot pre-bake prompt" section, and added the `pathfinding.prebakeMaps` lever row.
## Verified
- `dotnet build UOContent -c Release` → 0 errors (rebased on #2474).
- Pathfinding/StepCache tests: **90/90 pass**.
- Bootstrap streamlining of the startup phases is intentionally left as a follow-up.
## Problem
Running the full `UOContent.Tests` suite, the test host **hangs ~2.5 minutes at shutdown and then crashes** (`Test host process crashed` / run aborted). The tests themselves are fine — they complete in ~1s — but the process can't exit.
Captured via `--blame-hang` dump. The blocking thread:
```
System.Threading.WaitHandle.WaitOne()
Server.SerializationThreadWorker.Sleep() SerializationThreadWorker.cs:54 (_stopEvent.WaitOne())
Server.SerializationThreadWorker.Exit() SerializationThreadWorker.cs:61
Server.World.ExitSerializationThreads() World.cs:429
Server.Tests.UOContentFixture..ctor()
```
### Root cause
Both collection fixtures (`UOContentFixture` and `PathfindingTestFixture`) each run the **full process-global ModernUO bootstrap**. `World.Load()` is guarded to run once per process, so the **second** fixture's `World.Load()` is a no-op and does **not** respawn the serialization workers — but `World.ExitSerializationThreads()` is **not** guarded, so the second fixture calls `Exit()` on workers whose threads have already terminated. `Exit()` → `Sleep()` → `_stopEvent.WaitOne()` then blocks forever (a dead thread never sets the event). The first collection's tests run; the second collection's fixture deadlocks in its constructor; the host eventually gets killed.
This is why single-collection (filtered) runs were fine — only one fixture ever bootstraps — but the full suite hangs. It's not a parallelization race: even strictly sequential, the second fixture deadlocks.
## Fix
**(a) Engine — idempotent `SerializationThreadWorker.Exit()`**
A second `Exit()` is now a safe no-op instead of a permanent block. Only the owning (main) thread calls `Exit()`, so no synchronization is needed, and the single-call production shutdown path is unchanged.
**(b) Tests — one shared bootstrap, strictly sequential collections**
- New `TestServerBootstrap.EnsureInitialized()` runs the superset global init **exactly once per process** (lock + once-flag).
- `UOContentFixture` / `PathfindingTestFixture` slim down to delegate to it and no longer tear down global state (which the single-bootstrap model owns for the host's lifetime).
- `[assembly: CollectionBehavior(DisableTestParallelization = true)]` so collections never overlap.
## Result
| | Before | After |
|---|---|---|
| Tests run (full suite) | 258 (UOContent collection deadlocked) | **418** |
| Outcome | 2.5-min hang → host crash | **418 passed, clean exit** |
| Wall time | killed | **~7s** |
## Summary
Fixes two related pet-behavior bugs and the underlying design flaw behind both:
1. **Post-combat erratic** — after a pet killed its `all kill` target it milled around erratically at the kill site (or failed to return to the master) until the player issued `all follow`/`all stop`.
2. **`all stop` returns home** — a pet with a non-zero `Home` walked back toward that location on `all stop` (on ML; on non-ML the old `DoOrderStop` was a no-op, so the sighting there came from a residual `Stay`).
### Root cause
`ControlOrder` and the wild-creature `Home` field were overloaded to express several distinct ideas, mutated/read inconsistently across order transitions:
- `Home` doubled as the controlled-pet "stay anchor" (`HandleStayOrder` set `Home = Location`) but nothing cleared it when the pet left the staying state; `HandleStopOrder` was the only handler that never touched it.
- `DoOrderStop` had dropped RunUO's `Home = Location` re-anchor, so on ML it walked to a stale anchor.
- The post-combat fallback was a fragile `_lastPetOrder` hack in `DoOrderNone` that re-anchored a resumed `Stay` at the corpse.
- Controlled idle-wander bypassed the `CheckIdle()` rest gate that every non-controlled creature uses, so idling pets jittered every AI tick.
## Approach
Separate three concepts that were tangled together:
- **`ControlOrder`** — the active order (may be transient: Come/Attack/Drop).
- **Persistent command** (`PersistentOrder` ∈ `{None, Stay, Follow, Guard}`) — the standing directive a pet falls back to when a transient order completes. Runtime-only (not serialized; reset to `None` on load) and **derived from master proximity on login** (near → Follow, far → Stay).
- **Anchor** (`Home`) — a pure function of the persistent command, set only when that command changes (never on transient transitions or fallback-resume), so it can't go stale.
### Behavior
- **Stop** is resolved immediately from what the pet was doing: Attack/Come → resume the persistent command; Follow/Guard → cancel to idle where it stands; Stay → stay put.
- **Stay** holds its post (returns only if displaced, e.g. after a fight) — no shuffle.
- **Idle** (`None`) is a gentle wander routed through `CheckMove/CanMoveNow/CheckIdle`, so idling pets take the same 15–25s rest periods as other creatures, on both ML and non-ML.
- **Post-combat** the pet resumes its persistent command (a staying pet returns to its original post, not the corpse).
- **Release** without a spawner anchors where the pet stands instead of pathing to a stale anchor.
This restores the RunUO-intended behavior (verified against the RunUO reference) while fixing the ModernUO regressions.
## Tests
New `PetOrderTests` (13 deterministic xUnit tests) cover: anchor lifecycle, the full Stop truth table, report 1 (post-combat return to post), report 2 (no stale-anchor walk-home), frozen-Stay/gated-idle wiring, release fix, derive-on-login, and a non-ML spot-check. The subjective wander *feel* is covered by a manual-QA checklist in the implementation plan.
## Notes
- Engine project (`Projects/Server`) untouched; the one `BaseCreature.cs` change is the `ControlOrder` setter passing the previous order to `OnCurrentOrderChanged`.
- `DoOrderCome` keeps auto-converting to `Stay` on arrival, which under the new model cleanly means "come and hold near me."
- Commits in this PR are temporarily **unsigned** (the signing agent's passphrase cache expired mid-session); happy to re-sign / amend on request.
## Summary
Phase #3b (final roadmap item), stacked on #2470. Compacts the index trailer from 20 to 8 bytes/chunk.
Trammel: 19.2 MB → 17.9 MB. Roadmap total: 565 MB → 17.9 MB (−96.8%).
## Details
- Trailer stores `{ u32 packedKey = (ChunkX << 16) | ChunkY, u32 recordLength }` per chunk, in record write order; the file offset is dropped and reconstructed by cumulative recordLength from HeaderSize.
- No record reordering, no varint; fixed-stride, TryReadChunk unchanged.
- Also simplifies the accumulated `.swb` code comments across the stack.
- Format v8; v7 files rejected and re-baked once.
## Tests
v8 multi-chunk round-trip (cumulative offset reconstruction) + the v6/v7 suite; full pathfinding suite green; Release build clean.
## Summary
Phase #3a, stacked on #2469. Compresses each chunk record independently with libdeflate (random access preserved).
Trammel: 124.7 MB → 19.2 MB (−85%).
## Details
- Whole-record framing `[u32 UncompressedLen][payload]`; records that don't shrink (tiny Uniform) are stored raw, detected as payload length == UncompressedLen.
- Codec chosen by full-Trammel spike: libdeflate VeryHigh (16.5 MB, 1.83 µs/chunk decompress) over zstd L19/22 (17.4 MB) and managed Brotli q11 (16.4 MB) — best native ratio, fastest decompress, already the repo's packet codec (no new dependency).
- Reuses cached thread-static bindings: `Deflate.Maximum` for bake, `Deflate.Standard` for reads.
- Compression is bake-time only; decompression is one-time per chunk (LRU-cached).
- Format v7; v6 files rejected and re-baked once.
## Tests
v7 unit tests + the v6 suite run through the compression path; full pathfinding suite green; Release build clean.
## Summary
Phase #2 of the `.swb` step-cache size-reduction roadmap (after #2465, v5 uniform elision). Stores the 16 base directional Z arrays as masked residuals against each cell's own SourceZ and omits any array that matches its prediction. Lossless, byte-identical reconstruction.
Trammel: 231.9 MB → 124.7 MB (−46%).
## Details
- Predictor: `predict = mask bit ? SourceZ : 0` (matches the baker's 0 on unwalkable directions); residual `Z − predict` via unchecked two's-complement (byte-exact for all inputs); reconstruct `Z = predict + residual`.
- A `u16 ZArrayMask` flags which of the 16 base arrays differ from prediction; matching arrays are omitted and synthesized from mask + SourceZ at read.
- Serializer-layer only: StepChunk, the cache, the algorithm, and the baker are unchanged.
- Format v6; v5 files rejected and re-baked once.
## Tests
21 v6 unit tests; full pathfinding suite green; Release build clean.
## Issue
Fixes#2452. A player with 30 Ninjitsu reported that the Animal Form menu showed **every** form; selecting one above their skill (e.g. Dog, req 40) **consumed mana** and returned "you need at least 40 skill", and afterwards the **gump never reopened** — every recast silently re-attempted the unusable form and drained more mana.
## Root cause
Three linked bugs, all reproduced from the code:
1. **Gump not gated by skill.** `AnimalFormGump.BuildLayout` compared `Skill.Fixed` (which is `Value * 10`, so 30 skill → `300`) against the raw 0–100 `ReqSkill` (Dog = `40`). `300 >= 40` is always true, so all forms were shown. `Morph` itself correctly uses `.Value`.
2. **Mana charged on a no-skill cast.** `Morph` returns `MorphResult.NoSkill` for an under-skilled form, but both call sites (`OnCast`, `OnResponse`) only special-cased `MorphResult.Fail`; `NoSkill` fell through to the branch that deducts mana.
3. **Menu never reopened.** Per OSI ([uo.com](https://uo.com/wiki/ultima-online-wiki/skills/ninjitsu/), [uoguide](https://www.uoguide.com/Animal_Form)), casting while **standing still always opens the selection menu**, and casting while **moving** quick-transforms into the last selected form. ModernUO only opened the menu when `lastAnimalForm == -1`, so once any form was selected a stationary recast skipped the menu.
## Fix
- Add `AnimalForm.CanSelectEntry` (compares `Skill.Value` to `ReqSkill`, plus the talisman check) and use it for the gump's per-entry enable check.
- `OnCast`: standing still always opens the menu; moving quick-transforms into the last form. `NoSkill` no longer costs mana.
- `OnResponse`: handle `Success` / `Fail` / `NoSkill` explicitly so `NoSkill` costs no mana.
## Tests
Adds `AnimalFormTests`:
- `CanSelectEntry` rejects forms above skill, accepts forms at/below skill, and requires a talisman for talisman-gated forms.
- `Morph` returns `NoSkill` (without transforming) when under-skilled, and `Success` when sufficiently skilled.
Verified the gating test catches the regression (reintroducing `.Fixed` fails it). Full solution build is clean; the 5 new tests plus 284 other UOContent tests pass (the pathfinding/AI sequential tests were excluded only because they deadlock under concurrent local runs — they are unrelated to this change).
## Summary
Sub-project #1 of the `.swb` step-cache size-reduction roadmap (`dev-docs/pathfinding.md` § Future work). Adds **uniform-chunk elision** to the `StepCacheFile` format, bumping it **v4 → v5**.
A fully-uniform 16×16 chunk — no strata, **no swim layer**, all 19 base arrays constant (open ocean, Green Acres, void) — serializes to a **~28-byte record** (`KindUniform`) instead of ~5,393, and reconstructs **byte-identically** via `Array.Fill`. Non-uniform chunks use the existing v4 body (`KindFull`) with the swim-layer and strata trailers **fully preserved** — the Kind byte is just prepended.
## Calibrated result (measured, not projected)
Baked Trammel via `SaveToFile`:
| | |
|---|---:|
| Chunks | 114,688 |
| Uniform (swim-aware) → elided | 62.7% |
| Swim-layer chunks (stay Full) | 8.9% |
| Strata chunks (stay Full) | 1.8% |
| Baseline (full records) | 592.2 MB |
| **Actual v5 `.swb`** | **231.9 MB (−61%)** |
The residual is ~150 MB of non-uniform land Z-blocks (targeted by #2 predictive-Z) + ~81 MB of swim-layer trailers (#2/#3). #2 and #3 are separate follow-up PRs.
## Implementation
- `StepChunk.IsUniform()` — false if it has strata **or a swim layer**, else true only when all 19 base arrays are constant (the "all-same" check uses the SIMD-accelerated `ContainsAnyExcept`).
- `StepCacheFile` v5 — `Kind` byte (`KindFull=0`/`KindUniform=2`, 1 reserved); uniform write/read; `FormatVersion`/`MinSupportedVersion` → 5 (v4 files rejected on open and re-baked). No `StepCache`/algorithm/index changes; fingerprint logic untouched.
## Tests
7 `StepCacheFileV5Tests` (uniform round-trip + `<200 B` compactness, varied-full, swim-layer-full, strata-full, swim+strata combined, v4 version-gate rejection) + the existing StepCache/pathfinding suite — **70 pass**, including the prior `SwimLayer_RoundTrips`. An independent review verified write/read symmetry, cast round-tripping, swim/strata preservation, and the version gate (READY TO MERGE).
Fixes#2462
## Summary
Removes the per-object `VirtualHairInfo` heap wrapper for mobile/corpse hair. Hair is now stored **inline** on `Mobile` and `Corpse` as `int _hairItemId` / `int _hairHue` plus a lazily-allocated, **non-serialized** ephemeral `Serial _hairSerial` (in the high virtual-serial range) — and likewise for facial hair. The `VirtualHairInfo` class is deleted, with a **lossless** save migration.
This delivers three things:
1. **Fixes a hair-removal bug.** `Delta(MobileDelta.Hair)` is deferred (it enqueues; `ProcessDeltaQueue` runs later in the tick). The old `HairItemID = 0` setter nulled `_hair` *immediately*, so by the time `ProcessDelta` built the remove packet the equipped virtual serial was already gone — the old `??=` code then re-materialized a **fresh** serial (≠ the equipped one), so clients never removed the right entity, and it left a phantom ItemId-0 object behind. The serial now lives on the entity and **persists across removal**, so remove packets carry the correct serial.
2. **Lightens the entity.** No heap hair object; bald mobiles allocate nothing (the serial is minted lazily only when hair is present). This was the original reason `HairItemID`/`HairHue` exist.
3. **Removes `VirtualHairInfo` entirely**, keeping the high-range virtual serial behavior.
## How
- **Mobile** (manual serialization): inline `_hairItemId/_hairHue/_hairSerial` (+facial); lazy `HairSerial`/`FacialHairSerial`; `ProcessDelta` reads those. Serialization **v36 → v37** — the v30-v37 deserialize is unified, reading the legacy per-hair `VirtualHairInfo` version int only when `version < 37`. Setting item id to 0 clears the hue (matching the old object-nulling) while retaining the serial.
- **Corpse** (codegen serialization): decomposed to `[SerializableField] int _hairItemId/_hairHue` (+facial) + ephemeral serial; **v16 → v17** with `MigrateFrom(V16Content)`.
- **Lossless migration:** the loader validates exact byte length, and the old corpse hair is a presence-bool-gated block, so a tiny **migration-only** `LegacyHairInfo` reader (no runtime role) consumes the legacy `[bool][int ver][int itemId][int hue]` bytes. Frozen `Corpse.v14/v15/v16.json` are retyped to it; `v17.json` describes the new int fields.
- All consumers updated to discrete accessors: `OutgoingMobilePackets`, `CorpsePackets`, corpse subclasses (`MilitiaFighterCorpse`, `SchmendrickApprenticeCorpse`), and the packet test mirrors.
- `VirtualHair.cs` renamed to `OutgoingVirtualHairPackets.cs` (the only type left in it after `VirtualHairInfo` was removed).
## Test Plan
- [x] Full solution build: **0 warnings, 0 errors** (`TreatWarningsAsErrors`).
- [x] `Server.Tests`: **708 passed** (incl. new `RemoveHairUsesEquippedSerial` / `RemoveFacialHairUsesEquippedSerial` proving the serial survives removal + hue clears).
- [x] `UOContent.Tests` corpse/hair: **6 passed** (incl. `CorpseHairMigrationTests` asserting the legacy hair bytes are consumed exactly — the loader's length invariant).
- [x] Generated migration code inspected: V14/V15/V16 readers consume the legacy block byte-for-byte; serial never written to disk.
## Upgrade notes
- Old Mobile (v30–v36) and Corpse (v13–v16) saves load losslessly.
- Minor cosmetic-only change: `SchmendrickApprenticeCorpse` hair/facial-hair RNG draws shift order within each pair (same draw count); irrelevant for a quest NPC corpse.
