Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refresh vs Add/Remove for scalar datom changes in IObservable #73

Merged
merged 1 commit into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 35 additions & 13 deletions src/NexusMods.MnemonicDB.Abstractions/Query/ObservableDatoms.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}
});
}
Expand Down Expand Up @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[
{
Added: 3
},
{
Refreshes: 1
},
{
Added: 1
},
{
Removed: 1
}
]
49 changes: 49 additions & 0 deletions tests/NexusMods.MnemonicDB.Tests/DbTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
{
Expand Down
Loading