Upgrade Shifty to 2.14.1

[jeremyckahn]

Hello! This PR upgrades Shifty to 2.14.0. This latest release should be fully backwards-compatible with the 2.8.0 it's replacing. It provides some significant performance improvements!

I'm not sure how to test this change within the app, so let me know if there's anything I should do before the PR can be merged. I just wanted to make sure that GDevelop has the best performance possible! 🙂

[4ian]

Thanks for the PR!

It provides some significant performance improvements!

Very nice! I'll take a look at what changed, I'm interested to see what you changed to get these perf improvements :)

I'm not sure how to test this change within the app, so let me know if there's anything I should do before the PR can be merged. I just wanted to make sure that GDevelop has the best performance possible! 🙂

If you want to try out:

• Follow the README (be sure to use Node.js 12 or 10, not 14, we have a dependency to update).

• Wait for the initial compilation to done.

• Run the electron app (in the readme too: https://github.com/4ian/GDevelop/blob/master/newIDE/README.md#development-of-the-standalone-app)

• When the app is launched, click "Create a new Project". Either choose the Platformer, which uses the "Tween" behavior for collected coins, or search in "examples" for "tween" to find 2 other examples.

• Press "Play":
  

• ...and there is an issue 😄

  

We were using the promise returned by the tweenable.
See this file:

GDevelop/Extensions/TweenBehavior/tweenruntimebehavior.js

Lines 181 to 197 in 275f699

	this._tweens[identifier].instance
	.tween()
	.catch(function() {
	// Do nothing if the Promise is rejected. Rejection is used
	// by Shifty.js to signal that the tween was not finished.
	// We catch it to avoid an uncaught promise error, and to
	// ensure that the content of the "then" is always applied:
	})
	.then(function() {
	that._removeObjectFromScene(identifier);
	})
	.catch(function() {
	// Do nothing if the Promise is rejected. Rejection is used
	// by Shifty.js to signal that the tween was not finished.
	// We catch it to avoid an uncaught promise error, and to
	// ensure that the content of the "then" is always applied:
	});

You can change this file and the change are imported in the editor. You can close the game preview and relaunch it with the play button to verify the fix/changes you made.

I think I can do the fixes that are needed but I'm a bit busy now, so will be tomorrow or later.

Thanks again for the PR! Looking forward to benefit from all these sweet performance improvements.

[jeremyckahn]

Whoops! Thank you for taking the time to make this excellent writeup. I see exactly what the problem is!

This can be easily fixed in GDevelop by changing the places where there is something like:

tween(...)
  .catch(...)
  .then(...)
  .catch(...);

To just:

tween(...)
  .then(...)
  .catch(...);

The reason we're seeing this is that Tweenable#tween no longer returns a Promise in 2.14.0. Instead, it now returns the Tweenable instance. I made this change so that a tween Promise would only be defined lazily for the sake of performance, as not every use case needs the Promise functionality. I thought I could preserve backwards compatibility by simply turning Tweenable into a thenable: https://github.com/jeremyckahn/shifty/blob/2161484574d6e1ecccc98dff3684ec48f2ef80f1/src/tweenable.js#L457-L474

However, I neglected to define Tweenable#catch, thus the null reference error you're seeing. 🤦‍♂️ I'll fix Shifty to handle that for full backwards compatibility and update this PR with an updated release. If you'd rather just make the fix I described above, that would work just as well. I'll get a start on the Shifty fix now, and I should hopefully have a 2.14.1 release by the end of this weekend. I'll also test this PR as you described to make sure I'm not missing anything else!

I'm interested to see what you changed to get these perf improvements

Tons of little things! I wouldn't say that 2.8.0 performed poorly by any measure, but GSAP has long been the best-performing JS animation engine available, and I wanted to meet that level of performance. Shifty 2.14.0 does exactly that and delivers superior memory utilization to boot! 😁

The code is a little hairy as a result, but something I've learned through Shifty is that fully-optimized code can get a little ugly. That degree of micro-optimization is rarely necessary... but an animation library is one of the exception cases, I think. :)

[jeremyckahn]

That was quicker than I thought! Shifty 2.14.1 has been published: https://github.com/jeremyckahn/shifty/releases/tag/v2.14.1

And this PR has been updated to use it! I also cleaned up the tween().catch calls as I described in my previous comment. I tested this against all of the suggested demos locally, and everything seems to be working great. You might want to give it a try yourself before merging, in any case.

Let me know if you need anything else here. It's been a while since I played with GDevelop, it's come really far. Super impressive work! 🙂

[4ian]

Thanks for the super quick turnaround! I've checked on all the examples using tweens and it seems to work well indeed! Thanks for updating Shifty and this PR 👍 Happy to see these extra catch gone :)

Let me know if you need anything else here. It's been a while since I played with GDevelop, it's come really far. Super impressive work! 🙂

Thanks 🙌 It's progressing well and getting more contributions so we're on a nice road :)

The code is a little hairy as a result, but something I've learned through Shifty is that fully-optimized code can get a little ugly. That degree of micro-optimization is rarely necessary... but an animation library is one of the exception cases, I think. :)

Yeah code is sometimes getting hard to read because of optimizations. I would love a way (linters, babel mode to compile to "garbage free" mode etc...) to enforce these micro optimizations - I've also done some of them (avoiding allocation/deallocation of objects as much as possible, etc...) in the GDevelop game engine, but it's hard to keep track of them, ensure they are used in PRs and see the impact they have.
Your benchmark against GSAP seems a great one, and it's good to see that Shifty is the smallest and fastest now - exactly what we love for GDevelop!

Anyway, seems that we're all clear to go ahead with this! 👍