Core - Fix node script and pin bugs found by the tests (#911)

* Core - Keep nodes of missing plugins and the connection an input reads from in node scripts

- Saving a script keeps nodes whose plugin is missing, and their connections, so they come back once the plugin is available again
- Loading keeps the first connection of an input, the one it reads from
- Disabling a node provider only removes the nodes it created, not those of another provider registering the same type
- RemoveNode only raises NodeRemoved for nodes that were in the script

* Core - Fix pin connections, null values and failing nodes

- Connecting pins that are already connected no longer adds a second connection
- Untyped pins turn null into the default value for value types, as intended
- A new InputPin<T> reads its connection right away instead of after its first reset
- Matching a specific enum to a pin of any enum now depends on forgivingEnumMatching
- A node that fails to evaluate sets its outputs to their default values, so it is evaluated once per run and old values don't flow on

* Core - Round Numeric to long and clamp it to the range of an sbyte

src/Artemis.Core.Tests/Services/ProfileImportExportTests.cs

@@ -246,7 +246,7 @@ public class ProfileImportExportTests : ProfileServiceTestBase
     }
 
     [Fact]
-    public async Task ImportProfile_WithUnavailableNodes_DropsThemFromTheActivationCondition()
+    public async Task ImportProfile_WithUnavailableNodes_KeepsThemInTheActivationCondition()
     {
         TestNodeProvider nodes = EnableFeature(new TestNodeProvider());
         ProfileConfiguration original = CreateSavedProfile("Test profile");
@@ -257,17 +257,15 @@ public class ProfileImportExportTests : ProfileServiceTestBase
 
         ProfileConfiguration imported = await ProfileService.ImportProfile(export, General, true, false, null);
 
-        // Pinned as-is: the node can't be loaded without its plugin, and because importing saves the category right
-        // away, the node and its connection are removed from the stored activation condition for good. Unlike missing
-        // brushes and effects, enabling the plugin afterwards doesn't bring it back, and the imported profile now
-        // activates unconditionally
+        // The node can't be loaded without its plugin, but importing saves the category right away and that keeps it, so
+        // enabling the plugin afterwards brings it back like missing brushes and effects
         imported.ActivationCondition.Nodes.OfType<TestBooleanNode>().ShouldBeEmpty();
-        imported.Entity.ProfileConfiguration.ActivationCondition!.Nodes.ShouldNotContain(n => n.Type == nameof(TestBooleanNode));
+        imported.Entity.ProfileConfiguration.ActivationCondition!.Nodes.ShouldContain(n => n.Type == nameof(TestBooleanNode));
         EnableFeature(new TestNodeProvider());
         ProfileConfiguration reloaded = CreateNewProfileService().ProfileCategories.Single(c => c.Name == "General").ProfileConfigurations
             .Single(c => c.ProfileId == imported.ProfileId);
-        reloaded.ActivationCondition.Nodes.OfType<TestBooleanNode>().ShouldBeEmpty();
-        reloaded.ActivationCondition.ExitNodeConnected.ShouldBeFalse();
+        reloaded.ActivationCondition.Nodes.OfType<TestBooleanNode>().ShouldHaveSingleItem();
+        reloaded.ActivationCondition.ExitNodeConnected.ShouldBeTrue();
     }
 
     [Fact]

src/Artemis.Core.Tests/VisualScripting/NodeScriptStorageTests.cs

@@ -214,10 +214,9 @@ public class NodeScriptStorageTests : ScriptTestBase
     }
 
     [Fact]
