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

Tweenable cannot be seeked if not previously played #56

Closed SBoudrias closed 9 years ago

SBoudrias commented 9 years ago

The resume method is the only place this._timeoutHandler get setup.

This mean you cannot create a tween and seek it directly:

var tween = new Tweenable({ duration: 1000 });
tween.seek(50); // this throw because undefined is passed
                // to the scheduler (requestAnimationFrame)

seek should probably make sure the tween is configured before it tries playing it.

Also, is there a reason why seek() play a tween? For our use case, we just want to use shifty for coordinating animations based on user interaction.

jeremyckahn commented 9 years ago

Hi @SBoudrias, thanks for reporting this. Indeed, this seems like a bug. For what it's worth, Tweenable#seek was added for #46, and since I haven't had a need for it myself, I have not extensively tested it in a practical setting. That being said, I've put together a potential fix: https://github.com/jeremyckahn/shifty/commit/361fb2ff6cadd7c8940c97ff7038a76aa9132404. This is in the bugfix/call-setconfig-before-seeking branch, so please pull it down and test it out when you get a chance.

As for the code you provided, you will need to configure the tween before seeking. Given your code, Shifty does not know enough about the tween to properly seek within it. I suggest configuring the tween with the second constructor parameter, like this:

var tween = new Tweenable({}, { // Notice the empty Object as the first parameter
  from: { x: 0 }, 
  to: { x: 10 }, 
  duration: 100,
  step: function (state) {
    console.log(state);
  }
});

tween.seek(50);

You can test the above with this CodePen.

Admittedly this isn't the most elegant approach, but I'm hesitant to change the API in an incompatible way so as not to break the library for other users. Given that, how does the bugfix/call-setconfig-before-seeking branch paired with the suggested code change seem to you?

SBoudrias commented 9 years ago

Hey, so the proposed fix is correctly fixing the issue we had. And thanks for clarifying the Tweenable arguments order!

jeremyckahn commented 9 years ago

Awesome, thanks for letting me know. I will merge this into master soon.