## 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.
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.
ShortPants and StuddedLegs both occupy Layer.Pants, so the second EquipItem on a male ranger is refused and its item stays in the world with no parent. Nothing deletes such an item, so every male ranger created leaves one behind in the save.
StuddedLegs already cover the layer and female rangers wear them, so the male ranger now skips the shorts. A test creates and deletes rangers and checks that World.Items does not grow.
Part of #2687. Magic fish and footwear roll the same in every era: magic fish get rarer with skill (5% at 0 skill, 1% at GM) and footwear has a 20% chance at 0 skill. The leviathan rolls the generic 1% for its level 5 map.
A fishing table row can now end at an expansion, so each era gets its own magic fish and footwear rows. The chances at GM come from the Fishing Council of Britannia's cast counts before and after Publish 16, which came out in parts from April to December 2002, during LBR [O14]. The leviathan gets its 25% through a `BaseCreature.TreasureMapChance` override; leviathans only come from the fabled net, which already needs AOS.
| Change | Era | Source |
|---|---|---|
| Footwear: 75% at 0 skill, 2.5% at GM | T2A to UOTD | "The beginning fisherman will find this item on his hook most of the times" [S1–S7]; 2.5% of catches [F1] |
| Footwear: 75% at 0 skill, 10% at GM | LBR on | The same quote; 10.5% of catches [F2] |
| Magic fish: 80 skill, 2.5% at GM | T2A to UOTD | 80 from July 1999 to October 2002 [S1–S6, S9]; 2.4% of catches [F1] |
| Magic fish: no minimum, 5% at GM | LBR to SA | 0 from December 2002 [S7], so the minimum went in late 2002, during LBR; 5.6% of catches [F2] |
| Magic fish: 80 skill again | HS on | Still 0 in June 2010, during SA [S13]; 80 today [O9]. HS is the first expansion after that |
| Leviathan's map: 25% | AOS | Publish 27: "It also has a 25% chance to carry a level 5 treasure map." [O5] |
- These are chances per roll. The FCB's counts leave out empty casts, so they line up once the "nothing" row #2687 lists is gone (a follow-up PR removes it); while it stays, footwear is 3.2% of a GM's catches before LBR and 12% from LBR.
- No source counts footwear below GM, so 75% at 0 skill only follows Stratics' "most of the times". The numbers in my comment were placeholders.
- 80 again from HS is the one change beyond my comment, which proposed no minimum from LBR on.
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/
- [F1] The Fishin' Statistician, 11,101 casts from August 2001 to April 2002, before Publish 16; skill not stated, most likely GM. 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; skill not stated either. https://web.archive.org/web/20030216074120/http://www.thefcb.com:80/novellettes/stats.html
- [S13] Stratics' fishing guide, captured June 5, 2010. https://web.archive.org/web/20100605065528/http://uo.stratics.com:80/content/skills/fishing.shtml
GetDelay sets min to 6.0 - del * 0.75 and max to 6.0 - del * 1.25, so for any Magery above zero min is the larger value. The draw min + (max - min) * r still covers the right interval, but it runs downward and the names say the opposite.
Swap the two definitions so min is the lower bound. The interval and its uniform distribution do not change; for a given random draw the delay comes out mirrored.
## Summary
Stacked on #2676. Review that first; this PR's diff is against it.
Items players sell to NPC vendors go into the vendor's buyback (`ShopBuy`) pack. That pack was purged only when someone opened the buy menu, so a vendor that players sold to but never bought from grew without limit.
- A server operator reported 250k+ items piling up from players crafting and selling on a macro.
- Every one of those items was saved.
- Purging or deleting that vendor then sent one remove packet per item to every nearby client and disconnected them.
The behavior is inherited from RunUO.
- **`VendorBuybackPack`** replaces the plain backpack on `Layer.ShopBuy`.
- It opts in to `SkipsChildSerialization`, so buyback items are never saved.
- Its contents are private like any container's, so removing them reaches only the vendor (no client) and openers. Purges and deletes send nothing to bystanders.
- **Capacity.** The buyback holds at most `250 − buy-list entries` items, which is what the buy gump can show. The oldest items are evicted first. At capacity 0 the sold item is consumed and the seller is still paid.
- **Purge on the restock grid.**
- `Restock()` empties the buyback.
- A purge-only timer, armed on the first sale, covers vendors nobody buys from. It is due at the next restock boundary after now, so a vendor whose restock hasn't run for hours doesn't purge an item the moment it's sold.
- The timer never restocks, so stock doubling and halving is unchanged.
- The old per-item one-hour `LastMoved` check is gone.
- **Existing saves.** A legacy `ShopBuy` pack is replaced on the first tick after load. Thanks to #2676, deleting a pack of 250k+ items takes well under a second.
**Behavior changes for shard operators:**
- A buyback item lasts until the next restock boundary (at most `RestockDelay`), not one hour after its sale.
- A vendor that changes maps drops its buyback.
- A buy window that is open during a purge can show a row whose item is gone. Buying that row does nothing, and reopening shows the current list.
## Test plan
- [x] `VendorBuybackPackTests`: evicts oldest first; capacity 0 consumes the item; purge; a stale front entry can't spin eviction
- [x] `VendorBuybackTests`:
- purge due time stays on the restock grid, including a stale restock and a restock time in the future;
- partial stack sale: the split goes to buyback and the seller keeps the remainder;
- zero capacity consumes the item and still pays;
- restock purges and clears the timer, and the next sale re-arms it;
- deleting the vendor cancels the timer;
- a legacy 20k-item pack is replaced.
- [x] UOContent.Tests and Server.Tests green; schema generator clean
- [x] Manual in-game pass:
- sell and buy back, including a partial stack;
- over-capacity sale;
- purge with no buyer;
- `[restock`;
- vendor facet change;
- restart;
- legacy save with a large `ShopBuy` pack;
- mass selling with bystanders present;
- deleting a vendor with a full buyback.
The fishing mutate table offers every special catch in every era, so a pre-T2A shard fishes up boots, magic fish, treasure maps and bottles (added in the March 1999 fishing update), and a T2A shard gets the special fishing net and big fish, which no T2A publish or guide lists.
Each entry now carries the expansion that added it; the chances are unchanged. The nets in SOS chests and on krakens need UO:R as well, and before SE ancient SOS roll 1 in 40 and their chests aren't hued white, since Publish 28 changed both.
Two fixes from the same notes come with it. An SOS now only works in the facet it was found in, so kraken and leviathan bottles are made at death with the creature's facet. Big fish, their trophy deed and their trophy show the fisherman's mark and weight only from SE.
## Why each change
### The rule
- `Core.T2A`, `Core.UOR`, `Core.SE`… mean "the shard is this expansion **or newer**".
- An expansion includes every publish that came out while it was the newest one. Publish 27 (September 2004) came out before Samurai Empire (November 2004), so it belongs to AOS.
- An item can't drop on a shard older than the expansion that added it.
- When the sources only narrow it down to a window that spans two expansions, the item takes the older one, so no shard loses something it may really have had.
### When each fishing item arrived
| Expansion | Released | What it added to fishing | Sources |
|---|---|---|---|
| T2A | Oct 1, 1998 | Boots, magic fish, treasure maps, bottles (update of March 17, 1999) | [1] |
| UO:R | May 4, 2000 | Special fishing net (first listed in August 2000) | [2], [6], [7] |
| UO:R or Third Dawn | | Big fish (first listed between November 2000 and March 2001) | [3], [8], [9] |
| AOS | Feb 11, 2003 | Ancient SOS at 1 in 40, fabled fishing net (Publish 27, September 14, 2004) | [4] |
| SE | Nov 2, 2004 | Ancient SOS at 1 in 25, white ancient chests, the fisherman's mark on big fish (Publish 28, January 12, 2005) | [5] |
Release dates: [13].
### `Fishing.cs`
**`MutateEntry` gets an `Expansion` field and constructor argument.**
Each row has to know which expansion added it. `MutateEntry` is private to `Fishing`, so no other code is affected.
**`MutateType` skips rows newer than the shard:**
```csharp
if (Core.Expansion < entry.m_Expansion)
{
continue;
}
```
A skipped row behaves as if it weren't in the table. Every other row rolls exactly as before.
**The rows:**
| Row | Expansion | Why |
|---|---|---|
| Boots, shoes, sandals, thigh boots | T2A | The March 17, 1999 update is the first time fishing gives anything but fish: "you now have a chance of pulling up … One of any of several types of boots". [1] |
| Magic fish (prized, wondrous, truly rare, peculiar) | T2A | Same update: "Magical fish, which can boost stamina, strength, intelligence, or dexterity". [1] |
| Treasure map | T2A | Same update: "Treasure maps". [1] |
| Message in a bottle | T2A | Same update: "Messages in bottles". [1] |
| Special fishing net | UO:R | Missing from the 1999 list [1] and from the March 2000 fishing changes, which only mention treasure maps and SOS bottles [2]. Stratics' fishing guide doesn't have it from July 1999 to July 8, 2000 [6] and has it on August 10, 2000 [7], after UO:R came out. |
| Big fish | UO:R | Stratics' fishing guide doesn't have it from July 1999 [6] to November 7, 2000 [8] and has it on May 24, 2001 [9]. Publish 11 (March 14, 2001) already lets taxidermy kits use it [3]. So it is certainly not T2A. It came at the end of UO:R or with Third Dawn (March 7, 2001); the sources can't tell which, so it takes UO:R, the older one. |
| Nothing (the empty row) | None | Exists in every expansion; unchanged. |
**SOS chest, white hue: `if (sos.IsAncient && Core.SE)`**
Publish 28: "Hued level 4 fishing chests white." [5] It came out after SE, so before SE the ancient chest keeps its normal colour.
**SOS chest, net: `if (Core.UOR) { chest.DropItem(...); }`**
You can fish up an SOS chest from T2A on, because bottles are T2A [1]. The net inside it is UO:R (see the table), so before UO:R the chest comes without a net. The fabled net only goes in ancient chests, and those already need AOS [4].
**SOS: `from.Map == sos.TargetMap`, in `CheckResources` and `Construct`**
Both accepted an SOS whenever the fisher was in Felucca or Trammel, without comparing the SOS's `TargetMap`, so a Trammel SOS paid out in Felucca at the same spot. Publish 5: "SOS bottles and treasure maps will only work in the land they are found in (for instance, a map found in Felucca will only work in Felucca)." [14] Treasure maps already check their facet.
### `MessageInABottle.cs`
**`GetRandomLevel`: `Utility.Random(Core.SE ? 25 : 40) < 1`**
Publish 27 added ancient SOS at "a 1 in 40 chance" [4]. Publish 28 "Changed Ancient SoS spawn chance from 1 in 40 to 1 in 25" [5]. So AOS rolls 1 in 40, SE and newer 1 in 25. The `Core.AOS` check was already there.
### `Kraken.cs`
**Bottle, now made in `OnBeforeDeath`: `if (Core.T2A && Utility.RandomDouble() < .05)`**
Bottles don't exist before T2A [1], so a kraken can't carry one there. The 5% is unchanged; no official source gives a number. The kraken is in Stratics' 1999 creature list [10]; its kraken page says "Loot: Unknown" in January 2000 [11] and "Loot: Messages in a Bottle, Treasure Maps, 300 - 600 gold" in October 2000 [12].
The constructor packed the bottle before the kraken had a map, so it always pointed to Trammel; with the SOS check above, a kraken killed in Felucca would drop an SOS that only works in Trammel. It is now made at death with the kraken's facet, as `BaseCreature.OnBeforeDeath` does for treasure maps; the net already does the same for the kraken it brings up.
**Net: `if (Core.UOR) { PackItem(new SpecialFishingNet()); // Confirm? }`**
Nets are UO:R [6], [7]. The `// Confirm?` stays: in 2000 Stratics says the net was "loot on Sea Serpents" [7], [12], not on krakens. Which creature drops the net is out of scope; this PR only fixes the expansion.
### `Leviathan.cs`
**Bottle, now made in `OnBeforeDeath`**
Same as the kraken: it was packed in the constructor and pointed to Trammel. It is now made at death with the leviathan's facet, and still drops every time.
### `BigFish.cs` and `TaxidermyKit.cs`
**Mark and weight: `if (Core.SE && Weight >= 20)`, and the same in the trophy and its deed**
Publish 28: "Added variable weight and fisherman's mark on big fish, big fish trophy deed, and big fish trophy. Any big fish caught that is below 20 stones will not display weight or fisherman's mark." [5] It came out after SE, so before SE none of the three shows them.
### What this PR doesn't change
No chance, skill threshold or row order changes. The open questions (which creature drops the net, the exact expansion of big fish, its weight before Publish 28, the rates) are in #2687.
## References
1. Origin Update Center, update of March 17, 1999: the fishing overhaul that adds boots, magic fish, treasure maps and bottles. https://web.archive.org/web/19990417083713/http://update.owo.com/latest.html
2. Publish 4, March 8, 2000: "Every time you successfully fish up a treasure map or SOS bottle a sea serpent will surface and attack." https://uo.com/wiki/ultima-online-wiki/technical/previous-publishes/2000-2/2000-publish-04-8th-march/ (copy from the time: https://web.archive.org/web/20000408162342/http://update.uo.com/design_180.html)
3. Publish 11, March 14, 2001: taxidermy kits accept "Big Fish". https://uo.com/wiki/ultima-online-wiki/technical/previous-publishes/2001-2/2001-publish-11-14th-march/ (copy from the time: https://web.archive.org/web/20010331072129/http://update.uo.com/design_321.html)
4. Publish 27, September 14, 2004: "There is a 1 in 40 chance that any message in a bottle becomes 'an ancient SOS'", "Added fabled fishing nets and ancient sos treasure". https://uo.com/wiki/ultima-online-wiki/technical/previous-publishes/2004-2/publish-27/
5. Publish 28, January 12, 2005: "Hued level 4 fishing chests white", "Changed Ancient SoS spawn chance from 1 in 40 to 1 in 25". https://uo.com/wiki/ultima-online-wiki/technical/previous-publishes/2005-2/publish-28-12th-january/
6. Stratics fishing guide without the net or big fish, the same text from July 24, 1999 to July 8, 2000: last modified July 24, 1999, https://web.archive.org/web/19991009203653/http://uo.stratics.com:80/fishing.htm ; last modified July 8, 2000, https://web.archive.org/web/20000711071050/http://uo.stratics.com:80/fishing.shtml
7. Stratics fishing guide, last modified August 10, 2000: lists the special fishing net, "found as loot on sea serpents". https://web.archive.org/web/20000831173915/http://uo.stratics.com:80/fishing.shtml
8. Stratics fishing guide, last modified November 7, 2000: still no big fish. https://web.archive.org/web/20001209013100/http://uo.stratics.com:80/fishing.shtml
9. Stratics fishing guide, last modified May 24, 2001: lists big fish. https://web.archive.org/web/20010608153326/http://uo.stratics.com:80/content/skills/fishing.shtml
10. Stratics Hunter's Guide menu, last modified July 4, 1999: the kraken is in the creature list. https://web.archive.org/web/19991013090543/http://uo.stratics.com:80/hunters/huntmenu.htm
11. Stratics kraken page, last modified January 1, 2000: "Loot: Unknown". https://web.archive.org/web/20000119061647/http://uo.stratics.com:80/hunters/kraken.html
12. Stratics kraken page, last modified October 15, 2000: "Loot: Messages in a Bottle, Treasure Maps, 300 - 600 gold"; the net "can be found as loot on Sea Serpents". https://web.archive.org/web/20001017190252/http://uo.stratics.com:80/hunters/kraken.shtml
13. Expansion release dates: https://en.wikipedia.org/wiki/Ultima_Online#Expansions_and_follow-up_releases
14. Publish 5, April 27, 2000: "SOS bottles and treasure maps will only work in the land they are found in". https://uo.com/wiki/ultima-online-wiki/technical/previous-publishes/2000-2/2000-publish-05-27th-april/ (copy from the time: https://web.archive.org/web/20000510032101/http://update.uo.com/design_196.html)
## Summary
**Followers of a deleted character.** `PlayerMobile.OnDelete` deleted stabled pets and left every active follower pointing at the deleted mobile. Pets stayed controlled by nobody, and after a restart they loaded as owned pets with no owner. Now, when a character is deleted:
- bonded or dead pets and summons (controlled or not) are deleted;
- unbonded pets are released: they go wild and despawn on the abandoned-pet timer;
- escorts and hirelings are left alone, because they already notice a deleted master and walk off on their own.
**Familiars and animated dead that never unregistered.** `BaseCreature` unregistered them in `OnAfterDelete`, after `OnDelete` had already cleared the master. Both calls did nothing, so `SummonFamiliarSpell.Table` and the animated-dead registry kept deleted creatures until the caster's next cast. The unregisters now run in `OnDelete`, before the master is cleared.
## Test plan
- [x] `DeletedMasterTests` (6 cases, all failing on main): unbonded pet released; bonded and bonded-dead pets deleted; controlled and uncontrolled summons deleted; familiar unregistered on delete
- [x] UOContent.Tests (1184 passed, 2 skipped) and Server.Tests (907) green
## Summary
After #2670, `BaseCreature` stores one master. Callers still reached it through three differently gated names plus hand-rolled combinations of the `Controlled`/`Summoned` flags. Each gate is now one named view:
| Member | Definition | Meaning |
|---|---|---|
| `Master` | the reference; its setter does the follower bookkeeping; serialized as before | whoever the creature answers to |
| `ControlMaster` | `Controlled ? Master : null` | the owner |
| `SummonMaster` | `Summoned ? Master : null` | the summoner |
| `GetMaster()` | `ControlMaster ?? SummonMaster` | who answers for the creature |
`ControlMaster` and `SummonMaster` stay settable so custom code keeps compiling: both setters assign `Master`. There is no guard, so a write through either one can read back as null until the matching flag is set.
An enraged creature is the one shape with a master and neither flag. It keeps its meer through `Master`, while `GetMaster()` stays null, so the meer never answers for it (notoriety, kill credit).
Readers move onto the view they mean:
- "Owner or summoner" pairs and `ControlMaster ?? SummonMaster` collapse into `GetMaster()`: pack instinct, familiars, golem, house access, mounts, pack horse, Solen friendship, spell target and guild checks, and the `[` pet command.
- Checks such as `Controlled && ControlMaster == from` drop the flag the view already applies. Checks that don't reduce keep it: `Controlled && ControlMaster != from`, a possibly-null `from`, and `MageAI.CanDispel`'s `Summoned &&`.
**No behavior change, save format unchanged** (the schema generator produces no diff). Edge cases for custom code and shard operators:
- `SummonMaster = x` on a plain pet now replaces its owner (#2670 made that a no-op), and `ControlMaster = null` on an uncontrolled summon now clears its caster (it did nothing before).
- `SpellHelper.GetGuildFor` no longer falls back to an enraged creature's meer. Meer mages have no guild.
## Docs
- `dev-docs/content-patterns.md` § Masters: which view answers which question, how to set a master, and which flag checks are redundant.
- `dev-docs/runuo-migration-docs/09-items-mobiles-creatures.md` § Masters: RunUO's two-field reads mapped onto the views, plus the gotcha for creatures that set `SummonMaster` without `Summoned` (the write compiles, the read now returns null; use `Master`).
- The `modernuo-content-patterns` and `migrate-items-mobiles` skills point at both.
## Test plan
- [x] `MasterViewsTests`: all four views across wild, pet, controlled summon, uncontrolled summon and enraged shapes; reassigning `Master` moves follower slots; `SetControlMaster(null)` keeps an uncontrolled summon's caster; deleting a summon returns the caster's slots
- [x] Serialization tests updated to the new views (round-trip, legacy v22, v23 migration)
- [x] UOContent.Tests (1178 passed, 2 skipped) and Server.Tests (907) green
## Summary
A meer mage fights `FightMode.Evil` and its enraged creatures carry -1000 karma, so `BaseCreature.IsEnemy` made each an enemy of the other:
- The meer acquired its own enraged creatures as ordinary targets.
- An enraged creature fell back to attacking its meer when nothing else was in range. It isn't `Summoned`, so it reached the meer through `AcquireNewFocusMob`'s summon-master fallback.
`BaseEnraged` and `MeerMage` now override `IsEnemy` to exclude each other. Another meer's creatures stay enemies.
`BaseEnraged` also pulled its meer into fights against any creature with a `SummonMaster`, which included another meer's enraged creatures. It now checks `GetMaster()`, matching the "controlled/summoned" OSI note in that method.
## Test plan
- [x] `MeerAndItsEnragedCreature_AreNotEnemies`: fails before the fix
- [x] `AnotherMeersEnragedCreature_StaysAnEnemy`
- [x] `MeerAndItsEnragedCreature_NeverAcquireEachOther`: fails before the fix (the meer acquired its own creature)
- [x] UOContent.Tests (1171 passed, 2 skipped) and Server.Tests (907) green
## Summary
`BaseCreature` stored the pet owner (`ControlMaster`) and the summoner (`SummonMaster`) in two fields. Every flow that manages pets kept them in step by hand: summon, transfer, stable, claim, ball of summoning and GM obey. They now share one `_master` field, and the two properties read it through the Controlled and Summoned flags:
```csharp
ControlMaster => Controlled ? master : null
SummonMaster => Controlled && !Summoned ? null : master
```
Every existing reader keeps its meaning:
- `ControlMaster` is still "the owner".
- `SummonMaster` is still "the summoner, controlled or not".
Blade spirits, energy vortexes, clones, undead minions and enraged creatures (a summon master without `Summoned`) never become visible through `ControlMaster`, so the ungated readers (ML notoriety, aggressor entries and similar) behave as before.
- Setting `SummonMaster` on a plain pet does nothing, instead of quietly replacing the owner.
- `SetControlMaster` now keeps both masters in step, so the hand-written `SummonMaster` copies are removed from ball of summoning, GM obey, pet transfer, auto-stable and claim, the animal trainer's stable and claim, and jail stabling.
## Serialization
BaseCreature moves to v24 with one serialized master (slot 15); the old `SummonMaster` slot is dropped.
- `MigrateFrom(V23Content)` resolves absent fields through the same default-value methods the live reader uses.
- A save that carried both masters (a summon traded before ownership followed transfers) resolves to the owner. Legacy pre-codegen streams follow the same rule.
Requires `ModernUO.Serialization.Generator` 4.2.0 (#2671). `UOContent.Tests` was still on 4.1.0 and is bumped to match.
## Test plan
- [x] `MasterShapes_RoundTrip`: controlled pet, controlled summon, energy vortex, enraged creature
- [x] `SummonMaster_OnPlainPet_KeepsTheOwner`: owner and follower counts unchanged
- [x] `LegacyV22Stream_DifferingMasters_OwnerWins`
- [x] `V23Stream_MigratesToOneMaster_AndDefaultsAbsentFields`: both master slots -> owner; flags past bit 32; absent fields take class defaults
- [x] UOContent.Tests (1168 passed, 2 skipped) and Server.Tests (907) green
Readers of `ControlMaster` that act on "this creature is a player's pet" now also require
`Controlled`. Nothing sets a master on an uncontrolled creature today, so behavior does not
change, except that the ML notoriety branch was the one read with nothing in front of it.
The guard keeps these readers correct if a creature ever keeps a master without being
controlled (a suspended-control state such as a charm or turn-pet ability, or folding
SummonMaster into one master field).
- Notoriety (ML): a creature only takes its master's notoriety while controlled
- BaseCreature.AggressiveAction: the aggressor link to the master only applies to controlled pets
- PlayerMobile.AutoStablePets: uncontrolled summons (blade spirits, energy vortexes) are skipped
- CrystalCaveBarrier: only a controlled pet passes on its owner's quest progress
- Discordance: the own-pet exception only applies to a controlled pet
- Dismount (ML): the remount block only goes to the owner of a controlled pet
Also removes the "Summons from monsters can attack players" branch in Mobile_AllowHarmful.
It has been unreachable since #2000: a creature with no player master returns from the
NPC check above it.
Tests: UOContent.Tests 1158 passed.
Supersedes #2658 (rebased onto main; edits by maintainers were not enabled on the original branch). Credit to @MithrilHammer for both fixes and the regression tests; their three commits are kept as authored.
Addresses two items from #2627:
- The detect-hidden cooldown clamps its divisor to at least one. A creature whose effective intelligence reaches zero now uses the existing capped cooldown instead of throwing.
- Pet friends are unique. `AddPetFriend` ignores an existing friend, so one removal revokes the membership.
On top of that, `AddPetFriend` and `RemovePetFriend` now go through the generated `AddToFriends` / `RemoveFromFriends`, which mark the creature dirty. `Friends` goes back to null when the last friend is removed, and removing a non-friend no longer touches the list.
Duplicates already stored in saves are not migrated.
## Tests
`DetectHiddenTimerTests` (7 cases) drives the real AI timer and checks cooldown bounds and retry timing. `PetFriendMembershipTests` (3 cases) covers duplicate adds, removal, and re-adding without disturbing other friends. `UOContent.Tests`: 1158 passed, 2 skipped, 0 failed on Windows.
#2132 let a player's energy vortex or blade spirits in pre-AOS Felucca target their own caster, gated by `FollowsAcquireRules`. #2232's AI overhaul moved the summon checks into `IsInvalidSummonTarget`, which excluded the caster in every era, and dropped the gate. Since then `FollowsAcquireRules` has had no readers and summons have never turned on their caster.
## Changes
- `IsInvalidSummonTarget` reads `FollowsAcquireRules` again. A summon that follows the acquire rules spares its caster and anyone the caster's harmful spells would spare. The vortex and blade spirits don't follow them in pre-AOS Felucca when a player summoned them.
- The animated-dead restrictions (no players, no other animated dead, no pets) apply to every animated summon regardless of that setting.
- `AcquireNewFocusMob` sets a berserk summon's caster aside and targets them only when nobody else is available. #2132 meant to prefer others, but its check only worked when the caster was scanned before the other candidates, and the vortex's Int-plus-Magery ranking usually puts a nearby caster first.
- The AOS summon-target check no longer dereferences a missing `SummonMaster`. A summon whose caster was deleted before a reload comes back without one.
Controlled summons are unaffected: since #2665 they target through `HandleControlled` and never reach this scan.
## Tests
`SummonAcquireRulesTests` has 6 cases, skipped without client map data. Pre-AOS Felucca: a vortex and blade spirits each target their caster when the caster is alone, and a vortex targets a bystander even when the caster ranks higher. Pre-AOS Trammel and AOS Felucca: the caster is still spared. An AOS scan next to a summon with no master doesn't throw. `UOContent.Tests`: 1148 passed, 2 skipped, 0 failed on Windows.
Supersedes #2657 (rebased onto #2662, which the original branch conflicted with). Credit to @MithrilHammer for the diagnosis, the fix and the regression tests; their three commits are kept as authored.
Fixes#2656.
## Root cause
When a guarding pet's combatant hides or becomes invalid, its AI calls `AcquireFocusMob` to find a new target. A controlled creature with no `ControlTarget` (Guard normally has none) fell out of `HandleControlled` and into the wild-creature scan, where `IsEnemy` treats any player as an enemy. With `FightMode.Closest`, the owner won. The fall-through dates to #2232. RunUO never let a controlled creature reach the wild scan.
## Changes
- `AcquireFocusMob`: a controlled creature returns whatever `HandleControlled` decides. Guard uses `FindGuardTarget`, Attack uses its `ControlTarget`, and every other order acquires nothing.
- `FindGuardTarget` skips guard allies: the master and creatures the master controls, including controlled summons. Uncontrolled summons (energy vortex, blade spirits) attack their caster, so a guard still defends against them, as in RunUO.
- Friendly fire: `BaseAI.OnAggressiveAction` stands a guard down when an ally is the aggressor. `BaseCreature.Combatant` refuses an ally while guarding, because `Mobile.AggressiveAction` assigns the first aggressor as `Combatant` before any retaliation policy runs. The veto sits on `Combatant` rather than `CanBeHarmful`, so what a pet can harm doesn't depend on its current order. An explicit Attack order and bard provocation can still target an ally.
## Tests
16 regression cases in `GuardFriendlyFireTests` (skipped without client map data), including a real `WhiteWyrm` reproduction. `UOContent.Tests`: 1142 passed, 2 skipped, 0 failed on Windows.
> [!IMPORTANT]
> **Breaking change to API to fix NPC AI issues**
## Symptom
`FamiliarAITests.HiddenCaster_FamiliarRefusesRetaliation` fails on main at `Assert.False(wolf.Warmode)`: the familiar's `Combatant` stays null as intended, but it is left in war stance. In-game the familiar of a hidden caster flips into warmode for a tick when struck, until `FamiliarAI.StandDown()` clears it on the next think.
CI never saw it because the familiar tests are `SkippableFact` (they need client tiledata).
## Root cause
#2644 and #2645 landed a day apart and are individually correct but conflict at one seam.
Since #2645, a familiar (`Commandable == false`) is no longer "told to stand down", so `BaseCreature.AggressiveAction` continues past `AIObject.OnAggressiveAction(aggressor)` to its generic fallback:
```cs
else if (Combatant == null && !BardPacified)
{
Warmode = true;
Combatant = aggressor;
}
```
`FamiliarAI.OnAggressiveAction` had already declined (caster hidden), but the hook was `void`, so the creature could not tell. The fallback raised `Warmode` before `BaseFamiliar.Combatant` vetoed the target.
## Fix
This is the follow-up agreed in #2645: the retaliation policy moves into a virtual `BaseAI.OnAggressiveAction(aggressor) -> bool`.
- **`BaseAI.OnAggressiveAction`** now holds the stand-down verdict, the closer-aggressor swap (`PreferCloserAggressor`), and the `IssueOrder(Attack)` vs `Combatant =` decision. Returns `false` when the creature stands down.
- **`BaseCreature.AggressiveAction`** keeps only bookkeeping — `ControlMaster.Aggressors` entry, `StopFlee`, `ForceReacquire`, ethics `FinishShield` — keyed off the return, so an AI that declines is never second-guessed.
- **`FamiliarAI`** overrides the whole policy: no orders to stand down on, never fights for a hidden caster or a hidden aggressor.
Behaviour for every other AI is unchanged. Moving the fallback ahead of `StopFlee`/`ForceReacquire` is order-safe: both are pure field resets, `IssueAttack` does not `Think()`, and `OnCombatantChange` only derives `Warmode`.
Two commits: the first is the minimal `bool` hook that alone fixes the test, the second the policy move.
## Not changed (noted for later)
- `Warmode = true; Combatant = x;` (also in `FamiliarAI.Act`, `IssueAttack`) is redundant when the setter accepts — `BaseCreature.OnCombatantChange` derives `Warmode` — and is the dangling-warmode hazard whenever a `Combatant` setter vetoes.
- Since #2614 a stood-down pet skips the ethics `FinishShield`; RunUO ran it unconditionally. Preserved as-is.
## Tests
`HiddenCaster_FamiliarRefusesRetaliation` fails on main and passes here. Full `UOContent.Tests`: 1126 passed, 0 failed, 2 skipped (spawner fixture capture + benchmark).
When an AI debug-message cooldown crosses the signed tick-counter boundary, DebugInterpolatedStringHandler can emit before the deadline or suppress a message after it. Both constructors compare absolute tick values.
Use subtraction-based deadline comparisons in both constructors, following dev-docs/tick-counts.md. This is a two-line production change; message formatting, cooldown duration, and gameplay behavior are unchanged.
Adds 35 regression cases exercising the actual handler and DebugSayFormatted:
- Both constructor overloads, with debugging enabled and disabled.
- Before/at/after deadlines on positive and negative clocks.
- Future and expired deadlines across signed wraparound.
- Cooldown rearming across wraparound and buffer clearing.
- A compiler-generated interpolated call through the public extension method.
Validation on Linux / .NET 10, based on clean upstream 35e3a31b4:
- Release build: 0 warnings, 0 errors.
- New tests against unchanged production code: 28 passed, 7 failed.
- With the fix: 35 passed, including without client map data (no skips).
- Selected AI/pet suites: 134 passed / 7 failed before; 141 passed / 0 failed after.
Tests use synthetic entities; no running shard or world saves. The faulty comparisons were reverified on upstream main before submission. Windows execution remains unverified. The tests exercise explicitly assigned deadlines; deadline initialization is outside this patch.
Addresses only the DebugInterpolatedStringHandler comparison item in #2627; it does not close the other audit items.
## Summary
Profiling with thousands of creatures in range of each other showed `BaseAI.IsInvalidFactionTarget` checking Ethereal Voyage and active Honor on every acquisition candidate and then calling `BaseCreature.IsEnemy`, which checked both again. This makes `IsEnemy` the single authority and removes the duplicate per-pair work.
## Changes
- **`BaseCreature.IsEnemy`**: the Ethereal Voyage check sat below the `m is not BaseCreature` early return, so it never reached players — the only mobiles that cast it. It is hoisted next to the Honor veto. `GetMaster()` was called three times per creature pair (inside `Ethics.Player.Find(m, true)` and twice at the bottom); it is now computed once.
- **`BaseAI.IsInvalidFactionTarget`**: reduced to `IsFriend` / `IsEnemy` / `CanBeHarmful`. The removed `Combatant != m` Honor exception was dead code — `IsEnemy` vetoed Honor unconditionally on the following line.
- **`Ethics.Player.Find`** / **`TransformationSpellHelper.GetContext`**: small simplifications on the same path.
## Behavior
- `ShouldAcquireOnApproach` and `OnAggressiveAction` only consult `IsEnemy`, so movement-triggered acquisition now respects Ethereal Voyage for players (previously a player under Ethereal Voyage walking past a monster was still acquired on approach).
- `BaseFactionGuard.IsEnemy` does not call base, so faction guards no longer skip honoring/voyaging enemy-faction players. Accepted: both mechanics describe monsters, not guards.
- `HealerAI`/`BerserkAI`/`PredatorAI` (`bFacFriend`) callers are unaffected — `IsFriend` already required a `BaseCreature`, so players were excluded before these checks ran. `MilitiaFighter`/`MilitiaCanoneer` return `false` for all players, so they are unaffected too.
## Testing
- `dotnet build` clean.
- Pure predicate reorder; no new tests.
## Symptom
Since #2614, on any shard with `taming.petsStandDownOnCommand` (default `Core.ML`), every wild creature hits `"I'm being attacked but my master told me not to fight."` when struck. Brigands still chase (acquisition is a separate path in the think loop) but the retaliation path is dead: no `OnAggressiveAction`, no `StopFlee`, no `ForceReacquire`, `Combatant` never set, and `Warmode` forced off on every hit.
## Root cause
#2614 correctly added `OrderType.None` to `BaseAI.IsStandDownOrder` — a stopped pet rests on `None` and Publish 51 says it must not fight back. But the gate in `BaseCreature.AggressiveAction` never asked whether anybody could have given the order. The old predicate (`ct != Follow && ct != Stop && ct != Stay`) had only excluded wild creatures by accident: `None` was not in its set, so nothing ever needed to spell the check out. A wild creature's `_controlOrder` is always `None`.
## Fix
Both halves of `AggressiveAction` now gate on `Controlled && ControlMaster != null && Commandable` — the same predicate every order entry point already uses (`OnSpeech`, context menu, `IsValidTarget`). Only a creature somebody can command has been told to stand down. That excludes, and lets fight back:
| Creature | Why it was standing down |
|---|---|
| Wild creature | rests on `None` |
| Energy vortex, blade spirits, animated weapon, animate dead | `Summoned` with a `SummonMaster` but never `Controlled`; rests on `None` |
| Familiar, talisman summon, escortee, mirror image | `Controlled` with a master but `Commandable => false`; sits on a system-issued `Follow` |
The last row is a deliberate behaviour change from #2614 for ML+ shards: a familiar or escortee on `Follow` no longer stands down when attacked. The publish speaks of commanded pets; a creature that cannot take an order was never told anything, and pre-ML it always fought back.
Commandable summons (Summon Creature, elementals, daemons) are `Controlled` with `ControlMaster == SummonMaster`, so they stand down exactly as pets do.
## Also: stop after stay
Found while testing: `all come` then `all stop` left the pet on `Stay`, ticking *"I have been ordered to stay"*. `come` rests into `Stay` on arrival (deliberate ModernUO divergence, kept), and #2614 had `IssueStop` keep a previous `Stay`. RunUO and ServUO never consult the previous order — `DoOrderStop` is "wander around here" (or `None` pre-ML) — and Publish 51 says a stopped pet *"may wander"*. `Stay` now joins `Follow`/`Guard` in `IssueStop`: stop cancels the standing order and the pet idles anchored where it stands. `Stop_WhileStaying_RemainsStayingAtOriginalPost` is replaced by `Stop_WhileStaying_CancelsToIdleNone` plus the `come, stop` repro.
## Tests
`PetRetaliationTests` gains one case per row above: `WildCreature_`, `UncontrolledSummon_`, `UncommandableCreature_Retaliates_UnderStandDown`. Each fails on main and passes here; full `UOContent.Tests` green (1058 passed).
## Problem
`BaseFamiliar.OnThink` drove its own movement (`WalkMobileRange` toward the master) while the familiar was also a controlled pet running `Obey()`. `Summon → SetControlMaster` issues `Come`; `DoOrderCome` converts it to `Stay` within two tiles and anchors `Home`; from then on `OnThink` walked toward the caster while `DoOrderStay` greedy-stepped back toward the stale post — the backtracking. Combat never approached anything: main only copied `Combatant` while already adjacent to the caster. `CurrentSpeed = 0.01` was a 10 ms think / 50 ms step sprint hack, and `RangeCheck` teleported the familiar to a spot eight tiles *from* the caster.
## Change
A dedicated `FamiliarAI : BaseAI` (registered through `ForcedAI`, like `CloneAI`) owns every familiar decision, for both the controlled (`Obey`) and uncontrolled (`Think`) dispatch:
1. **Lifecycle** — caster gone → drop pack, delete. Caster on another map → stand down and wait for `TeleportPets`.
2. **Herding** (Dark Tides) — stands down, then `CheckHerding()`. Outranks combat.
3. **Assist** — combat-capable familiars (dark wolf, vampire bat, horde minion) engage the caster's target; otherwise anything in a fight with the caster's side — attacked the caster or the familiar, or attacked by the caster (a pet's attack is credited to the caster) — that is still fighting the caster, the familiar, or one of the caster's pets. Leashed to `RangePerception` of the caster; dropped when the caster hides. Shadow wisp and death adder never fight (`AssistsMaster => false`, enforced at the `Combatant` setter so no path can hand them a target).
4. **Follow** — `MoveTo(master, 1)` through the centralized `ApproachTarget` (greedy step / persistent `PathFollower` / stall detection).
5. **Keep-up** — snap to a validated tile beside the caster (on the caster's floor) when outpaced on open ground beyond 10 tiles, or when `ApproachTarget` gave up; never while a detour is working.
**Command immunity** is expressed inside the order machinery rather than around it: `FamiliarAI.IssueOrder` does nothing and rests the order on `Come`, so `TeleportPets` keeps working and no system-issued `Attack` (retaliation on ML's stand-down rule) can strand the familiar. `StandsDownOnCommand => false` so the ML rule never mutes it.
**Visibility** mirrors the caster from the familiar's own state (a step reveals a hidden NPC in `Mobile.OnMove`; the old cache compared the caster's previous state), `RevealingAction` is suppressed while the caster is hidden, and becoming hidden drops Warmode so no swing gives the caster away.
**Speed** is a flat 0.1 (`ReduceSpeedWithDamage => false`).
### Engine-side (all `Projects/UOContent`)
- `ApproachTarget` records which exit it took in `BaseAI.LastApproach` (`ApproachOutcome`: Arrived / Waiting / DirectProgress / Routing / Blocked / GaveUp / InvalidGoal). Callers' booleans are unchanged; keep-up reads this instead of running a second scheduler. `MoveTo`'s arrival return now also clears the move intent, as `ApproachTarget`'s own arrival does.
- `MoveToPoint(goal, range = 1)`; `CheckHerding` passes 0. **Fixes a main regression from #2591:** herding stopped one tile short, never cleared `TargetLocation`, and left the creature pinned to the herding pace — affects the shepherd's crook and the Dark Tides scroll fetch for every herded creature, not just familiars.
- `ChangeAIType` reads `ForcedAI` once. It read it twice, and each `BaseAI` ctor activates its timer for a non-sector-gated creature, so a `ForcedAI` creature with `PlayerRangeSensitive => false` got an orphan AI ticking it.
## Tests
`FamiliarAITests` are timer-wheel driven (the real `AITimer` thinks and moves; `PetPacingTests` style) against live Trammel statics, gated on client map data: follow without backtracking, the five-way assist theory, leash, retaliation, aggressor fallback (caster's own `Combatant` expired; caster's pet in the fight), target dropped when it stops fighting, keep-up on open ground / not while routing / after give-up, hidden mirror across steps, herding priority with a visible fighting caster, stand-down when left behind, no stale move intent. `ApproachOutcomeTests`, `HerdingTests` (fails on main), `ForcedAITests` (fails on main) cover the engine-side pieces.
Against `origin/main` with the familiar tests dropped in: 16/16 fail, including the reported backtracking. On this branch: `UOContent.Tests` 1082 passed / 2 skipped, `Server.Tests` 891/891, solution builds with 0 warnings.
## Summary
Moves spawner entry storage out of the abstract `BaseSpawner` into the concrete owner, so a spawner subclass can store its own entry type while every stock code path keeps working. Motivation: an out-of-tree spawner (ModernSpawner) needs `ModernSpawnerEntry : SpawnerEntry` with extra fields; today `BaseSpawner` owns `List<SpawnerEntry>` and a family of non-virtual members, and the serialization generator constructs list elements from the declared element type, so storage has to live in the class that declares the concrete list.
### What changed
- **`BaseSpawner` v13** no longer owns `_entries`. It reads entries through an abstract view and mutates them through an owner contract (`BaseSpawner.Entries.cs`):
`Entries` (`IReadOnlyList<SpawnerEntry>`, `[IgnoreDupe]`), `EntrySpan` (`ReadOnlySpan<SpawnerEntry>` for hot loops), `CreateEntry`, `AddEntryCore`, `RemoveEntryCore`, `ClearEntriesCore`, `AdoptEntries`, `CloneEntry`, `TransferSpawned`, plus public `RemoveAllEntries()`, `CopyEntriesTo(target)` and protected `RebuildSpawned()`. Every loop inside `BaseSpawner` is an indexed `for` over `EntrySpan`.
- **`Spawner` v2** owns `[SerializedIgnoreDupe] List<SpawnerEntry> _entryList` (generated `EntryList`, protected). `ProximitySpawner`/`RegionSpawner` inherit it unchanged. The `Spawned` rebuild and timer re-arm moved from the base `[AfterDeserialization]` (which runs before derived fields are read) into `Spawner`'s.
- **Save migration**: `MigrateFrom(V12Content)` (and the v10/v11/legacy readers) hand the old list to the owner via `AdoptEntries`; `Spawner.MigrateFrom(V1Content)` restores its own fields and leaves the adopted list alone. Three v12/v1/v0 save blobs captured before the change are committed as fixtures and loaded by tests.
- **Lifecycle hooks** (`BaseSpawner.Hooks.cs`, all no-op by default): `OnStarted`, `OnStopped`, `OnBeforeSpawn(entry)` veto, `OnConfigureSpawned(entry, spawned)` before positioning, entry-aware `GetSpawnPosition(entry, spawned, map)`, `OnSpawned(entry, spawned)`, `OnSpawnedDeath(entry, spawned, killer)`. `BaseCreature.OnDeath` calls `NotifySpawnedDeath` before base death (which deletes the mobile and unlinks the spawner). `Start()` and the `NextSpawn` setter share one start core so `OnStarted` fires on both.
- **`SpawnerEntry` v2**: per-entry `Disabled` (XmlSpawner's entry "lock"), stored inverted so the common case writes nothing in binary or JSON; skipped by weighted selection, live spawns untouched; toggle button per row in `SpawnerGump`. `SetParent` is public and `Parent` is protected so an out-of-tree entry subclass can adopt and dirty-track.
- **DTO**: `SpawnerDto` loses `Entries`; each concrete record declares its own `entries` at the same JSON order, so `Distribution/Data/Spawns/**` is byte-identical. Import adopts the deserialized entry objects instead of recreating them through `AddEntry`, which is what preserves subtype fields (and `disabled`).
### Breaking changes and behaviour changes
- **API:** `BaseSpawner.Entries` is `IReadOnlyList<SpawnerEntry>` instead of `List<SpawnerEntry>`. The generated `AddToEntries`/`RemoveFromEntries`/`InsertIntoEntries`/`RemoveFromEntriesAt`/`ClearEntries` helpers on `BaseSpawner` are gone; use `AddEntry`/`RemoveEntry`/`RemoveAllEntries`/`CopyEntriesTo`. `RemoveAllEntries()` deletes the entries' live spawns as well as the entries (the old generated `ClearEntries()` only cleared the list), which is why it has a new name rather than the old one.
- `SpawnerControllerGump` "copy entries" now goes through `CopyEntriesTo`, which deletes the target's live spawns (previously it cleared the list and left the spawns orphaned) and is a no-op when source == target (previously that wiped the source).
- `RemoveEntry` with an entry the spawner does not own is now a no-op (previously it deleted that entry's spawns).
- `Respawn()` honours `Disabled` because it calls `Spawn()`; `Spawn(int index)`, `RemoveSpawn`, and `RemoveSpawns` ignore it.
- Copying entries between spawners no longer forces a 1-second first spawn; the target re-arms on its normal delay.
- Subclasses that own a different entry list than `Spawner`'s must call `RebuildSpawned()` from their own `[AfterDeserialization]` (`Spawner`'s call runs before their list is read) and, when converting adopted entries into their own type, carry live spawns across with `TransferSpawned`. The in-repo test subclass demonstrates both.
### Performance
Manual harness (`Benchmark_SpawnPath_Manual`, skipped by default): 100k calls, entries all full so `Spawn()` does selection only.
| Path | Before (4bad0cc9e) | After |
|---|---|---|
| `Spawn()` 1 entry | 48.7 ns | 53.2 ns (within run-to-run noise) |
| `Spawn()` 10 entries | 243.7 ns | 153.1 ns |
| `Spawn()` 50 entries | 1077.6 ns | 645.1 ns |
| `Remove()` 10 entries | 139.6 ns | 83.6 ns |
Hooks are no-ops for stock spawners; `OnMovement` is untouched.
### Tests
- `SpawnerEntryOwnershipTests` (add/remove/clear, start after stop, dupe, copy, self-copy, foreign-entry removal, Disabled binary/JSON)
- `SpawnerHookTests` (hook order for mobiles and items, veto, death notification, `NextSpawn` start, a subclass with its own `List<TestEntry>` round-tripping and duping)
- `SpawnerSaveMigrationTests` (v12 fixtures through `Spawner`, `ProximitySpawner`, `RegionSpawner`; new-format byte-identical round trip with a live spawn reference)
- `SpawnerDtoEntryTests` (compact JSON position of `entries`, `disabled` only when set, import adopts the deserialized objects)
- Existing DTO/JSON/spawn-data tests unchanged. UOContent.Tests 801 passed, Server.Tests 869 passed. Migration schemas regenerated (`BaseSpawner.v13`, `Spawner.v2`, `SpawnerEntry.v2`).
Coverage caveats, stated plainly: v10, v11 and the pre-codegen legacy reader could not be captured as fixtures by the current code; each changed only `_entries = …` → `AdoptEntries(…)` into the same sink the v12 fixtures exercise, and is covered by review. The captured v12 fixtures carry no live spawns, so re-linking live `ISpawnable` references is proven by the new-format round trip, not by a legacy blob. The `SpawnerGump` toggle layout could not be checked in a client; the delete button moved from x=38 to x=46 to make room.
An uncontrolled creature with `Map == null` throws in `BaseAI.Deactivate()` when the condition reaches `Map.GetSector()`. A controlled creature with a null map avoids the exception but leaves its AI timer running.
Treat a null map like `Map.Internal` in the existing stop condition. This stops the timer without dereferencing the missing map and leaves the existing valid-map condition and return-home scheduling unchanged.
Addresses only the null-map `Deactivate()` item in #2627; the other audit items remain separate. No era-specific rules are changed.
A player can mount a horse or validate an ethereal mount inside a BaseRegion that overrides MountsAllowed to false because CheckMountAllowed never consults that property. This adds the missing check in the shared player permission path, using existing localized message 1042317.
## Summary
Pet orders lived in two files with nothing enforcing which phase owned what: `PetOrderHandlers.cs` ran one-shot handlers inside the `ControlOrder` setter and `PetOrders.cs` ran `DoOrderXxx` every AI tick from `Obey`. Friend/Unfriend refusals repeated their message every tick, Rename froze the pet, Drop on a dead pet never ended, the loyalty drain bypassed the release handler, and the command issuer leaked through a public field that only some handlers cleared (#2613 fixed the Release casualty of that split; this finishes the job).
Every order now lives in one place, `PetOrders.cs`, with two named phases:
- **Issue** — `BaseAI.IssueOrder(order, previous, issuer, resuming, interruptedTarget)` runs once, synchronously, from the new `BaseCreature.SetControlOrder` funnel. It may only set state and emit (message, sound, reveal) and returns the order to rest in. The funnel loops to a fixed point, so transient orders (Drop, Friend, Unfriend, Transfer, Release, Rename, Stop, Patrol) resolve before the setter returns and can never rest.
- **Tick** — `DoOrderXxx` runs from `Obey` for the six restable orders only (None, Come, Guard, Attack, Stay, Follow). Anything else that arrives there came from an old save and falls back to the standing order.
The issuer is a parameter: `BaseCreature.IssueOrder(order, issuer, target)` is the entry for player commands (speech, context menu, targeting), a raw `ControlOrder = x` assignment is a system-issued order, and nothing has to remember to clear anything. A resumed Follow restores the mobile the standing Follow was following, never a transient's target.
Because the funnel is synchronous it also carries the order being interrupted, so an administrative command can hand control back to what the pet was doing without storing anything per creature.
## Era behaviour, with sources
Two publishes govern most of the questions here, and the inherited code matched neither exactly.
[**Publish 16**](https://uo.com/wiki/ultima-online-wiki/technical/previous-publishes/2002-2/publish-16-part-2-4-23rd-july/) (23 July 2002) — *"The 'stop' command will stop a pet from guarding, following, and attacking."* Stop cancels the current attack and leaves the pet idle but still reactive. That is what this branch does whenever the stand-down policy below is off, in every era. (The same publish's *"Friends will only be able to issue movement commands to pets"* is the rule already enforced by `IsFriendOrder`.)
[**Publish 51**](https://uo.com/wiki/ultima-online-wiki/technical/previous-publishes/2008-2/publish-51-26th-march/) (26 March 2008) lists it per command:
> Follow: The pet should follow. It will not attack anything, even if it is attacked.
>
> Come: The pet should come. It will not attack anything, even if it is attacked.
>
> Stay: The pet will stay where it is currently, and will not attack anything, even if it is attacked.
>
> Stop: The pet will stop attacking. It will not attack anything, even if it is attacked, and may wander.
>
> Guard: The pet should guard as it does currently.
>
> Kill/Attack: The pet will attack its target as it does currently.
The inherited rule covered Follow and Stay only, so a pet told to come fought back, and it could not cover Stop at all: Stop resolves to None rather than resting, and None is exactly the state the publish describes. `BaseAI.IsStandDownOrder` now names the set — Follow, Come, Stay, None — and both halves of `AggressiveAction` read it.
## Configuration
`taming.petsStandDownOnCommand` (default `Core.ML`) controls the Publish 51 behaviour. The publish has no step of its own on the expansion ladder — it lands between ML and SA, and Kingdom Reborn was a client rather than an expansion — so it keeps riding ML as before, and the setting carries the rest: the behaviour is popular well outside its era, so a shard on AOS that wants it sets the key, and one that does not clears it. `BaseCreature.StandsDownOnCommand` is virtual for a creature that should differ.
## Bugs fixed along the way
- Friend/Unfriend refusal spam (every tick until the next command); Rename freezing the pet; Drop on a dead or non-`CanDrop` pet freezing the pet.
- Loyalty-zero release skipping the name clear and the summoned kill; a released pet keeping its `Friends` list and its previous owner's standing order.
- A transferred pet still answering to the previous owner's friends; transfer playing the idle sound twice.
- `BaseTalisman` summons issued `Friend` with no target (*looks confused* forever, or young-player spam); they follow their owner.
- Speech: single-pet commands lost their name gate after #2232 (a bare "come" moved every pet in range; "all stay" issued Stay twice); the speech cases passed a hardcoded `isOwner: true`, so a pet friend could say "`<name>` drop" and dump the pack, or issue Come/Guard.
- GM "`<name>` obey" was unreachable for a controlled pet; context-menu Release and speech Release disagreed about the control roll (resolved by removing it from both, below).
- Login derived the standing order from proximity even for a pet saved on Stay (zeroing its post), and only recorded the derived order without issuing it, so a pet saved mid-transient idled after a restart.
- `Friends` mutations never marked the creature dirty for delta saves.
- A resumed standing Follow was left without a target and cancelled itself to idle on the next think — the same defect as #2616, fixed here by `IssueFollow(resuming)` restoring the remembered target.
## Behaviour changes a shard maintainer will notice
- Every player command reveals its issuer, including context-menu commands from a hidden owner. This restores the blanket reveal (RunUO reveals in every order arm) minus its bug: it revealed the **control master**, so a friend's command popped the owner wherever they stood. Speech already reveals through `Mobile.OnSaid`, so the practical change is the context menu, target picks and the release gump.
- Resumed/chained orders are silent (no idle sound when falling back after Drop, Stop, a refused Friend, etc.).
- **Administrative commands no longer call the pet off.** Drop, Friend, Unfriend and Rename keep the pet's combat posture and hand control back to the order they interrupted, target and all; the standing order is the fallback only when the interrupted order cannot resume (a transient, or an attack whose target died, left or hid). Resuming an attack does not repeat its aggression or replay its bark.
- **Friend and Unfriend no longer rewrite the standing order.** Previously a success pointed the pet at the new friend and made Follow its standing order, so a pet left on Stay silently became a Follow with its anchor cleared. Friending grants a permission and nothing else; the friend has movement commands and can ask the pet to follow.
- **Releasing a pet no longer rolls the control chance**, on either path. A refused roll cost 3 loyalty, and loyalty reaching zero releases the pet anyway, so refusing only converted a deliberate release into an involuntary one minutes later. Both paths gate on `CanBeControlledBy` instead: if you can command it, you can dismiss it.
- **The pet distraction roll is gone.** A pet on Follow had a 10% chance per damage callback of dropping the order and attacking whoever hit it, issued without consulting anything, so it overrode the stand-down policy a few hits after the aggression path had correctly ignored it. Its era gate was guesswork by its own comment's admission and no publish describes it; `CanBeDistracted`, `CheckDistracted`, both call sites and the `Golem` override are deleted. Pre-ML shards lose the mechanic.
- **A pet's follow pace moved off the think clock.** The AOS sprint wrote a bespoke `CurrentSpeed = 0.1`, which fused both clocks — discarding any configured `ActiveMoveSpeed`/`PassiveMoveSpeed` — and pinned a following pet's AI at 10 Hz even while standing still. `BaseCreature.FollowMoveSpeed` (virtual, AOS 0.1) now caps the resolved step delay while the pet is closing on its master, the same way herding does: nothing stored, and a creature configured faster keeps its own pace.
- Transfer with an invalid target is a refusal (resumes) instead of forcing Stay; the transfer combat gate rests on the aggressor lists and `NextCombatTime`.
- Stop with no standing order anchors the idle where the pet stands (a vendor-bought pet no longer wanders off unbounded).
- The old "master must be alive" bails in the handlers are gone; stand-down and sounds run for orphaned pets too. RunUO gates only on the master being null or deleted, and a living friend commanding a dead owner's pet could not previously call it out of a fight.
- Login: a pet at None near its master is issued Follow silently; a saved Stay/Follow/Guard is adopted as is.
- GM "all obey" only reaches wild creatures; a controlled pet must be named.
- Death still issues Follow with the idle sound, as RunUO did.
## Tests
`PetOrderTests` grew from 16 to 63, plus 13 in a new `PetRetaliationTests` for the Publish 51 matrix and 13 in `PetPacingTests` for the clocks. Together they cover order resolution, reveal on every entry path, release parity (player vs drain, summoned), transfer/friend refusals and successes, stand-down and war-mode invariants, the interrupted-order resume, speech gating and permissions, GM obey, login derivation, the load probe, and the retaliation matrix across eras and both damage callbacks. Whole project green: 845 `UOContent.Tests`, 869 `Server.Tests`.
Tokuno scribes currently load only `SBScribe`, so the existing Zento scribe does not offer the Book of Bushido or Book of Ninjitsu. Add the existing `SBSamurai` and `SBNinja` inventories when `Core.SE && IsTokunoVendor`.
This reuses the existing book prices and default stock quantities. It does not change NPC placement, trainer behavior, or inventories outside Tokuno.
### Sources and reasoning
- The [official UO Ninjitsu guide](https://uo.com/wiki/ultima-online-wiki/skills/ninjitsu/) identifies the scribe at Rokuon Cultural Center in Zento as a seller of the Book of Ninjitsu.
- [UOGuide: Bushido](https://www.uoguide.com/Bushido), documenting standard UO, explicitly lists Tokuno Islands scribe NPCs as a source for the Book of Bushido. This supports adding both book inventories to Tokuno scribes.
- Documentation distinction: the [official UO Bushido guide](https://uo.com/wiki/ultima-online-wiki/skills/bushido/) describes the Zento seller as a samurai trainer, rather than a scribe. The Bushido scribe change follows the explicit UOGuide description. These references do not independently establish the exact historical introduction date or original prices.
### Validation
- Built the change in a customized ModernUO 0.15.6.145 deployment: Release, linux-x64, zero warnings or errors.
- In-game with Mondain's Legacy enabled: both books appeared on the existing Zento scribe, and a player successfully purchased both.
- Reviewed the expansion/location guard for pre-SE and non-Tokuno behavior; those cases were not gameplay-tested. This exact patch has not been built against current upstream main.
When a player selects Bulk Order Info while a cooldown is active on a pre-SE server, the vendor sends localized message 1049039 to itself instead of the requesting player. The player therefore receives no cooldown response.
Change the recipient from `vendor` to `from`, matching the SE branch. Cooldown durations and bulk-order eligibility remain unchanged.
Validation: This one-line fix compiled successfully in our customized 0.15.6.145 server build with zero warnings or errors. The current upstream main branch was not built locally.
## The bug
Pet is set to follow. It gets attacked and defends itself (the order flips to `Attack`), or the player tells it to `come`. The player then says `stop`. The pet quietly stops following and starts idle-wandering — the player has to re-issue the order to get it back.
`ResolveStop` clears `ControlTarget`, since the transient order that is ending owns it, and then resumes the standing order. Nothing restores a target, so the resumed `Follow` has none. `DoOrderFollow` checks for a target before anything else, finds none on the very next think, says "I have no one to follow" and cancels the order to `None`:
```
follow standing order=Follow target=set persist=Follow cur=0.2 => move=0.3
attacked (defends) order=Attack target=set persist=Follow cur=0.2 => move=0.3
stop issued order=Follow target=null persist=Follow cur=0.2 => move=0.3
next think order=None target=null persist=Follow cur=0.4 => move=0.9
```
Two things go wrong at once:
- `PersistentOrder` still reads `Follow` while `ControlOrder` is `None`, so the pet never recovers on its own.
- The pace falls from the active clock to the passive one. A shard that tunes `SetMoveSpeed(active, passive)` sees a following pet drop from its active move speed to its passive one for no visible reason — which is how this surfaced.
Era-independent. Guard is unaffected: `DoOrderGuard` works off `ControlMaster`, not `ControlTarget`.
## The fix
A standing `Follow` does not always mean "follow the master" — a pet friend can point it elsewhere with `all follow me` / `*follow me` (0x163, 0x16C) or `*follow` plus a target pick. So `SetPersistentOrder` remembers the target the standing order was given, and `ResumePersistentOrder` restores it, falling back to the master only when that target is gone. The field is runtime-only, like `PersistentOrder` itself.
Doing this in `ResumePersistentOrder` rather than `ResolveStop` also covers the Friend/Unfriend/Transfer resumes, where `ControlTarget` points at a third party and the pet would otherwise have resumed its follow on *them*. It matches what `HandleInvalidControlTarget` already does before its own resume.
## Tests
`Stop_WhileAttacking_FallsBackToPersistentFollow` already existed and passed, because it asserted the resumed order without checking for a target or driving a think tick. Three tests added alongside it:
- `Stop_WhileAttacking_ResumedFollowTargetsTheMaster`
- `Stop_WhileAttacking_ResumedFollowKeepsAPetFriendAsItsTarget` — a friend's follow order is not hijacked back to the owner
- `Stop_WhileAttacking_ResumedFollowSurvivesTheNextThink` — the one that catches the cancellation
Each was confirmed to fail without the change (`Expected: Follow, Actual: None`; wrong target). Full suites green: 782 UOContent, 869 Server.
Pet and summon bugs found by the master-reference audit for #2592 (comments there have the full inventory). Independent of delta saves.
## Releasing a pet never finished
The Release order ran two half-releases that never met:
- The order handler cleared the master. That set `Controlled` to false, so `Obey` never ran again and the think-side `DoOrderRelease` (re-home, three-day delete timer, backpack drop) was dead code: a released pet kept its pack and never despawned.
- The loyalty drain called `DoOrderRelease` directly, so a pet whose loyalty hit zero got the countdown and dropped its pack but kept its master and its owner's follower slots, the opposite of the code comment's intent.
`DoOrderRelease` is now the whole release (targets, bonding, `SetControlMaster(null)`, re-home, delete or countdown, pack drop) and runs once, synchronously, from the handler or the loyalty drain. Summons still die on release, as before. Two tests pin both entry points: master cleared, follower slots returned, countdown running, home anchored.
This is the one place the `PetOrders` / `PetOrderHandlers` split bit; the wider audit of that duplication is a separate task.
## Summon master follows the pet
Transfer, stable claim, GM "obey" and Ball of Summoning copied `SummonMaster` only when `Summoned`, so a talisman summon (`Summoned` is false, `SummonMaster` set) kept its original summoner after a transfer: two different masters on one creature, with the original summoner's area spells still exempting it. They now mirror the summon master whenever it is set. Jail stabling cleared only `ControlMaster`, so a jailed talisman summon kept charging the summoner's follower slots; it now clears both, like the stable master and auto-stable already do.
## Small ones
- The faction horse breeder set `Controlled`/`ControlMaster` directly; it now goes through `SetControlMaster` and tells the buyer why it refused (1049607) instead of silently deleting the horse.
- `Notoriety` dereferenced `SummonMaster` on a summoned creature without a null check.
## Tests
UOContent.Tests 778 green (two new).
The pure SerializationGenerator conversion of `BaseCreature`, split out of #2592 so it can serve as the reference for converting every other large hand-written class in the Delta Saves project (#7, phase 3). Two behaviour changes from #2592 are deliberately not here and follow in their own PRs on top of this one: `SpeedClass`, and the collapse of `ControlMaster`/`SummonMaster` into one reference (a creature can lose or keep either independently: Blade Spirits and Energy Vortexes are summoned but never controlled, EnragedCreature and talisman summons keep a summon master with neither flag set).
## What this is
- `BaseCreature` becomes `[SerializationGenerator(23, false)]` with a `[SerializableField]` per serialized slot and `[SaveFlag]` elision on nearly every field, so a stock creature serializes to its version plus flags. Field orders run 0..53 with no gaps (54 fields).
- The hand-written reader stays as `private void Deserialize(IGenericReader reader, int version)` for every pre-codegen version (0..22); post-codegen bumps use `MigrateFrom` from here on. `[AfterDeserialization]` carries the post-load fixups main did after reading (stat timers, AI type, followers, reacquire seeding).
- `ControlMaster` and `SummonMaster` stay two independent fields with main's semantics, each elided when null.
- `DamageMin`/`DamageMax`/`ActiveSpeed`/`PassiveSpeed` are no longer virtual (nothing in the tree overrode them); comments swept to the constraints that matter.
- Direct writes to serialized backing fields outside the generated setters (`SetDamage`, `SetResistance`, the move-speed helpers, the loot flag, feed loyalty, the delete timer) call `this.MarkDirty()`, matching the #2609 standard, so the class is ready for delta saves once `Mobile` is audited.
- Schema `Server.Mobiles.BaseCreature.v23.json` regenerated by the tool (a second run produces no diff).
## Deferred to follow-up PRs
SpeedClass: the serialized `_speedClass` field, `DefaultSpeedClass` replacing the type constant, `ApplySpeedClass`/`OnSpeedClassChange`, "None means custom", the four-speeds-as-one-block elision, `NPCSpeeds.FindEntry(SpeedLevel)`, the constructor fallback to Medium, and their tests.
Master references: serializing one `Master` with a `Controlled`/`Summoned` fan-out and the `SetControlMaster` lockstep.
## Tests
UOContent.Tests 776 / Server.Tests 855 green. `BaseCreatureSerializationTests` covers: a default creature elides to version + flags; a populated creature round-trips with exact byte consumption; back-to-back saves are byte-identical; an uncontrolled summon keeps its SummonMaster; byte-authentic v22 legacy streams (replicas of main's `Serialize`) load through the legacy reader for a wild tamable, a controlled pet, a controlled summon with an anchored `SummonEnd`, and a summon-master-only creature (the EnragedCreature shape); a running delete timer round-trips through `[DeserializeTimer]`; `Friends`, `CurrentWayPoint` and `HomeMap` round-trip; a `BaseVendor` stub round-trips the generated BaseVendor v2 → generated BaseCreature v23 chain.
## Behaviour notes for reviewers
- `ActiveMoveSpeed`/`PassiveMoveSpeed` getters return the raw override (0 = inherit); `CurrentMoveSpeed` is the resolved pace.
- Speeds elided as table defaults re-snap to the current `npc-speeds.json` on load, so table edits reach unmodified spawns on restart.
- `GetSpeeds` no longer throws on the save/load path when the table has no entry for the type: saves elide against the creature's own values and loads keep the stream. Construction still throws (`InvalidOperationException`, was `KeyNotFoundException`). An elided load with no table entry would otherwise resume at speed 0, so `[AfterDeserialization]` logs once and paces it at Medium.
- `virtual` removed from `ActiveSpeed`, `PassiveSpeed`, `DamageMin`, `DamageMax` (no overrides in the tree; forks may have some).
- `ControlMaster`, `SummonMaster`, `ControlOrder`, `Tamable`, `IsParagon` are `[SerializableProperty]` over hand-written setters because follower bookkeeping must run before the assignment, which a `fieldChanged` hook cannot express; the wire format is identical.
## Prerequisites for cherry-picking
#2609 (BaseVendor is already generated on top of BaseCreature) and SerializationGenerator 4.1.0.
## Why
Delta world saves re-serialize an entity only when it has been marked dirty. An audit of the generated classes found three ways serialized state changes without a mark; this PR closes the ones that do not need a new generator package.
## What
- **Custom `[SerializableProperty]` setters mark dirty.** Twelve hand-written setters assigned their backing field without `this.MarkDirty()`. `PlagueBeastLord.OpenedBy` was an auto-property carrying the attribute with no backing field at all; it is now a generated field with the same name, order and command-property exposure.
- **Generated sub-objects are linked to their owner.** Without a `[DirtyTrackingEntity]` member the generator emits setters that mark nothing. Eight entity-owned sub-objects now carry the link and receive the owner through the constructor the generator calls: `BOBFilter` (owner is `IEntity`: a `PlayerMobile` or a `BulkOrderBook`), `BOBLargeSubEntry`, `PuzzleChestSolution` and `PuzzleChestSolutionAndTime`, `TalismanAttribute` (the random factories now take the talisman), `VendorItem`, `PlayerBBMessage`, `RaffleEntry`, `ShardPollOption`. Migration schemas were regenerated with the pinned tool; the only change is the value rule argument becoming `DeserializationRequiresParent`.
- **BaseVendor uses the generator; restock amounts are no longer persisted.** The only state it wrote was which buy entries had grown restock amounts, packed by index into the live `SBInfos` tables. Restock is transient now and rebuilds on load; version 1 records are read and discarded through the legacy path. Every vendor subclass now serializes through a generated chain.
## Generator 4.1.0
This PR adopts SerializationGenerator 4.1.0 (modernuo/SerializationGenerator#55): SG3019/SG3020 diagnostics, `[VolatileSerializedState]`, `StopXxx()` timer helpers, and owner-constructor preference for sub-objects (so dictionary values are constructed with their owner; the 4.0.0 relink fallback is gone). `PlayerVendor.v3.json` gains `DeserializationRequiresParent` for its `VendorItem` values so the migration content struct constructs them with their vendor.
SG3019 is an error under `TreatWarningsAsErrors` and generator diagnostics ignore pragmas, so the five contexts that live in whole-file player-keyed persistences (`ChampionTitle`, `ChampionTitleContext`, `MurderContext`, `VirtueContext`, `JailRecord`) now carry a `[DirtyTrackingEntity]` link to their `PlayerMobile`, which is where they will live once those blobs move onto the player record. `JailSystem.EmptyRecord` keeps a null player (`[CanBeNull]`). `ChampionTitle` needs both a context and a player constructor because the generator resolves the rule against the containing type but emits the call with the parent field; a comment in the file records this.
## Wire format
Unchanged. No version bumps except `BaseVendor` 1 to 2 (which now writes nothing of its own). Schema diffs are rule-argument only (`DeserializationRequiresParent`).
## Testing
Server.Tests 848 passed, UOContent.Tests 759 passed.
## Summary
`Mobile.DamageEntries` was a `List<DamageEntry>` allocated for every mobile, including the ~99% that never take damage. It is now an inline `ValueLinkList<DamageEntry>` (24 bytes in the `Mobile` object, no separate allocation) ordered least recent → most recent.
- `DamageEntry` implements `IValueLinkListNode<DamageEntry>`.
- `RegisterDamage` moves the entry to the tail in O(1) instead of `Remove` + `Add` on a list.
- Expired entries are always a head prefix, so pruning walks from the head and stops at the first live entry. The `DamageEntries` getter prunes on access.
- `DamageEntries` is exposed as `ref readonly`; enumerate with `foreach` or `.ByDescending()`. Mutation goes through `RegisterDamage` / `ClearDamageEntries`.
- `BaseCreature.GetLootingRights` and `BaseCreature.ComputeBonusDamage` take `in ValueLinkList<DamageEntry>`; all callers compile unchanged. Files that `foreach` over `DamageEntries` need `using Server.Collections;` for the enumerator extension.
- RunUO migration docs (`dev-docs/runuo-migration-docs/09`, `11`) and the `migrate-items-mobiles` skill document the change.
Saves one object and 16 bytes per mobile (~8 MB and 500k gen2 objects on a 500k world). Second of three PRs from the lazy per-mobile collections design (first: #2604). Branched from `main`; the two diffs touch disjoint hunks of `Mobile.cs`.
## Breaking change
- `Mobile.DamageEntries` is no longer a `List<DamageEntry>`. Indexing, `.Clear()`, `.Add()`, `.Remove()` no longer compile; use `foreach`, `.ByDescending()`, `.Count`, `ClearDamageEntries()`, and `RegisterDamage`. Calling a `ValueLinkList` mutator on the `ref readonly` property compiles but operates on a copy while still unlinking the real nodes; do not.
- `BaseCreature.GetLootingRights` and `BaseCreature.ComputeBonusDamage` signatures changed to `(in ValueLinkList<DamageEntry>, …)`.
Save format is untouched: damage entries are not serialized.
## Behavior
Recency order, `allowSelf`, tie-breaking in `FindMostTotal`/`FindLeastTotal` (most recent wins), `Responsible` accounting, and loot-rights ordering are unchanged and covered by the new `DamageEntryTests` and `LootingRightsTests`.
## Testing
- `dotnet build -c Release` clean.
- New `DamageEntryTests` and `LootingRightsTests` plus full `Server.Tests` and `UOContent.Tests`.
Fixes walk-up aggro latency (up to a full 10 s of obliviousness) and hardens the reacquire gate so no state can silence acquisition, while turning `AcquireOnApproach` into the reaction-time knob for future per-creature intelligence tuning.
### Why
`AcquireFocusMob` re-armed the 10 s `ReacquireDelay` **before** scanning, success or failure. A creature that scanned an empty room was blind for 10 s to a player walking up — walk-up aggro latency was uniform in 0..10 s. Waking from sector sleep stacked the AI timer's 0–3 s construction stagger on top. And `NextReacquireTime` is not serialized: on hosts whose tick counter starts negative (GCP pass-through), the 0 default blocked **all** acquisition shard-wide after a restart until the counter crossed zero.
### What
**Event-driven reaction — `AcquireOnApproachDelay` (the intelligence gradient)**
- The paragon `AcquireOnApproach` bool becomes a `TimeSpan` on every creature: an enemy moving inside `AcquireOnApproachRange` (10 for all creatures — on-screen reactive aggro; the periodic scan keeps the wide `RangePerception` sweep) *clamps* the next scan to at most the delay. Repeated steps cannot shorten it further — one scan per delay period, not per step or think.
- `Zero` (paragons) also prods the AI timer: the ranked scan engages within a wheel turn — the old snap, minus the special-cased engage path. The target now comes from the normal FightMode ranking instead of whichever mobile happened to move, and the `Combatant == null` guard stops re-engage spam.
- The 2 s default reads as "took a beat to notice you"; larger values are dumber; `ReacquireDelay` alone is the oblivious floor. Mover checks are the approach logic's `IsEnemy` + `CanBeHarmful` (so pets count and hidden movers are excluded via `CanSee`), with `IsEnemy` first to cheaply reject same-team wild creatures wandering past. The check rides the `OnMovement` callback every step already pays for — no polling added.
**Gate correctness**
- Every scan re-arms the full `ReacquireDelay`, success or failure (classic semantics; reaction time is the approach path, not the poll).
- Self-healing by construction: a deadline further out than `ReacquireDelay` is an illegal state and reads as open — no wedged or wrapped value can silence acquisition beyond one delay period.
- `NextReacquireTime` is seeded from a live tick on deserialize (the GCP negative-tick blackout).
**AI timer wake**
- Activation (sector wake, spawn, resurrection) starts within a 0–256 ms spread instead of the 0–3 s construction stagger, which read as lag.
- The stagger's real job — keeping same-speed cohorts out of lock-step (the RunUO town artifact) — is now a zero-mean ±period/8 jitter on each **idle** think, so phases random-walk apart within seconds and can never re-lock. Instrumentation showed why a one-shot spread can't do this job: the timer wheel fires within ±1 ms, so with 10 creatures on a 500 ms period some pair collides on nearly the same phase ~75% of the time (birthday paradox) and then steps in the same loop iteration *forever*. Jitter is scoped to passive speed: engaged cadence stays exact, since pursuit timing anchors to real step times.
**Debug**
- The `AcquireFocusMob` scan message no longer re-arms the shared 5 s debug cooldown, which swallowed every AI's "I have detected X" transition line.
**API change** for custom scripts: `AcquireOnApproach` (bool) → `AcquireOnApproachDelay` (TimeSpan). Documented in `content-patterns.md` § Target Acquisition, `runuo-migration-docs/09` + `11`, and the migration skill checklist.
### Tests
`AcquisitionTests`: both scan outcomes honor `ReacquireDelay`; a 60 s-wedged gate still acquires; enemy movement clamps the deadline (same-team wild movers and out-of-range movers ignored); repeated movement cannot shorten below the delay; `Zero` opens the gate and prods without a direct engage. Full suite: 755 UOContent green.
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.
Closes#2593. Closes#2595.
Two related pet-AI fixes: the post-#2591 pacing/wake regression (#2593), and the guard order silently converting to Attack during combat (#2595). Root-cause analyses are in the issues.
## #2593 — pets follow slowly; stale AITimer wakes
**Why pets slowed:**
- The per-step budget grew from **half a think interval** (`CurrentSpeed * 500`) to the full RunUO-parity move table (`CurrentMoveSpeed * 1000`). Medium-bucket pets (Horse, Dog, most tamables): passiveMove **1.05s/step**.
- Pet order speed depended on stale `Warmode`: `HandleGuardOrder` set it once, but `OnCombatantChange` clears it whenever the combatant drops, so obedience ran active or passive **by combat history** — usually passive. Net: Guard/Come at ~1.05s/step (~2.1x slower than pre-#2591), vs a player running at 0.1–0.2s/step.
- The AITimer never rescheduled its pending wheel entry: the wheel reads `Interval` only after the next fire, so a speed-up or a fresh order (`Activate()` no-ops while running) waited out the stale wake — up to a full passive think, stacked on the residual move budget on Guard → Follow.
**What changed:**
- **Order handlers own obedience speed** (RunUO `OnCurrentOrderChanged`/`DoOrder*` parity, re-derived continuously): issuing a movement order (Come/Follow/Guard/Attack) sets the **active** think clock, resting orders (Stay/None/Transfer) set passive, and the guard/follow peaceful branches write **RunUO's AOS `CurrentSpeed = 0.1` sprint** — RunUO's guard else-branch had the identical write as follow. The bespoke 0.1 fuses to both clocks through #2591's existing classification, so `CurrentMoveSpeed` stays **pure herding + classification** with no obedience special case, and `DoMoveImpl`'s per-step flip skips obeying pets (their handler owns the pace) and loses its old follow-only 0.1 write. Combat still re-derives organically via warmode/combatant.
- **`AITimer`**: tracks the pending wake and reschedules (`Stop`, `Delay` = remaining, `Start`) when a speed-up or fresh order moves the earliest deadline up; changes inside a tick still flow through `ScheduleNext`. New `Prod()` wakes the AI immediately on player commands — including from a stopped timer, so stable claims no longer wait out the random construction stagger. Sector/spawn wakes keep the stagger. Spam-safe: a prodded think grants reaction, never action — steps/swings/casts/abilities are gated by their own budgets and timers.
The residual move budget is deliberately **not** cleared on order change — that would let order-spam macros grant free steps. Deadline changes reschedule the timer; rate changes take effect at the next deadline computation.
## #2595 — Guard order converts to Attack during combat
**Why:** `FindCombatant()` set `ControlOrder = OrderType.Attack` when engaging, so a guarding pet left the Guard order for the whole fight: OPL tags wiped (pet `1080078` + master `501129`), no retargeting (`DoOrderAttack` locks its target), `TeleportPets` left the pet behind on recall/gate, and every engage→kill→resume cycle replayed the guard flourish.
**What changed:**
- **`FindGuardTarget()`** (was `FindCombatant`): a pure selector — prefers the aggressor **closest to the master** (RunUO guard parity, dynamic retargeting to protect the owner), keeps the current combatant unless a strictly closer one exists, and never mutates order state. `DoOrderGuard` engages through it while **staying in Guard** the whole fight.
- **Persistent-order semantics** (the ModernUO improvement over RunUO): an explicit `all attack` completes → `ResumePersistentOrder()` returns to Guard → the guard scan engages remaining threats in-order. The Attack-chaining fallback (`FightMode.Closest/Aggressor`) now applies only to non-guard persistent orders. Resuming Guard no longer replays the sound/"is now guarding you" message.
- **Peaceful guard stands down deterministically** (`Warmode`/`Combatant`/`FocusMob` cleared) and returns to the master at the RunUO sprint (see above); at the master's side it stays organically active.
- **`WalkMobileRange` honors the caller's run flag** (the internal hardcoded `dist > 5` gate silently overrode it). Run is animation-only server-side; the only callers passing anything but `false` — follow, guard, clone — gate on their own thresholds.
## Resulting behavior (Medium-bucket pet)
| Scenario | Broken | This PR |
|---|---|---|
| Guard trailing master (AOS) | ~1.05s/step, think-grid quantized | 0.1s/step sprint (RunUO parity), smooth move wakes |
| Guard during combat | order flips to Attack; tags lost; no retarget; left behind on recall | stays Guard; retargets to master's closest aggressor; teleports with master |
| `all attack` while guarding | resume spams guard flourish per kill; chains into Attack | resumes Guard silently; guard scan takes over |
| Come / friend-follow | 1.05s/step | activeMove 0.45s/step (≈ pre-#2591 feel) |
| Guard → Follow reaction | up to ~1.5s dead time | think within one wheel turn |
| Follow master (AOS sprint) | 0.1s/step | 0.1s/step (unchanged) |
| Wild creature chase | RunUO-parity move table | unchanged |
Also documents two contracts this work leaned on: the `ControlOrder` setter deliberately fires on every assignment (a reissued order is a command — retarget/break-off/re-anchor), and `OnThink`/`MonsterAbility` must be excess-call tolerant (`dev-docs/content-patterns.md` § OnThink: the excess-call contract).
## Testing
- Full suite passes (1570: 837 Server + 733 UOContent).
- `PetPacingTests`: order-issue think-clock parity, follow-master sprint via Obey, guard organically active at the master's side, combat-chase and herding boundaries, plus two deterministic timer-wheel tests (8ms-lockstep slicing) proving a fresh order and a mid-wait speed-up wake the AI promptly.
- `GuardOrderTests`: engage keeps the Guard order; retargets to the aggressor closest to the master; explicit attack resumes Guard without chaining into Attack; peaceful guard stands down. Setup self-validates LOS/terrain.
- `GuardFollowTests`: guard-following registers a move intent, steps toward the master, sprints at 0.1 under AOS (per-step flip must not undo it), and runs active pre-AOS.
- All behavioral tests were written first and failed for the documented reasons.
Splits creature speed into two clocks so movement pace can be tuned without touching reaction time:
- **Think clock** — `ActiveSpeed`/`PassiveSpeed`/`CurrentSpeed`: seconds per AI decision. Unchanged in meaning, storage, and cadence.
- **Move clock** — `ActiveMoveSpeed`/`PassiveMoveSpeed` (+ resolved `CurrentMoveSpeed`): seconds per step. `0` = inherit the matching think value.
### How
- Move speeds come from optional `activeMove`/`passiveMove` in `npc-speeds.json`, are `[props`-tunable per instance (set `0` to re-inherit), and serialize (BaseCreature v22).
- `SetSpeed()` keeps its legacy one-clock semantics — sets the think clock **and clears move overrides** — so existing callers cannot half-configure a creature. `SetMoveSpeed()`/`ClearMoveSpeed()` configure movement explicitly; `ScaleMoveSpeed()` scales overrides for buffs.
- `CurrentMoveSpeed` is derived by classifying `CurrentSpeed`: a verbatim active/passive think value maps to the matching move value; a bespoke pace written directly (mount boosts, follow sprint) stays fused to both clocks. External `CurrentSpeed` writers need no changes.
- `AITimer` schedules the earlier of the two deadlines. Decisions run at the think cadence exactly as before; while a pursuit/investigation is live, the timer also wakes when the movement budget elapses and advances one step with no decisions. Steps no longer snap to the think grid, so any step delay paces smoothly on the 8ms wheel. A blocked creature schedules no move wakes.
- The movement budget is RunUO's `m_NextMove` accumulate-and-clamp at a full step, so long-run pacing averages `CurrentMoveSpeed` exactly.
### Behavior changes
- **`npc-speeds.json` buckets get RunUO `TransformMoveDelay`-parity move values**: creatures step at RunUO pace while thinking/reacting at current speed. The situational +0.1/+0.2 offsets are deliberately omitted.
- **Existing saves migrate on load**: a pre-v22 creature whose think speeds still match its npc-speeds entry (never hand-tuned) adopts the table's move values — worlds and pets pick up the new pacing without a respawn. Tuned creatures keep movement inheriting their think clock.
- **Paragons scale movement by `SpeedBuff` (1.2x)**: RunUO had no deliberate policy here — dividing by 1.2 knocked most speeds off `TransformMoveDelay`'s exact-equality table (raw pass-through, 2x+ faster), while 0.3/0.6 creatures landed back on it for ~1.33x. This applies the uniform 1.2x the buff always claimed. UnConvert snaps speeds back to exact table values within 1e-4 — /1.2 then ×1.2 drifts 0.45 and 0.9 by an ulp, which would read as hand-tuned (and defeat a future skip-table-conformant-values serialization pass); tuned speeds keep.
- **Herding paces the movement clock**: the old `CurrentSpeed` getter hack is gone. A herded creature walks at a fixed 0.3s/step — RunUO's forced pace, without its `TransformMoveDelay` inflation to 0.6 — so herding is never penalized by a slow creature. Thinking is untouched, and `CheckHerding` walks through `MoveToPoint`, so herded creatures path around obstacles.
- **Badly-hurt slowdown now inflates the step delay only** (RunUO parity), computed from the base each step. Previously it wrote `CurrentSpeed = CurrentSpeed + 0.05..0.15` back on every successful step — compounding unboundedly while hurt and slowing decisions too.
- Removes the vestigial `MoveSpeedMod` (never read, written, or serialized).
- With no bucket or per-instance move values, both clocks carry identical values and creatures pace as before.
### Testing
- Full suite passes (1557, including 12 new `MoveSpeedTests`: resolution classes, `SetSpeed` clearing, `0`-re-inherit, v22 round-trip with exact-consumption check, save migration adopt/skip, buff scale/snap, herding).
- In-game verified via local diagnostics build (per-step budget tracing): steady 700ms step cadence on a 0.3s think grid with one-step catch-up after idle, think grid unperturbed by move wakes.
### Summary
Fixes the long-standing reports of monsters losing track of players who run around a corner ("Is monster AI not using pathfinding? It seems to be LOS blocked by statics"). Root-cause investigation compared current behavior against RunUO line-by-line and traced the regressions through the AI overhaul era (#2232, #2246, #2379, #2401, #2461).
### Root causes and fixes
1. **Movement contract** — `MoveTo`/`ApproachTarget` returned false on every healthy mid-chase tick (true only on arrival), so MeleeAI's RunUO-inherited *"move failed and beyond RangePerception+1 → Guard"* clause — which RunUO only evaluated on genuine blockage — fired **every tick of every chase**. A mounted player trivially opens 17 tiles at a corner, the monster guards, Guard nulls the combatant, and re-acquisition is LOS-gated — unrecoverable through a wall. Movement now reports failure only on genuine failure (no step taken with no working path, or approach give-up). ArcherAI's equivalent clause moves to the hard leash.
2. **Last-known-position pursuit** — while a combatant is in LOS its position is recorded each think tick. When the target vanishes (corner, hiding, recall), the creature walks to the last-seen spot, stands guard there ~10s (restoring RunUO's guard grace, which had decayed to a single tick since #2246), and **re-engages instantly** if the same target re-enters view — bypassing the 10s reacquire throttle.
3. **`ChaseLeashRange`** — new virtual on BaseCreature (default `RangePerception * 2` = 32 tiles) replaces the inline `RangePerception * 3` (48) in Melee/Mage/Archer AI. Per-creature tunable via `[props`.
4. **Group movement demoted to a crowding refinement** — previously any uncontrolled creature with one ally within 8 tiles on the same target used greedy ring-stepping for the *entire* chase, with wall-slides counted as success, never invoking the pathfinder — the "aggroed but won't come around the corner" symptom for spawn groups. It now engages only near the target when allies actually contest the ring, and blocked/wall-slid steps escalate to the pathfinding approach primitive.
5. **Mages close distance on broken LOS** — a mage within casting range but LOS-blocked by geometry stood at the wall holding a spell target until the 60s combatant expiry (ProcessTarget short-circuits Think and its RunTo stands off at RangeFight). Geometry-blocked mages now close in until LOS returns, both pre-cast and while holding a target. Hidden targets (CanSee) and poison-cure priority unchanged. The new movement contract also stops the constant spurious `OnFailedMove` teleport rolls mid-chase.
6. **Move budget: one actual step per AI tick** — nothing advanced `NextMove` on a normal step (RunUO's `m_NextMove` budget was lost), so code paths attempting several moves in one think tick could cross multiple tiles at once — visible as "warping" when crowded creatures jockey for position. A successful step now consumes a half-step budget (floor 50ms): blocks intra-tick double moves, stays safely below the timer interval so legitimate next-tick moves are never jitter-throttled, and does not reintroduce `TransformMoveDelay` inflation. Blocked attempts consume nothing, so retry ladders (repath-and-step, the collision fan) are unaffected. `CanMoveNow` is also wraparound-safe now.
### Reference behavior
RunUO requires LOS to *acquire* a target and to *land* a hit or spell — never to *continue* a chase (its MeleeAI LOS bail-out is literally commented out in stock code). Chases drop only on: target hidden, target dead/off-map, beyond `RangePerception * 3`, 60s without combat interaction, or blocked movement while far away. This PR restores those semantics while adding the last-known-position investigation on top. NPC run flags are untouched — pace is AI-timer-driven and most NPC art has no run animation.
## Summary
Phase 3 of the anchored-time work: **every actively-written delta-time value in the engine now stores an anchored timestamp** — absolute on the wire, shifted forward by the downtime at load. Remaining time survives restarts (as delta did), and unlike delta, the bytes do not change on every save, so an idle world serializes identically save after save.
The answer to "is it possible everywhere": **yes** — including the one case that looked impossible.
## The GenericPersistence problem, solved
`GenericPersistence` bins (`Virtues.bin`, `StealableArtifacts.bin`, …) are raw payloads with no idx header, so they have no anchor of their own — anchored reads there would silently apply zero shift. But the anchor is a property of the **save**, not the file: every file in one save shares one `World.SaveStartTime`, and `Persistence.Load` reads **all** entity indexes (phase 1) before **any** persistence payload (phase 2). So the idx v5 header stamps a save-wide `World.LoadTimeShift`, and generic persistence readers inherit it. No file-format change, no per-bin header, old bins unaffected.
## Converted
- **Item v10 → v11**: `LastMoved` — previously whole-minute delta, rewritten every save for every item, the single largest source of idle-save churn — and `DecayResetTime` (retiring the TODO from #2583). **Mobile v37 → v38**: the three stat-gain stamps. **BaseCreature v20 → v21**: `SummonEnd`.
- **17 code-generated classes** (`[DeltaDateTime]` → `[AnchoredDateTime]`, version bump + `MigrateFrom` each): the five field spells, TransientItem, VirtueContext (×7 fields), PuzzleChestSolutionAndTime, BaseCamp, BaseBoat, RentedVendor, PlayerVendor, Ethics Player, Sheep, StarRoomGate, ChampionSpawn (×3), Corpse (`TimeOfDeath`, v19). The `MigrateFrom` bodies were generated from each class's current migration schema and are compiler-verified; VirtueContext's save-flagged nullables fall back to the same defaults the old deserialize left in place. Corpse's six migrations moved to a new `Corpse.Migrations.cs`.
- **Hand-written sites**: StealableArtifacts (v2), VendorInventory (v1), ML quest objectives (persistence v3) — each gated on its own version.
**Not converted, deliberately**: the ~25 read-only `ReadDeltaTime` sites in legacy version fallbacks and migration replays — they decode existing old bytes and must never change. `[DeltaDateTime]`/`WriteDeltaTime` remain available for them.
## Verification
- Build 0 errors / 0 warnings; **837 + 708 tests green**.
- Schema regeneration produced exactly the 17 expected new `vN.json` files (all `AnchoredTime` rule args), nothing else touched.
- **New acceptance tests** pin the point of the whole effort: serializing the same item at two save times **5 hours apart produces byte-identical output**, and `LastMoved`/`DecayResetTime` round-trip **exactly** at sub-minute precision (the old minutes encoding destroyed both properties).
## Notes for review
- `LastMoved` grows from a 1–3 byte encoded minutes value to 8-byte ticks per item — the price of byte-stability; it repays itself in incremental-save behavior since unchanged items now produce unchanged bytes.
- BaseEscortable-style semantics are unchanged: anchored shift preserves *remaining* time exactly, the same contract delta provided, so no gameplay-visible behavior changes — deadlines simply stop being consumed by downtime that delta already protected against, now with stable bytes.
## Enforcement
`WriteDeltaTime` is now `[Obsolete]` (interface + implementation). With the repo's warnings-as-errors, any new delta-time write — hand-written or emitted by a still-unconverted `[DeltaDateTime]` field — fails the build, with the migration instructions in the message. That the full solution still builds with **zero warnings** is itself the proof no active delta writer survived the conversion. `ReadDeltaTime` deliberately stays un-attributed: its remaining callers decode existing old bytes and are correct forever; its XML docs now state the legacy-decode-only contract.
## Summary
Folds **113** hand-written `[SerializableProperty]` members into plain `[SerializableField]` declarations using the v4 setter hooks — value coercion/vetoes via `allowFieldChange`, post-change side effects via `fieldChanged` (whose `oldValue` parameter covers the old-house/old-sender unsubscribe patterns). Net **-450 lines** of setter boilerplate.
```cs
// before
[SerializableProperty(1)]
[CommandProperty(AccessLevel.GameMaster)]
public int Charges
{
get => _charges;
set
{
_charges = Math.Clamp(value, 0, MaxCharges);
InvalidateProperties();
this.MarkDirty();
}
}
// after
[SerializableField(1, allowFieldChange: nameof(AllowChargesChange))]
[SerializedCommandProperty(AccessLevel.GameMaster)]
[InvalidateProperties]
private int _charges;
private bool AllowChargesChange(ref int value)
{
value = Math.Clamp(value, 0, MaxCharges);
return true;
}
```
## How sites were selected
A classifier parsed all 204 `[SerializableProperty]` sites and converted only those matching strict shapes: getter is exactly `get => _field;`, the assignment comes first (after at most an equality guard), and relocated side effects contain no `return`, no `value` mutation, and no field re-assignment. Everything else was left alone deliberately:
- **~34 custom getters** (fallback defaults like `_x == -1 ? Default : _x`, self-healing refs) — no setter hook can express these.
- **~35 pre-assignment logic** (durability Unscale/Scale sandwiches, old-state captures like PotionKeg's pile weight).
- **virtual/override members, name-mismatched backing fields (`m_`), exotic semantics** (guards' `Focus` does work on *equal* assignment; `ChampionSpawn.Active` never assigns its field).
Five sites the classifier refused were converted by hand where the hooks fit cleanly: `ReceiverCrystal.Sender`, `PlayerVendor.House`, `PlayerBarkeeper.House` (old-value unsubscribe via `oldValue`), `BaseSuit.AccessLevel` (its existing virtual `OnAccessLevelChanged` already had the exact callback shape), and `DyeTub.DyedHue` (a true veto: `AllowDyedHueChange(ref int value) => _redyable`).
## Verification
- Build: **0 errors, 0 warnings**.
- **Schema regeneration produces zero Migrations changes** — the conversion is wire- and schema-neutral by construction (same orders, types, and property names), and CI's schema diff check enforces it.
- **835 + 708 tests green.**
## Behavioral notes (all strict improvements, called out for review)
- Generated setters skip everything when the incoming value equals the current one; a few converted setters previously re-ran side effects on equal assignment (redundant `Update()`-style refreshes).
- Generated setters always `MarkDirty()` on change; several converted setters never did (e.g. `DyeTub.DyedHue`, `MorphItem` ranges) — their changes only persisted if something else dirtied the entity. Those latent persistence bugs are fixed by construction.
## Summary
Adopts ModernUO.Serialization 4.0.0 across the engine. Three commits, reviewable independently:
1. **Package + tool bump to 4.0.0** (`Server.csproj`, `UOContent.csproj`, `dotnet-tools.json`).
2. **Timers → `[DeserializeTimer]`** — the 8 drifting timers (BaseLight, TreasureMapChest, MarkContainer, FillableContainer, DeathRobe, DecayedCorpse, Corpse, BaseEscortable) now store their next tick as **anchored time**: server downtime no longer consumes the remaining delay, and idle-world saves are byte-stable. This changes their wire format, so each class bumps its serialization version with a `MigrateFrom` that replays the old delta-time read through the migration schema (the new `vN.json` files carry `@AnchoredTimer`; the old ones keep `@TimerDrift`, which the generator reads forever). The 2 wall-clock timers (Aquarium, FountainOfLife) keep their exact format via `wallClock: true` — no bump. Restart methods drop their `TimeSpan.MinValue` sentinel checks: v4 invokes them **only when a timer was actually running at save**.
3. **Linkage → field-side declarations** — 175 conversions across 25 files: `[SerializableFieldSaveFlag(order)]`/`[SerializableFieldDefault(order)]` become `[SaveFlag(nameof(...), nameof(...))]` on the field, and `[SerializableFieldChanged(order)]` becomes the `fieldChanged:` argument of `[SerializableField]`. **Wire-neutral: zero migration schemas changed.**
## Verification
- Solution builds with **0 errors, 0 warnings**; all three 4.0.0 packages verified indexed on nuget.org (no local feed needed).
- **835 + 708 tests green.**
- Generated output inspected: old-version content structs replay `ReadDeltaTime` (e.g. `V3Content.DecayTimerNext = reader.ReadDeltaTime()`), current versions write/read anchored time with the gated restart, and the wall-clock classes emit byte-identical `Write`/`ReadDateTime` framing.
- Schema tool run is committed (CI's `git diff --exit-code` schema check passes): exactly the 8 expected new `vN.json` files, nothing else touched.
- The conversion was scripted with a class-scoped resolver (order → same-class `[SerializableField(order)]`/`[SerializableProperty(order)]`); it planned 175/175 with zero ambiguities before applying.
## Notes
- New `MigrateFrom`s use the content structs' provided `XxxDelay` property, matching the pre-existing idiom in Corpse's and TreasureMapChest's older migrations.
- Follow-up candidate (separate PR, wire-neutral, any time): fold the ~150 eligible hand-written `[SerializableProperty]` setters (clamps, post-change side effects) down to `[SerializableField]` with `allowFieldChange`/`fieldChanged` hooks.
## What
`Dictionary<K,V>.Remove` and `HashSet<T>.Remove` do not bump the collection's version, so removing an entry during a `foreach` does not invalidate the enumerator. A number of loops were still paying for a `PooledRefQueue`/`PooledRefList` to collect keys and drain them in a second pass. This drops those guards.
## Why it's safe
Verified against .NET 10.0.10 rather than taken on trust, since the documented guarantee covers only `Dictionary<TKey,TValue>.Remove` while several of these call sites are `HashSet<T>` or enumerate `.Keys`/`.Values`:
| Case | Result |
|---|---|
| `Dictionary` foreach + `Remove` | safe, all entries visited |
| `Dictionary.Keys` / `.Values` foreach + `Remove` | safe, all entries visited |
| `HashSet` foreach + `Remove` | safe, all entries visited |
| `Dictionary` foreach + `Remove` **then `Add`** | throws `InvalidOperationException` |
Reflection on `_version` confirms the mechanism: neither `Dictionary.Remove` nor `HashSet.Remove` touches it. Because `Remove` never bumps the version, the `Keys` and `Values` enumerators are just as safe as the dictionary's own, even though only `Dictionary.Remove` documents the behaviour. No entries were skipped in any case.
The `HashSet` half is confirmed by [stephentoub on dotnet/dotnet-api-docs#8177](https://github.com/dotnet/dotnet-api-docs/issues/8177#issuecomment-1167251052): *"Both HashSet and Dictionary have been improved to support removal during enumeration. The docs may just benefit from updating."* The gap is in the documentation, not the runtime.
`Remove` followed by `Add` in the same enumeration still throws. That is the line this PR does not cross.
## Guards removed
`VisibilityList`, `ChampionTitleSystem`, `Channel`, `BombingRun`, `Ruleset`, `PuzzleChest`, `RaceChangeGump`, `StepCache`, `PlayerMurderSystem`, `VirtueSystem`, `ProjectedItem`, `StaminaSystem`, `AIGroupMovement`, `PromotedGuard`, `AutoDenylist`, `LoginAllowlist`, `AntiMacroSystem`, `DetectHidden`.
Both collection kinds are covered: `Dictionary` (including loops over `.Keys` and `.Values`) and `HashSet` (`ProjectedItem._active`, `PlayerMurderSystem._contextTerms`, `StaminaSystem._resetHash`). In `StaminaSystem.ResetTimer` the `Count == queue.Count → Clear()` branch goes away with the queue — it only existed to avoid paying for N individual removes.
Where the collection supports it, `Contains` + `Remove` and `TryGetValue` + `Remove` also collapse into a single lookup (`if (list.Remove(x))`, `if (m_Pending.Remove(ns, out var state))`).
`Utility.Tidy<K,V>` keeps its two branches: when `K` is serializable the value is not inspected, otherwise the value is. Only the serializable side may be cast, so `Dictionary<Mobile, int>` and `Dictionary<Mobile, string>` stay valid.
## Deliberately unchanged
**`BaseCreature.LoyaltyTimer.OnTick`** keeps its deferred-delete queue. Removing from `World.Mobiles` while enumerating it is safe, but `Mobile.Delete()` is not a `Remove` — it runs `OnDelete`/`OnAfterDelete`, the `OnParentDeleted` cascade over the creature's pack, `DropHolding()`, and region and guild callbacks. Anything in that surface that constructs a `Mobile` is an `Add` into the dictionary being enumerated, which does invalidate it. `BaseHire.PayTimer.OnTick` has the same shape and is likewise untouched.
**Spatial-query buffers** — `GuardedRegion.CallGuards`, `Thunderstorm`, `Exorcism`, `LeverPuzzleController`, `BaseCreature.TeleportPets` — are a different hazard. They buffer the result of a range query because the drain moves or harms mobiles, which mutates sectors mid-enumeration.
**Re-entrant drains.** The `_users` sets in `Firebomb` and the explosion, conflagration and confusion-blast potions look like this pattern but are not: the loop collects, `Clear()`s, and only then runs `Target.Cancel` on each, which can re-enter. `AnimalTrainer` enumerates `pm.Stabled` and drains through `RemoveStabled`, which nulls the `Stabled` field once it empties — safe for an in-flight enumerator, which holds the set reference rather than the field, but subtle enough not to be worth inlining on a cold path.
## Verification
`dotnet build` clean with 0 warnings; 810 Server and 684 UOContent tests pass.
### Summary
Moves inventory insurance out of `Mobile`/`PlayerMobile` into its own system at `Projects/UOContent/Engines/Insurance/`, wires it into the feature flag system, and makes disabling it actually disable it everywhere.
### Changes
**New `Server.Engines.Insurance.Insurance` system**
* Owns its own `Configure()`, seeding from the existing `insurance.enable` setting (default `Core.AOS`), so no config migration is needed. `Mobile.InsuranceEnabled` is gone, along with its line in `ExpansionConfiguration`.
* `CanInsure`, `GetInsuranceCost`, `ToggleItemInsurance`, `AutoRenewInventoryInsurance`, `CancelRenewInventoryInsurance` and `OpenItemInsuranceMenu` move here from `PlayerMobile`, which keeps four one-line shims for the context-menu callbacks.
* Every entry point is gated on `Insurance.Enabled`, and the death-time state is only allocated when insurance is on — a shard without insurance pays nothing for it.
**Feature flag integration**
Insurance is now a first-class feature flag: `ServerFeatureFlags.InsuranceEnabled`, registered under the `insurance` key in `FeatureFlagManager.SyncStaticFlag`, so it can be inspected and toggled through the normal flag command/gump rather than only at boot. `Insurance.Enabled` reads through to the flag, so there is one source of truth for every consumer.
**Fixes a memory leak from PvP**
`PlayerMobile.m_InsuranceAward` was a `Mobile` field assigned on every death and never cleared, so every player permanently pinned a strong reference to the last player who killed them. Killers were kept alive by their victims indefinitely.
Death-time insurance state now lives in a `Dictionary<Mobile, InsuranceContext>` owned by the insurance system: the entry is created in `OnBeforeDeath` and removed in `OnDeath`, so nothing outlives the death that created it.
**Removes insurance fields from every PlayerMobile**
`m_InsuranceAward`, `m_InsuranceBonus` and `m_NonAutoreinsuredItems` were carried by every `PlayerMobile` whether or not the shard ran insurance. All three are gone; the equivalent state is allocated per-death, only for players who actually die with insured items, only when insurance is enabled.
**Stale `Insured` flags are inert when insurance is off**
`Item.Insured` is a persisted flag, so items stay marked after a shard turns insurance off. Every read path now checks the flag first, so those items behave exactly as if they were never insured:
* `Item.CheckBlessed` / `Item.IsStandardLoot` — they drop again instead of acting blessed
* `Item.AddLootTypeProperty` — no more phantom "insured" tooltip
* `PlayerMobile.FindItems_Callback` — not yanked out of nested bags on death
* `DestroyEquipment` — no longer immune
* `ClothingBlessDeed` — no longer reports "that item is already blessed"
**Gumps promoted out of `PlayerMobile`**
`ItemInsuranceMenuGump`, `ItemInsuranceMenuConfirmGump` and `CancelRenewInventoryInsuranceGump` were private nested classes reaching into `PlayerMobile` privates. They are now public types in `Engines/Insurance/Gumps/`, talking to the insurance system through its public API. `ItemInsuranceMenuGump.ToggleSelected()` replaces the confirm gump's reach-in to the parent's `_items`/`_insure` arrays.
### Behavior changes
* The per-item "You lack the funds to purchase the insurance" message on failed auto-renewal is no longer sent during death; players get the single 1061115 summary instead. Marked with a TODO pending a decision on whether the per-item message should spam.
* The killer's insurance bonus is deposited once at the end of death processing rather than 300 gold at a time per insured item, and the "gold has been deposited" message is now conditional on the deposit succeeding. Same total.
### Drive-by cleanups
`PoisonImpl.IncreaseLevel` -> `Poison.IncreaseLevel`, a redundant `is NetState { } ns` pattern, `new List<Item>(Items)` -> collection expression, alignment of the `SyncStaticFlag` switch arms, and some comment/formatting fixes in `PlayerMobile`.
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.
## 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.
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
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
## 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.
Kamron Batman