Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Remove Unity related symbols from Nuget projects #1888

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Jul 19, 2024
Merged

Remove Unity related symbols from Nuget projects #1888

merged 10 commits into from
Jul 19, 2024

Conversation

pCYSl5EDgo
Copy link
Contributor

Closes #1886

@pCYSl5EDgo pCYSl5EDgo closed this Jul 18, 2024
@pCYSl5EDgo pCYSl5EDgo reopened this Jul 18, 2024
AArnott
AArnott approved these changes Jul 18, 2024
View reviewed changes
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.

Good. Thanks. Just one cleanup opportunity please.

src/MessagePack/Resolvers/DynamicObjectResolver.cs Outdated Show resolved Hide resolved
@pCYSl5EDgo
Copy link
Contributor Author

pCYSl5EDgo commented Jul 18, 2024 •
edited
Loading

@AArnott Please review the changes.
I do not mind that the commits will be squashed.

AArnott
AArnott reviewed Jul 18, 2024
View reviewed changes
src/MessagePack/Resolvers/DynamicObjectResolver.cs Outdated Show resolved Hide resolved
@AArnott AArnott merged commit 5793c81 into MessagePack-CSharp:develop Jul 19, 2024
4 of 5 checks passed
@AArnott AArnott added this to the v3.0 milestone Jul 19, 2024
@pCYSl5EDgo pCYSl5EDgo deleted the AvoidDynamic branch July 19, 2024 05:21
@AArnott AArnott mentioned this pull request Jul 19, 2024
Merged
@evorowy-jk
Copy link

evorowy-jk commented Jul 28, 2024 •
edited
Loading

Will you guys be able to push this fix onto nuget? There is a serious problem with AOT support right now as nuget packages don't obey MESSAGEPACK_FORCE_AOT or ENABLE_IL2CPP. Resulting in NOT_SUPPORTED exception on Android IL2CPP.

@AArnott
Copy link
Collaborator

AArnott commented Jul 29, 2024

@evorowy-jk Can you confirm the problem you're having is with the v3.0 version?
This PR merged into the v3.0 version and released a couple days ago as https://www.nuget.org/packages/MessagePack/3.0.129-beta.

@evorowy-jk
Copy link

evorowy-jk commented Jul 29, 2024 •
edited
Loading

Sorry, as I sent the comment I saw that new version has been released - unfortunately on Unity 6, on nuget MessagePack 3.0.129-beta with following MessagePack initialization code:

StaticCompositeResolver.Instance.Register(new[]
{
    MasterMemoryResolver.Instance, 
    UlidMessagePackResolver.Instance,
                    
    StandardResolver.Instance
});

During Runtime on Android (IL2CPP build) there is an error with DynamicAssembly:

Error Unity PlatformNotSupportedException: Operation is not supported on this platform.
Error Unity   at System.Reflection.Emit.AssemblyBuilder.DefineDynamicAssembly (System.Reflection.AssemblyName name, System.Reflection.Emit.AssemblyBuilderAccess access) [0x00000] in <00000000000000000000000000000000>:0 
Error Unity   at MessagePack.Internal.DynamicAssembly..ctor (System.String moduleName) [0x00000] in <00000000000000000000000000000000>:0 
Error Unity   at MessagePack.Resolvers.DynamicObjectResolver+<>c.<.cctor>b__4_0 () [0x00000] in <00000000000000000000000000000000>:0 
Error Unity   at ListWithEvents`1[T]..ctor () [0x00000] in ./Library/PackageCache/com.unity.addressables/Runtime/ResourceManager/Util/ListWithEvents.cs:8 
Error Unity   at System.Lazy`1[T].ExecutionAndPublication (System.LazyHelper executionAndPublication, System.Boolean useDefaultConstructor) [0x00000] in <00000000000000000000000000000000>:0

It is somewhat connected with the defines/this issue - should I open new one though?

@AArnott
Copy link
Collaborator

AArnott commented Jul 29, 2024

Yes, please open a new issue. Thanks for reporting.

@evorowy-jk evorowy-jk mentioned this pull request Jul 29, 2024
Closed
@AArnott AArnott mentioned this pull request Oct 1, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AArnott AArnott AArnott approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.0
Development

Successfully merging this pull request may close these issues.
3 participants
@pCYSl5EDgo @evorowy-jk @AArnott
Morty Proxy This is a proxified and sanitized view of the page, visit original site.