## Summary
About 114 tests skipped on CI because they needed the official client files (`tiledata.mul`, map and multi files). They gave CI no signal, and locally they ran against whichever client a developer had installed. The test hosts now build their own world instead, so every test runs on CI and runs the same on every machine. CI simulation: Server.Tests 837 passed, UOContent.Tests 1131 passed, **0 skipped**.
- **Generated tile data.** The fixtures write a pre-HS `tiledata.mul` from a small invented tile kit (`Server.Tests/Terrain/TestTiles`) and load it through the real `TileData.Load()`. The test hosts no longer read any client directory (`MODERNUO_CLIENT_PATH`, `MODERNUO_TEST_DATA_DIR` and `C:\Ultima Online Classic` are gone from test code). The file is written to `SyntheticData` in each test project's output, overwritten every run and removed by `dotnet clean`.
- **Injected terrain.** `TestTerrain` stamps land and statics into Felucca and Trammel, which share one terrain. Writes after startup must go in a `TestTerrain.Scope()`, which puts back every block it touched and keeps the step cache fresh.
- **Prefab scenes.** Seven named scenes (town, causeway, sewer walkway, coastline, deep sea, map-edge sea, an upstairs room with an L-shaped counter) replace the Britain coordinates the pathing, AI and fishing tests relied on. Each scene-based test asserts its scene is present first, so a missing scene fails instead of passing on flat ground. Stamping all seven takes 3-5 ms.
- **Multi footprints.** Contrived footprints for the three multi IDs content looks up (0x74, 0x7A, 0x0) are seeded through a new internal `MultiData.SetComponents`.
- **Fingerprint.** `StepCacheFile.HashTileDataFile` is extracted so a test can show that a one-flag change to the file changes the fingerprint, while runtime table mutation does not.
### Engine changes
- `TileMatrix.GetLandBlock`/`GetStaticBlock`: on a map without its files, return the block injected with `SetLandBlock`/`SetStaticBlock`, or the default when nothing was injected. Maps with files take the same path as before.
- `MultiData.SetComponents` (internal): registers a footprint without multi files.
### Test changes worth a look
- Values that were properties of real Britain terrain (probes at Z 10, a pinned plain walk mask of 0xC1) are re-pinned to the synthetic plain (Z 0, 0xFF).
- No tests were deleted. The multi walkability and boat-deck tests assert movement behaviour over a footprint, not what a real multi looks like, so they run on the contrived footprints.
- Maps 0 and 1 keep file indexes 0 and 1, and every stamp writes both, rather than sharing one file index. On a map without files the getters never consult the shared-file cache, and its patch bits would hide one facet's injection from the other.
## Test plan
- [x] `dotnet test -p:SolutionDir=... --filter "Category!=Manual"` with the client env vars pointed at an empty directory: 837 + 1131 passed, 0 skipped, 0 failed
- [x] Mutation check: removing the step-cache clear from `TestTerrain` makes the stale-cache tests fail
- [x] CI green on all platforms
Follow-up to #2721. It fixes how the remaining tests behave and changes no production code.
## CI
- Both `dotnet test` steps pass `--filter "Category!=Manual"`. Before this, Manual-tagged tests ran on CI anyway.
- The one surviving movement-throttle latency test is the only coverage of burst forgiveness over a full, wrapped history. It loses its Manual tag, so it runs on CI.
## Tests that passed on CI without testing anything
- **Corpse and throwing tests.** They relied on tiledata for the weapon `Layer`, so on CI the equip silently failed and they asserted against empty lists. They now set `Layer` explicitly and assert the equip took effect. `TestThrown` also pins its own strength and range values, so a shard retuning SoulGlaive no longer breaks them.
- **Healthbar poison rows.** Server.Tests registers no poisons, so every row tested `null`. They now use real poison levels.
- **Account packet tests.** The mock accounts reported `Length` 0, so the character-list tests only ever encoded empty slots. They now encode the real characters.
- **Pathing and AI tests that only mean something on real Trammel/Felucca terrain** are now guarded with `TileDataRequirement.SkipIfMissing()`: Britain desk and clutter scenes, house footprints (no MultiData on CI) and fingerprints. Tests that are valid on flat blank terrain stay unguarded. (A synthetic tiledata fixture to run these on CI is a planned follow-up.)
- **Assertions that couldn't fail now check their claim.** Affected tests include the `DefaultWalker` cache use, cache on/off parity requiring a path, the recorder capability flags, the idle-pet gate, the monthly DST recurrence, the AdvancedSearch `Record.Exception` wrappers, `CanFitItem` with no branch-guarded assert, the Firewall callback, and `Import_NoMap`. The Geometry Parse asserts also now use the same input as their TryParse.
## Shared state
Every test that changes process-wide state now restores it, even when an assert fails:
- **The clock.** A new `TimerWheelScope` saves and restores `Core._tickCount`, `Core._now` and the timer wheel. `SimulatedClockScope` covers decay tests.
- **StepCache settings and caches**, via `StepCacheStateScope`.
- **Network state.** The ManualAllowlist, LoginAllowlist, firewall and BanChannel reporters are restored. Global packet handlers are now registered per test and removed afterwards, so they no longer stay registered for later tests.
- **Data and config tables.** `ArchiveJournal` statics, `TileData.ItemTable`, `ObjectPropertyList.Enabled`, `Core.Expansion` and the localization table are restored. One `MobileStatusVersion` restore was resetting a different expansion than the one the test changed.
- **Entities.** Tests with fixed serials now delete what they create. Fixed mobile serials moved to `World.NewMobile`, because deleting a hard-coded serial could unregister a live entity.
- **Collection race.** `SpiralScanTests` no longer clears the spawn cache from a separate collection that could run in parallel with the cache-manager tests.
- **Shared statics.** Classes reading shared static allowlists joined the sequential collection.
## Duplication
New shared helpers replace copy-pasted harness code:
- **Server.Tests:** `PacketAssert.Sends`, used by about 130 packet tests; `TestClientHarness`, `PersistencePipelineScope`, `NetTestHelpers`, `EntityCleanup` and one `TestMulti`.
- **UOContent.Tests:** `TimerWheelScope`, which also holds one wraparound-safe `RunFor`/`RunUntil`; `AiTestStubs`, which merges six walker copies and five identical creature stubs; `MobileTestHarness`, `CommandTestHarness` and `TempDirectory`.
Dead oracle packet classes are removed.
## Cost
| Test | Before | After |
|---|---|---|
| `VirtualSerialTests` | 1.57s | 2ms (starts near the wrap via reflection instead of drawing ~17.9M serials) |
| Event scheduler recurrence loop | 473ms | 1ms |
| CrowdSec drain | 517ms | 16ms (event-driven instead of a fixed 500ms wait) |
| AuthId tests | 248ms | 99ms (SHA2 except the two GameLogin tests) |
| Container cap tests | 5,000 items | 3,449 / 3,277 (one past each protocol cap) |
| Bulk removal | 100k children | 20k children |
| OPL hash distribution | 60k builds | 10k builds |
## Shard-tuning decoupling
- AI assertions read `ReacquireDelay` and `FollowMoveSpeed` instead of hard-coding production tuning.
- The guard friendly-fire test uses an `AI_Mage` stub instead of WhiteWyrm.
- Argon2 assertions pin the `argon2id` variant and `v=19` but read the cost parameters from the configured hasher.
## Tests
| Run | Server.Tests | UOContent.Tests |
|---|---|---|
| With client files | 812 pass | 1,123 pass |
| Without client files (CI simulation) | 800 pass, 12 skip | 1,021 pass, 102 skip |
- The UOContent fixture reads `MODERNUO_TEST_DATA_DIR`; Server.Tests reads `MODERNUO_CLIENT_PATH`.
- The 15 new UOContent skips are the map-dependent guards above.
- Total run time with client files is about 5s, down from about 8s.
## 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.
Stacked on #2594. Fixes jerky creature movement (lich / Fast-bucket melee chases) by choosing the client animation flag from the actual step pace instead of a caller-supplied `run` argument, and fixes three step-pacing defects in the move budget found while verifying it with paired server/client traces.
### Why
The `Direction.Running` bit does nothing for creatures server-side (`Mobile.OnMove` reads it only for the player throttle and stealth reveal). Its whole effect is on the client, which animates each step over a fixed time selected by that bit: walk 400 ms / run 200 ms on foot, 200 / 100 ms mounted. ClassicUO queues up to 5 steps and *drops* the sixth, so a creature stepping every 300 ms while flagged as walking backs the queue up until it snaps forward — the observed jerk.
The `run` argument never carried the one fact that matters (the step interval). RunUO passed `true` in combat / `false` for pets and gated it on `dist > 5`; #2271 flipped every combat site to `false`; pets passed `currentDistance > 2`. None of that is a coherent signal.
### What
**Pace-derived run flag**
- `BaseAI.ShouldRun()`: run iff the effective step delay (move clock + badly-hurt inflation) is shorter than `Movement.WalkFootDelay` / `WalkMountDelay` (mounted or flying) — with a continuity rule: an *isolated* step (taken after standing at least a walk interval) goes out as a walk, because the client renders each step alone and a lone run-flagged step is a 200 ms dart. Only a continuing cadence flags run; a true sprinter (pace under the run interpolation) always runs, since a walk-rendered first step would flood the client's 5-step queue. This reproduces RunUO's close-in feel (its `dist > 5` gate) from first principles.
- `DoMoveImpl` stamps the bit; it is the single place the flag is set.
- `run` removed from `MoveTo`, `WalkMobileRange`, `ApproachTarget`, `MoveToPoint`, `MoveToWithGroup`, `MoveToWithCollisionAvoidance`, the move intent, and `PathFollower.Follow`. All 35 call sites updated. **API change** for custom scripts — documented in the RunUO migration docs (`09-items-mobiles-creatures.md`, `11-api-reference.md`) and `content-patterns.md` § Creature Speeds.
**Move-budget pacing fixes** (each confirmed by UTC-aligned server/client step traces)
- A stall no longer banks catch-up steps: the budget's snap-to-now released up to three steps in ~300 ms when a creature resumed chasing after standing beside its target — rendered as a teleport.
- Debt accrual removed entirely: a step landing sub-period late (think-grid vs budget misalignment during reactive mirroring) kept the remainder and fired a follow-up ~100 ms later — a dart pair. `ConsumeMoveBudget` now paces every step from when it was actually taken; in continuous pursuit the move-wake lands within wheel resolution of the deadline, so the cost is single-digit-ms drift.
- Net effect: a creature can never step faster than its pace, verified across a full chase session (zero sub-pace steps; metronomic 350 ms cadence for a 0.3 s lich).
- Test fixture now runs `Movement.Configure()` (the walk delays were 0 in tests).
### Accepted trade-off
Animal (LOW group) bodies without a run animation slide on their stand frames when flagged as running. Most are slow enough to stay flagged as walking; the client-side fallback is in ClassicUO/ClassicUO#1930.
### Tests
`RunFlagTests`: foot thresholds (0.3 / 0.125 run; 0.4 / 0.45 / 1.05 walk), flying uses the mount threshold, badly-hurt inflation flips a 0.35 s creature back to walk, a real `DoMove` stamps the bit, isolated steps drop to walk (sprinters keep running), a stall restarts the cadence with no banked steps, and a late step earns no quicker follow-up. Full suite: 837 Server + 747 UOContent green.
## 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.
## Summary
Creatures (pets following, monsters chasing, NPCs approaching) would **oscillate — "pace back and forth really fast"** at concave obstacles (reported at the Britain Inn L-desk: a pet at `(1493,1614,20)` never reaching its master at `(1494,1605,21)`) instead of routing around them.
**Root cause** (the A* pathfinder itself was correct): all goal-seeking funnels through `MoveTo` and `WalkMobileRange` → `MoveTowardsOrAwayFrom`, which step greedily via `DoMove(dir, badStateOk:true)`. `DoMove` returns `true` even when the direct step was blocked and the creature merely **auto-turned and sidestepped** (`MoveResult.SuccessAutoTurn`), and the caller then set `Path = null`, discarding the `PathFollower`. So at a concave obstacle a non-progressing sidestep was mistaken for progress and the creature never committed to a route. (AOS pet-follow runs at `CurrentSpeed = 0.1`, hence the "really fast" shuffle.)
## What changed
- **New centralized `BaseAI.ApproachTarget(target, run, range)` primitive.** A greedy step is committed only when it **fully succeeds (`MoveResult.Success`) and actually gets closer**; otherwise the creature commits to a **persistent `PathFollower`** that routes around the obstacle and is never discarded by a greedy step. The open-terrain fast path (one greedy step, no pathfinding) is preserved. `MoveTo`, `MoveTowardsOrAwayFrom`, and `MoveToWithCollisionAvoidance` all delegate to it — public signatures unchanged, so no AI-class call site changes.
- **Best-distance give-up + idle.** A creature that cannot reach a **stationary** in-range goal stops shuffling and idles after `ApproachGiveUpTicks` (40) ticks without lowering its closest-ever distance; a **moving** goal (active chase) never gives up. It resumes the moment the goal moves.
- **Pathfinder fix (required):** `BitmapAStarAlgorithm.IsBlockedByDynamic` now skips the dynamic mobile-block check **at the goal cell only** (`MoveImpl.Goal`). Previously A* returned `null` whenever the target mobile stood on the goal cell, so creatures could never pathfind *toward* another mobile — only toward empty ground. The follower stops within `range` short of it. Static/item blocking and all non-goal mobile blocking are unchanged.
## Tests
New AI-loop integration tests in `ApproachTargetTests.cs` drive the real `BaseAI` primitives against live Britain Inn map statics: exact-repro pet follow, open-terrain (asserts zero pathfinding), `MoveTo` chase (static + walking-away target), route-around-a-dynamic-wall, and walled-off give-up-and-idle.
- Pathfinding + AI subset: **52/52** pass.
- Full `UOContent.Tests`: **301/301** pass. (Note: the test host lingers on shutdown — a pre-existing infra quirk unrelated to this change; all tests complete and pass.)
- Full solution build: clean (0 warnings / 0 errors).
## Notes
- Branched off `main`; independent of the in-flight step-cache work.
- Out of scope (future work): proactive "SmartAI" look-ahead pathfinding so clever creatures plan a route before walking into the obstacle, rather than reacting after they hit it.
## Test Plan
- [X] In-game: order a pet to `follow`/`come` across the Britain Inn L-desk; confirm it routes around and reaches you instead of pacing.
- [X] Aggro a monster and kite it around a building/treeline; confirm it chases around obstacles.
- [X] Confirm open-terrain following/chasing feels unchanged (no extra latency).
- [X] Confirm a creature with a genuinely unreachable target idles rather than shuffling forever.
Kamron Batman