GDevelop icon indicating copy to clipboard operation
GDevelop copied to clipboard

[$525 Bounty] Add support for tilemaps made in LDtk (build a parser for .ldtk files)

Open Silver-Streak opened this issue 4 years ago • 70 comments

Description

Currently the Tilemap object in GD5 only supports Tilemap/Tileset JSON files made by Tiled Map Editor.

As discussed elsewhere, The Level Designer's Toolkit (LDtk) is a newer tilemap editor that has a much more user friendly interface, and allows for much faster iteration than what is available in Tiled Map Editor. LDtk is completely free and available at https://ldtk.io. It is developed by deepnight who is a dev at the studio Motion Twin, who have made a ton of retail games. (Dead Cells being a personal favorite of mine)

While LDtk does support exporting to Tiled TMX, this file cannot be consumed by GDevelop's tilemap objects, so it requires opening it in Tiled then exporting it as a JSON. This also strips some of the more advanced features in LDtk that are greatly beneficial for game development.

Solution suggested

Short-term solution :

  • Update GDevelop's tilemap object handling to interpret and support LDtk made maps. LDtk can export as JSON, so a new LDtk specific parser would need to be built (or th current one updated to detect which type of JSON and load it accordingly). LDtk's JSON format is documented here: https://ldtk.io/json/
  • This would allow users to use LDtk as their primary map editor outside of GDevelop, and skip numerous steps in the current process as LDtk already embeds tilesets into their files.
    • I would probably recommend this support be for files with the .LDtk extension to not get them confused with Tiled Map JSON files. .ldtk files are just JSON files with a custom extension
    • I'm unsure if it will help, but LDtk's json format is fully supported by Quicktype (https://app.quicktype.io), they give some instructions about that here: https://ldtk.io/docs/game-dev/loading-ldtk-in-your-game/. Quicktype can spit out a Javascript (or C++) interpreter for the JSON with a click of the button. Not sure how easily this will help loading it into the Tilemap object parser.

Long-term solution (outside of the scope of this feature request):

  • Fully integrate LDtk into GDevelop as the primary Tilemap editor. This has an open bounty and is under conversation here: https://github.com/4ian/GDevelop/discussions/2352
  • Additionally, add support for LDtk's more advanced features such as Worlds (Linking multiple tilemaps), IntGrid layers, and Entity/Enum Support would allow for a lot more flexibility for gamedev (especially all of the Entity Support. It would allow for custom paths/points and could be used for things like enemy paths and other things). This would require the Short-term solution to be implemented first.

Alternatives considered

I thought about asking to support TMX Format files since LDtk can export to that format, but those are in XML and would require a completely different interpreter. It also wipes out a lot of the more advanced features LDtk provides.

Bounty

Learn more about the bounty here: https://www.bountysource.com/issues/97132654-feature-request-add-support-for-tilemaps-made-in-ldtk-build-a-parser-for-ldtk-files

Silver-Streak avatar Mar 15 '21 07:03 Silver-Streak

Solution suggested

This is indeed the proper solution that should be implemented. I would add:

  • Optionally might be a good idea to upgrade to latest version of pixi-tilemap first (verify it works with the tilemap examples).
  • Then implement support for parsing .LDtk (json) files in the Tilemap object (pixi-tilemap-helper.js, so that it works both in the IDE and at runtime). This is the important part where good quality code is required to parse both Tiled and LDtk without dirty hacks.
  • Finally add a new ".ldtk" resource to the IDE (I can help if needed).

4ian avatar Mar 15 '21 09:03 4ian

Thank you for opening a separate issue. I can pick it up if nobody else wants to give it a try. :) This time we have a good starting point with the helper class. As Florian says it would be a good opportunity to update pixi tilemap

On Mon, 15 Mar 2021, 09:19 Florian Rival, @.***> wrote:

Solution suggested

This is indeed the proper solution that should be implemented. I would add:

  • Optionally might be a good idea to upgrade to latest version of pixi-tilemap first (verify it works with the tilemap examples).
  • Then implement support for parsing .LDtk (json) files in the Tilemap object (pixi-tilemap-helper.js, so that it works both in the IDE and at runtime). This is the important part where good quality code is required to parse both Tiled and LDtk without dirty hacks.
  • Finally add a new ".ldtk" resource to the IDE (I can help if needed).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/4ian/GDevelop/issues/2434#issuecomment-799256516, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRRWVM7XSGYRENWOYGQUU3TDXGLBANCNFSM4ZF7OIMA .

blurymind avatar Mar 16 '21 08:03 blurymind

Thank you for opening a separate issue. I can pick it up if nobody else wants to give it a try. :) This time we have a good starting point with the helper class. As Florian says it would be a good opportunity to update pixi tilemap

