fix: Items dropped on the ground never decay (#2536)

## 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.

Projects/Server.Tests/Tests/Items/DecayRegistrationTests.cs

@@ -0,0 +1,232 @@
+using System;
+using Server.Items;
+using Xunit;
+
+namespace Server.Tests;
+
+[Collection("Sequential Server Tests")]
+public class DecayRegistrationTests
+{
+    // The wheel is static and these tests move the clock, so start each from a known schedule.
+    public DecayRegistrationTests() => DecayScheduler.ResetForTests();
+
+    // Stands in for a region that refuses decay, e.g. HouseRegion over a locked down item.
+    private class RefusesDecayItem : Item
+    {
+        public RefusesDecayItem() : base(0x1234)
+        {
+        }
+
+        public override bool OnDecay() => false;
+    }
+
+    // The item is already dequeued, so a refusal that does not reschedule strands it forever.
+    [Fact]
+    public void ItemWhoseDecayIsRefused_StaysTrackedAndSurvives()
+    {
+        var start = Core._now;
+
+        try
+        {
+            var item = new RefusesDecayItem();
+            item.MoveToWorld(new Point3D(106, 100, 0), Map.Felucca);
+
+            AdvanceDecay(start, item.DecayTime + TimeSpan.FromMinutes(2), item);
+
+            Assert.False(item.Deleted, "A refused decay must not delete the item.");
+            Assert.True(
+                DecayScheduler.IsRegistered(item),
+                "A refused decay must leave the item tracked so it is retried."
+            );
+
+            item.Delete();
+        }
+        finally
+        {
+            Core._now = start;
+        }
+    }
+
+    // Drives the scheduler and the game clock without depending on the shared timer wheel.
+    private static void AdvanceDecay(DateTime start, TimeSpan duration, Item item)
+    {
+        var deadline = start + duration;
+
+        for (var now = start; now <= deadline && !item.Deleted; now += TimeSpan.FromMilliseconds(256))
+        {
+            Core._now = now;
+            DecayScheduler.ProcessTick(now);
+        }
+    }
+
+    [Fact]
+    public void ItemOnGround_ActuallyDecaysAfterDecayTime()
+    {
+        var start = Core._now;
+
+        try
+        {
+            var item = new Item(0x1234);
+            item.MoveToWorld(new Point3D(100, 100, 0), Map.Felucca);
+
+            Assert.True(DecayScheduler.IsRegistered(item));
+
+            AdvanceDecay(start, item.DecayTime + TimeSpan.FromMinutes(2), item);
+
+            Assert.True(item.Deleted, "Item on the ground must decay once DecayTime elapses.");
+        }
+        finally
+        {
+            Core._now = start;
+        }
+    }
+
+    [Fact]
+    public void ItemOnGround_DoesNotDecayBeforeDecayTime()
+    {
+        var start = Core._now;
+
+        try
+        {
+            var item = new Item(0x1234);
+            item.MoveToWorld(new Point3D(102, 100, 0), Map.Felucca);
+
+            AdvanceDecay(start, item.DecayTime - TimeSpan.FromMinutes(2), item);
+
+            Assert.False(item.Deleted, "Item must not decay before DecayTime elapses.");
+
+            item.Delete();
+        }
+        finally
+        {
+            Core._now = start;
+        }
+    }
+
+    [Fact]
+    public void ItemPlacedInWorld_IsRegisteredForDecay()
+    {
+        var item = new Item(0x1234);
+        item.MoveToWorld(new Point3D(100, 100, 0), Map.Felucca);
+
+        Assert.True(item.CanDecay());
+        Assert.True(DecayScheduler.IsRegistered(item), "Item on the ground must be tracked for decay.");
+
+        item.Delete();
+    }
+
+    // The real client path: Mobile.Lift internalizes, then Mobile.Drop calls MoveToWorld.
+    [Fact]
+    public void ItemLiftedThenDroppedToGround_IsRegisteredForDecay()
+    {
+        var item = new Item(0x1234);
+        item.MoveToWorld(new Point3D(100, 100, 0), Map.Felucca);
+
+        // Player lifts the item to the cursor.
+        item.Internalize();
+        Assert.Equal(Map.Internal, item.Map);
+
+        // Player drops it back onto the ground.
+        item.MoveToWorld(new Point3D(101, 100, 0), Map.Felucca);
+
+        Assert.True(item.CanDecay());
+        Assert.True(
+            DecayScheduler.IsRegistered(item),
+            "Item dropped to the ground after being lifted must be tracked for decay."
+        );
+
+        item.Delete();
+    }
+
+    // Held on the cursor means Map.Internal, which must not be tracked.
+    [Fact]
+    public void ItemLiftedToCursor_IsNotRegisteredForDecay()
+    {
+        var item = new Item(0x1234);
+        item.MoveToWorld(new Point3D(100, 100, 0), Map.Felucca);
+
+        item.Internalize();
+
+        Assert.False(item.CanDecay());
+        Assert.False(DecayScheduler.IsRegistered(item), "Item held on the cursor must not be tracked for decay.");
+
+        item.Delete();
+    }
+
+    // A new item is on Map.Internal, so construction must not touch the scheduler.
+    [Fact]
+    public void NewItem_IsNotTracked()
+    {
+        var item = new Item(0x1234);
+
+        Assert.Equal(DecayScheduler.SlotNone, item.DecaySlot);
+        Assert.False(DecayScheduler.IsRegistered(item), "A newly constructed item must not be tracked.");
+
+        item.Delete();
+    }
+
+    // Unregister() trusts DecaySlot; IsRegistered() scans the structures. Assert both to catch drift.
+    [Fact]
+    public void DecaySlot_AgreesWithStructures_AcrossLifecycle()
+    {
+        var item = new Item(0x1234);
+
+        item.MoveToWorld(new Point3D(103, 100, 0), Map.Felucca);
+        Assert.NotEqual(DecayScheduler.SlotNone, item.DecaySlot);
+        Assert.True(DecayScheduler.IsRegistered(item));
+
+        // Lifted to the cursor: untracked, and no stale slot left behind.
+        item.Internalize();
+        Assert.Equal(DecayScheduler.SlotNone, item.DecaySlot);
+        Assert.False(DecayScheduler.IsRegistered(item));
+
+        // Back to the ground: tracked again.
+        item.MoveToWorld(new Point3D(104, 100, 0), Map.Felucca);
+        Assert.NotEqual(DecayScheduler.SlotNone, item.DecaySlot);
+        Assert.True(DecayScheduler.IsRegistered(item));
+
+        item.Delete();
+        Assert.False(DecayScheduler.IsRegistered(item), "A deleted item must leave no trace in the scheduler.");
+    }
+
+    // Contents spilled onto the ground must start decaying.
+    [Fact]
+    public void ContainerDestroy_ContentsDroppedToGroundAreTracked()
+    {
+        var pack = new Container(0xE75);
+        pack.MoveToWorld(new Point3D(105, 100, 0), Map.Felucca);
+
+        var item = new Item(0x1234);
+        pack.AddItem(item);
+        Assert.False(DecayScheduler.IsRegistered(item));
+
+        pack.Destroy();
+
+        Assert.True(item.CanDecay());
+        Assert.True(DecayScheduler.IsRegistered(item), "Contents spilled by Container.Destroy must be tracked.");
+
+        item.Delete();
+    }
+
+    // Dropping into a container must untrack; taking it back out to the ground must re-track.
+    [Fact]
+    public void ItemMovedIntoContainerThenBackToGround_IsRegisteredForDecay()
+    {
+        var pack = new Container(0xE75);
+        pack.MoveToWorld(new Point3D(100, 100, 0), Map.Felucca);
+
+        var item = new Item(0x1234);
+        pack.AddItem(item);
+
+        Assert.False(DecayScheduler.IsRegistered(item), "Item inside a container must not be tracked for decay.");
+
+        item.Internalize();
+        item.MoveToWorld(new Point3D(101, 100, 0), Map.Felucca);
+
+        Assert.True(item.CanDecay());
+        Assert.True(DecayScheduler.IsRegistered(item), "Item taken out of a container must be tracked for decay.");
+
+        item.Delete();
+        pack.Delete();
+    }
+}

Projects/Server/Items/Container.cs

@@ -437,9 +437,7 @@ public partial class Container : Item
 
         for (var i = items.Count - 1; i >= 0; --i)
         {
-            var item = items[i];
+            items[i].MoveToWorld(loc, map);
-            item.SetLastMoved();
-            item.MoveToWorld(loc, map);
         }
 
         Delete();

Projects/Server/Items/DecayScheduler.cs

@@ -22,9 +22,24 @@ namespace Server.Items;
 /// Timer wheel-based decay scheduler with O(1) registration/unregistration.
 /// Uses coarse-grained HashSet buckets for time intervals, with a PriorityQueue
 /// for fine-grained processing of items due within the current window.
+/// <para>
+/// An item that becomes decay eligible must be passed to <see cref="Item.UpdateDecayRegistration" />
+/// once its parent, map and location are final. The scheduler drops items that stop being eligible,
+/// but nothing enrolls one that becomes eligible while untracked.
+/// </para>
+/// <para>
+/// <see cref="Item.Decays" /> and <see cref="Item.DecayTime" /> overrides must be stable rather than
+/// recomputed per read: <see cref="Item.ScheduledDecayTime" /> is read repeatedly, and a value that
+/// changes between reads re-buckets the item every tick and decays it at the wrong time.
+/// </para>
 /// </summary>
 public class DecayScheduler : Timer
 {
+    // Item.DecaySlot values; any non-negative value is a bucket index.
+    internal const sbyte SlotNone = -1;
+    internal const sbyte SlotOverflow = -2;
+    internal const sbyte SlotActive = -3;
+
     private const int BucketCount = 12;
     private static int _maxItemsPerTick;
     private static TimeSpan _tickInterval;
@@ -74,6 +89,65 @@ public class DecayScheduler : Timer
     {
     }
 
+    /// <summary>
+    /// Test hook: drops all tracked items and re-bases the rotation schedule on
+    /// <see cref="Core.Now" />, so deadlines left by a test that moved the clock do not suppress
+    /// rotations in the next one.
+    /// </summary>
+    internal static void ResetForTests()
+    {
+        Shared?.Stop();
+
+        _activeQueue.Clear();
+        _overflowBucket.Clear();
+
+        for (var i = 0; i < BucketCount; i++)
+        {
+            _buckets[i].Clear();
+        }
+
+        _currentBucketIndex = 0;
+
+        var now = Core.Now;
+        _nextBucketRotation = now + _bucketInterval;
+        _nextOverflowCheck = now + _totalBucketSpan;
+    }
+
+    /// <summary>
+    /// Diagnostics: true if the item is currently tracked in any bucket, the overflow
+    /// bucket, or the active queue.
+    /// </summary>
+    internal static bool IsRegistered(Item item)
+    {
+        if (item == null)
+        {
+            return false;
+        }
+
+        if (_overflowBucket.Contains(item))
+        {
+            return true;
+        }
+
+        for (var i = 0; i < BucketCount; i++)
+        {
+            if (_buckets[i].Contains(item))
+            {
+                return true;
+            }
+        }
+
+        foreach (var (queued, _) in _activeQueue.UnorderedItems)
+        {
+            if (queued == item)
+            {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     /// <summary>
     /// Checks if all decay tracking structures are empty.
     /// </summary>
@@ -115,11 +189,13 @@ public class DecayScheduler : Timer
         if (timeUntilDecay <= _bucketInterval)
         {
             _activeQueue.Enqueue(item, decayTime);
+            item.DecaySlot = SlotActive;
         }
         // If beyond the total bucket span, add to overflow bucket
         else if (timeUntilDecay > _totalBucketSpan)
         {
             _overflowBucket.Add(item);
+            item.DecaySlot = SlotOverflow;
         }
         else
         {
@@ -127,6 +203,7 @@ public class DecayScheduler : Timer
             var bucketOffset = GetBucketOffset(timeUntilDecay);
             var absoluteIndex = (_currentBucketIndex + bucketOffset) % BucketCount;
             _buckets[absoluteIndex].Add(item);
+            item.DecaySlot = (sbyte)absoluteIndex;
         }
     }
 
@@ -135,28 +212,27 @@ public class DecayScheduler : Timer
     /// </summary>
     public static void Unregister(Item item)
     {
-        if (item?.Deleted != false)
+        if (item == null || item.DecaySlot == SlotNone)
         {
             return;
         }
 
-        // Check overflow bucket first
+        var slot = item.DecaySlot;
-        if (_overflowBucket.Remove(item))
+        item.DecaySlot = SlotNone;
-        {
-            return;
-        }
 
-        // Check regular buckets
+        if (slot == SlotOverflow)
-        for (var i = 0; i < BucketCount; i++)
         {
-            if (_buckets[i].Remove(item))
+            _overflowBucket.Remove(item);
+        }
+        else if (slot == SlotActive)
         {
-                return;
-            }
-        }
-
             _activeQueue.Remove(item, out _, out _);
         }
+        else
+        {
+            _buckets[slot].Remove(item);
+        }
+    }
 
     /// <summary>
     /// Gets the bucket offset for an item based on time until decay.
@@ -171,10 +247,11 @@ public class DecayScheduler : Timer
         return Math.Clamp(bucketOffset, 0, BucketCount - 1);
     }
 
-    protected override void OnTick()
+    /// <summary>
+    /// Advances the wheel for <paramref name="now" />. Returns true when nothing is left to track.
+    /// </summary>
+    internal static bool ProcessTick(DateTime now)
     {
-        var now = Core.Now;
-
         // Process items from active queue first (smaller queue = faster log(n) operations)
         ProcessActiveQueue(now);
 
@@ -190,8 +267,13 @@ public class DecayScheduler : Timer
             ProcessOverflow(now);
         }
 
+        return IsEmpty();
+    }
+
+    protected override void OnTick()
+    {
         // Stop timer if nothing left to track
-        if (IsEmpty())
+        if (ProcessTick(Core.Now))
         {
             Stop();
             return;
@@ -220,7 +302,8 @@ public class DecayScheduler : Timer
         {
             if (item.Deleted || !item.CanDecay())
             {
-                continue; // Lazy cleanup - item was picked up or deleted
+                item.DecaySlot = SlotNone; // Lazy cleanup - item was picked up or deleted
+                continue;
             }
 
             var decayTime = item.ScheduledDecayTime;
@@ -233,6 +316,7 @@ public class DecayScheduler : Timer
                 {
                     // Extended beyond total span - move to overflow
                     _overflowBucket.Add(item);
+                    item.DecaySlot = SlotOverflow;
                 }
                 else
                 {
@@ -243,11 +327,13 @@ public class DecayScheduler : Timer
                     if (actualIndex != _currentBucketIndex)
                     {
                         _buckets[actualIndex].Add(item);
+                        item.DecaySlot = (sbyte)actualIndex;
                     }
                     else
                     {
                         // Still maps to current bucket - add to active queue
                         _activeQueue.Enqueue(item, decayTime);
+                        item.DecaySlot = SlotActive;
                     }
                 }
                 continue;
@@ -255,6 +341,7 @@ public class DecayScheduler : Timer
 
             // Due within next window - add to active queue
             _activeQueue.Enqueue(item, decayTime);
+            item.DecaySlot = SlotActive;
         }
 
         // Clear the processed bucket
@@ -276,10 +363,12 @@ public class DecayScheduler : Timer
         {
             if (item.Deleted || !item.CanDecay())
             {
+                item.DecaySlot = SlotNone;
                 return true; // Remove invalid items
             }
 
-            var timeUntilDecay = item.ScheduledDecayTime - now;
+            var decayTime = item.ScheduledDecayTime;
+            var timeUntilDecay = decayTime - now;
 
             if (timeUntilDecay > _totalBucketSpan)
             {
@@ -289,13 +378,15 @@ public class DecayScheduler : Timer
             // Now within regular bucket range - move to appropriate bucket
             if (timeUntilDecay <= _bucketInterval)
             {
-                _activeQueue.Enqueue(item, item.ScheduledDecayTime);
+                _activeQueue.Enqueue(item, decayTime);
+                item.DecaySlot = SlotActive;
             }
             else
             {
                 var bucketOffset = GetBucketOffset(timeUntilDecay);
                 var actualIndex = (_currentBucketIndex + bucketOffset) % BucketCount;
                 _buckets[actualIndex].Add(item);
+                item.DecaySlot = (sbyte)actualIndex;
             }
 
             return true; // Remove from overflow
@@ -319,6 +410,7 @@ public class DecayScheduler : Timer
 
             processed++;
             _activeQueue.Dequeue();
+            item.DecaySlot = SlotNone;
 
             if (item.Deleted || !item.CanDecay())
             {
@@ -333,6 +425,12 @@ public class DecayScheduler : Timer
             {
                 item.Delete();
             }
+            else
+            {
+                // Refused by the region. Restart the clock rather than dropping the item, which has
+                // already left the queue; re-registering as-is would spin on a due time in the past.
+                item.SetLastMoved();
+            }
         }
     }
 }

Projects/Server/Items/Item.cs

@@ -208,6 +208,10 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
     private ItemDelta m_DeltaFlags;
     private Direction m_Direction;
     private ImplFlag m_Flags;
+
+    // Where DecayScheduler tracks this item. Not serialized; PostDeserialize re-registers.
+    internal sbyte DecaySlot = DecayScheduler.SlotNone;
+
     private int m_Hue;
     private int m_ItemID;
     private Layer m_Layer;
@@ -225,12 +229,12 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
         m_ItemID = itemID;
         Serial = World.NewItem;
 
-        Visible = true;
+        // Assigned directly: the Visible/Movable setters and SetLastMoved() would register the item
-        Movable = true;
+        // for decay while m_Map is still null, then unregister it once it is Map.Internal.
+        m_Flags = ImplFlag.Visible | ImplFlag.Movable;
         Amount = 1;
         m_Map = Map.Internal;
-
+        LastMoved = Core.Now;
-        SetLastMoved();
 
         World.AddEntity(this);
     }
@@ -1107,7 +1111,9 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
         var oldLocation = GetWorldLocation();
         var oldRealLocation = m_Location;
 
-        SetLastMoved();
+        // Register at the end instead: CanDecay() reads parent, map and location, none of which
+        // are final yet.
+        LastMoved = Core.Now;
 
         if (Parent is Mobile mobile)
         {
@@ -1242,6 +1248,8 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
             Map = map;
             Location = location;
         }
+
+        UpdateDecayRegistration();
     }
 
     /// <summary>
