From 6807bbabdd0ce28ae80bee717dc176b0ad640436 Mon Sep 17 00:00:00 2001 From: erri120 Date: Wed, 6 Nov 2024 12:06:32 +0100 Subject: [PATCH 1/4] Improved Get/TryGet methods --- .../Attributes/ScalarAttribute.cs | 97 ++++++++----------- tests/NexusMods.MnemonicDB.Tests/DbTests.cs | 4 +- 2 files changed, 43 insertions(+), 58 deletions(-) diff --git a/src/NexusMods.MnemonicDB.Abstractions/Attributes/ScalarAttribute.cs b/src/NexusMods.MnemonicDB.Abstractions/Attributes/ScalarAttribute.cs index 00a7d263..dd319440 100644 --- a/src/NexusMods.MnemonicDB.Abstractions/Attributes/ScalarAttribute.cs +++ b/src/NexusMods.MnemonicDB.Abstractions/Attributes/ScalarAttribute.cs @@ -1,6 +1,8 @@ using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using DynamicData.Kernel; -using NexusMods.MnemonicDB.Abstractions.ElementComparers; +using JetBrains.Annotations; +using NexusMods.MnemonicDB.Abstractions.IndexSegments; using NexusMods.MnemonicDB.Abstractions.Models; namespace NexusMods.MnemonicDB.Abstractions.Attributes; @@ -8,6 +10,7 @@ namespace NexusMods.MnemonicDB.Abstractions.Attributes; /// /// An attribute that represents a scalar value, where there is a 1:1 ratio between the attribute and the value. /// +[PublicAPI] public abstract class ScalarAttribute(string ns, string name) : Attribute(ns, name) where TSerializer : IValueSerializer @@ -23,84 +26,66 @@ public bool IsOptional } /// - /// Gets the value of the attribute from the entity. + /// Tries to get the value of the attribute from the entity. /// - public TValue Get(IHasIdAndIndexSegment entity) + public bool TryGetValue(T entity, IndexSegment segment, [NotNullWhen(true)] out TValue? value) + where T : IHasEntityIdAndDb { - var segment = entity.IndexSegment; - var dbId = entity.Db.AttributeCache.GetAttributeId(Id); - for (var i = 0; i < segment.Count; i++) + var attributeId = entity.Db.AttributeCache.GetAttributeId(Id); + foreach (var datom in segment) { - var datom = segment[i]; - if (datom.A != dbId) continue; - return ReadValue(datom.ValueSpan, datom.Prefix.ValueTag, entity.Db.Connection.AttributeResolver); + if (datom.A != attributeId) continue; + value = ReadValue(datom.ValueSpan, datom.Prefix.ValueTag, entity.Db.Connection.AttributeResolver); + return true; } - if (DefaultValue.HasValue) - return DefaultValue.Value; - - ThrowKeyNotfoundException(entity); - return default!; + value = default!; + return false; } - + /// - /// Gets the value of the attribute from the entity, this performs a lookup in the database - /// so prefer using the overload with IHasIdAndIndexSegment if you already have the segment. + /// Gets the value of the attribute from the entity. /// - protected TValue Get(IHasEntityIdAndDb entity) + public TValue Get(T entity, IndexSegment segment) + where T : IHasEntityIdAndDb { - var segment = entity.Db.Get(entity.Id); - var dbId = entity.Db.AttributeCache.GetAttributeId(Id); - for (var i = 0; i < segment.Count; i++) - { - var datom = segment[i]; - if (datom.A != dbId) continue; - return ReadValue(datom.ValueSpan, datom.Prefix.ValueTag, entity.Db.Connection.AttributeResolver); - } - - if (DefaultValue.HasValue) - return DefaultValue.Value; - - ThrowKeyNotfoundException(entity); - return default!; + if (TryGetValue(entity, segment, out var value)) return value; + return ThrowKeyNotfoundException(entity.Id); } /// - /// Retracts the attribute from the entity. + /// Gets the value of the attribute from the entity. /// - public void Retract(IAttachedEntity entityWithTx) + public TValue Get(T entity) + where T : IHasIdAndIndexSegment { - Retract(entityWithTx, Get(entityWithTx)); + var segment = entity.IndexSegment; + return Get(entity, segment); } - private void ThrowKeyNotfoundException(IHasEntityIdAndDb entity) + /// + /// Gets the value of the attribute from the entity, , or . + /// + public Optional GetOptional(T entity) + where T : IHasIdAndIndexSegment { - throw new KeyNotFoundException($"Entity {entity.Id} does not have attribute {Id}"); + if (TryGetValue(entity, entity.IndexSegment, out var value)) return value; + return DefaultValue.HasValue ? DefaultValue : Optional.None; } /// - /// Try to get the value of the attribute from the entity. + /// Retracts the attribute from the entity. /// - public bool TryGet(IHasIdAndIndexSegment entity, out TValue value) + public void Retract(IAttachedEntity entityWithTx) { - var segment = entity.IndexSegment; - var dbId = entity.Db.AttributeCache.GetAttributeId(Id); - for (var i = 0; i < segment.Count; i++) - { - var datom = segment[i]; - if (datom.A != dbId) continue; - value = ReadValue(datom.ValueSpan, datom.Prefix.ValueTag, entity.Db.Connection.AttributeResolver); - return true; - } - - if (DefaultValue.HasValue) - { - value = DefaultValue.Value; - return true; - } + Retract(entityWithTx, value: Get(entityWithTx, segment: entityWithTx.Db.Get(entityWithTx.Id))); + } - value = default!; - return false; + [DoesNotReturn] + private TValue ThrowKeyNotfoundException(EntityId entityId) + { + throw new KeyNotFoundException($"Entity `{entityId}` doesn't have attribute {Id}"); + return default!; } /// diff --git a/tests/NexusMods.MnemonicDB.Tests/DbTests.cs b/tests/NexusMods.MnemonicDB.Tests/DbTests.cs index 718c2c4f..9debed02 100644 --- a/tests/NexusMods.MnemonicDB.Tests/DbTests.cs +++ b/tests/NexusMods.MnemonicDB.Tests/DbTests.cs @@ -538,13 +538,13 @@ public async Task CanReadAndWriteOptionalAttributes() var remapped = result.Remap(modWithDescription); remapped.Contains(Mod.Description).Should().BeTrue(); - Mod.Description.TryGet(remapped, out var foundDesc).Should().BeTrue(); + Mod.Description.TryGetValue(remapped, remapped.IndexSegment, out var foundDesc).Should().BeTrue(); foundDesc.Should().Be("Test Description"); remapped.Description.Should().Be("Test Description"); var remapped2 = result.Remap(modWithoutDiscription); remapped2.Contains(Mod.Description).Should().BeFalse(); - Mod.Description.TryGet(remapped2, out var foundDesc2).Should().BeFalse(); + Mod.Description.TryGetValue(remapped2, remapped2.IndexSegment, out var foundDesc2).Should().BeFalse(); } [Fact] From 97c956925aab38eecf3d350bcb64b20b6c673165 Mon Sep 17 00:00:00 2001 From: erri120 Date: Wed, 6 Nov 2024 12:13:49 +0100 Subject: [PATCH 2/4] Use optional --- .../Benchmarks/ReadThenWriteBenchmarks.cs | 2 +- src/NexusMods.MnemonicDB.SourceGenerator/Template.weave | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/benchmarks/NexusMods.MnemonicDB.Benchmarks/Benchmarks/ReadThenWriteBenchmarks.cs b/benchmarks/NexusMods.MnemonicDB.Benchmarks/Benchmarks/ReadThenWriteBenchmarks.cs index b3cb8b03..a9449e70 100644 --- a/benchmarks/NexusMods.MnemonicDB.Benchmarks/Benchmarks/ReadThenWriteBenchmarks.cs +++ b/benchmarks/NexusMods.MnemonicDB.Benchmarks/Benchmarks/ReadThenWriteBenchmarks.cs @@ -61,7 +61,7 @@ public async Task ReadThenWrite() using var tx = Connection.BeginTransaction(); var mod = Mod.Load(Connection.Db, _modId); var oldHash = mod.OptionalHash; - tx.Add(_modId, Mod.OptionalHash, Hash.From(oldHash.Value + 1)); + tx.Add(_modId, Mod.OptionalHash, Hash.From((ulong)oldHash.Value + 1)); var nextdb = await tx.Commit(); var loadout = Loadout.Load(Connection.Db, mod.LoadoutId); diff --git a/src/NexusMods.MnemonicDB.SourceGenerator/Template.weave b/src/NexusMods.MnemonicDB.SourceGenerator/Template.weave index 5f5bc0c2..de9ea7f9 100644 --- a/src/NexusMods.MnemonicDB.SourceGenerator/Template.weave +++ b/src/NexusMods.MnemonicDB.SourceGenerator/Template.weave @@ -371,8 +371,12 @@ public partial class {{= model.Name}} : __MODELS__.IModelFactory<{{= model.Name} {{elif attr.IsReference}} public {{= attr.ReferenceType.ToDisplayString()}}Id {{= attr.ContextualName}} => {{= attr.ReferenceType.ToDisplayString()}}Id.From({{= attr.FieldName}}.Get(this)); {{else}} + {{if attr.IsOptional}} + public global::DynamicData.Kernel.Optional<{{= attr.HighLevelType.ToDisplayString()}}> {{= attr.ContextualName }} => {{= attr.FieldName}}.GetOptional(this); + {{else}} public {{= attr.HighLevelType.ToDisplayString()}} {{= attr.ContextualName}} => {{= attr.FieldName}}.Get(this); {{/if}} + {{/if}} {{if attr.IsReference && attr.IsScalar}} From 4a80f50e707558bfe4008968f04c6a469e435596 Mon Sep 17 00:00:00 2001 From: erri120 Date: Wed, 6 Nov 2024 12:21:10 +0100 Subject: [PATCH 3/4] Add overload --- .../Attributes/ScalarAttribute.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/NexusMods.MnemonicDB.Abstractions/Attributes/ScalarAttribute.cs b/src/NexusMods.MnemonicDB.Abstractions/Attributes/ScalarAttribute.cs index dd319440..4ac7e08d 100644 --- a/src/NexusMods.MnemonicDB.Abstractions/Attributes/ScalarAttribute.cs +++ b/src/NexusMods.MnemonicDB.Abstractions/Attributes/ScalarAttribute.cs @@ -43,6 +43,15 @@ public bool TryGetValue(T entity, IndexSegment segment, [NotNullWhen(true)] o return false; } + /// + /// Tries to get the value of the attribute from the entity. + /// + public bool TryGetValue(T entity, [NotNullWhen(true)] out TValue? value) + where T : IHasIdAndIndexSegment + { + return TryGetValue(entity, segment: entity.IndexSegment, out value); + } + /// /// Gets the value of the attribute from the entity. /// @@ -85,7 +94,9 @@ public void Retract(IAttachedEntity entityWithTx) private TValue ThrowKeyNotfoundException(EntityId entityId) { throw new KeyNotFoundException($"Entity `{entityId}` doesn't have attribute {Id}"); +#pragma warning disable CS0162 // Unreachable code detected return default!; +#pragma warning restore CS0162 // Unreachable code detected } /// From d6688ca88d8baa9ce4d7e03785edd50498c175f2 Mon Sep 17 00:00:00 2001 From: erri120 Date: Wed, 6 Nov 2024 12:26:29 +0100 Subject: [PATCH 4/4] Fix tests --- tests/NexusMods.MnemonicDB.Tests/DbTests.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/NexusMods.MnemonicDB.Tests/DbTests.cs b/tests/NexusMods.MnemonicDB.Tests/DbTests.cs index 9debed02..505cc51b 100644 --- a/tests/NexusMods.MnemonicDB.Tests/DbTests.cs +++ b/tests/NexusMods.MnemonicDB.Tests/DbTests.cs @@ -540,8 +540,8 @@ public async Task CanReadAndWriteOptionalAttributes() remapped.Contains(Mod.Description).Should().BeTrue(); Mod.Description.TryGetValue(remapped, remapped.IndexSegment, out var foundDesc).Should().BeTrue(); foundDesc.Should().Be("Test Description"); - remapped.Description.Should().Be("Test Description"); - + remapped.Description.Value.Should().Be("Test Description"); + var remapped2 = result.Remap(modWithoutDiscription); remapped2.Contains(Mod.Description).Should().BeFalse(); Mod.Description.TryGetValue(remapped2, remapped2.IndexSegment, out var foundDesc2).Should().BeFalse(); @@ -1036,12 +1036,10 @@ public async Task CanGetAttributesThatRequireDI() Name = "Test Loadout", GamePath = path }; - + var result = await tx.Commit(); - var loadout = result.Remap(loadout1); - - loadout.GamePath.Should().Be(path); + loadout.GamePath.Value.Should().Be(path); } [Fact]