From d22f662f7d7449786bbdab662de30b82f0a79971 Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Mon, 14 Sep 2026 14:07:19 +0200 Subject: [PATCH] Decide element choices and integer narrowing in the inspection model ElementChoices says what a new list element, dictionary value or union pick may be (a union's declared subtypes, or the slot type itself), and CreateElement makes one or returns a notice when a subtype has no parameterless constructor. It replaces CreateUnionValue, whose throw the IMGUI caught and sent to the console. A dictionary with a union value type now offers the subtypes too instead of adding a null value. NarrowInteger clamps an integer edit to its type's range; the IMGUI no longer converts and swallows overflow itself. IMGUI renders the model's answers and shows its notices. Co-Authored-By: Claude Opus 5 --- src/GameCult.Caching/CultInspectorModel.cs | 31 ++++++-- .../Caching/Editor/CultCacheStudioDrawers.cs | 73 +++++++++---------- .../CultInspectorModelTests.cs | 43 +++++++++-- 3 files changed, 96 insertions(+), 51 deletions(-) diff --git a/src/GameCult.Caching/CultInspectorModel.cs b/src/GameCult.Caching/CultInspectorModel.cs index 57c4eef3..bb2fbb3f 100644 --- a/src/GameCult.Caching/CultInspectorModel.cs +++ b/src/GameCult.Caching/CultInspectorModel.cs @@ -353,13 +353,32 @@ namespace GameCult.Caching return type.IsAbstract || type.IsInterface || type.GetConstructor(Type.EmptyTypes) == null ? null : Activator.CreateInstance(type); } - public object CreateUnionValue(Type unionType, Type subtype) + // What a new value for a slot of elementType (a list element, a dictionary value, a union pick) may be: a union's + // declared subtypes, or elementType itself. + public IReadOnlyList ElementChoices(Type elementType) { - if (!ShapeOf(unionType).UnionChoices.Contains(subtype)) - throw new InvalidOperationException($"{subtype.Name} is not a declared [Union] subtype of {unionType.Name}."); - if (subtype.GetConstructor(Type.EmptyTypes) == null) - throw new InvalidOperationException($"{subtype.Name} has no parameterless constructor."); - return Activator.CreateInstance(subtype)!; + var shape = ShapeOf(elementType); + return shape.Kind == CultInspectorValueKind.Union ? shape.UnionChoices : new[] { elementType }; + } + + // A new value of choice for a slot of elementType, or null with a notice when choice cannot be made. + public object? CreateElement(Type elementType, Type choice, out string? notice) + { + if (!ElementChoices(elementType).Contains(choice)) + throw new ArgumentException($"{choice.Name} is not a choice for {elementType.Name}.", nameof(choice)); + notice = choice == elementType || choice.GetConstructor(Type.EmptyTypes) != null + ? null + : $"{choice.Name} has no parameterless constructor; nothing was created."; + return notice != null ? null : choice == elementType ? CreateDefault(elementType) : Activator.CreateInstance(choice); + } + + // An integer edit as a value of integerType, clamped to that type's range instead of overflowing. + public object NarrowInteger(Type integerType, long value) + { + if (!IntegerTypes.Contains(integerType)) + throw new ArgumentException($"{integerType.Name} is not an integer the inspector edits.", nameof(integerType)); + long Bound(string name) => Convert.ToInt64(integerType.GetField(name)!.GetValue(null)); + return Convert.ChangeType(Math.Max(Bound("MinValue"), Math.Min(Bound("MaxValue"), value)), integerType); } // A new collection holding items; the value it replaces is never mutated. diff --git a/src/GameCult.Unity/Assets/Caching/Editor/CultCacheStudioDrawers.cs b/src/GameCult.Unity/Assets/Caching/Editor/CultCacheStudioDrawers.cs index fd55369f..b704d0a8 100644 --- a/src/GameCult.Unity/Assets/Caching/Editor/CultCacheStudioDrawers.cs +++ b/src/GameCult.Unity/Assets/Caching/Editor/CultCacheStudioDrawers.cs @@ -170,21 +170,13 @@ namespace GameCult.Unity.Caching.Editor GUILayout.MaxHeight(Mathf.Max(textArea.MaxLines, textArea.MinLines, 1) * EditorGUIUtility.singleLineHeight)); } - private static object DrawInteger(string label, Type type, object value, CultInspectorMetadata metadata) + private object DrawInteger(string label, Type type, object value, CultInspectorMetadata metadata) { var current = value == null ? 0L : Convert.ToInt64(value); if (type == typeof(int) && metadata.Range != null) return EditorGUILayout.IntSlider(label, (int)current, Mathf.RoundToInt(metadata.Range.Min), Mathf.RoundToInt(metadata.Range.Max)); var next = EditorGUILayout.LongField(label, current); - if (next == current) return value ?? Activator.CreateInstance(type); - try - { - return Convert.ChangeType(next, type); - } - catch (OverflowException) - { - return value ?? Activator.CreateInstance(type); - } + return next == current && value != null ? value : Model.NarrowInteger(type, next); } private object DrawRecordRef(string label, Type type, object value) @@ -257,13 +249,13 @@ namespace GameCult.Unity.Caching.Editor changed = true; } - if (GUILayout.Button("Add")) + if (Add(shape.ValueType, path, out var created)) { 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))); + entries.Add(new KeyValuePair(fresh, created)); changed = true; } } @@ -310,29 +302,13 @@ namespace GameCult.Unity.Caching.Editor changed = true; } - var elementShape = Model.ShapeOf(element); - if (elementShape.Kind == CultInspectorValueKind.Union) + if (Add(element, path, out var created)) { - var picked = EditorGUILayout.Popup("Add", 0, elementShape.UnionChoices.Select(u => u.Name).Prepend("...").ToArray()); - if (picked > 0) - { - try - { - items.Add(Model.CreateUnionValue(element, elementShape.UnionChoices[picked - 1])); - changed = true; - } - catch (InvalidOperationException exception) - { - Debug.LogError(exception.Message); - } - } - } - else if (GUILayout.Button("Add")) - { - items.Add(Model.CreateDefault(element)); + items.Add(created); changed = true; } + DrawNotice(path); EditorGUI.indentLevel--; return changed ? Model.BuildList(shape.Type, items) : value; } @@ -354,17 +330,13 @@ namespace GameCult.Unity.Caching.Editor EditorGUI.indentLevel = indent; if (picked != index) { - try - { - value = picked == 0 ? null : Model.CreateUnionValue(shape.Type, choices[picked - 1]); - } - catch (InvalidOperationException exception) - { - Debug.LogError(exception.Message); - } + string notice = null; + value = picked == 0 ? null : Model.CreateElement(shape.Type, choices[picked - 1], out notice) ?? value; + Notice(path, notice); } } + DrawNotice(path); if (!expanded || value == null) return value; EditorGUI.indentLevel++; DrawMembers(value, value.GetType(), path); @@ -409,6 +381,29 @@ namespace GameCult.Unity.Caching.Editor return expanded; } + // The add control under a list or dictionary: a button when the model offers the element type itself, a popup of + // a union's declared subtypes otherwise. True with the model's new value once one is picked and made. + private bool Add(Type elementType, string path, out object created) + { + created = null; + var choices = Model.ElementChoices(elementType); + Type choice; + if (choices.Count == 1 && choices[0] == elementType) + { + choice = GUILayout.Button("Add") ? elementType : null; + } + else + { + var picked = EditorGUILayout.Popup("Add", 0, choices.Select(c => c.Name).Prepend("...").ToArray()); + choice = picked > 0 ? choices[picked - 1] : null; + } + + if (choice == null) return false; + created = Model.CreateElement(elementType, choice, out var notice); + Notice(path, notice); + return notice == null; + } + // The model's notice for the control at path, kept until that control's next answer. private void Notice(string path, string notice) { diff --git a/tests/GameCult.Caching.Tests/CultInspectorModelTests.cs b/tests/GameCult.Caching.Tests/CultInspectorModelTests.cs index f8ca5dce..6f49ae8e 100644 --- a/tests/GameCult.Caching.Tests/CultInspectorModelTests.cs +++ b/tests/GameCult.Caching.Tests/CultInspectorModelTests.cs @@ -65,15 +65,34 @@ namespace GameCult.Caching.Tests } [Test] - public void UnionChoicesAreTheDeclaredSubtypesOnly() + public void ElementChoicesAreTheDeclaredUnionSubtypesOrTheTypeItself() { var model = Model(); - var shape = model.ShapeOf(typeof(InspectShape)); - Assert.That(shape.Kind, Is.EqualTo(CultInspectorValueKind.Union)); - Assert.That(shape.UnionChoices, Is.EqualTo(new[] { typeof(InspectCircle) })); - Assert.That(model.CreateUnionValue(typeof(InspectShape), typeof(InspectCircle)), Is.InstanceOf()); - Assert.That(() => model.CreateUnionValue(typeof(InspectShape), typeof(InspectSquare)), Throws.InvalidOperationException); + Assert.That(model.ShapeOf(typeof(InspectShape)).Kind, Is.EqualTo(CultInspectorValueKind.Union)); + Assert.That(model.ElementChoices(typeof(InspectShape)), Is.EqualTo(new[] { typeof(InspectCircle) })); + Assert.That(model.CreateElement(typeof(InspectShape), typeof(InspectCircle), out var made), Is.InstanceOf()); + Assert.That(made, Is.Null); + Assert.That(() => model.CreateElement(typeof(InspectShape), typeof(InspectSquare), out _), Throws.ArgumentException, + "an undeclared subtype is not a choice"); + + Assert.That(model.ElementChoices(typeof(List)), Is.EqualTo(new[] { typeof(List) })); + Assert.That(model.CreateElement(typeof(List), typeof(List), out _), Is.EqualTo(new List())); + + Assert.That(model.CreateElement(typeof(InspectPicky), typeof(InspectNeedsArgs), out var refused), Is.Null); + Assert.That(refused, Does.Contain("parameterless")); + } + + [Test] + public void IntegerEditsClampToTheirType() + { + var model = Model(); + + Assert.That(model.NarrowInteger(typeof(byte), 300), Is.TypeOf().And.EqualTo(byte.MaxValue)); + Assert.That(model.NarrowInteger(typeof(sbyte), -300), Is.TypeOf().And.EqualTo(sbyte.MinValue)); + Assert.That(model.NarrowInteger(typeof(uint), -1), Is.TypeOf().And.EqualTo(0u)); + Assert.That(model.NarrowInteger(typeof(int), 42), Is.TypeOf().And.EqualTo(42)); + Assert.That(() => model.NarrowInteger(typeof(float), 1), Throws.ArgumentException); } [Test] @@ -300,6 +319,18 @@ namespace GameCult.Caching.Tests public new string Value = string.Empty; } + [Union(0, typeof(InspectNeedsArgs))] + public abstract class InspectPicky + { + } + + public sealed class InspectNeedsArgs : InspectPicky + { + public InspectNeedsArgs(int side) + { + } + } + public struct InspectOpenStruct { public float x;