-    public void NodesOfAMissingPlugin_AreLostWhenTheScriptIsSavedAgain()
+    public void NodesOfAMissingPlugin_AreKeptWhenTheScriptIsSavedAgain()
     {
-        // Pinned as-is: a script loaded while a plugin is missing forgets that plugin's nodes the next time it is saved.
-        // Saving a profile while a plugin is disabled permanently removes its nodes from the profile's scripts.
+        // So they come back once the plugin is available again
         TestPlugin missingPlugin = new();
         ScriptTestNodeProvider missingNodes = missingPlugin.AddFeature(new ScriptTestNodeProvider());
         NodeScript<string> script = CreateScript<string>();
@@ -227,11 +226,13 @@ public class NodeScriptStorageTests : ScriptTestBase
         missingPlugin.Dispose();
 
         NodeScript<string> loaded = Load<string>(script.Entity);
-        loaded.Entity.Nodes.Count.ShouldBe(2);
+        loaded.Nodes.ShouldHaveSingleItem().IsExitNode.ShouldBeTrue();
         loaded.Save();
 
-        loaded.Entity.Nodes.ShouldHaveSingleItem().IsExitNode.ShouldBeTrue();
-        loaded.Entity.Connections.ShouldBeEmpty();
+        loaded.Entity.Nodes.Count.ShouldBe(2);
+        loaded.Entity.Nodes.ShouldContain(n => n.Id == node.Id);
+        NodeConnectionEntity connection = loaded.Entity.Connections.ShouldHaveSingleItem();
+        new[] {connection.SourceNode, connection.TargetNode}.ShouldContain(node.Id);
     }
 
     [Fact]
@@ -301,10 +302,9 @@ public class NodeScriptStorageTests : ScriptTestBase
     }
 
     [Fact]
-    public void InputConnectedToTwoOutputs_KeepsOnlyTheLastConnectionWhenLoading()
+    public void InputConnectedToTwoOutputs_KeepsTheConnectionItReadsFromWhenLoading()
     {
-        // Pinned as-is: in code an input can be connected to several outputs, it then reads from the first one. Loading
-        // keeps only the last connection, so after loading the script reads from a different node than before.
+        // In code an input can be connected to several outputs, it then reads from the first one. Loading keeps that one
         NodeScript<Numeric> script = CreateScript<Numeric>();
         TestNumericNode first = Nodes.AddNode<TestNumericNode>(script);
         TestNumericNode second = Nodes.AddNode<TestNumericNode>(script);
@@ -318,8 +318,8 @@ public class NodeScriptStorageTests : ScriptTestBase
         NodeScript<Numeric> loaded = SaveAndLoad(script);
         loaded.Run();
 
-        loaded.GetExitNode().Input.ConnectedTo.ShouldHaveSingleItem().Node.Id.ShouldBe(second.Id);
-        loaded.Result.ShouldBe(new Numeric(2));
+        loaded.GetExitNode().Input.ConnectedTo.ShouldHaveSingleItem().Node.Id.ShouldBe(first.Id);
+        loaded.Result.ShouldBe(new Numeric(1));
     }
 
     [Fact]

src/Artemis.Core.Tests/VisualScripting/NodeScriptTests.cs

@@ -267,17 +267,15 @@ public class NodeScriptTests : ScriptTestBase
     }
 
     [Fact]
-    public void RemoveNode_RaisesNodeRemoved_EvenForANodeThatIsNotInTheScript()
+    public void RemoveNode_OfANodeThatIsNotInTheScript_DoesNotRaiseNodeRemoved()
     {
-        // Pinned as-is: removing a node that was never added still tells listeners it was removed, an editor listening
-        // to the event could remove a view model that belongs to another script
         NodeScript<bool> script = CreateScript<bool>();
         int raised = 0;
         script.NodeRemoved += (_, _) => raised++;
 
         script.RemoveNode(new TestNode());
 
-        raised.ShouldBe(1);
+        raised.ShouldBe(0);
     }
 
     [Fact]
@@ -311,10 +309,8 @@ public class NodeScriptTests : ScriptTestBase
     }
 
     [Fact]
