search

mapeditor / rs-tiled

Reads files from the Tiled editor into Rust
https://crates.io/crates/tiled
MIT License
268 stars 103 forks source link

Erase GIDs from the public interface #123

Closed aleokdev closed 2 years ago

aleokdev commented 2 years ago

Global tile IDs appear frequently in the codebase but I feel like they are not explained throughout, and they lack their own type so they can be confused with local (tileset) IDs.

bjorn commented 2 years ago

Related to this page that has been recently added to Tiled's manual: https://doc.mapeditor.org/en/stable/reference/global-tile-ids/

I'm not personally sure that global tile IDs are what should be exposed to the user, btw, given that they're just an artifact of the storage format. The C++ libtiled that is part of Tiled does not expose them, it has instead a Cell struct which stores a Tileset*, the local tile ID and the flipping flags. So, the resolving of the global IDs has already happened in the map loader. We could look into whether a similar approach could work for Rust.

aleokdev commented 2 years ago

If we do hide GIDs, what should happen with the first_gid parameter in the Tileset constructor? I guess we should just remove the first_gid member from tilesets entirely, given that they won't be useful outside of the library.

bjorn commented 2 years ago

If we do hide GIDs, what should happen with the first_gid parameter in the Tileset constructor? I guess we should just remove the first_gid member from tilesets entirely, given that they won't be useful outside of the library.

Yes, then there would be no reason to carry around the first_gid anymore.

aleokdev commented 2 years ago

Sounds like a great idea then. I can open the PR but won't be available until February (Have exams until then)

aleokdev commented 2 years ago

I've been thinking about possible gotchas. How would you be able to refer to a tile from a map property without using GIDs, for example?

bjorn commented 2 years ago

How would you be able to refer to a tile from a map property without using GIDs, for example?

Hmm, at the moment "tile reference" properties don't exist in Tiled. If you needed to do this, you'd have to consider some custom way to do this, for example using a string property that could refer to a custom name property that you've set on the referenced tile.

When tile references would be introduced as possible property values, I guess they could be saved as a GID, but loaded as a resolved tile reference, the same way as is done for tile layers and tile objects.

aleokdev commented 2 years ago

Is there a tracking issue for tile references in Tiled? I have a few instances where they'd be really useful (Mostly on tilesets though)

bjorn commented 2 years ago

Is there a tracking issue for tile references in Tiled? I have a few instances where they'd be really useful (Mostly on tilesets though)

It's come up before at some point, but I can't find an open issue about it. Feel free to open one. :-)