applejag/Newtonsoft.Json-for-Unity

Cant Populate ScriptableObject(Any) List

wewrere41 opened this issue · 6 comments

Its works correctly if its a field of ScriptableObject.

public TestScriptableObject TestScriptableObject;

public void Populate
{
----
 JsonConvert.PopulateObject(reader.ReadToEnd(), TestScriptableObject); 
}

But it doesn work when its a List of ScriptableObject. It s creating new ScriptableObject and then adding the list. And also any type of list doesnt work clearly with populate. Its adding json list to target list. So its works like a ScriptableObjectList.AddRange(populatedList);

public List<TestScriptableObject> ScriptableObjectList;

public void Populate
{
---
 JsonConvert.PopulateObject(reader.ReadToEnd(), ScriptableObjectList); 
}
Unity_NHAiM6ZYf9.mp4

Hello @wewrere41! No, sorry, this is a feature of PopulateObject. It will always concat the new values from the JSON to the existing object.

There's no way to change the array merging behavior when using JsonConvert.Populate.

If you want to ensure the list is replaced then you could call ScriptableObjectList.Clear() before populating, but by that point you'd be better off just calling JsonConvert.DeserializeObject directly.

But I assume your use case here is that you want to dynamically load a save/checkpoint with data on those ScriptableObjects. For this, I'd suggest taking a different route here, such as by using a dictionary instead.

Eg.:

using Newtonsoft.Json;
using Newtonsoft.Json.Linq;

var objA = new MyScriptableObj{
    myInt = 1,
    myString = "original abc",
};
var objB = new MyScriptableObj{
    myInt = 2,
    myString = "original def",
};

var arr = new [] { objA, objB, };

var json = SerializeObjects(arr);
Console.WriteLine($"Saved state: {json}");

// Emulates some changes
objA.myString = "changed it";
objB.myInt = 129919;

Console.WriteLine("Before populate:");
Console.WriteLine($"  objA.myString = {objA.myString}");
Console.WriteLine($"  objB.myInt = {objB.myInt}");

PopulateExistingObjects(arr, json);

// Notice how I'm using the original objects here:
Console.WriteLine("After populate:");
Console.WriteLine($"  objA.myString = {objA.myString}");
Console.WriteLine($"  objB.myInt = {objB.myInt}");

static string SerializeObjects(IList<MyScriptableObj> objs) {
    var dict = objs.ToDictionary(o => o.GetInstanceID());
    return JsonConvert.SerializeObject(dict, Formatting.Indented);
}

static void PopulateExistingObjects(IList<MyScriptableObj> objs, string json) {
    var jObj = JToken.Parse(json) as JObject;
    if (jObj != null) {
        foreach (var obj in objs) {
            var id = obj.GetInstanceID().ToString();
            var objJson = jObj[id]?.ToString();
            if (objJson != null) {
                JsonConvert.PopulateObject(objJson, obj);
            } else {
                Console.WriteLine("Saved state contains reference to obj that no longer exists.");
                // TODO: ScriptableObject.Create<TestScriptableObject>();
                // or just ignore it. Your call
            }
        }
    }
}

class MyScriptableObj {
    public int myInt;
    public string? myString;

    // Placeholder implementation, Unity's Object type already implements this
    public int GetInstanceID() => GetHashCode();
}

Above results in the following:

$ dotnet run
Saved state: {
  "58225482": {
    "myInt": 1,
    "myString": "original abc"
  },
  "54267293": {
    "myInt": 2,
    "myString": "original def"
  }
}
Before populate:
  objA.myString = changed it
  objB.myInt = 129919
After populate:
  objA.myString = original abc
  objB.myInt = 2

Also, when using Newtonsoft.Json together with ScriptableObjects I recommend using my other package https://github.com/jilleJr/Newtonsoft.Json-for-Unity.Converters/ as it makes Newtonsoft.Json honor Unity's SerializeField attribute, as well as lots of other fixes like being able to serialize Vector3, LayerMask, and many other types

Hello @wewrere41! No, sorry, this is a feature of PopulateObject. It will always concat the new values from the JSON to the existing object.

There's no way to change the array merging behavior when using JsonConvert.Populate.

If you want to ensure the list is replaced then you could call ScriptableObjectList.Clear() before populating, but by that point you'd be better off just calling JsonConvert.DeserializeObject directly.

But I assume your use case here is that you want to dynamically load a save/checkpoint with data on those ScriptableObjects. For this, I'd suggest taking a different route here, such as by using a dictionary instead.

Eg.:

using Newtonsoft.Json;
using Newtonsoft.Json.Linq;

var objA = new MyScriptableObj{
    myInt = 1,
    myString = "original abc",
};
var objB = new MyScriptableObj{
    myInt = 2,
    myString = "original def",
};

var arr = new [] { objA, objB, };

var json = SerializeObjects(arr);
Console.WriteLine($"Saved state: {json}");

// Emulates some changes
objA.myString = "changed it";
objB.myInt = 129919;

