IssueOnRemoveConnection() not called when replacing connection

[No3371]

Hi, I'm adding a ConnectionManager to the framework so we can directly check and manipulate connections, ex: select a connection to remove on a multi-input port.

Then I noticed that it seems that at the moment when users applying a new connection to a single input that already connected, it does not call the remove connection callback for the port that already connected to the target port, is this intended?

[Seneral]

No, so thanks for the note! :)

[No3371]

I have some issue when testing this ConnectionManager thing. Basically my connection selecting logic is: Right click on knobs and drag, for every 20 pixel of y offset, select next connection on this knob (the line color changes to show it's selected), then after selection made, show a context menu of action to perform (ex: Delete this connection)... or if we don't have other needs just delete it on right mouse up)

But I can't get this handler to run:

        [EventHandlerAttribute (EventType.MouseDown, 105)] // Priority over hundred to make it call after the GUI
        private static void HandlePortDraggingStart (NodeEditorInputInfo inputInfo) 
        {
            if (GUIUtility.hotControl > 0)
                return; // GUI has control

            NodeEditorState state = inputInfo.editorState;
            if (inputInfo.inputEvent.button == 1 && inputInfo.editorState.selectedConnectionHash == string.Empty && inputInfo.editorState.focusedConnectionKnob != null) 
            { // Clicked inside the selected Node, so start dragging it
                inputInfo.editorState.selectedConnectionKnob = inputInfo.editorState.focusedConnectionKnob;
                state.selectingConnection = true;
                state.StartDrag ("port", inputInfo.inputPos, Vector2.zero);
                    Debug.Log("start");
            }
        }

I doubt it's because:

        /// <summary>
        /// Returns the node at the specified canvas-space position in the specified editor and returns a possible focused knob aswell
        /// </summary>
        public static Node NodeAtPosition (NodeEditorState editorState, Vector2 canvasPos, out ConnectionKnob focusedKnob)
        {
            focusedKnob = null;
            if (editorState == null || NodeEditorInputSystem.shouldIgnoreInput (editorState))
                return null;
            NodeCanvas canvas = editorState.canvas;
            for (int nodeCnt = canvas.nodes.Count-1; nodeCnt >= 0; nodeCnt--) 
            { // Check from top to bottom because of the render order
                Node node = canvas.nodes [nodeCnt];
                if (node.ClickTest (canvasPos, out focusedKnob))
                    return node; // Node is clicked on
            }
            return null;
        }

But not sure yet and gotta sleep. Maybe I'll see an answer right there when I woke up? ;)

BTW, the reason I wanna try build this ConnectionManager is because I believe that collision detecting would be rather heavy and complicated to implement, and you need to store every Connections into objects... maybe? However I haven't find anything that need to be stored in connections themselves besides the 2 ports it connected.

This ConnectionManager uses Dictionary<string, data> where the string is a hash that generated by port1.getInstaceID().ToString() + port1.getInstaceID().ToString(). It raverse all nodes when a canvas is loaded, also utilize nice Add/Remove Connection callback you put in there to update changes in the dictionary.

So this manager stuff should be enough to select connections without collision detection. Kinda boring, though. I really think the UX of cutting the connection with a drag is really good.

[Seneral]

Ok, interesting approach... I do think collision detection is viable, it can be optimized quite alot (BoundingBox check first) and in the editor it's not too big of a problem. No objects per connection, although I do hope to atleast SUPPORT custom data per connection. I'll definitely won't make that mandatory though. For stuff like statemachine transitions with conditions it is a requirement though. Perhaps I'll make a completely new system for that that does NOT have ports but only a single connection knob, which works for the typical transition concept...

As for your question, I suppose you have checked if the handler method itself runs? If not, it's probably interfering with an other handler that consumes the event (event.Use()) higher up in the priority. To fix that, you'll need a higher priority (<105) and make sure you handle the event apropriately and only use it when it's required so you in turn don't block any other handlers. Makes sense, I hope:)

[Seneral]

Ok, error fixed. Still keeping open for your issue though:)

[No3371]

Wooo I got it work! Imgur Should work even on multi-input node. If you are interested: Portal

[Seneral]

Good work, works really well + good code:) One thing, in NodeEditorInputControls.HandleConnectionDrawing, you overwrote a previous commit that allows drawing connections from unconnected single-knobs. Definitely useful... Still prefer the other way of editing, but for now a good workaround. Thanks!

[RomanZhu]

Works great, thank you