fix: stop items from insta-decaying when decay eligibility is restored without a move (#2583)

## Summary

A GM flipping `Movable` back on for a long-frozen item made it vanish within one scheduler tick. The setter registered the item with a deadline computed from its stale `LastMoved`, so `ProcessActiveQueue` deleted it almost immediately. The pre-#2311 save-time sweep had the same semantics, just hidden behind the save cadence. The same failure existed for `Visible` and `Spawner` transitions.

`LastMoved` is deliberately left meaning actual movement — it feeds vendor inventory expiry and house moving-crate checks — so the fix does not rewrite it for state changes.

## Changes

- **`DecayResetTime`** (CompactInfo-backed): the decay countdown runs from the later of `LastMoved` and this stamp. `RestartDecay()` stamps it only when the item can decay and the stamp extends the current deadline, so hot paths with a fresh `LastMoved` allocate nothing.
- **`Movable`/`Visible`/`Spawner` setters** call `RestartDecay()` instead of registering a stale deadline.
- **Region-refusal retry** in `DecayScheduler` uses `RestartDecay()` instead of rewriting `LastMoved`.
- **Persistence**: the stamp survives save/load as a `WriteDeltaTime` delta under `SaveFlag.DecayReset` (to become `WriteAnchoredTime` once the save-time anchor is ported) (Item serialization v10), so a restart mid-window no longer deletes the item.
- **`LastMoved` setter** drops a superseded stamp so the `CompactInfo` can collapse instead of being held (~40 bytes) forever.
- **Raw `Map` setter** now counts as a move for parentless items: it stamps `LastMoved` and updates decay registration, closing the gap where an item moved out of `Map.Internal` via the setter never decayed.
- **`LiftItemDupe`**: the remainder of a partially lifted *ground* stack was placed via raw `Location`/`Map` assignments and never enrolled for decay (lingering-trash leak since #2311) — now enrolled via the Map setter. Parented remainders get their map from `AddItem` (parent first, then map), so container splits never transit the scheduler.
This commit is contained in:
Kamron Batman 2026-08-22 12:56:00 -07:00 committed by GitHub
parent fd27b7a3c9
commit 971d7b6a77
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 397 additions and 10 deletions

View file

@ -208,6 +208,272 @@ public class DecayRegistrationTests
item.Delete(); item.Delete();
} }
// Unfreezing an item with a stale LastMoved must grant a fresh decay window,
// not delete it on the next tick.
[Fact]
public void StaleImmovableItemMadeMovable_GetsAFreshDecayWindow()
{
var start = Core._now;
try
{
var item = new Item(0x1234);
item.MoveToWorld(new Point3D(107, 100, 0), Map.Felucca);
item.Movable = false;
Assert.False(DecayScheduler.IsRegistered(item), "A frozen item must not be tracked for decay.");
Core._now = start + TimeSpan.FromDays(30);
var flipped = Core._now;
item.Movable = true;
Assert.True(DecayScheduler.IsRegistered(item), "An unfrozen item must be tracked for decay.");
AdvanceDecay(flipped, item.DecayTime - TimeSpan.FromMinutes(2), item);
Assert.False(item.Deleted, "An unfrozen item must get a full decay window, not vanish immediately.");
AdvanceDecay(Core._now, TimeSpan.FromMinutes(4), item);
Assert.True(item.Deleted, "An unfrozen item must still decay once the fresh window elapses.");
}
finally
{
Core._now = start;
}
}
// Same transition through the Visible setter: unhiding a long-hidden item.
[Fact]
public void StaleHiddenItemMadeVisible_GetsAFreshDecayWindow()
{
var start = Core._now;
try
{
var item = new Item(0x1234);
item.MoveToWorld(new Point3D(109, 100, 0), Map.Felucca);
item.Visible = false;
Assert.False(DecayScheduler.IsRegistered(item), "A hidden item must not be tracked for decay.");
Core._now = start + TimeSpan.FromDays(30);
var flipped = Core._now;
item.Visible = true;
Assert.True(DecayScheduler.IsRegistered(item), "An unhidden item must be tracked for decay.");
AdvanceDecay(flipped, item.DecayTime - TimeSpan.FromMinutes(2), item);
Assert.False(item.Deleted, "An unhidden item must get a full decay window, not vanish immediately.");
AdvanceDecay(Core._now, TimeSpan.FromMinutes(4), item);
Assert.True(item.Deleted, "An unhidden item must still decay once the fresh window elapses.");
}
finally
{
Core._now = start;
}
}
// A refusal restarts the countdown without rewriting LastMoved.
[Fact]
public void RefusedDecay_DoesNotRewriteLastMoved()
{
var start = Core._now;
try
{
var item = new RefusesDecayItem();
item.MoveToWorld(new Point3D(110, 100, 0), Map.Felucca);
var lastMoved = item.LastMoved;
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.");
Assert.Equal(lastMoved, item.LastMoved);
item.Delete();
}
finally
{
Core._now = start;
}
}
// The fresh window must survive a save/load cycle, or a restart mid-window deletes the item.
[Fact]
public void FreshDecayWindow_SurvivesSerialization()
{
var start = Core._now;
try
{
var item = new Item(0x1234);
item.MoveToWorld(new Point3D(111, 100, 0), Map.Felucca);
item.Movable = false;
Core._now = start + TimeSpan.FromDays(30);
item.Movable = true;
var expected = item.ScheduledDecayTime;
var writer = new BufferWriter(new byte[512], true);
item.Serialize(writer);
var copy = new Item(item.Serial);
copy.Deserialize(new BufferReader(writer.Buffer));
// The stamp is stored as a delta, so it ages only by the real time between
// write and read - milliseconds here, the downtime in production.
Assert.True(
(copy.ScheduledDecayTime - expected).Duration() <= TimeSpan.FromSeconds(5),
"The restarted decay window must survive a save/load cycle."
);
item.Delete();
copy.Delete();
}
finally
{
Core._now = start;
}
}
// A real move supersedes the reset stamp; it must be dropped so the CompactInfo can collapse.
[Fact]
public void MovingAnItem_ClearsASupersededDecayResetStamp()
{
var start = Core._now;
try
{
var item = new Item(0x1234);
item.MoveToWorld(new Point3D(112, 100, 0), Map.Felucca);
item.Movable = false;
Core._now = start + TimeSpan.FromDays(30);
item.Movable = true;
Assert.NotEqual(default, item.DecayResetTime);
Core._now += TimeSpan.FromMinutes(1);
item.MoveToWorld(new Point3D(113, 100, 0), Map.Felucca);
Assert.Equal(default, item.DecayResetTime);
Assert.Equal(item.LastMoved + item.DecayTime, item.ScheduledDecayTime);
Assert.True(DecayScheduler.IsRegistered(item));
item.Delete();
}
finally
{
Core._now = start;
}
}
// Losing decay eligibility makes the stamp meaningless; it must be dropped so the
// CompactInfo is not held for as long as the item stays ineligible.
[Fact]
public void ItemBecomingIneligible_DropsTheDecayResetStamp()
{
var start = Core._now;
try
{
var item = new Item(0x1234);
item.MoveToWorld(new Point3D(115, 100, 0), Map.Felucca);
item.Movable = false;
Core._now = start + TimeSpan.FromDays(30);
item.Movable = true;
Assert.NotEqual(default, item.DecayResetTime);
item.Movable = false;
Assert.Equal(default, item.DecayResetTime);
item.Delete();
}
finally
{
Core._now = start;
}
}
// Moving a stamped item into a container programmatically (no drop, no SetLastMoved)
// must also drop the stamp.
[Fact]
public void StampedItemAddedToContainer_DropsTheDecayResetStamp()
{
var start = Core._now;
try
{
var pack = new Container(0xE75);
pack.MoveToWorld(new Point3D(116, 100, 0), Map.Felucca);
var item = new Item(0x1234);
item.MoveToWorld(new Point3D(117, 100, 0), Map.Felucca);
item.Movable = false;
Core._now = start + TimeSpan.FromDays(30);
item.Movable = true;
Assert.NotEqual(default, item.DecayResetTime);
pack.AddItem(item);
Assert.Equal(default, item.DecayResetTime);
pack.Delete();
}
finally
{
Core._now = start;
}
}
// A raw Map assignment (e.g. a GM changing Map through props) is a move: it must
// enroll an untracked item for decay.
[Fact]
public void ItemMovedToRealMapViaMapSetter_IsRegisteredForDecay()
{
var item = new Item(0x1234);
Assert.False(DecayScheduler.IsRegistered(item));
item.Map = Map.Felucca;
Assert.True(item.CanDecay());
Assert.True(DecayScheduler.IsRegistered(item), "Item placed on a map via the Map setter must be tracked.");
item.Delete();
}
// LiftItemDupe places the remainder of a partially lifted ground stack via raw
// Location/Map assignments, with no MoveToWorld fallback: it must still be tracked.
[Fact]
public void PartialLiftOfGroundStack_LeavesRemainderRegisteredForDecay()
{
var stack = new Item(0x1234) { Stackable = true, Amount = 10 };
stack.MoveToWorld(new Point3D(114, 100, 0), Map.Felucca);
var remainder = Mobile.LiftItemDupe(stack, 3);
Assert.NotNull(remainder);
Assert.Equal(7, remainder.Amount);
Assert.Null(remainder.Parent);
Assert.Equal(Map.Felucca, remainder.Map);
Assert.True(
DecayScheduler.IsRegistered(remainder),
"The remainder of a partially lifted ground stack must be tracked for decay."
);
stack.Delete();
remainder.Delete();
}
// Dropping into a container must untrack; taking it back out to the ground must re-track. // Dropping into a container must untrack; taking it back out to the ground must re-track.
[Fact] [Fact]
public void ItemMovedIntoContainerThenBackToGround_IsRegisteredForDecay() public void ItemMovedIntoContainerThenBackToGround_IsRegisteredForDecay()

