colyseus / colyseus-unity-sdk

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

Unity fatally crashes when using class implementing Schema as property #114

Closed jcjveraa closed 4 years ago

jcjveraa commented 4 years ago

When I use a class in unity generated by the colyseus codegen tool, unity crashes to desktop if I use the generated clas as a property with a getter/setter and I try to immediately register the onChange event to a listener, it crashes Unity to desktop.

I use Unity 2019.3.7f1, and behavior is consistent - also with other classes deriving from Schema.

/// The working version:
TileEntity tileEntity = new TileEntity();
tileEntity.tile = tile; // tile is an existing Tile passed as a method argument from a MapSchema OnAdd
tileEntity.Init();

// not working, crashing Unity
TileEntityCrashes tileEntityCrashes = new TileEntityCrashes (); // works till this point
tileEntityCrashes.tile = tile; // Unity crashes here

Classes:

using Colyseus.Schema;

public class Tile : Schema {
    [Type(0, "int8")]
    public int id = 0;

    [Type(1, "int8")]
    public int type = 0;

    [Type(2, "int8")]
    public int status = 0;

    [Type(3, "ref", typeof(Point2D))]
    public Point2D location = new Point2D();
}

Working fine

// Works fine when instantiating a new TileEntity and setting tile to an existing Tile instance
public class TileEntity : MonoBehaviour
{
   public Tile tile;

   public void Init() {
      tile.OnChange += Tile_OnChange;
   }

    private void Tile_OnChange(List<Colyseus.Schema.DataChange> changes)
    {
        // dosomething
    }
}

Should be the same, but crashes:

// Unity crashes to desktop when setting tile to existing Tile instance
public class TileEntityCrashes : MonoBehaviour
{
    public Tile tile
    {
        get
        {
            return tile;
        }
        set
        {
            tile = value;
            tile.OnChange += Tile_OnChange;
        }
    }

    private void Tile_OnChange(List<Colyseus.Schema.DataChange> changes)
    {
        // dosomething
    }
}
gioragutt commented 4 years ago

I think it's because you have an infinite loop here 😅the name of the property is tile, which is also what your return in the getter.

C# property names should be UPPER_CASE, and should be backed by a private field.

jcjveraa commented 4 years ago

... Good point. Had not been doing C# for a while. Weird that the compiler/linter does not see this and that in runtime it causes unity to completely crash rather than throwing some 'too many references' error.

I guess this can be closed :)

Op wo 22 apr. 2020 22:50 schreef Giora Guttsait notifications@github.com:

I think it's because you have an infinite loop here 😅the name of the property is tile, which is also what your return in the getter.

C# property names should be UPPER_CASE, and should be backed by a private field.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/colyseus/colyseus-unity3d/issues/114#issuecomment-618032919, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA6CPHPTFLKDLTQJFRJTECLRN5KAHANCNFSM4MDIQ5DA .

endel commented 4 years ago

Nice, good catch @gioragutt, gonna close this, then :)