No concerns from me, although I'm not an approver for work on this, so we'll wait on @4ian to chime in.

Silver-Streak avatar Mar 16 '21 21:03 Silver-Streak

Feel free to go ahead!

4ian avatar Mar 16 '21 21:03 4ian

Note though as this will create more maintenance in the future, I expect properly organized and clear code so that it's easy to update in the future. We could also integrate unit test (with .spec.js files)

4ian avatar Mar 16 '21 21:03 4ian

So on the topic of file types, I want my extension to take in both a json and an ldtk file as tilemap resource.

Option 1 Now doing that means adding the "ldtk" extension to the json resource type. There is a downside to that - when you select json files, now the ldtk extension is added to them. The ldtk file is technically a json file yes,but the extension should let the user filter it out as a tilemap specifically and sepparate it from other json files.

Option 2: Adding a new resource kind - a tilemap resource, which allows for both json and ldtk files. That way when we open json files we wont be getting ldtk files also displayed in the file chooser.

So for option 2 - do I need to recompile the c++ GD to work? I notice that the 'json' kind is in the source code. If so I might need help to get that in order to have something working with it. Didnt we have instructions somewhere on how to edit the c++engine and recompile it? If not couldnt you push a change to it to allow for a timemap resource or something - it wont affect the users since it wont yet be exposed to the frontend

I can go with Option 1 for now and we can sort it out later if its more complicated than it seems - since it seems like a polish thing to do anyways.

If I can at least get to properly open ldtk and json files from one field in my extension, I can then focus on the actual parsing. Btw Option 1 seems easy to do for me without much help, but I'm not sure if its what we want

blurymind avatar Mar 19 '21 19:03 blurymind

@Bouh added a "bitmapFont" in the upcoming BitmapText PR so yeah that's the way to go here too: create a "tilemap" resource (that can be a .json or .ldtk). I.e: option 2

But you can go for option 1 and I can rework the core to have a "tilemap" resource. Then we'll update the extension to have a "tilemap" resource property instead of a resource of type "json".

We should do this btw for yarn json files too. We should rework the actions so that they don't use a "jsonResource" but a "yarnResource" :)

4ian avatar Mar 19 '21 19:03 4ian

I really want to go with option 2, but have no idea how to recompile and use the c++part of GD. The bitmapFont commit could inform me how to do the code part, but not the update gd.js part.

I am now developing it on linux, so in theory at least compiling c++ should be easier, right?

is that the pr? https://github.com/4ian/GDevelop/pull/2432/files

in my case it should be easier since they are all technically a jsonResource

blurymind avatar Mar 19 '21 19:03 blurymind

Yes. It's a massive one but you can search for "bitmapFont" everywhere to see.

Anyway, maybe you can concentrate on coming up with a clean and nice parser that adapts well in what we have now, and we can solve this "json" => "tilemap" resource latter :)

I am now developing it on linux, so in theory at least compiling c++ should be easier, right?

Yes, follow the README in the GDevelop.js folder and let me know :)

4ian avatar Mar 19 '21 19:03 4ian

is that the pr? https://github.com/4ian/GDevelop/pull/2432/files

Yes. I can compile gd.js if needed. Just tag me in the PR i copy the branch and compile and share with you the compiled file.

Bouh avatar Mar 19 '21 19:03 Bouh

no worries, I got it to compile now :) I copy and pasted some boilerplate c++ code to create a TilemapResource, but am yet to see if that did the trick

blurymind avatar Mar 19 '21 20:03 blurymind

Some progress report today:

  • I successfully added a new tilemap resource that recognises both ldtk and json file extensions and loads the json data from them.
  • added a new levelIndex parameter, since the ldtk file allows for several levels to be stored in a single file. Using levelIndex we can tell GD to only render one level at a time in a tilemap object

blurymind avatar Mar 21 '21 19:03 blurymind

Nice! :)

4ian avatar Mar 21 '21 20:03 4ian

Some progress today - I got the conditional splitting to two different methods to create the generic tilemapData- based on the file header's key telling GD what application was used to create it. I am using Ldtk's official examples for testing.

There is still a lot to do before it can get a basic render, but its interesting how the automatically generated tiles are just generated in each tilemap level's layer.

A tilemap layer in it can have a different atlas from the other layers and stores a relative path to that atlas - so I will for sure need to get it to automatically add these image resources in the extension's helper module somehow and not the IDE - that is if we want to be able to render all layers of a tilemap at once- which we most definitely do. For now I am just trying to get it to work with a single one, but will try to tackle that later on. I wonder if I can pass to it Gd's resources manager and just auto-add these relative path atlases when looping through each layer.

blurymind avatar Mar 22 '21 20:03 blurymind

Some progress today:

I came up with a way to automatically load atlas image resources on a per layer basis - if they are not identified by GD as an existing image resource - they will get added via the resource manager.

Started figuring out how to slice up the tileset for ldtk's tilemap, managed to cache some tileset textures, but I am yet to find out if they are the right rects

blurymind avatar Mar 24 '21 19:03 blurymind

Some progress today, I got it to render LDTK layer! Yay :)

image

Still lots to do before this can be a pr - there are different types, flipped tiles, etc etc. Thankfully the data structure is kind of nice when compared to tiled's.I had to jump through less hoops to identify the position of tiles because led spells out their position in px - unlike tiled which makes us manually do the math. Same for the slicing of the tileset. https://ldtk.io/json/ It is kind of competing with tiled really well here - simply because it gives us easier to use data.

Why in the name of crazy bats doesn't tiled do that too? @bjorn ? I can get rid of a lot of the code for the tiled parsing if we simply had the damned tile src and px coordinates in the exported file. pixi-tilemap requires these absolute x,y coordinates - both for the tile positions in the tileset atlas and the tile x/y coordinates on the tilemap.

blurymind avatar Mar 25 '21 19:03 blurymind

Yes, looking at the LDtk json format made a ton of sense to me, and I haven't dealt with JSON/Javascript for work in like 4 years. Deepknight did really good work designing it.

Excited to see the continued progress.

Silver-Streak avatar Mar 25 '21 19:03 Silver-Streak

I must say removing all this complexity also results in less fragile code - less things can break. Its just those two values that make a huge difference.

If @bjorn adds them to the output of tiled data - I will be able to remove the nested for loops and all the math expressions to compute where the tile is on the atlas and on the tilemap. That stuff is what makes it fragile

blurymind avatar Mar 25 '21 19:03 blurymind

Why in the name of crazy bats doesn't tiled do that too? @bjorn ?

I think the clear redundancy of the information was the main reason not to include it, especially 17 years ago when the TMX format was introduced. I could certainly consider adding this information, and had actually opened issue mapeditor/tiled#2863 about this last year, which is about including the tile rectangle on the tileset image in the tileset definition.

The same applies to the information storing the location of each tile on a tile layer, which can be almost trivially derived from its index and the grid size. It wasn't "crazy bats" to not include that information, it made perfect sense because including that would have made the files that much bigger and we could no longer just use an array of numbers to store a tile layer (nor compress it). Actually, when exported like this a tile layer would look exactly like an object layer, so maybe we could add an option to export tile layers as object layers?

bjorn avatar Mar 26 '21 09:03 bjorn

@bjorn please consider adding it even if optionally - I dont think it will make it that much bigger. The tiled parser on gdevelop is more prone to break because of all these manual expressions just to evaluate the tile's x,y on the tilemap and on the tileset.

Having it will allow me to remove these nested for loops and expressions - ultimately resulting in much cleaner code and less computation needed for gdevelop to do when parsing the data. I didn't realise what a huge difference this makes until writing a parser for ldtk.

You are sort of adding tons of complexity to the engine's parser by omitting that data from the export just to save a few bites. You are correct that it made more sense 17 years ago, but today even a simple smartphone with bad internet connection will have no problem with it

blurymind avatar Mar 26 '21 09:03 blurymind

@blurymind Please be careful about your wording and the way you address people on what is technical considerations. It is poor form to tag multiple times someone that has been making what is a great product for years just to hit on a way some data is formatted. I know it's not your intent, but this can really be lived as a micro agression for someone to be tagged on some other project just to read that the way your project formats data has some "crazy" and "damned" choices. These things can cause severe burn out, especially in the open-source world.

As with every product, technical decisions and choices, there are advantages and disadvantages. Not only 17 years ago it was a right decision, it was also a very clever one and probably helped a lot of games perform correctly and save memory.
It's certainly now less usual to store flags like this in bits of an integer, and it's certainly a good idea to suggest an alternative way that make the parsing less painful :)

But it's our decision and responsibility to own this parsing in GDevelop:

  • Either we decide we can support it, as we already do. It's a bit painful but ok once you understand bitwise operators in JS.
    • We can always add unit tests to make sure it's robust.
    • And we can open or vote on the Tiled GitHub issues to advocate for a format that would avoid this complexity.
  • Or we decide it's too complex and don't support Tiled format.
    • We can make the feedback on the Tiled GitHub issue of course.

But remember that you may not have the full picture behind a choice, the legacy/technical considerations at the time or now, or missed an actual advantage of a specific format. So let's refrain from tagging and being too harsh at people especially at contributors from other projects :) The proper etiquette would be to say in an issue "We've identified things are more painful than what they could, so I suggest this approach instead. It may have drawbacks (more bytes, redundancy of information, etc...), but also some advantages (easier parsing). It would be helpful for us (GDevelop) but also surely for other games".

