hooks: one pending action per mouse action, so the drop swap actually runs

Houdini finishes pending actions with `for action in list: if
action.completeAction(ev): list.remove(action)` -- removal while iterating,
which skips the entry after the removed one. The grid sweep and the drop
swap both completed on the same mouseup, so the swap was skipped every
time and lingered until a later mouseup, where it fired with a stale start
position: swaps worked only some of the time. One _PendingMouseAction now
does the sweep and then the swap; check.py fails if a second class ever
completes on mouseup again.

Co-Authored-By: Claude Fable 5.1 <[email protected]>

 python3.13libs/nodegraphhooks.py | 98 ++++++++++++++++++++++------------------
 tools/check.py                   | 20 ++++++++
 2 files changed, 75 insertions(+), 43 deletions(-)

diff --git a/python3.13libs/nodegraphhooks.py b/python3.13libs/nodegraphhooks.py
index 74bbee8..1a82c70 100755
--- a/python3.13libs/nodegraphhooks.py
+++ b/python3.13libs/nodegraphhooks.py
@@ -36,53 +36,62 @@ class _PendingSelectionSyncAction(base.PendingDelayedAction):
         _syncSelection(HCNetworkEditor(self.editor), self.editor)
 
 
+# Houdini's event loop finishes pending actions with
+#
+#     for action in pending_actions:
+#         if action.completeAction(uievent):
+#             pending_actions.remove(action)
+#
+# -- removal while iterating, which skips the entry after the one removed. Two
+# hc actions completing on the same event therefore never both run on it: the
+# second lingers and fires on the next such event with stale data. Everything
+# hc wants done after a mouse action goes through the one class below, in
+# order, and tools/check.py holds it to that.
+
+
 class _PendingSnapAction(base.PendingAction):
-    """Sweep the network onto the grid once the action a hook-seen event
-    started has finished.
+    """Sweep the network onto the grid once the action a key hit started has
+    finished.
 
     The hook only sees an event when no handler is active, and the pending
     actions run after the handler on every later event. A key hit is handled
-    to completion inside the same event, so a keyboard sweep completes on the
-    first call, after layoutChildren or paste has placed its nodes. A mouse
-    action lasts until mouseup, which Houdini's move handler consumes -- but
-    the pending action still sees it, after applyAdjustments has written the
-    positions.
+    to completion inside the same event, so this completes on the first call,
+    after layoutChildren or paste has placed its nodes.
     """
 
-    def __init__(self, editor, from_keyboard):
+    def __init__(self, editor):
         base.PendingAction.__init__(self)
         self.editor = editor
-        self.from_keyboard = from_keyboard
 
     def completeAction(self, uievent):
-        done = self.from_keyboard or (
-            isinstance(uievent, MouseEvent) and uievent.eventtype == 'mouseup')
-        if done:
-            HCNetworkEditor(self.editor).sweepToGrid()
-        return done
+        HCNetworkEditor(self.editor).sweepToGrid()
+        return True
 
 
-class _PendingSwapAction(base.PendingAction):
-    """After a plain drag of one node ends, swap it with whatever it landed on.
+class _PendingMouseAction(base.PendingAction):
+    """Everything hc does once the mouse action a mousedown started has ended.
 
-    Queued on the mousedown that starts the drag, which is the last event the
-    hook sees before Houdini's move handler takes over; completes on the
-    mouseup that handler consumes, after it has written the new position.
+    Queued on the mousedown, the last event the hook sees before Houdini's
+    handler takes over; completes on the mouseup that handler consumes, after
+    applyAdjustments has written the new positions. First the grid sweep, then
+    -- when the press was a plain drag of one node -- the drop swap, which
+    must see the swept, final position.
     """
 
-    def __init__(self, editor, node_path, start_pos):
+    def __init__(self, editor, swap=None):
         base.PendingAction.__init__(self)
         self.editor = editor
-        self.node_path = node_path
-        self.start_pos = start_pos
+        self.swap = swap  # (node path, start position) or None
 
     def completeAction(self, uievent):
         if not (isinstance(uievent, MouseEvent) and uievent.eventtype == 'mouseup'):
             return False
-        # Alt was down at mousedown too when this was a copy drag, but the
-        # copy check here catches a modifier pressed mid-drag as well.
-        if not getattr(uievent.modifierstate, "alt", False):
-            HCNetworkEditor(self.editor).swapDroppedNode(self.node_path, self.start_pos)
+        hc_editor = HCNetworkEditor(self.editor)
+        hc_editor.sweepToGrid()
+        # Alt was checked at mousedown too when this was a copy drag; this
+        # catches a modifier pressed mid-drag as well.
+        if self.swap is not None and not getattr(uievent.modifierstate, "alt", False):
+            hc_editor.swapDroppedNode(*self.swap)
         return True
 
 
