## Summary
Folds **113** hand-written `[SerializableProperty]` members into plain `[SerializableField]` declarations using the v4 setter hooks — value coercion/vetoes via `allowFieldChange`, post-change side effects via `fieldChanged` (whose `oldValue` parameter covers the old-house/old-sender unsubscribe patterns). Net **-450 lines** of setter boilerplate.
```cs
// before
[SerializableProperty(1)]
[CommandProperty(AccessLevel.GameMaster)]
public int Charges
{
get => _charges;
set
{
_charges = Math.Clamp(value, 0, MaxCharges);
InvalidateProperties();
this.MarkDirty();
}
}
// after
[SerializableField(1, allowFieldChange: nameof(AllowChargesChange))]
[SerializedCommandProperty(AccessLevel.GameMaster)]
[InvalidateProperties]
private int _charges;
private bool AllowChargesChange(ref int value)
{
value = Math.Clamp(value, 0, MaxCharges);
return true;
}
```
## How sites were selected
A classifier parsed all 204 `[SerializableProperty]` sites and converted only those matching strict shapes: getter is exactly `get => _field;`, the assignment comes first (after at most an equality guard), and relocated side effects contain no `return`, no `value` mutation, and no field re-assignment. Everything else was left alone deliberately:
- **~34 custom getters** (fallback defaults like `_x == -1 ? Default : _x`, self-healing refs) — no setter hook can express these.
- **~35 pre-assignment logic** (durability Unscale/Scale sandwiches, old-state captures like PotionKeg's pile weight).
- **virtual/override members, name-mismatched backing fields (`m_`), exotic semantics** (guards' `Focus` does work on *equal* assignment; `ChampionSpawn.Active` never assigns its field).
Five sites the classifier refused were converted by hand where the hooks fit cleanly: `ReceiverCrystal.Sender`, `PlayerVendor.House`, `PlayerBarkeeper.House` (old-value unsubscribe via `oldValue`), `BaseSuit.AccessLevel` (its existing virtual `OnAccessLevelChanged` already had the exact callback shape), and `DyeTub.DyedHue` (a true veto: `AllowDyedHueChange(ref int value) => _redyable`).
## Verification
- Build: **0 errors, 0 warnings**.
- **Schema regeneration produces zero Migrations changes** — the conversion is wire- and schema-neutral by construction (same orders, types, and property names), and CI's schema diff check enforces it.
- **835 + 708 tests green.**
## Behavioral notes (all strict improvements, called out for review)
- Generated setters skip everything when the incoming value equals the current one; a few converted setters previously re-ran side effects on equal assignment (redundant `Update()`-style refreshes).
- Generated setters always `MarkDirty()` on change; several converted setters never did (e.g. `DyeTub.DyedHue`, `MorphItem` ranges) — their changes only persisted if something else dirtied the entity. Those latent persistence bugs are fixed by construction.
## Summary
Adopts ModernUO.Serialization 4.0.0 across the engine. Three commits, reviewable independently:
1. **Package + tool bump to 4.0.0** (`Server.csproj`, `UOContent.csproj`, `dotnet-tools.json`).
2. **Timers → `[DeserializeTimer]`** — the 8 drifting timers (BaseLight, TreasureMapChest, MarkContainer, FillableContainer, DeathRobe, DecayedCorpse, Corpse, BaseEscortable) now store their next tick as **anchored time**: server downtime no longer consumes the remaining delay, and idle-world saves are byte-stable. This changes their wire format, so each class bumps its serialization version with a `MigrateFrom` that replays the old delta-time read through the migration schema (the new `vN.json` files carry `@AnchoredTimer`; the old ones keep `@TimerDrift`, which the generator reads forever). The 2 wall-clock timers (Aquarium, FountainOfLife) keep their exact format via `wallClock: true` — no bump. Restart methods drop their `TimeSpan.MinValue` sentinel checks: v4 invokes them **only when a timer was actually running at save**.
3. **Linkage → field-side declarations** — 175 conversions across 25 files: `[SerializableFieldSaveFlag(order)]`/`[SerializableFieldDefault(order)]` become `[SaveFlag(nameof(...), nameof(...))]` on the field, and `[SerializableFieldChanged(order)]` becomes the `fieldChanged:` argument of `[SerializableField]`. **Wire-neutral: zero migration schemas changed.**
## Verification
- Solution builds with **0 errors, 0 warnings**; all three 4.0.0 packages verified indexed on nuget.org (no local feed needed).
- **835 + 708 tests green.**
- Generated output inspected: old-version content structs replay `ReadDeltaTime` (e.g. `V3Content.DecayTimerNext = reader.ReadDeltaTime()`), current versions write/read anchored time with the gated restart, and the wall-clock classes emit byte-identical `Write`/`ReadDateTime` framing.
- Schema tool run is committed (CI's `git diff --exit-code` schema check passes): exactly the 8 expected new `vN.json` files, nothing else touched.
- The conversion was scripted with a class-scoped resolver (order → same-class `[SerializableField(order)]`/`[SerializableProperty(order)]`); it planned 175/175 with zero ambiguities before applying.
## Notes
- New `MigrateFrom`s use the content structs' provided `XxxDelay` property, matching the pre-existing idiom in Corpse's and TreasureMapChest's older migrations.
- Follow-up candidate (separate PR, wire-neutral, any time): fold the ~150 eligible hand-written `[SerializableProperty]` setters (clamps, post-change side effects) down to `[SerializableField]` with `allowFieldChange`/`fieldChanged` hooks.
### Summary
Players reported an exploit: decipher a treasure map, then run a ClassicUO/Razor organizer agent that pulls the gold out of the chest in small amounts. Each pull spawned more monsters, turning one chest into an unbounded farmable spawn generator.
### Root cause
`TreasureMapChest.OnItemLifted` grants a 10% guardian spawn roll per first-time-lifted item, deduplicated by the instance-keyed `_lifted` set. But a partial lift goes through `Mobile.LiftItemDupe`, which re-adds the stack remainder to the chest as a **brand-new item instance** (engine-side `AddItem`, bypassing the `CheckHold` block on refilling). Every subsequent pull lifts an instance the `_lifted` set has never seen, so each one re-rolls the 10% spawn chance:
- A level 4 chest holds 4,000 gold → pulled coin by coin, ~400 spawned creatures (plus more from reagent stacks), hands-free, per chest.
- Spawns use `guardian: false`, so nothing tracks or caps them.
- Legit full-stack looting yields roughly 5–8 bonus spawns per chest for comparison.
The code is inherited from RunUO, so descendant shards likely share the hole.
### Fix
Mark every item that enters the chest **after the initial fill** as already lifted, via an `OnItemAdded` override gated by a non-serialized `_filled` flag (set at the end of the constructor and in `[AfterDeserialization]`). Ordering makes this exact: `LiftItemDupe` re-adds the remainder *before* the chest's `OnItemLifted` runs, so the lifted original still gets its one legitimate roll while the remainder is pre-marked.
This also covers packing items *into* the chest (e.g., merging gold back in to lift it out again) and bounce-backs — anything not part of the original loot can never grant a spawn roll.
### Tests
- `PartialLift_MarksSplitRemainderAsLifted` — drives the real `Mobile.Lift` path with a 1-coin pull and asserts the split remainder is marked (failed before the fix).
- `ItemAddedAfterFill_IsMarkedLifted` — post-fill additions are marked (failed before the fix).
- `OriginalFillLoot_IsNotMarkedLifted` — original loot keeps spawn-roll eligibility.
Full `UOContent.Tests` suite: 701 passed.
## 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
Migrates the five-step SoulStone wizard and the TreasureMapChest remove-confirmation dialog from legacy `Gump` to the modern builder API.
Per-gump base type:
- **`SelectSkillGump` -> `DynamicGump`** -- the skill picker iterates the player's skill list and emits one button per non-zero skill, so the layout shape varies per instance.
- **`ConfirmSkillGump` -> `DynamicGump`** -- skill name uses `AosSkillBonuses.GetLabel(...)` which returns dynamic clilocs in the `1044060 + (int)skill` range, plus current/cap skill values rendered as text labels.
- **`ConfirmTransferGump` -> `DynamicGump`** -- same dynamic skill cliloc plus per-instance Base/Cap/Stored values.
- **`ConfirmRemovalGump` -> `StaticGump<ConfirmRemovalGump>`** -- only fixed clilocs (warning text, Continue, Cancel), so the layout caches.
- **`ErrorGump` -> `DynamicGump`** -- title and message clilocs are constructor parameters that vary per call site.
- **`TreasureMapChest.RemoveGump` -> `StaticGump<RemoveGump>`** -- fixed-cliloc confirmation prompt (no item list, despite the name); `Closable=false`/`Disposable=false` are now `builder.SetNoClose()`/`builder.SetNoDispose()`.
All six gumps are now `Singleton => true`, have private constructors, and expose a static `DisplayTo` entry point that validates `from`, `NetState`, and the underlying entity before constructing -- prevents the empty-gump leak. Wizard navigation between steps now goes through `DisplayTo` (e.g. `ConfirmSkillGump.DisplayTo(from, _stone, skill)` from the skill picker, `ErrorGump.DisplayTo(...)` from absorption pre-checks, `SelectSkillGump.DisplayTo(...)` from the "make another selection" button on `ConfirmSkillGump` and from `ErrorGump` bounce-back). Because each gump is Singleton, sending the same type again automatically closes any prior instance instead of stacking; the explicit `gumps.Close<T>()` chain on `OnDoubleClick` is preserved so opening the soulstone still resets any orphaned step from another wizard.
`OnResponse` now uses `in RelayInfo info`. All inline `AddX(...)` calls move to `builder.AddX(...)` inside `BuildLayout`. `Skill.Base.ToString("F1")` etc. are converted to `$"{value:F1}"` interpolation passed to `AddLabel(ReadOnlySpan<char>)`. Skill picker pagination still uses client-side `AddPage` / `GumpButtonType.Page` -- no server-state pagination to migrate.
### Summary
Updates all calls to container.EnumerateItems() to properly dispose of the underlying PooledRefQueue so that we are properly recycling pooled arrays.
> [!Important]
> **Developer Note**
> This code change will **completely move gumps out of the core**
### Summary
- Adds `GetGumps()` convenience which exposes methods to Find/Close/Send multiple gumps. This helper is a performance improvement by eliminating the Dictionary<Player, List> lookup for gumps.
## Summary
- Removes allocation of a `List<ContextMenuEntry>` every time a context menu is created.
- Moves packet/context menu creation logic out of the core
- Fixes tame entry
## BREAKING CHANGE
> [!Important]
> **Developer Note**
> ```cs
> public virtual void GetContextMenuEntries(Mobile from, List<ContextMenuEntry> list)
> ```
> and similar functions changed to
> ```cs
> public virtual void GetContextMenuEntries(Mobile from, ref PooledRefList<ContextMenuEntry> list)
> ```
### Summary
- Fixes an issue that causes CUO to crash due to bad string caching in static gumps
- Fixes wrong/bad 16bit html gump hues.
- Moves `C16232` (16-bit to 32-bit) and `C32216` (32-bit to 16-bit) to Utility class for broader use.
TODO:
- Some gumps have different 32bit (for string content) vs 16bit (for localized content) strings. Does this matter?
### Summary
- Fixes `[AdvancedSearch` being accessible by players 😱
- Adds `[GenCommands` to generate the same commands html page on https://muo.gg/commands.
- Fixes `[helpinfo` so all commands properly show up!
> [!WARNING]
> ### Developer Warning:
> Commands must now be registered in the `Configure` bootup phase.
> If a command is not registered early enough, it may not be available to systems like [helpinfo
> that cache their information.
> [!NOTE]
> ### Developer Note:
> Various commands related to generating content have been changed to _Developer_ and above access level.
### Screenshots
<img width="673" alt="image" src="https://github.com/modernuo/ModernUO/assets/3953314/b105b5c9-5eb4-4ace-93ff-1bfb31e7132f">
<img width="547" alt="image" src="https://github.com/modernuo/ModernUO/assets/3953314/e97487e8-47a5-4aa7-89cc-9fe3deda584d">
### Summary
Modifying a ValueLinkList using one of the methods will bump the "version". This field is used by iterators (foreach loops) to determine if the link list was modified while iterating. The sector.Items (and in the future other lists), will no longer be safe to modify while iterating. The server will _CRASH_ if the ValueLinkList is modified.
Thanks to @stefanomerotta for help!
### Screenshots
<img width="588" alt="image" src="https://github.com/modernuo/ModernUO/assets/3953314/83ee0b6e-ff4f-4768-9e29-84456e04b1ec">
### Summary
Eliminates `IPooledEnumerable<T>` and `eable.Free()` from `Map` for items. This drastically simplifies code that iterates in range, for example:
```cs
foreach (var item in m.GetItemsInRange(5))
{
}
```
The code above no longer requires an eable and calling `Free()`.
### 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.
* Adds `SerializationProperty` - This will generate the private variable for serialization. Also provides an option `useField` to specify your own. Example:
```cs
[SerializationProperty(0, useField: nameof(_resource)]
```
## Breaking Changes (New API)
ObjectPropertyList supports the following API:
```cs
list.Add(500000);
list.Add(500001, stringArgument);
list.Add("Some text");
list.Add($"Some text with {argument}");
list.Add(500002, $"{arg1}\t{arg2}");
```
## Notes
1. All API uses that require a formatter like this:
```cs
list.Add(500002, "{0}\t{1}", arg1, arg2);
```
Should be changed to use string interpolation, for example:
```cs
list.Add(500002, $"{arg1}\t{arg2}");
```
2. The following paradigm should no longer be used:
```cs
list.Add(1061170, prop.ToString()); // strength requirement ~1_val~
```
The new string interpolation API will avoid having to convert the argument to a string before writing it to the packet. Instead use the following:
```cs
list.Add(1061170, $"{prop}"); // strength requirement ~1_val~
```
### Benchmarks
```cs
| Method | Mean | Error | StdDev | Gen 0 | Allocated |
|------------------------------- |---------:|--------:|--------:|-------:|----------:|
| BenchmarkOldOPL | 241.0 ns | 0.56 ns | 0.47 ns | 0.0105 | 88 B |
| BenchmarkStringInterpolatedOPL | 199.9 ns | 2.44 ns | 2.39 ns | - | - |
```
### Changes
- [X] Removes crash in STArray.Return when array is null.
- [X] Fixes NPE in OPL when entity is null. Serial in packet will be 0 when entity is null.
- [X] Fixes NPE in AosAttributes when Parent is null.
- [X] Changes OPL to use string interpolation.
- [X] Introduces `IPropertyList` to allow extending PropertyList for other uses.
