Seneral / Node_Editor_Framework

A flexible and modular Node Editor Framework for creating node based displays and editors in Unity
https://nodeeditor.seneral.dev
MIT License
2.01k stars 414 forks source link

Fixed null pointer on node. #71

Closed aphex- closed 8 years ago

aphex- commented 8 years ago

For some reason I get a NullReferenceException if I start Unity with the Node_Editor. A node in curNodeCanvas.nodes seems to be null. This leads to a frozen node window.

Unity 5.3.4f1 Personal (Mac)

NullReferenceException:` Object reference not set to an instance of an object
NodeEditorFramework.NodeEditor.DrawSubCanvas (NodeEditorFramework.NodeCanvas nodeCanvas, NodeEditorFramework.NodeEditorState editorState) (at Assets/Node_Editor/Framework/NodeEditor.cs:223)
NodeEditorFramework.NodeEditor.DrawCanvas (NodeEditorFramework.NodeCanvas nodeCanvas, NodeEditorFramework.NodeEditorState editorState) (at Assets/Node_Editor/Framework/NodeEditor.cs:131)
NodeEditorFramework.NodeEditorWindow.OnGUI () (at Assets/Editor/Node_Editor/NodeEditorWindow.cs:127)
System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) (at /Users/builduser/buildslave/mono/build/mcs/class/corlib/System.Reflection/MonoMethod.cs:222)

To fix this issue I checked for null on the nodes.

Seneral commented 8 years ago

This seems to happen in some special cases, everytime I've encountered this I tried to fix the cause instead but there seems to be an unpredictable amount of occasions where this happens...

Though there are many more cases where the nodes get accessed, so instead of adding a check in each case, I'd propose to remove the node list entry before anything gets accessed (in the drawing function after background and scaling but before input and drawing). If such a null entry was found, we also need to check if the NodeKnobs that belonged to the node are still left somewhere, referencing a null body, which would also cause an error. So we'd need to iterate over each node and it knobs if such a null entry was found (not every time).

Can you add that while you're at it? That's be more solid I hope:)

aphex- commented 8 years ago

My solution is just a hotfix. I thought it would be a good idea to have it fixed to avoid crashes for new users.I would love to fix the cause of it but I am very new to this project. I do not know how much time I will spend to understand the internals. Wouldn't it be the best to avoid to set null entries instead of cleaning the list at some point? Anyway.. like I said I am new to this :)

Seneral commented 8 years ago

Ok I can make an additional fix for this later:)