Unhandled errors corrupt graph data and sometimes crash editor

[Appleguysnake]

There are a few related errors I've been dealing with while learning my way around NewGraph. I'm grouping them here because they're related and hopefully have some common point where they can all be caught together.

Main issue

When there's a problem with the use of attributes or saved graph data, the graph data can sometimes be unrecoverable resulting in cases where a lot of work can be lost due to a typo or other simple mistake.

Sometimes on recompile the graph window is cleared completely and the errors in the console give little indication of what actually went wrong so it's hard to tell where the error came from. In some cases the nodes load, but can't be used and the graph seems to load thousands of invisible nodes. In others, the only way to fix the graph is to delete it and recreate it from scratch.

Case 1 : Simple Value fields

Using this code

[Serializable, Node]
public class NodeTest2 : INode
{
    [SerializeReference, Port]
    public float num;
}

Adding a single node to a new empty graph results in the node being created and no errors in the console. It can be moved around and assigned as a reference to another node's output without issue. But trying to delete it throws an InvalidOperationException and fails. Saving the graph and trying to open it — or just recompiling code — throws the same error, and then throws an ArgumentException every update. The result looks like this. Only one node was added, the others are from trying to drag it. The node count in the inspector is from a box selection, which matches the count of console errors. It appears a new node is added every time even though loading the data failed. The errors:

InvalidOperationException: managedReferenceValue is only available on fields with the [SerializeReference] attribute
UnityEditor.SerializedProperty.get_managedReferenceValue () (at <80a8ce1980c648dca8e68f0d8aa3b930>:0)
NewGraph.GraphController+<>c__DisplayClass43_0.<Load>b__2 (GraphViewBase.BaseNode node) (at Assets/NewGraph/Editor/Controllers/GraphController.cs:503)
NewGraph.GraphView.ForEachNodeDo (System.Action`1[T] callback) (at Assets/NewGraph/Editor/Views/GraphView.cs:117)
NewGraph.GraphController+<>c__DisplayClass43_0.<Load>b__0 () (at Assets/NewGraph/Editor/Controllers/GraphController.cs:497)
UnityEngine.UIElements.VisualElement+SimpleScheduledItem.PerformTimerUpdate (UnityEngine.UIElements.TimerState state) (at <332857d8803a4878904bcf8f9581ec33>:0)
UnityEngine.UIElements.TimerEventScheduler.UpdateScheduledEvents () (at <332857d8803a4878904bcf8f9581ec33>:0)
UnityEngine.UIElements.UIElementsUtility.UnityEngine.UIElements.IUIElementsUtility.UpdateSchedulers () (at <332857d8803a4878904bcf8f9581ec33>:0)
UnityEngine.UIElements.UIEventRegistration.UpdateSchedulers () (at <332857d8803a4878904bcf8f9581ec33>:0)
UnityEditor.RetainedMode.UpdateSchedulers () (at <cc76bab7efe9480f901125fd04a708b6>:0)

ArgumentException: An item with the same key has already been added. Key: NodeTest2
System.Collections.Generic.Dictionary`2[TKey,TValue].TryInsert (TKey key, TValue value, System.Collections.Generic.InsertionBehavior behavior) (at <27124aa0e30a41659b903b822b959bc7>:0)
System.Collections.Generic.Dictionary`2[TKey,TValue].Add (TKey key, TValue value) (at <27124aa0e30a41659b903b822b959bc7>:0)
NewGraph.GraphController+<>c__DisplayClass43_0.<Load>g__ForEachNodeProperty|1 (System.Collections.Generic.List`1[T] nodes, UnityEditor.SerializedProperty nodesProperty) (at Assets/NewGraph/Editor/Controllers/GraphController.cs:483)
NewGraph.GraphController+<>c__DisplayClass43_0.<Load>b__0 () (at Assets/NewGraph/Editor/Controllers/GraphController.cs:489)
UnityEngine.UIElements.VisualElement+SimpleScheduledItem.PerformTimerUpdate (UnityEngine.UIElements.TimerState state) (at <332857d8803a4878904bcf8f9581ec33>:0)
UnityEngine.UIElements.TimerEventScheduler.UpdateScheduledEvents () (at <332857d8803a4878904bcf8f9581ec33>:0)
UnityEngine.UIElements.UIElementsUtility.UnityEngine.UIElements.IUIElementsUtility.UpdateSchedulers () (at <332857d8803a4878904bcf8f9581ec33>:0)
UnityEngine.UIElements.UIEventRegistration.UpdateSchedulers () (at <332857d8803a4878904bcf8f9581ec33>:0)
UnityEditor.RetainedMode.UpdateSchedulers () (at <cc76bab7efe9480f901125fd04a708b6>:0)

