Siccity / xNode

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

NodeGraph.Copy() does not clone NodePorts #172

Closed taylank closed 5 years ago

taylank commented 5 years ago

Using NodeGraph.Copy() I've noticed that while the graph and its nodes are instantiated, NodePorts still have data pertaining to the original graph.

/// <summary> Swap connected nodes from the old list with nodes from the new list /summary>        
public void Redirect(List<Node> oldNodes, List<Node> newNodes) {
            foreach (PortConnection connection in connections) {
                int index = oldNodes.IndexOf(connection.node);
                if (index >= 0) connection.node = newNodes[index];
            }
}

The redirection method changes the node reference of the connection object, which in most cases produces correct functionality, but it leaves NodePort objects the same as the original graph's. This is undesirable in situations where the data contained in NodePorts are used for graph traversal.

On my end I appear to have solved this problem by cloning all nodePorts in a new port dictionary and assigning that to the node, followed by cloning each connection:

public void ClonePorts(List<NodePort> oldPorts)
        {
            var newDict = new NodePortDictionary();
            foreach (var port in oldPorts)
            {
                var newPort = new NodePort(port, this);
                newDict.Add(port.fieldName, newPort);
            }

            ports = newDict;
        }
public void Redirect(List<Node> oldNodes, List<Node> newNodes, IEnumerable<PortConnection> oldPortConnections)
        {
            var newConnections = new List<PortConnection>();

            foreach (var oldConnection in oldPortConnections)
            {
                var nodeIndex = oldNodes.IndexOf(oldConnection.node);
                var newPort = newNodes[nodeIndex].Ports
                    .FirstOrDefault(p => p.fieldName == oldConnection.Port.fieldName);

                if (newPort == null) continue;

                var newConn = new PortConnection(newPort);
                newConnections.Add(newConn);
            }

            this.connections = newConnections;
        }
Siccity commented 5 years ago

What you're describing has been working correctly for two years, and has just been tested to work as well on Unity 2019.1.8. We're gonna need some more help finding the cause of this bug

taylank commented 5 years ago

I'll get back to you with a repro project sometime next week.

Siccity commented 5 years ago

Any progress? Or did you find that this was caused by something in your project?

taylank commented 5 years ago

I haven't been able to reproduce it with a clean install, so it's probably something related to changes I've made to xNode core. Thanks for checking in and looking into it.