SyncField buffering: expanded docs, simplified code flow, and improved UX

Source/Client/Syncing/Game/SyncFields.cs

@@ -152,7 +152,7 @@ public static void Init()
                 nameof(Bill_Production.targetCount),
                 nameof(Bill_Production.pauseWhenSatisfied),
                 nameof(Bill_Production.unpauseWhenYouHave)
-            );
+            ).SetBufferChanges();
 
             SyncBillIncludeCriteria = Sync.Fields(
                 typeof(Bill_Production),

Source/Client/Syncing/Handler/SyncField.cs

@@ -141,10 +141,19 @@ public ISyncField PostApply(Action<object, object> action)
             return this;
         }
 
+        /// Throttles network updates for this field to reduce network chatter.
+        /// An update is sent only after the field has remained unchanged for 200ms.
+        /// If changes continue occurring within that window, the timer resets, and only
+        /// the latest value is eventually sent. All intermediate updates are discarded.
+        ///
+        /// The UI reflects the latest value immediately, without waiting for server
+        /// confirmation — avoiding rollback, and reapply behavior on server response.
+        ///
+        /// Ideal for rapidly changing fields (e.g., sliders) where sending every
+        /// intermediate value would be inefficient and unnecessary.
         public ISyncField SetBufferChanges()
         {
             SyncFieldUtil.bufferedChanges[this] = new();
-            Sync.bufferedFields.Add(this);
             bufferChanges = true;
             return this;
         }

Source/Client/Syncing/Sync.cs

@@ -13,7 +13,6 @@ namespace Multiplayer.Client
     public static class Sync
     {
         public static List<SyncHandler> handlers = new();
-        public static List<SyncField> bufferedFields = new();
 
         // Internal maps for Harmony patches
         public static Dictionary<MethodBase, int> methodBaseToInternalId = new();

Source/Client/Syncing/SyncFieldUtil.cs

@@ -12,6 +12,11 @@ public static class SyncFieldUtil
         public static Dictionary<SyncField, Dictionary<BufferTarget, BufferData>> bufferedChanges = new();
         private static Stack<FieldData?> watchedStack = new();
 
+        public static void StackPush(SyncField field, object target, object value, object index)
+        {
+            watchedStack.Push(new FieldData(field, target, value, index));
+        }
+
         public static void FieldWatchPrefix()
         {
             if (Multiplayer.Client == null) return;
@@ -23,99 +28,102 @@ public static void FieldWatchPostfix()
         {
             if (Multiplayer.Client == null) return;
 
-            while (watchedStack.Count > 0)
+            // Iterate until we hit the marker (`null`).
+            while (watchedStack.TryPop(out var dataOrNull) && dataOrNull is { } data)
             {
-                var dataOrNull = watchedStack.Pop();
-
-                if (dataOrNull is not { } data)
-                    break; // The marker
-
                 SyncField handler = data.handler;
-
                 object newValue = MpReflection.GetValue(data.target, handler.memberPath, data.index);
                 bool changed = !ValuesEqual(handler, newValue, data.oldValue);
                 var cache =
-                    handler.bufferChanges && !Multiplayer.IsReplay && !Multiplayer.GhostMode ?
-                        bufferedChanges.GetValueSafe(handler) :
-                        null;
-
-                if (cache != null && cache.TryGetValue(new(data.target, data.index), out BufferData cached))
+                    handler.bufferChanges && !Multiplayer.IsReplay && !Multiplayer.GhostMode
+                        ? bufferedChanges.GetValueSafe(handler)
+                        : null;
+                var bufferTarget = new BufferTarget(data.target, data.index);
+                var cachedData = cache?.GetValueSafe(bufferTarget);
+
+                // Revert the local field's value for simulation purposes.
+                // For unbuffered fields, this rollback is only necessary if the value actually changed.
+                // For buffered fields, however, we always perform the restore, since the value is overwritten
+                // whenever watching it begins — assuming the value was previously changed and thus the buffer was
+                // initialized.
+                // If any change has happened, we record it and either send it immediately (unbuffered field) or queue
+                // it (buffered field). The server will eventually acknowledge the change and send it back, at which
+                // point the field is updated.
+                if (changed || cachedData != null)
                 {
-                    if (changed && cached.sent)
-                        cached.sent = false;
-
-                    cached.toSend = SnapshotValueIfNeeded(handler, newValue);
-                    MpReflection.SetValue(data.target, handler.memberPath, cached.actualValue, data.index);
-                    continue;
+                    var simulationValue = cachedData?.actualValue ?? data.oldValue;
+                    MpReflection.SetValue(data.target, handler.memberPath, simulationValue, data.index);
                 }
 
-                if (!changed) continue;
+                // No changes happened means no syncing needed either - we are done.
+                if (!changed)
+                    continue;
 
-                if (cache != null)
+                // For unbuffered fields, just immediately sync any changes.
+                if (cache == null)
                 {
-                    BufferData bufferData = new BufferData(handler, data.oldValue, SnapshotValueIfNeeded(handler, newValue));
-                    cache[new(data.target, data.index)] = bufferData;
+                    handler.DoSyncCatch(data.target, newValue, data.index);
+                    continue;
                 }
-                else
+
+                // For buffered fields, update the value to be sent.
+                if (cachedData != null)
                 {
-                    handler.DoSyncCatch(data.target, newValue, data.index);
+                    cachedData.sent = false;
+                    cachedData.lastChangedAtMillis = Utils.MillisNow;
+                    cachedData.toSend = SnapshotValueIfNeeded(handler, newValue);
+                    continue;
                 }
 
-                MpReflection.SetValue(data.target, handler.memberPath, data.oldValue, data.index);
+                // The field is buffered but had no prior changes; initialize the cache entry now that a change has occurred.
+                cache[bufferTarget] = new BufferData(handler, data.oldValue, SnapshotValueIfNeeded(handler, newValue));
             }
         }
 
-        public static void StackPush(SyncField field, object target, object value, object index)
+        public static void UpdateSync()
         {
-            watchedStack.Push(new FieldData(field, target, value, index));
+            foreach (var (field, fieldBufferedChanges) in bufferedChanges)
+            {
+                if (field.inGameLoop) continue;
+                fieldBufferedChanges.RemoveAll(SyncPendingAndPruneFinished);
+            }
         }
 
-        public static Func<BufferTarget, BufferData, bool> BufferedChangesPruner(Func<long> timeGetter)
+        private static bool SyncPendingAndPruneFinished(BufferTarget target, BufferData data)
         {
-            return (target, data) =>
+            if (data.IsAlreadySynced(target))
+                return true;
+
+            var millisNow = Utils.MillisNow;
+            if (!data.sent && millisNow - data.lastChangedAtMillis > 200)
             {
-                if (CheckShouldRemove(data.field, target, data))
+                // If syncing fails with an exception don't try to reattempt and just give up.
+                if (data.field.DoSyncCatch(target.target, data.toSend, target.index) is false)
                     return true;
 
-                if (!data.sent && timeGetter() - data.timestamp > 200)
-                {
-                    if (data.field.DoSyncCatch(target.target, data.toSend, target.index) is false)
-                        return true;
-
-                    data.sent = true;
-                    data.timestamp = timeGetter();
-                }
-
-                return false;
-            };
-        }
-
-        private static Func<BufferTarget, BufferData, bool> bufferPredicate =
-            BufferedChangesPruner(() => Utils.MillisNow);
-
-        public static void UpdateSync()
-        {
-            foreach (SyncField f in Sync.bufferedFields)
-            {
-                if (f.inGameLoop) continue;
-                bufferedChanges[f].RemoveAll(bufferPredicate);
+                data.sent = true;
             }
+
+            return false;
         }
 
-        public static bool CheckShouldRemove(SyncField field, BufferTarget target, BufferData data)
+        private static bool IsAlreadySynced(this BufferData data, BufferTarget target)
         {
-            if (data.sent && ValuesEqual(field, data.toSend, data.actualValue))
-                return true;
+            var field = data.field;
+            if (data.sent && ValuesEqual(field, data.toSend, data.actualValue)) return true;
 
             object currentValue = target.target.GetPropertyOrField(field.memberPath, target.index);
 
-            if (!ValuesEqual(field, currentValue, data.actualValue))
-            {
-                if (data.sent)
-                    return true;
-                data.actualValue = SnapshotValueIfNeeded(field, currentValue);
-            }
+            // Data hasn't been sent yet, or we are waiting for the server to acknowledge it and send it back.
+            if (ValuesEqual(field, currentValue, data.actualValue)) return false;
+
+            // The last seen value differs from the current field value — likely because a sync command from the server
+            // has overwritten it (possibly even with the desired value). If we've already sent our update, we assume it's fine;
+            // once the server processes it, it will likely acknowledge and send back the update via another sync command,
+            // restoring the field to the intended value.
+            if (data.sent) return true;
 
+            data.actualValue = SnapshotValueIfNeeded(field, currentValue);
             return false;
         }
 
@@ -173,6 +181,10 @@ public readonly struct FieldData
     {
         public readonly SyncField handler;
         public readonly object target;
+        /// Reflects the value the field had before the most recent GUI update.
+        /// For unbuffered fields, this is also the current simulation value.
+        /// For buffered fields, which may modify the field across multiple `Watch` calls,
+        /// this represents the value at the start of the latest `Watch` invocation.
         public readonly object oldValue;
         public readonly object index;
 
@@ -207,19 +219,15 @@ public override int GetHashCode()
         }
     }
 
-    public class BufferData
+    public class BufferData(SyncField field, object actualValue, object toSend)
     {
-        public SyncField field;
-        public object actualValue;
-        public object toSend;
-        public long timestamp;
+        public SyncField field = field;
+        /// This is the real field's value. If this were an unbuffered field, it'd be equivalent to `FieldData.oldValue`,
+        /// however for buffered fields `oldValue` reflects the value prior to the last GUI update. Use this field to
+        /// access the original value before any user interaction occurred.
+        public object actualValue = actualValue;
+        public object toSend = toSend;
+        public long lastChangedAtMillis = Utils.MillisNow;
         public bool sent;
-
-        public BufferData(SyncField field, object actualValue, object toSend)
-        {
-            this.field = field;
-            this.actualValue = actualValue;
-            this.toSend = toSend;
-        }
     }
 }

Source/Client/Util/CollectionExtensions.cs

@@ -53,8 +53,6 @@ public static T RemoveFirst<T>(this List<T> list)
         public static int RemoveAll<TKey, TValue>(this Dictionary<TKey, TValue> dictionary, Func<TKey, TValue, bool> predicate)
         {
             List<TKey> list = null;
-            int result;
-
             try
             {
                 foreach (var (key, value) in dictionary)
@@ -66,21 +64,12 @@ public static int RemoveAll<TKey, TValue>(this Dictionary<TKey, TValue> dictiona
                     }
                 }
 
-                if (list != null)
+                if (list == null) return 0;
+                foreach (var key in list)
                 {
-                    int i = 0;
-                    int count = list.Count;
-                    while (i < count)
-                    {
-                        dictionary.Remove(list[i]);
-                        i++;
-                    }
-                    result = list.Count;
-                }
-                else
-                {
-                    result = 0;
+                    dictionary.Remove(key);
                 }
+                return list.Count;
             }
             finally
             {
@@ -90,8 +79,6 @@ public static int RemoveAll<TKey, TValue>(this Dictionary<TKey, TValue> dictiona
                     SimplePool<List<TKey>>.Return(list);
                 }
             }
-
-            return result;
         }
 
         public static bool EqualAsSets<T>(this IEnumerable<T> enum1, IEnumerable<T> enum2)