tileson
tileson copied to clipboard
Add support for "Collection of Images" tilsets
It looks like "Collection of Images" tilsets (described in the tiled docs here: https://doc.mapeditor.org/en/stable/manual/editing-tilesets/#two-types-of-tileset) aren't supported.
On Linux, trying to load a map that contains such a tileset results in a SIGFPE
due to a division by zero error whilst trying to parse the tileset:
(gdb) run
<snip>
0x000055555568337c in tson::Tile::performDataCalculations (this=0x555555992120) at /home/gfg/git/foo/ext/tileson/include/common/tileson_forward.hpp:59
59 int rows = m_tileset->getTileCount() / columns;
(gdb) bt
#0 0x000055555568337c in tson::Tile::performDataCalculations (this=0x555555992120) at /home/gfg/git/foo/ext/tileson/include/common/tileson_forward.hpp:59
#1 0x000055555567ec8f in tson::Tile::parse (this=0x555555992120, json=..., tileset=0x555555ed59e0, map=0x555555ce2da0) at /home/gfg/git/foo/ext/tileson/include/tiled/Tile.hpp:175
#2 0x000055555567dba4 in tson::Tile::Tile (this=0x555555992120, json=..., tileset=0x555555ed59e0, map=0x555555ce2da0) at /home/gfg/git/foo/ext/tileson/include/tiled/Tile.hpp:102
#3 0x00005555556ab2c3 in __gnu_cxx::new_allocator<tson::Tile>::construct<tson::Tile, tson::IJson&, tson::Tileset*, tson::Map*&> (this=0x555555ed5a90, __p=0x555555992120) at /usr/include/c++/7/ext/new_allocator.h:136
<snip>
(gdb) print columns
$1 = 0
Is this something that you'd consider adding support or accepting patches for?
Thank you for the issue! You're right: "Collection of Images"-tilsets are not supported as of now. Mostly because I've never used it myself, and no one else has requested it, so I haven't really looked at it. I'll have to look into the details on how this can affect the Tile resolving and calculations that are done by Tileson on fixed size tilesets, but I'll put it on the 1.3.0 roadmap, and will look at it when I continue the work on this project :slightly_smiling_face: If it breaks the other features that makes Tileson convenient for fixed size tilesets, which I believe is the most common, I'll have to leave this unsupported, but I'll update this case when I have looked more into the details :slightly_smiling_face:
You should probably note that this is not available in the README lol. I've been troubleshooting why my project hasn't been working for a couple hours now. Good to finally get some resolution though.
I'm sorry to hear that, @jakehffn ! I've added this detail to the README now to prevent others having the same struggle 🙂
Man! Even I wasted some time trying to figure out what I was doing wrong😂. Guess I'll have to manually create a texture atlas of my collection of images.
@SSBMTonberry I'm thinking of working on adding support for collections of images, but thought I should ask if you've either already started, or already decided it would break too much to add support for this. Thoughts?
@jakehffn : I've added it to the next Release
(1.4.0), but I haven't really looked at the details yet, and it's now more than a year since this was adressed. Sorry about that, but feel free to pick up this issue and create a solution. It will have to go through a review process either way, so it'll be fine :slightly_smiling_face: Just try to preserve as much features as possible when using collection of images
vs tilesets
. Also make sure you read the contribution guidelines before you start, to prevent common mistakes :slightly_smiling_face:
Since this case is in the v1.4.0
features list, I would have a look at it eventually, but there are other cases I'm prioritizing first, and to be honest I haven't had much time working with this project lately, so any contribution really helps :+1: Assigning this case to you!
I've got collection of images working. No pull request yet, because there are no unit tests for it yet. Help creating those would be welcome.
https://github.com/SSBMTonberry/tileson/compare/master...ksdhans:tileson:master
Thanks for sharing your changes, @ksdhans! :smile:
Not sure if all the changes there can be used, as I must be sure all previous versions supported by Tileson
will still work, but I'll make sure this feature gets fully implemented after Tiled 1.8 (almost done) and Tiled 1.9 support is finished.
Moved this to v1.5.0, but will make this the first thing on the list when we get there.
- Should be fixed with #117