Skip to content

Asset store update - #634

Closed
steinbitglis wants to merge 2 commits into
aaubry:masterfrom
steinbitglis:unity_compatible
Closed

Asset store update#634
steinbitglis wants to merge 2 commits into
aaubry:masterfrom
steinbitglis:unity_compatible

Conversation

@steinbitglis

Copy link
Copy Markdown
Contributor

Hey Antoine!
This is not a pull request that I actually want you to merge anywhere,
but I'm in need of a little bit of code review. I've held back on updating
the Unity Asset Store version of YamlDotNet, because C# 8 features were being used
after version 6.1.2. But now that Unity has a much upgraded compiler that
supports C# 8, I was making another attempt, just to stay on par with your
progress. Unity now supports .net standard 2.0. There were a few
things in YamlDotNet 11.2.1 that required .net standard 2.1, but I think it's
pretty close.

So what this is, is just me asking if you could review my branch unity_compatible,
and see if you think I broke anything. I don't have a proper IDE set up for
the project, and I'm not able to run the test suite. It would be a really big job
for me just to run the tests, but if it's easier for you, could you do a quick review
and maybe a sanity check (if I've misunderstood the .net standard 2.1 features
or something.). The sample code seems to run alright in Unity, but I haven't
checked the output properly.

If you think it looks ok, I'll go ahead and package it up for the asset store, and
probably also figure out which exact Unity version is recent enough for this updated
code.

@aaubry

aaubry commented Sep 16, 2021 via email

Copy link
Copy Markdown
Owner

@aaubry

aaubry commented Sep 17, 2021

Copy link
Copy Markdown
Owner

I see that you have removed a bunch of nullable annotations, such as [MaybeNullWhen(true)]. Those are needed for other platforms, so I would like to avoid removing them. Can you explain how you are compiling this with Unity so that I can try a fix that works for all cases?

@steinbitglis

Copy link
Copy Markdown
Contributor Author

Yes, I've exported a unitypackage that you can look at.
YamlDotNet_as_unitypackage.zip

Unzip, then import the package from "Assets"->"Import Package"->"Custom Package".
There is a file called YamlDotNet.asmdef, this makes unity compile the folder and everything beneath it into one assembly.

Create a new game object in a scene, and put an ExampleRunner on it. Press play.

The newer versions of Unity will be compiling to .net standard 2.0

@steinbitglis

Copy link
Copy Markdown
Contributor Author

Did you manage to run the examples in Unity? Let me know if I can help further.

@aaubry

aaubry commented Sep 28, 2021

Copy link
Copy Markdown
Owner

Did you manage to run the examples in Unity? Let me know if I can help further.

I'm trying right now. I'll let you know how it goes.

@aaubry

aaubry commented Sep 28, 2021

Copy link
Copy Markdown
Owner

So, I followed your instructions and was able to build a project with your package.
Then, I replaced the code from your package and could see the compilation problems, which I was able to fix. I have already pushed this commit that contains the changes needed to compile the code.

Then, I created a new, empty Unity project in the YamlDotNet directory and added the YamlDotNet code by creating symlinks to include the necessary files and directories. This way, I was able to have a working Unity project that uses the code directly from the main repository. The advantage of this is that anyone can check for Unity copatibility issues while working on the project.

This also seems useful as a step to produce the unity package, although I don't know how that step is performed. I have opened a pull request so that you can check this and maybe make any changes needed to create the unity package from there.

Please have a look and let me know if this is useful. Keep in mind that, if you're on Windows, you will need to enable symlinks on Git, by setting core.symlinks to true, as explained here.

@aaubry aaubry mentioned this pull request Apr 29, 2022
@aaubry
aaubry force-pushed the master branch 8 times, most recently from a0f8359 to 78b1ab3 Compare December 2, 2022 22:39
@EdwardCooke

Copy link
Copy Markdown
Collaborator

This PR appears to be abandoned, it's over a year old. I'm going to close it. There is a couple feature requests for Unity related packages/processes which I'm planning on looking in to in the near future.

@steinbitglis
steinbitglis deleted the unity_compatible branch January 13, 2023 21:49
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

Successfully merging this pull request may close these issues.

3 participants