Gentlymad-Studios / NewGraph

A general node graph solution centered on data management. This is based on the idea to visualize complex data structures as graph networks without having to modify already established data classes.
MIT License
244 stars 19 forks source link

Programmatically Created Ports Do Not Save #38

Closed akatriel closed 9 months ago

akatriel commented 9 months ago

I added a NewGraph.ContextMenu and called GraphSearchWindowProvider.AddNodeEntry to add a context menu option with an Action. In the action I am programmatically creating my nodes and ports and connecting them. Also adding VisualElements to the NodeView. All are generated fine, however, after making any changes GraphController.Load(IGraphModel graphModel) is called and seems to wipe out all my changes. I even added a call to GraphController.EnsureSerialization() to ensure serialization of the changes, but they do not persist. I am using a custom data class that inherits from INode and is decorated with the necessary attributes. Could you help with this?

Doppelkeks commented 9 months ago

Sure, can you share some code so I can take a look?

akatriel commented 9 months ago

I am constructing a directed graph for a collection of Jobs and each Job has a list of dependent Jobs. At this time they are acyclic. The Job class is a ScriptableObject.

NodeView:

   [Serializable, Node()]
    public class JobNodeData : INode
    {
        [SerializeReference]
        public Job Job;

        [PortList, SerializeReference]
        public List<JobNodeData> Output;
    }

Context Menu:

using System;

[CustomContextMenu]
public class JobGraphContextMenu : NewGraph.ContextMenu
{
    protected override string GetHeader() => "Job Graph Menu";
    protected override void AddNodeEntries()
    {
        base.AddNodeEntries();
        AddSeparator("Automation");
        AddNodeEntry("Create Job Graph", ConstructJobGraph);
    }

    private void ConstructJobGraph(object _)
    {
        var classes = Resources.LoadAll<Job>("");

        var entryLevelJobs = classes.Where(i => !i.JobDependencies.Any());

        Dictionary<Job, NodeView> createdNodes = new(); 
        foreach (var job in entryLevelJobs)
        {
            NodeView leafNode = CreateNode(job);
            createdNodes.Add(job, leafNode);

            foreach (var dependent in classes.Where(jc => jc.JobDependencies.Contains(job)))
            {
                if (!createdNodes.ContainsKey(dependent))
                {
                    NodeView dependentNode = CreateNode(dependent);

                    PortView leafPort = leafNode.CreatePortUI(
                        CreatePortInfo(dependent),
                        dependentNode.controller.nodeItem.GetSpecificSerializedProperty());

                    graphController.ConnectPorts(dependentNode.inputPort, leafPort);
                    createdNodes.Add(dependent, dependentNode);
                }
            }
        }

        graphController.graphData.ForceSerializationUpdate();

        NodeView CreateNode(Job job)
        {
            var view = graphController.CreateNewNode(typeof(JobNodeData));
            if (view.controller.nodeItem.nodeData is JobNodeData data)
            {
                data.Job = job;
                view.TitleContainer.Add(new Label(data.Job.name));
            }
            return view;
        }
        static PortInfo CreatePortInfo(Job dependent) => new(
            string.Empty,
            typeof(JobNodeData),
            new PortBaseAttribute(
                name: dependent.JobType.ToString(),
                direction: PortDirection.Output));
    }
}

Also, I tried creating a custom editor for the NodeView by extending NodeEditor but that seemed to overwrite all my scripted changes.

Any help appreciated!

Doppelkeks commented 9 months ago

Not sure if this is 100% in line with what you wanted to achieve but I'm confident that you can get things rolling this way: image

using NewGraph;
using System.Collections.Generic;
using UnityEngine;

[CustomContextMenu]
public class CustomMenu : NewGraph.ContextMenu {
    protected override string GetHeader() => "Job Graph Menu";

    protected override void AddNodeEntries() {
        base.AddNodeEntries();
        AddSeparator("Automation");
        AddNodeEntry("Create Job Graph", ConstructJobGraph);
    }

