Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 68 additions & 2 deletions ImGui.NodeEditor/AttributeBasedNodeFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,10 @@ private static void ScanPins(Type nodeType, NodeDefinition definition)
IEnumerable<MemberInfo> 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<object?> prototype = new(() => CreatePrototype(nodeType));

foreach (MemberInfo? member in members)
{
// Check for any pin attribute (input, output, execution input, execution output)
Expand Down Expand Up @@ -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
Expand All @@ -371,6 +376,67 @@ private static void ScanPins(Type nodeType, NodeDefinition definition)
definition.OutputPins.Sort((a, b) => a.Order.CompareTo(b.Order));
}

/// <summary>
/// Reads what a pin's member holds on a freshly constructed instance of its declaring type,
/// which is the value its C# initializer wrote.
/// </summary>
/// <param name="pin">The pin whose member to read.</param>
/// <param name="prototype">The prototype instance, or null if the type could not be constructed.</param>
/// <returns>The declared default, or null if there is no prototype or the member cannot be read.</returns>
private static object? ReadDeclaredDefault(PinDefinition pin, Lazy<object?> 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;
}
}

/// <summary>
/// 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.
/// </summary>
/// <param name="nodeType">The type to construct.</param>
/// <returns>The instance, or null.</returns>
/// <remarks>
/// 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
/// <see cref="Activator.CreateInstance(Type)"/> is reached, which is where its
/// <c>MissingMethodException</c> and <c>MemberAccessException</c> would have come from. What is
/// left is a constructor that runs and throws.
/// </remarks>
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;
}
}

private static void ScanMethodPins(MethodInfo method, NodeDefinition definition)
{
AddInstancePinForMethod(method, definition);
Expand Down
142 changes: 142 additions & 0 deletions tests/ImGui.NodeEditor.Tests/AttributeBasedNodeFactoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,91 @@ public void CreateNode_CarriesEachPinsDeclaredConnectionCapacity()
"The instance output is there to be chained onward, by as many nodes as want it.");
}

/// <summary>
/// Every default in the shipped node library is written as a C# initializer rather than as
/// <c>[InputPin(DefaultValue = ...)]</c>, 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.
/// </summary>
[TestMethod]
public void RegisterNodeType_ReadsAnInputDefaultFromItsPropertyInitializer()
{
AttributeBasedNodeFactory factory = Factory;
factory.RegisterNodeType<InitializedDefaultsNode>();

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<InitializedDefaultsNode>();

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.");
}

/// <summary>
/// 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.
/// </summary>
[TestMethod]
public void RegisterNodeType_ReportsTheZeroForAnUninitializedValueTypePin()
{
AttributeBasedNodeFactory factory = Factory;
factory.RegisterNodeType<InitializedDefaultsNode>();

NodeDefinition definition = Registered(factory.GetNodeDefinition(typeof(InitializedDefaultsNode)));

Assert.AreEqual(0.0, Input(definition, "Untouched").DefaultValue);
}

/// <summary>
/// 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.
/// </summary>
[TestMethod]
public void RegisterNodeType_SurvivesATypeItCannotConstruct()
{
AttributeBasedNodeFactory factory = Factory;

factory.RegisterNodeType<ParameterisedNode>();
factory.RegisterNodeType<UnconstructableNode>();

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.");
}

/// <summary>
/// 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.
/// </summary>
[TestMethod]
public void RegisterNodeType_SurvivesAPinWhoseGetterThrows()
{
AttributeBasedNodeFactory factory = Factory;

factory.RegisterNodeType<TouchyGetterNode>();

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]
public void GetNodeDefinition_ReturnsNullForAnythingUnregistered()
{
Expand All @@ -319,6 +404,11 @@ public void GetNodeDefinition_ReturnsNullForAnythingUnregistered()
private static NodeDefinition Registered(NodeDefinition? definition) =>
definition ?? throw new AssertFailedException("The definition was not registered.");

/// <summary>Finds the one input pin with the given display name.</summary>
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,
Expand Down Expand Up @@ -393,6 +483,58 @@ public sealed class NotANode
public int Value { get; set; }
}

/// <summary>Mirrors how the shipped library writes its defaults: initializers, not attributes.</summary>
[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;
}

/// <summary>Takes a constructor argument, so there is nothing to construct a prototype from.</summary>
[Node("Parameterised")]
public sealed class ParameterisedNode(double scale)
{
[InputPin("Factor")]
public double Factor { get; set; } = scale;
}

/// <summary>A property that refuses to be read until it has been written.</summary>
[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")]
Expand Down
Loading