Node script editor - Prevent creating loops

Node script popout editor - Added auto-layout hotkey (Ctrl+F)

src/Artemis.Core/VisualScripting/Extensions/NodeExtension.cs

@@ -1,26 +1,88 @@
 using System;
+using System.Collections.Generic;
 using System.Linq;
 
 namespace Artemis.Core;
 
+/// <summary>
+///     Provides extension methods for nodes.
+/// </summary>
 public static class NodeExtension
 {
     #region Methods
 
+    /// <summary>
+    ///     Estimates a height of a node in the editor.
+    /// </summary>
+    /// <param name="node">The node whose height to estimate.</param>
+    /// <returns>The estimated height in pixels.</returns>
     public static double EstimateHeight(this INode node)
     {
         const double PIN_HEIGHT = 26;
         const double TITLE_HEIGHT = 46;
 
         int inputPinCount = node.Pins.Count(x => x.Direction == PinDirection.Input)
-                          + node.PinCollections.Where(x => x.Direction == PinDirection.Input).Sum(x => x.Count() + 1);
+                            + node.PinCollections.Where(x => x.Direction == PinDirection.Input).Sum(x => x.Count() + 1);
         int outputPinCount = node.Pins.Count(x => x.Direction == PinDirection.Output)
-                           + node.PinCollections.Where(x => x.Direction == PinDirection.Output).Sum(x => x.Count() + 1);
+                             + node.PinCollections.Where(x => x.Direction == PinDirection.Output).Sum(x => x.Count() + 1);
 
-        return TITLE_HEIGHT + (Math.Max(inputPinCount, outputPinCount) * PIN_HEIGHT);
+        return TITLE_HEIGHT + Math.Max(inputPinCount, outputPinCount) * PIN_HEIGHT;
     }
 
-    public static double EstimateWidth(this INode node) => 120; // DarthAffe 13.09.2022: For now just assume they are all the same size
+    /// <summary>
+    ///     Estimates a width a node in the editor.
+    /// </summary>
+    /// <param name="node">The node whose width to estimate.</param>
+    /// <returns>The estimated width in pixels.</returns>
+    public static double EstimateWidth(this INode node)
+    {
+        // DarthAffe 13.09.2022: For now just assume they are all the same size
+        return 120;
+    }
+
+    /// <summary>
+    ///     Determines whether the node is part of a loop when the provided pending connecting would be connected.
+    /// </summary>
+    /// <param name="node">The node to check</param>
+    /// <param name="pendingConnection">The node to which a connection is pending</param>
+    /// <returns><see langword="true" /> if there would be a loop; otherwise <see langword="false" />.</returns>
+    public static bool IsInLoop(this INode node, INode pendingConnection)
+    {
+        HashSet<INode> checkedNodes = new();
+
+        bool CheckNode(INode checkNode, INode? pending)
+        {
+            if (checkedNodes.Contains(checkNode)) return false;
+
+            checkedNodes.Add(checkNode);
+
+            List<INode> connectedNodes = checkNode.Pins
+                .Where(x => x.Direction == PinDirection.Input)
+                .SelectMany(x => x.ConnectedTo)
+                .Select(x => x.Node)
+                .Concat(checkNode.PinCollections
+                    .Where(x => x.Direction == PinDirection.Input)
+                    .SelectMany(x => x)
+                    .SelectMany(x => x.ConnectedTo)
+                    .Select(x => x.Node))
+                .Distinct()
+                .ToList();
+            if (pending != null)
+                connectedNodes.Add(pending);
+
+            foreach (INode connectedNode in connectedNodes)
+            {
+                if (connectedNode == node)
+                    return true;
+                else if (CheckNode(connectedNode, null))
+                    return true;
+            }
+
+            return false;
+        }
+
+        return CheckNode(node, pendingConnection);
+    }
 
     #endregion
 }

src/Artemis.Core/VisualScripting/Extensions/NodeScriptExtension.cs

@@ -3,10 +3,17 @@ using System.Linq;
 
 namespace Artemis.Core;
 
