Improvement Graph Merge

[aronzeParticipant]

Hi,

We’re activity to use NodeCanvas in our project for a long times, NodeCanvas is really great asset! Thanks, Paradox. But sometimes We have painful time because Graph merge conflict. So I thought why graph merge is always conflict?

First, merge engine itself could not be understand all context. So, specialized merge engines needed. For example, C# context is included default set, Unity supply YAML merge(https://docs.unity3d.com/Manual/SmartMerge.html), and maybe json context can be merged.(https://www.npmjs.com/package/git-json-merge)

Here is first problem. Graph is scriptable object, so graph json string is warping YAML context. I thought YAML merge engine parse right way but json string area could not understand to json, so just merge to apply string compare rules.

My solution is get rid of json in scriptable object and just link TextAsset. So, each files can be clearly parsed.

Seconds, Graph json data include position datas. It is very useful for develop purpose but if so many people can be work together, always can be created change set they just open the graph. Of course, if you just work alone, it is still problem because it is not easy to find simple change set.

How about this?

[GavalakisKeymaster]

Hey,

Thanks for your positive feedback on NodeCanvas.
Indeed, merging is a point that has previously been brought up by other people as well, and can be a problem exactly for the reasons you mention here, that is because the graph is serialized into a json string “within” a Unity Object (YAML).

Your solution of having a linked TextAsset (with the json) is indeed something I have considered doing, but the only thing that kept me from actually doing it, was that it might be considered “complicated” for some people. Having said that though, I still consider it as a solution only if I manage to make it “optional”. Remember though, that Unity Object references of course, will still need to be serialized within the Unity Object, since the json string only holds a list index to their reference.

Regarding Graph position/zoom, in the next version, I plan to make the Graph position/zoom only be serialized when there is at least some other change as well instead of always be serialized like it is now. This will of course avoid marking the file as “changed” when someone in the team opens up a graph only to view it rather than change it, but moving any node around will still mark it as changed though.

Let me know what you think.
Thanks.

[aronzeParticipant]

Hi, Gavalakis.

Thanks for your great answers.
Yes, I agree. Seperate stratege is complicated and maybe occurred some other problems because two files must use to be pair. Simply way, how about to use sub-asset for avoid this issue?

Now a day, NodeCanvas core models include two parts that core data and editor purposed data. If it is possible, how about split two models from GraphSerializationData? Then least, users can distinguish either changed or not. And of course, it can be excluded to deserialize in !UNITY_EDITOR environments.)
Anyway, it is also very helpful that detect opened files. I glad to wait for next NodeCanvas version.

Thanks:)

[GavalakisKeymaster]

Hey,

Thanks for your further suggestions into the subject.
I like your two separate serialization models better than the sub-asset suggestion. I will take a look at this approach and see how far it can go with 🙂

Thanks again!

[Author]

Posts