fix: properly restore connections when changing parameter or return t…

…ypes of functions/modules

fixes #20

Refactorings/ChangeFunctionReturnTypeRefactoring.cs

@@ -50,6 +50,10 @@ public override void PerformRefactoring(RefactoringContext context)
                 var affectedConnections = graph.GetAllConnections()
                     .Where(it => it.InvolvesAnyPort(scadNode, returnValuePorts))
                     .ToList();
+
+                // and remove them, we'll add the valid ones back later. We do this now because the connection rules
+                // check for duplicate connections and would veto a connection that already exists.
+                affectedConnections.ForAll(it => graph.RemoveConnection(it));
 
                 // now instruct the node to rebuild its ports using the updated return type
                 node.SetupPorts(_description);
@@ -60,13 +64,13 @@ public override void PerformRefactoring(RefactoringContext context)
                     scadNode.DropPortLiteral(it);
                     scadNode.BuildPortLiteral(it);
                 });
-
-                // now for all the connections we have saved, check if they are still valid.
+
+                // now for all the connections we have saved, check if they are still valid and re-add the ones
+                // that were not vetoed.
                 affectedConnections
-                    .Where(it => ConnectionRules.CanConnect(it).Decision == ConnectionRules.OperationRuleDecision.Veto)
+                    .Where(it => ConnectionRules.CanConnect(it).Decision != ConnectionRules.OperationRuleDecision.Veto)
                     .ToList()
-                    // and remove the ones that are vetoed.
-                    .ForAll(it => graph.RemoveConnection(it));
+                    .ForAll(it => graph.AddConnection(it.From.Id, it.FromPort, it.To.Id, it.ToPort));
             }
         }
     }

Refactorings/ChangeInvokableParameterTypeRefactoring.cs

@@ -64,6 +64,11 @@ public override void PerformRefactoring(RefactoringContext context)
                         .Where(it => it.IsFrom(scadNode, outputPort)));
                 }
 
+                // now remove all affected connections. we will add them later, but we need to remove them first
+                // because otherwise the connection rules would detect duplicate connections and would veto
+                // valid connections.
+                affectedConnections.ForAll(it => graph.RemoveConnection(it));
+
                 // now instruct the node to rebuild its ports using the updated parameter type
                 node.SetupPorts(_description);
 
@@ -96,13 +101,12 @@ public override void PerformRefactoring(RefactoringContext context)
                     }
                 }
 
-
                 // now for all the connections we have saved, check if they are still valid.
+                // and re-add them if they are.
                 affectedConnections
-                    .Where(it => ConnectionRules.CanConnect(it).Decision == ConnectionRules.OperationRuleDecision.Veto)
+                    .Where(it => ConnectionRules.CanConnect(it).Decision != ConnectionRules.OperationRuleDecision.Veto)
                     .ToList()
-                    // and remove the ones that are vetoed.
-                    .ForAll(it => graph.RemoveConnection(it));
+                    .ForAll(it => graph.AddConnection(it.From.Id, it.FromPort, it.To.Id, it.ToPort));
 
                 // finally if the parameter was optional before but the new parameter type does not support literals
                 // then we need to make the parameter mandatory

Tests/ParameterRefactoringTests.cs

@@ -0,0 +1,79 @@
+using System;
+using System.Linq;
+using System.Threading.Tasks;
+using Godot;
+using GodotTestDriver.Drivers;
+using GodotXUnitApi;
+using OpenScadGraphEditor.Nodes;
+using Xunit;
+
+namespace OpenScadGraphEditor.Tests
+{
+    public class ParameterRefactoringTests : MainWindowTest
+    {
+        [GodotFact(Frame = GodotFactFrame.Process)]
+        public async Task ChangingParameterTypesKeepsConnections()
+        {
+            // when i create a module
+            var moduleName = "mod" + Guid.NewGuid().ToString().Replace("-", "");
+            await MainWindow.AddModuleButton.ClickCenter();
+            await MainWindow.InvokableRefactorDialog.NameEdit.Type(moduleName);
+            await MainWindow.InvokableRefactorDialog.AddParameterButton.ClickCenter();
+            await MainWindow.InvokableRefactorDialog.OkButton.ClickCenter();
+
+            var entryPoint = MainWindow.GraphEditor.Nodes.Last();
+
+            // and i add a plus node to the module
+            await MainWindow.AddNode("++");
+            var plusNodeModule = MainWindow.GraphEditor.Nodes.Last();
+
+            // and i connect the first output of the entry point to the first input of the plus node
+            await entryPoint.DragConnection(Port.Output(0), plusNodeModule, Port.Input(0));
+
+            // and i switch to the main graph
+            await MainWindow.TabContainer.SelectTabWithTitle("<main>");
+
+            // and i create a module instance
+            await MainWindow.AddNode(moduleName);
+            var invocation = MainWindow.GraphEditor.Nodes.Last();
+
+            // and i move this a bit to the right
+            await invocation.DragByOwnSize(2, 0);
+
+            // and i create an add node
+            await MainWindow.AddNode("++");
+            var plusNodeMain = MainWindow.GraphEditor.Nodes.Last();
+            // and i drag this a bit to the left
+            await plusNodeMain.DragByOwnSize(-2, 0);
+
+            // and i connect the output of the plus node to the input of the invocation
+            await plusNodeMain.DragConnection(Port.Output(0), invocation, Port.Input(0));
+
+            // and i right-click the module invocation
+            await invocation.ClickAtSelectionSpot(ButtonList.Right);
+            // and i select  the "Refactor <module>" entry in the popup menu
+            await MainWindow.PopupMenu.SelectItemWithText("Refactor " + moduleName);
+
+            // and i change the type of the parameter to "number"
+            await MainWindow.InvokableRefactorDialog.ParameterLines.First().Type.SelectItemWithText("number");
+            // and i press the "OK" button
+            await MainWindow.InvokableRefactorDialog.OkButton.ClickCenter();
+
+            // then
+            // the input type of the invocation is now "number"
+            Assert.Equal((int)PortType.Number, invocation.GetPortType(Port.Input(0)));
+
+            // and the plus node in the main graph is still connected to the invocation
+            Assert.True(MainWindow.GraphEditor.HasConnection(plusNodeMain, Port.Output(0), invocation, Port.Input(0)));
+
+            // and when i go back to the module
+            await MainWindow.TabContainer.SelectTabWithTitle(moduleName);
+
+            // then the plus node in the module is still connected to the entry point
+            Assert.True(MainWindow.GraphEditor.HasConnection(entryPoint, Port.Output(0), plusNodeModule, Port.Input(0)));
+            // and the port type of the entry point is now "number"
+            Assert.Equal((int)PortType.Number, entryPoint.GetPortType(Port.Output(0)));
+
+        }
+    }
+}