Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

accessing tileset firstgid property from python missing? #2191

Closed
sverx opened this issue Sep 4, 2019 · 15 comments
Closed

accessing tileset firstgid property from python missing? #2191

sverx opened this issue Sep 4, 2019 · 15 comments

Comments

@sverx
Copy link
Contributor

sverx commented Sep 4, 2019

In a python exporter I'm writing, I'd like to access the tileset's firstgid property but I can't find any way to do that. Is that missing?
(the point is that tileLayer.cellAt(x, y).tile().id() returns the id of the tile, not his gid, so if I'm using more than one tileset I can't really tell tiles apart...)
Any suggestion?

@bjorn
Copy link
Member

bjorn commented Sep 4, 2019

Tilesets don't inherently have a firstgid property. They are assigned and used by the JSON and TMX formats to map references to possibly multiple tilesets into a single range of numbers. But any other system could be used, like for example storing each tile as a tileset index + tile id pair.

If you want to use the same mechanism in your Python plugin, the assigning of firstgids is quite straight-forward since it starts with 1 for the first tileset and then goes up by the value of the highest local tile ID + 1 for each tileset.

For figuring out the highest local tile ID + 1, you could rely on the tileCount() function, but I just realized that this doesn't cover image collection tilesets, which can have gaps in the range of local tile IDs when tiles have been removed from the collection. If you need to handle that case, you'd need to increment the ID passed to findTile(id) until it has returned non-null tileCount() times.

@bjorn bjorn closed this as completed Sep 4, 2019
@sverx
Copy link
Contributor Author

sverx commented Sep 9, 2019

I see no way to get the tileset index directly from a tileset.
Also, using a for in tilesetCount range, I can't check if the tilesetAt(t) is the expected one, that does never become true:

for t in range(tileMap.tilesetCount()):
    if tileMap.tilesetAt(t).data() is tileLayer.cellAt(x, y).tileset():
     # this is never true

any hint?

@bjorn
Copy link
Member

bjorn commented Sep 9, 2019

@sverx I can't really explain this, apart from that they are apparently not the same Python objects while still holding the same Tileset* value. Possibly something related to how the generated bindings work. I'm not sure if there is another way to compare them, but I would suggest comparing by tileset name instead.

@sverx
Copy link
Contributor Author

sverx commented Sep 10, 2019

using:
if tileMap.tilesetAt(t).data().name() == tileLayer.cellAt(x, y).tileset().name():
works.

I'd say I can live with it, even if probably having an id() method in tileset class would be better (hint...)

Thanks!

@bjorn
Copy link
Member

bjorn commented Sep 10, 2019

I'd say I can live with it, even if probably having an id() method in tileset class would be better (hint...)

Right, tilesets don't currently have an ID. I have been considering to add globally unique IDs to each asset, but actually the file name should already be uniquely identifying the asset. The only problem with file names is that relative paths can break when you move files around and having globally unique IDs would provide a way to fix up these references, at least within one project. One problem with globally unique IDs is of course that if you copy a file you end up with two files that have the same "unique" IDs.

In any case, ideally, the direct Tileset* comparison should have just worked here... I'm not sure why it doesn't, but if it's related to the way the bindings work then maybe switching to a different mechanism would resolve that issue (see #2190).

@sverx
Copy link
Contributor Author

sverx commented Sep 10, 2019

I actually think some method (index()?) that would return 't' from a tileset so that
tileMap.tilesetAt(t).data()
would point to the tileset itself would be sufficient...

@bjorn
Copy link
Member

bjorn commented Sep 10, 2019

@sverx The tileset could be used by multiple maps, and it could have a different index in each map. But, I could imagine to add a function to the map to get the index of the tileset, so tileMap.indexOfTileset(tileset).

@sverx
Copy link
Contributor Author

sverx commented Sep 10, 2019

