Fixed loop detection and added FailedMessage to changes

src/PixiEditor.ChangeableDocument/ChangeInfos/ChangeError_Info.cs

@@ -0,0 +1,11 @@
+namespace PixiEditor.ChangeableDocument.ChangeInfos;
+
+public struct ChangeError_Info : IChangeInfo
+{
+    public string Message { get; }
+
+    public ChangeError_Info(string message)
+    {
+        Message = message;
+    }
+}

src/PixiEditor.ChangeableDocument/Changes/Change.cs

@@ -2,6 +2,7 @@
 
 internal abstract class Change : IDisposable
 {
+    public string FailedMessage { get; protected set; }
     public Guid ChangeGuid { get; } = Guid.NewGuid();
 
     /// <summary>

src/PixiEditor.ChangeableDocument/Changes/NodeGraph/ConnectProperties_Change.cs

@@ -169,7 +169,7 @@ internal class ConnectProperties_Change : Change
         return changes;
     }
 
-    private bool IsLoop(InputProperty input, OutputProperty output)
+    private static bool IsLoop(InputProperty input, OutputProperty output)
     {
         if (input.Node == output.Node)
         {

src/PixiEditor.ChangeableDocument/Changes/Structure/MoveStructureMember_Change.cs

@@ -33,10 +33,16 @@ internal class MoveStructureMember_Change : Change
     public override bool InitializeAndValidate(Document document)
     {
         var member = document.FindMember(memberGuid);
-        var targetFolder = document.FindNode(targetNodeGuid);
-        if (member is null || targetFolder is null)
+        var targetNode = document.FindNode(targetNodeGuid);
+        if (member is null || targetNode is null)
             return false;
 
+        if (WillCreateLoop(member, targetNode))
+        {
+            FailedMessage = "ERROR_LOOP_DETECTED_MESSAGE";
+            return false;
+        }
+
         originalConnections = NodeOperations.CreateConnectionsData(member);
 
         return true;
@@ -67,7 +73,7 @@ internal class MoveStructureMember_Change : Change
         var previouslyConnected = inputProperty.Connection;
 
         bool isMovingBelow = false;
-        
+
         inputProperty.Node.TraverseForwards(x =>
         {
             if (x.Id == sourceNodeGuid)
@@ -75,13 +81,14 @@ internal class MoveStructureMember_Change : Change
                 isMovingBelow = true;
                 return false;
             }
-            
+
             return true;
         });
 
         if (isMovingBelow)
         {
-            changes.AddRange(NodeOperations.AdjustPositionsBeforeAppend(sourceNode, inputProperty.Node, out originalPositions));
+            changes.AddRange(
+                NodeOperations.AdjustPositionsBeforeAppend(sourceNode, inputProperty.Node, out originalPositions));
         }
 
         changes.AddRange(NodeOperations.DetachStructureNode(sourceNode));
@@ -129,8 +136,9 @@ internal class MoveStructureMember_Change : Change
 
         return changes;
     }
-    
-    private static List<IChangeInfo> AdjustPutIntoFolderPositions(Node targetNode, Dictionary<Guid, VecD> originalPositions)
+
+    private static List<IChangeInfo> AdjustPutIntoFolderPositions(Node targetNode,
+        Dictionary<Guid, VecD> originalPositions)
     {
         List<IChangeInfo> changes = new();
 
@@ -144,14 +152,14 @@ internal class MoveStructureMember_Change : Change
                     {
                         originalPositions[node.Id] = node.Position;
                     }
-                    
+
                     node.Position = new VecD(node.Position.X, folder.Position.Y + 250);
                     changes.Add(new NodePosition_ChangeInfo(node.Id, node.Position));
                 }
-                
+
                 return true;
             });
-            
+
             folder.Background.Connection?.Node.TraverseBackwards(bgNode =>
             {
                 if (bgNode is Node node)
@@ -167,15 +175,53 @@ internal class MoveStructureMember_Change : Change
                     {
                         pos -= 250;
                     }
-                    
+
                     node.Position = new VecD(node.Position.X, pos);
                     changes.Add(new NodePosition_ChangeInfo(node.Id, node.Position));
                 }
-                
+
                 return true;
             });
         }
 
         return changes;
     }
