Siccity / xNode

Unity Node Editor: Lets you view and edit node graphs inside Unity
MIT License
3.34k stars 593 forks source link

NullReferenceException in SelectNode when mouseDown is followed by mouseUp too quickly because hoveredNode is null #367

Open YoriKv opened 1 year ago

YoriKv commented 1 year ago

When mouseDown is followed by mouseUp too quickly, exactly one frame apart, SelectNode causes a null reference exception.

                        // If click node header, select it.
                        if (currentActivity == NodeActivity.HoldNode && !(e.control || e.shift)) {
                            selectedReroutes.Clear();
                            SelectNode(hoveredNode, false);

This appears to be because of the following line of code: https://github.com/Siccity/xNode/blob/5967cef2770475cd75c1058cb4b118bbe60f3ee9/Scripts/Editor/NodeEditorGUI.cs#L521

On the frame that mouseDown is called, we get a frame that is the mouse event frame and on this line of code GUILayoutUtility.GetLastRect().size; returns (1, 1) for the size causing hoveredNode to be set to null and cause the exception on the next frame.

I'm not sure if this is the correct solution, but locally my solution was to change that line to this instead: Vector2 nodeSize = nodeSizes[node];

YoriKv commented 1 year ago

After a bit further testing, my above solution seems to fix the issue in 99% of cases but it still occasionally happens immediately after the window is re-created the first time you click on a node and therefore before the nodeSize is cached. Still a significant improvement, but not a complete fix. I'm not entirely sure why nodeSize isn't cached on previous repaints, but that seems to be what's happening.