fix: Adds AfterSerialize support. Removes BeforeSerialize support. (#1208)

### Changes
* Implements an AfterSerialize method that is executed synchronously.
* Removes `BeforeSerialize` support since it was dangerous in its current implementation.
* Moves PlayerMobile kill/virtual decay to AfterSerialize.
* Adds kill decay to after Deserialize.

Projects/Server/Guild.cs

@@ -39,6 +39,10 @@ public abstract class BaseGuild : ISerializable
     public abstract string Name { get; set; }
     public abstract GuildType Type { get; set; }
     public abstract bool Disbanded { get; }
+
+    public abstract bool ShouldExecuteAfterSerialize { get; }
+    public abstract void AfterSerialize();
+
     public abstract void Delete();
 
     public bool Deleted => Disbanded;
@@ -58,7 +62,6 @@ public abstract class BaseGuild : ISerializable
 
     public int TypeRef { get; private set; }
 
-    public abstract void BeforeSerialize();
     public abstract void Serialize(IGenericWriter writer);
 
     public abstract void Deserialize(IGenericReader reader);

Projects/Server/IEntity.cs

@@ -103,10 +103,6 @@ public class Entity : IEntity
         && p.Y >= Location.m_Y - range
         && p.Y <= Location.m_Y + range;
 
-    public void BeforeSerialize()
-    {
-    }
-
     public void Deserialize(IGenericReader reader)
     {
         // Should not actually be saved
@@ -116,4 +112,10 @@ public class Entity : IEntity
     public void Serialize(IGenericWriter writer)
     {
     }
+
+    public bool ShouldExecuteAfterSerialize => false;
+
+    public void AfterSerialize()
+    {
+    }
 }

Projects/Server/Items/Item.cs

@@ -1041,6 +1041,12 @@ public class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropertyListEnt
         }
     }
 
+    public virtual bool ShouldExecuteAfterSerialize => false;
+
+    public virtual void AfterSerialize()
+    {
+    }
+
     public void MoveToWorld(WorldLocation worldLocation)
     {
         MoveToWorld(worldLocation.Location, worldLocation.Map);
@@ -2551,10 +2557,6 @@ public class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropertyListEnt
         }
     }
 
-    public virtual void BeforeSerialize()
-    {
-    }
-
     public virtual void Deserialize(IGenericReader reader)
     {
         var version = reader.ReadInt();

Projects/Server/Main.cs

@@ -20,6 +20,7 @@ using System.IO;
 using System.Linq;
 using System.Reflection;
 using System.Runtime;
+using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using System.Text;
 using System.Text.Json;
@@ -123,7 +124,9 @@ public static class Core
     [ThreadStatic]
     private static long _tickCount;
 
-    [ThreadStatic]
+    // Don't access this from other threads than the game thread.
+    // Persistence accesses this via AfterSerialize or Serialize in other threads, but the value is set and won't change
+    // since the game loop is frozen at that moment.
     private static DateTime _now;
 
     // For Unix Stopwatch.Frequency is normalized to 1ns
@@ -153,12 +156,17 @@ public static class Core
 
     public static DateTime Now
     {
-        get => _now == DateTime.MinValue ? DateTime.UtcNow : _now;
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        set => _now = value;
+        get
+        {
+            // See notes above for _now and why this is a volatile variable.
+            var now = _now;
+            return now == DateTime.MinValue ? DateTime.UtcNow : now;
+        }
     }
 
     private static long _cycleIndex = 1;
-    private static float[] _cyclesPerSecond = new float[100];
+    private static float[] _cyclesPerSecond = new float[128];
 
     public static float CyclesPerSecond => _cyclesPerSecond[(_cycleIndex - 1) % _cyclesPerSecond.Length];
 

Projects/Server/Mobiles/Mobile.cs

@@ -2387,6 +2387,12 @@ public class Mobile : IHued, IComparable<Mobile>, ISpawnable, IObjectPropertyLis
         writer.Write((byte)m_IntLock);
     }
 
+    public virtual bool ShouldExecuteAfterSerialize => false;
+
+    public virtual void AfterSerialize()
+    {
+    }
+
     public bool Deleted { get; private set; }
 
     public virtual void Delete()