Console.WriteLine("Before populate:");
Console.WriteLine($"  objA.myString = {objA.myString}");
Console.WriteLine($"  objB.myInt = {objB.myInt}");

PopulateExistingObjects(arr, json);

// Notice how I'm using the original objects here:
Console.WriteLine("After populate:");
Console.WriteLine($"  objA.myString = {objA.myString}");
Console.WriteLine($"  objB.myInt = {objB.myInt}");

static string SerializeObjects(IList<MyScriptableObj> objs) {
    var dict = objs.ToDictionary(o => o.GetInstanceID());
    return JsonConvert.SerializeObject(dict, Formatting.Indented);
}

static void PopulateExistingObjects(IList<MyScriptableObj> objs, string json) {
    var jObj = JToken.Parse(json) as JObject;
    if (jObj != null) {
        foreach (var obj in objs) {
            var id = obj.GetInstanceID().ToString();
            var objJson = jObj[id]?.ToString();
            if (objJson != null) {
                JsonConvert.PopulateObject(objJson, obj);
            } else {
                Console.WriteLine("Saved state contains reference to obj that no longer exists.");
                // TODO: ScriptableObject.Create<TestScriptableObject>();
                // or just ignore it. Your call
            }
        }
    }
}

class MyScriptableObj {
    public int myInt;
    public string? myString;

    // Placeholder implementation, Unity's Object type already implements this
    public int GetInstanceID() => GetHashCode();
}

Above results in the following:

$ dotnet run
Saved state: {
  "58225482": {
    "myInt": 1,
    "myString": "original abc"
  },
  "54267293": {
    "myInt": 2,
    "myString": "original def"
  }
}
Before populate:
  objA.myString = changed it
  objB.myInt = 129919
After populate:
  objA.myString = original abc
  objB.myInt = 2

Also, when using Newtonsoft.Json together with ScriptableObjects I recommend using my other package https://github.com/jilleJr/Newtonsoft.Json-for-Unity.Converters/ as it makes Newtonsoft.Json honor Unity's SerializeField attribute, as well as lots of other fixes like being able to serialize Vector3, LayerMask, and many other types

Thank you very much. Its works.
And I wondered some people suggest to use ObjectCreationHandling = ObjectCreationHandling.Replace for array/list replace. But it seems doesnt work. Its duplicating. So why they are suggesting this solution. I am confused.

var serializerSettings = new JsonSerializerSettings { ObjectCreationHandling = ObjectCreationHandling.Replace };

var list = new List<int> { 1, 2, 3 };
Debug.Log(list.Count); //3

var jsonString = JsonConvert.SerializeObject(list, serializerSettings);
JsonConvert.PopulateObject(jsonString, list, serializerSettings);

Debug.Log(list.Count); //6

https://stackoverflow.com/questions/20270266/json-net-populateobject-appending-list-rather-than-setting-value
https://stackoverflow.com/questions/25553459/json-net-calls-property-getter-during-deserialization-of-list-resulting-in-dupl/25555839#25555839
https://stackoverflow.com/questions/68648502/how-to-avoid-jsonconvert-populateobject-enqueue-on-lists/68653308#68653308

This is tricky to explain in just words, so let's look at an example:

using Newtonsoft.Json;
using Newtonsoft.Json.Linq;

var serializerSettings = new JsonSerializerSettings {ObjectCreationHandling = ObjectCreationHandling.Replace};

var myList = new List<MyScriptableObject> {
    new MyScriptableObject{myString="originalA"},
    new MyScriptableObject{myString="originalB"},
};

Console.WriteLine("=== Populate on list ===");
Console.WriteLine($"myList before (object ID={myList.GetHashCode()}):");
PrintObjectIds(myList);

var myListJson = @"[{""myString"":""newA""}, {""myString"":""newB""}]";
JsonConvert.PopulateObject(myListJson, myList, serializerSettings);

Console.WriteLine($"\nmyList after (object ID={myList.GetHashCode()}):");
PrintObjectIds(myList);


var myObject = new ContainerObject {
    objs = new List<MyScriptableObject> {
        new MyScriptableObject{myString="originalA"},
        new MyScriptableObject{myString="originalB"},
    },
};
Console.WriteLine("\n=== Populate on object containing list ===");
Console.WriteLine($"myObject.objs before (object ID={myObject.objs.GetHashCode()}):");
PrintObjectIds(myObject.objs);

var myObjectJson = @"{""objs"": [{""myString"":""newA""}, {""myString"":""newB""}]}";
JsonConvert.PopulateObject(myObjectJson, myObject, serializerSettings);

Console.WriteLine($"\nmyObject.objs after (object ID={myObject.objs.GetHashCode()}):");
PrintObjectIds(myObject.objs);


static void PrintObjectIds(IList<MyScriptableObject> objs) {
    for (var i = 0; i < objs.Count; i++) {
        Console.WriteLine($"  index {i}: object ID={objs[i].GetHashCode()}, myString=\"{objs[i].myString}\"");
    }
}


class ContainerObject {
    public List<MyScriptableObject>? objs;
}

