Bugfix/90 prevent graph cycles (Redux) (#105)

* Export detectCycles function from compiler * Add detectCycles function on Model * Extract ConnectNodes creation, use Model.detectCycles, display dialog on cycle * Use errorDialog --------- Co-authored-by: Maxime Chevalier-Boisvert <[email protected]>
maximecb · Apr 5, 2023 · 74b7f39 · 74b7f39
1 parent fc8689f
commit 74b7f39
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 18 deletions.
diff --git a/public/compiler.js b/public/compiler.js
@@ -185,6 +185,23 @@ function topoSort(graph)
 }
 
 /**
+ * Detect cycles in a graph of nodes
+ */
+export function detectCycles(graph)
+{
+    try
+    {
+        topoSort(splitNodes(graph));
+        // A graph sorted with no issues has no cycle
+        return false;
+    }
+    catch (err)
+    {
+        // The only error thrown from topoSort is the SyntaxError, indicating a cycle
+        return true;
+    }
+}/**
+
  * Compile a sound-generating function from a graph of nodes
  */
 export function compile(graph)

diff --git a/public/editor.js b/public/editor.js
@@ -1,5 +1,5 @@
 import { assert, anyInputActive, makeSvg, setSvg, getBrightColor } from './utils.js';
-import { Dialog } from './dialog.js';
+import { Dialog, errorDialog } from './dialog.js';
 import { NODE_SCHEMA } from './model.js';
 import * as model from './model.js';
 import * as music from './music.js';
@@ -920,6 +920,33 @@ class UINode
     {
     }
 
+    /**
+     * Generate model.ConnectNodes instance
+     */
+    generateConnectAction(side, portIdx)
+    {
+        let editor = this.editor;
+
+        if (side == 'dst')
+        {
+            return new model.ConnectNodes(
+                editor.edge.srcNode.nodeId,
+                editor.edge.srcPort,
+                this.nodeId,
+                portIdx
+            );
+        }
+        else
+        {
+            return new model.ConnectNodes(
+                this.nodeId,
+                portIdx,
+                editor.edge.dstNode.nodeId,
+                editor.edge.dstPort
+            );
+        }
+    }
+
     /**
      * Setup DOM elements for this node
      */
@@ -1071,25 +1098,15 @@ class UINode
                 return;
             }
 
-            if (side == 'dst')
-            {
-                editor.model.update(new model.ConnectNodes(
-                    editor.edge.srcNode.nodeId,
-                    editor.edge.srcPort,
-                    this.nodeId,
-                    portIdx
-                ));
-            }
-            else
-            {
-                editor.model.update(new model.ConnectNodes(
-                    this.nodeId,
-                    portIdx,
-                    editor.edge.dstNode.nodeId,
-                    editor.edge.dstPort
-                ));
+            let connectAction = this.generateConnectAction(side, portIdx);
+
+            if (editor.model.detectCycles(connectAction)) {
+                errorDialog('This connection would create a cycle in the node graph.');
+                return;
             }
 
+            editor.model.update(connectAction);
+
             // Done connecting
             editor.edge = null;
         }

diff --git a/public/model.js b/public/model.js
@@ -1,4 +1,5 @@
 import { assert, isInt, isPosInt, treeCopy, treeEq, isString, isObject } from './utils.js';
+import { detectCycles } from './compiler.js';
 import * as music from './music.js';
 
 // Maximum number of undo steps we support
@@ -2022,6 +2023,18 @@ export class Model
         this.views.push(view);
     }
 
+    detectCycles(action)
+    {
+        // Slightly wasteful duplication allows us to avoid polluting the model's state before we've finished detection
+        let clone = new Model();
+        clone.deserialize(this.serialize());
+
+        // Simulate updating the model with the ConnectNodes action
+        action.update(clone);
+
+        return detectCycles(clone.state);
+    }
+
     // Reinitialize the state for a brand new project
     new()
     {