-    public void DisablingTheNodeProvider_AlsoRemovesNodesOfTheSameTypeFromAnotherProvider()
+    public void DisablingTheNodeProvider_LeavesNodesOfTheSameTypeFromAnotherProvider()
     {
-        // Pinned as-is: the script removes nodes by their type, not by the provider that created them. When two enabled
-        // providers register the same node type, disabling one also removes the nodes created by the other.
         TestPlugin otherPlugin = new();
         TestNodeProvider otherNodes = otherPlugin.AddFeature(new TestNodeProvider());
         NodeScript<bool> script = CreateScript<bool>();
@@ -324,7 +320,7 @@ public class NodeScriptTests : ScriptTestBase
         otherPlugin.Dispose();
 
         script.Nodes.ShouldNotContain(fromOtherProvider);
-        script.Nodes.ShouldNotContain(fromStillEnabledProvider);
+        script.Nodes.ShouldContain(fromStillEnabledProvider);
     }
 
     [Fact]

src/Artemis.Core.Tests/VisualScripting/NodeTests.cs

@@ -218,10 +218,9 @@ public class NodeTests : ScriptTestBase
     }
 
     [Fact]
-    public void FailingNode_InAScript_LeavesTheResultAtTheLastValueItProduced()
+    public void FailingNode_InAScript_SetsItsOutputsToTheirDefaultValues()
     {
-        // Pinned as-is: when a node fails, its output keeps the value from the last run that worked. The script carries on
-        // with that old value and only the broken state of the node shows something is wrong.
+        // Rather than letting the value from the last run that worked flow on
         NodeScript<Numeric> script = CreateScript<Numeric>();
         TestThrowingNode node = ScriptNodes.AddNode<TestThrowingNode>(script);
         script.GetExitNode().Input.ConnectTo(node.Output);
@@ -232,15 +231,15 @@ public class NodeTests : ScriptTestBase
         node.FailsOn = TestThrowingNode.FailureMode.Evaluate;
         script.Run();
 
-        script.Result.ShouldBe(new Numeric(5));
+        script.Result.ShouldBe(new Numeric(0));
         node.BrokenState.ShouldBe("Failed to evaluate");
         script.GetExitNode().BrokenState.ShouldBeNull();
     }
 
     [Fact]
-    public void FailingNode_InAScript_IsEvaluatedAgainForEveryInputReadingIt()
+    public void FailingNode_InAScript_IsEvaluatedOncePerRun()
     {
-        // A failing node never marks its outputs as evaluated, so every pin reading from it tries again within one run
+        // A failing node sets its outputs to their default values, so the other pins reading from it don't try again
         NodeScript<Numeric> script = CreateScript<Numeric>();
         TestNode failing = new() {OnEvaluate = _ => throw new InvalidOperationException()};
         OutputPin<Numeric> output = failing.CreateOutputPin<Numeric>();
@@ -252,7 +251,7 @@ public class NodeTests : ScriptTestBase
 
         script.Run();
 
-        failing.Evaluations.ShouldBe(2);
+        failing.Evaluations.ShouldBe(1);
         sum.BrokenState.ShouldBeNull();
     }
 

src/Artemis.Core.Tests/VisualScripting/NumericTests.cs

@@ -99,14 +99,12 @@ public class NumericTests
     }
 
     [Fact]
-    public void ToLong_CutsOffTheDecimals_UnlikeTheOtherWholeNumberConversions()
+    public void ToLong_Rounds_LikeTheOtherWholeNumberConversions()
     {
-        // Pinned as-is: turning a Numeric into a long cuts off the decimals, while turning it into an int rounds, and so
-        // does the IConvertible conversion to a long
         long cast = new Numeric(2.7f);
         int castToInt = new Numeric(2.7f);
 
-        cast.ShouldBe(2);
+        cast.ShouldBe(3);
         castToInt.ShouldBe(3);
         AsConvertible(2.7f).ToInt64(null).ShouldBe(3);
     }
@@ -123,12 +121,11 @@ public class NumericTests
     }
 
     [Fact]
