Siccity / xNode

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

Self linking in RuntimeMathGraph Example crashes Unity / App #48

Open ecurtz opened 6 years ago

ecurtz commented 6 years ago

I'll continue to look at this in the debugger, but if I attach a node output to an input on the same node in RuntimeMathGraph (either in Unity editor or a built application) it crashes. It isn't from the Destroy() call in UGUIPort.OnEndDrag, but I haven't been able to find anywhere else to break in the xNode code between that and the crash.

OS X, several versions of Unity.

ecurtz commented 6 years ago

Looks like infinite recursion in MathNode.GetValue

Siccity commented 6 years ago

MathGraph is a simple example project. The way it works is simply ask the previous node for its value, and use it to return the next. If you create recursion it will never be able to determine a result as there is no end to the calculation. This could be fixed in MathGraph, but as it is just an example meant to show beginners around, i didn't see it as anything important. After all, a recursive math graph shouldn't exist in the first place.

ecurtz commented 6 years ago

True enough, but recursion in the graphs is something that will come up in a lot of cases, maybe there should be some sort of built-in callback in the connector logic to reject illegal connections?

ecurtz commented 6 years ago

Actually I guess I shouldn't close this, since it's still a nasty crash in the demos. I'll leave it open for you to do as you see fit.

Siccity commented 6 years ago

Connections can be legal in some cases while being illegal in others. I can't make assumptions without getting in the way of some special use cases. But youre right, i should probably make a safeguard in the example

xiajjjjjjjjjj commented 5 years ago

I think you should add an attribute to the input to indicate whether the node can connect itself

ikold commented 2 years ago

In my project, I found it really easy to create a reference loop in node connections, crashing as result, so I created a dependency parameter in OutputAttribute that stores a list of input names that are used to check if a new connection will result in a loop. Check is done recursively during an attempt to connect two ports, with no overhead during other operations on the node graph.

By dependency, I mean a situation where a call GetValue() to get output port value might result in a call GetInputValue() of an input port.

Usage in the example MathNode would look like so:

...
[Input] public float a;
[Input] public float b;

[Output(dependencies = new []{"a", "b"})] public float result;
[Output(dependencies = new []{"a", "b"})] public float sum;
...

It is backward compatible, as it doesn't change anything if you don't use it. I can create a Pull Request if you are interested in this solution.