Bug: UnityConverterInitializer registers too many converters

[rumcgit]

Description

JsonConvert.SerializeObject() converts integers to strings, if we pass JToken !

Example

var str = "{\"a\":123}";
var jtoken = JsonConvert.DeserializeObject<JToken>(str);
var badSerializedString = JsonConvert.SerializeObject(jtoken);
var goodSerializedString = jtoken.ToString(Formatting.None);
Debug.Log(badSerializedString);  // outputs: {"a":"123"}
Debug.Log(goodSerializedString); // outputs: {"a":123}

Expected behavior

badSerializedString should be just the same as goodSerializedString: {"a": 123}

Versions

Newtnosoft.Json-for-Unity version: 12.0 (latest) Unity version: 2019.1.14f1 Platforms affected: android, ios, webgl, editor

[applejag]

Hi! Thanks for reporting this!

I do not have a computer with Unity installed at the moment so can only try reproduce later, but I tried it in dotnet fiddle where it works as expected. This is intriguing!

Will get back to you once I've tested your example.

[applejag]

I am unable to reproduce this issue. I tried it with the following tests:

using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using NUnit.Framework;

namespace Tests
{
    public class Issue60
    {
        [Test]
        public void BadSerialized()
        {
            var str = "{\"a\":123}";
            var jtoken = JsonConvert.DeserializeObject<JToken>(str);
            var badSerializedString = JsonConvert.SerializeObject(jtoken);
            Assert.AreEqual(@"{""a"":""123""}", badSerializedString);
        }

        [Test]
        public void GoodSerialized()
        {
            var str = "{\"a\":123}";
            var jtoken = JsonConvert.DeserializeObject<JToken>(str);
            var goodSerializedString = jtoken.ToString(Formatting.None);
            Assert.AreEqual(@"{""a"":123}", goodSerializedString);
        }
    }
}

And received the results:

BadSerialized (0,089s)
---
Expected string length 11 but was 9. Strings differ at index 5.
  Expected: "{"a":"123"}"
  But was:  "{"a":123}"
  ----------------^

I ran the tests in a matrix of the following setups:

• Editor and in a built player with IL2CPP
• Unity 2019.1.14f1, 2019.3.14f1, and 2020.1.0b6
• .NET 4.x and .NET Standard 2.0

I was unable to reproduce your issue in neither of those configuration combinations.

---

If you could supply a full repro of when this occurs then I can investigate further (that is, for example a minimalistic Unity project zipped and uploaded here), but as of right now I cannot help you as it works on my machine

[rumcgit]

Amazing! But using exactly your code I have the following ^) :

• Windows 7 and Mac OS X
• Unity 2019.1.14f1
• .NET Standard 2.0

Maybe we use different json libraries? )) Can you attach dlls here?

[applejag]

Which DLLs are you interested in?

Also, do you have any custom converters registered at all? Do you get this issue in a newly created project as well with ONLY these tests and the jillejr.newtonsoft.json-for-unity package added?

[rumcgit]

I tried to run it on an empty project, and the result is similar to yours. How can it be so ? Any ideas ?

[applejag]

My guess is that you have some JsonConverter that does it. If you upload your project to me I could investigate. I would love to get to the bottom of this, it's an interesting case. If you're worried about uploading it publicly here you can upload it to wetransfer.com or something and send it privately to my email if you're ok with that option.

[rumcgit]

I've found the reason! The reason is in UnityConverterInitializer from Newtonsoft.Json-for-Unity.Converters . UnityConverterInitializer goes through all assemblies and register all converters in your code as default converters! I think that's not good idea, because the project can be very big, with dozens of custom logics. In my case I had a custom converter for a very specific case, but surprisingly it was registered as a default converter too. I suggest to limit the list of auto-registered converters to the list of your library converters.

[applejag]

Ah! Did not realize you also used the converters package. Yea I'm not too proud of how it registers converters. I copied the registering routine from Wanzyee Studios solution, but explicit is far better than implicit. Especially when you can't turn it off 😅

I'll move this issue over to that repo and see if I can get a fix implemented within reasonable time.

Nice debugging by the way!

[applejag]

@rumcgit There is a new preview out with an editor to enable/disable any given converter. Try it out already by updating to version 1.1.0-preview.2, then access the settings via "Edit > Json .NET converters settings..."

and then disabling your converter from the global default converters settings.

Example:

Don't fray to reopen or create new issues if you have more troubles. :)

[mastef]

@applejag I also just stumbled across this. Newtonsoft was crashing the editor due to serialization of the Color element, so I was lucky I stumbled across your package.

But then a custom converter we have for a specific type ( only attached through JsonConverter attribute ) was suddenly throwing errors, since it didn't have a CanConvert method implementation. Even more curiously it happened during "unity.service.core" requests that the editor was sending.

What's the expected default behavior here normally that requires this package to work correctly?

It seems weird that external converters are automatically attached. The default behavior would be to just include the Unity Converters from this repo - the Newtonsoft.Json converters are normally also not included unless specified in the options, right? Or are the VersionConverter and StringEnumConverter required for this package to work?

[applejag]

@mastef Hello! The original reason for this is because I wanted this project to be a follow-up on Wanzyee's package: http://wanzyeestudio.blogspot.com/2017/03/jsonnet-converters.html

When creating this package originally, I got permission from Wanzyee to base my work off of his.

So to make this project only be an extension of that package, I adopted this behavior from his package.

I agree now that it might not have been the wisest decision. However, changing this is a breaking change. And now that this package has gained some traction, it's even harder to revert this decision.

I'm no longer using Unity anymore (not even in hobby projects), so I'm not having any will to put any effort into this project anymore. I'm only spending minimal effort into it. The only reason I haven't archived this repo is to allow people to submit issues about critical bugs if there are any, or to allow other to submit PRs.