... so that you'll get the index that the specified tileset has in the map, which means you'll then be able to scan all the tilesets that are 'before' this one to calculate the needed tile's GID adding up the previous tileset's tileCount() - if I got that correctly.
I would say it should work!

@bjorn
Copy link
Member

bjorn commented Sep 10, 2019

Hrm, I just noticed that indexOfTileset is already there:

https://github.com/bjorn/tiled/blob/49516275e2fd02a787bad1136a8a8f8f531d4f64/src/plugins/python/tiledbinding.py#L195-L196

However, it takes a SharedTileset and not a Tileset*, so it can't be used with the tileset returned from the tile unless we expose Tileset::sharedPointer() in the Python API.

bjorn added a commit that referenced this issue Sep 11, 2019
To get the shared pointer from a Tileset* value, since this is needed to
pass it as an arguement to Map.indexOfTileset().

Also added trimming of trailing whitespace, which was causing empty
lines in the Issues view (affecting Python script output).

See issue #2191
@bjorn
Copy link
Member

bjorn commented Sep 11, 2019

The above commit adds the needed function, which should enable you to get the index of the tileset. It will be available in the next development snapshot (or if you compile Tiled yourself from master of course).

... so that you'll get the index that the specified tileset has in the map, which means you'll then be able to scan all the tilesets that are 'before' this one to calculate the needed tile's GID adding up the previous tileset's tileCount() - if I got that correctly.

Actually it's a bit easier, the steps I imagined were:

  • Iterate all the tilesets in the map, calculating for each of them the "firstgid" and storing this in an array (my previous note about finding the highest local tile ID applies here, unless you don't need to support collections, then tileCount is fine).
  • Iterate the cells of a tile layer, and for each tile, use tileMap.indexOfTileset to get the index of its tileset (passing the returned value from tileset.sharedPointer()).
  • Use that index to look up the "firstgid" from the array, which you can then add to the local tile ID to get the global ID.

Hope that helps!

@sverx
Copy link
Contributor Author

sverx commented Sep 11, 2019

Got it! I'll try it when the next development snapshot will be out.
Thanks!

@sverx
Copy link
Contributor Author

sverx commented Oct 16, 2019

OK I tried with latest development snapshot to use the new sharedPointer() instead of comparing the names of the tilesets.

# if tileMap.tilesetAt(t).data().name() == tileLayer.cellAt(x, y).tileset().name():
if tileMap.tilesetAt(t).data().sharedPointer() is tileLayer.cellAt(x, y).tileset().sharedPointer():

now it won't work, even using == instead of is

I'll try a different approach, as you suggested, but I'm quite puzzled about why the above wouldn't work.

@sverx
Copy link
Contributor Author

sverx commented Oct 16, 2019

this seems to work, instead:

for t in range(tileMap.indexOfTileset(tileLayer.cellAt(x, y).tileset().sharedPointer())):  
    tile_id+=tileMap.tilesetAt(t).data().tileCount()

@bjorn
Copy link
Member

bjorn commented Oct 16, 2019

I'll try a different approach, as you suggested, but I'm quite puzzled about why the above wouldn't work.

In one situation, the pointer comparison happens on the C++ side, where raw tileset pointers are compared and it will say whether they are the same tileset instance. In your situation, the comparison happens by Python and who knows what its comparison boils down to. At least we know, that it's comparing something else than tileset pointers. The Python API exposed for Tiled plugins may be creating different Python objects to wrap around the same tileset pointers, which could throw off that comparison.

Your last snippet should indeed work in general, but doing that for each tile it should be faster to set up a small array of "first global tile ID" first, then you could trivially add it for a given tileset index instead of having to do a for loop.

@sverx
Copy link
Contributor Author

sverx commented Oct 16, 2019

I see, so we really shouldn't trust pointer comparisons.

As for your hint, that's surely a possibile optimization. As in my current scenario I have only two tilesets and tiles in the second one are very rarely used, I think I don't even need that :)

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants