Merge pull request #73 from Nexus-Mods/observe-datom-flickering

Refresh vs Add/Remove for scalar datom changes in IObservable

src/NexusMods.MnemonicDB.Abstractions/Internals/IAttributeRegistry.cs

@@ -16,6 +16,12 @@ public interface IAttributeRegistry
     /// <param name="datom"></param>
     /// <returns></returns>
     public IReadDatom Resolve(in KeyPrefix prefix, ReadOnlySpan<byte> datom);
+
+
+    /// <summary>
+    /// Resolve the given attribute id into an attribute
+    /// </summary>
+    public IAttribute GetAttribute(AttributeId attributeId);
 
     /// <summary>
     /// Populates the registry with the given attributes, mostly used for

src/NexusMods.MnemonicDB.Abstractions/Query/ObservableDatoms.cs

@@ -7,6 +7,7 @@
 using NexusMods.MnemonicDB.Abstractions.DatomComparators;
 using NexusMods.MnemonicDB.Abstractions.DatomIterators;
 using NexusMods.MnemonicDB.Abstractions.IndexSegments;
+using NexusMods.MnemonicDB.Abstractions.Internals;
 
 namespace NexusMods.MnemonicDB.Abstractions.Query;
 
@@ -55,7 +56,7 @@ public static IObservable<IChangeSet<Datom>> ObserveDatoms(this IConnection conn
 
                 if (idx == 0)
                     return Setup(set, rev.Database, descriptor);
-                return Diff(set, rev.AddedDatoms, descriptor, equality);
+                return Diff(conn.Registry, set, rev.AddedDatoms, descriptor, equality);
             }
         });
     }
@@ -93,31 +94,52 @@ public static IObservable<IChangeSet<Datom>> ObserveDatoms(this IConnection conn
         return conn.Revisions.DelayUntilFirstValue(() => conn.ObserveDatoms(SliceDescriptor.Create(attribute, conn.Registry)));
     }
 
-    private static IChangeSet<Datom> Diff(SortedSet<Datom> set, IndexSegment updates, SliceDescriptor descriptor, IEqualityComparer<Datom> comparer)
+    private static IChangeSet<Datom> Diff(IAttributeRegistry registry, SortedSet<Datom> set, IndexSegment updates, SliceDescriptor descriptor, IEqualityComparer<Datom> comparer)
     {
         List<Change<Datom>>? changes = null;
-
-        // TEMPORARY for testing
-        var prevSet = set.ToArray();
-
-        foreach (var datom in updates)
+
+        for (int i = 0; i < updates.Count; i++) 
         {
+            var datom = updates[i];
             if (!descriptor.Includes(datom))
                 continue;
             if (datom.IsRetract)
             {
                 var idx = set.IndexOf(datom, comparer);
-                if (idx >= 0)
+                if (idx < 0)
+                {
+                    throw new InvalidOperationException("Retract without assert in set");
+                }
+
+                set.Remove(datom);
+                changes ??= [];
+
+                // If the attribute is cardinality many, we can just remove the datom
+                if (registry.GetAttribute(datom.A).Cardinalty == Cardinality.Many)
                 {
-                    set.Remove(datom);
-                    changes ??= [];
                     changes.Add(new Change<Datom>(ListChangeReason.Remove, datom, idx));
+                    continue;
                 }
-                else
+
+                // If the next datom is not the same E or A, we can remove the datom
+                if (updates.Count <= i + 1)
                 {
-                    var existsOriginally = prevSet.Any(d => comparer.Equals(d, datom));
-                    throw new InvalidOperationException("Retract without assert in set");
+                    changes.Add(new Change<Datom>(ListChangeReason.Remove, datom, idx));
+                    continue;
+                }
+
+                var nextDatom = updates[i + 1];
+                if (nextDatom.E != datom.E || nextDatom.A != datom.A)
+                {
+                    changes.Add(new Change<Datom>(ListChangeReason.Remove, datom, idx));
+                    continue;
                 }
+
+                // Otherwise we skip the add, and issue a refresh, update the cache, and skip the next datom
+                // because we've already processed it
+                changes.Add(new Change<Datom>(ListChangeReason.Refresh, nextDatom, datom, idx));
+                set.Add(nextDatom);
+                i++;
             }
             else
             {

tests/NexusMods.MnemonicDB.Tests/DbTests.CanObserveIndexChangesWithoutFlickering.verified.txt

@@ -0,0 +1,14 @@
+[
+  {
+    Added: 3
+  },
+  {
+    Refreshes: 1
+  },
+  {
+    Added: 1
+  },
+  {
+    Removed: 1
+  }
+]

tests/NexusMods.MnemonicDB.Tests/DbTests.cs

@@ -2,6 +2,7 @@
 using DynamicData;
 using NexusMods.MnemonicDB.Abstractions;
 using NexusMods.Hashing.xxHash64;
+using NexusMods.MnemonicDB.Abstractions.DatomIterators;
 using NexusMods.MnemonicDB.Abstractions.Models;
 using NexusMods.MnemonicDB.Abstractions.Query;
 using NexusMods.MnemonicDB.Abstractions.TxFunctions;
@@ -624,6 +625,54 @@ public async Task CanObserveIndexChanges()
         await Verify(changes);
     }
 
+    /// <summary>
+    /// Test for a flickering bug in UI users, where datoms that are changed result in a `add` and a `remove` operation
+    /// causing the UI to flicker, instead we want to issue changes on a ScalarAttribute as a refresh/change
+    /// </summary>
+    [Fact]
+    public async Task CanObserveIndexChangesWithoutFlickering()
+    {
+
+        var loadout = await InsertExampleData();
+
+        List<IChangeSet<Datom>> changes = new();
+
+        // Define the slice to observe
+        var slice = SliceDescriptor.Create(Mod.Name, Connection.Db.Registry);
+
+        // Setup the subscription
+        using var _ = Connection.ObserveDatoms(slice)
+            // Add the changes to the list
+            .Subscribe(x => changes.Add(x));
+
+        // Rename a mod, should result in a refresh, not a add and remove
+        using var tx = Connection.BeginTransaction();
+        tx.Add(loadout.Mods.First().Id, Mod.Name, "Test Mod 1");
+        await tx.Commit();
+
+        // Add a new mod
+        using var tx2 = Connection.BeginTransaction();
+        tx2.Add(tx2.TempId(), Mod.Name, "Test Mod 2");
+        await tx2.Commit();
+
+        // Delete the first mod
+        using var tx3 = Connection.BeginTransaction();
+        tx3.Retract(loadout.Mods.First().Id, Mod.Name, "Test Mod 1");
+        await tx3.Commit();
+
+
+        var changesProcessed = changes.Select(change => new
+        {
+            Added = change.Adds,
+            Removed = change.Removes,
+            Refreshes = change.Refreshes
+        });
+
+        await Verify(changesProcessed);
+
+
+    }
+
     [Fact]
     public async Task MultipleIncludesDontSplitEntities()
     {