Improve connecting link logic (#697)

### Current

- Connections are disconnected the moment a link starts being dragged
- Reseating a connection where it came from creates a new connection
- If the process is somehow interrupted, the links are already gone

### Proposed

- Connection is disconnected after a new connection is made
- Rendering is bypassed for the link segment being moved
- Does nothing if dropping a link exactly where it came from
- Adds early return when trying to connect a node to itself

Author: webfiltered

src/LGraphCanvas.ts

@@ -69,7 +69,7 @@ import {
   TitleMode,
 } from "./types/globalEnums"
 import { alignNodes, distributeNodes, getBoundaryNodes } from "./utils/arrange"
-import { findFirstNode, getAllNestedItems } from "./utils/collections"
+import { findFirstNode, getAllNestedItems, isDraggingLink } from "./utils/collections"
 import { toClass } from "./utils/type"
 import { WIDGET_TYPE_MAP } from "./widgets/widgetMap"
 
@@ -2253,6 +2253,7 @@ export class LGraphCanvas implements ConnectionColorContext {
                   output: null,
                   pos,
                   direction: LinkDirection.RIGHT,
+                  link,
                 })
               }
 
@@ -2321,17 +2322,20 @@ export class LGraphCanvas implements ConnectionColorContext {
                   output: linked_node.outputs[slot],
                   pos: linked_node.getConnectionPos(false, slot),
                   afterRerouteId: link_info.parentId,
+                  link: link_info,
                 }
