Error calculating GID's from Object Layers (from TILED)

[jackfreak]

Version

• Phaser Version: 3.16.2

• Operating system: MacOS 10.13.6

• Browser:

Description

I'm building a 2d platformer, making heavy use of TILED's Object Layers. I found that when I delete an object from a Tileset (Collection of Images), TILED does not re-use the internal id of the deleted object. So when developing a game, one usually add objects to a tileset, removes them, moves them to another Tileset that makes better sense to have it in, etc. That might left us with Tilesets with internal id's that are not consecutive, for example:

<?xml version="1.0" encoding="UTF-8"?> 
<tileset version="1.2" tiledversion="1.2.1" name="Castle" tilewidth="300" tileheight="913" tilecount="7" columns="1">
 <grid orientation="orthogonal" width="1" height="1"/>
 <tile id="2">
  <image width="300" height="350" source="../assets/castle/InnerGateBottomBG.png"/>
 </tile>
 <tile id="3">
  <image width="300" height="450" source="../assets/castle/InnerGateTop.png"/>
 </tile>
 <tile id="4">
  <image width="65" height="270" source="../assets/castle/Portcullis.png"/>
 </tile>
 <tile id="5">
  <image width="300" height="500" source="../assets/castle/TowerBottom.png"/>
 </tile>
 <tile id="6">
  <image width="300" height="450" source="../assets/castle/TowerTop.png"/>
 </tile>
 <tile id="10">
  <image width="300" height="500" source="../assets/castle/InnerGateBottomBase.png"/>
 </tile>
 <tile id="12">
  <image width="163" height="913" source="../assets/near/NearColumn.png"/>
 </tile>
</tileset>

In that .tsx file (TILED external tilesets files) we see that the tile id's 0,1, 7, 8, 9 and 11 are missing, they were deleted by me in the process of building the levels. The tile id's are not consecutive: (2, 3, 4, 5, 6, 10, 12) instead of (0, 1, 2, 3, 4, 5, 6).

This is ok within TILED, cause the internal gid is calculated as firstgid + internal id of a tile in a tileset.

I saw that when I deleted some object from a tileset in TILED, Phaser would show me objects (declared within that Tileset/ImageCollection) with different images than they were supposed to have.

I tracked the problem to be within Phaser.Tilemaps.Parsers.Tiled.ParseTilesets

Currently it has :

var newCollection = new ImageCollection(set.name, set.firstgid, set.tilewidth,
set.tileheight, set.margin, set.spacing, set.properties);

for (stringID in set.tiles)
{
    var image = set.tiles[stringID].image;
    var gid = set.firstgid + parseInt(stringID, 10);
    newCollection.addImage(gid, image);
}

imageCollections.push(newCollection);

stringID is the index within the array of tiles, NOT the id of the tile.

It should be something like this:

var newCollection = new ImageCollection(set.name, set.firstgid, set.tilewidth,
set.tileheight, set.margin, set.spacing, set.properties);

for (t = 0; t < set.tiles.length; t++)
{
    var tile = set.tiles[t];

    var image = tile.image;
    var gid = set.firstgid + parseInt(tile.id, 10);
    newCollection.addImage(gid, image);
}

imageCollections.push(newCollection);

I've tested it and it worked, now my Phaser objects have the correct image, taking into account the non-consecutive ids that the Tileset may have.

EDIT: Here's a link to a description as of why TILED work this way.
mapeditor/tiled#1233
mapeditor/tiled#1118

[jackfreak]

After writing this, I noticed another bug (actually part of the same thing). The objects with tile ids 10 and 12 were not showing up. I assume because of the way Phaser.Tilemaps.ImageCollection#containsImageIndex() calculates the range to return if the imageIndex belongs to that collection or not.

Currently it says:

 containsImageIndex: function (imageIndex)
    {
        return (imageIndex >= this.firstgid && imageIndex < (this.firstgid + this.total));
    },

this.total in the example above would be 7, but the actual range of internal ids is different, it's is equal to the maximum tile id + 1, in our case it would be 12 + 1.

It should be like this:

 /**
     * Returns true if and only if this image collection contains the given image index.
     *
     * @method Phaser.Tilemaps.ImageCollection#containsImageIndex
     * @since 3.0.0
     *
     * @param {integer} imageIndex - The image index to search for.
     *
     * @return {boolean} True if this Image Collection contains the given index.
     */
    containsImageIndex: function (imageIndex)
    {
        return (imageIndex >= this.firstgid && imageIndex < (this.firstgid + this.maxId + 1));
    },

Where this.maxId is the maximum value of all the tile id's in the collection.

maxId could be set from within ParseTilesets, or calculated in Phaser.Tilemaps.ImageCollection#addImage in which case we will need to pass the tile id also.

I tested it in Phaser.Tilemaps.Parsers.Tiled.ParseTilesets and it worked:

            var newCollection = new ImageCollection(set.name, set.firstgid, set.tilewidth,
                set.tileheight, set.margin, set.spacing, set.properties);

            var maxId = 0;

            for (t = 0; t < set.tiles.length; t++)
            {
                var tile = set.tiles[t];

                var image = tile.image;
                var tileId = parseInt(tile.id, 10);
                var gid = set.firstgid + tileId;
                newCollection.addImage(gid, image);

                maxId = Math.max(tileId, maxId);
            }

            newCollection.maxId = maxId;

[martinlindhe]

I suspect this may be the same issue I am observing.

A tile ID in my Tiled map has ID 101, and 101 is what gets exported to a json file by Tiled.
But after I load the json map into Phaser, the id (actually tile.index) is 102 in my particular case.
The tsx I use also has holes in it (id 2,3,4,14,16 etc is used).
May also be due to the fact that the id property does not seem to be preserved internally.

[jackfreak]

@martinlindhe TILED does this to prevent gid collisions. Tileset and image collections can be used by multiple maps so if you delete or move an object from an image collection it would have to re-index all its content, that also affects the map (if you change the local id of a tileset/image collection, the gid in the map will also change). The problem is that you would need to have all the maps that use that tileset/image collection opened in TILED at the moment you delete or move an object so it can re-index the gid's in the maps too. That's very fragile, you could end up with maps that no longer work if you forgot to open them when you delete and object in a collection.

I hope this fix gets to a release soon cause it should be affecting everyone that uses TILED and ObjectLayers. In the midtime a custom build of Phaser with the changes I mentioned above is the workaround.

[sean256]

I was having this same issue and your solutions @jackfreak saved me a ton of time. Thanks.

[jackfreak]

Glad to hear that it was useful. I should make a PR about this.

[mikuso]

Unfortunately this bug is still present in 3.22.0. This issue could use some love.

[photonstorm]

Thank you for submitting this issue. We have fixed this and the fix has been pushed to the master branch. It will be part of the next release. If you get time to build and test it for yourself we would appreciate that.