#2933 Set the firstgid property on Tilesets

[csueiras]

Fixes issue #2933

I've also added test coverage for the property being set.

@bjorn I know there's some on going work to release the latest version of the library to Maven central, let me know how can I help out to get these changes into master and released. Thanks!

[bjorn]

Hmm, why do you need the tileset's firstGid attribute set? What part of the code relies on it?

I'd prefer if we could remove the firstGid member from the Tileset class instead, because I don't think it belongs there. It is only set when saving a map and only used while loading a map, and for that we have tilesetPerFirstGid.

[csueiras]

@bjorn isn't the firstgid required to map a global id back to a tileset? I am using this library in a server-side setting btw to parse out maps/tilesets.

[bjorn]

@csueiras Normally that happens while loading the map, here:

https://github.com/bjorn/tiled/blob/54d145e28661ca1ed8429ab8b137662b05590d46/util/java/libtiled-java/src/main/java/org/mapeditor/io/TMXMapReader.java#L1028-L1030

which is used from:

https://github.com/bjorn/tiled/blob/54d145e28661ca1ed8429ab8b137662b05590d46/util/java/libtiled-java/src/main/java/org/mapeditor/io/TMXMapReader.java#L780-L787

which is used from:

https://github.com/bjorn/tiled/blob/54d145e28661ca1ed8429ab8b137662b05590d46/util/java/libtiled-java/src/main/java/org/mapeditor/io/TMXMapReader.java#L765-L771

So in the end, you would be working with Tile references and should not need to care about mapping any global IDs.

[bjorn]

@csueiras With the clarifications above, do you still see a use for exposing the firstgid attribute? If you still need it, please let us know why.

[duarm]

More so than setting the firstgid to an specific value, For my use case, it would be enough to reorder each tileset when they are embedded into the map. I have an exclusive tileset for "non-colliding" tiles, setting that to the first tileset allows me to check for collision with tile < tilesets[0].firstgid. Of course I can reorder them while importing, but if they are already in order, it would help. Currently, if you create a new map and arbitrarily start painting, the firstgids would be out of order, and you can't reorder them without destroying the map.

[bjorn]

@duarm This PR is a proposed change affecting the Java library for loading Tiled maps. I don't think it has anything to do with your concern, which appears to be about manually setting the firstgid or manually ordering the tilesets. The latter is covered by issue #1613.

Note however, that the firstgids for the tilesets will always be incremental, as defined in the file format documentation. So, manually placing a tileset at the start of the list would change its firstgid to 1:

"If there are multiple <tileset> elements, they are in ascending order of their firstgid attribute. The first tileset always has a firstgid value of 1."

So I don't think your plan would work as such. You'd have to put that tileset first, and then check against the firstgid of the second tileset: tile < tilesets[1].firstgid. However, I'd still recommend against such an approach, and would rather advise to just find out which tileset the tile is from (as documented here) and check a custom tile or tileset property to decide whether a tile is colliding. It would be more flexible and more readable that way.

[duarm]

Sorry for the wrong issue, yeah, I meant to check with the second tileset, wrong pseudo code