4ian / GDevelop

๐ŸŽฎ Open-source, cross-platform 2D/3D/multiplayer game engine designed for everyone.
https://gdevelop.io
Other
11.73k stars 886 forks source link

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

Closed Silver-Streak closed 1 year ago

Silver-Streak commented 3 years ago

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 :

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

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

4ian commented 3 years ago

Solution suggested

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

blurymind commented 3 years ago

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 .

Silver-Streak commented 3 years ago

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.

4ian commented 3 years ago

Feel free to go ahead!

4ian commented 3 years ago

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)

blurymind commented 3 years ago

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

4ian commented 3 years ago

@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" :)

blurymind commented 3 years ago

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

4ian commented 3 years ago

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 :)

Bouh commented 3 years ago

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.

blurymind commented 3 years ago

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 commented 3 years ago

Some progress report today:

4ian commented 3 years ago

Nice! :)

blurymind commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.

Silver-Streak commented 3 years ago

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.

blurymind commented 3 years ago

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

bjorn commented 3 years ago

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?

blurymind commented 3 years ago

@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

4ian commented 3 years ago

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

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.

blurymind commented 3 years ago

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 commented 3 years ago

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

Silver-Streak commented 3 years ago

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.

blurymind commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

deepnight commented 3 years ago

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

4ian commented 3 years ago

@blurymind Great job for the merged and the ongoing PR on pixi-tilemap :) Seems like this is reaching an interesting state!

Silver-Streak commented 3 years ago

@blurymind just checking in on this. Any roadblocks/hurdles on this one?

blurymind commented 3 years ago

Not atm, needed gd upgraded to pixi6 to continue and it looks like that's done now so I'm back on track. Added some features to pixi-tilemap in the mean time

On Wed, 21 Apr 2021, 00:26 Silver-Streak, @.***> wrote:

@blurymind https://github.com/blurymind just checking in on this. Any roadblocks/hurdles on this one?

โ€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/4ian/GDevelop/issues/2434#issuecomment-823664341, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRRWVJX4QK4JUMP6BNK3WDTJYERTANCNFSM4ZF7OIMA .

Silver-Streak commented 3 years ago

Just doing a status check on this one, any new hurdles @blurymind

blurymind commented 3 years ago

To be honest, after upgarding it to pixi-tilemap for pixi6, it just stopped working. The component seems to be mounting ok, but its failing to initiate

I am a bit stuck yeah. Trying to figure out how to fix it

blurymind commented 3 years ago

Its looking for stuff in renderer renderer.plugins.tilemap.tileAnim[0] and the stuff is not there

blurymind commented 3 years ago

Looks like its expecting to find and use a tilemap plugin, which does not exist image

possibly tilemap is not registering itself as a plugin, which makes it fail.

Also something is preventing me from being able to commit an updated dist/pixi-tilemap to the branch. I am wondering if something was added to gitignore somewhere

Silver-Streak commented 3 years ago

Hmm... so is this a question for 4ian or the Pixi Tilemap maintainers?

Silver-Streak commented 3 years ago

@blurymind

A: I had enough extra funds to bump up the bounty on this a bit, it is now at $175. Any word on if there's someone I can poke to help with the issue you were running into? B: Congrats on your own tilemap editor launch!

blurymind commented 3 years ago

@Silver-Streak thank you :) sorry for getting a bit sidetracked. I will try to get some work on this done soon.

About the tilemap editor, for anyone interested its here https://github.com/blurymind/tilemap-editor https://blurymind.github.io/tilemap-editor/

I was playing with kaboomjs and really enjoyed how they use ascii to make tilemaps, then I thought maybe i can make my own kaboomjs editor that i can use on my phone. Then I realized that not a single one of these tilemap editors out there can really run on a smartphone and they are all huge. So thats kind of how I hacked together a quick and dirty one - the code is messy atm, but it is tiny and does work on a smartphone. Its really basic and has none of the amazing features of ldtk or tiled.

There is something I did discover the other day that might be of interest for gdevelop, since it uses react and pixi https://www.npmjs.com/package/react-pixi-tilemap not sure if it could be useful for us, since gd already has a lot of custom code, but it might help with moving closer to an editor inside gdevelop, which uses react and pixi already. (does it support rendering objects, opt visible layers, etc?).

Maybe if I manage to get my tilemap editor to do i/o of tiled data, gdevelop could use it as an external editor or wrapped by a react component too - it depends on what you, @4ian and others here like as an idea :) The nice thing for me in that is that we wont have to re-implement a tilemap editor in react

Silver-Streak commented 3 years ago

The more I proceed further in my game, the more important this becomes because of how much it speeds up the development process compared to Tiled.

I've bumped up the bounty a bit more (probably as much as I can at this point) and it is now at $200. Incase @blurymind has availability to continue. ๐Ÿ˜„

deepnight commented 3 years ago

Hi, is it possible for me to also contribute to the bounty? ๐Ÿค”

4ian commented 3 years ago

You can participate there: https://www.bountysource.com/issues/97132654-175-bounty-add-support-for-tilemaps-made-in-ldtk-build-a-parser-for-ldtk-files

Silver-Streak commented 3 years ago

@deepnight 4ian posted the link above if you want to contribute to the bounty fund itself, but if you meant you'd like to contirbute code to the ask, I'm pretty sure @blurymind wouldn't mind, either. ๐Ÿ˜†

deepnight commented 3 years ago

Unfortunately, I'm running out of time between LDtk development, game projects & open source game stuff ๐Ÿ˜… But I can definitely contribute to LDtk support in other ways! So I bumped up the bounty up to 500 USD.

4ian commented 3 years ago

Wow ๐Ÿ˜„ Thanks @deepnight - was not expecting this amount! Thanks for the work on LDtk and for this contribution to the bounty.

Would be really awesome to have direct support for LDtk maps :) Note for other potential interesting contributors to this issue: I'm happy to make a separate bounty so that the whole bounty is split in 2 (or 3) according to the number of people helping this to become a reality.

kmausolf commented 3 years ago

A great first step toward completing this one would be to first unit test the Tiled parser. That way, it'll be easier to verify the correctness of (and document) the LDtk parser.

blurymind commented 3 years ago

@deepnight wow, thank you for bumping this. Also thanks to @Silver-Streak for being patient with me. I started this and got a bit discouraged for a while to work on it because the pixi upgrade broke it.

But now with such a huge bounty, I am more than determined to give it my full effort to get this finished and put up a pr.

Its worth noting that gdevelop is currently under a lot of refactoring, which makes it a bit of a moving target. My branch is behind dev again and will need upgrading, but it is close as it can already parse the file.

The pixi upgrade regression needs fixing as well as adding layer opacity to the extension itself and this can be ready for a pr. I also have no idea how to write these unit tests for gdevelop extensions, so an example would be very helpful

blurymind commented 3 years ago

ok so I have updated my branch and am now getting pixi-tilemap's latest version to work on it as well as tidying up the code.

@4ian do we have any other js extensions with unit tests I can use as example?

I have filed a bug report here: https://github.com/pixijs/tilemap/issues/124

when this is resolved, we will be able to get the tilemap layer opacity I added to pixi-tilemap to work with the extension

blurymind commented 3 years ago

@4ian I can continue to get this to PR without the layer opacity feature and later on add the layer opacity. This is blocking it for no reason imo