-    public void ToSByte_KeepsTheValueBetween0And255_WhichDoesNotFitAnSByte()
+    public void ToSByte_KeepsTheValueWithinTheRangeOfAnSByte()
     {
-        // Pinned as-is: an sbyte holds -128 to 127, but the conversion clamps to 0-255 like a byte does. Anything above
-        // 127 wraps around (200 becomes -56) and negative numbers become 0.
-        AsConvertible(200f).ToSByte(null).ShouldBe((sbyte) -56);
-        AsConvertible(-5f).ToSByte(null).ShouldBe((sbyte) 0);
+        AsConvertible(200f).ToSByte(null).ShouldBe(sbyte.MaxValue);
+        AsConvertible(-200f).ToSByte(null).ShouldBe(sbyte.MinValue);
+        AsConvertible(-5f).ToSByte(null).ShouldBe((sbyte) -5);
         AsConvertible(100f).ToSByte(null).ShouldBe((sbyte) 100);
     }
 
@@ -175,8 +172,9 @@ public class NumericTests
     [Fact]
     public void PassingANumericToConvert_PicksTheByteVersion()
     {
-        // Pinned as-is: Numeric converts implicitly to byte, int, long, float, double and decimal, so the compiler sends
-        // Convert.ToInt32(numeric) and friends to their byte versions. The value is clamped to 0-255 and loses its
+        // Known limitation: Numeric converts implicitly to byte, int, long, float, double and decimal, so the compiler
+        // sends Convert.ToInt32(numeric) and friends to their byte versions. Fixing this means removing implicit
+        // conversions, which would break plugins that rely on them. The value is clamped to 0-255 and loses its
         // decimals before it is converted. Passing it as an object uses IConvertible instead.
         Numeric numeric = new(2.5f);
 

src/Artemis.Core.Tests/VisualScripting/PinTests.cs

@@ -105,17 +105,15 @@ public class PinTests
     }
 
     [Fact]
-    public void ConnectTo_TheSamePinTwice_AddsTheConnectionTwiceOnOneSide()
+    public void ConnectTo_TheSamePinTwice_ConnectsThemOnce()
     {
-        // Pinned as-is: connecting pins that are already connected adds a second connection to the pin it is called on,
-        // while the other pin keeps one. Disconnecting once removes both again.
         OutputPin<Numeric> output = _source.CreateOutputPin<Numeric>();
         InputPin<Numeric> input = _target.CreateInputPin<Numeric>();
 
         input.ConnectTo(output);
         input.ConnectTo(output);
 
-        input.ConnectedTo.ShouldBe([output, output]);
+        input.ConnectedTo.ShouldBe([output]);
         output.ConnectedTo.ShouldBe([input]);
 
         input.DisconnectFrom(output);
@@ -215,14 +213,15 @@ public class PinTests
     }
 
     [Fact]
-    public void IsTypeCompatible_MatchesEnums_EvenWhenNotForgiving()
+    public void IsTypeCompatible_OnlyMatchesASpecificEnumToAnyEnumWhenForgiving()
     {
-        // Pinned as-is: the forgiving enum check has no effect, a specific enum type already counts as an Enum without it
         InputPin input = _target.CreateInputPin(typeof(Enum));
         OutputPin output = _source.CreateOutputPin(typeof(TestEnum));
 
-        input.IsTypeCompatible(typeof(TestEnum), false).ShouldBeTrue();
-        output.IsTypeCompatible(typeof(Enum), false).ShouldBeTrue();
+        input.IsTypeCompatible(typeof(TestEnum), false).ShouldBeFalse();
+        output.IsTypeCompatible(typeof(Enum), false).ShouldBeFalse();
+        input.IsTypeCompatible(typeof(TestEnum), true).ShouldBeTrue();
+        output.IsTypeCompatible(typeof(Enum), true).ShouldBeTrue();
     }
 
     [Fact]
@@ -390,25 +389,19 @@ public class PinTests
     }
 
     [Fact]
