Follow-up to #2729.
## Problem
At load, `ReadAnchoredTime` shifted **every** anchored value forward by the downtime, including values that were already in the past when the save started. That caused two problems.
**An expired deadline came back.** A deadline that passed 5 minutes before a 30-minute outage loaded 25 minutes in the future. For example:
- `Sheep.NextWoolTime`: a sheep that was ready to shear stopped being ready.
- `PlayerVendor.NextPayTime`: an overdue payment was pushed back.
- `BaseCreature.SummonEnd`, the spell fields' `End`, `TransientItem.Expiration`, and the ChampionSpawn times could move the same way.
**Records were rewritten for nothing.** The first save after every restart rewrote:
- stamps of things that had already happened: VirtueContext's `LastHonorUse`, `LastSacrificeGain`, the four `Last*Loss` stamps, and `Corpse.TimeOfDeath`
- every deadline that had expired long ago.
## Change
- **`IGenericReader.AnchoredTimeOrigin`** is the loaded save's start time.
- `GenericEntityPersistence` and `GenericPersistence` set it from `World.LoadedSaveStartTime`.
- `BufferReader`, `UnmanagedDataReader` and `BinaryFileReader` expose it.
- **`ReadAnchoredTime` returns any value at or before the origin exactly as saved.**
- A deadline still running at save start is shifted as before and keeps its remaining time.
- Readers without an origin keep the old behaviour.
- **This covers every anchored field, including generated ones.** Generated `[AnchoredDateTime]` fields call `reader.ReadAnchoredTime()`, so the one change covers all 26 generated fields as well as the manual reads. `[DeserializeTimer]` only restores timers that were running at save time, so it is unaffected.
- **The `[AnchoredDateTime]` section of `dev-docs/serialization.md` and the `WriteAnchoredTime` doc comment describe the new rule.**
## Behaviour change
Downtime now counts toward windows measured from a past stamp:
| Window | Length |
|---|---|
| Honor use | 5 minutes |
| Sacrifice gain | 1 day |
| Virtue atrophy | 7 days |
| Instanced corpse looting | 3 minutes |
A 30-minute outage makes a 7-day atrophy fire about 0.3% sooner. No format change.
## Tests
New tests in `AnchoredTimeTests`:
- A deadline running at save start keeps its remaining time.
- A deadline that passed before save start stays passed.
- A past stamp is not shifted.
- A value exactly at save start is not shifted.
`Server.Tests`: 845 passed. `UOContent.Tests`: 1140 passed.
ReadAnchoredTime shifted Item.LastMoved, Item.DecayResetTime and
Mobile.LastStr/Int/DexGain forward by the downtime at load. The bytes
were stable from save to save, but the first save after every restart
rewrote one timestamp in nearly every item record and most mobile
records, which defeats any per-record backup or delta save.
- LastMoved and DecayResetTime are read as saved. The downtime credit
is applied after load by Item.CreditDowntimeToDecay, which moves
DecayResetTime only for items whose decay countdown was still running
when the save started. Every other item keeps its bytes, and decay
timing is unchanged.
- World.LoadedSaveStartTime exposes the loaded save's anchor so the
credit can tell which countdowns were running.
- Stat-gain stamps are read raw. The cooldown is minutes long, so
downtime counting toward it costs nothing.
No format change: these fields were already absolute ticks.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Moving Shot charged its mana in `OnBeforeSwing`, before `BaseRanged.OnSwing` decided whether to fire. A swing that fired nothing (a paralyzed or frozen archer, no ammo, a failed harmful check) still took the mana and left the move armed. Every swing after that charged again, so inside the 3-second window you paid twice, until the archer ran out of mana.
`OnBeforeSwing` now only checks the mana. Moving Shot's own `OnHit`/`OnMiss` charge it, and those only run once a shot has fired. This is how the other weapon abilities pay. A miss still costs mana. A parried shot no longer does: the parry clears the move before `OnHit` runs, the same as for every other ability.
## Summary
With T2A packet crafting menus enabled (`t2aCraftMenus`, default for pre-UOTD eras), cooking did nothing. Double-clicking a Skillet set a target, and `T2ACraftSystem.ShowMenu` had no Cooking branch, so the target was accepted and silently dropped. Nothing else could cook either: the old RunUO "use food on a heat source" code was never hooked up and was removed in #1054.
T2A cooking never used a menu, so this restores the item-on-item / item-on-heat-source flow:
- **Per-item handling:** each ingredient (`CookableFood`, `Dough`, `SweetDough`, `SackFlour`, `BowlFlour`, `JarHoney`) implements `IT2ACombinable.TryCombine` and switches on the target. `T2ACooking` holds the shared target, water, combine and heat-timer helpers.
- **Heat source:** all `CookableFood` (raw meats, fish, eggs, unbaked pies/quiche/pizzas, cookie and cake mix), dough → bread loaf, sweet dough → muffins. One unit is used up at the start, then a 5 s timer; the Cooking check runs from `CookingLevel` to 100 (bread and muffins use `DefCooking`'s range). The food burns on failure or if the cook moves more than 3 tiles away.
- **Combinations:**
- Open flour sack or bowl of flour + water → dough
- Dough + pear / peach / apple / pumpkin / eggs / meat / sausage → the matching unbaked pie, quiche or pizza
- Dough + honey → sweet dough
- Sweet dough + honey → cookie mix
- Flour + sweet dough → cake mix
- The honey and flour + sweet dough recipes work in either direction.
- **Water:** water drinks (pitchers), buckets, barrels, tubs and trough add-ons, in the pack, in hand, or on the ground within 2 tiles and in line of sight.
- **Cooking tools are passive:** with T2A menus on, a tool whose system has no T2A menu (`T2ACraftSystem.HasMenu`) falls through to `ShowCraftMenu`, which does nothing under T2A. No dead target is left.
Behaviour with T2A menus off is unchanged.
## Follow-ups
- Static troughs/barrels and open water aren't water sources here or for pitcher filling (no static water ID list exists); to be fixed separately.
- Whether a bowl of flour returns an empty bowl is unconfirmed (`//TODO`).
- Cooking tools that must merely be in the pack for some recipes are not modelled yet.
## Testing
- `dotnet build`: 0 warnings, 0 errors
- New `T2ACookingTests`: flour + water, closed sack refused, dough + apple, honey in both directions, cookie mix, cake mix, wrong target consumes nothing, water in another player's pack refused
- Not covered: the heat timer (untested); not yet exercised in game
## Summary
The T2A crafting menus offered items the chosen resource couldn't make:
- **Tailoring honours the targeted cloth hue.** The menu was built from all cloth in the pack, but the craft consumes only the targeted hue, so it offered items that then failed on resources. `CanCraftItem` / `FilterEntries` / `AnyCraftableInCategory` now take the hue and count with the same helper the craft uses.
- **Uncut cloth counts in the hued path.** The hue-aware count and consume matched only `Cloth`, so targeting uncut cloth gave an empty menu. The hued path now uses `CraftItem`'s equivalence table like the unhued path. `T2ACraftSystem` reuses that table instead of a narrower copy, which also covers the coloured woods and leathers on later-era shards.
- **T2A tinkering only uses iron ingots.** Coloured ingots are refused with a re-prompt. The menu filter now uses the selected ingot, and Make Last for tinkering always resolves to iron, so a coloured choice left over from the gump flow can't be used.
## Testing
- `dotnet build`: 0 warnings, 0 errors
- New `T2AHuedFilterTests`: only cloth of the targeted hue counts, and uncut cloth of the same hue counts toward the recipe
- No test for the tinkering target and message flow; not yet exercised in game
## 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.
## Summary
`Localization.Remove` drops a language's dictionary from `_localizations` once its last entry is removed. For the fallback language (`enu`), `_fallbackEntries` still pointed at the dropped dictionary. A later `Add("enu", ...)` then created a new dictionary, and because `_fallbackEntries` was not null, `Add`'s `??=` left it on the old one. Every fallback lookup missed the new entries.
The fallback table now stays registered when it empties. Later adds land in the dictionary lookups read, and removed entries stay removed. Nulling `_fallbackEntries` instead would have made the next lookup reload `cliloc.enu` from disk and bring back the removed entries.
Nothing in production calls `Remove` today. It is public API for shards and tests; #2722 adds a test cleanup that removes entries.
## Tests
- `LocalizationEntryTests.RemovingTheLastFallbackEntry_KeepsLaterAddsVisible` clears the tables, adds then removes one `enu` entry, adds another, and checks that the fallback lookup finds the new entry and not the removed one. It clears again afterwards so later tests reload the real tables. It fails without the fix.
- Server.Tests: all pass.
## Summary
`Timer.ClearAllTimers` walked each wheel slot with a `do/while` that read `node._nextTimer` but never advanced `node`. `Stop()` unlinks the head from its slot, so the second pass read a null successor from the same, now-detached node and exited. Only the first timer in every slot was stopped; the rest kept running.
The loop now reads the successor, stops the node, and advances.
Nothing in the repo calls `ClearAllTimers` today, so no running server is affected. It is public API for shards and tests.
## Tests
- `TimerTests.ClearAllTimers_StopsEveryTimerInASlot` puts three timers in the same slot, clears them, asserts none is `Running`, and slices past their due time. It fails without the fix.
- Server.Tests: all pass.
## Summary
Many tests proved a specific change when it landed but don't need to run on every CI build forever. This removes **202 test methods and 9 files: 313 test cases, 2,247 → 1,934**. UOContent.Tests drops from about 14s to about 5s locally.
Removed:
- **Tautologies.** Asserting a constant, a constructor readback or a trivial getter, or testing BCL behavior.
- **Exact duplicates** of a surviving test on the same code path.
- **Obsolete regressions** whose failure can no longer be expressed.
- **Game-content values** shard owners customize (era drop rates, loot and skill tables, single-creature tuning). Each customization would otherwise force a test fix.
## How it was decided
1. Every test was classified, with a reason, by a per-shard sweep over all 287 test files.
2. Each proposed deletion was then checked by a second, adversarial reviewer against the surviving tests and the production code.
3. Anything that was the only coverage of a separate code path was **kept**, even where it looked like a duplicate. Examples: the unsigned `SpanReader`/`SpanWriter` methods, the at-location versus bounds enumerators, null-map enumerables, the security filters, archive-journal state transitions. About 130 proposed deletions were rejected this way.
Whole files removed:
- `FishingTests`, apart from 3 tests: the map-edge scan, serpent cleanup, and SOS chest consumption. The SOS test now stops at the first chest instead of measuring a catch rate. The multi-million-cast rate simulations are gone.
- `LordOaksTests` and `MeerEnragedTests` (single-creature tests).
- `AutoArchiveHelperTests`, `JsonDiscoverableTypeAttributeTests`, `DecimalSerializationTests` and `EnumConversionTests` (tautologies, or tests of their own local helpers).
- `NetworkStatsTests`.
- `MigratedDataLoadTests` (a subset of `AllSpawnFilesLoadTests`).
- `SpawnerFixtureCapture`: a capture tool, not a test, and its `version == 12` assert can no longer pass. The `.bin` fixtures it produced stay.
## Tests
- Server.Tests: 811 pass.
- UOContent.Tests: 1,123 pass.
A follow-up PR, stacked on this one, fixes how the remaining tests behave: shared state, CI-vacuous tests, harness duplication.
## Summary
`ManagedArchive.ExtractTarFromStream` joined each tar entry name directly onto the output directory, so an entry such as `../x`, `sub/../../x` or an absolute path was written outside it. The restore path (`AutoArchive` → `ExtractTarZstd`/`ExtractTarGz`/`ExtractTar`) feeds archives through this method.
Each entry now resolves to a full path and must stay under the output root (case-insensitive on Windows). An entry that escapes fails the extraction; the existing `catch` in each `Extract*` method logs it and returns `false`.
The input is the server's own backup archives, so this is hardening against a tampered or foreign archive, not something a player can reach.
## Tests
- `ExtractTar_EntryOutsideOutputDirectory_IsRejected` (`../escaped.txt`, `sub/../../escaped.txt`) and `ExtractTar_RootedEntry_IsRejected`. All three fail without the fix.
- All 18 `ManagedArchiveTests` pass.
`BaseRanged.OnCraft` passed the raw resource type to `ApplyWoodBonuses`, which can be null; `CraftResources.GetFromType` threw on the null key and crashed the server. Every other `OnCraft` override already falls back to `craftItem.Resources[0].ItemType` — `BaseRanged` now does the same.
`GetFromType` also returns `CraftResource.None` for a null type, matching its documented contract for unregistered types, so other callers can't hit the same throw.
Stacked on #2716. Retarget to `main` after it merges.
## Problem
Mobiles a champion summons mid-fight aren't tracked by anything:
| Summoner | Minions |
|---|---|
| LordOaks | Silvani queen |
| `SummonPixiesCounter` (LordOaks, Silvani, Twaulo) | 3-6 pixies per trigger |
| Barracoon | 3-6 ratmen per trigger |
`ChampionSpawn.DeleteCreatures` (called on expiry and when the spawn is deleted) clears only `Creatures` and the champion. These minions survive every reset and stay in the save. I surveyed every content creature that creates other mobiles; the others already clean up after themselves: Meraktus deletes its followers, Harrower kills its tentacles, PlagueSpawn and the Enraged animals delete themselves when their owner is gone, and BogThing eats its boglings.
## Changes
- **`BaseCreature.SpawnMinion(minion, target, location)`:** sets the team, places the minion and sets its combatant. Ownership: if the summoner is a champion spawn's champion, one of its creatures, or one of its minions (found through the spawn region), the minion joins that spawn. It refuses, and deletes the minion, if the summoner isn't on a real map. `SummonPixiesCounter` read `source.Map` even though `MonsterAbility.Trigger` notes it can fire after the creature has left the world.
- **`ChampionSpawn.Minions`:** a new serialized list (v12, `MigrateFrom(V11Content)`) kept apart from `Creatures`:
- killing a minion never counts toward a level, so a leftover summoner can't feed a restarted spawn;
- `DeleteCreatures` deletes minions along with the creatures, which matches when the spawn is deactivated (`ExpireCreatures`, 10 minutes after `Stop`) or deleted.
- **Callers:** LordOaks, `SummonPixiesCounter` and Barracoon spawn through `SpawnMinion`. A summoner that isn't part of a spawn (a GM `[add]`) still makes standalone minions, as today.
## Testing
- `ChampionMinionTests`:
- the champion's queen is listed in `Minions` and deleted by `DeleteCreatures`;
- a minion's minion also belongs to the spawn;
- a Lord Oaks in the region that isn't the spawn's champion keeps a standalone queen.
- UOContent.Tests: 1237 passed, 2 skipped. The Application build is clean.
## Problem
`GetMonsterAbilities()` answers three questions through one virtual: which abilities a creature has (`GetAbility`, `AOS.cs` reflect/barrier, bard fire-breath difficulty), which abilities are enabled right now, and where cooldown state must be cleaned up (`InvalidateNextAbilityTriggers` on death and delete). Every caller assumes it is a pure, stable list. About 70 creatures return a static array; only LordOaks computed it:
```
Mobile.Delete()
→ BaseCreature.OnDelete() → CreatureDeletedEvent
→ MonsterAbility.InvalidateNextAbilityTriggers()
→ LordOaks.GetMonsterAbilities() → CheckQueen() → new Silvani().MoveToWorld(...)
```
- Deleting a Lord Oaks nobody had fought (champion reset, `DeleteCreatures`, GM delete) spawned an orphaned Silvani where he stood.
- The spell damage scalar (`AlterDamageScalarFrom`) spawned her too.
- With the queen dead his list was empty, so cleanup skipped his cooldown entries. The static `Dictionary<BaseCreature, long>` on the ability singleton kept a reference to the deleted creature. `MonsterAbilityGroup` children's entries were never cleaned either.
- Neira threw unholy bones from `AlterDamageScalarFrom`, so any caller asking for a damage scalar (a preview gump, for example) could fire the effect.
## Changes
- **Per-creature cooldowns:** a lazy `Dictionary<MonsterAbility, long>` on `BaseCreature` (`IsAbilityOnCooldown`, `SetAbilityCooldown`, `ClearAbilityCooldowns`). The static dictionaries and the query-driven cleanup are gone, and death still resets cooldowns. Tick comparisons are wraparound-safe.
- **Pure contract:** `GetMonsterAbilities()` is documented as pure. The new `virtual bool CanUseAbility(MonsterAbility)` gates abilities that only work some of the time. Every `TriggerAbility*` loop and `GetAbility` check it.
- **LordOaks:** always lists his abilities; `CanUseAbility` and the 10% spell shield read "queen alive". The queen spawns when he is first aggressed or first given a combatant, never on the internal map. Aggression lands before the damage it causes, so the first spell is still shielded, as before.
- **Neira:** the unholy bone throw moves to `OnDamagedBySpell`.
A follow-up PR makes minions such as the queen, pixies and Barracoon's rats belong to their champion spawn, so a spawn reset cleans them up.
## Testing
- `LordOaksTests` covers three cases:
- querying or deleting an unengaged Lord Oaks spawns nothing;
- aggression spawns the queen exactly once, and the shield and abilities follow her life;
- an Oaks on the internal map never spawns her.
- UOContent.Tests: 1234 passed, 2 skipped.
- A local constructor sweep (every Mobile built and deleted repeatedly) no longer reports LordOaks.
The two `Spawner (225)` entries at 1333,1808 in `shared/felucca/Vendors.json` and `shared/trammel/Vendors.json` name `escortablewanderinghealer`, which no class has. `BaseSpawner` marks the entry `InvalidType`, so the spawner never spawns anything.
This changes the entry to `EscortableMage`, the only `Escortable` class. It already spawns in Britain in `TownsPeople.json`, and a vendor spawner at 1351,1816 nearby uses it too.
If the original intent was a different type, or the spawner should go, I can change that.
Part of #2687, after #2709. One commit per section, in the same order.
### Failures, maps, bottles and colored nets
A GM's cast fails about a quarter of the time in any water, from the fishing table's last row, and deep water is no harder than a river. Maps and bottles are 0.5% per roll from T2A on, and 1 net in 100 is colored in every era.
This removes the last row, so a cast only fails its skill check. From UO:R a cast in deep water checks skill from 65 to 115, through a `Fishing.MutateResource` override. Publish 5, which "was known as the Renaissance Publish", made deep water harder: "because the difficulty level is higher, the rate of success for pulling up SoS bottles will be lower." [O3] From LBR maps and bottles come up more often and 1 net in 10 is colored, both from Publish 16, which came out in parts during LBR [O14].
| Change | Era | Source |
|---|---|---|
| No "nothing" row | All | #2687 found no source for it; "A prepatch, you never failed catching something at GM" [S15]; a GM succeeds every time in lakes and rivers [C1] |
| Deep water: nothing below 65 skill | UO:R on | "The minimum required skill level to catch *anything* in deep water is 65." [S5–S7] |
| Deep water: 10% at 70, 70% at GM | UO:R on | "Even at 70, it is only about a 10% chance of success in deep water." [S14]; 29% of casts fail before Publish 16 and 30% after [F1, F2]; 70% success at 100 offshore [C1] |
| Maps (skill − 80) / 2400, bottles (skill − 80) / 1500 | LBR on | A bottle "about once per hour" until October 2002 [S1–S6], then "a special item (a net, a message in a bottle or a treasure map) about five times per hour" [S7]; bottles went from 0.4% of catches before Publish 16 [F1] to 1.7% after [F2] |
| Colored nets: none, then 10% | LBR on | "With Publish 16, we got a nice surprise--colored special fish nets!" [F4]; "Any net created has a 10% chance of being colored." [F3] |
- From LBR a GM's deep-water catches are 0.8% maps and 1.3% bottles. Chrome counted 0.9% and 1.3% [C1], the FCB 0.7% and 1.7% after Publish 16 [F2]. Nets and big fish stay at 0.5%, close to both counts.
- At Stratics' 280 casts an hour [S1–S6], that is about one bottle an hour from UO:R to UOTD and five nets, maps or bottles an hour from LBR, the two rates Stratics gives.
- The gain chance rises as the success chance falls, so above 65 a cast in deep water gains faster than one near land, Publish 5's "best method for gaining at high levels" [O3]. Below 65 it neither catches nor gains.
- Chrome's table also has three kinds of water near land, with difficulties of 15, 35 and 55. No other source has them, so near land stays at 0–100.
### Deep water is at least 14 tiles from land
`SpecialFishingNet.FullValidation` decides what counts as deep water: where a special fishing net works, where maps, bottles, nets and big fish can be caught, and from UO:R where a cast is harder. It checks the target and 20 points on its four diagonals, 5 to 25 tiles out. Nothing off those points is read, so a spot 8 tiles from shore can count as deep, while one with land 15 to 25 tiles away on a diagonal does not.
The sources give one rule in every era: land at least 14 tiles away. This checks every land tile within 13 of the target. ModernUO has no server lines inside a map, so the only boundary is the map's edge, which counts as land as before. Only land tiles are read, so statics such as rocks are ignored. `SOS.FindLocation` had its own copy of the old check and now calls `FullValidation`, so wrecks follow the same rule.
- 1999 to 2000: "which means you must be at least 14 tiles away from all land" [S1–S4]
- March 2001: "14 tiles away from any land mass" [F5]
- 2001 to 2002: "which means you must be at least 14 tiles away from all land and server lines" [S5–S7]
- Today: "You need to be a minimum of 14 tiles from land or a server boundary to be in ‘deep water’." [O9]
One guide from January 2000 says "16 tiles away from land" [S9]; every other source says 14.
- In the area an SOS picks from on Felucca, 59% of spots pass the old check and 63% pass the new one; in Ilshenar's two sea regions, 16% and 30%.
- The check reads up to 729 land tiles instead of 21: about 6 µs per check in a Release build, against under 0.1 µs before; a cast makes at most two.
### Sea serpents
In ModernUO a sea serpent comes up from fishing only with a treasure map, a bottle or a net, which it carries. A quarter are deep sea serpents. It has no timer, and nothing makes it attack the fisherman first.
This follows the notes of March 28, 1999 and Publish 4. A fished serpent attacks the fisherman at once. Five minutes after it surfaced it goes back to deep water, taking what it carries, unless someone is still fighting it: it took damage in the last two minutes and has not healed to full since, which is what ModernUO's damage entries record. From T2A, at 80 skill or more in deep water, 1% of casts also bring up a serpent that carries nothing, whether the cast catches something or not. 1 serpent in 10 is a deep sea serpent.
| Change | Era | Source |
|---|---|---|
| Attacks the fisherman at once | All | "Sea Serpents will now immediately attack the fisherman that brought them from the deep." [O1] |
| Goes back after 5 minutes unless fought | All | "Sea Serpents will return to deep water after 5 minutes if not attacked." [O1]; "will return to the depths after a brief period if they are not engaged" [S10] |
| 1% of casts bring up an empty serpent | T2A on | "Fishermen at over 80 skill will have a chance to pull up a sea serpent regardless of whether they succeed or fail at fishing when in deep sea." [O2]; 1% at 80 or more, before the skill check [B1] |
| 1 in 10 is a deep sea serpent | All | "A small percentage of Sea Serpents will be Deep-Sea Serpents." [O1]; about 10% [B1]; 9.8% and 10.8% of the serpents counted [F1, F2] |
- Publish 4 gives no rate. The 1% comes from a post by Hance, a UO developer, in December 2000 [B1]; T2A uses it too, since it is the rule Publish 4 brought in (our inference). The FCB's counts agree: 1.76% of casts brought up a serpent and 55% of serpents carried nothing, so about 1% of casts brought up an empty one [F1] (our calculation).
- Before Publish 4 serpents came up "on failure" [O1], and Stratics kept "if you are fishing with 80+ skill and you fail" from July 1999 to 2002 [S1–S7]. Publish 4 is the last rule in T2A, so it is the one used from T2A.
- Hance also relays a side comment that the empty serpent cancels the rest of the cast [B1]. If it did, no cast could bring up two serpents, but the FCB saw two from one cast on 0.96% of serpents [F1], and Chrome caught empty serpents together with fish and footwear [C1]. So the cast goes on.
- Hance's post also needs room in the backpack for a fish and fish left in the spot [B1]. A spot with no fish left already ends the cast before the roll, unless an SOS keeps the cast going (next section). A full backpack doesn't: ModernUO doesn't check for room before a cast, so neither does the serpent.
- Serpents from a special fishing net are unchanged; Hance's post says nets don't follow its formula [B1].
### The SOS chest
In ModernUO, a fisherman carrying an SOS within 60 tiles of its spot brings up the chest on 1 catch in 8 and a shipwrecked item on the other 7. The SOS also lets a cast go on where the fish have run out.
This makes the chest 1 catch in 4, in every era. From UO:R an SOS needs 65 skill, and below that the cast is an ordinary one.
| Change | Era | Source |
|---|---|---|
| The chest on 1 catch in 4 | All | "You have a 25% chance of getting the chest and a 75% chance of getting something else with each cast." [F3]; "a 25% chance to fish up the actual treasure chest" [S7] |
| An SOS needs 65 skill | UO:R on | 65 in the chest's row of Stratics' table: "The skills mentioned above are minimum skills needed to fish up the corresponding item." [S5–S7] |
- No source gives a number before October 2002, or any for ModernUO's 1 in 8, so the 25% applies in every era. The other catches keep ModernUO's seven kinds of shipwrecked items, equally likely. Hanse's die splits them in three: "One roll is body parts; another roll is shells, barrel staves, globes, etc.; another roll is paintings, broken clocks, etc.; and another is the chest itself." [F3]
- Stratics' guide of November 2000 has neither the chest's row nor the 65 for deep water [S4]. The May 2001 version, during Third Dawn, has both: "The minimum required skill level to catch *anything* in deep water is 65." [S5] The first section applies that from UO:R, after Publish 5 made deep water harder [O3], so the chest follows it (our inference). An SOS's spot is picked in deep water, but a cast within 60 tiles of it can be nearer land, so the 65 is checked on its own.
- Below 65 there are no shipwrecked items either, because Hanse's die is rolled once you "can even attempt to get a treasure chest" [F3] (our reading). Before UO:R an SOS still works at any skill; the only period line on it is "You do not have to be GM Fisherman to actually fish up the treasure from a bottle, only to fish up A bottle." [S9]
### Fishing reveals from T2A
ModernUO reveals a hidden fisherman from ML. The notes of March 28, 1999 already say "If you are hidden, fishing will now unhide you." [O1], so this reveals from T2A, at the same two points: the splash and the end of the cast.
### Big fish and the leviathan's artifact
In ModernUO every big fish weighs 3 to 200 stones and gives max(16, weight) / 4 steaks, and the net thrower has a 1 in 4 chance at an artifact at any skill.
| Change | Era | Source |
|---|---|---|
| Big fish weigh 100 stones | Before SE | "They weigh 100 stones each" [S5–S7]; Publish 28 "Added variable weight and fisherman's mark on big fish" [O6] |
| A steak per 5 stones, at least 4 | SE on | "now its weight / 5 will determine number of fish steaks (minimum of 4)" [O7] |
| The net thrower's own chance at an artifact: 1 in 4, only at GM | SE on | "Give the fabled net thrower (if a GM Fisherman) a 1 in 4 chance of getting a minor artifact (not modified by luck or damage)." [O6] |
- Before SE a big fish keeps ModernUO's weight / 4 steaks, 25 for 100 stones: no source gives a number before Publish 29.
- Publish 27 names only the attackers' chance: "Minor Artifacts will have a chance of spawning in each attacker's backpack after a paragon creature or a leviathan is slain." [O5] Publish 28 adds the thrower's roll [O6], so before SE there is none (our inference). The attackers' chance is unchanged.
### Magic fish
From SE the prized, wondrous and truly rare fish give +1 for 12 seconds instead of +5 for a minute [C1]. A Stratics FAQ of January 5, 2005 already says "The stat boost fish used to provide the person who ate it a +5 boost, but now it's only one or two points, and lasts only a short while." [S16]
- Stratics still gives +5 in December 2002 [S7] and no note dates the change. [S16], during SE, is the earliest source with the new values, so the eras before SE keep +5 and ModernUO's minute (our inference).
- Today's guide gives "Adds 8 – 12% to your Dexterity for 150 seconds when eaten" [O9]. No note dates that change either, so it is left out.
### The white pearl at an SOS's spot
From ML the white pearl is a bonus on top of the catch, but it went through `Fishing.Construct`'s SOS branch: at an SOS's spot the fisherman read "You have found a white pearl!" and got a shipwrecked item or the chest, a second SOS roll in the same cast. The pearl now skips that branch.
The keys are #2687's, plus:
- [O14] uo.com, the 2002 publishes: Publish 16 in parts on April 23, July 23, September 20 and December 2, 2002. https://uo.com/wiki/ultima-online-wiki/technical/previous-publishes/2002-2/
- [S14] Stratics fishing board, FAQ 1.2 "How do I train fishing?", January 5, 2005. https://web.archive.org/web/20050410171430/http://boards.stratics.com/php-bin/uo/showthreaded.php?Cat=&Board=uofisherman&Number=5622892
- [S15] Stratics fishing board, a post on Publish 5's SOS change, April 25, 2000. https://web.archive.org/web/20000511000424/http://boards.stratics.com:80/cgi-bin/forums/showflat.pl?Cat=21&Board=uofisherman&Number=144300&page=0&view=collapsed&sb=5
- [S16] The FAQ of [S14], 1.4 "What's with the horned leather and scales? (Resources)", January 5, 2005, in the FAQ's flat view. https://web.archive.org/web/20050410171356/http://boards.stratics.com/php-bin/uo/showflat.php?Cat=&Number=5622877&page=0&view=collapsed&sb=5&o=&fpart=1
- [F1] The Fishin' Statistician, 11,101 casts from August 2001 to April 2002, before Publish 16. Skill and water not stated; most likely GM and mostly deep water, since the counts include big fish and serpents. https://web.archive.org/web/20020609120859/http://www.thefcb.com:80/novellettes/stats.html
- [F2] The same page, 7,371 casts from July 2002 to February 2003, after Publish 16, with nets, maps and bottles counted too. https://web.archive.org/web/20030216074120/http://www.thefcb.com:80/novellettes/stats.html
- [F3] The FCB's interview with Hanse, an OSI designer, October 21, 2002. https://web.archive.org/web/20021104194424/http://www.thefcb.com:80/Hanse.htm
- [F4] The FCB's encyclopaedia, colored nets, last modified July 13, 2002. https://web.archive.org/web/20020715111230/http://www.thefcb.com:80/encyclopaedia/colornets.html
- [F5] The FCB's "Illustrated Treatise on Deep Water Defined", by Captain Pickles and Mason, last modified March 20, 2001. https://web.archive.org/web/20010419135823/http://www.thefcb.com:80/novellettes/deep-water.html
- [B1] Brikan's skill guide, which quotes a post on Origin's boards by Hance, a UO developer, on December 20, 2000; most likely Hanse, the OSI designer the FCB interviewed in 2002. Page last modified February 3, 2007. https://web.archive.org/web/20070715201811/http://www.jgnn.com:80/uo/bk/2skill/skill.htm
- [C1] Chrome's fishing page on OSI's Yamato shard, captured November 25, 2007: 5,000 offshore catches at GM, a table of success by water and what each magic fish does. https://web.archive.org/web/20071125144104/http://www.geocities.co.jp:80/Playtown-Yoyo/4812/fishing02.html
Companion to #2712, which covers HireRanger's refused `EquipItem`. A sweep that built every constructible Mobile 2,000 times under each expansion found these other outfits that put two items on one layer. They don't leak, because `AddItem` skips `CheckEquip`, but the NPC wears both items and the client draws only one.
| NPC | Conflict |
|---|---|
| Miner, Ranger, Thief | Own shirt/pants/ThighBoots added on top of the random `BaseVendor.InitOutfit` (shoes doubled on every spawn) |
| MasterTheophilus | Shoes + Robe added over `EvilMageLord`'s Sandals + Robe |
| Merchant | Shirt + FancyShirt, or PlainDress + FancyDress |
| IronWorker | Bandana/JesterHat + Bascinet |
| MeerCaptain | One-handed sword + Crossbow |
| BakeKitsune | `BodyMod` isn't saved, so after a restart `RemoveDisguise` bailed and the next `Disguise()` stacked a second robe and pair of shoes |
## Changes
- `BaseCreature.ReplaceEquipment(item)` equips an item in place of whatever an inherited outfit put on its layer, and deletes the item if it still can't be equipped.
- Miner, Ranger, Thief, MasterTheophilus: their own pieces replace the inherited ones.
- Merchant and IronWorker: dropped the duplicate roll. Each kept the item the client was already drawing, which is the last one sent: the fancy set and the Bascinet.
- MeerCaptain: the sword goes into its pack as loot. **Behavior change:** `Mobile.Weapon` checks the one-handed layer first, so it was fighting with the sword despite its archer AI, its Archery skill and the bolts in its pack. It now uses the crossbow.
- BakeKitsune: `RemoveDisguise` strips the disguise without checking `IsBodyMod`.
## Testing
- I re-ran the sweep (local diagnostic, not committed; needs client tiledata) at 500 rolls × 12 expansions. Only HireRanger (#2712) and LordOaks (separate fix coming) remain.
- UOContent.Tests: 1218 passed, 2 skipped.
