Implement RpTools RpMap format

[cpetig]

Fixes #2922
Reading the format is not implemented.
I am most unhappy with the hackish Qt build system integration, but I don't know how to do it correctly (depend on module K5FArchive that means).

[cpetig]

Probably a good solution is to only build the module when K5FArchive is installed.

[cpetig]

• I just realized that I should make the static functions and variables class members ASAP, as this is horrible style.

[cpetig]

Help, @bjorn : I was able to include KArchive via git subtree and get it compiled as a static archive on all target architectures. But codeQL is unhappy with error: ‘class QFileInfo’ has no member named ‘birthTime’, which is documented to exist since Qt 5.10.

[bjorn]

CodeQL is unhappy with error: ‘class QFileInfo’ has no member named ‘birthTime’, which is documented to exist since Qt 5.10.

Right, the CodeQL workflow is currently building against Qt 5.9. We could use QFileInfo::created for older Qt versions, but since this is in KArchive code I think it would be preferable to set the minimum version for the RpMap plugin to 5.10.

I see the KArchive subtree appears to add about 1 MB to the size of the repository since it includes tests and examples. I'd also prefer if we didn't need to maintain a Qbs project file for KArchive, so I think we should go with requiring KArchive alongside when building this Tiled plugin. That would of course also resolve the above.

[bjorn]

@cpetig So I've pushed some small changes but will have to stop for today. I'll have to look into how to detect KArchive presence or we'll need to re-introduce the manual option.

In testing the plugin I didn't get much output, and on inspecting the code it turned out it was because it only does something useful when an image collection tileset is used. Probably would be good to raise a warning for this (see Tiled::WARNING function used in for example tBIN plugin).

[cpetig]

@bjorn Another option would be to include the tiles in the rpmap file (this way they are not required in maptool's library), but I was unaware that there are non-imgage collection tilesets 😮 For our purpose of drawing RPG maps this would just increase the file size. Probably adding an option for the plugin would be good (not sure how complex that would be).

Thank you for looking into detecting KArchive, I was not totally unhappy with having to maintain the <50 line KArchive qbs script, since we want to build a static library anyway (which IIRC isn't the default).

Also I found that git subtree commits make rebasing very difficult (at least my first attempt failed miserably)

[bjorn]

I was unaware that there are non-image collection tilesets

Just to clarify, all tilesets are based on images but usually the tiles are all in the same image, rather than stored individually. Would the RpMap format support this case? Or does it require each tile to be stored as its own image, and hence you are suggesting we'd include those cut tiles in the archive in this case?

Regarding KArchive, I looked into building it externally and it looks like too much of a maintenance burden to do this for all platforms, especially with it depending on stuff like cmake-extra-modules. It does a lot of stuff we don't need, so in the end I opted for keeping the copy and just trimming it down. Now we just need to get it to compile statically everywhere.

[cpetig]

@bjorn If you run into trouble rebasing this branch, there is lots of explanatory material at https://stackoverflow.com/questions/12858199/how-to-rebase-after-git-subtree-add

The short answer is
git rebase --preserve-merges

[cpetig]

Of course, tile collection images make perfect sense to me.

I have not taken an in depth look into the source code of RpMap, but on the GUI I was unable to find any functionality to use a tileset image. Also all the "tile sets" included with RpMap come with one tile/token per file.

Implementing an on demand export of tile images into RpMap format would be possible, but once you don't have an external image file (simply referring the MD5 sum is all you need here) you would need to encode it in either png or jpg and add it to the jar/zip/rpmap archive. With libz included generating a png on the fly should not be that difficult.

[bjorn]

Implementing an on demand export of tile images into RpMap format would be possible, but once you don't have an external image file (simply referring the MD5 sum is all you need here) you would need to encode it in either png or jpg and add it to the jar/zip/rpmap archive. With libz included generating a png on the fly should not be that difficult.

QImage::save can be used to save an image to a PNG (see the small example snippet). I guess it would be interesting if this worked, but I'll leave it up to you whether it's worth the effort.

I keep running into new problems when compiling with MSVC. Latest one is:

karchive.lib(karchive.cpp.obj) : error LNK2019: unresolved external symbol __imp_GetUserNameW referenced in function "class QString __cdecl getCurrentUserName(void)" (?getCurrentUserName@@YA?AVQString@@XZ)
C:\projects\tiled\release\rpmap.bf8dc609\rpmap.dll : fatal error LNK1120: 1 unresolved externals

Maybe this SO answer will help or maybe we can just disable this functionality since I don't see why we'd need to get the user name.

[cpetig]

Oh, having opened examples/sticker-knight/map/sandbox2.tmx I realize that I underestimated the complexity of tiled's map features a lot. I also see that there is a larger overlap (multiple layers, polygons, text, token names) of features.
To be honest the current rpmap export covers our needs well, I might add some features as we grow to use them (or if someone else asks for them), but I don't feel that feature completeness is my goal.

Thanks a lot for the QImage::save hint, that feels like creating image subparts as needed is really easy to add. Anyone has a need for that feature? Do I assume correctly that exporting desert.tmx would trigger the problem you described?

[bjorn]

Thanks a lot for the QImage::save hint, that feels like creating image subparts as needed is really easy to add. Anyone has a need for that feature? Do I assume correctly that exporting desert.tmx would trigger the problem you described?

I don't know if there's a need, I think you're a better judge of that than me. :-)

Indeed the desert.tmx example triggers this problem.

[bjorn]

I've tried many fixes for Visual Studio but I'm a bit at a loss for more options to fix this linker error that remains. Since actually Visual Studio isn't critical (since releases are done based on MinGW), we could consider disabling the RpMap plugin for this compiler for now and just add a comment explaining why.

[cpetig]

GetUserNameW sounds ubiquitous enough to not cause problems, you do link to advapi32.lib ?

[bjorn]

Not explicitly. Please feel free to try additional fixes. :)

[cpetig]

Having worked with visual studio at work and MinGW at home, with the offical binary built by MinGW, I see considerable effort for me to install the compiler but little gain. [I know it should be easy for me to fix this in the future if someone shows interest] If you could easily prevent visualC++ from compiling this plugin, I see more value in effort spent improving the features covered by this plugin.

[bjorn]

Seems I need to reconnect AppVeyor and Travis CI now that the repository has been moved.

[bjorn]

Thanks for your hard work on this, @cpetig!