-    public void UntypedOutput_ForAValueType_AcceptsNull()
+    public void UntypedOutput_ForAValueType_TurnsNullIntoTheDefaultValue()
     {
-        // Pinned as-is: the setter means to turn null into the default value for value types (see its comment), but
-        // then stores null anyway. A typed input reading the pin gets its default value, an untyped one turns the null
-        // into the default value itself.
         OutputPin output = _source.CreateOutputPin(typeof(Numeric));
         output.Value = new Numeric(3);
 
         output.Value = null;
 
-        output.Value.ShouldBeNull();
+        output.Value.ShouldBe(new Numeric(0));
     }
 
     [Fact]
-    public void NewTypedInput_IgnoresItsConnectionUntilItIsReset()
+    public void NewTypedInput_ReadsItsConnection_LikeAnUntypedInput()
     {
-        // Pinned as-is: a new InputPin<T> counts as evaluated with its default value, so it only reads the pin it is
-        // connected to after a reset. Running a script resets every pin first, so this only shows outside a script.
-        // Untyped input pins don't do this.
         OutputPin<Numeric> output = _source.CreateOutputPin<Numeric>();
         InputPin<Numeric> typed = _target.CreateInputPin<Numeric>();
         InputPin untyped = _target.CreateInputPin(typeof(Numeric));
@@ -416,12 +409,9 @@ public class PinTests
         untyped.ConnectTo(output);
         output.Value = new Numeric(5);
 
-        typed.IsEvaluated.ShouldBeTrue();
-        typed.Value.ShouldBe(new Numeric(0));
-        untyped.Value.ShouldBe(new Numeric(5));
-
-        typed.Reset();
+        typed.IsEvaluated.ShouldBeFalse();
         typed.Value.ShouldBe(new Numeric(5));
+        untyped.Value.ShouldBe(new Numeric(5));
     }
 
     [Fact]

src/Artemis.Core/Utilities/Numeric.cs

@@ -182,7 +182,7 @@ public readonly struct Numeric : IComparable<Numeric>, IConvertible
 
     public static implicit operator long(Numeric p)
     {
-        return (long) p._value;
+        return (long) MathF.Round(p._value, MidpointRounding.AwayFromZero);
     }
 
     public static bool operator >(Numeric a, Numeric b)
@@ -367,7 +367,7 @@ public readonly struct Numeric : IComparable<Numeric>, IConvertible
     /// <inheritdoc />
     public sbyte ToSByte(IFormatProvider? provider)
     {
-        return (sbyte) Math.Clamp(_value, 0, 255);
+        return (sbyte) Math.Clamp(_value, sbyte.MinValue, sbyte.MaxValue);
     }
 
     /// <inheritdoc />

src/Artemis.Core/VisualScripting/NodeScript.cs

@@ -22,7 +22,10 @@ public abstract class NodeScript : CorePropertyChanged, INodeScript
 
     private void NodeTypeStoreOnNodeTypeRemoved(object? sender, NodeTypeStoreEvent e)
     {
-        List<INode> nodes = Nodes.Where(n => n.GetType() == e.TypeRegistration.NodeData.Type).ToList();
+        // Match on the node data rather than the type, another provider may have registered the same type
+        List<INode> nodes = Nodes.Where(n => n.NodeData != null
+            ? n.NodeData == e.TypeRegistration.NodeData
+            : n.GetType() == e.TypeRegistration.NodeData.Type).ToList();
         foreach (INode node in nodes)
             RemoveNode(node);
     }
