[FEATURE REQUEST] Add an option to disable minified JSON graph serialization

NodeCanvas Forums General Discussion [FEATURE REQUEST] Add an option to disable minified JSON graph serialization

Viewing 12 posts - 1 through 12 (of 12 total)
  • Author
    Posts
  • #16035
    kiamvdd
    Participant

    When working with version control, the minified json used in graph assets can be a bit of a pain to work with, as any small change to the graph result in huge diffs that can be annoying to sift through. Using pretty print instead isn’t a perfect solution, as formatting is kind of ruined once written to the asset (and I’m not sure we have any control over this?), but I still believe it’s a more readable solution for version control.

    I think it would be a good middle ground to keep the default minified, but allow us the option to enable pretty printed serialization. This would also have to be a project based setting that can be picked up by version control so the entire team uses the same setting for serialization.

    #16041
    Gavalakis
    Keymaster

    Hello there,

    This is something I’ve been struggling with in the past, but unfortunately serializing the json in a clean pretty format is not possible. A serialized pretty json string field in a Unity Asset, actually ends up looking worst than a minified one and not very helpful for diff purposes.  🙁 Here is an example of how it looks:

    SerializedPrettyJSON

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

    Attachments:
    You must be logged in to view attached files.
    #16273
    danielk
    Participant

    This is a pretty big issue for any project working with VCS – would it be possible to write out the serialized json as a companion file and load it in at runtime? Then maybe bake everything together at build time?

    #16280
    Gavalakis
    Keymaster

    Hello there.

    There is already a similar feature suggestion to automatically serialize the graph to an external pretty json file alongside the graph and load from that file in runtime. This is something that I will implement soon. With that said, there is already the possibility of manually exporting the graph serialization to a pretty json file and importing from json in runtime, but of course the goal above it to do this automatically.

    Is this described feature above what you were suggesting as well?

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

    #16291
    danielk
    Participant

    Wait, so if I manually export the graph – it will load it from the file in runtime going forward?

    I’m not seeing that functionality, but was hoping you could point me in the direction of the serialization so that I could hack a solution. We’re getting multiple daily conflicts and its just not an acceptable workflow

    #16293
    danielk
    Participant

    Here is a first start on reading/writing to disk. This is only editor atm, need to add build logic to bake it BACK in. This is a git patch. I name the resulting json files with a preceding “.” to ensure Unity ignores them. NOTE: this is super hacky. My recommendation would be to serialize into YAML. Mixing data formats is generally going to result in an inelegant solution

    For Graph.cs

    And for GraphOwner.cs (to also generate for bound prefabs):

    #16295
    danielk
    Participant

    And here is a git patch to resolve the viewport changes – way too many updates due to this:

    #16301
    Gavalakis
    Keymaster

    Hello again.

    Thank you. I will look at the changes even though I have already worked on the external file serialization/deserialization 🙂

    Regarding your last post changes, do you mean about serializing the translation and zoom factor and marking the asset as dirty?

    Thank you.

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

    #16331
    danielk
    Participant

    Hey!

    This is more of a hack for users who need a solution. Less of a suggestion for you. The second git patch is to disable serializing the translation. Way too many updates for just a view change.

    Ultimately, though, it looks like node canvas might not meet the needs for our use case. The merge conflicts are just not viable.

    #16372
    ciyapa official
    Participant

    The automatic serialisation of the graph to an external nice json file alongside the graph and runtime loading from that file is already a suggested feature. This is something I’ll start doing soon.

    ciyapa

    #16538
    ramonmiles
    Participant

    This is a big issue for any project working with VCS – would it be possible to write out the HCMI  as a companion file and load it in at runtime? Then maybe bake everything together at build time?

    #16308
    danielk
    Participant

    Hello again.

    Thank you. I will look at the changes even though I have already worked on the external file serialization/deserialization 🙂

    Regarding your last post changes, do you mean about serializing the translation and zoom factor and marking the asset as dirty?

    Thank you.

    This would be for anyone else who needs a workaround until you have your external changes live (like me and the OP).

    And yes – that is nuking the translation + zoom serializing changes. I saw some questions asking to disable that, and I did it for myself. Thought I would share the patch as well.

    Its just a shame that Unity doesn’t support chomping indicator |, or this would be so much easier

Viewing 12 posts - 1 through 12 (of 12 total)
  • You must be logged in to reply to this topic.