xoofx/Tomlyn

struct field not being deserialized

Opened this issue ยท 9 comments

Hello, I have a model with fields that consist of primitive types and a custom struct. The primitive fields get deserialized just fine, but the struct field does not and retains it's default values. Here are the TomlModelOptions I am using:

public static readonly TomlModelOptions TomlOptions = new TomlModelOptions {
    IgnoreMissingProperties = true,
    IncludeFields = true,
    ConvertPropertyName = name => name,
    ConvertFieldName = name => name
};

Any ideas on how I can get it to also deserialize the custom struct field?

xoofx commented

Could you post a reproducible simple Program.cs just to make sure about your exact use case?

xoofx commented

I can see in the tests that there are no tests with struct, so it is likely that I didn't need struct for my use cases and so didn't implement anything to support this (it always requires a special path with reflection, as you need to reassign the value type back after setting its fields to where it should be stored)

xoofx commented

(So, as a workaround, if you can, use class for now. It could be that I won't support this scenario - because I don't have a personal interest to implement this - but would emit an error instead)

I appreciate the fast response. I just wanted to mention this since I assumed it was a bug. Thanks for the suggestion. Changing the struct to a class is unideal for my situation which requires the use of structs for performance. I also pass the value around a lot and don't want to pass a reference. The struct fields change, so I would have to invoke a clone/copy method everywhere. But I did implement an intermediary class just for the deserialization. A bit of a hack, but it works just fine ๐Ÿ‘

Here is a simple reproducible script in case you decide you would like to implement it:

using Newtonsoft.Json;
using System;
using Tomlyn;

class Program
{
    public class Character
    {
        public string Name;
        public int Level;
        public Stats Stats;
    }

    public struct Stats
    {
        public int Strength;
        public int Dexterity;
        public int Wisdom;
    }

    const string toml = 
        @"
        Name = ""Tom""
        Level = 10
        [Stats]
        Strength = 7
        Dexterity = 8
        Wisdom = 9
        ";

    public static readonly TomlModelOptions TomlOptions = new TomlModelOptions
    {
        IgnoreMissingProperties = true,
        IncludeFields = true,
        ConvertPropertyName = name => name,
        ConvertFieldName = name => name
    };

    static void Main(string[] args)
    {
        Character character = Toml.ToModel<Character>(toml, null, TomlOptions);
        string json = JsonConvert.SerializeObject(character, Formatting.Indented);
        Console.WriteLine(json);
        /*
        {
            "Name": "Tom",
            "Level": 10,
            "Stats": {
                "Strength": 0,
                "Dexterity": 0,
                "Wisdom": 0
            }
        }
        */
        Console.ReadLine();
    }
}

Thanks again for the fast response and help!

xoofx commented

Changing the struct to a class is unideal for my situation which requires the use of structs for performance.

Yeah, Tomlyn was not really designed with performance in mind ๐Ÿ™‚ (the parser is not wasting GC but otherwise, it's using reflection when mapping to .NET objects which kills quickly performance anyway...) but mainly to help loading easily config files, hence why I didn't bothered with structs (afair)

If you point me in the right direction, I can take a look and attempt a PR when I get some time, if you're open to adding support for structs.

@xoofx I just wanted to follow up and see if you would be accept a PR that added support for structs. Let me share my use case: I am a game developer who uses toml + tomlyn for game data. Right now I use toml to define mods. A lot of the built-in Unity data types as well as our own custom data types are structs for various reasons ranging from performance to the need to pass by value a lot. So, while the class hack works for now, it's unideal for our project needs long term. Let me know what you think! I am happy to give it a go, but wanted to run it by you before attempting a PR.

xoofx commented

If you point me in the right direction, I can take a look and attempt a PR when I get some time, if you're open to adding support for structs.

My apologize, forgot to come back to you.

Most of the code is in SyntaxToModelTransform.

Then there is the code associated with list, primitives and managed object accessors (for example: StandardObjectDynamicAccessor)

The challenge for working with struct is that when you start to update a property/field and the target type is a struct, you would need to visit the stack of objects being written and apply the set on the parent objects (and so on, if the parent is the struct, you need to repeat this).

The class SyntaxToModelTransform contains the stack of objects being written to via _objectStack. I would believe that this could be used as is to traverse parent objects.

Thanks so much for this, I will take a look when I have some time and see if I can figure it out.