Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Parser with VYaml or similar #2

Open
Grimeh opened this issue Jun 12, 2024 · 0 comments
Open

Replace Parser with VYaml or similar #2

Grimeh opened this issue Jun 12, 2024 · 0 comments

Comments

@Grimeh
Copy link
Owner

Grimeh commented Jun 12, 2024

As seen in upstream#11 and upstream#12, the parser is prone to breaking over time as Unity introduces small changes in their own YAML serialiser. It's also extremely rudimentary and not robust enough to ensure no future (or existing) parser errors.

IMHO it would be better to replace the parser entirely with one that is battle-tested and actively maintained.

This is of course complicated by the fact that the full YAML spec is monstrous, and extremely difficult to write a fully compliant parser for (see here1). In fact, Unity themselves only support their own flavour of YAML which is both a subset and superset (I'll call this "Unity YAML"/"UYAML" from here on). As a result the landscape for YAML parsers is generally not fantastic, and vanishly few support UYAML.

The most ubiquitous parser written in C# is probably YamlDotNet, but even that doesn't fully support YAML 1.2. It also doesn't support UYAML's quirks2, and while this is only currently an issue for scene files, we cannot guarantee Unity won't continue to diverge UYAML from the vanilla spec so I think we have to rule it out as a long-term solution.

We could integrate a prebuilt library (made with C/C++/Rust/etc.) that supports UYAML and use PInvoke to call into it. However I think this would be a pain to maintain, especially when it comes to updating it as a dependency. I have one written in Rust specifically for UYAML (ie. doesn't support vanilla YAML) that I could make public if necessary, but I'd like to avoid that as it's embedded in & optimised for a pretty specific usecase (and tbh is pretty messy), it would take a bunch of work to make a standalone library.

Thankfully there exists a performance-oriented parser written entirely in C# that explicitly supports UYAML as a feature: VYaml. I think this is by far the best option.

In terms of integrating VYaml, we're currently not setup to do this in a clean way. We could add it as a git submodule and probably call it a day, but that would result in more work for users to install USS.
I think #1 will be required to get any sort of sane setup process. I think we can then add VYaml as a dependency in our package manifest. I can't see any details in Unity's docs about whether dependencies can or cannot be git URLs so it probably requires some testing.

Footnotes

  1. https://github.com/cblp/yaml-sucks

  2. https://forum.unity.com/threads/scene-files-invalid-yaml.355653/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant