tileson icon indicating copy to clipboard operation
tileson copied to clipboard

Add support for "Collection of Images" tilsets

Open FinnG opened this issue 4 years ago • 8 comments

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?

FinnG avatar Feb 02 '21 13:02 FinnG

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:

SSBMTonberry avatar Feb 02 '21 16:02 SSBMTonberry

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.

jakehffn avatar Jul 03 '21 04:07 jakehffn

I'm sorry to hear that, @jakehffn ! I've added this detail to the README now to prevent others having the same struggle 🙂

image

SSBMTonberry avatar Jul 03 '21 05:07 SSBMTonberry

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.

ufrshubham avatar Sep 05 '21 11:09 ufrshubham

@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 avatar May 11 '22 22:05 jakehffn

@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!

SSBMTonberry avatar May 12 '22 05:05 SSBMTonberry

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

ksdhans avatar Aug 29 '22 12:08 ksdhans

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.

SSBMTonberry avatar Sep 03 '22 06:09 SSBMTonberry

Moved this to v1.5.0, but will make this the first thing on the list when we get there.

SSBMTonberry avatar Dec 11 '22 08:12 SSBMTonberry

  • Should be fixed with #117

SSBMTonberry avatar Dec 30 '23 18:12 SSBMTonberry