Better Cycle Detection and feedback

this commit adds a new-ish function that checks for cycles
during tree parsing (specifically while inserting lazy parents)
and it ensures the user sees meaningful error messages
for each kind of cycle

base_definitions.py

@@ -159,6 +159,10 @@ class MantisTree(NodeTree):
             finally:
                 scene.render.use_lock_interface = False
                 self.is_executing = False
+        if self.parsed_tree is None:
+            self.parsed_tree = {}
+            return 1 # failure
+        return 0 # success
 
     def display_update(self, context):
         if self.is_exporting:

execution/readtree.py

@@ -75,8 +75,9 @@ def insert_lazy_parents(mantis_node):
     from ..link_nodes import LinkInherit
     inherit_node = None
     if mantis_node.inputs["Relationship"].is_connected:
-        from ..utilities.node_common import trace_single_line
-        node_line, last_socket = trace_single_line(mantis_node, 'Relationship')
+        from ..utilities.node_common import trace_single_line_safe
+        # BUG this line can create infinite loops
+        node_line, last_socket = trace_single_line_safe(mantis_node, 'Relationship')
         # if last_socket is from a valid XFORM, it is the relationship in
         # because it was traversed from the xForm Out... so get the traverse target.
         if last_socket.traverse_target is None:
@@ -576,7 +577,7 @@ def parse_tree(base_tree, error_popups=False):
                     solved_nodes = {}
                     if error_popups == False:
                         raise e
-                    return # break out of this function regardless.
+                    return # it is not possible to parse the tree.
                 unsolved_schema.remove(n)
                 schema_solve_done.add(n)
                 for node in solved_nodes.values():
@@ -634,10 +635,17 @@ def parse_tree(base_tree, error_popups=False):
             init_connections(mantis_node) # because we have removed many connections.
 
         if (mantis_node.node_type in ['XFORM']) and ("Relationship" in mantis_node.inputs.keys()):
-            if (new_mantis_node := insert_lazy_parents(mantis_node)):
-                kept_mantis_node[new_mantis_node.signature]=new_mantis_node
-                # be sure to add the Mantis context.
-                new_mantis_node.mContext =mContext
+            from ..utilities.node_common import GraphError
+            try:
+                if (new_mantis_node := insert_lazy_parents(mantis_node)):
+                    kept_mantis_node[new_mantis_node.signature]=new_mantis_node
+                    # be sure to add the Mantis context.
+                    new_mantis_node.mContext =mContext
+            except GraphError as e:
+                e = execution_error_cleanup(mantis_node, e, show_error=error_popups)
+                if error_popups == False:
+                    raise e
+                return  # it is not possible to parse the tree.
         kept_mantis_node[mantis_node.signature]=mantis_node
     prWhite(f"Parsing tree took {time.time()-start_time} seconds.")
     prWhite("Number of Nodes: %s" % (len(kept_mantis_node)))
@@ -645,6 +653,14 @@ def parse_tree(base_tree, error_popups=False):
 
 from ..utilities.utilities import switch_mode
 
+
+def error_popup_draw_inner(layout, exception, ui_sig):
+    message = f"Error: {exception}\n" \
+              f"See node: {ui_sig[1:]}."
+    lines = message.splitlines()
+    for line in lines:
+        layout.label(text=line)
+
 def execution_error_cleanup(node, exception, switch_objects = [], show_error=False ):
     from bpy import  context
     ui_sig = None
