diff --git a/src/GameCult.Caching.MessagePack/CultCacheMessagePack.cs b/src/GameCult.Caching.MessagePack/CultCacheMessagePack.cs index 04bdc220..1592bf36 100644 --- a/src/GameCult.Caching.MessagePack/CultCacheMessagePack.cs +++ b/src/GameCult.Caching.MessagePack/CultCacheMessagePack.cs @@ -19,6 +19,13 @@ namespace GameCult.Caching.MessagePack public static class CultCacheMessagePack { + // The inspection model over this codec. Values serialize without the document guard, so dictionary keys and other + // non-document values compare by their bytes; clones deserialize as registered documents. + public static CultInspectorModel CreateInspectorModel(CultDocumentRegistry registry) => new CultInspectorModel( + registry, + (value, type) => global::MessagePack.MessagePackSerializer.Serialize(type, value, CultDocumentMessagePackSerialization.OptionsFor(type.Assembly)), + (type, bytes) => CultDocumentMessagePackSerialization.DeserializeUntyped(type, bytes, registry)); + // Opening hydrates. A consumer that wants a fresh store removes the records it does not keep, upserts, and // flushes; the atomic replace does the rest. public static CultCache Create(string filePath, CultCacheOpenOptions? options = null) diff --git a/src/GameCult.Caching/CultInspectorModel.cs b/src/GameCult.Caching/CultInspectorModel.cs index dd7e5e1c..6061791c 100644 --- a/src/GameCult.Caching/CultInspectorModel.cs +++ b/src/GameCult.Caching/CultInspectorModel.cs @@ -289,8 +289,8 @@ namespace GameCult.Caching private readonly Dictionary _shapes = new Dictionary(); private readonly Dictionary _metadata = new Dictionary(); - // serialize and deserialize are the store's document codec (CultDocumentMessagePackSerialization for .cc stores); - // edits clone through it and dictionary keys compare by it. + // serialize and deserialize are the store's codec (CultCacheMessagePack.CreateInspectorModel for .cc stores). Edits + // clone documents through both; dictionary keys compare by serialize, so it must take any value, not only documents. public CultInspectorModel(CultDocumentRegistry registry, Func serialize, Func deserialize) { Registry = registry ?? throw new ArgumentNullException(nameof(registry)); @@ -392,16 +392,58 @@ namespace GameCult.Caching public object BuildDictionary(Type dictionaryType, IEnumerable> entries) { - var shape = ShapeOf(dictionaryType); - if (shape.Kind != CultInspectorValueKind.Dictionary) - throw new ArgumentException($"{dictionaryType.Name} is not a dictionary the inspector builds.", nameof(dictionaryType)); - var dictionary = (IDictionary)Activator.CreateInstance(shape.BuildType!)!; + var dictionary = (IDictionary)Activator.CreateInstance(DictionaryShape(dictionaryType).BuildType!)!; foreach (var entry in entries) dictionary.Add(entry.Key!, entry.Value); return dictionary; } - // Keys are the same key when they serialize the same. A CultRecordRef key is its key string, null and "" alike. - public string? KeyIdentity(object? key) + // The key entry `index` of a dictionary holding `keys` takes when a lowering offers `candidate`: the candidate, or + // the kept key and a notice saying why. An unchanged key is never refused. Any other key must be non-null, not an + // empty record reference, and not taken by another entry, so BuildDictionary cannot throw. + public object? ReplaceKey(Type dictionaryType, IReadOnlyList keys, int index, object? candidate, out string? notice) + { + var shape = DictionaryShape(dictionaryType); + var identity = KeyIdentity(candidate); + var refusal = identity == KeyIdentity(keys[index]) ? null + : identity == null ? "a null key" + : identity == RecordRefIdentity ? "an empty record reference as a key" + : Taken(shape, candidate!, keys.Where((_, i) => i != index)) ? "a duplicate key" + : null; + notice = refusal == null ? null : $"Refused {refusal} on entry {index}; its key was kept."; + return refusal == null ? candidate : keys[index]; + } + + // A key to add beside `keys`: a record-reference key takes the first candidate record not already used; any other + // key is the type's default. Null with a notice when no such key is free. + public object? FreshKey(Type dictionaryType, IReadOnlyList keys, IEnumerable records, out string? notice) + { + var shape = DictionaryShape(dictionaryType); + var keyType = shape.KeyType!; + var candidates = ShapeOf(keyType).Kind == CultInspectorValueKind.RecordRef + ? RecordCandidates(keyType, records).Select(record => CreateRecordRef(keyType, record.Key.Value)) + : new[] { CreateDefault(keyType) }; + var fresh = candidates.FirstOrDefault(key => key != null && !Taken(shape, key, keys)); + notice = fresh == null ? $"No unused {keyType.Name} key is available; nothing was added." : null; + return fresh; + } + + // A key is taken when another serializes the same (a CultRecordRef by its key string, null and "" alike) or when + // the dictionary's own comparer calls them equal (0.0 and -0.0 serialize apart but are one double key). + private bool Taken(CultInspectorShape dictionary, object key, IEnumerable keys) + { + var identity = KeyIdentity(key); + var probe = (IDictionary)Activator.CreateInstance(dictionary.BuildType!)!; + var filler = dictionary.ValueType!.IsValueType ? Activator.CreateInstance(dictionary.ValueType) : null; + foreach (var other in keys.Where(other => other != null)) + { + if (KeyIdentity(other) == identity) return true; + probe[other!] = filler; + } + + return probe.Contains(key); + } + + private string? KeyIdentity(object? key) { if (key == null) return null; if (key is ICultRecordRef reference) return RecordRefIdentity + reference.Key.Value; @@ -415,29 +457,12 @@ namespace GameCult.Caching } } - // Why candidate may not replace current among otherKeys, or null when it may (an unchanged key always may). - public string? RefuseKey(object? candidate, object? current, IEnumerable otherKeys) + private CultInspectorShape DictionaryShape(Type dictionaryType) { - var identity = KeyIdentity(candidate); - if (identity == KeyIdentity(current)) return null; - if (identity == null) return "a null key"; - if (identity == RecordRefIdentity) return "an empty record reference as a key"; - return otherKeys.Any(key => KeyIdentity(key) == identity) ? "a duplicate key" : null; - } - - // A key to add: a record-reference key takes the first candidate record not already used; any other key is the - // type's default. Null when that key is taken or cannot be made. - public object? FreshKey(Type keyType, IEnumerable existingKeys, IEnumerable records) - { - var used = new HashSet(existingKeys.Select(KeyIdentity)); - if (ShapeOf(keyType).Kind == CultInspectorValueKind.RecordRef) - { - var record = RecordCandidates(keyType, records).FirstOrDefault(candidate => !used.Contains(RecordRefIdentity + candidate.Key.Value)); - return record == null ? null : CreateRecordRef(keyType, record.Key.Value); - } - - var fresh = CreateDefault(keyType); - return fresh == null || used.Contains(KeyIdentity(fresh)) ? null : fresh; + var shape = ShapeOf(dictionaryType); + return shape.Kind == CultInspectorValueKind.Dictionary + ? shape + : throw new ArgumentException($"{dictionaryType.Name} is not a dictionary the inspector builds.", nameof(dictionaryType)); } // The records a CultRecordRef may point at: every record whose document is a T, by label. diff --git a/src/GameCult.Unity/Assets/Caching/Editor/CultCacheStudioDrawers.cs b/src/GameCult.Unity/Assets/Caching/Editor/CultCacheStudioDrawers.cs index d442d1d7..8873a194 100644 --- a/src/GameCult.Unity/Assets/Caching/Editor/CultCacheStudioDrawers.cs +++ b/src/GameCult.Unity/Assets/Caching/Editor/CultCacheStudioDrawers.cs @@ -291,18 +291,8 @@ namespace GameCult.Unity.Caching.Editor var item = DrawValue("Value", shape.ValueType, entries[i].Value, member, path + "{" + i + "}.value"); if (EditorGUI.EndChangeCheck()) { - var index = i; - var refusal = Model.RefuseKey(key, entries[i].Key, entries.Where((_, j) => j != index).Select(e => e.Key)); - if (refusal != null) - { - _notices[path] = "Refused " + refusal + " on entry " + i + "; its key was kept."; - key = entries[i].Key; - } - else - { - _notices.Remove(path); - } - + key = Model.ReplaceKey(shape.Type, entries.Select(e => e.Key).ToArray(), i, key, out var notice); + Notice(path, notice); entries[i] = new KeyValuePair(key, item); changed = true; } @@ -320,20 +310,16 @@ namespace GameCult.Unity.Caching.Editor if (GUILayout.Button("Add")) { - var fresh = Model.FreshKey(shape.KeyType, entries.Select(e => e.Key), Records); - if (fresh == null) - { - _notices[path] = "No unused " + shape.KeyType.Name + " key is available; nothing was added."; - } - else + var fresh = Model.FreshKey(shape.Type, entries.Select(e => e.Key).ToArray(), Records, out var notice); + Notice(path, notice); + if (fresh != null) { entries.Add(new KeyValuePair(fresh, Model.CreateDefault(shape.ValueType))); - _notices.Remove(path); changed = true; } } - if (_notices.TryGetValue(path, out var notice)) EditorGUILayout.HelpBox(notice, MessageType.Warning); + DrawNotice(path); EditorGUI.indentLevel--; return changed ? Model.BuildDictionary(shape.Type, entries) : dictionary; } @@ -474,6 +460,18 @@ namespace GameCult.Unity.Caching.Editor return expanded; } + // The model's notice for the control at path, kept until that control's next answer. + private void Notice(string path, string notice) + { + if (notice == null) _notices.Remove(path); + else _notices[path] = notice; + } + + private void DrawNotice(string path) + { + if (_notices.TryGetValue(path, out var notice)) EditorGUILayout.HelpBox(notice, MessageType.Warning); + } + private static object ErrorRow(string label, string message, object value) { EditorGUILayout.LabelField(label, message, ErrorStyle); diff --git a/src/GameCult.Unity/Assets/Caching/Editor/CultCacheStudioWindow.cs b/src/GameCult.Unity/Assets/Caching/Editor/CultCacheStudioWindow.cs index c4920d31..cffe5a07 100644 --- a/src/GameCult.Unity/Assets/Caching/Editor/CultCacheStudioWindow.cs +++ b/src/GameCult.Unity/Assets/Caching/Editor/CultCacheStudioWindow.cs @@ -273,10 +273,7 @@ namespace GameCult.Unity.Caching.Editor if (!File.Exists(path)) throw new IOException("the store wrote no file"); } - var registry = _cache.Registry; - _model = new CultInspectorModel(registry, - (value, type) => CultDocumentMessagePackSerialization.SerializeUntyped(value, type, registry), - (type, bytes) => CultDocumentMessagePackSerialization.DeserializeUntyped(type, bytes, registry)); + _model = CultCacheMessagePack.CreateInspectorModel(_cache.Registry); _inspector = new CultInspector(_model); _path = path; EditorPrefs.SetString(LastPathKey, path); diff --git a/tests/GameCult.Caching.Tests/CultInspectorModelTests.cs b/tests/GameCult.Caching.Tests/CultInspectorModelTests.cs index 4929f448..c7f265b0 100644 --- a/tests/GameCult.Caching.Tests/CultInspectorModelTests.cs +++ b/tests/GameCult.Caching.Tests/CultInspectorModelTests.cs @@ -16,10 +16,7 @@ namespace GameCult.Caching.Tests private static readonly CultDocumentRegistry Registry = CultDocumentRegistry.ForTypes(new[] { typeof(InspectItem), typeof(InspectOther), typeof(InspectFixed) }); - private static CultInspectorModel Model() => new CultInspectorModel( - Registry, - (value, type) => CultDocumentMessagePackSerialization.SerializeUntyped(value, type, Registry), - (type, bytes) => CultDocumentMessagePackSerialization.DeserializeUntyped(type, bytes, Registry)); + private static CultInspectorModel Model() => CultCacheMessagePack.CreateInspectorModel(Registry); private static CultRecordRef Ref(string key) => new CultRecordRef(new CultRecordKey(key)); @@ -98,24 +95,62 @@ namespace GameCult.Caching.Tests public async Task DictionaryKeysRefuseNullEmptyReferencesAndDuplicates() { var model = Model(); - var others = new object?[] { Ref("b") }; + var links = typeof(Dictionary, int>); + var keys = new object?[] { Ref("a"), Ref("b") }; + string? Refusal(object? candidate) + { + var kept = model.ReplaceKey(links, keys, 0, candidate, out var notice); + Assert.That(kept, Is.EqualTo(notice == null ? candidate : keys[0]), "a refused key keeps the entry's key"); + return notice; + } - Assert.That(model.KeyIdentity(default(CultRecordRef)), Is.EqualTo(model.KeyIdentity(Ref("")))); - Assert.That(model.RefuseKey(Ref(""), Ref("a"), others), Does.Contain("empty")); - Assert.That(model.RefuseKey(default(CultRecordRef), Ref("a"), others), Does.Contain("empty")); - Assert.That(model.RefuseKey(Ref("b"), Ref("a"), others), Does.Contain("duplicate")); - Assert.That(model.RefuseKey(null, Ref("a"), others), Does.Contain("null")); - Assert.That(model.RefuseKey(Ref("c"), Ref("a"), others), Is.Null); - Assert.That(model.RefuseKey(Ref("a"), Ref("a"), others), Is.Null, "an unchanged key is never refused"); - Assert.That(model.RefuseKey("x", "y", new object?[] { "x" }), Does.Contain("duplicate")); + Assert.That(Refusal(Ref("")), Does.Contain("empty")); + Assert.That(Refusal(default(CultRecordRef)), Does.Contain("empty"), "null and \"\" are one empty reference"); + Assert.That(Refusal(Ref("b")), Does.Contain("duplicate").And.Contain("entry 0")); + Assert.That(Refusal(null), Does.Contain("null")); + Assert.That(Refusal(Ref("c")), Is.Null); + + // An entry an old store left with an empty reference key keeps it through an edit of its value. + Assert.That(model.ReplaceKey(links, new object?[] { Ref(""), Ref("b") }, 0, default(CultRecordRef), out var unchanged), + Is.EqualTo(default(CultRecordRef))); + Assert.That(unchanged, Is.Null, "an unchanged key is never refused"); using var cache = new CultCache(Registry); var first = await cache.UpsertAsync(typeof(InspectItem), new InspectItem { Name = "first" }); var second = await cache.UpsertAsync(typeof(InspectItem), new InspectItem { Name = "second" }); - var fresh = model.FreshKey(typeof(CultRecordRef), new object?[] { new CultRecordRef(first) }, cache.AllStoredDocuments); + var fresh = model.FreshKey(links, new object?[] { new CultRecordRef(first) }, cache.AllStoredDocuments, out _); Assert.That(CultInspectorModel.RecordKey(fresh), Is.EqualTo(second.Value)); - Assert.That(model.FreshKey(typeof(CultRecordRef), - new object?[] { new CultRecordRef(first), new CultRecordRef(second) }, cache.AllStoredDocuments), Is.Null); + Assert.That(model.FreshKey(links, new object?[] { new CultRecordRef(first), new CultRecordRef(second) }, + cache.AllStoredDocuments, out var none), Is.Null); + Assert.That(none, Does.Contain("No unused")); + } + + [Test] + public void DictionaryKeysAgreeWithTheDictionaryComparer() + { + var model = Model(); + var doubles = typeof(Dictionary); + var keys = new object?[] { 0.0, 1.0 }; + + Assert.That(model.ReplaceKey(doubles, keys, 1, -0.0, out var notice), Is.EqualTo(1.0)); + Assert.That(notice, Does.Contain("duplicate"), "-0.0 serializes apart from 0.0 but Equals it"); + Assert.That(() => model.BuildDictionary(doubles, keys.Select(key => new KeyValuePair(key, 0))), Throws.Nothing); + Assert.That(model.FreshKey(doubles, new object?[] { -0.0 }, Array.Empty(), out _), Is.Null, + "the default 0.0 is taken by -0.0"); + Assert.That(model.ReplaceKey(doubles, new object?[] { 0.0 }, 0, -0.0, out _), Is.EqualTo(-0.0), "a key may replace itself"); + } + + [Test] + public void DictionaryKeysCompareBySerializedValueNotText() + { + var model = Model(); + var dictionary = typeof(Dictionary); + + Assert.That(model.FreshKey(dictionary, new object?[] { new InspectKey(1) }, Array.Empty(), out _), + Is.EqualTo(new InspectKey(0)), "keys whose text collides are still distinct keys"); + Assert.That(model.ReplaceKey(dictionary, new object?[] { new InspectKey(1), new InspectKey(2) }, 1, new InspectKey(3), out var notice), + Is.EqualTo(new InspectKey(3))); + Assert.That(notice, Is.Null); } [Test] @@ -262,6 +297,12 @@ namespace GameCult.Caching.Tests public readonly record struct InspectPair(float x, float y); + [MessagePackObject] + public readonly record struct InspectKey([property: Key(0)] int Value) + { + public override string ToString() => "key"; + } + public interface IFakeDrawer { }