View file

@ -311,7 +311,7 @@ public class DecayScheduler : Timer
if (timeUntilDecay > _bucketInterval) if (timeUntilDecay > _bucketInterval)
{ {
// Item was moved (SetLastMoved called) - re-bucket or move to overflow // Deadline was pushed out (SetLastMoved/RestartDecay) - re-bucket or move to overflow
if (timeUntilDecay > _totalBucketSpan) if (timeUntilDecay > _totalBucketSpan)
{ {
// Extended beyond total span - move to overflow // Extended beyond total span - move to overflow
@ -429,7 +429,7 @@ public class DecayScheduler : Timer
{ {
// Refused by the region. Restart the clock rather than dropping the item, which has // 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. // already left the queue; re-registering as-is would spin on a due time in the past.
item.SetLastMoved(); item.RestartDecay();
} }
} }
} }

View file

@ -335,7 +335,25 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
[CommandProperty(AccessLevel.GameMaster)] [CommandProperty(AccessLevel.GameMaster)]
public virtual bool Decays => Movable && Visible && Spawner == null; public virtual bool Decays => Movable && Visible && Spawner == null;
public DateTime LastMoved { get; set; } private DateTime _lastMoved;
public DateTime LastMoved
{
get => _lastMoved;
set
{
_lastMoved = value;
// A move at or past the reset stamp supersedes it; drop it so the CompactInfo can collapse.
var info = LookupCompactInfo();
if (info != null && info.m_DecayReset != default && info.m_DecayReset <= value)
{
info.m_DecayReset = default;
VerifyCompactInfo();
}
}
}
[CommandProperty(AccessLevel.GameMaster)] [CommandProperty(AccessLevel.GameMaster)]
public bool Stackable public bool Stackable
@ -373,7 +391,7 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
} }
Delta(ItemDelta.Update); Delta(ItemDelta.Update);
UpdateDecayRegistration(); RestartDecay();
} }
} }
} }
@ -389,7 +407,7 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
SetFlag(ImplFlag.Movable, value); SetFlag(ImplFlag.Movable, value);
Delta(ItemDelta.Update); Delta(ItemDelta.Update);
UpdateDecayRegistration(); RestartDecay();
} }
} }
} }
@ -845,7 +863,7 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
public virtual void Serialize(IGenericWriter writer) public virtual void Serialize(IGenericWriter writer)
{ {
writer.Write(9); // version writer.Write(10); // version
var flags = SaveFlag.None; var flags = SaveFlag.None;
@ -955,6 +973,11 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
{ {
flags |= SaveFlag.SavedFlags; flags |= SaveFlag.SavedFlags;
} }
if (info.m_DecayReset > LastMoved)
{
flags |= SaveFlag.DecayReset;
}
} }
if (info == null || info.m_Weight < 0) if (info == null || info.m_Weight < 0)
@ -1001,6 +1024,12 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
writer.WriteEncodedInt((int)Math.Clamp(minutes, int.MinValue, int.MaxValue)); writer.WriteEncodedInt((int)Math.Clamp(minutes, int.MinValue, int.MaxValue));
/* end */ /* end */
if (GetSaveFlag(flags, SaveFlag.DecayReset))
{
//TODO Use WriteAnchoredTime once the save-time anchor is ported
writer.WriteDeltaTime(info.m_DecayReset);
}
if (GetSaveFlag(flags, SaveFlag.Direction)) if (GetSaveFlag(flags, SaveFlag.Direction))
{ {
writer.Write((byte)m_Direction); writer.Write((byte)m_Direction);
@ -1318,6 +1347,12 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
OnMapChange(); OnMapChange();
if (m_Parent == null)
{
// A map change is a move; nothing else updates decay registration for a raw Map change.
SetLastMoved();
}
if (old == null || old == Map.Internal) if (old == null || old == Map.Internal)
{ {
InvalidateProperties(); InvalidateProperties();
@ -1548,7 +1583,7 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
if (oldValue != value) if (oldValue != value)
{ {
UpdateDecayRegistration(); RestartDecay();
} }
} }
} }
@ -1742,6 +1777,7 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
|| info.m_HeldBy != null || info.m_HeldBy != null
|| info.m_BlessedFor != null || info.m_BlessedFor != null
|| info.m_Spawner != null || info.m_Spawner != null
|| info.m_DecayReset != default
|| info.m_TempFlags != 0 || info.m_TempFlags != 0
|| info.m_SavedFlags != 0 || info.m_SavedFlags != 0
|| info.m_Weight >= 0; || info.m_Weight >= 0;
@ -2326,7 +2362,64 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
public virtual bool OnDecay() => public virtual bool OnDecay() =>
CanDecay() && Region.Find(Location, Map).OnDecay(this); CanDecay() && Region.Find(Location, Map).OnDecay(this);
public DateTime ScheduledDecayTime => LastMoved + DecayTime; public DateTime ScheduledDecayTime
{
get
{
var reset = DecayResetTime;
var lastMoved = LastMoved;
return (reset > lastMoved ? reset : lastMoved) + DecayTime;
}
}
/// <summary>
/// When decay eligibility was last restored without the item moving, e.g. a GM unfreezing it.
/// The decay countdown runs from the later of this and <see cref="LastMoved" />.
/// </summary>
public DateTime DecayResetTime
{
get => LookupCompactInfo()?.m_DecayReset ?? default;
private set
{
if (value == default)
{
var info = LookupCompactInfo();
if (info != null && info.m_DecayReset != default)
{
info.m_DecayReset = default;
VerifyCompactInfo();
}
}
else
{
AcquireCompactInfo().m_DecayReset = value;
}
}
}
/// <summary>
/// Restarts the decay countdown without touching <see cref="LastMoved" />: call when decay
/// eligibility changes state (Movable/Visible/Spawner) or a region refuses a decay, where a
/// stale <see cref="LastMoved" /> would otherwise decay the item on the next tick.
/// Stamps <see cref="DecayResetTime" /> only when that extends the current deadline, then
/// updates the scheduler registration.
/// </summary>
public void RestartDecay()
{
if (CanDecay())
{
var now = Core.Now;
if (ScheduledDecayTime < now + DecayTime)
{
DecayResetTime = now;
}
}
UpdateDecayRegistration();
}
public void UpdateDecayRegistration() public void UpdateDecayRegistration()
{ {
@ -2336,6 +2429,12 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
{ {
DecayScheduler.Register(this); DecayScheduler.Register(this);
} }
else
{
// No countdown to anchor while ineligible; drop the stamp so the CompactInfo
// can collapse. Re-eligibility always re-anchors.
DecayResetTime = default;
}
} }
public void SetLastMoved() public void SetLastMoved()
@ -2673,6 +2772,7 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
switch (version) switch (version)
{ {
case 10:
case 9: case 9:
case 8: case 8:
case 7: case 7:
@ -2698,6 +2798,18 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
} }
} }
if (version >= 10 && GetSaveFlag(flags, SaveFlag.DecayReset))
{
var reset = reader.ReadDeltaTime();
// LastMoved is stored at whole-minute precision; keep the stamp only
// while it still extends the deadline.
if (reset > LastMoved)
{
DecayResetTime = reset;
}
}
if (GetSaveFlag(flags, SaveFlag.Direction)) if (GetSaveFlag(flags, SaveFlag.Direction))
{ {
m_Direction = (Direction)reader.ReadByte(); m_Direction = (Direction)reader.ReadByte();
@ -4361,6 +4473,8 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
public ISpawner m_Spawner; public ISpawner m_Spawner;
public DateTime m_DecayReset;
public int m_TempFlags; public int m_TempFlags;
public double m_Weight = -1; public double m_Weight = -1;
@ -4399,6 +4513,7 @@ public partial class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropert
IntWeight = 0x01000000, IntWeight = 0x01000000,
SavedFlags = 0x02000000, SavedFlags = 0x02000000,
NullWeight = 0x04000000, NullWeight = 0x04000000,
PlayerConstructed = 0x08000000 PlayerConstructed = 0x08000000,
DecayReset = 0x10000000
} }
} }

View file

@ -5250,7 +5250,13 @@ public partial class Mobile : IHued, IComparable<Mobile>, ISpawnable, IObjectPro
item.PlayerConstructed = oldItem.PlayerConstructed; item.PlayerConstructed = oldItem.PlayerConstructed;
item.Amount = oldAmount - amount; item.Amount = oldAmount - amount;
item.Map = oldItem.Map;
// A parented remainder gets its map from AddItem (parent first, then map), keeping the
// split off the decay scheduler; a ground remainder is placed and enrolled here.
if (oldItem.Parent == null)
{
item.Map = oldItem.Map;
}
oldItem.OnAfterDuped(item); oldItem.OnAfterDuped(item);