@@ -6041,10 +6047,6 @@ public class Mobile : IHued, IComparable<Mobile>, ISpawnable, IObjectPropertyLis
     {
     }
 
-    public virtual void BeforeSerialize()
-    {
-    }
-
     public virtual void Deserialize(IGenericReader reader)
     {
         var version = reader.ReadInt();

Projects/Server/Serialization/GenericPersistence.cs

@@ -26,6 +26,7 @@ public static class GenericPersistence
         string name,
         Action<IGenericWriter> serializer,
         Action<IGenericReader> deserializer,
+        Action afterSerialize = null,
         int priority = Persistence.DefaultPriority
     )
     {
@@ -49,6 +50,6 @@ public static class GenericPersistence
         void Deserialize(string savePath, Dictionary<ulong, string> typesDb) =>
             AdhocPersistence.Deserialize(Path.Combine(savePath, name, $"{name}.bin"), deserializer);
 
-        Persistence.Register(name, Serialize, WriteSnapshot, Deserialize, priority);
+        Persistence.Register(name, Serialize, WriteSnapshot, Deserialize, afterSerialize, priority);
     }
 }

Projects/Server/Serialization/ISerializable.cs

@@ -31,13 +31,17 @@ public interface ISerializable
 
     Serial Serial { get; }
 
-    // Executed on every entity, before it's serialized.
-    // For example, this is used to clean up weak references and mark them dirty.
-    void BeforeSerialize();
     void Deserialize(IGenericReader reader);
     void Serialize(IGenericWriter writer);
-    void Delete();
+
+    // Determines if AfterSerialize should execute. This is checked on a worker thread.
+    bool ShouldExecuteAfterSerialize { get; }
+
+    // Executes after serialization if ShouldExecuteAfterSerialize is true. This is run on the game thread synchronously.
+    void AfterSerialize();
+
     bool Deleted { get; }
+    void Delete();
 
     public void InitializeSaveBuffer(byte[] buffer, ConcurrentQueue<Type> types)
     {
@@ -56,7 +60,12 @@ public interface ISerializable
     {
         SaveBuffer ??= new BufferWriter(true, types);
 
-        BeforeSerialize();
+        // Queue for post serialization if this entity has it enabled
+        // This will run AfterSerialize in the main game thread after the world is done saving
+        if (ShouldExecuteAfterSerialize)
+        {
+            World.EnqueueAfterSerialization(this);
+        }
 
         // Clean, don't bother serializing
         if (SavePosition > -1)

Projects/Server/Serialization/Persistence.cs

@@ -32,6 +32,7 @@ public static class Persistence
         Action serializer,
         Action<string> snapshotWriter,
         Action<string, Dictionary<ulong, string>> deserializer,
+        Action afterSerialize = null,
         int priority = DefaultPriority
     )
     {
@@ -41,6 +42,7 @@ public static class Persistence
                 Name = name,
                 Priority = priority,
                 Serialize = serializer,
+                AfterSerialize = afterSerialize,
                 WriteSnapshot = snapshotWriter,
                 Deserialize = deserializer
             }
@@ -89,6 +91,12 @@ public static class Persistence
     public static void Serialize()
     {
         Parallel.ForEach(_registry, entry => entry.Serialize());
+
+        // Synchronously run the AfterSerialize on the main game thread
+        foreach (var entry in _registry)
+        {
+            entry.AfterSerialize?.Invoke();
+        }
     }
 
     public static void WriteSnapshot(string path, ConcurrentQueue<Type> types)
@@ -130,7 +138,10 @@ public static class Persistence
     {
         public string Name { get; init; }
         public int Priority { get; init; }
-        public Action Serialize { get; init; } // Serializing to memory buffers
+
+        // Serializes to memory buffers and run in parallel
+        public Action Serialize { get; init; }
+        public Action AfterSerialize { get; init; }
         public Action<string> WriteSnapshot { get; init; }
         public Action<string, Dictionary<ulong, string>> Deserialize { get; init; }
     }

Projects/Server/World/World.cs

@@ -44,6 +44,7 @@ public static class World
     private static Dictionary<Serial, IEntity> _pendingAdd = new();
     private static Dictionary<Serial, IEntity> _pendingDelete = new();
     private static ConcurrentQueue<Item> _decayQueue = new();
+    private static ConcurrentQueue<ISerializable> _afterSerializeEntities = new();
 
     private static string _tempSavePath; // Path to the temporary folder for the save
     private static bool _enableSaveStats;
@@ -147,7 +148,7 @@ public static class World
         _enableSaveStats = ServerConfiguration.GetOrUpdateSetting("world.enableSaveStats", false);
 
         // Mobiles & Items
-        Persistence.Register("Mobiles & Items", SaveEntities, WriteEntities, LoadEntities, 1);
+        Persistence.Register("Mobiles & Items", SaveEntities, WriteEntities, LoadEntities, AfterSerialize, 1);
     }
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
@@ -168,6 +169,9 @@ public static class World
         _decayQueue.Enqueue(item);
     }
 
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public static void EnqueueAfterSerialization(ISerializable entity) => _afterSerializeEntities.Enqueue(entity);
+
     public static void Broadcast(int hue, bool ascii, string text)
     {
         var length = OutgoingMessagePackets.GetMaxMessageLength(text);
@@ -495,7 +499,15 @@ public static class World
      */
     public static ConcurrentQueue<Type> SerializedTypes { get; } = new();
 
-    internal static void SaveEntities()
+    private static void AfterSerialize()
+    {
+        while (_afterSerializeEntities.TryDequeue(out var entity))
+        {
+            entity.AfterSerialize();
+        }
+    }
+
+    private static void SaveEntities()
     {
         _serializationStart = DateTime.UtcNow;
         EntityPersistence.SaveEntities(Items.Values, SaveEntity);

Projects/UOContent/Accounting/Account.cs

@@ -295,10 +295,6 @@ namespace Server.Accounting
 
         public Serial Serial { get; set; }
 
-        public void BeforeSerialize()
-        {
-        }
-
         [AfterDeserialization]
         private void AfterDeserialization()
         {
@@ -337,6 +333,12 @@ namespace Server.Accounting
             }
         }
 
+        public bool ShouldExecuteAfterSerialize => false;
+
+        public void AfterSerialize()
+        {
+        }
+
         /// <summary>
         ///     Deletes the account, all characters of the account, and all houses of those characters
         /// </summary>

Projects/UOContent/Misc/Guild.cs

@@ -1148,10 +1148,6 @@ namespace Server.Guilds
             list.TrimExcess();
         }
 