+/// <summary>
+///     Provides extension methods for node scripts.
+/// </summary>
 public static class NodeScriptExtension
 {
     #region Methods
 
+    /// <summary>
+    ///     Organize a node script, attempting to lay out nodes in a logical manner.
+    /// </summary>
+    /// <param name="nodeScript">The node script to organize.</param>
     public static void Organize(this NodeScript nodeScript)
     {
         const double SPACING_HORIZONTAL = 160;
@@ -22,20 +29,22 @@ public static class NodeScriptExtension
         while (currentLevelNodes.Count > 0 && currentLevel < 1000)
         {
             List<INode> nextLevelNodes = currentLevelNodes.SelectMany(node => node.Pins
-                                                                                  .Where(x => x.Direction == PinDirection.Input)
-                                                                                  .SelectMany(x => x.ConnectedTo)
-                                                                                  .Select(x => x.Node)
-                                                                                  .Concat(node.PinCollections
-                                                                                              .Where(x => x.Direction == PinDirection.Input)
-                                                                                              .SelectMany(x => x)
-                                                                                              .SelectMany(x => x.ConnectedTo)
-                                                                                              .Select(x => x.Node)))
-                                                          .Distinct()
-                                                          .ToList();
+                    .Where(x => x.Direction == PinDirection.Input)
+                    .SelectMany(x => x.ConnectedTo)
+                    .Select(x => x.Node)
+                    .Concat(node.PinCollections
+                        .Where(x => x.Direction == PinDirection.Input)
+                        .SelectMany(x => x)
+                        .SelectMany(x => x.ConnectedTo)
+                        .Select(x => x.Node)))
+                .Distinct()
+                .ToList();
 
             foreach (INode nextLevelNode in nextLevelNodes)
+            {
                 if (currentLevel > levels[nextLevelNode])
                     levels[nextLevelNode] = currentLevel;
+            }
 
             currentLevelNodes = nextLevelNodes;
             currentLevel++;
@@ -60,7 +69,7 @@ public static class NodeScriptExtension
         foreach (IGrouping<int, INode> levelGroup in levels.GroupBy(x => x.Value, x => x.Key).OrderBy(x => x.Key))
         {
             List<INode> nodes = levelGroup.ToList();
-            double levelHeight = nodes.Sum(x => x.EstimateHeight()) + ((nodes.Count - 1) * SPACING_VERTICAL);
+            double levelHeight = nodes.Sum(x => x.EstimateHeight()) + (nodes.Count - 1) * SPACING_VERTICAL;
             double levelWidth = nodes.Max(x => x.EstimateWidth());
             double positionY = -(levelHeight / 2.0);
 
@@ -81,7 +90,7 @@ public static class NodeScriptExtension
         }
 
         if (unusedNodes != null)
-            LayoutLevel(unusedNodes, level0Width + (SPACING_HORIZONTAL / 2.0), unusedPosY);
+            LayoutLevel(unusedNodes, level0Width + SPACING_HORIZONTAL / 2.0, unusedPosY);
     }
 
     #endregion

src/Artemis.UI/Screens/VisualScripting/NodeScriptViewModel.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
+using System.Diagnostics;
 using System.Linq;
 using System.Reactive;
 using System.Reactive.Linq;
@@ -197,8 +198,14 @@ public class NodeScriptViewModel : ActivatableViewModelBase
             DragViewModel = new DragCableViewModel(sourcePinViewModel);
 
         DragViewModel.DragPoint = position;
+        if (targetPinVmModel == null)
+            return true;
+        if (!targetPinVmModel.IsCompatibleWith(sourcePinViewModel))
+            return false;
 
-        return targetPinVmModel == null || targetPinVmModel.IsCompatibleWith(sourcePinViewModel);
+        return sourcePinViewModel.Pin.Direction == PinDirection.Output
+            ? !targetPinVmModel.Pin.Node.IsInLoop(sourcePinViewModel.Pin.Node)
+            : !sourcePinViewModel.Pin.Node.IsInLoop(targetPinVmModel.Pin.Node);
     }
 
     public void FinishPinDrag(PinViewModel sourcePinViewModel, PinViewModel? targetPinVmModel, Point position)
@@ -211,7 +218,10 @@ public class NodeScriptViewModel : ActivatableViewModelBase
         // If dropped on top of a compatible pin, connect to it
         if (targetPinVmModel != null && targetPinVmModel.IsCompatibleWith(sourcePinViewModel))
         {
-            _nodeEditorService.ExecuteCommand(NodeScript, new ConnectPins(sourcePinViewModel.Pin, targetPinVmModel.Pin));
+            if (sourcePinViewModel.Pin.Direction == PinDirection.Output && !targetPinVmModel.Pin.Node.IsInLoop(sourcePinViewModel.Pin.Node))
+                _nodeEditorService.ExecuteCommand(NodeScript, new ConnectPins(sourcePinViewModel.Pin, targetPinVmModel.Pin));
+            else if (sourcePinViewModel.Pin.Direction == PinDirection.Input && !sourcePinViewModel.Pin.Node.IsInLoop(targetPinVmModel.Pin.Node))
+                _nodeEditorService.ExecuteCommand(NodeScript, new ConnectPins(sourcePinViewModel.Pin, targetPinVmModel.Pin));
         }
         // If not dropped on a pin allow the user to create a new node
         else if (targetPinVmModel == null)

src/Artemis.UI/Screens/VisualScripting/NodeScriptWindowView.axaml

@@ -16,6 +16,7 @@
     <Window.KeyBindings>
         <KeyBinding Command="{CompiledBinding History.Undo}" Gesture="Ctrl+Z" />
         <KeyBinding Command="{CompiledBinding History.Redo}" Gesture="Ctrl+Y" />
+        <KeyBinding Command="{CompiledBinding AutoArrange}" Gesture="Ctrl+F" />
     </Window.KeyBindings>
     <Grid Margin="15" ColumnDefinitions="*,*" RowDefinitions="Auto,Auto,*">
         <Menu Grid.Row="0" Grid.ColumnSpan="2" VerticalAlignment="Top" Margin="-10 -10 -10 0">
@@ -43,7 +44,7 @@
                         </DataTemplate>
                     </MenuItem.DataTemplates>
                 </MenuItem>
-                <MenuItem Header="Auto-arrange" Command="{CompiledBinding AutoArrange}">
+                <MenuItem Header="Auto-arrange" Command="{CompiledBinding AutoArrange}" InputGesture="Ctrl+F">
                     <MenuItem.Icon>
                         <avalonia:MaterialIcon Kind="Sort" />
                     </MenuItem.Icon>