class MyScriptableObject {
    public string? myString;
}
$ dotnet run
=== Populate on list ===
myList before (object ID=58225482):
  index 0: object ID=18643596, myString="originalA"
  index 1: object ID=33574638, myString="originalB"

myList after (object ID=58225482):
  index 0: object ID=18643596, myString="originalA"
  index 1: object ID=33574638, myString="originalB"
  index 2: object ID=26753075, myString="newA"
  index 3: object ID=39451090, myString="newB"

=== Populate on object containing list ===
myObject.objs before (object ID=19515494):
  index 0: object ID=41421720, myString="originalA"
  index 1: object ID=37251161, myString="originalB"

myObject.objs after (object ID=6480969):
  index 0: object ID=58328727, myString="newA"
  index 1: object ID=55196503, myString="newB"

For context, the default GetHashCode implementation in .NET is to return a unique ID of this object (usually just its pointer in RAM, sort of).

If you inspect the "object ID" of the output, you'll see that in the first "Populate on list" example keeps the original list and the 2 first objects (as they have the same IDs), in addition to the 2 new objects with the different myString value.

In the second example, "Populate on object containing list", you see that the myString is correctly different, but the object IDs are all different, including the list's object IDs.

What's happening under the hood here is that Newtonsoft.Json does not consider the ObjectCreationHandling on the root node. It's also worth noting that no matter what you set ObjectCreationHandling to, it will always create new MyScriptableObject/TestScriptableObject objects if those are inside a list. There's no merging-logic implemented into Newtonsoft.Json on the lists.

I'm not trying to advocate that the way Newtonsoft.Json is implemented is the way it should be, just trying to explain how it works. And as I am trying to stay as feature-par with upstream as possible then I will not be changing this kind of behavior in this fork.

My solution I posted in the comment above is a way to get around this, by effectively just not using lists/arrays at all.


In 99.99999% of use cases, this behavior does not matter. A common design pattern is to instead use Data-Transfer-Objects (DTOs), such as by doing:

  1. Create slimmed down class with only the fields you need to save.
  2. When serializing, create instance of this DTO class and manually populate all fields how you want them
  3. When deserializing, do so towards this DTO class and then manually update your currently-using objects based on these DTO instances.

There's multiple benefits of such a pattern, such as:

  • Full control of what data is stored, instead of having to add [NonSerializable]/[Serializable] attributes everywhere.
  • Easy way to format your data in the (de)serialization process. E.g controlling exactly how to format a DateTime as a string.
  • Other devs get an easy glance of all fields that are serialized by just looking at this DTO class, where there's no bloat from business logic code.
  • You can easily change the other code without accidentally breaking the serialization contract (what fields exist, what they are named), so no save files or API messages accidentally get corrupted/unreadable by older/newer versions.

The thing though with the DTO-pattern is that it takes more time in the short-term, but wins you time in the long-term. And for our monkey brains that doesn't really compute, so on the surface it's quite the uncharming design pattern. Btw I'm super guilty of this as well. I'm not using DTOs nearly as much as I should be in my projects.

Yes you can serialize and populate scriptable objects, but moving the serialization code to a different domain than the business logic could clean up the code a lot and make it less error-prone.

Closing as "resolved". If you have further questions dont refrain from reopening the issue, or create another issue.

Closing as "resolved". If you have further questions dont refrain from reopening the issue, or create another issue.

Thank you very much for detailed explanation.
I didn't quite understand some things. How its populate the real object without reference? Its using reflection? If its ,how can use reflection with local variables?
Edit: I get it. Json doesn't care parent object. So doesnt need parent reference.

What is the reason that root node does not consider the ObjectCreationHandling.
And I wrote ScriptableObject List converter for container objects inspired by you. And what is the reason that readjson doesnt read parent node. Like directly populate List<>
Edit: Doesnt consider because doesnt have parent reference. So have to recreate parent.
Cant read parent node because doesnt have parent reference.Then there is no reason to read parent when populate.

public class ScriptableObjectListConverter<T> : JsonConverter where T : ScriptableObject
{
    public override bool CanConvert(Type objectType) => typeof(List<T>).IsAssignableFrom(objectType);

    public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
    {
        var jObj = JObject.Load(reader);
        var scriptableObjects = (List<T>)existingValue;
        foreach (var obj in scriptableObjects)
        {
            var id = obj?.GetInstanceID().ToString();
            var objJson = jObj[id]?.ToString();
            if (objJson != null)
                JsonConvert.PopulateObject(objJson, obj);
        }
        return existingValue;
    }

    public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
    {
        var dict = ((List<T>)value).ToDictionary(o => o.GetInstanceID());
        serializer.Serialize(writer, dict);
    }
}

Yea populate has to keep the root object intact, otherwise it would just create a new object that was inaccessible. This is an implementation detail that PopulateObject only considers the children nodes (be it properties of a class or elements of a List<T>), which I wouldn't hold my breath expecting this to change, but instead you have to work around it, which you successfully have done.