4
\$\begingroup\$

I am creating a tile based game in Unity and ran into a problem with my architecture of the Map and Tile types. The tile class contains logic like events when the tile type changes. When I tried to save the type to disk those events were being serialized as well, which I don't want (causing several problems in Unity). I basically want to make sure that I only save pure data like x, y position and enum types. Here is my approach:

public class Tile
{
    public event EventHandler<TileTypeChangedEventArgs> TileTypeChanged;

    public const TileType defaultType = TileType.Empty;

    public TileType type
    {
        get { return _type; }
        set
        {
            if (Enum.IsDefined(typeof(TileType), value) == false)
                throw new InvalidOperationException("Trying to set undefined tile type.");

            if (value != _type)
            {
                _type = value;
                var args = new TileTypeChangedEventArgs(_type);
                OnTileTypeChanged(args);
            }
        }
    }

    public int x { get { return _x; } }
    public int z { get { return _z; } }

    TileType _type;

    readonly int _x;
    readonly int _z;

    public Tile(int x, int z, TileType type = Tile.defaultType)
    {
        _x = x;
        _z = z;
        this.type = type;
    }

    protected virtual void OnTileTypeChanged(TileTypeChangedEventArgs e)
    {
        var handler = TileTypeChanged;
        if (handler != null)
            handler.Invoke(this, e);
    }
}

public class TileTypeChangedEventArgs : EventArgs
{
    public TileType type { get; private set; }

    public TileTypeChangedEventArgs(TileType type)
    {
        this.type = type;
    }
}

public enum TileType
{
    Empty,
    Grass,
    Water,
    Dirt
}

[Serializable]
public sealed class TileData
{
    public static readonly int version = 1;

    public TileType type
    {
        get { return _type; }
        set { _type = value; }
    }

    public int x
    {
        get { return _x; }
        set { _x = value; }
    }

    public int z
    {
        get { return _z; }
        set { _z = value; }
    }

    TileType _type;
    int _x;
    int _z;

    public static TileData Convert(Tile tile)
    {
        var data = new TileData();
        data.x = tile.x;
        data.z = tile.z;
        data.type = tile.type;
        return data;
    }

    public static Tile Convert(TileData data)
    {
        return new Tile(data.x, data.z, data.type);
    }
}

I created the TileData class because it would work similar to Data Transfer Objects in web programming. This way I am sure that my data remains serializable and I don't run into problems when I save a tile map and change it simultaneously.

However, it also means duplicated code and more methods for conversion. What do you think of my approach? Would you just modify the tile class so that it serializes correctly? I believe that I would also need to include copy methods to make sure saving to disk works while modifying the map state etc.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Welcome to Code Review! Nice first question! \$\endgroup\$
    – syb0rg
    May 19 '16 at 13:07
2
\$\begingroup\$

I have been experimenting and found this solution to be the best:

[Serializable]
public class Tile
{
    [field: NonSerialized]
    public event EventHandler<TileTypeChangedEventArgs> TileTypeChanged;

    [NonSerialized]
    public const TileType defaultType = TileType.Empty;

    public TileType type
    {
        get { return _type; }
        set
        {
            if (Enum.IsDefined(typeof(TileType), value) == false)
                throw new InvalidOperationException("Trying to set undefined tile type.");

            if (value != _type)
            {
                _type = value;
                var args = new TileTypeChangedEventArgs(_type);
                OnTileTypeChanged(args);
            }
        }
    }

    public int x { get; private set; }
    public int z { get; private set; }

    TileType _type;

    public Tile() { }

    public Tile(int x, int z, TileType type = Tile.defaultType)
    {
        this.x = x;
        this.z = z;
        this.type = type;
    }

    public Tile(Tile original)
    {
        this.x = original.x;
        this.z = original.z;
        this.type = original.type;
    }

    public override string ToString()
    {
        return string.Format("Tile: {0}|{1} = {2}", x, z, type);
    }

    protected virtual void OnTileTypeChanged(TileTypeChangedEventArgs e)
    {
        var handler = TileTypeChanged;
        if (handler != null)
            handler.Invoke(this, e);
    }
}

public class TileTypeChangedEventArgs : EventArgs
{
    public TileType type { get; private set; }

    public TileTypeChangedEventArgs(TileType type)
    {
        this.type = type;
    }
}

public enum TileType
{
    Empty,
    Grass,
    Water,
    Dirt
}

The problematic point was the accidental serialization of the event. I didn't know about the [field: NonSerialized] attribute for properties, but this works very well for the BinaryFormatter, XmlSerializer and Json.Net. I also had to add a little bit of ContractResolver logic for the later to deserialize private properties, but I found it much cleaner than having to supply more attributes in my data classes.

Another thing to note is the use of a copy constructor in the Tile class as well as in the Map which holds an array of tiles. When writing data to disk I copy everything before saving to avoid problems when I save on another thread while my users change the current map instance.

I'd still be happy to read any suggestions about how to improve my class design.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.