@@ -671,8 +687,7 @@ def execution_error_cleanup(node, exception, switch_objects = [], show_error=Fal
                 pass
             finally:
                 def error_popup_draw(self, context):
-                    self.layout.label(text=f"Error: {exception}")
-                    self.layout.label(text=f"see node: {ui_sig[1:]}.")
+                    error_popup_draw_inner(self.layout, exception, ui_sig)
                 context.window_manager.popup_menu(error_popup_draw, title="Error", icon='ERROR')
     switch_mode(mode='OBJECT', objects=switch_objects)
     for ob in switch_objects:
@@ -680,7 +695,13 @@ def execution_error_cleanup(node, exception, switch_objects = [], show_error=Fal
     prRed(f"Error: {exception} in node {ui_sig}")
     return exception
 
-def sort_execution(nodes):
+def print_cycle(nodes):
+    ret_str = ""
+    for node in nodes:
+        ret_str+= node.__repr__() + '\n'
+    return ret_str
+
+def sort_execution(nodes, error_popups):
     from ..utilities.node_common import GraphError
     from collections import deque
     xForm_pass = deque()
@@ -705,8 +726,15 @@ def sort_execution(nodes):
             if child.unmet_hierarchy_dependency_count == 0:
                 xForm_pass.append(child)
     if processed_count < len(nodes):
+        cycle = []
+        for n in nodes.values():
+            if n.execution_prepared == False:
+                cycle.append(n)
         execution_failed = True
-        raise GraphError("detected a cycle in the tree")
+        e = GraphError("Detected a cycle in the tree, containing at "
+                         "least one of the following nodes: " + print_cycle(cycle))
+        e = execution_error_cleanup(cycle[-1], e, show_error=error_popups)
+        raise e
     return sorted_nodes, execution_failed
 
 def execute_tree(nodes, base_tree, context, error_popups = False, profile=False):
@@ -751,8 +779,15 @@ def execute_tree(nodes, base_tree, context, error_popups = False, profile=False)
         profiler.new_session(mContext.execution_id)
         sort_key = 'total'
     try:
-        sorted_nodes, execution_failed = sort_execution(nodes)
+        sorted_nodes, execution_failed = sort_execution(nodes, error_popups=error_popups)
+    except Exception as e:
+        # We'll fire the popup in sort_execution because it has the node in scope
+        if error_popups == False:
+            raise e
+        execution_failed = True
+    try:
         for n in sorted_nodes:
+            if execution_failed: break
             try:
                 check_and_apply_overlay(n)
                 if not n.prepared:

ops_nodegroup.py

@@ -299,7 +299,9 @@ class ExecuteNodeTree(Operator):
                 print(s.getvalue())
 
         else:
-            tree.update_tree(context, error_popups = pass_error)
+            update_failed = tree.update_tree(context, error_popups = pass_error)
+            if update_failed: # failed. Cannot execute.
+                return {"FINISHED"}
             tree.execute_tree(context, error_popups = pass_error)
         prGreen("Finished executing tree in %f seconds" % (time() - start_time))
         return {"FINISHED"}

utilities/node_common.py

@@ -11,6 +11,23 @@ from collections.abc import Callable
 from bpy.app import version as bpy_version
 
 
+# Utility to print links nicely for end user.
+def print_link_list(links):
+    ret_str = ""
+    for link in links:
+        ret_str+= link.__repr__() + '\n'
+    return ret_str
+
+# I've moved this to a separate function because the trace functions
+# are kind of hard to understand and the cycle check breaks the flow
+# of the code. 
+def detect_cycle_in_trace(link, link_circle):
+    if link.is_hierarchy:
+        if link in link_circle:
+            raise GraphError( "There is a cycle in the following links:\n" +
+                                print_link_list(link_circle))
+        link_circle.append(link)
+
 #TODO: refactor the socket definitions so this becomes unnecessary.
 def get_socket_value(node_socket):
     value = None
@@ -20,12 +37,38 @@ def get_socket_value(node_socket):
         value =  node_socket.TellValue()
     return value
 
-# TODO: modify this to work with multi-input nodes
-def trace_single_line(mantis_node, input_name, link_index=0):
+# NOTE this is a dupe of trace_single_line, and it must be used
+#      at any time before the tree is parsed (to check for cycles)
+#      I don't want or need to pay the performance cost to check this everywhere
+def trace_single_line_safe(mantis_node, input_name, link_index=0):
     # DO: refactor this for new link class
     """Traces a line to its input."""
     nodes = [mantis_node]
+    # a test for cyclic connections
+    link_circle = []
     # Trace a single line
+    if (socket := mantis_node.inputs.get(input_name) ):
+        while (socket.is_linked):
+            link = socket.links[link_index]; link_index = 0
+            # should throw an error if it is in the set
+            if detect_cycle_in_trace(link, link_circle):
+                return
+            if (socket := link.from_node.outputs.get(link.from_socket)):
+                nodes.append(socket.node)
+                if socket.can_traverse:
+                    socket = socket.traverse_target
+                else: # this is an output.
+                    break
+            else:
+                break
+    return nodes, socket
+
+
+def trace_single_line(mantis_node, input_name, link_index=0):
+    # DO: refactor this for new link class
+    """Traces a line to its input."""
+    nodes = [mantis_node]
+    # a test for cyclic connections
     if (socket := mantis_node.inputs.get(input_name) ):
         while (socket.is_linked):
             link = socket.links[link_index]; link_index = 0
@@ -39,7 +82,7 @@ def trace_single_line(mantis_node, input_name, link_index=0):
                 break
     return nodes, socket
 
-# this is same as the other, just flip from/to and in/out
+
 def trace_single_line_up(mantis_node, output_name,):
     """I use this to get the xForm from a link node."""
     nodes = [mantis_node]