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

Unity IL2CPP and New CodeGenerator #591

Merged
merged 18 commits into from Oct 29, 2019
Merged

Unity IL2CPP and New CodeGenerator #591

merged 18 commits into from Oct 29, 2019

Conversation

neuecc
Copy link
Member

@neuecc neuecc commented Oct 17, 2019

About #568, includes fix #559, #560, #562, #564

I've added new code generator MessagePack.Generator and MessagePack.MSBuild.Tasks.
MessagePack.UniversalCodeGenerator is deperecated.

MessagePack.Generator can be dotnet core global tools so require to add nuget push script to ci.
MessagePack.MSBuild.Tasks is similar, so also require to add nuget push script to ci.

New generator is port from #396.

@neuecc neuecc requested a review from AArnott October 17, 2019 09:52
@neuecc
Copy link
Member Author

neuecc commented Oct 17, 2019

IL2CPP test can run on CI.

- run:
    name: Build Windows(IL2CPP)
    command: /opt/Unity/Editor/Unity -quit -batchmode -nographics -silent-crashes -logFile -projectPath . -executeMethod UnitTestBuilder.BuildUnitTest /headless /ScriptBackend IL2CPP  /BuildTarget StandaloneWindows64
    working_directory: MessagePack.UnityClient
# execute headless player on CI
- run: MessagePack.UnityClient/bin/UnitTest/StandaloneWindows64_IL2CPP/test.exe

image

@neuecc
Copy link
Member Author

neuecc commented Oct 17, 2019

and Added StaticCompositeResolver which is similar as v1's CompositeResolver.
Because v2's CompositeResolver does not run on IL2CPP, simple is always best:)

{
source.Add(file);
}
// TODO:resolve NuGet reference
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For our use case this is a blocker, but I understand that it isn't trivial to implement.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I should support it but it will be a later.
Old mpc has many issues so must move to this model in v2.

Copy link
Collaborator

@AArnott AArnott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mostly reviewed the common MessagePack code. I didn't spend much time on the MPC-specific stuff. Looks good.

Also fix some issues the newer version of the analyzers found.
@neuecc
Copy link
Member Author

neuecc commented Oct 24, 2019

thanks.
I'll add doc comment(maybe there comment will be poor:)) and will add some fix to create unitypackage.
after, do merge.

@AArnott AArnott added this to the v2.0 milestone Oct 26, 2019
@neuecc
Copy link
Member Author

neuecc commented Oct 28, 2019

commented and add some fixes, it works in my local.
I don't know CI fails.

@AArnott
Copy link
Collaborator

AArnott commented Oct 28, 2019

The CI is failing with several errors that were warnings in VS' error list including these two:

Error VSTHRD111: Add .ConfigureAwait(bool) to your await expression.
Error RS0016: Symbol 'QueueFormatter' is not part of the declared API.

The first one is because of #595 which ensures we don't accidentally forget .ConfigureAwait(false) that can result in deadlocks when GUI apps use MessagePack and synchronously block on the result.

The second one is because of #596 which requires that all public API changes to the MessagePack library now be explicitly declared in a PublicAPI.*.txt file, as explained in that PR.

Both of these warnings have code fix providers to make fixing all of them very easy:

codefixes

@neuecc neuecc merged commit 2eba504 into master Oct 29, 2019
@neuecc neuecc deleted the unity-il2cpp branch October 29, 2019 06:06
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.

Removing UNITY_2018_3_OR_NEWER
3 participants