jeremyckahn/shifty

getInterpolatedValues() does not take "delay" into account.

Opened this issue · 8 comments

It seems that getInterpolatedValues() method from shifty.interpolate.js module does not take "delay" factor into account, thus, we are actually left with a fake frame (ahead by "delay" milliseconds). Should we get delay into account?

Hi @adrianvoica, thanks for reporting. I don't think that a delay value should be taken into account for the shifty.interpolate.js extension. That extension provides a low-level utility that isn't really applicable to a normal time-based tween. For instance, interpolate is the heart of Rekapi, rather than Tweenable#tween.

However, I may be misunderstanding you. Do you mean to say that something isn't working as expected for you? The delay functionality was added very recently due to #65, and I didn't notice any broken tests or functionality after adding it, but it's possible that I may have broken shifty.interpolate.js in the process. Can you describe the use case for taking delay into account for interpolate calls?

Hi @jeremyckahn. Well, the lookup table optimization that I'm doing is based on interpolate.
For instance, I have a tweenable object with start value X = 10, end value X = 20, delay of 500ms and duration of 1000ms. I am expecting that after 500ms from start, X is 10, but without taking delay into account, I get a totally different value (X is 15), which is not quite correct. I hope this example is clear enough of a use case. Cheers.

Ok, I think I understand now. In that case, interpolate should account for the delay. Did you want to try to fix the bug, or should I take a stab at it?

I didn't try to fix it. At most, I would have adapted my code in the lookup optimization code, but the optimal place to fix it, I think, would be in the shifty.interpolate.js file. If you could fix it, it would be super great.

Ok. I'll try to fix it this weekend.

@adrianvoica, I'm having some trouble implementing this. I have some tests for how the API might work, but I can't seem to get them to pass. I'm missing something mathematically that's probably pretty simple, can you take a look at my delay-interpolate branch and see if you can get the tests to pass? Or even make sure that the tests are correct?

Yes, I think it can be wrapped in a new release. But on the other hand, I think it would be a good time to halt development of new features and focus completely on performance optimisations, even if code readability has to suffer a bit. I think there is huge room for improvement in speedups and efficiency, and Shifty being at the core of other libraries/tools, it should perform at its highest. I am committed to helping out in this regard (I am also directly interested in performance since I have some good stuff of my own in development based on Shifty core).

That seems like a good idea. Rather than package up a release to master, I just created a develop branch to hold stable commits that haven't been released (built and tagged). This is the workflow I use in my job, and it scales well for a team. I have merged the feature/delay-interpolate into develop (and subsequently deleted feature/delay-interpolate) so you can keep working on your performance improvements without worrying about master releases. From this point forward, please branch off of develop and make a Pull Request back into it. Does this approach work for you?

Full disclosure: I am not actively working on this project right now, as it meets the needs I built it for. However, I encourage you to continue on with your improvements to Shifty. I'll do my best to work with you to get your work integrated into the project and help out where I can, quickly and effectively.