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

Ldtk file support for tilemaps in Gdevelop #2828

Open
wants to merge 54 commits into
base: master
Choose a base branch
from

Conversation

blurymind
Copy link
Contributor

@blurymind blurymind commented Jul 21, 2021

This adds support for ldtk parsing in gdevelop.
imageimage

It also makes an attempt at loading resources from relative paths to the tilemap file (auto - loading textures from paths in the tilemap file) - this is required since ldtk can have a different atlas for each layer/map and can have many maps and atlases in a single file

Todo

  • add tests
  • add some examples
  • update to latest pixi-tilemap and implement layer opacity in parser
  • add rotated tiles support for ldtk (rework the existing one to work on both
  • add layer opacity for tiled files too
  • register pixi tilemap as a renderer plugin in the runtime + fix other regressions
  • remove all arrow functions from the helper module. Note from 4ian: no need for this
  • fix regressions from merging dev (changing map at runtime no longer works)
  • implement a better approach for handling loading of multiple resources found in the json file's relative path entries

closes #2434

@blurymind blurymind requested a review from 4ian as a code owner July 21, 2021 08:33
@blurymind blurymind changed the title Ldtk file support for tilemaps in Gdevelop Ldtk file support for tilemaps in Gdevelop Jul 21, 2021
@blurymind
Copy link
Contributor Author

blurymind commented Jul 21, 2021

It looks like I need to get to pixi renderer that gdevelop uses in order to register this now
pixi>core> Renderer.registerPlugin('tilemap', TileRenderer as any);
pixijs/tilemap#124 (comment)

@4ian what would be the correct way to do that in gdevelop? Should this be done from the extension files?

@blurymind
Copy link
Contributor Author

blurymind commented Jul 21, 2021

I think I can use this to get to the renderer in the runtime.Now how to do it in the ide

      const pixiRenderer = runtimeScene
        .getGame()
        .getRenderer()
        .getPIXIRenderer();

might have to add a getter

@blurymind
Copy link
Contributor Author

blurymind commented Jul 21, 2021

InstancesEditor has this line

    this.pixiRenderer = PIXI.autoDetectRenderer(
      {
        width: this.props.width,
        height: this.props.height,
        preserveDrawingBuffer: true,
        antialias: false,
      }
      // Disable anti-aliasing(default) to avoid rendering issue (1px width line of extra pixels) when rendering pixel perfect tiled sprites.
    );

I need to somehow pass this.pixiRenderer to the extension so I can do what was suggested by pixi-tilemap devs
renderer.plugins.tilemap = new TileRenderer(renderer);

EDIT: I think I can do it now via
InstancesEditor>LayerRenderer>ObjectsRenderingService

@blurymind
Copy link
Contributor Author

blurymind commented Jul 21, 2021

I managed to pass it through and register this as a pixi plugin. Now ther is another issue however
image

PIXI.utils method is missing? I do seem to have it

EDIT: Ah I got it, there is a bug in pixi-tilemap.its in utils.utils!

@Silver-Streak
Copy link
Collaborator

@blurymind question on this:

Would it be possible to have it surface int grid and entity data in the parser, even if it's not utilized in the engine currently?

My thought is that would allow a separate PR down the road that anyone could take up that could allow for people to build logic based off Int Grid and Entity stuff from LDTK files. (I think?)

Or would passing data that's not used break stuff?

@blurymind
Copy link
Contributor Author

@blurymind question on this:

Would it be possible to have it surface int grid and entity data in the parser, even if it's not utilized in the engine currently?

My thought is that would allow a separate PR down the road that anyone could take up that could allow for people to build logic based off Int Grid and Entity stuff from LDTK files. (I think?)

Or would passing data that's not used break stuff?

This pr adds support for both autolayers and intGrid already!
https://github.com/4ian/GDevelop/pull/2828/files#diff-230804730aae7eef4ce89228e9120570e8e383b295c728892644045c056baa23R458

Entities however are for another pr as they require some careful planning.

I can add an expression to gdevelop, which lets you get data from ldtk maps in the event sheet. From there on you can use it to set things. That would be potentially simple to do, but it will leave you with the task of manually setting things up with the event sheet

@blurymind
Copy link
Contributor Author

@4ian for some reason I now get this crash when clicking on an instance, I wonder if its missing something on the c++ engine still
image

The tilemap properties are still accessible if you select it from the object

@4ian
Copy link
Owner

4ian commented Jul 22, 2021

@blurymind This means you have an outdated libGD.js, can you try to npm install and npm start again in newIDE/app?
If you have compiled the C++ sources (in GDevelop.js) yourself, you need to recompile - otherwise it will download the latest version online when you run npm start :)

@blurymind
Copy link
Contributor Author

@blurymind This means you have an outdated libGD.js, can you try to npm install and npm start again in newIDE/app?
If you have compiled the C++ sources (in GDevelop.js) yourself, you need to recompile - otherwise it will download the latest version online when you run npm start :)

thanks, that sorted it :) I pasted the code twice from my other repository by mistake which was causing this issue. Fixed now

@blurymind
Copy link
Contributor Author

blurymind commented Jul 22, 2021

@4ian I added support for flipped ldtk tiles.
image