Again I know the intent was not bad, but still, these things are important :) We're the one choosing to support Tiled format, so we must own it.

4ian avatar Mar 26 '21 11:03 4ian

ah sorry, there was no bad feelings,I was trying to say it jokingly but it came out bad. I shouldn't have tagged Bjorn here, just wanted to bring it up because some bugs cropping out can be fixed in theory in a better way if we can get the coordinates directly. My current tilemap bugfix pr for example can really work much better with that data available

Yes indeed tiled is a fantastic editor and it is still the most popular out there, just bringing this up with hope to put some light into how this data can improve its adoption even more.

I love the both tiled and ldtk and supporting both is important to me

blurymind avatar Mar 26 '21 11:03 blurymind

Just to be more specific,this PR here https://github.com/4ian/GDevelop/pull/2296

Its not fixing the problem completely and the tilemap still has corner cases where rendering has issues. If we can get the x,y values - this will let me fix it in a much better way with no corner case issues.

Just out of the box Ldtk will not have these problems with pixi-tilemap because of that. Complicated code leads to higher chance of corner case bugs and having these x,y values will lead to much cleaner and bug free parsing for pixi-tilemap. Tiled parsing can also benefit from this when x,y values are included in the json file

blurymind avatar Mar 26 '21 11:03 blurymind

Just out of the box Ldtk will not have these problems with pixi-tilemap because of that. Complicated code leads to higher chance of corner case bugs and having these x,y values will lead to much cleaner and bug free parsing for pixi-tilemap. Tiled parsing can also benefit from this when x,y values are included in the json file

If LDtk doesn't have the issue OOTB, and your PR fixes most use cases, along with the eventual goal of integrating LDtk as a native editor, I think the edge cases could be reasonable, to the point where we can just document it and treat it as a minor compatibility issue rather than having it continue to take more and more of your available hours trying to fix.

Obviously, I have no say in any of the above, it just seems like the best path forward as an outside observer.

Silver-Streak avatar Mar 26 '21 14:03 Silver-Streak

Some progress today - I got it to render all ldtk layers,apart of entities, which I am not sure if we will support.

One limitation of pixi-tilemap atm is the inability to render semi-transparent tiles. I might do a PR to pixi-tilemap to add that some time in the week, as the shadow layer of the ldtk example project looks really ugly without it.

There was a pr before, but it was abandoned before it could be merged https://github.com/pixijs/tilemap/pull/88

I will try to redo it and get it in before we upgrade pixi tilemap.Might as well address more of its limitations while at it. I believe tiled also has opacity on layers, so we do need it for both formats

blurymind avatar Mar 28 '21 18:03 blurymind

Ok so there is a huge refactor between pixi-tilemap for pixi v5 and v6 and in order to add opacity to tiles, gdevelop will need to be on pixi v6.

In essence, I cannot upgrade pixi-tilemap in gdevelop until gdevelop is not upgraded to pixi v6.

The commits to pixi-tilemap-v5 seem to have stopped for since almost half a year ago.

I need this to get in before I can work on improving pixi-tilemap to support opacity for opacity on layers in gdevelop https://github.com/4ian/GDevelop/pull/2447

I guess I can still work on it for v6,it just wont be in GD until we are on v6

blurymind avatar Mar 30 '21 11:03 blurymind

I submitted a PR to pixi-tilemap to enable us to have layer alpha for both tiled and ldtk format parsing in the future https://github.com/pixijs/tilemap/pull/115 113038446-73da4500-918e-11eb-9b15-d6e521634d2e

it adds an alpha option to rendered tiles

blurymind avatar Mar 30 '21 18:03 blurymind

Pr got in today, I will also submit a second PR to pixi-tilemap to add support for on a per tile animation speed

blurymind avatar Mar 31 '21 17:03 blurymind

Submitted a second PR to add to pixi-tilemap the ability to set independent animation speed for each tile - so we can improve compatibility with tiled and ldtk https://github.com/pixijs/tilemap/pull/116 Peek 2021-03-31 19-22

tiled does allow to set duration on a per frame basis, so we are still not there yet with it - but this gets us closer. We can now at least use the overall speed of animations that is set in a tiled animated tile

@deepnight can Ldtk do animated tiles yet? I couldn't find anything in the json spec, so I wont have to do anything for it in my parser for gdevelop for now

blurymind avatar Mar 31 '21 18:03 blurymind

@deepnight can Ldtk do animated tiles yet? I couldn't find anything in the json spec, so I wont have to do anything for it in my parser for gdevelop for now

Hi! No LDtk doesn't support animated tiles for now. But, this might be a long term feature though.

deepnight avatar Mar 31 '21 20:03 deepnight