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

Attempt to add a Unity project to test compilation #636

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aaubry
Copy link
Owner

@aaubry aaubry commented Sep 28, 2021

Another take at #634

@aaubry aaubry mentioned this pull request Sep 28, 2021
@steinbitglis
Copy link
Contributor

Thanks! That was quite convenient, I have to say.

I'm now working your changes into the package that I'm going to upload.
But I'm getting a lot of warnings (using Unity 2021.1.18f1).

The following classes seem to conflict with .net standard 2.0 libraries:

  • ConcurrentDictionary
  • IReadOnlyDictionary
  • IReadOnlyList
  • Lazy
  • LazyBuilder

Then at the bottom of TypeConverter.cs, there seems to be a portion of code that is
not meant for Unity, but it's going to be if the user doesn't explicitly define the symbol "UNITY" in their project settings.

A lot of files seem to expect a "nullable context". This is easily fixable by adding a file 'csc.rsp' next to the assembly definition file,
with the line '-nullable:enable'.

At the end of all that, I was left with these two warnings:

Assets\Plugins\YamlDotNet\RepresentationModel\YamlDocument.cs(61,18): warning CS8618: Non-nullable property 'RootNode' is uninitialized. Consider declaring the property as nullable.
Assets\Plugins\YamlDotNet\Serialization\YamlAttributeOverrides.cs(97,35): warning CS8600: Converting null literal or possible null value to non-nullable type.

Should I turn off the warnings, or is there some change that should be done?

image

@steinbitglis
Copy link
Contributor

It seems that the following files and folders in Portability are sufficient, and compiles with only the two mentioned warnings.
image

@steinbitglis
Copy link
Contributor

I could also delete DeconstructionExtensions.cs and TargetFrameworkAttribute.cs, and everything seems fine.

@steinbitglis
Copy link
Contributor

I have fixed every issue I could find and submitted a package to the asset store. I guess I spent most of the time updating the store images.
I'll try and put together a pull request against unity-compatible, so you can have a look, but I'll have to make dinner first now 😅

@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
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.

2 participants