Type-check pins when creating a link

ImGui.NodeEditor/NodeEditorEngine.cs

@@ -7,6 +7,7 @@
 using System.Linq;
 using System.Numerics;
 using ktsu.ForceDirectedLayout;
+using ktsu.NodeGraph;
 
 /// <summary>
 /// Core business logic for the node editor - completely independent of ImNodes.
@@ -539,6 +540,12 @@
 	/// <summary>
 	/// Attempt to create a link between two pins.
 	/// </summary>
+	/// <remarks>
+	/// A pair that both declare a <see cref="Pin.DataType"/> must also be type-compatible under
+	/// <see cref="PinTypeUtilities.CanConnect"/>, and the returned message names both types when
+	/// they are not, so a host can say why the link was refused rather than only that it was.
+	/// A pin with no declared type is untyped and is not type-checked.
+	/// </remarks>
 	public LinkCreationResult TryCreateLink(int fromPinId, int toPinId)
 	{
 		Pin? fromPin = FindPin(fromPinId);
@@ -557,6 +564,18 @@
 		Pin outputPin = fromPin.Direction == PinDirection.Output ? fromPin : toPin;
 		Pin inputPin = fromPin.Direction == PinDirection.Input ? fromPin : toPin;
 
+		// Whether the value can survive the trip is a property of the pair itself, like direction,
+		// so it is settled before the graph's own state is consulted. Only a pair that both declare
+		// a type is checked: a null DataType is an untyped pin, which PinSpec documents as a
+		// supported way to describe one, and CanConnect answers false for a null rather than
+		// "unknown" - so asking it about an untyped pin would refuse every link the pin appears in.
+		if (outputPin.DataType is Type outputType
+			&& inputPin.DataType is Type inputType
+			&& !PinTypeUtilities.CanConnect(outputType, inputType))
+		{
+			return new LinkCreationResult(false, $"Cannot connect {outputType.Name} output '{outputPin.EffectiveDisplayName}' to {inputType.Name} input '{inputPin.EffectiveDisplayName}'");
+		}
+
 		if (links.Any(l => l.OutputPinId == outputPin.Id && l.InputPinId == inputPin.Id))
 		{
 			return new LinkCreationResult(false, "Those pins are already linked");
@@ -620,7 +639,7 @@
 
 		// A removed node's pins are gone, so what they held and where they were measured goes with
 		// them. Without this both tables grow for the life of the process.
 		foreach (Pin pin in node.InputPins.Concat(node.OutputPins))
 		{
 			pinValues.Forget(pin.Id);
 			pinValueAccessors.Remove(pin.Id);

examples/ImGuiAppDemo/Demos/CleanImNodesDemo.cs

@@ -677,8 +677,11 @@ private void CreateDemoData()
 		Node setNumber1 = nodeFactory.CreateNode<SetNumberNode>(new Vector2(450, 100));
 		Node splitNumber2 = nodeFactory.CreateNode<SplitNumberNode>(new Vector2(650, 100));
 
-		// Mathematical operation chain with data mutation
+		// Mathematical operation chain with data mutation. Add takes two doubles, so each operand
+		// reaches it through a Split rather than straight off a Make: a Make's output is the
+		// NumberData structure, and TryCreateLink refuses that into a double parameter.
 		Node makeNumber2 = nodeFactory.CreateNode<MakeNumberNode>(new Vector2(50, 250));
+		Node splitNumber3 = nodeFactory.CreateNode<SplitNumberNode>(new Vector2(150, 250));
 		Node addNode = nodeFactory.CreateMethodNode(addMethod, new Vector2(250, 250));
 		Node setNumber2 = nodeFactory.CreateNode<SetNumberNode>(new Vector2(450, 250));
 
@@ -696,9 +699,11 @@ private void CreateDemoData()
 		engine.TryCreateLink(splitNumber1.OutputPins[1].Id, setNumber1.InputPins[1].Id); // Absolute value as new value
 		engine.TryCreateLink(setNumber1.OutputPins[0].Id, splitNumber2.InputPins[0].Id); // Updated data to final Split
 
-		// Math operation with data update: Make → Add → Set (update existing with result)
-		engine.TryCreateLink(makeNumber1.OutputPins[0].Id, addNode.InputPins[0].Id);
-		engine.TryCreateLink(makeNumber2.OutputPins[0].Id, addNode.InputPins[1].Id);
+		// Math operation with data update: Make → Split → Add → Set (update existing with result).
+		// Each operand is the Split's double Value, not the Make's NumberData.
+		engine.TryCreateLink(makeNumber2.OutputPins[0].Id, splitNumber3.InputPins[0].Id);
+		engine.TryCreateLink(splitNumber1.OutputPins[0].Id, addNode.InputPins[0].Id);
+		engine.TryCreateLink(splitNumber3.OutputPins[0].Id, addNode.InputPins[1].Id);
 		engine.TryCreateLink(makeNumber1.OutputPins[0].Id, setNumber2.InputPins[0].Id); // Original data
 		engine.TryCreateLink(addNode.OutputPins[0].Id, setNumber2.InputPins[1].Id); // Add result as new value
 

tests/ImGui.NodeEditor.Tests/NodeEditorEngineTests.cs

@@ -125,6 +125,89 @@ public void TryCreateLink_RefusesTwoPinsOfTheSameDirection()
 		Assert.Contains("Cannot connect", result.Message);
 	}
 
+	/// <summary>
+	/// The defect: direction, duplicates, limits and self-links were all checked, but the pins'
+	/// declared types never were, so a string output fed an int input and the result said Success.
+	/// </summary>
+	[TestMethod]
+	public void TryCreateLink_RefusesPinsWhoseDeclaredTypesCannotConnect()
+	{
+		Node source = engine.CreateNodeFromSpecs(new Vector2(0, 0), "Source", [], [new PinSpec("Text", typeof(string))]);
+		Node target = engine.CreateNodeFromSpecs(new Vector2(300, 0), "Target", [new PinSpec("Count", typeof(int))], []);
+
+		LinkCreationResult result = engine.TryCreateLink(source.OutputPins[0].Id, target.InputPins[0].Id);
+
+		Assert.IsFalse(result.Success, "A string output should not feed an int input.");
+		Assert.IsNull(result.Link);
+		Assert.IsEmpty(engine.Links);
+
+		// The message has to name the mismatch, not merely report a refusal, so a host can say
+		// which end is wrong instead of only that the drag failed.
+		Assert.Contains(nameof(String), result.Message);
+		Assert.Contains(nameof(Int32), result.Message);
+		Assert.Contains("Text", result.Message);
+		Assert.Contains("Count", result.Message);
+	}
+
+	/// <summary>
+	/// The over-correction: refusing every typed pair, or reusing the direction check's own
+	/// notion of compatibility, would pass the test above while breaking the graphs that work.
+	/// A widening numeric conversion is compatible under
+	/// <see cref="ktsu.NodeGraph.PinTypeUtilities.CanConnect"/> and must stay connectable.
+	/// </summary>
+	[TestMethod]
+	public void TryCreateLink_ConnectsPinsWhoseDeclaredTypesAreCompatible()
+	{
+		Node source = engine.CreateNodeFromSpecs(new Vector2(0, 0), "Source", [], [new PinSpec("Value", typeof(int))]);
+		Node target = engine.CreateNodeFromSpecs(new Vector2(300, 0), "Target", [new PinSpec("Scale", typeof(double))], []);
+
+		LinkCreationResult result = engine.TryCreateLink(source.OutputPins[0].Id, target.InputPins[0].Id);
+
+		Assert.IsTrue(result.Success, result.Message);
+		Assert.HasCount(1, engine.Links);
+	}
+
+	/// <summary>
+	/// <see cref="PinSpec"/> documents a null <c>DataType</c> as an untyped pin, and
+	/// <c>CanConnect</c> answers false for a null rather than "unknown" - so a check that asked it
+	/// about one would refuse every link an untyped pin appears in, which is most of this suite and
+	/// every caller of the name-only <c>CreateNode</c> overloads.
+	/// </summary>
+	[TestMethod]
+	public void TryCreateLink_DoesNotTypeCheckAnUntypedPin()
+	{
+		Node untyped = engine.CreateNode(new Vector2(0, 0), "Untyped", [], ["Out"]);
+		Node typed = engine.CreateNodeFromSpecs(new Vector2(300, 0), "Typed", [new PinSpec("Count", typeof(int))], []);
+
+		// One end declared, one end not: still a link, since there is no declared pair to compare.
+		LinkCreationResult onlyInputTyped = engine.TryCreateLink(untyped.OutputPins[0].Id, typed.InputPins[0].Id);
+
+		Node alsoUntyped = engine.CreateNode(new Vector2(300, 200), "AlsoUntyped", ["In"], []);
+		LinkCreationResult neitherTyped = engine.TryCreateLink(untyped.OutputPins[0].Id, alsoUntyped.InputPins[0].Id);
+
+		Assert.IsTrue(onlyInputTyped.Success, onlyInputTyped.Message);
+		Assert.IsTrue(neitherTyped.Success, neitherTyped.Message);
+	}
+
+	/// <summary>
+	/// An execution pin carries no value and is typed <c>void</c>, which
+	/// <c>CanConnect</c> allows only against another <c>void</c>. Wiring execution flow into a data
+	/// input is the same class of mistake as the string-to-int case and is refused with it.
+	/// </summary>
+	[TestMethod]
+	public void TryCreateLink_RefusesExecutionFlowIntoADataPin()
+	{
+		Node source = engine.CreateNodeFromSpecs(new Vector2(0, 0), "Source", [], [new PinSpec("Then", typeof(void))]);
+		Node data = engine.CreateNodeFromSpecs(new Vector2(300, 0), "Data", [new PinSpec("Count", typeof(int))], []);
+		Node execution = engine.CreateNodeFromSpecs(new Vector2(300, 200), "Execution", [new PinSpec("Run", typeof(void))], []);
+
+		LinkCreationResult intoData = engine.TryCreateLink(source.OutputPins[0].Id, data.InputPins[0].Id);
+		LinkCreationResult intoExecution = engine.TryCreateLink(source.OutputPins[0].Id, execution.InputPins[0].Id);
+
+		Assert.IsFalse(intoData.Success, "Execution flow should not feed a data pin.");
+		Assert.IsTrue(intoExecution.Success, intoExecution.Message);
+	}
+
 	[TestMethod]
 	public void TryCreateLink_RefusesASecondConnectionIntoOneInput()
 	{