Note that we can now do this to flip them (rotate option,corresponds to pixi's rotate):

 const pixiTilemapFrame = pixiTileMap.tile(
                tileTexture,
                xPos,
                yPos,
                { alpha: layer.opacity, rotate }
              );

so there is no need for caching and complicated code. Should I remove that from how tiled does it? I am having trouble understanding how exactly you are getting the rotation for it.

EDIT: I will try today to remove the need to cache flipped tiles when parsing tiled.

@blurymind
Copy link
Contributor Author

blurymind commented Nov 9, 2021

@4ian I resolved conflicts on this after merging dev and now it is crashing
image
Not sure why, seems unrelated to this pr

Could it be a case of having to recompile gd?

@Bouh
Copy link
Collaborator

Bouh commented Nov 9, 2021

Could it be a case of having to recompile gd?

Yes, a recompile is required, there were a few changes on the core and code generation.

@Silver-Streak
Copy link
Collaborator

@blurymind Any success on the recompile?

@blurymind
Copy link
Contributor Author

blurymind commented Nov 21, 2021

@Silver-Streak unfortunately it wont let me compile it
image

what is emconfigure?

@Bouh any ideas?

Edit: I think I managed to recompile it now - it required updating emsdk too, switching it to its master branch

@blurymind
Copy link
Contributor Author

@Silver-Streak I am back on track now, will address the review notes this week
image

@Bouh thanks for the assistance

@blurymind
Copy link
Contributor Author

blurymind commented Nov 28, 2021

@4ian I have now applied all review notes (apart of the resource dependency one, since Im not sure how else we will solve that without implementing something on the c++ side)

After merging dev I am now experiencing a new problem - incrementing the level index during runtime gives me some sort of a gdjs error
image

"TypeError: gdjs.New_32sceneCode.GDNewObjectObjects1[i].getLevelndex is not a function
    at Object.gdjs.New_32sceneCode.eventsList0 (file:///tmp/GDTMP-1000/preview/code0.js:45:107)
    at u.gdjs.New_32sceneCode.func [as _eventsFunction] (file:///tmp/GDTMP-1000/preview/code0.js:60:22)
    at u.renderAndStep (file:///tmp/GDTMP-1000/preview/runtimescene.js:1:6149)
    at a.step (file:///tmp/GDTMP-1000/preview/scenestack.js:1:409)
    at file:///tmp/GDTMP-1000/preview/runtimegame.js:1:5665
    at i (file:///tmp/GDTMP-1000/preview/pixi-renderers/runtimegame-pixi-renderer.js:1:7000)"

"gdjs.New_32sceneCode.GDNewObjectObjects1[i].getLevelndex is not a function"

perhaps I broke it in some commit when resolving conflicts?

@Silver-Streak
Copy link
Collaborator

@blurymind Based off the other thread, it sounds like Davy believes his change is no longer blocking this. Is that accurate (and if so are you running into any other blockers), or is it still impactful?

@Silver-Streak
Copy link
Collaborator

(ignore me hitting close with comment instead of comment. Apologies)

@blurymind
Copy link
Contributor Author

blurymind commented Jan 1, 2022

Happy new year guys 🥳

@Silver-Streak @4ian as mentioned merging dev into it and converting it to the new api broke a feature - still need to fix.

Other than that I am not sure how to proceed with it without using the current approach to load dependent resources, which is "hacky". I need some input from @4ian about what to do on that front to get it moving to a realistic resolution.

In terms of #2775 I dont mind it either way. If he merges his first, I am gonna have to refactor this entire pr - might be easier to close it and create a new pr or something. But I will do it depending on what @4ian thinks is the better way of moving forward :)

Btw I updated it to include the two new todo entries

@Silver-Streak
Copy link
Collaborator

Silver-Streak commented Jan 23, 2022

Just checking in on this to tag @4ian as I think blurymind's question on resource dependancies could impact other things later like music editors or timeline animators (both of which I know are longrunning asks on the trello), so getting a "best practice" answer here can help set those up for success whenever/if they are ever implemented.

@Silver-Streak
Copy link
Collaborator

Silver-Streak commented Apr 6, 2022

It's been a few months since the last update on this.

@4ian I know you had some open questions on how to solve these items, with the now-larger team, has there been any thought on how to proceed with the open items?

There's been continual rumblings on the discord about people being unhappy they have to use tiled or go from LDTK > Tiled >GDevelop, and while the big thing people want is a built in tilemap editor (which, hopefully, we can get to including LDTK after this is done), getting this file support would go a long way.

Any updates are appreciated.

@blurymind
Copy link
Contributor Author

blurymind commented Apr 7, 2022

I could redo the pr, but without any support for resource dependencies - this means the official ldtk examples will not work though - since they depend on loading resources from relative paths found in the ldtk file itself (with more than one image files for atlases). Making the user do that will also make it a pain in the neck to use ldtk data in gdevelop - kind of defeating one of its perks :(

So was hoping to get support for resource dependency that is not a hack (like the one here)

@Silver-Streak
Copy link
Collaborator

@blurymind the time has arrived and the automatic collision import for tilemaps pr has completed.

Were you still wanting to proceed with this bounty and take another look at this to see if you need to refactor?

@Silver-Streak
Copy link
Collaborator

Hey @blurymind, just wanting to check back in on this. I know @D8H is working on updating Pixi-tilemap to 3.2.2 currently, but wanted to see if you were wanting to pick this bounty back up afterwards?

I wanted to be sure to better understand if I need to start promoting the bounty as open again or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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