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
`SendKarmaMessage` maps the karma change to a message, and its fallback arm is "You have lost a little karma." (1019063). A change of `0` lands in that arm, so the player is told they lost karma when nothing changed.
How to reproduce on a clean build: kill a creature with 0 karma, such as a grey wolf. The kill awards 0 karma and the player sees "You have lost a little karma."
This change returns early when the change is `0`, which matches how `AwardFame` already handles fame: it sends nothing when fame doesn't change.
## Test plan
- [x] `dotnet build` succeeds
- [x] Kill a 0-karma creature: no karma message
- [x] Kill a negative-karma creature: "gained karma" message is unchanged
Fixes#2700 (with #2704)
A restart (`restart` console command, `AutoRestart`, `CrashGuard`) launches a copy of the server and exits. That copy is detached from the console it was started from: the shell takes the terminal back and console commands stop working. Under systemd or in a container it's killed as soon as the original process exits.
- New setting `server.restartExitCode` (off by default). When it's set, a restart saves and shuts down as usual and then exits with that code, so a shell loop, systemd unit or container restart policy can start the server again on the same console. Values outside 1–255 log a warning and fall back to the current behaviour.
- Fixes `CrashGuard` crash recovery never restarting the server. Since #1809, `Kill()` only records the request, and the unhandled-exception path called `DoKill()` without it.
- Documents systemd, Docker and shell-loop setups in `dev-docs/server-requirements.md`.
## 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.
## Summary
Removing an item from a container now reaches only the clients that were sent the item. This PR also adds container-scoped skip serialization, makes container deletion linear, and closes a lift-rejection resync gap. It is the groundwork for the NPC vendor buyback fix (#2677), and it is safe to merge by itself.
### Container child removal reaches only the clients that were sent the item
Adds and updates for an item inside a private container have always gone only to the root mobile, the secure-trade partner, and the container's `Openers` (`ProcessDelta`, the same as RunUO). Removes, though, were broadcast to every client in range. That too is inherited from RunUO; #326 only converted the packets. Two costs:
- **Bandwidth.** One packet per nearby client for every reagent, bandage or arrow stack that runs out, and every container-to-container move.
- **Disclosure.** It sent the serial and timing of items moved in someone else's pack to every nearby client.
Now:
- **`Item.SendRemovePacket`:** a private container child's remove goes only to the root mobile, both secure-trade parties (found by walking up to the trade container, as `ProcessDelta` does), and the container's `Openers`. Each recipient is deduplicated and must be on the same map and in range.
- **`Container.IsChildPublic(Item child)`** (default `IsPublicContainer`) marks children that were sent to every client in range. Their removal still broadcasts. Overrides can only widen: a public container's children always broadcast.
- **`Corpse`** marks its worn gear public on human corpses. `SendInfoTo` shows that gear to everyone who sees the corpse, so bystanders still see looted gear disappear. Loot stays private, and corpses are not made public.
- **`IsPublicContainer` is unchanged** and remains the escape hatch.
- **Unchanged:** items equipped on mobiles, children of non-container items (spellbooks, bulletin boards), and ground items.
- **`Item.Map` no longer sends removes for contained items.** A contained item only changes map along with its parent or root, and that ancestor's own remove already clears the subtree for everyone who knew it. The owner keeps their nested bags across a facet change, as before, and bystanders no longer get a remove per nested item.
**For forks:** a custom container that sends its contents to clients other than the root, trade parties and openers must override `IsChildPublic` or `IsPublicContainer`. Otherwise those clients keep the item until they reopen the container, it leaves their range, or they log out.
- **`Item.MoveToWorld` no longer sends a second remove for an item that had a parent.** It used to detach through the parent's `RemoveItem` (correctly scoped), then broadcast another remove from the old location. Every lift goes through `Internalize()`, so every lift from a pack or container broadcast the item's serial. Ground items are unchanged.
- **Property-only updates** (`ItemDelta.Properties` without `Update`) of a private child now send the object property list revision only to the same recipients, not every client in range. Gump tooltips that reference such an item without the client holding it no longer get the revision nudge; their cached tooltip refreshes when re-requested.
### Lift rejection resyncs only items the requester was sent
A rejected lift re-sent the item's full data to the requester without checking that their client had ever been sent it. `Item.IsSentTo(Mobile)` now applies the same rule as the adds, including `CanSee`:
- **Private container children:** the root mobile, the trade parties, or the openers.
- **Everything else:** visible and in update range.
`Mobile.Lift` still always sends the lift-reject packet, but resyncs the item only when `IsSentTo` holds.
### Container content packets stay within the protocol limit
`SendContainerContent` stack-allocated `items.Count × 20` bytes, which overflowed the stack for a container with hundreds of thousands of items. The `0x3C` length and count fields are 16-bit anyway. It now caps entries so the packet fits (3448 entries, or 3276 with grid lines), and rents a pooled buffer above a small size. Output is byte-identical for normal containers.
### Container-scoped skip serialization
**`Container.SkipsChildSerialization`** (default `false`). When a container returns true:
- its direct children are left out of world saves (the base `Item.SkipSerialization` checks the parent);
- it writes no child list, so a skipped child's serial can never resolve to an unrelated item at load;
- an item that loads naming such a container as its parent is deleted by the existing missing-parent path.
**Known limitation:** `SkipSerialization` is read while the save files are written, after the world unfreezes. An item that enters an opted-in container in that window can be left out of that save. The next save is correct. Moving the decision into the freeze is a follow-up.
### Linear container deletion
`Item.Delete` removes children back to front, but `RemoveItem` searched from the front with `List.Remove`, so deleting a container of *n* items was O(n²). `RemoveItem` now checks the last entry first.
- 100,000 children took 8.0s before and 162ms after.
- Other removals cost the same as before.
**Save format unchanged for every existing type** (the schema generator produces no diff).
## Test plan
- [x] `ContainerChildRemovalTests`:
- a private child's removal isn't broadcast;
- openers, the root owner's client and a trade party (who never opened the pouch) still receive it;
- public containers and per-child `IsChildPublic` overrides still broadcast;
- an override can't narrow a public container;
- no duplicate packets when root and opener overlap;
- deleting the owner sends no remove per child;
- a facet change sends no remove for nested children to the owner or bystanders;
- lifting from a backpack or moving a contained item to the ground sends no remove to bystanders;
- equipped items and ground items keep their broadcast.
- [x] `PrivateChildPropertiesTests`: a property-only update of a private child reaches the owner and openers, not bystanders; public-container children and ground items still broadcast.
- [x] `CorpseChildPublicTests`: human corpse worn gear is public and loot isn't, bones and non-human bodies aren't; end to end, worn gear leaving the corpse reaches a bystander and deleted loot doesn't.
- [x] `LiftRejectResyncTests`:
- a bystander and an out-of-range requester get no resync;
- the owner, an opener and a trade party do;
- an invisible item isn't resynced to a non-staff owner, but is to staff;
- equipment on a hidden mobile and an invisible ground item aren't resynced.
- [x] `ContainerPacketTests`: large containers are capped at the protocol limit (with and without grid lines) and the pooled-buffer path is byte-identical.
- [x] `ContainerChildSkipTests`, `ContainerBulkRemovalTests`.
- [x] Server.Tests (944) and UOContent.Tests (1203) green on current `main`; schema generator clean.
- [x] Manual in-game pass with three clients:
- consumption;
- snooping;
- co-opened containers;
- bank;
- trade with and without opening the pouch;
- facet change with nested bags;
- pack animals;
- player vendors;
- human corpse looting by drag and by script;
- every lift-rejection reason.
Verified with a ClassicUO build instrumented to count:
- removes for unknown serials;
- adds dropped for an unknown parent;
- items learned inside containers the client never opened;
- lift resyncs of items the client never had.
A corpse's worn-gear list was fixed at creation, and clients learn worn gear only from the 0x89 packet sent on first sight. A piece lifted and dropped back showed as loose loot to everyone watching, but came back "worn" after a relog. Lifting now takes the piece off the list, so every view agrees it is loot.
Programmatic moves (the duel gather-and-re-equip in `DuelContext`) don't go through `OnItemLifted` and still see the original list.
`Stealing` moves the item with `AddToBackpack` and never asks the container, so a theft skipped the rules a container sets for a hand lift: a treasure chest's crime, refusal and guardian roll, a boat hold from outside the boat, a house's moving crate, and the Haochi/Emino quest chests.
An item in a container on the ground now goes through its `CheckLift` after the weight check, and a successful theft calls `OnItemLifted`, as a hand lift does. Deco containers are skipped: their `CheckLift` refuses everyone, and stealing is the only way into them.
These are the containers with their own `CheckLift` that a thief can reach:
| Container on the ground | What a hand lift already enforces | A theft until now |
|---|---|---|
| Treasure map chest | crime for a non-owner; refused if temporary, or for a non-owner on Trammel rules; guardian roll | none of it |
| Boat hold | only from aboard, while the boat is stopped | also from outside the boat, or while it moves |
| House moving crate | owner only | anyone |
| Haochi / Emino quest chests | one item, only while on the quest | anyone, any number, and it didn't count for the quest |
| Fish bowl not locked down | the fish stay in | the fish could be stolen |
## Summary
T2A tailoring Make Last failed after crafting a cloth item and then a leather item. `CraftContext.LastHue` was set only on the hued (cloth) path, so the next Make Last of a leather item took the hue-aware path and failed with "You don't have the resources". The leather path now resets `LastHue` to -1.
## Testing
- `dotnet build`: 0 warnings, 0 errors
- No automated test: driving `TailoringMenu` needs a NetState plus tiledata-backed resources, and CI has no tiledata. Not yet exercised in game.
## Summary
Digging up a decoded treasure map of level 0, or of level 2 and above, throws inside `DigTimer.OnTick` once the chest surfaces. The exception escapes the timer, so the server goes down:
```
System.NullReferenceException: Object reference not set to an instance of an object.
at Server.Items.TreasureMap.DigTimer.OnTick() in ...\Items\Maps\TreasureMap.cs:line 809
```
#2568 made `TreasureMapChest._guardians` lazily created: the constructor no longer initialises it, and migration nulls an empty list. #2569 updated the read in `CheckLocked` to match, but the dig timer still adds each spawned guardian directly:
```csharp
if (bc != null)
{
m_Chest.Guardians.Add(bc); // null on a freshly dug chest
}
```
Level 1 maps survive only because they spawn no guardians. I confirmed the regression by running the dig test below at #2568's parent, where it passes, and at #2568, where it fails.
The dig timer now calls the generated `AddToGuardians`, which creates the list on first use.
## Testing
- `dotnet build`: 0 warnings, 0 errors.
- New `TreasureMapChestGuardianTests`:
- `Dig_AddsSpawnedGuardiansToTheChest` (levels 0 and 2) drives the real `DigTimer` tick by tick until the chest surfaces. Before the fix both cases fail with the `NullReferenceException` above. It needs client map data, so it calls `TileDataRequirement.SkipIfMissing()` and skips on CI.
- `AddToGuardians_OnNewChest_KeepsLevelZeroChestShut` needs no client data, so it runs on CI: the list starts null, the first guardian creates it, and a level 0 chest stays shut to players until that guardian is dead.
- Full `UOContent.Tests`: 1196 passed and 2 skipped with client data; 1105 passed and 93 skipped without.
## Summary
Pre-AOS, a Meteor Swarm target that resists should take half of its share of the damage. Instead the resist branch halves the **shared** `damage` variable:
```csharp
var toDeal = damage;
if (!Core.AOS && CheckResisted(m))
{
damage *= 0.5; // should be toDeal
```
`toDeal` has already been copied, so:
- the target that resisted takes **full** damage, and
- every target processed after it is halved, once per earlier resist, so a group where several resist ends up taking a fraction of the intended damage for reasons unrelated to their own resist.
`ChainLightning` already handles this correctly (`toDeal *= 0.5;`), and this change makes Meteor Swarm match it. The AOS path is unaffected, since resist checks only run pre-AOS.
The bug predates ModernUO (it was carried over from RunUO in the first alpha).
## Testing
- `dotnet build Projects/UOContent/UOContent.csproj`: 0 warnings, 0 errors.
- No unit test: the per-target damage sits inside `Target()`'s queue loop alongside a random resist roll, so testing it would mean refactoring the spell for a one-word fix. Happy to factor out a helper and add one if preferred.
Crash reports never reached SMTP: `SendCrashEmail` returned immediately when email was enabled and never called the sender. It now respects the enabled setting, sends the report, and waits up to 30 seconds before the crash handler returns and the process can exit.
The sender uses the `Message-ID` generated by `MimeMessage` and the sender address already added by each caller. This removes the duplicate `Message-ID` header and duplicate address within the `From` header. Gmail rejects duplicate `Message-ID` headers: https://support.google.com/a/answer/13567860.
Email retries use a bounded doubling schedule instead of squaring an integer delay, which could overflow and throw through an `async void` method. The sender returns `Task`, uses `ConfigureAwait(false)` throughout, and logs failures. With the default five attempts and three-second initial delay, waits are 3, 6, 12, and 24 seconds. Each wait is capped at one hour, and there is no wait after the last attempt.
Validation:
- Full Release test suite on the branch updated with `main`: 2,103 passed, 2 intentionally skipped.
- Eight email test cases cover disabled-email early returns, retry timing, low attempt counts, and overflow-prone delays.
- SMTP delivery and the crash-time shutdown path are not covered by an integration test.
Loading the jail records added every player still serving a sentence to
CurrentlyBeingJailed, which only guards the jail sequence and is cleared
when it ends. Nothing removed them on release, so JailPlayer silently
returned for them until the next restart.
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)
`GumpSystem` clears a connection's gumps on `EventSink.Disconnected`, but `Mobile.NetState` raises it after the state is already null, so nothing is ever removed: every connection that touched a gump stays in `_gumps` until restart, with any gumps still open when it closed.
It now listens to `BeforeDisconnected`, while `m.NetState` still returns the connection. That event also fires on login, where there is no previous state and the handler does nothing.
* fix: duel teams start on their own arena points, in formation
MoveInside clamped the team index with Math.Min instead of Math.Max, so
every team started on the west edge point. The formation rotation also
reused the already-rotated X when computing Y, stacking players on the
north and south edges, and the SW/NE corners swapped axes, which leaves
the corner formation pointing out of the arena.
* refactor: one start point index clamp for GetBaseStartPoint and MoveInside
The clamp was written twice, and only one copy was correct.
* refactor: leave the placement loop as it was
Only the rotation lines in the loop belong to the fix.
Fixes#2682. Thanks @DatMoshu for the detailed report and repro.
While fixing it I audited the other map, item and multi enumerators for the same class of defect. None repeats #2682 itself, but four others turned up. All are fixed here.
## Fixes
- **`StaticTileEnumerator` (#2682)**: `SetMulti` returned `SetTile()` for the first multi whose bounds contained the point. If that multi had no tile at the cell, the enumeration ended and every later multi in the sector was skipped. Movement and line of sight lost a floor that stood inside another multi's bounding box. It now moves on to the next multi.
- **`MultiBoundsEnumerator`**: deduplication used one static `HashSet` that every new query cleared. A query run inside another made the outer query skip multis or return them twice. Each multi is now yielded only from the sector that holds its `Location`; the bounds filter guarantees that sector is visited. A per-enumerator set covers a multi that isn't registered in that sector (components changed after placement). Normal queries allocate nothing.
- **`Item`/`Mobile`/`ClientDistanceEnumerator`**: ring sectors were bounded only by `sx >= _sectorStartX`. Ring cells past the map edge resolved to the shared invalid sector, once per cell. Entities at off-map coordinates were returned more than once, and sectors outside the query were created lazily. The check now covers all four sides. Results match the bounds enumerators, which never returned off-map entities.
- **`BaseBoat.MovingEntitiesEnumerator`**: `GetMovingEntities` handles a null map, but the enumerator constructor still called `boat.Map.GetMobilesInBounds` and threw. A boat with no map now gets empty enumerators.
- **`BaseDoor.ChainEnumerator`**: the walk stopped only on returning to the start door. Links are set in pairs, so `[ChainLink A B C` followed by `[Link B C` leaves A→B→C→B, and enumerating that chain never ended. The walk now stops on any door it has already yielded. Only reachable when `UseChainedFunctionality` is true, which is off in base and in all shipped content.
## Testing
- A regression test for each fix except the boat null guard. Each failed before its fix and passes after.
- Server.Tests 910/910. UOContent.Tests 1185 passed, 2 skipped.
- Test note: the `MultiComponentList(List<MultiTileEntry>)` constructor drops entries after the first whose `Flags` are 0. The new test multis use non-zero flags.
`TryStealItem` doesn't check for a locked container, so an item that can still be targeted inside one (the gump stays open when it gets locked, or the client targets by serial) is stolen without picking the lock.
It now refuses with 501747 ("It appears to be locked."); GMs are exempt, and stealing the locked container itself is unchanged. It checks the lock directly rather than calling `CheckLift`, which would also refuse items in deco containers.
## 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
CorpseNotoriety returned Murderer for a player corpse whenever the corpse body was a monster
body. RunUO's check was `body.IsMonster && IsSummoned(owner)`, which is never true for a player;
the second half was lost when the branch was simplified (#134). Players who died while
polymorphed or in a necromancer form left a corpse with the wrong notoriety.
Player corpses also check murderer and criminal status before guild standing again, matching
RunUO and MobileNotoriety.
Tests: UOContent.Tests 1162 passed (4 new CorpseNotorietyTests).
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.
## Summary
- `AnchoredItemSerializationTests.LastMovedAndDecayReset_RoundTripExactly` never deleted its deserialized copy. `Item.Deserialize` enters the item into its map's sectors, so the copy stayed live on Felucca at (121,100). `ItemEnumerator_FiltersByBoundsAndOrder`, `ItemByDistanceEnumerator_Bounds_FindsItemsInBounds` and `ItemByDistanceEnumerator_Bounds_MakeBoundsInclusive` scan that area and found an extra item whenever the serialization test ran before them, so they failed depending on test order. The copy is now deleted in a `finally`.
- Bump MailKit 4.17.0 → 4.18.0 and Microsoft.NET.Test.Sdk 18.10.0 → 18.10.1.
## Test plan
- [x] Probe test: a deserialized copy shows up in `GetItemsInBounds` (reproduced), and after the fix the area is empty
- [x] Server.Tests: 907/907 pass
- [x] UOContent.Tests: 1126 pass, 2 skipped
- [x] Solution builds with 0 errors
> [!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).
## Summary
The admin gump's **Performance** page still rendered `ThreadPool` worker / IOCP quotas — a RunUO leftover from `Socket.BeginReceive` days. Both figures are dead: IORingGroup drives RIO / io_uring / epoll / kqueue synchronously on the game loop, so the IOCP figure reads 0% forever, and the real off-loop work (serialization workers, password checks, advanced search, console input) runs on dedicated threads the worker figure never sees.
This replaces the block with a read-only network readout.
## Engine
- `NetState.GetNetworkStats()` — new public accessor returning a `NetworkStats` snapshot (ModernUO-owned record types in `Network/NetworkStats.cs`, so UOContent takes no dependency on transport types):
- live: connected / max sockets, authenticated count, per-tier growth usage, the three loop queues (throttled, flush pending, pending disconnects)
- configured: base / pre-auth / max buffer sizes, growth budget, memory ceiling, last-read available memory
- `LastSweep`: everything the once-a-minute maintenance sweep saw, including the refusal counters it resets
- The sweep already kept four of its results (`_lastTier*`, `_lastBaseCapacityBytes`) for change detection; those fold into one `SendBufferSweep` snapshot. Sweep logic and logging are unchanged.
- The accessor never calls `Maintain()`, so opening the page cannot rotate the retention window.
- Tick age is compared by subtraction; "has a sweep run" is a `bool`, not a zero-tick sentinel.
## Gump
```
Event Loop: Healthy
Connections: 123 / 4096 (117 authenticated)
Loop Queues: throttled 0, flush 3, closing 0
Send Buffers: [scrollable]
Base: 64.0 KB recv, 256.0 KB send; pre-auth 4.0 KB / 4.0 KB
Base pools: 10.2 MB
Growth tiers (live):
512.0 KB: 3 / 32 in use, floor 5
1.0 MB: 0 / 0 in use, floor 0
2.0 MB: 0 / 0 in use, floor 0
Budget: 16.0 MB of 256.0 MB, max 2.0 MB per socket
Ceiling: 80% of 7.8 GB available
Last sweep 42s ago: released 0 tier, 0 base
Refused: budget 0, at max 0, ceiling 0
```
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.
Two bugs found while reworking #2653.
**Custom flags throw.** `SyncStaticFlag` is a switch expression with no discard arm, so any key that has no static behind it throws `SwitchExpressionException`. `[FeatureFlag mykey create <category> <desc>` crashes in `CreateOrUpdateFlag`, and once such a key is in `flags.json`, every boot's `SyncAllStaticFlags()` throws mid-iteration — caught and logged as "Failed to load feature flags", but stock flags later in dictionary order never sync. Added `_ => enabled`.
**Removing a flag turns some features on.** `RemoveFlag` hardcoded `SyncStaticFlag(flagKey, true)`. For `speedhack_detection` and `insurance` the real default is `false` / `insurance.enable`, so deleting the flag enabled the feature. It now syncs the removed flag's `DefaultEnabled`, which #2653 seeds from the static.
Verified: `dotnet build Projects/UOContent/UOContent.csproj -c Release`, 0 `CS` diagnostics (post-build copy to `Distribution/` was blocked by a running server instance).
## 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.
### Summary
`default-flags.json` was never shipped — `Configuration/` is gitignored — so the predefined-flag loader has been dead since #2328. A fresh shard boots with 0 flags and writes no JSON until an admin changes something, so `[FeatureList` is empty on first run.
Stock flags are now defined in code. `Initialize()` runs `Load()` first, then `LoadDefaultFlags()` seeds any of the 14 stock keys the save is missing, reading each default from the static it syncs (`ServerFeatureFlags` / `ContentFeatureFlags`) rather than a duplicated boolean — so `speedhack_detection` stays off and `insurance` honors `Insurance.Configure` (`insurance.enable`). Existing entries are never overwritten, so admin state survives upgrades and saves predating a flag pick it up. `Save()` runs only when something was seeded, so all five JSON files exist from first boot.
Verified: `dotnet build Projects/UOContent/UOContent.csproj -c Release`, 0 warnings 0 errors.
## 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.
## Symptom
`[set TargetLocation (x, y)` (quoted or not) answers **"That is not properly formatted."**, and in `[props` the `>` next to `TargetLocation` does nothing when the value is null — which is its normal idle state (`BaseAI.cs:650` clears it).
This looked like a `Point3D` parsing regression from #2624/#2625, but `Point3D`/`Point2D`-typed properties (`Location`, etc.) were never affected. The only `[CommandProperty]` in the tree declared as an **interface** is `BaseCreature.TargetLocation : IPoint2D` (`BaseCreature.cs:1111`), and both code paths only knew the structs. `git log -S"IPoint"` over the parser and gump files hits nothing but the initial import — the gap is inherited from RunUO, not recent.
## Root cause
- **`[set`** — `Types.TryParse` has no branch for `IPoint2D`/`IPoint3D`. An interface has no static `Parse`, so `GetParseMethod` returns null and the value falls into `Convert.ChangeType("(x, y)", typeof(IPoint2D))`, which throws → "not properly formatted".
- **Props gump** — `PropsGump` routes on `obj?.GetType() ?? prop.PropertyType` (since #2180). With a null value the type is `IPoint2D`; `Point2D.IsAssignableFrom(IPoint2D)` is false, no branch matches, and the click is inert. It only worked when the slot already held a `Point2D`, because the runtime type is then the struct.
## Fix
- `Types.TryParse`: `IPoint3D`/`IPoint2D` targets resolve to the concrete struct — `Point3D` first, then `Point2D` for an `IPoint2D` target (a 3-tuple is a valid `IPoint2D`). `(-null-)` still clears; the existing null branch runs first.
- `PropsGump`: the interface types route to `SetPoint3DGump`/`SetPoint2DGump`. The entity branch stays ahead of them — `TargetLocation` legitimately holds a Mobile too (`ShepherdsCrook.cs:148`, herding toward the shepherd), and that case still opens `SetObjectGump`.
- `SetPoint2DGump`/`SetPoint3DGump`: seed the text entries from `value is IPoint2D/IPoint3D` rather than a hard cast, so a `Point3D` sitting in an `IPoint2D` slot cannot `InvalidCast`.
## Not covered
`[set TargetLocation 0x40001234` (assigning a mobile by serial) still reports "not properly formatted" — the entity branch keys on the *target* type being `IEntity`, which `IPoint2D` isn't. Real state, but niche; left out to keep this to the reported symptom.
## Testing
Five cases in `InterfacePointParseTests`, watched fail before the change (three returned the error string; two pin existing behaviour that must survive): tuple → `Point3D` for both interfaces, pair → `Point2D`, pair rejected for `IPoint3D`, `(-null-)` clears.
`dotnet build` 0 warnings. **1059 UOContent** and **891 Server** tests pass, 0 failures. The gump routing is a one-line branch with no automated test — needs an in-game check: `[props` a creature with a null `TargetLocation`, press `>`, expect the Point2D editor.
## Summary
Two small additive changes a derived spawner needs.
- **`BaseSpawner.OnTick()` is now `virtual`.** A subclass that gates spawning on external state (time windows, event triggers) must gate *timer* spawns without gating the manual `Spawn()` API, and `OnTick` is the only place the two paths differ: it is the timer callback and `Spawn()` is both what it calls and what commands and scripts call. Cost: one virtual dispatch on the existing timer callback; no change to stock behaviour.
- **`group` in the JSON DTO.** `BaseSpawner.Group` (all dead, then respawn) is binary-persisted but was missing from `SpawnerDto`, so it did not survive export/import. Added to the abstract record after `spawnLocationIsHome`, assigned in `ApplyDto` after `InitSpawn` (which resets it), and exported by the three stock `ToDto` implementations.
## Test plan
- [x] A derived spawner overriding `OnTick` with a closed gate: `OnTick()` spawns nothing; manual `Spawn()` still spawns and does not pass through `OnTick`.
- [x] DTO round trip with `Group = true` carries `group` and restores it on `ToSpawner()`.
- [x] `UOContent.Tests` full suite green.
- [ ] CI
## Summary
Two small additive changes that an external content assembly (ModernSpawner) needs, as separable commits.
**1. `SkillEvents.SkillUsed`** (`Projects/UOContent/Skills/SkillEvents.cs`, namespace `Server.Misc`): a plain C# event `Action<Mobile, Skill, bool success>` raised once per skill attempt from each of the four `Mobile_SkillCheck*` handlers, with the handler's own result. Attempts the handler resolves without a roll (too difficult, no challenge) raise too, so a grandmaster's trivial success and a guaranteed combat roll are observable. Not raised when the mobile lacks the skill. Each handler keeps its logic in a private core method and raises on the way out, so there is exactly one raise per attempt and `CheckSkill` itself is unchanged.
- **Why a plain event and not a `[GeneratedEvent]`:** generated events are compile-time static dispatch inside the UOContent compilation, so a subscriber in another assembly cannot use `[OnEvent]`. Shape follows `HelpEvents`.
- **Why "used", not "gained":** this is the XmlSpawner skill-trigger semantic (it wrapped the same four handlers and passed their result as `success`; its grammar was `Skill[+/-]` for success-only or failure-only). Gains are already observable through the existing skill-change notification on `Mobile`.
- **Cost:** one delegate null-check per attempt when nothing is subscribed; no boxing, no closure, no allocation. The handlers sit on the combat swing path.
- **Exception contract:** subscriber exceptions propagate, matching `EventSink`/`HelpEvents`; no try/catch by design.
**2. `InternalsVisibleTo("ModernSpawner.Tests")`** on `Server.csproj`, beside the existing `Server.Tests`/`UOContent.Tests` entries, so an external test host can seed `Core._now` the way the engine's own test initializers do. Separable; a public test seam on `Core` would serve the same need without naming a downstream assembly.
## Open question
The payload is the `Skill` object plus a positional `bool`. A `readonly struct` args type passed `in` would leave room to add `chance` or the target later without breaking subscribers. Happy to change before merge.
## Test plan
- [x] `UOContent.Tests`: 4 tests — a rolled attempt raises once with the returned outcome; each short-circuit path (no challenge, too difficult, on both the direct and value-window handlers) raises with the handler's result; a direct `CheckSkill` call does not raise; no subscriber does not throw. Full suite green.
- [x] `Server` and `UOContent` build clean with `TreatWarningsAsErrors`.
- [ ] CI
## Summary
Follow-up to #2621, which added a per-entry `Disabled` flag to the spawner gump as a checkbox. The checkbox was placed between the Expand and Delete buttons at x=22, overlapping Expand (5–35), and Delete moved to 46, overlapping the creature text box at 71.
- **Lock column on the far left.** Each entry row gets a toggle at x 5–25: padlock `0x82C` when the entry is disabled (press to unlock), green orb `0x2C88/0x2C89` when enabled (press to lock). The client's gump art has only closed padlocks (`0x82C`, `0x0020`), so the orb marks the unlocked state; XmlSpawner uses the same padlock paired with a blue gem.
- **Expand and Delete are adjacent again** (28 / 61, the original 33px pitch), and everything to the right shifts 23px: creature/#/Max/Prb boxes, headers, the totals row, page arrows, Save/Cancel. The gump is now 369 wide (was 346). Params/Props boxes widen to match.
- **Locked entries are read-only.** Name, max, probability, and Params/Props (when expanded) render as `AddLabelCropped` on a grey tile (`0x23F4`) instead of `AddTextEntry`. There is no read-only text entry in the UO gump protocol, so omitting the entry is the only way to make the field truly non-editable. `CreateArray` already `continue`s on a null type entry, so Save leaves locked entries untouched.
- **Totals row aligned under its columns.** Spawned under `#`, a new total max under `Max`, total weight under `Prb`. Max and weight exclude locked entries, matching `BaseSpawner`, which skips `Disabled` entries when rolling.
## Test plan
- [x] `[props` a spawner with several entries: lock column is on the left, Expand/Delete sit side by side, no overlaps.
- [x] Lock an entry: padlock shows, fields go grey and cannot be typed into; unlock: orb shows, fields are editable again.
- [x] Edit an unlocked entry, lock another, Save: unlocked edits persist, locked entry keeps its values.
- [x] Expand a locked entry: Params/Props are grey and read-only.
- [x] Totals sit under `#` / `Max` / `Prb`; locking an entry drops it from the max and weight totals.
## 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.
