colyseus / colyseus-unity-sdk

⚔ Colyseus Multiplayer SDK for Unity
https://docs.colyseus.io/getting-started/unity-sdk/
MIT License
371 stars 100 forks source link

Schema Backward Compatibility not working as expected #205

Closed bawahakim closed 1 year ago

bawahakim commented 1 year ago

According to the documentation, schemas should be backwards compatible, even in statically-typed languages like C#: https://docs.colyseus.io/colyseus/state/schema/#versioning-and-backwardsforwards-compability

However, if I add a new type at the end of my schema on the server, I get the schema mismatched error. Would it make more sense to simply not check for schema types, and leave it to the responsibility of the developper? Propose code change

From:

System.Type schemaType = Array.Find(namespaceSchemaTypes, t => CompareTypes(t, reflection.types[i]));

if (schemaType == null)
{
    throw new Exception(
        "Local schema mismatch from server. Use \"schema-codegen\" to generate up-to-date local definitions.");
}

ColyseusContext.GetInstance().SetTypeId(schemaType, reflection.types[i].id);

To:

System.Type schemaType = Array.Find(namespaceSchemaTypes, t => CompareTypes(t, reflection.types[i]));

if (schemaType != null)
{
    ColyseusContext.GetInstance().SetTypeId(schemaType, reflection.types[i].id);
}
endel commented 1 year ago

Hi @bawahakim, thanks for the feedback and suggestion. I agree the SDK should be logging a warning instead of throwing an exception on schema mismatch.

To add a bit to this discussion, this should be properly documented somewhere soon:

The reason for throwing an error initially was because the CompareTypes() may return a false positive if two different types have the exact same properties. To avoid any false positives the best approach would be to avoid the global type registry (through Context.create, if you have more than one room, partially described on https://github.com/colyseus/colyseus-unity-sdk/issues/131), and generate client-side definitions with different namespaces per room type. (through schema-codegen --namespace Project.UniqueRoomNamespace)

bawahakim commented 1 year ago

@endel I opened a PR #206, tested to work locally for me. I am unsure if you want to merge this until the documentation is updated for the Context.create and false positives