-
Notifications
You must be signed in to change notification settings - Fork 906
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
Tiled pixi tilemap rendering #1901
Conversation
Hi |
Not a problem at all :) Currently the UMD module stuff breaks pixi-tilemap, which has nothing to do with pixi's version. Pixi-tilemap should work ok with the latest version |
@blurymind Is the UMD module still not working on pixi-tilemap (even outside GDevelop)? |
Yes, positive. I tested it on a minimal pixi project. I closed my pr in favour of the more complete one there. Progress is on hold until it works as UMD. I am trying to help the developer get it to work though and he's making progress. |
A small update on this - the UMD module at the PR I get this
OR
|
Ah great, at least a proper UMD module :) So it seems that there is something that is If we look at |
Ah wait, I think I celebrated too early - his PR has the umd module built with the umd name, but inside it, do you see any wrapping? Its not wrapping everything, just some functions. but still works fine in the minimal project outside of Gdevelop |
Weird, seems not like a real UMD module, it will work without issue in the browser, but I see no "require" at all that would make it compatible with "CommonJS" and so it's possibly not loaded properly in Electron. |
@4ian thank you, I opened a new ticket here |
Can you give me a tiled json and tilemap atlas image so I can quick check something? |
It's included in the PR :) the atlas is: the tiledMap json is Sorry, I should rename them for clarity. |
good catch.I should probably rename this property to get around the problem? :) I am thinking of expanding it to also be able to only render a specified group Maybe call it displayMode? |
I updated the name of the property now. I suspect that is due to the UMD module not working properly yet? |
I'm looking at |
It worked previously yes, but only in the ide. The runtime rendering is
missig atm.
My belief is that its either due to pixi5, the umd or something I did when
merging the new updates from gdevelop into the branch. I wonder if it will
work in the web app. I only tested with electron
…On Fri, 21 Aug 2020, 21:37 Florian Rival, ***@***.***> wrote:
I'm looking at Uncaught TypeError: Cannot read property 'transform' of
null but I'm not sure what's happening 🤔 It worked previously right?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1901 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABRRWVMXAO5IMGKRHW4KG43SB3LJPANCNFSM4PP3BKPQ>
.
|
Yeah not sure what broke but this is very strange... maybe we could try to make it work first in games in the browser, to unblock this? I'll be back to this to try to understand what is going wrong but it's quite time consuming so I have to alternate with other PR/issues :) I'll give this a new try though. |
I tried with the updated pixi-tilemap library, I get this:
|
Thanks for trying again. I've pushed to master this commit: 0ac2ef7 which adds support for @pixi/... requires. Can you try one more time with this? We should be "back to" the issue of the transform being null ( |
Actually no need to merge master, I've updated the file in this branch :) |
The pixi tilemap UMD file is still not working 😬😬 Because there is a footer that was added, I'm removing it now. |
Can you include the image file as well as the tilemap file? All the files that are intended to be used in GD |
Sure, here you go: I'm sorry files were in different directories so if you want to try to open it in Tiled you might have to change these in the JSON directly? |
Somehow I cannot select a tilemap in the scene editor, nor move it. I have the impression this bug was already there since I started working on this, but let me know if that's not the case and I broke something? |
I noticed that you can only box select tilemaps. My theory is that gdevelops ide requires us to set something on the pixi object to make it click selectable 🤔 |
Indeed the tilemap is not "interactive" by default as |
This is implemented, selection/move/resize in the scene editor works well. |
// Implement `containsPoint` so that we can set `interactive` to true and | ||
// the Tilemap will properly emit events when hovered/clicked. | ||
// By default, this is not implemented in pixi-tilemap. | ||
this._pixiObject.containsPoint = (position) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps @ivanpopelyshev might be interested in having it added to pixi-tilemap itself? Or I suppose there is a good reason for not having any extra stuff like this
@blurymind As this seems like it's getting really close to implementation, and finances are a bit more stable, I've bumped up the bounty on this again. Total is now at $175. |
@Silver-Streak I humbly thank you for the generous donations 🙇 |
@blurymind Thanks! For the documentation, remember to write it in the same style as other objects, for example: http://wiki.compilgames.net/doku.php/gdevelop5/objects/tiled_sprite Notably, it's important to remember that the audience is the user of GDevelop, so we avoid talking about technical details:
Also some nitpickings ;)
I'll proofread and edit this anyway before we distribute the object publicly, so if you're not sure go ahead and I'll make fixes :) Almost there! ;) |
hmm are you sure you commented in the right PR? :) |
@4ian why are there suddenly over 900 files in the dif? :) you must have pushed a big change. I will try to update the wiki in the weekend. You are right, that graphic is lazy |
@@ -38,6 +38,17 @@ void GD_CORE_API BuiltinExtensionsImplementer::ImplementsWindowExtension( | |||
true) | |||
.SetDefaultValue("yes"); | |||
|
|||
extension | |||
.AddCondition( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like some commit from another pr somehow bleeded over this one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's a GitHub issue, try to close this PR and open another one. That should do the trick. It's because master got merged in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just checking in on this, is the new PR going to be a big issue for progress, @blurymind ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally it's nothing at all, should just give more clarity before merging.
@blurymind Could you see if you can close this PR and open another one (don't do any change, just close the PR and open another one from We should also have explanations on the wiki about how to use Tiled - even if it's very succinct (I'm happy to proofread/rewrite part of the page). |
Ok I will close this one and open a new one. Sorry for my slow reactions atm. I was asked to self isolate (again) in a place with no internet connection so its a bit hard to work with my mobile broadband. Hopefully this will be over in a week or so. The airports are closed to the uk so it might become impossible to get back home in the next month, but at least I will get a normal internet connection again and be able pull gigabites of node modules to rebuild GDevelop and see whats going on with that file. In the time being it shouldnt be hard to at least update the documentation |
I was able to recreate the PR at #2147 |
This is a wip PR of the tilemap extension.
#503
Request - support for compiling as an UMD module + support for pixijs-legacy pixijs-userland/tilemap#92
write documentation (After merge)
Edit: the pixi5 regression has now been fixed and this PR is back on track!