@@ -145,7 +148,8 @@ public abstract class NodeScript : CorePropertyChanged, INodeScript
     {
         lock (_nodes)
         {
-            _nodes.Remove(node);
+            if (!_nodes.Remove(node))
+                return;
         }
 
         NodeRemoved?.Invoke(this, new SingleValueEventArgs<INode>(node));
@@ -253,6 +257,8 @@ public abstract class NodeScript : CorePropertyChanged, INodeScript
     public void LoadConnections()
     {
         List<INode> nodes = Nodes.ToList();
+        // An input reads from its first connection, so when there are several the first one is kept
+        HashSet<IPin> connectedInputs = [];
         foreach (NodeConnectionEntity nodeConnectionEntity in Entity.Connections.OrderBy(p => p.SourcePinCollectionId))
         {
             INode? source = nodes.FirstOrDefault(n => n.Id == nodeConnectionEntity.SourceNode);
@@ -275,6 +281,9 @@ public abstract class NodeScript : CorePropertyChanged, INodeScript
             // Ensure the connection is valid
             if (sourcePin.Direction == targetPin.Direction)
                 continue;
+            IPin inputPin = targetPin.Direction == PinDirection.Input ? targetPin : sourcePin;
+            if (!connectedInputs.Add(inputPin))
+                continue;
 
             // Clear existing connections on input pins, we don't want none of that now
             if (targetPin.Direction == PinDirection.Input)
@@ -310,7 +319,16 @@ public abstract class NodeScript : CorePropertyChanged, INodeScript
     {
         Entity.Name = Name;
         Entity.Description = Description;
+
+        // Nodes whose type isn't registered couldn't be loaded, most likely because their plugin is missing or disabled.
+        // Keep them and their connections, so they come back once the plugin is available again
+        List<NodeEntity> unavailableNodes = Entity.Nodes.Where(e => !e.IsExitNode && Nodes.All(n => n.Id != e.Id) && NodeTypeStore.Get(e) == null).ToList();
+        List<NodeConnectionEntity> unavailableConnections = Entity.Connections
+            .Where(c => unavailableNodes.Any(n => n.Id == c.SourceNode || n.Id == c.TargetNode))
+            .ToList();
+
         Entity.Nodes.Clear();
+        Entity.Nodes.AddRange(unavailableNodes);
 
         foreach (INode node in Nodes)
         {
@@ -346,6 +364,7 @@ public abstract class NodeScript : CorePropertyChanged, INodeScript
 
         // Store connections
         Entity.Connections.Clear();
+        Entity.Connections.AddRange(unavailableConnections);
         foreach (INode node in Nodes)
         {
             SavePins(node, -1, node.Pins);

src/Artemis.Core/VisualScripting/Nodes/Node.cs

@@ -385,7 +385,13 @@ public abstract class Node : BreakableModel, INode
     /// <inheritdoc />
     public void TryEvaluate()
     {
-        TryOrBreak(Evaluate, "Failed to evaluate");
+        if (TryOrBreak(Evaluate, "Failed to evaluate"))
+            return;
+
+        // Don't let the values of the last successful run flow on, and don't try again for every pin that reads from this
+        // node. Setting a value marks the pin as evaluated until the next run
+        foreach (Pin pin in Pins.Concat(PinCollections.SelectMany(c => c)).OfType<Pin>().Where(p => p.Direction == PinDirection.Output))
+            pin.SetDefaultValue();
     }
 
     /// <summary>

src/Artemis.Core/VisualScripting/Pins/InputPin.cs

@@ -12,7 +12,7 @@ public sealed class InputPin<T> : Pin
     internal InputPin(INode node, string name)
         : base(node, name)
     {
-        Value = default;
+        _value = default;
         IsNumeric = typeof(T) == typeof(Numeric);
     }
 
@@ -149,15 +149,13 @@ public sealed class InputPin : Pin
 
         private set
         {
+            // We can't take null for value types so use the default value for that type
             if (Type.IsValueType && value == null)
-                // We can't take null for value types so set it to the default value for that type
-                _value = Type.GetDefault();
-            else if (value != null)
-                // If a value was given make sure it matches
-                if (!Type.IsInstanceOfType(value))
-                    throw new ArgumentException($"Value of type '{value.GetType().Name}' can't be assigned to a pin of type {Type.Name}.");
+                value = Type.GetDefault();
+            // If a value was given make sure it matches
+            else if (value != null && !Type.IsInstanceOfType(value))
+                throw new ArgumentException($"Value of type '{value.GetType().Name}' can't be assigned to a pin of type {Type.Name}.");
 
-            // Otherwise we're good and we can put a null here if it happens to be that
             _value = value;
             IsEvaluated = true;
             OnPropertyChanged(nameof(PinValue));

src/Artemis.Core/VisualScripting/Pins/OutputPin.cs

@@ -29,6 +29,11 @@ public sealed class OutputPin<T> : Pin
     /// <inheritdoc />
     public override PinDirection Direction => PinDirection.Output;
 
+    internal override void SetDefaultValue()
+    {
+        Value = default;
+    }
+
     private T? _value;
 
     /// <summary>
@@ -100,6 +105,11 @@ public sealed class OutputPin : Pin
     /// <inheritdoc />
     public override PinDirection Direction => PinDirection.Output;
 
+    internal override void SetDefaultValue()
+    {
+        Value = Type.GetDefault();
+    }
+
     private object? _value;
     private Type _type;
 
@@ -117,15 +127,13 @@ public sealed class OutputPin : Pin
         }
         set
         {
+            // We can't take null for value types so use the default value for that type
             if (Type.IsValueType && value == null)
-                // We can't take null for value types so set it to the default value for that type
-                _value = Type.GetDefault();
-            else if (value != null)
-                // If a value was given make sure it matches
-                if (!Type.IsInstanceOfType(value))
-                    throw new ArgumentException($"Value of type '{value.GetType().Name}' can't be assigned to a pin of type {Type.Name}.");
+                value = Type.GetDefault();
+            // If a value was given make sure it matches
+            else if (value != null && !Type.IsInstanceOfType(value))
+                throw new ArgumentException($"Value of type '{value.GetType().Name}' can't be assigned to a pin of type {Type.Name}.");
 
-            // Otherwise we're good and we can put a null here if it happens to be that
             _value = value;
             IsEvaluated = true;
             OnPropertyChanged(nameof(PinValue));

src/Artemis.Core/VisualScripting/Pins/Pin.cs

@@ -85,9 +85,19 @@ public abstract class Pin : CorePropertyChanged, IPin
         IsEvaluated = false;
     }
 
+    /// <summary>
+    ///     Sets the value of an output pin to the default value of its type, used when its node fails to evaluate
+    /// </summary>
+    internal virtual void SetDefaultValue()
+    {
+    }
+
     /// <inheritdoc />
     public void ConnectTo(IPin pin)
     {
+        if (_connectedTo.Contains(pin))
+            return;
+
         _connectedTo.Add(pin);
         if (!pin.ConnectedTo.Contains(this))
             pin.ConnectTo(this);
@@ -129,11 +139,15 @@ public abstract class Pin : CorePropertyChanged, IPin
     /// <inheritdoc />
     public bool IsTypeCompatible(Type type, bool forgivingEnumMatching = true)
     {
-        return Type == type
-               || (Direction == PinDirection.Input && type.IsAssignableTo(Type))
-               || (Direction == PinDirection.Output && type.IsAssignableFrom(Type))
-               || (Direction == PinDirection.Input && Type == typeof(Enum) && type.IsEnum && forgivingEnumMatching)
-               || (Direction == PinDirection.Output && type == typeof(Enum) && Type.IsEnum && forgivingEnumMatching);
+        if (Type == type)
+            return true;
+
+        // A specific enum and a pin of any enum only match when forgiving, every enum is assignable to Enum otherwise
+        if ((Direction == PinDirection.Input && Type == typeof(Enum) && type.IsEnum) || (Direction == PinDirection.Output && type == typeof(Enum) && Type.IsEnum))
+            return forgivingEnumMatching;
+
+        return (Direction == PinDirection.Input && type.IsAssignableTo(Type))
+               || (Direction == PinDirection.Output && type.IsAssignableFrom(Type));
     }
 
     /// <summary>