Graph.SelfSerialize always serializes even when not required

[lumley]

Hi!

I have searched for this issue in both Discord and the Forum but I didn’t find any references. Please if it’s a duplicate, feel free to direct me to the place where it was answered.

We’re using NodeCanvas extensively in a project where we have tons of graphs. We’ve realized that every time Unity reloads the assemblies, it re-serializes all graphs consistently (regardless of them having a change or not).

We are currently in version 3.0.9, but 3.0.8 displays the same issue (we haven’t checked on previous versions).

Now after investigating a bit, we’ve come to the code in Graph.cs

///Serialize the Graph. Return if serialization changed
public bool SelfSerialize() {
(...)
var newReferences = new List<UnityEngine.Object>();
var newSerialization = this.Serialize(newReferences);
if ( newSerialization != _serializedGraph || !newReferences.SequenceEqual(_objectReferences) ) {
(...)

The key here is checking via LINQ with newReferences.SequenceEqual(_objectReferences). This method compares the new serialization with the old one, element by element on the same index. Now let’s look at a real-world case:

Notice the old values serialized, the first entry is a UnityEngine.Object with the value “null” as a name.

The new references hold an actual null object, no UnityEngine.Object.
Comparing these two elements is useless, as they are never equal. This results in all graphs always re-serializing.

This issue is a result of a specific piece of code in the class fsUnityObjectConverter.cs.
public override fsResult TrySerialize(object instance, out fsData serialized, Type storageType) {
(...)
//this is done to avoid serializing 0 because it's default value of int and will not be printed,
//which is done for performance. Thus we always start from index 1. 0 is always null.
if ( database.Count == 0 ) {
database.Add(null);
}
The first element is serialized as null, but once serialized we actually get a UnityObject that is not null (but it is named as null).

To fix this issue locally and not get a false positive on every single graph, we’ve replaced the comparison by ignoring those graphs where the first entry does not match but the new reference was actually null.

In case we understood this wrong, feel free to indicate what was the intention behind this code!

Thank you very much for your time and for this fantastic tool.

Attachments:

You must be logged in to view attached files.

[Gavalakis]

Hello there and thank you for your positive feedback.

Please note that the things that take place within the if ( newSerialization != _serializedGraph || !newReferences.SequenceEqual(_objectReferences) ) { clause, are not related to the graph being serialized or not. The graph is already serialized (exactly one line above that) var newSerialization = this.Serialize(newReferences); anyways before that clause.
With that said, what you stated can indeed happen in some cases, although it is not always the case and while trying to replicate this it only occurred in one graph out of all I have in the project; still looking out as of why it happens on that one specific graph. However, once again, please note that graph serialization takes place regardless of the SequenceEquals check.

For your information regarding the code, as explained in the comments above, we add null to the first element of the list because we are not serializing default values. Since UnityObject serialization is happening with integers representing indeces, if 0 was the representation of an object we actually do want to serialize, it would not happen because 0 is naturally the default value of integer and as such that object serialization would be lost since no entry would be added for that element in the json. There are of course other ways to handle this, but at the time it seemed a straightforward one (and kept that way ever since for backwards compatibility).

With all that said, I will take a look at this and find out why it only reproduces on one specific graph I have out of all and then handle this UnityObject intricacy somehow for consistency. (graph serialization would still take place regardless though before that check 🙂 )

Let me know.
Thanks!

Join us on Discord: https://discord.gg/97q2Rjh

[lumley]

Hi Gavalakis,

Thank you so much for your very detailed answer. I have to admit my wording may have caused confusion, when I said “Serialization” I was referring to Unity’s object serialization (updating the MonoBehaviour references) rather than the custom JSON serialization of Graphs.

On the code, I understood it’d be this part:
_serializedGraph = newSerialization;
_objectReferences = newReferences;

In any case, for us, this issue of the first element change between UnityObject “null” and object (null) was present in over 200 graphs.

Some background information:
– We’re using Unity2019.3.9f1, but we could reproduce with 2019.4 as well
– Most of our graphs came from version 2.x of NodeCanvas, after updating to 3.0.8 we started to notice that the editor was quite more unstable (things would not always serialize, cache errors in the console and often crashes because of corrupted memory). We didn’t report this as the issue since our investigation so far is not really conclusive (it could’ve been any other plugin).

After our small change to check equality:
– The editor so far is more stable, but we still cannot rule this as the main issue, since perhaps it was conflicting with other utilities present in our project
– None of the graphs seem to be displaying the difference in the first element anymore, so the editor was able to successfully save all of them

Unfortunately we also didn’t find any pattern in the graphs that were reproducing and we’re no longer experiencing it. I will make sure I come back here if we find any more information about this case.

[Author]

Posts