jeremyckahn / shifty

The fastest TypeScript animation engine on the web
https://jeremyckahn.github.io/shifty/doc/
MIT License
1.54k stars 88 forks source link

Browser is currently crashing the tab if I call .tween on a Tweenable instance while it's currently tweening the previous call #117

Closed kylewetton closed 4 years ago

kylewetton commented 4 years ago

I'm currently using "shifty": "^2.9.1",

I'm animating several axis of a 3D model, tween() by itself works great in that I can throw multiple tweens at it at once for difference axis and they all happily animate at the same time.

The downside of this however is I don't have a way of checking isPlaying.

Imagine this use case.

An function calls tween() that slowly rotates the model by 180 degrees over 5 seconds During that event, another one fires that rotates the model by 90 degrees in the opposite direction over 1 second.

This second tween() overrides the first one and it correctly rotates for that 1 second. Once it's down, the previous tween continues where it left off.

What I'd like to do is .stop() a tween. So, I'm now reaching for a Tweenable so I have more control over it.

The tricky thing is, you can't just call tween on a Tweenable class if its already playing, this instantly crashes the browser.

So am I looking for a Scene at this point? I haven't looked closely at it, but am I right in saying that a Scene needs a predefined amount of Tweenables defined on load, or can you dynamically add and remove them?

Awesome package by the way, I love its ease of use.

jeremyckahn commented 4 years ago

Hi @kylewetton, thanks for using Shifty and opening this issue! The detail you've provided is very helpful. There's a few different things to unpack here, so I'll address each one.

I'm animating several axis of a 3D model, tween() by itself works great in that I can throw multiple tweens at it at once for difference axis and they all happily animate at the same time.

The downside of this however is I don't have a way of checking isPlaying.

Typically this is true, but I built in a small escape hatch to get access to the underlying Tweenable instance if it's needed: https://github.com/jeremyckahn/shifty/blob/b85b9913e4953ad0537d02615b80811d82086ef8/src/tweenable.js#L575-L581

So if you'd like, you should be able to do something like this:

const promise = tween({ ... })
const { tweenable } = promise

My bad for not documenting that. I'll add a note to the docs for it!

The tricky thing is, you can't just call tween on a Tweenable class if its already playing, this instantly crashes the browser.

This is unexpected. Could you clarify what you mean by this? If it's completely locking up the browser such that you need to reload the page, then it sounds like there's an infinite loop issue. Shifty should never cause such a crash, and if it is I will prioritize getting that fixed. The expected behavior is that an instance's previous tween will cause the previous tween to throw an error. I could change that failure state to be something a little more helpful (like gracefully cancelling the previous tween and simply starting the new one), but I'd like to narrow down whether it's a poor design choice or an implementation bug. Do you have some time to perhaps put together a code sample to reproduce the issue?

So am I looking for a Scene at this point? I haven't looked closely at it, but am I right in saying that a Scene needs a predefined amount of Tweenables defined on load, or can you dynamically add and remove them?

I think a Scene should work for you here! A Scene is primarily a mechanism for controlling Tweenable instances in bulk. You can freely modify the contents of a Scene at any time. Scenes are pretty lightweight overall.

Awesome package by the way, I love its ease of use.

Thanks! šŸ˜

jeremyckahn commented 4 years ago

@kylewetton Update: I've reproduced the freeze and I'm working on a fix!

jeremyckahn commented 4 years ago

This issue is fixed in the new 2.9.2 release!

Here's a CodePen that works under 2.9.2 but was broken under 2.9.1: https://codepen.io/jeremyckahn/pen/GRZgqXr

Thank you again @kylewetton for reporting this issue and helping me get to the bottom of it with your excellent writeup!

kylewetton commented 4 years ago

Legend, thanks so much dude

kylewetton commented 4 years ago

By the way, here's the WIP app I'm building with Shifty as the basis for all animations

https://deviceful.app/

jeremyckahn commented 4 years ago

@kylewetton this app is so cool! I can see it being really useful for product and graphic designers. I'm glad to know that Shifty is working well for you here, and I can't wait to see where you take this tool! šŸ˜ƒ