## Problem
Items dropped on the ground never decay. Corpses do, which makes the breakage look selective — but corpses are unaffected only because `Corpse.BeginDecay` runs its own `InternalTimer` and never touches `DecayScheduler`. Ordinary items are the only things that depend on the scheduler.
## Root cause
`Item.MoveToWorld` called `SetLastMoved()` — which triggers `UpdateDecayRegistration()` — at the *top* of the method, before detaching the item from its parent and before assigning the new map. `CanDecay()` reads `Decays`, `Parent`, **and** `Map`, so registration was evaluated against the item's *pre-move* state.
Because the `Item` constructor sets `m_Map = Map.Internal`, and `Mobile.Lift` calls `item.Internalize()` to put an item on the cursor, registration was consistently one step behind:
| State | Tracked for decay? | |
|---|---|---|
| Item on the ground | **No** | never decays |
| Item held on the cursor | **Yes** | backwards |
Nothing corrected it afterwards: the later `m_Map = map` assigns the field directly, bypassing the `Map` property setter, and that setter does not refresh registration either. With no parent, `RemoveItem` (which *does* re-register) never runs.
World load masked this — `ItemPersistence.PostDeserialize` re-registers every item against its final state, so decay appears to work for items that survive a restart. Only freshly dropped items are affected.
**Fix:** stamp `LastMoved` up front so decay math stays correct, then call `UpdateDecayRegistration()` once the parent, map, and location are final.
## Audit of the rest of the call sites
All 16 `SetLastMoved()` call sites were reviewed. `SetLastMoved()` must keep refreshing registration — `LastMoved` feeds `ScheduledDecayTime` and therefore which bucket an item belongs in — but it may only run once parent/map are final. The vendor, house, lift and drop sites already satisfy that. The rest of this PR fixes the ones that did not, plus what the audit turned up:
- **`Item.Deserialize`** stamped via `SetLastMoved()` before the version data was read, registering against an unread `Map`/`Parent`. Safe only by accident (the `Item(Serial)` ctor leaves flags at 0, so `Decays` is false), and it cost an unregister per item per world load. Now stamps only; `PostDeserialize` does the registration.
- **`Item` constructor** registered then immediately unregistered every item — the `Movable` setter saw `m_Map` still null, so `CanDecay()` was true. Also removes a `Configure`-order landmine: constructing an `Item` before `DecayScheduler.Configure()` would have thrown in `Shared.Start()`.
- **`Container.Destroy`** stamped `LastMoved` immediately before `MoveToWorld`, which now stamps it itself.
- **`Unregister` was documented O(1)** but scanned twelve buckets and did a linear `PriorityQueue.Remove`, on every construction, deserialize and move. Items now record where they are tracked in `Item.DecaySlot` (1 byte), so untracked items — the common case — leave in O(1).
- **A refused decay silently dropped the item.** `ProcessActiveQueue` deleted on `OnDecay() == true` but did nothing when a region refused, leaving the item dequeued, untracked and on the ground forever. It now restarts the decay clock; re-registering as-is would spin, since `ScheduledDecayTime` is already past.
## Two content bugs of the same class
The decay system replaced a polling sweep. Under polling, a `Decays`/`DecayTime` override could read live state every pass. Under a registration model it cannot — the scheduler drops items that stop being eligible, but nothing enrols one that becomes eligible while untracked.
- **`TreasureChestLevel1-4`** overrode `DecayTime` as `Utility.Random(15, 60)` — a fresh roll on every read. `ScheduledDecayTime` is read repeatedly (to bucket, to re-bucket on rotation, to test whether due), so those reads disagreed: the chest re-bucketed every tick and decayed early instead of after its intended interval. Now rolled once per chest. Distribution unchanged — `Utility.Random(from, count)` is RunUO's `from + Next(count)`, so this is 15–74 minutes, as before.
- **`StrongBox`** overrode `Decays` with a live check on `_house`, `_owner.Deleted` and `IsCoOwner`. Nothing notifies the box when any of those change, so it was never enrolled and the override never decayed anything — and it could not have: `HouseRegion.OnDecay` refuses a secured item inside a standing house, and the box is in `Secures`. Decay was never the mechanism here.
- A strongbox is only ever its owner's. Without a house, or without an owner still co-owning that house, it would be a free container anyone could loot, so `Validate()` destroys it. The old check missed exactly those two cases — it required a non-null owner and treated a null house as valid. A deleted owner deserializes back as null, which `IsCoOwner` rejects. The now meaningless `Decays`/`DecayTime` overrides and an unhelpful `Console.WriteLine` are gone.
`DecayScheduler` now documents both constraints.
## Tests
`DecayRegistrationTests` (Server) covers world placement, lift/drop, container round-trip, cursor-held (must *not* track), `Container.Destroy` spill, `DecaySlot`/structure agreement, refused decay, and a full decay lifecycle driven through the scheduler. `TreasureChestDecayTests` (UOContent) locks `DecayTime`/`ScheduledDecayTime` stability across all four chest levels.
To make the lifecycle testable deterministically, `DecayScheduler` gains `internal` members (visible only via existing `InternalsVisibleTo`): `IsRegistered()`, `ProcessTick(now)` — extracted from `OnTick()` with no behaviour change — and `ResetForTests()`.
Red/green verified. Without the `MoveToWorld` fix, 5 of 6 of the original tests fail, including `ItemOnGround_ActuallyDecaysAfterDecayTime`, which shows a ground item never decays even after a full simulated hour. Without the chest fix, 8 of 8 chest tests fail. Without the refused-decay fix, that test fails.
Server.Tests 737/737 and UOContent.Tests 509/509 pass.
## Summary
Removes per-call heap allocations from `Container`'s consume / find / group hot paths and from `BaseCreature.OnDeath`'s fame/karma tracking. The headline wins: kill the `List<List<Item>>` + `Item[][]` + `int[]` grouping bridges in `ConsumeTotal*` / `ConsumeTotalGrouped*` / `GetBestGroupAmount*`, and kill the per-call `Predicate<Item>` allocations in `FindItemsByType(Type)` / `FindItemsByType(Type[])`.
### `Container.cs`
- `ConsumeTotal`, `ConsumeTotalGrouped`, `GetBestGroupAmount` now share four streaming helpers (`HasAmount`, `TryFindGroupMeetingAmount`, `BestGroupTotal`, `ConsumeSlice`) backed by `PooledRefList` instead of allocating per-group lists and jagged arrays. Two-phase validate-then-consume pattern preserved — all-or-nothing semantics for spell reagents, vendor pay, and crafting still hold.
- `(Type)` / `(Type[])` / `(Type[][])` overload trios collapsed to single `ReadOnlySpan<Type>` + `ReadOnlySpan<int>` implementations. Implicit `T[] → ReadOnlySpan<T>` conversion means UOContent callers compile unchanged.
- Unused overloads deleted: `ConsumeTotalGrouped(Type)`, `ConsumeTotalGrouped(Type[][])`, `GetBestGroupAmount(Type)`, `GetBestGroupAmount(Type[][])`, plus the never-called `TryDropItems` hook and its private `ItemStackEntry` struct.
- Fixes a `PooledRefList` leak in `GetBestGroupAmount(Type[], …)` (missing `using`).
- `m_ContainerData` / `m_Items` / `m_TotalGold` / `m_TotalItems` / `m_TotalWeight` / `ContainerData.m_Table` / `ContainerData.logger` renamed to the underscored convention. `m_Items` cross-file rename for the Container-side references in `Item.cs`; `Item.CompactInfo.m_Items` deliberately left alone (separate effort).
- `CheckHold` parent walk simplified; trivial dispatch methods (`CheckHold` overloads, `OnItemAdded`, `OnItemRemoved`, `OnStackAttempt`) get `[MethodImpl(AggressiveInlining)]`; `Destroy` and `DisplayTo` cache `Items` outside the loop; dead comments removed.
### `Item.Enumerable.cs`
- `FindItemsByType(Type)` previously allocated a `Predicate<Item>` per call (method-group conversion). `FindItemsByType(Type[])` allocated a closure capturing `types`. Both now construct the enumerator with a `Type` / `ReadOnlySpan<Type>` field directly, no delegate.
- `FindItemsByTypeEnumerator<T>` gains two constructors plus a `Matches(T)` helper that picks the right filter inline. Constructor chaining via a private 2-arg seed constructor incidentally fixes a pre-existing bug where `PooledRefQueue` was always rented at capacity 0 because `_recurse` hadn't been assigned yet.
- `(Type[])` overload of `FindItemsByType` becomes `(ReadOnlySpan<Type>)`.
- `EnumerateItemsByType(Type)` / `EnumerateItemsByType(ReadOnlySpan<Type>)` / `ListItemsByType(Type)` / `ListItemsByType(ReadOnlySpan<Type>)` simplified to delegate to the new alloc-free overloads instead of filtering manually.
### `Utility.cs`
- `InTypeList<T>(this T, Type[])` and `InTypeList(this Type, Type[])` switched to `ReadOnlySpan<Type>`.
### `BaseCreature.cs`
- `OnDeath` per-death `List<Mobile>` / `List<int>` / `List<int>` for fame/karma tracking switched to `PooledRefList`.
### Summary
Container enumeration is in dire need of optimization. Thanks to @stefanomerotta for initiating this work with PR #1443. This PR handles a small part of what Stefan started. Also included are some bug fixes.
### Method Signatures
```cs
// Use with foreach without moving/deleting items
FindItemsByTypeEnumerator<T> FindItemsByType<T>(bool recurse = true, Predicate<T> predicate = null)
// Use with foreach when moving/deleting items
QueuedItemsEnumerator<T> EnumerateItemsByType<T>(bool recurse = true, Predicate<T> predicate = null)
// Use when iterating multiple times or queuing
PooledRefQueue<T> QueueItemsByType<T>(bool recurse = true, Predicate<T> predicate = null)
// Use when iterating multiples times or manipulating elements without traversing
PooledRefList<T> ListItemsByType<T>(bool recurse = true, Predicate<T> predicate = null)
```
* `FindItemsByType<T>` has changed from returning `List<T>` to `FindItemsByTypeEnumerator<T>` - This method is not safe to use in situations where an item may get consumed, deleted, or moved.
* `EnumerateItemsByType<T>` was added as a safe way to iterate and manipulate items.
* **Note**: EnumerateItemsByType will _completely traverse the container_ before iteration starts because it uses `QueueItemsByType` under the hood.
* `QueueItemsByType<T>` and `ListItemsByType<T>` was added to return a queue or list of items to iterate multiple times and manipulate the items. This isn't the most efficient since it uses a predicate and can result in 2 or 3 total iterations unnecessarily.
### Bug Fixes
- [X] Fishing had an error in the random check that may have caused slight bias.
