From a83f17b9b2022153ac5bb4d1c1c37d0d79c11d1f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 23 Sep 2026 02:29:27 +0000 Subject: [PATCH 1/2] [patch] Read an input pin's default from its C# initializer PinDefinition.DefaultValue was populated only from an explicit [InputPin(DefaultValue = ...)]. Every default in the shipped node library is written as a property initializer instead, so GetAllNodeDefinitions() reported null for all of them and any menu or inspector built from the definitions had to invent its own, which would not match what the node holds when it runs. ScanPins now falls back to reading the member off a prototype instance of the declaring type, which is the value the initializer wrote. The attribute still wins where it supplied one, matching what parameter and constructor-parameter pins already did. The prototype is built lazily, once per type, and a type that cannot be constructed without arguments or whose constructor throws simply reports no default, as before. Fixes #439 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P6bYWX7oFj1YTAAw9M516w --- ImGui.NodeEditor/AttributeBasedNodeFactory.cs | 71 ++++++++++++- .../AttributeBasedNodeFactoryTests.cs | 99 +++++++++++++++++++ 2 files changed, 168 insertions(+), 2 deletions(-) diff --git a/ImGui.NodeEditor/AttributeBasedNodeFactory.cs b/ImGui.NodeEditor/AttributeBasedNodeFactory.cs index db1b9df8..b8a513b3 100644 --- a/ImGui.NodeEditor/AttributeBasedNodeFactory.cs +++ b/ImGui.NodeEditor/AttributeBasedNodeFactory.cs @@ -321,6 +321,10 @@ private static void ScanPins(Type nodeType, NodeDefinition definition) IEnumerable members = nodeType.GetMembers(BindingFlags.Public | BindingFlags.Instance) .Where(m => m is PropertyInfo or FieldInfo); + // A prototype answers what an initializer wrote, for the pins whose attribute said nothing. + // Built at most once per type, and only if such a pin turns up. + Lazy prototype = new(() => CreatePrototype(nodeType)); + foreach (MemberInfo? member in members) { // Check for any pin attribute (input, output, execution input, execution output) @@ -348,10 +352,11 @@ private static void ScanPins(Type nodeType, NodeDefinition definition) bool isInput = pinAttr is InputPinAttribute or ExecutionInputAttribute; pinDef.IsInput = isInput; - // Set default value for input pins + // Set default value for input pins - prefer the attribute, then the member's own + // initializer read off a prototype instance, matching what parameter pins already do. if (isInput && pinAttr is InputPinAttribute inputPin) { - pinDef.DefaultValue = inputPin.DefaultValue; + pinDef.DefaultValue = inputPin.DefaultValue ?? ReadDeclaredDefault(pinDef, prototype); } // Add to appropriate collection @@ -371,6 +376,68 @@ private static void ScanPins(Type nodeType, NodeDefinition definition) definition.OutputPins.Sort((a, b) => a.Order.CompareTo(b.Order)); } + /// + /// Reads what a pin's member holds on a freshly constructed instance of its declaring type, + /// which is the value its C# initializer wrote. + /// + /// The pin whose member to read. + /// The prototype instance, or null if the type could not be constructed. + /// The declared default, or null if there is no prototype or the member cannot be read. + private static object? ReadDeclaredDefault(PinDefinition pin, Lazy prototype) + { + object? instance = prototype.Value; + if (instance is null) + { + return null; + } + + try + { + return pin.GetValue(instance); + } + catch (TargetInvocationException) + { + // A getter that throws on a default-constructed instance has no default to report. + return null; + } + } + + /// + /// Constructs an instance of a node type purely to read its initializers, returning null when + /// the type cannot be constructed without arguments or its constructor refuses to run. + /// + /// The type to construct. + /// The instance, or null. + private static object? CreatePrototype(Type nodeType) + { + bool constructible = !nodeType.IsAbstract + && !nodeType.ContainsGenericParameters + && (nodeType.IsValueType || nodeType.GetConstructor(Type.EmptyTypes) is not null); + + if (!constructible) + { + return null; + } + + try + { + return Activator.CreateInstance(nodeType); + } + catch (TargetInvocationException) + { + // The constructor threw. Registration is metadata only, so this is not fatal here. + return null; + } + catch (MemberAccessException) + { + return null; + } + catch (NotSupportedException) + { + return null; + } + } + private static void ScanMethodPins(MethodInfo method, NodeDefinition definition) { AddInstancePinForMethod(method, definition); diff --git a/tests/ImGui.NodeEditor.Tests/AttributeBasedNodeFactoryTests.cs b/tests/ImGui.NodeEditor.Tests/AttributeBasedNodeFactoryTests.cs index 550032c5..009c1b6f 100644 --- a/tests/ImGui.NodeEditor.Tests/AttributeBasedNodeFactoryTests.cs +++ b/tests/ImGui.NodeEditor.Tests/AttributeBasedNodeFactoryTests.cs @@ -298,6 +298,71 @@ public void CreateNode_CarriesEachPinsDeclaredConnectionCapacity() "The instance output is there to be chained onward, by as many nodes as want it."); } + /// + /// Every default in the shipped node library is written as a C# initializer rather than as + /// [InputPin(DefaultValue = ...)], so a menu or inspector built from the definitions saw + /// null for all of them. The factory reads the initializer off a prototype instance instead. + /// + [TestMethod] + public void RegisterNodeType_ReadsAnInputDefaultFromItsPropertyInitializer() + { + AttributeBasedNodeFactory factory = Factory; + factory.RegisterNodeType(); + + NodeDefinition definition = Registered(factory.GetNodeDefinition(typeof(InitializedDefaultsNode))); + + Assert.AreEqual(128.0, Input(definition, "Threshold").DefaultValue); + Assert.AreEqual("unnamed", Input(definition, "Label").DefaultValue); + Assert.AreEqual(7, Input(definition, "Count").DefaultValue, "A field initializer is a default too."); + } + + [TestMethod] + public void RegisterNodeType_PrefersTheAttributeDefaultOverTheInitializer() + { + AttributeBasedNodeFactory factory = Factory; + factory.RegisterNodeType(); + + NodeDefinition definition = Registered(factory.GetNodeDefinition(typeof(InitializedDefaultsNode))); + + Assert.AreEqual( + 50.0, + Input(definition, "AreaMin").DefaultValue, + "An explicit DefaultValue is the author saying what it is, initializer or not."); + } + + /// + /// A property with neither form reports what the node will actually hold when it is constructed, + /// which for a value type is its zero rather than null. + /// + [TestMethod] + public void RegisterNodeType_ReportsTheZeroForAnUninitializedValueTypePin() + { + AttributeBasedNodeFactory factory = Factory; + factory.RegisterNodeType(); + + NodeDefinition definition = Registered(factory.GetNodeDefinition(typeof(InitializedDefaultsNode))); + + Assert.AreEqual(0.0, Input(definition, "Untouched").DefaultValue); + } + + /// + /// Reading initializers means constructing the type, and registration must survive types that + /// cannot be constructed at all. Both fall back to the attribute alone, as before. + /// + [TestMethod] + public void RegisterNodeType_SurvivesATypeItCannotConstruct() + { + AttributeBasedNodeFactory factory = Factory; + + factory.RegisterNodeType(); + factory.RegisterNodeType(); + + Assert.IsNull( + Input(Registered(factory.GetNodeDefinition(typeof(UnconstructableNode))), "In").DefaultValue, + "A constructor that throws leaves the pin as it was."); + Assert.IsNotNull(factory.GetNodeDefinition(typeof(ConstructedNode)), "A type with no parameterless constructor still registers."); + } + [TestMethod] public void GetNodeDefinition_ReturnsNullForAnythingUnregistered() { @@ -319,6 +384,11 @@ public void GetNodeDefinition_ReturnsNullForAnythingUnregistered() private static NodeDefinition Registered(NodeDefinition? definition) => definition ?? throw new AssertFailedException("The definition was not registered."); + /// Finds the one input pin with the given display name. + private static PinDefinition Input(NodeDefinition definition, string displayName) => + definition.InputPins.SingleOrDefault(p => p.DisplayName == displayName) + ?? throw new AssertFailedException($"No input pin named '{displayName}'."); + [Node("Add Numbers", ColorHint = "#4488ff", Tags = ["math", "arithmetic"])] [NodeBehavior( ExecutionMode = NodeExecutionMode.OnExecution, @@ -393,6 +463,35 @@ public sealed class NotANode public int Value { get; set; } } + /// Mirrors how the shipped library writes its defaults: initializers, not attributes. + [Node("Initialized Defaults")] + public sealed class InitializedDefaultsNode + { + [InputPin("Threshold")] + public double Threshold { get; set; } = 128.0; + + [InputPin("Label")] + public string Label { get; set; } = "unnamed"; + + [InputPin("Count")] + public int Count = 7; + + [InputPin("AreaMin", DefaultValue = 50.0)] + public double AreaMin { get; set; } = 1.0; + + [InputPin("Untouched")] + public double Untouched { get; set; } + } + + [Node("Unconstructable")] + public sealed class UnconstructableNode + { + public UnconstructableNode() => throw new InvalidOperationException("Not from here."); + + [InputPin("In")] + public int In { get; set; } = 3; + } + public static class MathNodes { [Node("Clamp")] From 1f434e9489e805aedd1c4fbeb2d756d303c35d2b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 23 Sep 2026 02:53:54 +0000 Subject: [PATCH 2/2] [patch] Cover the prototype-reading paths, and drop two catches that cannot fire SonarCloud's quality gate failed the branch at 68.2% coverage on new code against an 80% floor. Collecting cobertura locally named the gaps exactly: the TargetInvocationException arm in ReadDeclaredDefault, the !constructible early return in CreatePrototype, and both of the extra catch arms. Two of those were dead rather than untested. CreatePrototype's guard turns away abstract and open-generic types and reference types with no public parameterless constructor before Activator.CreateInstance is reached, which is where MissingMethodException and MemberAccessException would have come from, and NotSupportedException only arises for types that cannot carry a [Node] attribute in the first place. Removing the two catches leaves the one case the guard lets through: a constructor that runs and throws. The reasoning is now in a remarks block rather than implied. The other two were real paths with no test. The cannot-construct test used ConstructedNode, whose pins are all outputs, so it never forced the prototype at all and its assertion was vacuous; it now uses a node with an input pin and a constructor argument, which reaches the early return. TouchyGetterNode covers the throwing getter. New-code coverage in the file is 26/26 lines measured, 0 uncovered. 103/103 tests pass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P6bYWX7oFj1YTAAw9M516w --- ImGui.NodeEditor/AttributeBasedNodeFactory.cs | 15 +++--- .../AttributeBasedNodeFactoryTests.cs | 51 +++++++++++++++++-- 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/ImGui.NodeEditor/AttributeBasedNodeFactory.cs b/ImGui.NodeEditor/AttributeBasedNodeFactory.cs index b8a513b3..803a8ca9 100644 --- a/ImGui.NodeEditor/AttributeBasedNodeFactory.cs +++ b/ImGui.NodeEditor/AttributeBasedNodeFactory.cs @@ -408,6 +408,13 @@ private static void ScanPins(Type nodeType, NodeDefinition definition) /// /// The type to construct. /// The instance, or null. + /// + /// The guard is what makes the one catch enough: an abstract or open-generic type, and a + /// reference type without a public parameterless constructor, are all turned away before + /// is reached, which is where its + /// MissingMethodException and MemberAccessException would have come from. What is + /// left is a constructor that runs and throws. + /// private static object? CreatePrototype(Type nodeType) { bool constructible = !nodeType.IsAbstract @@ -428,14 +435,6 @@ private static void ScanPins(Type nodeType, NodeDefinition definition) // The constructor threw. Registration is metadata only, so this is not fatal here. return null; } - catch (MemberAccessException) - { - return null; - } - catch (NotSupportedException) - { - return null; - } } private static void ScanMethodPins(MethodInfo method, NodeDefinition definition) diff --git a/tests/ImGui.NodeEditor.Tests/AttributeBasedNodeFactoryTests.cs b/tests/ImGui.NodeEditor.Tests/AttributeBasedNodeFactoryTests.cs index 009c1b6f..993212b2 100644 --- a/tests/ImGui.NodeEditor.Tests/AttributeBasedNodeFactoryTests.cs +++ b/tests/ImGui.NodeEditor.Tests/AttributeBasedNodeFactoryTests.cs @@ -346,21 +346,41 @@ public void RegisterNodeType_ReportsTheZeroForAnUninitializedValueTypePin() } /// - /// Reading initializers means constructing the type, and registration must survive types that - /// cannot be constructed at all. Both fall back to the attribute alone, as before. + /// Reading initializers means constructing the type, and there are two ways that does not + /// happen: the type takes constructor arguments, so it is never attempted, or its constructor + /// throws when it is. Registration has to survive both, reporting no default rather than failing. /// [TestMethod] public void RegisterNodeType_SurvivesATypeItCannotConstruct() { AttributeBasedNodeFactory factory = Factory; - factory.RegisterNodeType(); + factory.RegisterNodeType(); factory.RegisterNodeType(); + Assert.IsNull( + Input(Registered(factory.GetNodeDefinition(typeof(ParameterisedNode))), "Factor").DefaultValue, + "A type that takes constructor arguments has no prototype to read an initializer off."); Assert.IsNull( Input(Registered(factory.GetNodeDefinition(typeof(UnconstructableNode))), "In").DefaultValue, "A constructor that throws leaves the pin as it was."); - Assert.IsNotNull(factory.GetNodeDefinition(typeof(ConstructedNode)), "A type with no parameterless constructor still registers."); + } + + /// + /// A property that refuses to be read before it is written is a real shape, and reading the + /// prototype must not turn it into a registration failure. + /// + [TestMethod] + public void RegisterNodeType_SurvivesAPinWhoseGetterThrows() + { + AttributeBasedNodeFactory factory = Factory; + + factory.RegisterNodeType(); + + NodeDefinition definition = Registered(factory.GetNodeDefinition(typeof(TouchyGetterNode))); + + Assert.IsNull(Input(definition, "Fragile").DefaultValue, "A getter that throws has no default to report."); + Assert.AreEqual(4, Input(definition, "Sturdy").DefaultValue, "Its neighbour is still read."); } [TestMethod] @@ -492,6 +512,29 @@ public sealed class UnconstructableNode public int In { get; set; } = 3; } + /// Takes a constructor argument, so there is nothing to construct a prototype from. + [Node("Parameterised")] + public sealed class ParameterisedNode(double scale) + { + [InputPin("Factor")] + public double Factor { get; set; } = scale; + } + + /// A property that refuses to be read until it has been written. + [Node("Touchy")] + public sealed class TouchyGetterNode + { + [InputPin("Fragile")] + public string? Fragile + { + get => field ?? throw new InvalidOperationException("Set me before reading me."); + set; + } + + [InputPin("Sturdy")] + public int Sturdy { get; set; } = 4; + } + public static class MathNodes { [Node("Clamp")]