Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Export into RPTools' MapTool .rpmap format #2922

Closed
cpetig opened this issue Oct 24, 2020 · 22 comments · Fixed by #2926
Closed

Export into RPTools' MapTool .rpmap format #2922

cpetig opened this issue Oct 24, 2020 · 22 comments · Fixed by #2926
Assignees
Labels
feature It's a feature, not a bug.

Comments

@cpetig
Copy link
Contributor

cpetig commented Oct 24, 2020

MapTool https://github.com/RPTools/maptool is a nice tool to share and interact on maps during an online pen&paper RPG, but its map creation features are much below Tiled. I would like to create an exporter into rpmap files as a plugin into tiled.

Strategy:
create jar/zip file with context.xml and properties.xml, the assets are not necessary to export if the tile image is part of the library (assumed for now to keep the file size low), the asset id is a md5 hash of the image file contents.
content.xml contains grid info and an entry per tile in net.rptools.maptool.util.PersistenceUtil_-PersistedMap/zone/tokenMap
with x,y,z=1,scaleX/Y,name,layer,isFlippedX/Y,facing (rotation),imageAssetMap/entry/net.rptools.lib.MD5Key/id (md5)

@cpetig
Copy link
Contributor Author

cpetig commented Oct 24, 2020

PS: I would classify this as a feature and assign it to me if I had sufficient privileges on this project.

@cpetig cpetig changed the title Export into rptools' maptool rpmap format Feature: Export into rptools' maptool .rpmap format Oct 24, 2020
@cpetig
Copy link
Contributor Author

cpetig commented Oct 24, 2020

You can see the progress in https://github.com/cpetig/tiled/tree/feature_2922 - for now I was able to link to KF5Archive (in a hacky way - anaconda messed with my Qt installation badly) and create an empty zip skeleton.

@bjorn bjorn added the feature It's a feature, not a bug. label Oct 24, 2020
@cpetig
Copy link
Contributor Author

cpetig commented Oct 25, 2020

Update: The format very much feels like a dump of Java internals instead of an exchange format (with references from one data structure to another one encoded in XML) - I will keep working on it but this additional format complexity was an unpleasant surprise.

@cpetig
Copy link
Contributor Author

cpetig commented Oct 25, 2020

Update: I basically have a fully operational export, I only need to figure out the correct rotation of tiles.

@cpetig
Copy link
Contributor Author

cpetig commented Oct 25, 2020