@@ -92,38 +101,44 @@ _NODE_BODY_SELECTORS = ('node', 'connectorarea', 'preview', 'footer',
                         'containerinput', 'containeroutput')
 
 
-def _queueDropSwap(editor, uievent, pending_actions):
+def _dropSwapFor(editor, uievent):
+    """(node path, start position) when this mousedown is a plain drag of one
+    node that may end in a swap, else None."""
     if not HCSettings().dropSwapEnabled():
-        return
+        return None
     mods = uievent.modifierstate
     if getattr(mods, "alt", False) or getattr(mods, "ctrl", False) or getattr(mods, "shift", False):
-        return  # copy, tree or selection-modifying drags: not a plain move
+        return None  # copy, tree or selection-modifying drags: not a plain move
     if not getattr(uievent.mousestate, "lmb", False):
-        return
+        return None
     selected = uievent.selected
     if selected is None or selected.name not in _NODE_BODY_SELECTORS:
-        return
+        return None
     node = selected.item
     if not isinstance(node, hou.Node) or node.parent() != editor.pwd():
-        return
+        return None
     # A press on one of several selected nodes drags the whole selection;
     # which of them "landed on" what is ambiguous, so only a lone node swaps.
     if node.isSelected() and len(editor.pwd().selectedItems()) > 1:
-        return
+        return None
+    pos = node.position()
+    return (node.path(), (pos[0], pos[1]))
+
+
+def _queueMouseAction(editor, uievent, pending_actions):
     for action in pending_actions:
-        if isinstance(action, _PendingSwapAction) and action.editor == editor:
+        if isinstance(action, _PendingMouseAction) and action.editor == editor:
             return
-    pos = node.position()
-    pending_actions.append(_PendingSwapAction(editor, node.path(), (pos[0], pos[1])))
+    pending_actions.append(_PendingMouseAction(editor, _dropSwapFor(editor, uievent)))
 
 
-def _queueSnapSweep(editor, pending_actions, from_keyboard):
+def _queueSnapSweep(editor, pending_actions):
     if not HCSettings().gridSnapEnabled():
         return
     for action in pending_actions:
         if isinstance(action, _PendingSnapAction) and action.editor == editor:
             return
-    pending_actions.append(_PendingSnapAction(editor, from_keyboard))
+    pending_actions.append(_PendingSnapAction(editor))
 
 
 def _queueSelectionSync(editor, pending_actions):
@@ -160,10 +175,7 @@ def createEventHandler(uievent, pending_actions):
 
     if isinstance(uievent, MouseEvent):
         if editor is not None and uievent.eventtype == 'mousedown':
-            # Order matters: both complete on the same mouseup, and the swap
-            # should see the dragged node's swept, final position.
-            _queueSnapSweep(editor, pending_actions, from_keyboard=False)
-            _queueDropSwap(editor, uievent, pending_actions)
+            _queueMouseAction(editor, uievent, pending_actions)
         # Everything below drives the hcnetcursor. With it off there is nothing
         # to move or refit: doing it anyway kept an invisible cursor tracking
         # the mouse and the selection, so turning it back on jumped it to
@@ -187,7 +199,7 @@ def createEventHandler(uievent, pending_actions):
         rawkey = str(getattr(uievent, "rawkey", "")).lower()
         eventtype = str(getattr(uievent, "eventtype", "")).lower()
         if editor is not None and eventtype == 'keyhit':
-            _queueSnapSweep(editor, pending_actions, from_keyboard=True)
+            _queueSnapSweep(editor, pending_actions)
         if (
             editor is not None
             and (key == "f" or rawkey == "f")
diff --git a/tools/check.py b/tools/check.py
index 9802032..70a9795 100644
--- a/tools/check.py
+++ b/tools/check.py
@@ -555,6 +555,26 @@ def check_nodegraph_hooks():
         check(f"{name} constructor arity",
               lambda n=name, nd=node, b=base_name: verify(n, nd, b))
 
+    def one_mouseup_completer():
+        """Houdini finishes pending actions with `for action in list: if
+        action.completeAction(ev): list.remove(action)` -- removal while
+        iterating, which skips the entry after the removed one. Two hc actions
+        completing on the same mouseup never both ran on it: the drop swap
+        lingered behind the grid sweep and fired on a later mouseup with a
+        stale start position, so swaps only worked some of the time. Whatever
+        runs after a mouse action has to share one class."""
+        completers = []
+        for name, node in hook_classes.items():
+            for item in node.body:
+                if isinstance(item, ast.FunctionDef) and item.name == "completeAction":
+                    if "mouseup" in ast.unparse(item):
+                        completers.append(name)
+        assert len(completers) == 1, \
+            f"{len(completers)} pending actions complete on mouseup: {completers}"
+        return f"only {completers[0]} completes on mouseup"
+
+    check("one mouseup completer", one_mouseup_completer)
+
 
 def check_geometry():
     """The visible-geometry walk, after HCNode was removed from under it."""