diff --git a/src/Artemis.Core.Tests/Services/ProfileImportExportTests.cs b/src/Artemis.Core.Tests/Services/ProfileImportExportTests.cs index dd31170ad..51ccffffa 100644 --- a/src/Artemis.Core.Tests/Services/ProfileImportExportTests.cs +++ b/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().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().ShouldBeEmpty(); - reloaded.ActivationCondition.ExitNodeConnected.ShouldBeFalse(); + reloaded.ActivationCondition.Nodes.OfType().ShouldHaveSingleItem(); + reloaded.ActivationCondition.ExitNodeConnected.ShouldBeTrue(); } [Fact] diff --git a/src/Artemis.Core.Tests/VisualScripting/NodeScriptStorageTests.cs b/src/Artemis.Core.Tests/VisualScripting/NodeScriptStorageTests.cs index c418ea974..082bc9dc8 100644 --- a/src/Artemis.Core.Tests/VisualScripting/NodeScriptStorageTests.cs +++ b/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 script = CreateScript(); @@ -227,11 +226,13 @@ public class NodeScriptStorageTests : ScriptTestBase missingPlugin.Dispose(); NodeScript loaded = Load(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 script = CreateScript(); TestNumericNode first = Nodes.AddNode(script); TestNumericNode second = Nodes.AddNode(script); @@ -318,8 +318,8 @@ public class NodeScriptStorageTests : ScriptTestBase NodeScript 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] diff --git a/src/Artemis.Core.Tests/VisualScripting/NodeScriptTests.cs b/src/Artemis.Core.Tests/VisualScripting/NodeScriptTests.cs index ecc59a0c7..39e6c538c 100644 --- a/src/Artemis.Core.Tests/VisualScripting/NodeScriptTests.cs +++ b/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 script = CreateScript(); 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 script = CreateScript(); @@ -324,7 +320,7 @@ public class NodeScriptTests : ScriptTestBase otherPlugin.Dispose(); script.Nodes.ShouldNotContain(fromOtherProvider); - script.Nodes.ShouldNotContain(fromStillEnabledProvider); + script.Nodes.ShouldContain(fromStillEnabledProvider); } [Fact] diff --git a/src/Artemis.Core.Tests/VisualScripting/NodeTests.cs b/src/Artemis.Core.Tests/VisualScripting/NodeTests.cs index 3ca83762f..b1f89695b 100644 --- a/src/Artemis.Core.Tests/VisualScripting/NodeTests.cs +++ b/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 script = CreateScript(); TestThrowingNode node = ScriptNodes.AddNode(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 script = CreateScript(); TestNode failing = new() {OnEvaluate = _ => throw new InvalidOperationException()}; OutputPin output = failing.CreateOutputPin(); @@ -252,7 +251,7 @@ public class NodeTests : ScriptTestBase script.Run(); - failing.Evaluations.ShouldBe(2); + failing.Evaluations.ShouldBe(1); sum.BrokenState.ShouldBeNull(); } diff --git a/src/Artemis.Core.Tests/VisualScripting/NumericTests.cs b/src/Artemis.Core.Tests/VisualScripting/NumericTests.cs index c5136ccbe..ecbb8f4aa 100644 --- a/src/Artemis.Core.Tests/VisualScripting/NumericTests.cs +++ b/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); diff --git a/src/Artemis.Core.Tests/VisualScripting/PinTests.cs b/src/Artemis.Core.Tests/VisualScripting/PinTests.cs index 07a967493..a2b500154 100644 --- a/src/Artemis.Core.Tests/VisualScripting/PinTests.cs +++ b/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 output = _source.CreateOutputPin(); InputPin input = _target.CreateInputPin(); 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 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 output = _source.CreateOutputPin(); InputPin typed = _target.CreateInputPin(); 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] diff --git a/src/Artemis.Core/Utilities/Numeric.cs b/src/Artemis.Core/Utilities/Numeric.cs index 06c2ca7fd..3b12b3ade 100644 --- a/src/Artemis.Core/Utilities/Numeric.cs +++ b/src/Artemis.Core/Utilities/Numeric.cs @@ -182,7 +182,7 @@ public readonly struct Numeric : IComparable, 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, IConvertible /// public sbyte ToSByte(IFormatProvider? provider) { - return (sbyte) Math.Clamp(_value, 0, 255); + return (sbyte) Math.Clamp(_value, sbyte.MinValue, sbyte.MaxValue); } /// diff --git a/src/Artemis.Core/VisualScripting/NodeScript.cs b/src/Artemis.Core/VisualScripting/NodeScript.cs index 7864300d0..a89251132 100644 --- a/src/Artemis.Core/VisualScripting/NodeScript.cs +++ b/src/Artemis.Core/VisualScripting/NodeScript.cs @@ -22,7 +22,10 @@ public abstract class NodeScript : CorePropertyChanged, INodeScript private void NodeTypeStoreOnNodeTypeRemoved(object? sender, NodeTypeStoreEvent e) { - List 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 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(node)); @@ -253,6 +257,8 @@ public abstract class NodeScript : CorePropertyChanged, INodeScript public void LoadConnections() { List nodes = Nodes.ToList(); + // An input reads from its first connection, so when there are several the first one is kept + HashSet 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 unavailableNodes = Entity.Nodes.Where(e => !e.IsExitNode && Nodes.All(n => n.Id != e.Id) && NodeTypeStore.Get(e) == null).ToList(); + List 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); diff --git a/src/Artemis.Core/VisualScripting/Nodes/Node.cs b/src/Artemis.Core/VisualScripting/Nodes/Node.cs index 90ca8da13..2c05b2060 100644 --- a/src/Artemis.Core/VisualScripting/Nodes/Node.cs +++ b/src/Artemis.Core/VisualScripting/Nodes/Node.cs @@ -385,7 +385,13 @@ public abstract class Node : BreakableModel, INode /// 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().Where(p => p.Direction == PinDirection.Output)) + pin.SetDefaultValue(); } /// diff --git a/src/Artemis.Core/VisualScripting/Pins/InputPin.cs b/src/Artemis.Core/VisualScripting/Pins/InputPin.cs index a183885c5..5ce4d3fee 100644 --- a/src/Artemis.Core/VisualScripting/Pins/InputPin.cs +++ b/src/Artemis.Core/VisualScripting/Pins/InputPin.cs @@ -12,7 +12,7 @@ public sealed class InputPin : 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)); diff --git a/src/Artemis.Core/VisualScripting/Pins/OutputPin.cs b/src/Artemis.Core/VisualScripting/Pins/OutputPin.cs index 8397eddf6..5ec32d1d1 100644 --- a/src/Artemis.Core/VisualScripting/Pins/OutputPin.cs +++ b/src/Artemis.Core/VisualScripting/Pins/OutputPin.cs @@ -29,6 +29,11 @@ public sealed class OutputPin : Pin /// public override PinDirection Direction => PinDirection.Output; + internal override void SetDefaultValue() + { + Value = default; + } + private T? _value; /// @@ -100,6 +105,11 @@ public sealed class OutputPin : Pin /// 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)); diff --git a/src/Artemis.Core/VisualScripting/Pins/Pin.cs b/src/Artemis.Core/VisualScripting/Pins/Pin.cs index f82799e2c..222053542 100644 --- a/src/Artemis.Core/VisualScripting/Pins/Pin.cs +++ b/src/Artemis.Core/VisualScripting/Pins/Pin.cs @@ -85,9 +85,19 @@ public abstract class Pin : CorePropertyChanged, IPin IsEvaluated = false; } + /// + /// Sets the value of an output pin to the default value of its type, used when its node fails to evaluate + /// + internal virtual void SetDefaultValue() + { + } + /// 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 /// 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)); } ///