    private void ConstructJobGraph(object _) {
        Job[] classes = Resources.LoadAll<Job>("");
        Dictionary<Job, NodeView> createdNodes = new();

        // go through each job...
        foreach (Job parentJob in classes) {
            // make sure every parent gets its own node view equivalent...
            (JobNodeData, NodeView) parentNodeInfo = CreateOrGetNodeView(parentJob);

            // go through the list of dependencies for the given node
            foreach (Job dependentJob in parentJob.JobDependencies) {
                // make sure every node gets its own node view equivalent...
                (JobNodeData, NodeView) dependentNodeInfo = CreateOrGetNodeView(dependentJob);
                // recreate the job dependencies list of the JobDataNode (ports & connections are automatically resolved)
                parentNodeInfo.Item1.JobDependencies.Add(dependentNodeInfo.Item1);
            }
        }

        (JobNodeData, NodeView) CreateOrGetNodeView(Job job) {
            JobNodeData data;
            NodeView view;

            // make sure a corresponding node is only created once
            if (!createdNodes.ContainsKey(job)) {
                // create node and setup data
                view = graphController.CreateNewNode(typeof(JobNodeData));
                data = view.controller.nodeItem.nodeData as JobNodeData;
                data.Job = job;

                // delay one frame, so we can savely overwrite the default name of a created node
                view.schedule.Execute(() => {
                    // don't operate directly in the ui as this won't serialize the node name
                    // use SetName instead
                    view.controller.nodeItem.SetName(data.Job.name);
                });

                createdNodes.Add(job, view);
                return (data, view);
            }

            view = createdNodes[job];
            data = view.controller.nodeItem.nodeData as JobNodeData;
            return (data, view);
        }
    }
}

The main thing to understand is that usually you don't want to draw ports and stuff manually as all of this won't serialize until you write your custom logic for it. The main idea here is that you can instead just setup your data and nodes by referencing it and let NewGraph take care of the rest.

Additionally, you might not even need Job scriptable objects and JobNodeData. Depending on your scenario, you might be able to turn your Job class directly into a node as the graph would serialize it right away and references would stay intact. If however you need the scriptable object structure for other purposes you can still go down this route by converting your SOs into JobDataNode equivalents.

akatriel commented 9 months ago

This doesn't seem to run for me. The error I got was: InvalidOperationException: Collection was modified; enumeration operation may not execute. This is because you can't modify a collection inside a foreach loop -> parentNodeInfo.Item1.JobDependencies.Add(dependentNodeInfo.Item1);

I need to be able to able to use an already created list that is in sync with the editor. My eventual goal is to be able to modify an already created ScriptableObject and it's data from the graph. In other words, add dependencies to a job from the graph and sync the changes from the editor (bidirectionally).

I think the issue here is that in your example the list of dependencies on the job is of the type JobNodeData. In my example, I have field for the Job that contains it's own field for the dependencies, and then another for the port outputs of the type JobNodeData.

Additionally, you might not even need Job scriptable objects and JobNodeData. Depending on your scenario, you might be able to turn your Job class directly into a node as the graph would serialize it right away and references would stay intact. If however you need the scriptable object structure for other purposes you can still go down this route by converting your SOs into JobDataNode equivalents.

Wouldn't this make the view and the data the same thing when you call graphController.CreateNewNode(typeof(Job)?

The main thing to understand is that usually you don't want to draw ports and stuff manually as all of this won't serialize until you write your custom logic for it.

What kind of custom logic?

I appreciate you taking the time to help with this.

Doppelkeks commented 9 months ago

Wouldn't this make the view and the data the same thing when you call graphController.CreateNewNode(typeof(Job)?

Yes, if your Job class can implement the INode interface, you can turn it directly into a node without the need for JobNodeData. If that works, you could avoid having to manually translate your data into graph elements. This was one of my main goals with this framework, your nodes can are your data.🙂But depending on your use case this might or might not be feasible...

I think the issue here is that in your example the list of dependencies on the job is of the type JobNodeData. In my example, I have field for the Job that contains it's own field for the dependencies, and then another for the port outputs of the type JobNodeData.

[Serializable, Node()]
 public class JobNodeData : INode
 {
     [SerializeReference]
     public Job Job;

     [PortList, SerializeReference]
     public List<JobNodeData> Output;
 }

In your example code for JobNodeData you created a List [PortList] and never used it. In my implementation I simply filled it with the converted JobNodeData equivalents of the dependencies and named it Dependencies instead Output. I have attached the whole example so you can investigate the differences further. 🙂 JobNodes.zip

akatriel commented 9 months ago

My bad, the naming got me :/ Your solution works nicely. Thanks for the support!