Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Reflection and serialization issues between classes with the same number and type of fields #226

Open
JDWon opened this issue Apr 8, 2024 · 4 comments

Comments

@JDWon
Copy link

JDWon commented Apr 8, 2024

Hi, There is an issue related to Reflection and I would like to report it.

export class State2 extends Schema {
  @type('string')
  fieldString: string;

  @type('number') // varint
  fieldNumber: number;

  @type(Player)
  player: Player;

  @type([ Player ])
  arrayOfPlayers: ArraySchema<Player>;

  @type({ map: Player })
  mapOfPlayers: MapSchema<Player>;
}


export class State extends Schema {
  @type('string')
  fieldString: string;

  @type('number') // varint
  fieldNumber: number;

  @type(Player)
  player: Player;

  @type([ Player ])
  arrayOfPlayers: ArraySchema<Player>;

  @type({ map: Player })
  mapOfPlayers: MapSchema<Player>;
}

  • Two different classes with the same number and type of fields
  • In Handshake() of SchemaSerializer.cs, State2, not State, is expected to be specified as type id and State.
    => ColyseusContext.GetInstance().SetTypeId(schemaType, reflection.types[i].id); schemaType is State2
/// <inheritdoc />
public void Handshake(byte[] bytes, int offset)
{
    System.Type targetType = typeof(T);

    System.Type[] allTypes = targetType.Assembly.GetTypes();
    System.Type[] namespaceSchemaTypes = Array.FindAll(allTypes, t => t.Namespace == targetType.Namespace &&
                                                                      typeof(Schema.Schema).IsAssignableFrom(
                                                                          targetType));

    ColyseusReflection reflection = new ColyseusReflection();
    Iterator it = new Iterator {Offset = offset};

    reflection.Decode(bytes, it);

    for (int i = 0; i < reflection.types.Count; i++)
    {
        System.Type schemaType = Array.Find(namespaceSchemaTypes, t => CompareTypes(t, reflection.types[i]));

        if (schemaType != null)
        {
            ColyseusContext.GetInstance().SetTypeId(schemaType, reflection.types[i].id);   
        } 
        else 
        {
            UnityEngine.Debug.LogWarning(
                "Local schema mismatch from server. Use \"schema-codegen\" to generate up-to-date local definitions.");
        }
    }
}

In the following case, after reflecting the State, I expect the State to be designated as the Type id in the Serialize that occurs during the handshake, but State2 is designated, so I think I need to check.

Is it possible to check whether this is a restriction or whether separate exception handling is required?

@endel
Copy link
Member

endel commented Apr 10, 2024

Hi @JDWon, thanks for reporting. Curious if this is related with the "global context" described here? #131

If you define the properties of each State with their own "context" does this issue goes away? e.g.:

const type = Context.create(); // this is your @type() decorator bound to a context

class State2 extends Schema {
  @type(...) my_prop
}

(This limitation will be fixed on colyseus/colyseus#709 with no need to manually create contexts.)

@JDWon
Copy link
Author

JDWon commented Apr 11, 2024

Hi @endel , thanks for your reply. Unfortunately, even with the solution you provided, this problem occurs in Colyseus Unity sdk (Client schema).

// ex) State type id is 3, global context
export class State1 extends Schema {
  @type('string')
  fieldString: string;

  @type('number') // varint
  fieldNumber: number;
}

C# code State schema

[System.Serializable]
public class State2 : Schema
{
	//public string updateHash;

	[Type(0, "string")]
	public string id = default(string);

	[Type(1, "string")]
	public string ownerId = default(string);
}

[System.Serializable]
public class State1 : Schema
{
	//public string updateHash;

	[Type(0, "string")]
	public string id = default(string);

	[Type(1, "string")]
	public string ownerId = default(string);
}

From what I understand, the HandShake step of ColyseusSchemaSerializer in C# code performs reflection based on the field type, name, and field index. (
Assets/Colyseus/Runtime/Colyseus/Serializer/SchemaSerializer.cs => public void Handshake(byte[] bytes, int offset))

At this time, it appears that a class with the same configuration (type, name, index...) is found in the assembly and the typeID is set. (Assets/Colyseus/Runtime/Colyseus/Serializer/Schema/Context.cs => ColyseusContext.GetInstance().SetTypeId(..))

        /// <inheritdoc />
        public void Handshake(byte[] bytes, int offset)
        {
            System.Type targetType = typeof(T);

            System.Type[] allTypes = targetType.Assembly.GetTypes();
            // namespaceSchemaTypes = State2, State1
            System.Type[] namespaceSchemaTypes = Array.FindAll(allTypes, t => t.Namespace == targetType.Namespace &&
                                                                              typeof(Schema.Schema).IsAssignableFrom(
                                                                                  targetType));

            ColyseusReflection reflection = new ColyseusReflection();
            Iterator it = new Iterator {Offset = offset};

            reflection.Decode(bytes, it);

            for (int i = 0; i < reflection.types.Count; i++)
            {
                // Find State2, but I excepted find State1
                System.Type schemaType = Array.Find(namespaceSchemaTypes, t => CompareTypes(t, reflection.types[i]));

                if (schemaType != null)
                {
                   // Set type id 3, State2
                    ColyseusContext.GetInstance().SetTypeId(schemaType, reflection.types[i].id);   
                } 
                else 
                {
                    UnityEngine.Debug.LogWarning(
                        "Local schema mismatch from server. Use \"schema-codegen\" to generate up-to-date local definitions.");
                }
            }
        }

It is expected to read State1 during the reflection stage, but it reads State2 first, causing an unintended operation. This appears to be because State2 is declared first on an assembly basis and has the same field configuration.

I wonder if there is a plan to deal with this or if it is placed as a restriction.

@endel
Copy link
Member

endel commented Apr 11, 2024

Hi @JDWon, you're right, thanks for the detailed report! This is considered a bug and is intended to be fixed in the next version.

On the incoming v3 of schemas we have a TypeContext for the encoder and decoder. I'm not yet sure how to approach this in C#, will need to experiment!

@JDWon
Copy link
Author

JDWon commented Apr 12, 2024

Thanks for your reply. I will wait next version. Thanks!

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

No branches or pull requests

2 participants