From 7d7ded4cb62b3729e34a56cc497a4e462081b023 Mon Sep 17 00:00:00 2001 From: glopesdev Date: Tue, 13 Jun 2023 08:32:43 +0100 Subject: [PATCH 1/9] Add regression test for disconnecting graph nodes --- Bonsai.Editor.Tests/WorkflowEditorTests.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Bonsai.Editor.Tests/WorkflowEditorTests.cs b/Bonsai.Editor.Tests/WorkflowEditorTests.cs index 0c8b8835..59ee61a3 100644 --- a/Bonsai.Editor.Tests/WorkflowEditorTests.cs +++ b/Bonsai.Editor.Tests/WorkflowEditorTests.cs @@ -194,6 +194,22 @@ public void UngroupGraphNodes_NestedPassthrough_KeepOuterEdges() editor.UngroupGraphNodes(nodesToUngroup); assertIsReversible(); } + + [TestMethod] + public void DisconnectGraphNodes_TargetIsNotRoot_InsertAfterClosestRoot() + { + var workflow = EditorHelper.CreateEditorGraph("A", "B", "C", "D"); + var (editor, assertIsReversible) = CreateMockEditor(workflow); + editor.ConnectNodes("A", "C"); + editor.ConnectNodes("B", "C"); + editor.ConnectNodes("C", "D"); + var sourceNode = editor.FindNode("B"); + var targetNode = editor.FindNode("C"); + Assert.AreEqual(expected: editor.Workflow.Count - 1, editor.FindNode("D").Index); + editor.DisconnectGraphNodes(new[] { sourceNode }, targetNode); + Assert.AreEqual(expected: editor.Workflow.Count - 1, editor.FindNode("B").Index); + assertIsReversible(); + } } static class WorkflowEditorHelper From 9df06bcc5fcb69a4f2ea97ab6d122a6d12b82393 Mon Sep 17 00:00:00 2001 From: glopesdev Date: Tue, 13 Jun 2023 08:37:45 +0100 Subject: [PATCH 2/9] Fix implementation argument reference --- Bonsai.Editor/GraphModel/WorkflowEditor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Bonsai.Editor/GraphModel/WorkflowEditor.cs b/Bonsai.Editor/GraphModel/WorkflowEditor.cs index c212c9c8..bf8f257f 100644 --- a/Bonsai.Editor/GraphModel/WorkflowEditor.cs +++ b/Bonsai.Editor/GraphModel/WorkflowEditor.cs @@ -198,7 +198,7 @@ private void SortWorkflow() private void SortWorkflow(ExpressionBuilderGraph workflow) { - Workflow.InsertRange(0, Workflow.TopologicalSort()); + workflow.InsertRange(0, workflow.TopologicalSort()); } private GraphCommand GetSimpleSort() From bfcf151b8099ce09f36c2a9caacd896363f4786c Mon Sep 17 00:00:00 2001 From: glopesdev Date: Tue, 13 Jun 2023 08:40:58 +0100 Subject: [PATCH 3/9] Insert disconnected roots at the end of component --- Bonsai.Editor/GraphModel/WorkflowEditor.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Bonsai.Editor/GraphModel/WorkflowEditor.cs b/Bonsai.Editor/GraphModel/WorkflowEditor.cs index bf8f257f..001ff1aa 100644 --- a/Bonsai.Editor/GraphModel/WorkflowEditor.cs +++ b/Bonsai.Editor/GraphModel/WorkflowEditor.cs @@ -688,7 +688,9 @@ where siblingEdge.Item2.Label.Index.CompareTo(edgeIndex) > 0 } GraphCommand reorder; - var targetIndex = workflow.IndexOf(target) + 1; + var components = workflow.FindConnectedComponents(); + var targetComponent = components.First(component => component.Contains(target)); + var targetIndex = LastIndexOfComponentNode(workflow, targetComponent) + 1; if (sinkNodes != null) { reorder = GetReversibleSort(); From 91fc8abb961e49ae62b15d47955b2d93dc36e4ba Mon Sep 17 00:00:00 2001 From: glopesdev Date: Tue, 13 Jun 2023 14:02:46 +0100 Subject: [PATCH 4/9] Add regression test for inserting annotation node --- Bonsai.Editor.Tests/EditorHelper.cs | 6 ++++++ Bonsai.Editor.Tests/WorkflowEditorTests.cs | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/Bonsai.Editor.Tests/EditorHelper.cs b/Bonsai.Editor.Tests/EditorHelper.cs index 78e64821..ef97a1c7 100644 --- a/Bonsai.Editor.Tests/EditorHelper.cs +++ b/Bonsai.Editor.Tests/EditorHelper.cs @@ -31,6 +31,12 @@ internal static GraphNode FindNode(this WorkflowEditor editor, string name) return editor.FindGraphNode(node.Value); } + internal static GraphNode FindNode(this WorkflowEditor editor, ExpressionBuilder builder) + { + var node = editor.Workflow.First(n => ExpressionBuilder.Unwrap(n.Value) == builder); + return editor.FindGraphNode(node.Value); + } + internal static void ConnectNodes(this WorkflowEditor editor, string source, string target) { var sourceNodes = new[] { editor.FindNode(source) }; diff --git a/Bonsai.Editor.Tests/WorkflowEditorTests.cs b/Bonsai.Editor.Tests/WorkflowEditorTests.cs index 59ee61a3..bf037efd 100644 --- a/Bonsai.Editor.Tests/WorkflowEditorTests.cs +++ b/Bonsai.Editor.Tests/WorkflowEditorTests.cs @@ -210,6 +210,17 @@ public void DisconnectGraphNodes_TargetIsNotRoot_InsertAfterClosestRoot() Assert.AreEqual(expected: editor.Workflow.Count - 1, editor.FindNode("B").Index); assertIsReversible(); } + + [TestMethod] + public void CreateAnnotation_EmptySelection_InsertAfterClosestRoot() + { + var workflow = EditorHelper.CreateEditorGraph("A"); + var (editor, assertIsReversible) = CreateMockEditor(workflow); + var annotationBuilder = new AnnotationBuilder(); + editor.CreateGraphNode(annotationBuilder, null, CreateGraphNodeType.Successor, branch: false); + Assert.AreEqual(expected: editor.Workflow.Count - 1, editor.FindNode(annotationBuilder).Index); + assertIsReversible(); + } } static class WorkflowEditorHelper From 3bd78d4a9561893b3f9e0c94f7f0a832403b14da Mon Sep 17 00:00:00 2001 From: glopesdev Date: Tue, 13 Jun 2023 14:04:35 +0100 Subject: [PATCH 5/9] Create property source as successor --- Bonsai.Editor/GraphView/WorkflowGraphView.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Bonsai.Editor/GraphView/WorkflowGraphView.cs b/Bonsai.Editor/GraphView/WorkflowGraphView.cs index 185d7bcf..ab3a9ddb 100644 --- a/Bonsai.Editor/GraphView/WorkflowGraphView.cs +++ b/Bonsai.Editor/GraphView/WorkflowGraphView.cs @@ -1692,7 +1692,7 @@ private ToolStripMenuItem CreatePropertySourceMenuItem( valueProperty.SetValue(propertySource, memberValue); propertySource.MemberName = memberName; - Editor.CreateGraphNode(propertySource, default(GraphNode), CreateGraphNodeType.Predecessor, branch: true); + Editor.CreateGraphNode(propertySource, default(GraphNode), CreateGraphNodeType.Successor, branch: true); contextMenuStrip.Close(ToolStripDropDownCloseReason.ItemClicked); }); From 696b9141a824e51c9d7015ce922733b867f3a3c9 Mon Sep 17 00:00:00 2001 From: glopesdev Date: Tue, 13 Jun 2023 14:10:43 +0100 Subject: [PATCH 6/9] Refactor related overloads for consistency --- Bonsai.Editor/GraphModel/WorkflowEditor.cs | 29 ++++++++++++++------ Bonsai.Editor/GraphView/WorkflowGraphView.cs | 2 +- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/Bonsai.Editor/GraphModel/WorkflowEditor.cs b/Bonsai.Editor/GraphModel/WorkflowEditor.cs index 001ff1aa..66484ddc 100644 --- a/Bonsai.Editor/GraphModel/WorkflowEditor.cs +++ b/Bonsai.Editor/GraphModel/WorkflowEditor.cs @@ -964,12 +964,23 @@ static string MakeGenericType(string typeName, ExpressionBuilder builder, out El return genericType.MakeGenericType(inspectBuilder.ObservableType).AssemblyQualifiedName; } - public void InsertGraphNode(string typeName, ElementCategory elementCategory, CreateGraphNodeType nodeType, bool branch, bool group) + public void CreateGraphNode( + string typeName, + ElementCategory elementCategory, + CreateGraphNodeType nodeType, + bool branch, + bool group) { - InsertGraphNode(typeName, elementCategory, nodeType, branch, group, null); + CreateGraphNode(typeName, elementCategory, nodeType, branch, group, arguments: null); } - public void InsertGraphNode(string typeName, ElementCategory elementCategory, CreateGraphNodeType nodeType, bool branch, bool group, string arguments) + public void CreateGraphNode( + string typeName, + ElementCategory elementCategory, + CreateGraphNodeType nodeType, + bool branch, + bool group, + string arguments) { if (string.IsNullOrEmpty(typeName)) { @@ -1019,11 +1030,6 @@ public void InsertGraphNode(string typeName, ElementCategory elementCategory, Cr builder = CreateBuilder(typeName, elementCategory, group); ConfigureBuilder(builder, selectedNode, arguments); - if (typeName == typeof(ExternalizedMappingBuilder).AssemblyQualifiedName || - typeName == typeof(AnnotationBuilder).AssemblyQualifiedName) - { - nodeType = CreateGraphNodeType.Predecessor; - } var commands = GetCreateGraphNodeCommands(builder, selectedNodes.Select(GetGraphNodeTag), nodeType, branch); commandExecutor.BeginCompositeCommand(); commandExecutor.Execute(EmptyAction, commands.updateLayout.Undo); @@ -1156,6 +1162,13 @@ void ConfigureWorkflowBuilder( var inspectParameter = new ExpressionBuilderArgument(); var targetNodes = selectedNodes.ToArray(); + if (targetNodes.Length > 0 && + (builder is ExternalizedMappingBuilder || + builder is AnnotationBuilder)) + { + nodeType = CreateGraphNodeType.Predecessor; + } + var restoreSelectedNodes = CreateUpdateSelectionDelegate(targetNodes); if (insertIndex < 0) { diff --git a/Bonsai.Editor/GraphView/WorkflowGraphView.cs b/Bonsai.Editor/GraphView/WorkflowGraphView.cs index ab3a9ddb..539aacea 100644 --- a/Bonsai.Editor/GraphView/WorkflowGraphView.cs +++ b/Bonsai.Editor/GraphView/WorkflowGraphView.cs @@ -335,7 +335,7 @@ public void CreateGraphNode( bool group, string arguments) { - try { Editor.InsertGraphNode(typeName, elementCategory, nodeType, branch, group, arguments); } + try { Editor.CreateGraphNode(typeName, elementCategory, nodeType, branch, group, arguments); } catch (TargetInvocationException e) { var message = string.Format(Resources.CreateTypeNode_Error, name, e.InnerException.Message); From 82daed2e4e25438d84578589d3c211c34b060ecf Mon Sep 17 00:00:00 2001 From: glopesdev Date: Tue, 13 Jun 2023 14:12:41 +0100 Subject: [PATCH 7/9] Reorder method declaration order for readability --- Bonsai.Editor/GraphModel/WorkflowEditor.cs | 66 +++++++++++----------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/Bonsai.Editor/GraphModel/WorkflowEditor.cs b/Bonsai.Editor/GraphModel/WorkflowEditor.cs index 66484ddc..e1b699e1 100644 --- a/Bonsai.Editor/GraphModel/WorkflowEditor.cs +++ b/Bonsai.Editor/GraphModel/WorkflowEditor.cs @@ -964,6 +964,39 @@ static string MakeGenericType(string typeName, ExpressionBuilder builder, out El return genericType.MakeGenericType(inspectBuilder.ObservableType).AssemblyQualifiedName; } + private void ConfigureBuilder(ExpressionBuilder builder, GraphNode selectedNode, string arguments) + { + if (string.IsNullOrEmpty(arguments)) return; + // TODO: This special case for binary operator operands should be avoided in the future + if (builder is BinaryOperatorBuilder binaryOperator && selectedNode != null) + { + if (GetGraphNodeTag(selectedNode).Value is InspectBuilder inputBuilder && + inputBuilder.ObservableType != null) + { + binaryOperator.Build(Expression.Parameter(typeof(IObservable<>).MakeGenericType(inputBuilder.ObservableType))); + } + } + + var workflowElement = ExpressionBuilder.GetWorkflowElement(builder); + var defaultProperty = TypeDescriptor.GetDefaultProperty(workflowElement); + if (defaultProperty != null && + !defaultProperty.IsReadOnly && + defaultProperty.Converter != null && + defaultProperty.Converter.CanConvertFrom(typeof(string))) + { + try + { + var context = new TypeDescriptorContext(workflowElement, defaultProperty, serviceProvider); + var propertyValue = defaultProperty.Converter.ConvertFromString(context, arguments); + defaultProperty.SetValue(workflowElement, propertyValue); + } + catch (Exception ex) + { + throw new SystemException(ex.Message, ex); + } + } + } + public void CreateGraphNode( string typeName, ElementCategory elementCategory, @@ -1039,39 +1072,6 @@ public void CreateGraphNode( commandExecutor.EndCompositeCommand(); } - private void ConfigureBuilder(ExpressionBuilder builder, GraphNode selectedNode, string arguments) - { - if (string.IsNullOrEmpty(arguments)) return; - // TODO: This special case for binary operator operands should be avoided in the future - if (builder is BinaryOperatorBuilder binaryOperator && selectedNode != null) - { - if (GetGraphNodeTag(selectedNode).Value is InspectBuilder inputBuilder && - inputBuilder.ObservableType != null) - { - binaryOperator.Build(Expression.Parameter(typeof(IObservable<>).MakeGenericType(inputBuilder.ObservableType))); - } - } - - var workflowElement = ExpressionBuilder.GetWorkflowElement(builder); - var defaultProperty = TypeDescriptor.GetDefaultProperty(workflowElement); - if (defaultProperty != null && - !defaultProperty.IsReadOnly && - defaultProperty.Converter != null && - defaultProperty.Converter.CanConvertFrom(typeof(string))) - { - try - { - var context = new TypeDescriptorContext(workflowElement, defaultProperty, serviceProvider); - var propertyValue = defaultProperty.Converter.ConvertFromString(context, arguments); - defaultProperty.SetValue(workflowElement, propertyValue); - } - catch (Exception ex) - { - throw new SystemException(ex.Message, ex); - } - } - } - public void CreateGraphNode( ExpressionBuilder builder, GraphNode selectedNode, From 075f531205f04dc2df5bb397c163128d4d7d5433 Mon Sep 17 00:00:00 2001 From: glopesdev Date: Tue, 13 Jun 2023 14:14:11 +0100 Subject: [PATCH 8/9] Add node to end of workflow if no target specified --- Bonsai.Editor/GraphModel/WorkflowEditor.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Bonsai.Editor/GraphModel/WorkflowEditor.cs b/Bonsai.Editor/GraphModel/WorkflowEditor.cs index e1b699e1..77810316 100644 --- a/Bonsai.Editor/GraphModel/WorkflowEditor.cs +++ b/Bonsai.Editor/GraphModel/WorkflowEditor.cs @@ -153,7 +153,8 @@ private int GetInsertIndex( Node target, CreateGraphNodeType nodeType) { - var insertOffset = target == null || nodeType == CreateGraphNodeType.Successor ? 1 : 0; + if (target == null) return workflow.Count; + var insertOffset = nodeType == CreateGraphNodeType.Successor ? 1 : 0; return workflow.IndexOf(target) + insertOffset; } From 5a90afb570f39f33361af32564fe825e48958d73 Mon Sep 17 00:00:00 2001 From: glopesdev Date: Tue, 13 Jun 2023 14:55:29 +0100 Subject: [PATCH 9/9] Ensure sort is reversible when adding a new node --- Bonsai.Editor/GraphModel/WorkflowEditor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Bonsai.Editor/GraphModel/WorkflowEditor.cs b/Bonsai.Editor/GraphModel/WorkflowEditor.cs index 77810316..15713d5e 100644 --- a/Bonsai.Editor/GraphModel/WorkflowEditor.cs +++ b/Bonsai.Editor/GraphModel/WorkflowEditor.cs @@ -1209,7 +1209,7 @@ void ConfigureWorkflowBuilder( _ => false })); - var reorder = GetSimpleSort(); + var reorder = GetReversibleSort(); var updateGraphLayout = CreateUpdateGraphLayoutDelegate(); var updateSelectedNode = CreateUpdateSelectionDelegate(builder); var insertCommands = GetInsertGraphNodeCommands(inspectNode, inspectNode, targetNodes, nodeType, branch, validateInsert);