-        public override void BeforeSerialize()
-        {
-        }
-
         public override void Serialize(IGenericWriter writer)
         {
             if (LastFealty + TimeSpan.FromDays(1.0) < Core.Now)
@@ -1231,6 +1227,12 @@ namespace Server.Guilds
             writer.Write(Website);
         }
 
+        public override bool ShouldExecuteAfterSerialize => false;
+
+        public override void AfterSerialize()
+        {
+        }
+
         public override void Delete()
         {
             World.RemoveGuild(this);

Projects/UOContent/Mobiles/PlayerMobile.cs

@@ -3402,6 +3402,17 @@ namespace Server.Mobiles
             writer.Write(GameTime);
         }
 
+        // Do we need to run an after serialize?
+        public override bool ShouldExecuteAfterSerialize => ShouldKillDecay() || ShouldAtrophy();
+
+        public override void AfterSerialize()
+        {
+            base.AfterSerialize();
+
+            CheckKillDecay();
+            CheckAtrophies();
+        }
+
         public bool ShouldAtrophy()
         {
             var sacrifice = SacrificeVirtue.ShouldAtrophy(this);

Projects/UOContent/Multis/Boats/BaseBoat.cs

@@ -29,36 +29,36 @@ namespace Server.Multis
             Decaying
         }
 
-        private static readonly Rectangle2D[] m_BritWrap =
+        private static Rectangle2D[] m_BritWrap =
             { new(16, 16, 5120 - 32, 4096 - 32), new(5136, 2320, 992, 1760) };
 
-        private static readonly Rectangle2D[] m_IlshWrap = { new(16, 16, 2304 - 32, 1600 - 32) };
+        private static Rectangle2D[] m_IlshWrap = { new(16, 16, 2304 - 32, 1600 - 32) };
-        private static readonly Rectangle2D[] m_TokunoWrap = { new(16, 16, 1448 - 32, 1448 - 32) };
+        private static Rectangle2D[] m_TokunoWrap = { new(16, 16, 1448 - 32, 1448 - 32) };
 