Case 1a: Value field on nested class

[Serializable, Node]
public class NodeTest2 : INode
{
    public NodeTestValue next;
}
[Serializable, Node]
public class NodeTestValue : INode
{
    [SerializeReference, Port]
    public float num;
}

Case 2: UnityEngine.Object

Same as above (probably the same for anything unity can serialize). Only notable because the errors are less clear than if you try to make a node from a monobehavior (see #41). Code used:

[Serializable, Node]
public class NodeTest2 : INode
{
    [SerializeReference, Port]
    public NodeTestValue next;
}
public class NodeTestValue : MonoBehaviour, INode { }

Case 3: Changed data structure (crash)

• Create a two node graph with the code below
• Change NodeTest3 to inerit from UnityEngine.Object and keep INode. (or anything Unity can serialize, I haven't tested everything)
• Recompile changes in Unity

  [Serializable, Node]
  public class NodeTest2 : INode
  {
  [SerializeReference, Port]
  public NodeTest3 something;
  }
  [Serializable, Node]
  public class NodeTest3 : INode 
  {
  public object obj;
  }

  The result is similar to above, with the same two errors, but with an additional one that happens every inspector update as long as the gameobject with the MonoGraphModel component is open, even when the graph window is closed. Update: This appears to be thrown by Unity if you assign an object to an interface field with SerializeReference on it, but not while the field is null. Not sure why it's being thrown in this situation since there's no interface field being drawn in the inspector.

  Fields with [SerializeReference] cannot serialize objects that derive from Unity.Object, in this case: Assembly-CSharp NodeTest3

  In this case, the editor crashed when trying to reopen the graph. The same might happen with the other bugs and might be a result of how many nodes were created but I don't have time to test that in detail right now.

Case 4: Null properties

I unfortunately have to go so I can't finish figuring out the exact reproduction steps, but I have a file with more console errors related to null references and things. But you probably get the gist by now.

For this one I modified GraphController to catch null properties, but it's just a quick fix for the constant loading null nodes, it doesn't actually allow the graph to load the non-corrupt nodes.

void ForEachNodeProperty(List<NodeModel> nodes, SerializedProperty nodesProperty) {
                    // go over every node...
                    for (int i = 0; i < nodes.Count; i++) {
                        NodeModel node = nodes[i];

            if(node.nodeData == null)
            {
                Debug.LogError("Trying to load null nodeData. Probably an attribute error");
                return;
            }

                        // initialize the node...
                        node.Initialize();

The values to handle are here https://docs.unity3d.com/ScriptReference/Serialization.ManagedReferenceUtility.html

Expected behavior

I don't know UIToolkit enough to know what's reasonable to expect or implement, but anything that avoids having to delete the graph data (or manually edit the .asset file) would be a huge improvement.

[kajloevesijn]

only INode can be connected / used without a problem, no other datatypes can be used as a port.

workarounds like generics don't make it possible either. (example below)

[Serializable, Node("200020ff", "types", createInputPort = false)]
public class FloatGraphNode : INode
{
    [SerializeReference, Port(capacity = Capacity.Single, connectionPolicy = ConnectionPolicy.IdenticalOrSubclass, direction = PortDirection.Output)]
    public Signal<float> output;

    [SerializeReference, Port(capacity = Capacity.Single, connectionPolicy = ConnectionPolicy.IdenticalOrSubclass, direction = PortDirection.Input)]
    public Signal<float> input;

}

[Node("200020ff", "types", createInputPort = false)]
public class Signal<T> : INode
{
    public T value = default;
}

[kajloevesijn]

anyway it makes sense because NewGraph is only visual. Port is an attribute, it doesn't inherently allow you to communicate data. so it makes sense that the only thing you can connect together are other "INode"'s.