Bug Incorrect output value with multiple Subgraph instances

Hi all!

I've just stumbled on a behavior with Subgraphs that I consider a bug (or perhaps an inconsistency), and I'd like to document that here.

Quick Description
When multiple instances of a Subgraph node are added and then executed in a Visual Scripting Graph, the output value of the last executed Subgraph node can "overwrite" the output value of the first executed Subgraph node.
See repro steps below, it might be easier to understand. The problem is quite simple to reproduce, actually.

Version Info:
Visual Scripting Version 1.7.7 - February 07, 2022
Unity Version : 2021.2.19f1.4644
Revision: 2021.2/staging 602ecdbb2fb0
Built: Tue, 29 Mar 2022 11:46:35 GMT

Repro Steps

1. Add this simple code to a C# script, that only returns the input string
(then Regenerate Nodes in Project Settings -> Visual Scripting)

Code (CSharp):

1. [IncludeInSettings(true)]
2. public class UserScript
3. {
4.     public static string UserFunction(string txt)
5.     {
6.         return txt;
7.     }
8. }

2. Create a new Script Graph, call that new UserFuntion



3. In a Script Machine, add 2 Subgraph instances of that newly created Script Graph


4. Run that script, and see that the string "_B_B" is output instead of expected "_A_B" value.

This, to me, is either a bug or an inconsistency.

This only happens with Subgraphs that are in "Graph" source mode.
If the Subgraph Source is converted from "Graph" to "Embed" as shown below,

the logged output becomes "_A_B", which is the correct expected result.


This bug (or stange behavior) is really a problem for us, as we have a lot of Subgraph nodes in our library and often have many instances of the same node in our Visual Scriping Graphs.
Converting our Subgraph source from "Graph" to "Embed" is not a very good workaround for us, as we'd like to keep a reference on a common Subgraph asset, without being restricted to using only a single instance of that Subgraph to avoid this bug.

Any thoughts?

 

For anyone interested in temporary workarounds, adding a cache node to prevent the Subgraph output value to be overwritten by the 2nd Subgraph instance works.



DISCLAIMER : this above is not a pretty solution. I would really like multiple Subgraph instances to properly work as actual instances, rather than the current behavior.

 

This has always been the default behaviour since pre-Unity-ownership times. I do agree that it shouldn't work like that though.

 

That's because graph macro are like static variables, there's only 1 instance. Add Inputs calls static Sub Graph Test output which = b in this flow when it gets to Debug Log. I should think, take away the pre-update to a and then b, and go straight from Update to Debug, and each will be call in sequence from Add Inputs, returning ab.

 

You are right that the Subgraph output will be different, depending on where that output is consumed along the node sequence.

Regarding the idea that Subgraph instances behave like static variable, I decided to dig a little more.
Are Subgraph variables also treated like member variables of a static class instances?

To test that hypothesis, the following Subgraph was created:



Running 2 instances of that Subgraph with a different Step value, we can see that each Subgraph instance keeps a separate "Counter" graph variable.

Therefore, Subgraphs do not entirely behave like shared static instances.



Executing the graph above, we can see weird things going on:
The log output is:
A=32
A=64; B=64

The 2 Subgraph instances seem to properly behave like separate instances with their own local data.
It is only when the Subgraph output is consumed later on in the graph execution sequence that inconsistencies start to show up.

 

I've run more simple tests, in an attempt to diagnose how and when Subgraph data gets mixed up across instances.

It definitely seems to be an issue with Visual Scripting implicit caching implementation, specifically with Subgraphs.

As far as I can tell, the way Subgraph instance data is handled during execution is correct : internal graph variables are independent, the values of their inputs are also independent and can be different across multiple Subgraph instances. That's good. However, once a Subgraph is executed, resulting output values are cached somewhere (internally), and this seems to be where a Subgraph instance can overwrite the output value cache of another Subgraph instance. This is bad.

How can we confirm this? We can start first by checking if some kind of caching is performed when the output of a Subgraph is consumed by multiple nodes along the graph execution sequence.



If the Subgraph instance is executed only once, that means the Script Machine has cached the output Counter value, and is pulling the result from that internal cache.
If the Subgraph instance is executed twice, that means no caching occurred and the same Subgraph instance is executed a second time when assembling the final Debug.Log string.

We can also verify the graph execution sequence, by logging something when the addition to the Counter variable takes place in the Subgraph:

Code (CSharp):

1. [IncludeInSettings(true)]
2. public class VSTest
3. {
4.     public static int AddInt(int a, int b)
5.     {
6.         string result = a + b;
7.         Debug.Log("AddInt=" + result)
8.         return result;
9.     }
10. }

So what do we see above:
* Subgraph internal variable Counter is PER INSTANCE (not static, that's good)
* Subgraph input variable Step is PER INSTANCE (not static, that's good)
* Each Subgraph instance is executed only once, even if output data is consumed from multiple nodes further down the graph sequence (that's good).
* Subgraph output cache is shared across instances of a Subgraph asset (BAD!)

I don't see how this behavior can be anything else than a bug in the way Subgraph output data is cached. IMHO, that can't be "by design", it does not make sense to me.

 

I think you will find there is 1 variable. Look at the flow, initially you Debug.Log the Sub1.counter, = 57. Then you call Sub2.counter = 114. When you call Debug.Log again, Add Input calls the static Sub1.counter = 114, then Sub2.counter = 114. There is 1 instance = 1 variable.

When I first used Bolt and encountered this, thought it would be useful to have a Unique checkbox. The only way to use SubGraph's is as functions/processors while variables are input, unless you intend for them to be global variables. You could even have an input being a variable number or name for lookup, while an array/list/dictionary inside is global.

 

I agree that adding a "Unique" checkbox would be a decent and safe workaround solution, assuming Unity could be reluctant to modify a default behavior that some users take for granted - even if that behavior is inaccurate or inconsistent.

However, regarding the idea that "There is 1 instance = 1 variable", that does not appear to be the case.
The reason why Sub2.counter value is double Sub1.counter value is not because the same instance is executed twice, that is because my test was intentionally "designed" with a different increment step for each Subgraph instance, so we can see each their value drift apart over time.

Translating that in code, for each update call we have:
Sub1.counter += 1;
Sub2.counter += 2;

Again, we can see these graph variable are independent during Subgraph execution, as their data is properly preserved across multiple frames:




I confirm that sub1.Counter is not the same as sub2.Counter.
The memory associated with the Counter variable is kept separate for each Subgraph instance, as seen when logging from inside each Subgraph instance.
Only the external fetching mechanism seems broken, when the parent graph uses the output data.

That is why I believe this to be a bug in the Script Machine Subgraph caching mechanism, as from all the tests I have performed so far, the internal Subgraph data appears to be independent for each instance.

 

I agree, a "unique" feature is urgently needed here.

In my case I have a counter inside a custom node and if this node is in a subgraph, and several of these subgraphs are loaded, the counter is incremented per node. So the scope of the counter is no longer local but global... Only if the script graphs are emmbeded does this work as expected.

 

Thank you so much for writing this up! This is exactly what is happening to me and it was driving me crazy.

After spending much time searching for an explanation, it was such a relief to find such a thoughtful and well-researched post explaining why I'm not actually losing my mind.