-        private static readonly TimeSpan BoatDecayDelay = TimeSpan.FromDays(9.0);
+        private static TimeSpan BoatDecayDelay = TimeSpan.FromDays(9.0);
 
-        private static readonly TimeSpan SlowInterval = TimeSpan.FromSeconds(NewBoatMovement ? 0.50 : 0.75);
+        private static TimeSpan SlowInterval = TimeSpan.FromSeconds(NewBoatMovement ? 0.50 : 0.75);
-        private static readonly TimeSpan FastInterval = TimeSpan.FromSeconds(NewBoatMovement ? 0.25 : 0.75);
+        private static TimeSpan FastInterval = TimeSpan.FromSeconds(NewBoatMovement ? 0.25 : 0.75);
 
-        private static readonly int SlowSpeed = 1;
+        private const int SlowSpeed = 1;
-        private static readonly int FastSpeed = NewBoatMovement ? 1 : 3;
+        private static int FastSpeed = NewBoatMovement ? 1 : 3;
 
-        private static readonly TimeSpan SlowDriftInterval = TimeSpan.FromSeconds(NewBoatMovement ? 0.50 : 1.50);
+        private static TimeSpan SlowDriftInterval = TimeSpan.FromSeconds(NewBoatMovement ? 0.50 : 1.50);
-        private static readonly TimeSpan FastDriftInterval = TimeSpan.FromSeconds(NewBoatMovement ? 0.25 : 0.75);
+        private static TimeSpan FastDriftInterval = TimeSpan.FromSeconds(NewBoatMovement ? 0.25 : 0.75);
 
-        private static readonly int SlowDriftSpeed = 1;
+        private const int SlowDriftSpeed = 1;
-        private static readonly int FastDriftSpeed = 1;
+        private const int FastDriftSpeed = 1;
 
-        private static readonly Direction Forward = Direction.North;
+        private const Direction Forward = Direction.North;
-        private static readonly Direction ForwardLeft = Direction.Up;
+        private const Direction ForwardLeft = Direction.Up;
-        private static readonly Direction ForwardRight = Direction.Right;
+        private const Direction ForwardRight = Direction.Right;
-        private static readonly Direction Backward = Direction.South;
+        private const Direction Backward = Direction.South;
-        private static readonly Direction BackwardLeft = Direction.Left;
+        private const Direction BackwardLeft = Direction.Left;
-        private static readonly Direction BackwardRight = Direction.Down;
+        private const Direction BackwardRight = Direction.Down;
-        private static readonly Direction Left = Direction.West;
+        private const Direction Left = Direction.West;
-        private static readonly Direction Right = Direction.East;
+        private const Direction Right = Direction.East;
-        private static Direction Port = Left;
+        private const Direction Port = Left;
-        private static Direction Starboard = Right;
+        private const Direction Starboard = Right;
 
         private int m_ClientSpeed;
 
@@ -296,6 +296,14 @@ namespace Server.Multis
             }
         }
 
+        public override bool ShouldExecuteAfterSerialize => !m_Decaying && CheckDecay();
+
+        public override void AfterSerialize()
+        {
+            base.AfterSerialize();
+            CheckDecay();
+        }
+
         public override void Serialize(IGenericWriter writer)
         {
             base.Serialize(writer);
@@ -316,8 +324,6 @@ namespace Server.Multis
             writer.Write(Hold);
             writer.Write(Anchored);
             writer.Write(m_ShipName);
-
-            CheckDecay();
         }
 
         public override void Deserialize(IGenericReader reader)
@@ -527,6 +533,8 @@ namespace Server.Multis
             TillerMan?.InvalidateProperties();
         }
 
+        private bool ShouldDecay => !IsMoving && Core.Now >= m_DecayTime;
+
         public bool CheckDecay()
         {
             if (m_Decaying)
@@ -534,12 +542,10 @@ namespace Server.Multis
                 return true;
             }
 
-            if (!IsMoving && Core.Now >= m_DecayTime)
+            if (ShouldDecay)
             {
                 new DecayTimer(this).Start();
-
                 m_Decaying = true;
-
                 return true;
             }