GDevelop icon indicating copy to clipboard operation
GDevelop copied to clipboard

Use custom PIXI bundle

Open arthuro555 opened this issue 1 year ago • 18 comments

  • Gets rid of the GlobalPIXIModule hack
  • Adds a script to compile and generate type definitions for an ESM module as a typescript namespace easily
  • Use a custom pixi bundle, which allows to remove what we do not use, and thereby gain performance and reduce bundle size:
    • Interactions plugin
    • Ticker plugin
    • Spritesheet support
    • PIXI.Loader plugin
    • Support for loading obscure file formats like dds and ktx
    • Etc... (Note that we can add them back as simply as adding an export from their @pixi/modulename npm package in pixi.ts)

I roughly tested it (opened a few projects including the layer effects example) and everything seems to work properly, but additional testing would be welcome.

We could expend on this PR in the future by putting stuff like @pixi/text, @pixi/bitmap-text , ... as their own bundles in their respective extensions, to basically remove PIXI capabilities that are unused in the current project. This would require a change to the type generator script to merge definitions in the same namespace across files.

arthuro555 avatar Jul 11 '22 08:07 arthuro555

This seems very promising. I had a lot of issues with PIXI imports when trying to add types definition for Pixi extensions (pixi-tilemap) and this will probably avoid some not great workarounds I had to do.

D8H avatar Jul 11 '22 09:07 D8H

This is ready for review btw

arthuro555 avatar Jul 12 '22 21:07 arthuro555

@D8H I went ahead and updated the PR to also bundle your TileMapHelper library using the same bundling and type generation script as for the PIXI bundle, and I added pixi-tilemap to the PIXI bundle. This allows to move it back into the extension directory, and get rid of all the extra stuff on top of it (its own rollup config, its own typescript config, its own karma config...). By doing so, this made a few typescript errors popup because gdevelops's tsconfig is set up in a stricter way than the one you had. I slapped some ts-ignores and a TODO comment on them for the moment, but you might want to take a look at those.


Also, I noticed this PR fixes all the TS errors when trying to generate declarations files for all of GDJS 👀 That's pretty cool, this may allow to not have to keep a copy of the source files for autocompletions in the IDE.

arthuro555 avatar Jul 19 '22 22:07 arthuro555

I've not reviewed anything but huge fan of this allowing to move away all the rollup configs and get back the custom lib into the Extensions folder. Looks more flexible and better than having generated .d.ts files!

4ian avatar Jul 23 '22 07:07 4ian

I've fixed all of the review comments so far, this is ready for another round of reviews!

arthuro555 avatar Jul 26 '22 03:07 arthuro555

I did a quick test:

  • BBText needs to have pixi.js-legacy replaced by pixi.js in Extensions/BBText/pixi-multistyle-text/dist/pixi-multistyle-text.umd.js
  • Strangely I cannot select anything in the scene editor in Electron 🤷‍♂️ The scene editor is working well on the web-app, but not in the electron app. Not sure if I missed something, I did a npm install, relaunched npm start, relaunched Electron a few times.
  • Crash in Bounce And Hook and in the particles effect demo: Uncaught TypeError: PIXI.particles.Emitter is not a constructor

4ian avatar Jul 26 '22 19:07 4ian

Let me know if you can reproduce the scene editor issue, and we can dig from there :)

4ian avatar Jul 26 '22 19:07 4ian

I can indeed reproduce the scene editor issue 🤔

arthuro555 avatar Jul 31 '22 04:07 arthuro555

Any idea what could be going wrong with the scene editor? A change in the PIXI version maybe?

4ian avatar Aug 03 '22 21:08 4ian

I have no clue :p

The PIXI version should be unchanged..? https://github.com/4ian/GDevelop/pull/4090/files#diff-b720ce4f7ad00d65536495ab15c15ff6fd16ffdcde8fc68e3ad017983e460cf1R61

arthuro555 avatar Aug 04 '22 06:08 arthuro555

I really want this to get merged, I'll take a new look but would be interesting to see what is missing (is this related to the interaction manager or something like this that would not have been bundled? Or is this totally unrelated?)

4ian avatar Aug 12 '22 08:08 4ian

I did indeed not bundle the interactions manager (i used the same bundle as for GDJS). Is it used in the IDE? It'd be weird that it works on the web version of the ide relied on it as I see no reason the interactions manager would be present there 🤔

arthuro555 avatar Aug 12 '22 08:08 arthuro555

It's indeed very much used, it's what is used to check if an instance is hovered/clicked/moved/panned. Weird that it would work on the web version (could that be a cache of an older version somehow?).

Let's add it back and see how it works?

4ian avatar Aug 12 '22 14:08 4ian

Let's add it back and see how it works?

I did and it still did not work. I've continues to mess around for a while, even copied the official pixi legacy bundle source and it still did not work. I also noticied other weird errors, like WebGL not working anymore, only canvas. So I ended up just reexporting pixi.js-legacy with the additional modules we are using. The only logical explanation I have is that CRA has some non-standard behavior that somehow must compile the PIXI ES Modules differently than intended leading to malfunctions. I've read a lot of blog posts where people were complaining about CRA having weird behavior especially when it comes to importing modules, leading to those kind of issues. So much for diminishing the webapp's bundle size :p


I have no clue why, I have changed nothing to my environment but now my types generation script is failing and typescript spits out a whole new bunch of issues it didn't just before. I am so tired of working with "unconventional typescript", it's so janky 😩

arthuro555 avatar Aug 12 '22 21:08 arthuro555

I have no clue why, I have changed nothing to my environment but now my types generation script is failing and typescript spits out a whole new bunch of issues it didn't just before. I am so tired of working with "unconventional typescript", it's so janky 😩