@@ -2601,7 +2609,9 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
     {
         var version = reader.ReadInt();
 
-        SetLastMoved();
+        // Default for versions without a saved value. Stamp only: map and parent are still unread,
+        // and PostDeserialize registers every item once its state is final.
+        LastMoved = Core.Now;
 
         switch (version)
         {

Projects/UOContent.Tests/Tests/Items/TreasureChests/TreasureChestDecayTests.cs

@@ -0,0 +1,65 @@
+using System;
+using Server;
+using Server.Items;
+using Server.Tests;
+using Xunit;
+
+namespace UOContent.Tests;
+
+[Collection("Sequential UOContent Tests")]
+public class TreasureChestDecayTests
+{
+    public static TheoryData<Type> ChestTypes =>
+    [
+        typeof(TreasureChestLevel1),
+        typeof(TreasureChestLevel2),
+        typeof(TreasureChestLevel3),
+        typeof(TreasureChestLevel4)
+    ];
+
+    // DecayScheduler reads ScheduledDecayTime repeatedly and needs the reads to agree, so the
+    // random interval must be rolled once per chest rather than per read.
+    [Theory]
+    [MemberData(nameof(ChestTypes))]
+    public void DecayTimeIsStableAcrossReads(Type chestType)
+    {
+        var chest = chestType.CreateInstance<LockableContainer>();
+
+        try
+        {
+            var expected = chest.DecayTime;
+
+            for (var i = 0; i < 100; i++)
+            {
+                Assert.Equal(expected, chest.DecayTime);
+            }
+        }
+        finally
+        {
+            chest.Delete();
+        }
+    }
+
+    [Theory]
+    [MemberData(nameof(ChestTypes))]
+    public void ScheduledDecayTimeIsStableAcrossReads(Type chestType)
+    {
+        var chest = chestType.CreateInstance<LockableContainer>();
+
+        try
+        {
+            chest.MoveToWorld(new Point3D(150, 150, 0), Map.Felucca);
+
+            var expected = chest.ScheduledDecayTime;
+
+            for (var i = 0; i < 100; i++)
+            {
+                Assert.Equal(expected, chest.ScheduledDecayTime);
+            }
+        }
+        finally
+        {
+            chest.Delete();
+        }
+    }
+}

Projects/UOContent/Items/Containers/Strongbox.cs

@@ -32,10 +32,6 @@ public partial class StrongBox : BaseContainer, IChoppable
 
     public override int DefaultMaxWeight => 0;
 
-    public override bool Decays => _house == null || _owner?.Deleted != false || !_house.IsCoOwner(_owner);
-
-    public override TimeSpan DecayTime => TimeSpan.FromMinutes(30.0);
-
     public void OnChop(Mobile from)
     {
         if (_house?.Deleted != false || _owner?.Deleted != false || from == _owner || _house.IsOwner(from))
@@ -50,11 +46,13 @@ public partial class StrongBox : BaseContainer, IChoppable
         Timer.StartTimer(TimeSpan.FromSeconds(1.0), Validate);
     }
 
+    // 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 it goes away instead. A deleted
+    // owner deserializes back as null, which IsCoOwner rejects.
     private void Validate()
     {
-        if (_owner != null && _house?.IsCoOwner(_owner) == false)
+        if (_house?.IsCoOwner(_owner) != true)
         {
-            Console.WriteLine("Warning: Destroying strongbox of {0}", _owner.Name);
             Destroy();
         }
     }

Projects/UOContent/Items/TreasureChests/TreasureChestLevel1.cs

@@ -72,7 +72,10 @@ public partial class TreasureChestLevel1 : LockableContainer
 
     public override bool IsDecoContainer => false;
 
-    public override TimeSpan DecayTime => TimeSpan.FromMinutes(Utility.Random(15, 60));
+    // Rolled once: DecayScheduler reads ScheduledDecayTime repeatedly and needs a stable value.
+    private readonly TimeSpan _decayTime = TimeSpan.FromMinutes(Utility.Random(15, 60));
+
+    public override TimeSpan DecayTime => _decayTime;
 
     public override int DefaultGumpID => 0x44;
 

Projects/UOContent/Items/TreasureChests/TreasureChestLevel2.cs

@@ -69,7 +69,10 @@ public partial class TreasureChestLevel2 : LockableContainer
 
     public override bool IsDecoContainer => false;
 
-    public override TimeSpan DecayTime => TimeSpan.FromMinutes(Utility.Random(15, 60));
+    // Rolled once: DecayScheduler reads ScheduledDecayTime repeatedly and needs a stable value.
+    private readonly TimeSpan _decayTime = TimeSpan.FromMinutes(Utility.Random(15, 60));
+
+    public override TimeSpan DecayTime => _decayTime;
 
     public override int DefaultGumpID => 0x42;
 

Projects/UOContent/Items/TreasureChests/TreasureChestLevel3.cs

@@ -114,7 +114,10 @@ public partial class TreasureChestLevel3 : LockableContainer
 
     public override bool IsDecoContainer => false;
 
-    public override TimeSpan DecayTime => TimeSpan.FromMinutes(Utility.Random(15, 60));
+    // Rolled once: DecayScheduler reads ScheduledDecayTime repeatedly and needs a stable value.
+    private readonly TimeSpan _decayTime = TimeSpan.FromMinutes(Utility.Random(15, 60));
+
+    public override TimeSpan DecayTime => _decayTime;
 
     public override int DefaultGumpID => 0x42;
 

Projects/UOContent/Items/TreasureChests/TreasureChestLevel4.cs

@@ -117,7 +117,10 @@ public partial class TreasureChestLevel4 : LockableContainer
 
     public override bool IsDecoContainer => false;
 
-    public override TimeSpan DecayTime => TimeSpan.FromMinutes(Utility.Random(15, 60));
+    // Rolled once: DecayScheduler reads ScheduledDecayTime repeatedly and needs a stable value.
+    private readonly TimeSpan _decayTime = TimeSpan.FromMinutes(Utility.Random(15, 60));
+
+    public override TimeSpan DecayTime => _decayTime;
 
     public override int DefaultGumpID => 0x42;