CCLayer appendTileForGID not setting Z index tag causing first screen of tiles to be re-created

I think I found a problem with the way CCLayer.cpp creates the initial screen of tiles. The problem seems to be in the function “CCTMXLayer::appendTileForGID” which doesn’t set the Z “index tag” value of the tiles it adds. So, when your game is running and you call a CCTMXLayer function that checks for tiles with a call to “getChildByTag” (just about any function), the result is NULL and the tile is re-created.

I’m not 100% what this means but I’m guessing you end up with either two tiles for each tile in the view area or a memory leak, or maybe just a bit of extra code re-creating the tile sprites unnecessarily.

Anyway, to prove it is simple. Just display a TMX tiled map and and then do a TileAt call to a point on the screen (and same layer) where you know there is a tile displayed. You’ll find the line in the “TileAt” function:

      tile = (CCSprite*) this->getChildByTag(z);

… doesn’t find the tile (tile==NULL) and so a new tile is created. This is because the Z “index tag” wasn’t set during “setupTiles”.

“setupTiles()” calls “appendTileForGID” to add tiles, it is “appendTileForGID” that should be setting the Z “index tag” value but isn’t.

The current “appendTileForGID” looks like this (last 12 lines):

        // optimization:
        // The difference between appendTileForGID and insertTileforGID is that append is faster, since
        // it appends the tile at the end of the texture atlas
        unsigned int indexForZ = m_pAtlasIndexArray->num;

        // don't add it using the "standard" way.
        addQuadFromSprite(m_pReusedTile, indexForZ);

        // append should be after addQuadFromSprite since it modifies the quantity values
        ccCArrayInsertValueAtIndex(m_pAtlasIndexArray, (void*)z, indexForZ);

        return m_pReusedTile;

A fixed but messy version is this:

        // optimization:
        // The difference between appendTileForGID and insertTileforGID is that append is faster, since
        // it appends the tile at the end of the texture atlas
        unsigned int indexForZ = m_pAtlasIndexArray->num;

        // append should be after addQuadFromSprite since it modifies the quantity values
        ccCArrayInsertValueAtIndex(m_pAtlasIndexArray, (void*)z, indexForZ);

        addSpriteWithoutQuad(m_pReusedTile, indexForZ, z);         // adds the sprite and sets the 'z' index tag
        addQuadFromSprite(m_pReusedTile, indexForZ);                // sets the tiles quad coordinates

        m_pReusedTile->release();
        m_pReusedTile = NULL;                                                   

        return m_pReusedTile;

In the above version you’ll notice we are using “m_pReusedTile” here and releasing it at the end anyway. So may as well chop a few lines out and have a CCSprite created each time. Here is the whole new function:

CCSprite * CCTMXLayer::appendTileForGID(unsigned int gid, const CCPoint& pos)
{
        CCRect rect = m_pTileSet->rectForGID(gid);
        rect = CCRect::CCRectMake(rect.origin.x / m_fContentScaleFactor, rect.origin.y / m_fContentScaleFactor, rect.size.width/ m_fContentScaleFactor, rect.size.height/ m_fContentScaleFactor);

        int z = (int)(pos.x + pos.y * m_tLayerSize.width);

        CCSprite* tile = new CCSprite();
        tile->initWithBatchNode(this, rect);

        tile->setPosition(positionAt(pos));
        tile->setVertexZ((float)vertexZForPos(pos));
        tile->setAnchorPoint(CCPointZero);
        tile->setOpacity(m_cOpacity);

        // optimization:
        // The difference between appendTileForGID and insertTileforGID is that append is faster, since
        // it appends the tile at the end of the texture atlas
        unsigned int indexForZ = m_pAtlasIndexArray->num;

        // append should be after addQuadFromSprite since it modifies the quantity values
        ccCArrayInsertValueAtIndex(m_pAtlasIndexArray, (void*)z, indexForZ);

        // don't add it using the "standard" way.
        addSpriteWithoutQuad(tile, indexForZ, z);
        addQuadFromSprite(tile, indexForZ);
        tile->release();

        return NULL;
}

Now to test, with the fixed “appendTileForGID” you’ll find calling “TileAt” with a coordinate of a tile that is already on screen, it will find the tile correctly at the line “tile = (CCSprite*) this->getChildByTag(z);” and so doesn’t re-create the tile.

Yeap, I think it is a bug.
#686 is created for it.
Thank you.

Why return NULL for CCTMXLayer::appendTileForGID()?

Before it was returning m_pReusedTile, but we aren’t using it anymore or a sprite which is re-useble. Or at least that was my guess, no documentation on the function as far as I know, and I couldn’t find any calls to appendTileForGID that used the return value. I guess you could make it “void appendTileForGID”.

All in all I don’t know, best ask the original programmer.