Let's have a step back :) Nothing will ever be "conventional". Tooling is indeed a tiresome job, but that's why we must go for what is simple and stupid.

I've not followed everything so it's a bit hard to help you currently. Could you do a recap of:

  • How we use PIXI in the runtime?
  • How we use PIXI in the editor?
  • How do we handle extensions like the multistyle text and particles?
  • What does generate-esm-types does?

I think that by writing this down this will help getting back to a good state :)

4ian avatar Aug 14 '22 15:08 4ian

I think I figured it out, it was some kind of weird typescript module resolution issue with the fact that we use pixi.js-legacy and not pixi.js, while other modules import as peer dependency pixi.js... It was all fixed by installing pixi.js alongside pixi.js-legacy.

arthuro555 avatar Aug 29 '22 21:08 arthuro555

Interesting. Do you think there is still a risk of conflict between the two pixi packages? Like, do we risk having two concurrent PixiJS running at the same time? Or was the issue actually that we had two versions before your fix, so two concurrent versions were "fighting" at runtime?

4ian avatar Aug 29 '22 21:08 4ian

The issue was not runtime related, it was just typescript being weird. The code itself never broke or stopped working. There should be no risk of duplicated PIXI versions - pixi.js and pixi-legacy.js themselves are only wrapper modules that reexport from the real PIXI modules from other NPM packages. pixi legacy also reexports pixi.js so we are sure it will not generate two copies of PIXI in the bundle. We keep them because:

  1. Some external plugins import those as a peer-dependency
  2. They install for us all of the npm packages in the base bundles for us to reexport, without needing to install each one by one

Now the CI is failing because pixi-multistyle-text has not been updated in 3 years and still has PIXI v5 as peer dependency, which makes npm v7 upset 😩 I had made an npmrc to tell it to use legacy peer deps (ignore conflicts in version between the peer dependency and the installed version), but it does not seem to have worked 😕

arthuro555 avatar Aug 29 '22 22:08 arthuro555

There goes my entire last summer (project) 🙃

I'm closing this PR. I believe it is too hacky after all, and comes with a lot of issues that could be solved by just moving the whole runtime to ES Modules. The conflicts have also gotten too numerous/out of hand.

Truth be told, I think ES Modules are also necessary to making a headless version of the GDevelop runtime for node & workers, which I realized must have first-class in-engine support for THNK to be stable and production-ready. So I'll be trying to do that.

My plan (PR per PR):

  1. Make a codemod to make the Runtime ES Modules. Dependencies (e.g. pixi.js) will be pre-bundled into a single module file with es-build and made accessible to other modules through an importmap. Runtime modules are only transpiled, but not pre-bundled.

    • At this point, the runtime is effectively the same, albeit running using es modules under the hood
  2. Add bundling to the exports. Simply use the wasm version of esbuild to bundle a GDevelop export, minifying and tree-shaking the output, for a more optimized game.

    • At this point, we get the first user-noticeable change from the switch to es modules
    • This could be skipped if the GDevelop company wishes to provide this as an online build service instead of being available in-app
  3. Rework the Runtime to make the renderer totally optional. Tasks would include:

    • Make GDevelop disable features like culling and pre/post render callbacks when there is no rendering going on
    • Make sure there is no dependency to PIXI in non-pixi renderer code
    • Make sure all objects store their properties on themselves and not on the renderer object (Looking at you, PanelSpriteRuntimeObject#get/setColor 👀)
    • Make the GDevelop IDE read the image sizes in advance and store it in project data for the default sprite size instead of reading it from the renderer at runtime (lack of renderer would break AABBs otherwise)
    • Clearly separate renderers from the Runtime engine (put them in separate folders, separate node_modules)
    • At this point, users would not notice anything, but this refactor would allow great things in the future, including the next two PRs
  4. Add (manual & headless) multi-threading support to GDevelop:

    • Add a worker entrypoint to the runtime that runs GDevelop in headless mode
    • Add an action to start a scene in another thread (without rendering)
    • Add a condition "Is running in headless mode"
    • Add conditions & actions for messaging between the scenes that run in another thread and the main thread
    • At this point, THNK can realistically run the server code separately from the client code & in parallel, allowing for both speed and isolation. Users can also run more complex calculations or a part of the game logic on another thread if they wish to. It adds a lot of potential of optimization and would be a killer feature compared to construct. Heck, Ashley made his construct RTS game with JavaScript for the sake of multi-threading!
  5. Add a nodejs export to GDevelop:

    • Add a nodejs entrypoint to the runtime that runs GDevelop in headless mode
    • Add an exporter that generates a package.json & a build that uses the node entrypoint
    • Add a way to select another default scene for the node export
    • (Stretch goal) Add a raylib (or another simple native rendering library) renderer to allow doing a basic render of the scene running on the node export. Purposes would include making a server management interface & debugging the server export among other things.
    • At this point, users would be able to export game servers directly for them to run or distribute to their players. GDevelop would become fully "multiplayer ready" at that point.

That's a lot of work though... Although I am unsure what to ask help for here, I don't have the impression it is a task I can fully tackle all by myself.

Either way, for now, goodbye #4090! I need to get to work on that codemod 💪

arthuro555 avatar Feb 24 '23 21:02 arthuro555

No major concerns, this sounds like a proper plan. The first part is almost certainly a codemod indeed. This allows to regularly re-run it until it's entirely working and ready. Manual operations should be almost zero. Once it's ready, we run it and merge it.

Then for the rest I'm happy to help. Making the renderers decoupled is a matter of ensuring the few parts which are stil coupled are changed. This is the direction I want to follow and happy to help.

Ideally, a multiplayer server would run the headless version and this could either be exported manually by a savvy user or automatically exported to a full automated multiplayer service!

4ian avatar Feb 26 '23 16:02 4ian