Bug: Does not work with UniTask in WebGL (IL2CPP)

nopjia commented 4 years ago

When I try to have both this package and UniTask included in my project, the Converters stopped working when built to WebGL. They would still work fine in Editor. I spent a day trying out different scenarios in a fresh project, all of the following scenarios fail:

Importing UniTask and simply having it in the project in the Packages folder
Calling JsonConvert.SerializeObject() from both normal synchronous and asynchronous function
Linking UniTask into asmdef
Linking Converters into asmdef

Expected behavior

Serialize JSON when built and run in WebGL, while UniTask is also included in the project.

Actual behavior

This still works in Editor and Desktop build. Crash while running in WebGL. It seems the Converters aren't being built. Error:

JsonSerializationException: Self referencing loop detected for property 'normalized' with type 'UnityEngine.Vector3'. Path 'normalized'.

Steps to reproduce

New project
Import jillejr.newtonsoft.json-for-unity via UPM
Import jillejr.newtonsoft.json-for-unity.converters via UPM
Import UniTask via UPM
Create asmdef and add UniTask
Add following script to scene:

void Start() {
  str = JsonConvert.SerializeObject(new Vector3(0, 2.5f, 5.1f));
}

Switch to WebGL
Build and Run

Details

Host machine OS running Unity Editor: Windows 10

Unity build target: WebGL

Newtonsoft.Json-for-Unity package version: 12.0.301

Newtonsoft.Json-for-Unity.Converters package version: 1.0.0

I was using Unity version: 2018.4.18f1, 2019.4.11f1

Checklist

[x] Shutdown Unity, deleted the /Library folder, opened project again in Unity, and problem still remains.
[x] Checked to be using latest version of the package.

applejag commented 4 years ago

Hi @nopjia ! Thanks for your research into this issue. Well put!

I'm sorry to say I'm a little busy in life right now but I'll try find the time to look at this!

topisiro commented 4 years ago

I just ran into an identical problem but with Android instead of WebGL, also using IL2CPP. According to my detective work, the method FindUnityConverters() in UnityConverterInitializer wasn't able to find any of the converter types in the build. Maybe it has something to do with how UniTask also initializes itself using the [RuntimeInitializeOnLoadMethod(RuntimeInitializeLoadType.BeforeSceneLoad)] attribute?

Nevertheless, I was happy to discover that updating to Newtonsoft.Json-for-Unity.Converters package version 1.1.0-preview.2 solved this problem for me.

nopjia commented 4 years ago

Oh thanks for the message! I'll give it another try.

topisiro commented 4 years ago

One addition, it looks like you also have to add the following to your Assets/link.xml (within the linker tag) to prevent code stripping:

<assembly fullname="Newtonsoft.Json.UnityConverters">
               <type fullname="*" preserve="all"/>
</assembly>

<assembly fullname="Newtonsoft.Json.Converters">
               <type fullname="*" preserve="all"/>
</assembly>

Curiously, this wasn't enough to fix the problem with version 1.0.0 but with 1.1.0-preview.2 it seems to work.

applejag commented 4 years ago

I applaud your detective work @topisiro :)

The three big changes that is included in that 1.1.0-preview.2 version is:

35 Removed __makeref usage for IL2CPP
39 Feature: ScriptableObject & SerializeFieldAttribute
40 Feature: Serializer config v1

The latter, #40, made changes to how converters are loaded. #39 made changes to how contracts are created where it respects Unity's [SerializeField] attribute, but Vector3.normalized isn't attributed with anything.

As I mentioned earlier, I'll have to doublecheck this, but awesome that you found a workaround. Makes it possible to A:B test the changes.

I'll consider including some link.xml values as well as it seems like they're necessary.

Sorry to say that there's no timeline for this at the moment. I'm currently spending my free time on #hacktoberfest :)

nopjia commented 4 years ago

Happy to report that I was able to get this to work in WebGL after following the steps from @topisiro :

Updating to version 1.1.0-preview.2

Adding the following Assets/link.xml

<linker>
<assembly fullname="Newtonsoft.Json.Converters">
<type fullname="*" preserve="all"/>
</assembly>
<assembly fullname="Newtonsoft.Json.UnityConverters">
<type fullname="*" preserve="all"/>
</assembly>
</linker>

Thank you so much for looking into this @topisiro !!

For more information, adding the Assets/link.xml file alone without updating the version will result in the following compilation error:

IL2CPP error for method 'UnityEngine.CullingGroupEvent Newtonsoft.Json.UnityConverters.Camera.CullingGroupEventConverter::CreateInstanceFromValues(Newtonsoft.Json.UnityConverters.ValuesArray`1<System.Object>)' in CullingGroupEventConverter.cs:28

adampsst commented 3 years ago

Out of interest, why would this link.xml not be contained by default in this package? There is one inside the Json-for-Unity package by default? Took me a while to figure this issue out and find this ticket which helped.

applejag commented 3 years ago

@adampsst It's on the forever long list of todos. :) That's mostly why

adampsst commented 3 years ago

@adampsst It's on the forever long list of todos. :) That's mostly why

Ah, I understand. No worries, thanks for responding!

applejag / Newtonsoft.Json-for-Unity.Converters