applejag/Newtonsoft.Json-for-Unity

Bug: Serializing Color out of the box causes Editor crash even with ReferenceLoopHandling set to Ignore

hk1ll3r opened this issue · 6 comments

I've seen Unity Editor crash and sometimes this exception printed. Somtimes stack overflow from the json settings error handler. Complaining about "linear" field of Color class which is of type "Color".

InvalidOperationException: Current error context error is different to requested error.
Newtonsoft.Json.Serialization.JsonSerializerInternalBase.GetErrorContext (System.Object currentObject, System.Object member, System.String path, System.Exception error) (at /root/repo/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalBase.cs:103)
Newtonsoft.Json.Serialization.JsonSerializerInternalBase.IsErrorHandled (System.Object currentObject, Newtonsoft.Json.Serialization.JsonContract contract, System.Object keyValue, Newtonsoft.Json.IJsonLineInfo lineInfo, System.String path, System.Exception ex) (at /root/repo/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalBase.cs:121)
Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.Serialize (Newtonsoft.Json.JsonWriter jsonWriter, System.Object value, System.Type objectType) (at /root/repo/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalWriter.cs:86)
Newtonsoft.Json.JsonSerializer.SerializeInternal (Newtonsoft.Json.JsonWriter jsonWriter, System.Object value, System.Type objectType) (at /root/repo/Src/Newtonsoft.Json/JsonSerializer.cs:1146)
Newtonsoft.Json.JsonSerializer.Serialize (Newtonsoft.Json.JsonWriter jsonWriter, System.Object value, System.Type objectType) (at /root/repo/Src/Newtonsoft.Json/JsonSerializer.cs:1046)
Newtonsoft.Json.JsonConvert.SerializeObjectInternal (System.Object value, System.Type type, Newtonsoft.Json.JsonSerializer jsonSerializer) (at /root/repo/Src/Newtonsoft.Json/JsonConvert.cs:665)
Newtonsoft.Json.JsonConvert.SerializeObject (System.Object value, System.Type type, Newtonsoft.Json.JsonSerializerSettings settings) (at /root/repo/Src/Newtonsoft.Json/JsonConvert.cs:614)
Newtonsoft.Json.JsonConvert.SerializeObject (System.Object value, Newtonsoft.Json.JsonSerializerSettings settings) (at /root/repo/Src/Newtonsoft.Json/JsonConvert.cs:592)
CasinoPersia.Connected.Data.Item.ToString () (at Assets/Scripts/CasinoPersia/Runtime/connected/data/Item.cs:58)
System.Text.StringBuilder.AppendFormatHelper (System.IFormatProvider provider, System.String format, System.ParamsArray args) (at <eae584ce26bc40229c1b1aa476bfa589>:0)
System.String.FormatHelper (System.IFormatProvider provider, System.String format, System.ParamsArray args) (at <eae584ce26bc40229c1b1aa476bfa589>:0)
System.String.Format (System.String format, System.Object arg0) (at <eae584ce26bc40229c1b1aa476bfa589>:0)
CasinoPersia.View.VisualCarpet.SyncUI () (at Assets/Scripts/CasinoPersia/Runtime/view/VisualCarpet.cs:54)
CasinoPersia.View.VisualCarpet.OnValidate () (at Assets/Scripts/CasinoPersia/Runtime/view/VisualCarpet.cs:36)
UnityEngine.GUIUtility:ProcessEvent(Int32, IntPtr)

Expected behavior

Actual behavior

Steps to reproduce

  • New project
  • Import jillejr.newtonsoft.json-for-unity via UPM
  • Add following script to scene:
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using Newtonsoft.Json;
public class ColorSerializationTest : MonoBehaviour
{
    // Start is called before the first frame update
    void Start()
    {
        Test();
    }

    public static JsonSerializerSettings JsonConvertSettings()
    {
        var ret = new JsonSerializerSettings
        {
            ReferenceLoopHandling = ReferenceLoopHandling.Ignore,
            PreserveReferencesHandling = PreserveReferencesHandling.None // reference handling adds a $id field to most arrays/dictionaries which is a pain to deal with.
        };
        // ret.Converters.Add(new Newtonsoft.Json.UnityConverters.Math.ColorConverter());
        return ret;
    }

    public void Test()
    { 
        Color testColor = new Color(1f, 0.5f, 0.5f);
        var jsonStr = JsonConvert.SerializeObject(testColor, JsonConvertSettings());
        Debug.Log($"color json {jsonStr}");
        var desColor = JsonConvert.DeserializeObject<Color>(jsonStr);
        Debug.Log($"color deserialized {jsonStr}");
    }

    // Update is called once per frame
    void OnValidate()
    {
        Test();
    }
}
  • enable / disable the script added to camera or any object in scene.

Details

Windows 10 Home Version

Editor regardless of target

"jillejr.newtonsoft.json-for-unity": "13.0.102"
repros with older versions too.

2019.4.20f1

Checklist

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

Using the ColorConverter from UnityConverters namespace solves the issue, but the crash is bizzare given newtonsoft json should supposedly ignore the 'linear' field if referenceloophandling is set to ignore. Also the error stack is not very helpful and doesn't point to the actual underlying problem.

@hk1ll3r Hi! Yeah that error message is not that helpful, and Newtonsoft.Json usually has pretty good error messages. Thanks for reporting this!

The ReferenceLoopHandling setting only applies to reference types, and UnityEngine.Color is a value type (struct), so Newtonsoft.Json cannot check for this.

The Color.linear creates endless recursion, same as with Vector3.normalized and others. I had a vague memory that an error like "recursion detected" or "stack overflow" was thrown, but it seems not

Can confirm it still happens -even on builds-

super simple way to reproduce it:

var color = new Color(0.1f,0,0); //crash doesn't trigger with 0,0,0
var jsonSettings = new JsonSerializerSettings{ReferenceLoopHandling = ReferenceLoopHandling.Ignore};
var jsonData = JsonConvert.SerializeObject(color, jsonSettings);

@Aranclanos The UnityEngine types are not that serializing-friendly. Have you tried using the Newtonsoft.Json-for-Unity.Converters package?

I haven't checked if it's in the project, but I found that sending a Vector4 was a good and easy workaround for this issue

AS @jilleJr mentioned, the converters package solves the issue for color (and many other Unity types).