Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Deserialization issue with generic type and ReadOnlySequence<byte> #351

Open
nathanAjacobs opened this issue Nov 20, 2024 · 2 comments
Open

Comments

@nathanAjacobs
Copy link

nathanAjacobs commented Nov 20, 2024

There is an issue deserializing the generic type defined below when using a ReadOnlySequence<byte>.

It doesn't seem to handle types that contain NO reference types correctly in this case, however using ReadOnlySpan<byte> it works just fine.

This was tested in Unity. I have not tested in a normal .NET environment.

[MemoryPackable]
public partial struct TestStruct<T>
{
    public int Id;
    public T Value;

    public override string ToString()
    {
        return $"{Id}, {Value}";
    }
}

public class TestDeserialization : MonoBehaviour
{
    void Start()
    {
        RunTest(1, 1);
        RunTest(2, "Hello World");
    }

    private void RunTest<T>(int id, T value)
    {
        TestStruct<T> testStruct = new TestStruct<T>() { Id = id, Value = value };

        byte[] data = MemoryPackSerializer.Serialize(testStruct);

        ReadOnlySpan<byte> span = new ReadOnlySpan<byte>(data);
        ReadOnlySequence<byte> sequence = new ReadOnlySequence<byte>(data);

        TestStruct<T> resultSpan = MemoryPackSerializer.Deserialize<TestStruct<T>>(span);
        TestStruct<T> resultSequence = MemoryPackSerializer.Deserialize<TestStruct<T>>(sequence);

        Debug.Log($"Span: {resultSpan}");
        Debug.Log($"Sequence: {resultSequence}");
    }
}

// Output:
// Span: 1, 1
// Sequence: 16777216, 0
// Span: 2, Hello World
// Sequence: 2, Hello World

// Expected:
// Span: 1, 1
// Sequence: 1, 1
// Span: 2, Hello World
// Sequence: 2, Hello World
@nathanAjacobs
Copy link
Author

@neuecc could this fix be merged in? If there are issues with the pull request, let me know.

@neuecc
Copy link
Member

neuecc commented Feb 5, 2025

sorry for delayed response.
I'll check it right away (I'll get back to you with the results tomorrow...!)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants