Possible TileMap bug: tileAt and setTileGID

cocos2d-x C++
#1

Hello,

I believe I found a bug in CCTMXLayer, but since this code is new to me, I’d like to run it by some experts!

I have been playing around with the TileMap a bit, creating Sprites with CCTMXLayer::tileAt and then later eliminating them with CCTMXLayer::setTileGID. I noticed that the results of the setTileGID call were unstable: tiles were disappearing in places that I was not modifying.

Taking a closer look after successive calls to CCTMXLayer::setTileGID(0), which call into CCTMXLayer::removeTileAt, I noticed that atlasIndexForExistantZ(z) and sprite~~>getAtlasIndex are no longer consistent with each other after the first call.
I believe this is a result of CCSpriteBatchNode::removeSpriteFromAtlas.
This segment of code:
for
{
CCSprite* s = );
s~~>setAtlasIndex( s~~>getAtlasIndex~~ 1 );
}

Should be:

for(int i = 0; i < uIndex; i++)
{
CCSprite* s = (CCSprite*)(m_pobDescendants~~>objectAtIndex);
s~~>setAtlasIndex( s~~>getAtlasIndex~~ 1 );
}

Because the m_pobDescendants array is in ascending order of atlas index.

Any thoughts?

Thanks!

Ben

1 Like
#2

cocos2d-x does support gid now, and it seems that cocos2d-iphone doesn’t support it, either.

#3

I’m sorry… I don’t completely understand. This method is NOT supported by cocos2d? Is it just this method, or are there others in the tilemap?

Thanks

#4

here is a topic about this issue:

I have a problem of “TileMap”
http://www.cocos2d-x.org/boards/6/topics/3586?r=3592#message-3592

#5

Had the same problem as well. Consecutively adding and removing tiles from a tilemap messed up the atlas index and caused crashes.
Thanks OP for finding the problem.

Please implement this fix, as the problem is really hard to spot and fix

#6

Hi.

Confirm. Here is code sample and map.

Also render failed if map has 30x30 size. You can extend my map and check.

#7

I can confirm, call like

_map->layerNamed("elements")->removeTileAt(_selectedElementPosition);

leads to crash at

void CCTextureAtlas::updateQuad(ccV3F_C4B_T2F_Quad *quad, unsigned int index)
{
    CCAssert( index >= 0 && index < m_uCapacity, "updateQuadWithTexture: Invalid index");

and the fix is to change code in:

removeSpriteFromAtlas

to this

// update all sprites beyond this one
        unsigned int count = uIndex;
        for (uIndex = 0; uIndex < count; ++uIndex) {

            CCSprite* s = (CCSprite*)(m_pobDescendants->objectAtIndex(uIndex));
            s->setAtlasIndex( s->getAtlasIndex() - 1 );
        }
#8

Thanks Sergey Buravtsov, Bug #1519 created for this. We will merge your code soon.

#9

I am currently investigating this issue as well. With the latest github pull from master my code would crash on an assertion upon inserting a new sprite into SpriteBatch.

In insertQuad():
CCAssert( index < m_uCapacity, “insertQuadWithTexture: Invalid index”);

Using Ben & Sergey Buravtsov code allowed me to run my game without having it hit this assertion, however there were artifacts with some tiles being replaced by others, and tiles being removed at a random tile position instead of the tile the code was supposed to remove.

Just for information: In my case my m_pobDecendants were actually ordered in the ccArray in descending order: 152,151,150…

I noticed that the goal of removeSpriteFromAtlas seems to be to remove the tile with an AtlasIndex of uIndex and then decrement every sprite’s atlas index which is greater than uIndex.

So here’s currently the code I’m using that seems to so far not cause any artifacts or crashes. It would appear that Sergey’s code should work if they descendants are indeed ordered correctly at all times in the array, but I’ve noticed they’re ordered both in descending, ascending, and if enough inserts are done in one frame they can even be out of order (10,11,12,13,14,0,1,2,3).

* I will report back if I find it still produces a crash or artifacts
<pre>
void CCSpriteBatchNode::removeSpriteFromAtlas
{
// remove from TextureAtlas
unsigned int atlasIndex = pobSprite~~>getAtlasIndex;
auto descendantsData = m_pobDescendants~~>data;
unsigned int uIndex = ccArrayGetIndexOfObject;
CCLOGERROR (“spriteAtlasIndex = %d, uIndex = %d”, atlasIndex, uIndex);
m_pobTextureAtlas~~>removeQuadAtIndex);
// Cleanup sprite. It might be reused
pobSprite~~>setBatchNode;
if
{
CCObject obj = NULL;
CCARRAY_FOREACH(m_pobDescendants,obj)
{
CCSprite* child = static_cast<CCSprite*>(obj);
if(child)
printf(“removeSpriteFromAtlas: BEFORE: child p - atlasIndex:d\n”, child, child~~>getAtlasIndex);
}
ccArrayRemoveObjectAtIndex;
// update all sprites beyond this one
unsigned int count = descendantsData~~>num;
for(uIndex = 0; uIndex < count; uIndex++)
{
CCSprite s = descendantsData~~>arr[uIndex];
if
{
unsigned int ai = s~~>getAtlasIndex;
if
{
//CCLog;
s->setAtlasIndex;
}
}
}
}
// remove children recursively
CCArraypChildren = pobSprite->getChildren();
if (pChildren && pChildren->count() > 0)
{
CCObject* pObject = NULL;
CCARRAY_FOREACH(pChildren, pObject)
{
CCSprite* pChild = (CCSprite*) pObject;
if (pChild)
{
removeSpriteFromAtlas(pChild);
}
}
}
}