-                this.connecting_links = [connecting]
+                const connectingLinks = [connecting]
+                this.connecting_links = connectingLinks
 
                 pointer.onDragStart = () => {
                   connecting.output = linked_node.outputs[slot]
                 }
                 pointer.onDragEnd = (upEvent) => {
-                  if (this.allow_reconnect_links && !LiteGraph.click_do_break_link_to) {
+                  const shouldDisconnect = this.#processConnectingLinks(upEvent, connectingLinks)
+
+                  if (shouldDisconnect && this.allow_reconnect_links && !LiteGraph.click_do_break_link_to) {
                     node.disconnectInput(i)
                   }
-                  this.#processConnectingLinks(upEvent)
                   connecting.output = linked_node.outputs[slot]
                   this.connecting_links = null
                 }
@@ -2712,7 +2716,7 @@ export class LGraphCanvas implements ConnectionColorContext {
                 // Node background / title under the pointer
                 if (!linkOverWidget) {
                   const targetSlotId = firstLink.node.findConnectByTypeSlot(true, node, firstLink.output.type)
-                  if (targetSlotId !== null && targetSlotId >= 0) {
+                  if (targetSlotId !== undefined && targetSlotId >= 0) {
                     node.getConnectionPos(true, targetSlotId, pos)
                     highlightPos = pos
                     highlightInput = node.inputs[targetSlotId]
@@ -2739,7 +2743,7 @@ export class LGraphCanvas implements ConnectionColorContext {
               if (inputId === -1 && outputId === -1) {
                 const targetSlotId = firstLink.node.findConnectByTypeSlot(false, node, firstLink.input.type)
 
-                if (targetSlotId !== null && targetSlotId >= 0) {
+                if (targetSlotId !== undefined && targetSlotId >= 0) {
                   node.getConnectionPos(false, targetSlotId, pos)
                   highlightPos = pos
                 }
@@ -2912,7 +2916,7 @@ export class LGraphCanvas implements ConnectionColorContext {
 
       if (this.connecting_links?.length) {
         // node below mouse
-        this.#processConnectingLinks(e)
+        this.#processConnectingLinks(e, this.connecting_links)
       } else {
         this.dirty_canvas = true
 
@@ -2944,25 +2948,33 @@ export class LGraphCanvas implements ConnectionColorContext {
     return
   }
 
-  #processConnectingLinks(e: CanvasPointerEvent) {
-    const { graph, connecting_links } = this
+  #processConnectingLinks(e: CanvasPointerEvent, connecting_links: ConnectingLink[]): boolean | undefined {
+    const { graph } = this
     if (!graph) throw new NullGraphError()
-    if (!connecting_links) return
 
     const { canvasX: x, canvasY: y } = e
     const node = graph.getNodeOnPos(x, y, this.visible_nodes)
     const firstLink = connecting_links[0]
 
     if (node) {
+      let madeNewLink: boolean | undefined
+
       for (const link of connecting_links) {
         // dragging a connection
         this.#dirty()
 
+        // One should avoid linking things to oneself
+        if (node === link.node) continue
+
         // slot below mouse? connect
         if (link.output) {
           const slot = this.isOverNodeInput(node, x, y)
           if (slot != -1) {
-            link.node.connect(link.slot, node, slot, link.afterRerouteId)
+            // Trying to move link onto itself
+            if (link.link?.target_id === node.id && link.link?.target_slot === slot) return
+
+            const newLink = link.node.connect(link.slot, node, slot, link.afterRerouteId)
+            madeNewLink ||= newLink !== null
           } else if (this.link_over_widget) {
             this.emitEvent({
               subType: "connectingWidgetLink",
@@ -2974,28 +2986,33 @@ export class LGraphCanvas implements ConnectionColorContext {
           } else {
             // not on top of an input
             // look for a good slot
-            link.node.connectByType(link.slot, node, link.output.type, {
-              afterRerouteId: link.afterRerouteId,
-            })
+            const slotIndex = link.node.findConnectByTypeSlot(true, node, link.output.type)
+            if (slotIndex !== undefined) {
+              // Trying to move link onto itself
+              if (link.link?.target_id === node.id && link.link?.target_slot === slotIndex) return
+
+              const newLink = link.node.connect(link.slot, node, slotIndex, link.afterRerouteId)
+              madeNewLink ||= newLink !== null
+            }
           }
         } else if (link.input) {
           const slot = this.isOverNodeOutput(node, x, y)
 
-          if (slot != -1) {
+          const newLink = slot != -1
             // this is inverted has output-input nature like
-            node.connect(slot, link.node, link.slot, link.afterRerouteId)
-          } else {
+            ? node.connect(slot, link.node, link.slot, link.afterRerouteId)
             // not on top of an input
             // look for a good slot
-            link.node.connectByTypeOutput(
+            : link.node.connectByTypeOutput(
               link.slot,
               node,
               link.input.type,
               { afterRerouteId: link.afterRerouteId },
             )
-          }
+          madeNewLink ||= newLink !== null
         }
       }
+      return madeNewLink
     } else if (firstLink.input || firstLink.output) {
       // For external event only.
       const linkReleaseContextExtended: LinkReleaseContextExtended = {
@@ -3033,6 +3050,7 @@ export class LGraphCanvas implements ConnectionColorContext {
           }
         }
       }
+      return true
     }
   }
 
@@ -4897,6 +4915,8 @@ export class LGraphCanvas implements ConnectionColorContext {
         const link = this.graph._links.get(link_id)
         if (!link) continue
 
+        const draggingLink = isDraggingLink(link.id, this.connecting_links)
+
         // find link info
         const start_node = this.graph.getNodeById(link.origin_id)
         if (start_node == null) continue
@@ -4960,6 +4980,8 @@ export class LGraphCanvas implements ConnectionColorContext {
               const startPos = prevReroute?.pos ?? start_node_slotpos
               reroute.calculateAngle(this.last_draw_time, this.graph, startPos)
 
+              // Skip the first segment if it is being dragged
+              if (j === 0 && draggingLink?.input) continue
               this.renderLink(
                 ctx,
                 startPos,
@@ -4984,6 +5006,9 @@ export class LGraphCanvas implements ConnectionColorContext {
             startControl = [dist * reroute.cos, dist * reroute.sin]
           }
 
+          // Skip the last segment if it is being dragged
+          if (draggingLink?.output) continue
+
           // Use runtime fallback; TypeScript cannot evaluate this correctly.
           const segmentStartPos = points.at(-2) ?? start_node_slotpos
 
@@ -5000,7 +5025,8 @@ export class LGraphCanvas implements ConnectionColorContext {
             end_dir,
             { startControl },
           )
-        } else {
+        // Skip normal render when link is being dragged
+        } else if (!draggingLink) {
           this.renderLink(
             ctx,
             start_node_slotpos,

src/LGraphNode.ts

@@ -2170,7 +2170,7 @@ export class LGraphNode implements Positionable, IPinnable, IColorable {
     node: LGraphNode,
     slotType: ISlotType,
     options?: ConnectByTypeOptions,
-  ): number | null {
+  ): number | undefined {
     // LEGACY: Old options names
     if (options && typeof options === "object") {
       if ("firstFreeIfInputGeneralInCase" in options) options.wildcardToTyped = !!options.firstFreeIfInputGeneralInCase
@@ -2188,7 +2188,7 @@ export class LGraphNode implements Positionable, IPinnable, IColorable {
 
     if (node && typeof node === "number") {
       const nodeById = this.graph.getNodeById(node)
-      if (!nodeById) return null
+      if (!nodeById) return
 
       node = nodeById
     }
@@ -2217,7 +2217,6 @@ export class LGraphNode implements Positionable, IPinnable, IColorable {
         : node.findOutputSlotFree(opt)
       if (nonEventSlot >= 0) return nonEventSlot
     }
-    return null
   }
 
   /**
@@ -2239,7 +2238,7 @@ export class LGraphNode implements Positionable, IPinnable, IColorable {
       target_slotType,
       optsIn,
     )
-    if (slotIndex !== null)
+    if (slotIndex !== undefined)
       return this.connect(slot, target_node, slotIndex, optsIn?.afterRerouteId)
 
     console.debug("[connectByType]: no way to connect type:", target_slotType, "to node:", target_node)
@@ -2270,7 +2269,7 @@ export class LGraphNode implements Positionable, IPinnable, IColorable {
       source_slotType,
       optsIn,
     )
-    if (slotIndex !== null)
+    if (slotIndex !== undefined)
       return source_node.connect(slotIndex, this, slot, optsIn?.afterRerouteId)
 
     console.debug("[connectByType]: no way to connect type:", source_slotType, "to node:", source_node)

src/interfaces.ts

@@ -290,6 +290,8 @@ export interface ConnectingLink extends IInputOrOutput {
   pos: Point
   direction?: LinkDirection
   afterRerouteId?: RerouteId
+  /** The link being moved, or `undefined` if creating a new link. */
+  link?: LLink
 }
 
 interface IContextMenuBase {

src/utils/collections.ts

@@ -1,4 +1,5 @@
-import type { Positionable } from "../interfaces"
+import type { ConnectingLink, Positionable } from "../interfaces"
+import type { LinkId } from "@/LLink"
 
 import { LGraphNode } from "@/LGraphNode"
 
@@ -35,3 +36,13 @@ export function findFirstNode(items: Iterable<Positionable>): LGraphNode | undef
     if (item instanceof LGraphNode) return item
   }
 }
+
+/** @returns `true` if the provided link ID is currently being dragged. */
+export function isDraggingLink(linkId: LinkId, connectingLinks: ConnectingLink[] | null | undefined): ConnectingLink | undefined {
+  if (connectingLinks == null) return
+
+  for (const connectingLink of connectingLinks) {
+    if (connectingLink.link == null) continue
+    if (linkId === connectingLink.link.id) return connectingLink
+  }
+}