+
+    private bool WillCreateLoop(StructureNode member, Node targetNode)
+    {
+        InputProperty? input = targetNode.GetInputProperty("Background");
+        OutputProperty output = member.Output;
+
+        if (input is null)
+            return false;
+
+        return IsLoop(input, output);
+    }
+
+    private static bool IsLoop(InputProperty input, OutputProperty output)
+    {
+        if (input.Node == output.Node)
+        {
+            return true;
+        }
+
+        if (input.Node.OutputProperties.Any(x => x.InternalPropertyName != "Output" && x.Connections.Any(y => y.Node == output.Node)))
+        {
+            return true;
+        }
+
+        bool isLoop = false;
+        input.Node.TraverseForwards((node, inputProp) =>
+        {
+            if (node == output.Node && inputProp.InternalPropertyName != "Background")
+            {
+                isLoop = true;
+                return false;
+            }
+
+            return true;
+        });
+
+        return isLoop;
+    }
 }

src/PixiEditor.ChangeableDocument/DocumentChangeTracker.cs

@@ -210,7 +210,7 @@ public class DocumentChangeTracker : IDisposable
         }
 
         bool ignoreInUndo = false;
-        List<IChangeInfo> changeInfos = new();
+        System.Collections.Generic.List<IChangeInfo> changeInfos = new();
 
         if (activeUpdateableChange is InterruptableUpdateableChange interruptable)
         {
@@ -232,9 +232,10 @@ public class DocumentChangeTracker : IDisposable
         var validationResult = change.InitializeAndValidate(document);
         if (!validationResult)
         {
-            Trace.WriteLine($"Change {change} failed validation");
+            string? failedMessage = change.FailedMessage;
+            Trace.WriteLine($"Change {change} failed validation. Reason: {failedMessage}");
             change.Dispose();
-            return new None();
+            return string.IsNullOrEmpty(failedMessage) ? new None() : new ChangeError_Info(failedMessage);
         }
 
         var info = change.Apply(document, true, out ignoreInUndo);

src/PixiEditor/Data/Localization/Languages/en.json

@@ -923,5 +923,6 @@
   "LOAD_LAZY_FILE_MESSAGE": "To improve startup time, PixiEditor didn't load this file. Click the button below to load it.",
   "EASING_NODE": "Easing",
   "EASING_TYPE": "Easing Type",
-  "OPEN_DIRECTORY_ON_EXPORT": "Open directory on export"
+  "OPEN_DIRECTORY_ON_EXPORT": "Open directory on export",
+  "ERROR_LOOP_DETECTED_MESSAGE": "Moving this layer will create a loop. Fix it in the Node Graph."
 }

src/PixiEditor/Models/DocumentModels/DocumentUpdater.cs

@@ -1,5 +1,6 @@
 using System.Collections.Immutable;
 using System.Reflection;
+using Avalonia.Threading;
 using ChunkyImageLib;
 using ChunkyImageLib.DataHolders;
 using PixiEditor.ChangeableDocument.Changeables.Graph.Exceptions;
@@ -22,6 +23,7 @@ using PixiEditor.Models.DocumentPassthroughActions;
 using PixiEditor.Models.Handlers;
 using PixiEditor.Models.Layers;
 using Drawie.Numerics;
+using PixiEditor.Models.Dialogs;
 using PixiEditor.ViewModels.Document;
 using PixiEditor.ViewModels.Document.Nodes;
 using PixiEditor.ViewModels.Nodes;
@@ -59,6 +61,9 @@ internal class DocumentUpdater
         //TODO: Find a more elegant way to do this
         switch (arbitraryInfo)
         {
+            case ChangeError_Info error:
+                ProcessError(error);
+                break;
             case InvokeAction_PassthroughAction info:
                 ProcessInvokeAction(info);
                 break;
@@ -220,6 +225,14 @@ internal class DocumentUpdater
         }
     }
 
+    private void ProcessError(ChangeError_Info info)
+    {
+        Dispatcher.UIThread.Post(() =>
+        {
+            NoticeDialog.Show(info.Message, "ERROR");
+        });
+    }
+
     private void ProcessInvokeAction(InvokeAction_PassthroughAction info)
     {
         info.Action.Invoke();