marshallward/TiledSharp

Suggestion to change TmxTileset.Tiles to a Dictionary

bjorn opened this issue · 4 comments

bjorn commented

While using this library, I can into the issue that looking up a tile by its local ID required looping over the Tiles collection to find it:

string tileImage = null;
if (obj.Tile != null) {
    int gid = obj.Tile.Gid;
    for (int tilesetIndex = map.Tilesets.Count - 1; tilesetIndex >= 0; --tilesetIndex) {
        var tileset = map.Tilesets[tilesetIndex];
        if (tileset.FirstGid <= gid) {
            int tileId = gid - tileset.FirstGid;
            TmxTilesetTile tile = null;
            // todo: tileset.Tiles could be a map from tileId -> Tile
            foreach (var t in tileset.Tiles) {
                if (t.Id == tileId) {
                    tile = t;
                    break;
                }
            }
            if (tile != null && tile.Image != null)
                tileImage = tile.Image.Source;
        }
    }
}

Maybe Tiles should be a Dictionary<int, TmxTilesetTile>? I have it like that in libtiled and wondered if there would be any concerns in changing this library in that way.

Yes, I agree it's very annoying! I think I have to do something similar in one of my projects. I had assumed it was a TMX quirk and figured it was up to the user or render developrs to hash the values.

But I have no problem modifying this, particuarly since there will probably be some other API changes. Most people probably would not even notice the difference between a list and integer keys.

I'm on daddy-duty this weekend, but can give this a try myself in a few days. Or feel free to send something and I can have a play with it.

I've just made this change in the master branch, it was a bit rushed and not very well tested. But can you give it a try?

Will assume this is OK, please reopen if there are any issues.

bjorn commented

@marshallward I assume it will be fine, but I had no time to try it. Thanks for the change!