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

Added export to Defold .collection files #2084

Merged
merged 7 commits into from Aug 18, 2019

Conversation

@CodeSpartan
Copy link
Contributor

commented Mar 12, 2019

This pull request adds a plugin to Tiled, which allows exporting a map into a .collection (Defold's prefab).

Exporting into a collection allows the use of:

  • Group layers (only top-level group layers are supported, not nested ones!)
  • Multiple Tilesets per Tilemap

Upon export:

  • The Path property of each Tileset may need to be set up manually in Defold after each export. However, Tiled will attempt to find the .tilesource file corresponding with the name of the Tileset in Tiled in your project's /tilesources/ directory. If one is found, manual adjustments won't be necessary. The way Tiled finds the root of your project is by looking for a file called "game.project" while going up the hierarchy, starting from the directory where you save the collection. Therefore, if you save your Collection outside your project, manually setting up the Path property will be necessary afterwards.
  • If you create custom properties on your map called "x-offset" and "y-offset", these values will be used as coordinates for your top-level GameObject in the Collection.

All layers of a Tilemap will have Z-index property assigned with values ranging between 0 and 0.1.
The plugin supports the use of 9999 Group Layers and 9999 Tile Layers per Group Layer.
Z-Index is assigned by combining Group Layer's order (from bottom up) with Tile Layer's order (from bottom up) in the following way: group's order is divided by 10000, layer's order is divided by 1'0000'0000, and their sum is the Z-index. So, for example, if the Group Layer's order was 15, and the Tile Layer's order was 200, the resulting Z-index would be 0.00150200. This guarantees correct superimposing of all elements in Defold.

@bjorn
Copy link
Owner

left a comment

Thanks, this looks like a great new export option!

I am wondering if it still makes sense to keep the original Defold export plugin, or does it make sense to replace it with this one?

I've left a bunch of inline comments, mostly regarding coding style.

bjorn and others added 2 commits Mar 12, 2019
Update src/plugins/defoldcollection/defoldcollectionplugin.cpp
Co-Authored-By: CodeSpartan <codespartan.supp@gmail.com>
@CodeSpartan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2019

I've left a bunch of inline comments, mostly regarding coding style.

Done, let me know if I missed anything. The only thing that I didn't change was underscores in names like "cell_t", "cell_h", because I'm assuming that this is fine? This is how it is in the original defold plugin.

I am wondering if it still makes sense to keep the original Defold export plugin, or does it make sense to replace it with this one?

I would absolutely keep both, because not everyone needs collections. Besides, Defold heavily emphasizes light-weightness and performance, and they didn't implement out-of-the-box multiple tilesets per map because each tileset adds a draw call. So I would leave both and add a warning that using multiple tilesets results in multiple drawcalls, which can introduce a source of poor performance if users mix tiles from many tilesets.

Regarding Travis-CI failing, it looks like it's an internal error on their side, correct?

@bjorn

This comment has been minimized.

Copy link
Owner

commented Mar 13, 2019

Done, let me know if I missed anything. The only thing that I didn't change was underscores in names like "cell_t", "cell_h", because I'm assuming that this is fine? This is how it is in the original defold plugin.

Yeah, I realize we have precedence there. It's not my style, but I had decided to leave that be. Personally I don't like these names, because I do not know what the "t" or "h" mean (though my guesses would be "template" and "hash"). For string constants I usually use UPPERCASED_UNDERSCORED_STYLE.

I would absolutely keep both, because not everyone needs collections. Besides, Defold heavily emphasizes light-weightness and performance, and they didn't implement out-of-the-box multiple tilesets per map because each tileset adds a draw call. So I would leave both and add a warning that using multiple tilesets results in multiple drawcalls, which can introduce a source of poor performance if users mix tiles from many tilesets.

Alright. You're definitely right that using multiple tilesets will result in multiple drawcalls, unless of course some technical solution is implemented to avoid that, but either will have a runtime impact that can be avoided by creating the map using only a single tileset in the first place.

But when this plugin creates the same tilemap file as the other plugin, in addition to the collection file, then somebody using Defold can always choose to ignore the collection file and use the tilemap file directly if that suits his needs better. Then we avoid maintenance and a bit of confusion on the user side about which option to choose. Alternatively, we could merge the plugins so that a single plugin exposes the two different export formats, to share some of the code.

Regarding Travis-CI failing, it looks like it's an internal error on their side, correct?

Yes, it appears the Qt download is failing. I may need to make it more verbose somehow, but not too verbose. Right now it fails due to 10 minutes of no output while it is downloading Qt (which is quite large). But when I make the download verbose, it fails because it reaches the maximum log length...

@bjorn
Copy link
Owner

left a comment

Looks much better now (well I know it's partly a matter of taste), thanks for the changes! I've added one small request and a general question about the behavior.

@CodeSpartan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 15, 2019

Rewrote a lot of stuff today:

  • Added the possibility of having both top-level tile layers and top-level group layers
  • Now tilemaps only contain layers that have cells on them
  • Fixed a bug where the max amount of layers wasn't 9999 as intended, but 1000
  • Removed underscores from all variable names, even ones that were "_t" and "_h"

Regarding keeping the old format or not, in any case I would recommend leaving both in at least for a while, until this one is proven to be stable. I tested it in all ways I could think of, but I don't think it should count as "thorough". And I don't intend on working on the world for my game until much later, so if there's any bugs, I won't catch them myself. So consider keeping both export options at least for now.

Let me know if there's anything else that needs to be corrected.

@britzl

This comment has been minimized.

Copy link

commented Jul 28, 2019

Wow, this is great! @CodeSpartan could you please take a look at the requested changes so that this can get merged?

@bjorn

This comment has been minimized.

Copy link
Owner

commented Jul 28, 2019

@britzl I believe @CodeSpartan has already responded to all my requested changes and I've made some tweaks myself already as well. I'm not sure why I haven't merged this yet, but I'll be sure to take another look at it this week.

@stefankendall

This comment has been minimized.

Copy link

commented Aug 16, 2019

Any timeline for merging this?

@bjorn

This comment has been minimized.

Copy link
Owner

commented Aug 16, 2019

@stefankendall I really need to learn how not to forget about stuff I plan to do. I tend to get driven by email notifications and my last comment generated an email I expected to get around to, but I ended up working on other things that popped up afterwards.

My next full Tiled development day is Wednesday and I have set a reminder about this in my calendar, but I'll try to get around to it earlier.

@britzl

This comment has been minimized.

Copy link

commented Aug 16, 2019

@bjorn You could use something like Pull Panda to get reminders on open pull requests. Its integrated into the GitHub Marketplace ecosystem and is free to use.

@bjorn

This comment has been minimized.

Copy link
Owner

commented Aug 16, 2019

@britzl Hmm, it appears Pull Panda can only be installed on an organization account. Well, let's see whether using my calendar works first. :-)

@britzl

This comment has been minimized.

Copy link

commented Aug 16, 2019

@britzl Hmm, it appears Pull Panda can only be installed on an organization account. Well, let's see whether using my calendar works first. :-)

Ah, I see. Well let's hope the calendar works.

@bjorn bjorn merged commit 28bceb2 into bjorn:master Aug 18, 2019

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@bjorn

This comment has been minimized.

Copy link
Owner

commented Aug 18, 2019

So in fact I'm working on Tiled today. I've looked over the code again and did a quick test and it seemed to work fine. Unfortunately, I didn't test the merged result so I had to push a quick follow-up fix (72ba25d) due to a signature change.

Thanks a lot @CodeSpartan for your contribution! The plugin will be included in the next development snapshot (probably later this week). Do you think you'd have time to add a note about this new export option to the Export Formats section of the manual?

@CodeSpartan CodeSpartan deleted the CodeSpartan:defold-multi-tilesets branch Aug 19, 2019

@CodeSpartan

This comment has been minimized.

Copy link
Contributor Author

commented Aug 19, 2019

@bjorn done: #2186

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.