Decide dictionary key edits in the inspection model

A refused key and an unavailable fresh key are now the model's answer:
ReplaceKey returns the kept key with the notice text, FreshKey returns
null with its notice, and IMGUI only shows them.

A key is taken when another serializes the same or when the dictionary's
own comparer calls them equal, so 0.0 against -0.0 is refused instead of
throwing in BuildDictionary.

Keys never actually compared by bytes: the Studio handed the model the
document codec, whose registry guard throws for any non-document value,
so every key fell back to ToString. CultCacheMessagePack now builds the
model with a value serializer, and the Studio and tests both use it.

Tests pin the comparer rule, the unchanged-key rule (an old empty
reference key survives an edit of its value) and serialized identity
(keys whose ToString collides); each fails with its rule removed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Meta Vens 2026-09-14 14:03:00 +02:00
commit 2a46409017
5 changed files with 138 additions and 70 deletions

View file

@ -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)

View file

@ -289,8 +289,8 @@ namespace GameCult.Caching
private readonly Dictionary<Type, CultInspectorShape> _shapes = new Dictionary<Type, CultInspectorShape>();
private readonly Dictionary<MemberInfo, CultInspectorMetadata> _metadata = new Dictionary<MemberInfo, CultInspectorMetadata>();
// 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<object, Type, byte[]> serialize, Func<Type, byte[], object> deserialize)
{
Registry = registry ?? throw new ArgumentNullException(nameof(registry));
@ -392,16 +392,58 @@ namespace GameCult.Caching
public object BuildDictionary(Type dictionaryType, IEnumerable<KeyValuePair<object?, object?>> 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<T> 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<object?> 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<object?> keys, IEnumerable<CultStoredDocument> 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<T> 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<object?> 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<object?> 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<object?> existingKeys, IEnumerable<CultStoredDocument> records)
{
var used = new HashSet<string?>(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<T> may point at: every record whose document is a T, by label.

View file

@ -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<object, object>(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<object, object>(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);

View file

@ -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);

View file

@ -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<InspectItem> Ref(string key) => new CultRecordRef<InspectItem>(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<CultRecordRef<InspectItem>, 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<InspectItem>)), Is.EqualTo(model.KeyIdentity(Ref(""))));
Assert.That(model.RefuseKey(Ref(""), Ref("a"), others), Does.Contain("empty"));
Assert.That(model.RefuseKey(default(CultRecordRef<InspectItem>), 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<InspectItem>)), 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<InspectItem>), out var unchanged),
Is.EqualTo(default(CultRecordRef<InspectItem>)));
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<InspectItem>), new object?[] { new CultRecordRef<InspectItem>(first) }, cache.AllStoredDocuments);
var fresh = model.FreshKey(links, new object?[] { new CultRecordRef<InspectItem>(first) }, cache.AllStoredDocuments, out _);
Assert.That(CultInspectorModel.RecordKey(fresh), Is.EqualTo(second.Value));
Assert.That(model.FreshKey(typeof(CultRecordRef<InspectItem>),
new object?[] { new CultRecordRef<InspectItem>(first), new CultRecordRef<InspectItem>(second) }, cache.AllStoredDocuments), Is.Null);
Assert.That(model.FreshKey(links, new object?[] { new CultRecordRef<InspectItem>(first), new CultRecordRef<InspectItem>(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<double, int>);
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<object?, object?>(key, 0))), Throws.Nothing);
Assert.That(model.FreshKey(doubles, new object?[] { -0.0 }, Array.Empty<CultStoredDocument>(), 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<InspectKey, int>);
Assert.That(model.FreshKey(dictionary, new object?[] { new InspectKey(1) }, Array.Empty<CultStoredDocument>(), 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
{
}