POCO structs are treated as unmanaged despite GenerateType.VersionTolerant

[Rurouni]

Hi, we've recently run into an unforeseen backward compatibility issue in such scenario:

1. let's say we have such struct in protocol

[MemoryPackable(GenerateType.VersionTolerant)]
public partial struct SomeStruct
{
    [MemoryPackOrder(0)] public int SomeField1;
    [MemoryPackOrder(1)] public int SomeField2;
}

2. next we add another field

[MemoryPackable(GenerateType.VersionTolerant)]
public partial struct SomeStruct
{
    [MemoryPackOrder(0)] public int SomeField1;
    [MemoryPackOrder(1)] public int SomeField2;
    [MemoryPackOrder(2)] public int NewField;
}

Expectation: based on docs, we expected the old data would be still compatible just result in NewField getting default value on deserialisation

Actual Behavior: The struct is treated as unmanaged type, and the generated formatter does basic memcopy, ignoring GenerateType.VersionTolerant

From my initial investigation, it appears that problem lies in

MemoryPack/src/MemoryPack.Generator/MemoryPackGenerator.Parser.cs

Line 112 in 4b1e3f8

this.IsUnmanagedType = symbol.IsUnmanagedType;

so as soon as Roslyn tells us that it's unmanaged struct we treat it as unmanaged type disregarding GenerateType in attribute

This is quite dangerous, as one can expect the protocol to be backwards compatible if you use GenerateType.VersionTolerant and adheres to rules, but even adding a new property will make old data unreadable.

Thank you

[Rurouni]

@neuecc Sorry for pinging you directly, but can you please take a look at this issue? I can write an MR with a fix but I might need some pointers on where is the best place to correct this check

[neuecc]

Thank you. In this case, the principle from the README that we should follow is:

• unmanaged struct can't change any more

Therefore, if we were to add anything, it would be to include a warning that VersionTolerant cannot be specified for unmanaged structs.

[Rurouni]

Wouldn't it be better to generate version-tolerant formatter code instead when struct is unmanaged but attribute is present?
In some cases, you might have user-space structs in a shared game codebase between client and server that get reused in player profile data stored in a database, for example. In this case having version tolerant serialisation/deserialistion is a must-have. I don't think it's even a warning; it should be an error because the line between "unmanaged" and not "unmanaged" struct (from Roslyn's perspective) is not that obvious to the average developer, but the consequences are dire and only detected when much later down the line you can't suddenly break backward compatibility

Technically, there is a non-obvious workaround already, for example, if someone slightly changes the struct definition, for example, by adding some kind of reference type field that is always empty, and now suddenly formatter generated code is version tolerant.

[neuecc]

If we allow that, it will lead to inconsistencies in various places. The primary check for Serialize<SomeStruct> or Serialize<SomeStruct[]> is whether they are Unmanaged.

To maintain consistency, we would need to refactor the entire system to always prioritize the VersionTolerant check over the Unmanaged check.

Personally, I think struct that changes frequently should be made as class.

[Rurouni]

I see. In that case, analyser error is probably the best way forward indeed

[vrecluse]

If we allow that, it will lead to inconsistencies in various places. The primary check for Serialize<SomeStruct> or Serialize<SomeStruct[]> is whether they are Unmanaged.

To maintain consistency, we would need to refactor the entire system to always prioritize the VersionTolerant check over the Unmanaged check.

Personally, I think struct that changes frequently should be made as class.

The presumption that unmanaged structs should always be serialized bitwisely makes some cumbersome.
We are developing a game which using struct-based ECS archetecture.
It's hard to sync struct-based entities / components which need more fine-grain serialization controll and customization, and version-tolerant save data is also impossible.
Change struct to class hurts performance.
I forked MemoryPack and add implemented [PackAsManagedType] attribute, which makes the code generator and serializer treating unmanaged structs as managed structs.

db1dd70

By replacing most calls of RuntimeHelpers.IsReferenceOrContainsReferences<T> to

It works on simple cases and most of collections, but still failed when using ValueTuple<>, Nullable<T>, KeyValuePairs<K,V> etc.

@neuecc Any comments or suggestions would be appreciated.

[kostasbesson]

Hi,

I’ve fixed the issue where unmanaged structs were allowed to use GenerateType.VersionTolerant. Now, an error is triggered if an unmanaged struct tries to use VersionTolerant, preventing deserialization issues.

The fix includes:

•	Diagnostic MEMPACK041: Prevents unmanaged structs from being marked as VersionTolerant.
•	UnitTest: Added MEMPACK041_UnmanagedStructCannotBeVersionTolerant to validate the behavior.

Please check the merge request for more details.

[github-actions]

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 30 days.