#10

almost nice to read I’m not the only one, were changes originally merged?

Did you have a chance to check and see if you had any errors? I am quite interested in this as I also suffer crashes with removetile, soon to try adding tiles.

#11

Haven’t had any issues since, using the code I posted, however I still am not sure if the actual issue was in the method `removeSpriteFromAtlas` or if the code I am using is just a temporary fix hiding the real issue??

#12

The problem still persists in cocos2d-x 3.0 alpha. There I have solved it by replacing the following code in CCSpriteBatchNode.cpp, removeSpriteFromAtlas

    auto it = std::find(_descendants.begin(), _descendants.end(), sprite );
    if( it != _descendants.end() )
    {
        auto next = std::next(it);

        std::for_each(next, _descendants.end(), [](Sprite *sprite) {
            sprite->setAtlasIndex( sprite->getAtlasIndex() - 1 );
        });

        _descendants.erase(it);
    }

with the following code:

    //Find index of sprite in _descendants
    std::vector::iterator it = find (_descendants.begin(), _descendants.end(), sprite);
    unsigned int uIndex = it - _descendants.begin();
    if (uIndex != UINT_MAX) {
        _descendants.erase(it);

        unsigned int count = uIndex;
        for (uIndex = 0; uIndex < count; ++uIndex) {

            Sprite* s = (Sprite*)(_descendants.at(uIndex));
            s->setAtlasIndex( s->getAtlasIndex() - 1 );
        }

    }
#13

I am not having any problems with 2.2 and my code is making hundreds of calls per turn to tileAt and setTileGID…

Am I missing something?

#14

Hi Cory, yes you are overlooking something. Setting the tile Gid is completely fine. But setting it to 0 (means removing it) does complete bogus. Here I have a minimal example to demonstrate this. What it is supposed to do is to iterate over the whole map and with each tap it removes/changes 1 tile going from (0,0), (1,0), (2,0), …(0,1), (1,1), (2,1)… and so on.

You will see, it does that just fine, using a tileGID bigger than 0, but for 0 it breaks down.

Upload did not work, for some reason, so download from https://drive.google.com/file/d/0B7hNxJW0HpqEZk1mUl94SXN1X2c/edit?usp=sharing.