Update: The export is fully covering my needs. Import seems to be a significant effort (which I won't need, AFAICT) and I have no idea on how to optionally depend on K5FArchive in a portable way (e.g. deactivating the module if missing) with Qt.

@bjorn
Copy link
Member

bjorn commented Oct 26, 2020

@cpetig That sounds great! Will you open a pull request for this feature? That would also make it easier to collaborate.

I don't know how heavy a dependency on K5Archive will be or how difficult it would be to set up this dependency on macOS or Windows for release. Alternatively, we could try QuaZip which seems pretty light-weight.

@bjorn bjorn changed the title Feature: Export into rptools' maptool .rpmap format Export into RPTools' MapTool .rpmap format Oct 26, 2020
@cpetig
Copy link
Contributor Author

cpetig commented Oct 26, 2020

Another option (which I would like to avoid) is to use qCompress() on the XML streams and write the zip header (only two files required) by hand. This would get rid of all dependencies. I am not sure whether this feels worth the effort to me.

@bjorn
Copy link
Member

bjorn commented Oct 26, 2020

Well, actually I find it quite interesting to also be able to read from .zip files since then we could support JavaScript extensions that are distributed as archives, without requiring the user to extract them.

@cpetig
Copy link
Contributor Author

cpetig commented Oct 26, 2020

That is a good reason to depend on a zip/jar/… module/library. Unfortunately writing a decent Qt build script is beyond my skills - but I volunteer to adapt this plugin to any zip library you include into tiled, sadly I can't contribute here.

@cpetig
Copy link
Contributor Author

cpetig commented Oct 26, 2020

On the other hand QuaZIP - as is - depends on zlib, which is a dependency nightmare on Windows of its own. Given qCompress and qUncompress are included in Qt one could probably port it using these.
KArchive on the other hand has only Qt (and optonally bzip2/lzma) as a dependency, see https://invent.kde.org/frameworks/karchive/-/blob/master/KF5ArchiveConfig.cmake.in

@cpetig
Copy link
Contributor Author

cpetig commented Oct 26, 2020

I found that KArchive internally also uses zlib https://invent.kde.org/frameworks/karchive/-/blob/master/src/kgzipfilter.cpp and QuaZIP is smaller but less flexible. 🤷 Any preferences from your side?

@bjorn
Copy link
Member

bjorn commented Oct 27, 2020

@cpetig Zlib dependency is not an issue. Tiled already uses zlib directly as well, and on Windows it relies on the zlib symbols being exported from Qt:

https://github.com/bjorn/tiled/blob/8b316fd217bd132aa551c5e3a2e799f622eabf11/src/libtiled/compression.cpp#L31-L35

Of course I can help with the build system stuff.

@cpetig
Copy link
Contributor Author

cpetig commented Oct 27, 2020

I took a closer look:

  quazip karchive
license LGPL-2.1 LGPL-2+
sloc 6249 7090
last release 2020-10-11 2020-10-04
c* files 16 15
h files 17 21
build tool cmake cmake
dependencies   ECM (>ubuntu)
binary size 232943 375832
optional deps   bz2+lzma
Formats zip (deflate+JLCompr) 7z+ar+tar+zip (deflate)+qrc
notes   ditched qmake

so basically I don't see any technical knock outs for either of them.
I don't like KArchive's dependency on ECM (but when we want to write our own build script this is moot) and I like that KArchive covers so many formats.

I assume that you want to bundle the library source code with Tiled and build it from qbs. I would suggest to place it inside src/KArchive or src/QuaZIP. Do you want to ship these as a dynamic library (hopefully works on Windows as well (most STL containers and strings don't when returned from a DLL)) or link statically to them?
[KArchive returns QByteArray by value which might be fine (don't know), QuaZIP returns QString. While I am not an expert on Qt on Windows I have a bad feeling [typically you need to deallocate and allocate from the same dll by providing a custom allocator]

@bjorn Any thoughts? I tend to prefer KArchive (also since I already used it). And I would recommend to avoid a DLL here (on Windows).

@cpetig
Copy link
Contributor Author

cpetig commented Oct 27, 2020

QuaZIP ditched qmake to avoid supporting two build systems: stachenov/quazip@1cf777c

@bjorn
Copy link
Member

bjorn commented Oct 27, 2020

It appears QuaZip is using a rather old fork of minizip and updating would be hard due to the patches having been applied on top. This kind of put me off from QuaZip. We could consider using minizip directly instead, though on a quick look it has an incredibly scary looking API.

So maybe we should indeed stick with KArchive, provided we can get it working on Windows and macOS. But what is ECM?

And indeed I think building as a static library would be best for now, to avoid having to ship additional libs. It could probably be built separately using cmake (similar to Zstandard, which is also built separately using just make) and then we just set the include path and linker options as needed.

And if it's really just 7k lines of code then including it in the repository is not really a problem. I'd look into using git subtree for this purpose.

@cpetig
Copy link
Contributor Author

cpetig commented Oct 28, 2020

ECM is extended cmake module of KDE (a common set of scripts of which a newer version is required than included with Ubuntu 20.10).
Thank you for the git subtree hint (I arrogantly thought I had a good idea of git's features until yesterday)

@cpetig
Copy link
Contributor Author

cpetig commented Oct 28, 2020

After reading https://medium.com/@porteneuve/mastering-git-subtrees-943d29a798ec I feel that a plain subtree is not what we want for tiled (it will import every commit from KArchive into tiled (unless we squash history) and it will by default push all adaptations by tiled back to KArchive). So I am quite sure we want a vendor branch (separate branch of tiled which only contains KArchive material and is synched with upstream) and a normal development branch containing tiled specific adaptations/add-ons (e.g. qmake file and zlib include adaptation etc.) not destined for upstream.

Do you have a (different) specific strategy in mind?
If so could you please import/subtree KArchive into tiled in a way which you feel comfortable maintaining for years to come?

@bjorn
Copy link
Member

bjorn commented Oct 28, 2020

I would definitely squash the history, but it could of course be that a subtree is not the best approach. It would probably be alright to set it up the same way as Zstandard, which is essentially that it's an optional dependency that is fetched and built for releases but not actually part of the repository (see c054911).

@cpetig
Copy link
Contributor Author

cpetig commented Oct 28, 2020

Thank you for explaining this interesting option. I feel that this might be a good option (fetching KArchive during the build), I will give it a try to disable building the rpmap plugin when KArchive is not enabled (e.g. present), then later we can decide on how to automatically include/compile it during the build process.

@cpetig
Copy link
Contributor Author

cpetig commented Oct 28, 2020

Oh, I now realize that qmake (.pro/.pri) and qbs (.qbs) are two totally independent build systems - with (I assume) qbs being the recommended system. And qtcreator can handle both (unless a forgotten anaconda installation in path breaks your system's Qt5, which it did on my machine).
And on ubuntu KArchive comes with a pri file, so probably you can easily use it from qmake.

@bjorn
Copy link
Member

bjorn commented Oct 28, 2020

Yeah... I wanted to remove the qmake projects since a long time, but I still can't remove them because the snap build of Tiled uses qmake (because the snap build system supports qmake but not qbs, so the latter would be rather harder to set up...).

@cpetig
Copy link
Contributor Author

cpetig commented Nov 14, 2020

Please see #2926 for further discussion.

bjorn added a commit that referenced this issue Nov 24, 2020
This new RpMap plugin adds support for exporting to the RPTools MapTool RpMap format.

Added `src/karchive` based on KArchive commit 0d55829a2ce093e83c68ac0d1cbef8b6a1814332.

The plugin is disabled when compiling with MSVC due to an unresolved linker error:

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

Closes #2922

Co-authored-by: Thorbjørn Lindeijer <bjorn@lindeijer.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature It's a feature, not a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants