Follow-up to #2721. It fixes how the remaining tests behave and changes no production code.
## CI
- Both `dotnet test` steps pass `--filter "Category!=Manual"`. Before this, Manual-tagged tests ran on CI anyway.
- The one surviving movement-throttle latency test is the only coverage of burst forgiveness over a full, wrapped history. It loses its Manual tag, so it runs on CI.
## Tests that passed on CI without testing anything
- **Corpse and throwing tests.** They relied on tiledata for the weapon `Layer`, so on CI the equip silently failed and they asserted against empty lists. They now set `Layer` explicitly and assert the equip took effect. `TestThrown` also pins its own strength and range values, so a shard retuning SoulGlaive no longer breaks them.
- **Healthbar poison rows.** Server.Tests registers no poisons, so every row tested `null`. They now use real poison levels.
- **Account packet tests.** The mock accounts reported `Length` 0, so the character-list tests only ever encoded empty slots. They now encode the real characters.
- **Pathing and AI tests that only mean something on real Trammel/Felucca terrain** are now guarded with `TileDataRequirement.SkipIfMissing()`: Britain desk and clutter scenes, house footprints (no MultiData on CI) and fingerprints. Tests that are valid on flat blank terrain stay unguarded. (A synthetic tiledata fixture to run these on CI is a planned follow-up.)
- **Assertions that couldn't fail now check their claim.** Affected tests include the `DefaultWalker` cache use, cache on/off parity requiring a path, the recorder capability flags, the idle-pet gate, the monthly DST recurrence, the AdvancedSearch `Record.Exception` wrappers, `CanFitItem` with no branch-guarded assert, the Firewall callback, and `Import_NoMap`. The Geometry Parse asserts also now use the same input as their TryParse.
## Shared state
Every test that changes process-wide state now restores it, even when an assert fails:
- **The clock.** A new `TimerWheelScope` saves and restores `Core._tickCount`, `Core._now` and the timer wheel. `SimulatedClockScope` covers decay tests.
- **StepCache settings and caches**, via `StepCacheStateScope`.
- **Network state.** The ManualAllowlist, LoginAllowlist, firewall and BanChannel reporters are restored. Global packet handlers are now registered per test and removed afterwards, so they no longer stay registered for later tests.
- **Data and config tables.** `ArchiveJournal` statics, `TileData.ItemTable`, `ObjectPropertyList.Enabled`, `Core.Expansion` and the localization table are restored. One `MobileStatusVersion` restore was resetting a different expansion than the one the test changed.
- **Entities.** Tests with fixed serials now delete what they create. Fixed mobile serials moved to `World.NewMobile`, because deleting a hard-coded serial could unregister a live entity.
- **Collection race.** `SpiralScanTests` no longer clears the spawn cache from a separate collection that could run in parallel with the cache-manager tests.
- **Shared statics.** Classes reading shared static allowlists joined the sequential collection.
## Duplication
New shared helpers replace copy-pasted harness code:
- **Server.Tests:** `PacketAssert.Sends`, used by about 130 packet tests; `TestClientHarness`, `PersistencePipelineScope`, `NetTestHelpers`, `EntityCleanup` and one `TestMulti`.
- **UOContent.Tests:** `TimerWheelScope`, which also holds one wraparound-safe `RunFor`/`RunUntil`; `AiTestStubs`, which merges six walker copies and five identical creature stubs; `MobileTestHarness`, `CommandTestHarness` and `TempDirectory`.
Dead oracle packet classes are removed.
## Cost
| Test | Before | After |
|---|---|---|
| `VirtualSerialTests` | 1.57s | 2ms (starts near the wrap via reflection instead of drawing ~17.9M serials) |
| Event scheduler recurrence loop | 473ms | 1ms |
| CrowdSec drain | 517ms | 16ms (event-driven instead of a fixed 500ms wait) |
| AuthId tests | 248ms | 99ms (SHA2 except the two GameLogin tests) |
| Container cap tests | 5,000 items | 3,449 / 3,277 (one past each protocol cap) |
| Bulk removal | 100k children | 20k children |
| OPL hash distribution | 60k builds | 10k builds |
## Shard-tuning decoupling
- AI assertions read `ReacquireDelay` and `FollowMoveSpeed` instead of hard-coding production tuning.
- The guard friendly-fire test uses an `AI_Mage` stub instead of WhiteWyrm.
- Argon2 assertions pin the `argon2id` variant and `v=19` but read the cost parameters from the configured hasher.
## Tests
| Run | Server.Tests | UOContent.Tests |
|---|---|---|
| With client files | 812 pass | 1,123 pass |
| Without client files (CI simulation) | 800 pass, 12 skip | 1,021 pass, 102 skip |
- The UOContent fixture reads `MODERNUO_TEST_DATA_DIR`; Server.Tests reads `MODERNUO_CLIENT_PATH`.
- The 15 new UOContent skips are the map-dependent guards above.
- Total run time with client files is about 5s, down from about 8s.
## Summary
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.
Kamron Batman