tweenjs / es6-tween

ES6 version of tween.js
MIT License
186 stars 34 forks source link

Fix update loop #50

Closed moroine closed 6 years ago

moroine commented 6 years ago

Hi,

I figured out that the order of complete events wasn't properly repsected.

After dig in, I found that the update loop uses

while ( i < _tweens.length)  {
  _tweens[i++].update( time, preserve)
}

The issue is that _tweens[i++].update( time, preserve) can (actually will by default) remove the tween from our _tweens meaning we will skip the next index.

I've added a unit test :)

dalisoft commented 6 years ago

Hi. Thanks for PR. I never see problem of events, but may skipped to test some event. Are you sure, this not breaks delayed tweens? I now haven’t time to check, but i will merge it for you now, i hope this fix make happy other users too.

dalisoft commented 6 years ago

Please change var to const, as this is ES6 inplementation, if you haven’t time for now, use fix immediately from github repo, when i have time, i will test. Thank you anyway

moroine commented 6 years ago

Thanks,

I did #51 to fix this, and also could you release it over npm ? What about adding a travis integration to auto-publish versions ?

dalisoft commented 6 years ago

Thanks. — i will merge #51 — i will publish to npm when have time — about ravis and auto publish i will not like it, because i had been issues with that

moroine commented 6 years ago

Thanks,

about ravis and auto publish i will not like it, because i had been issues with that

Yeah I agree that this can goes wrong easily ^^

moroine commented 6 years ago

@dalisoft do you have time to publish on npm ?

dalisoft commented 6 years ago

Hi @moroine

I'm sorry for delayed publishing. I am working in full-time job that has a lot of task, so am just now PUBLISHED to npm as v5.2.4 that should work for you.

I think there very small amount of users that use es6-tween. Starts also only 75 (best for me anyway) and i am can't give to someone permission to continue because i have not such as permission.

Thank you for PR. Amazing catch...

moroine commented 6 years ago

Hi @dalisoft,

Thank you for publishing.

So do you think this repository will start dying ? You're the only contributor, maybe we could add more maintainer ? (I could be part of it, for basic maintenance)

So far there are 900+ dowload/week which is not that bad.

dalisoft commented 6 years ago

Hi @moroine. I am very want to add more maintainer, for adding maintainer, we need help of @sole. There 900+ download/week is good, you right, even this best for me. If you see commits history, when i am been free (not worked on full-time job) there have been minimum 2 commits per week, created many plugins, GUI and examples on Codepen, but now, i have not much of time to spent to this library, on my full-time job work hourly, so my open source project can't replace my hourly work cost, i am very sorry.

This library does more that what does the most popular libraries, even more features like "deep tween" that there NO ONE LIBRARIES can't do that while keeping performance stable. I worked on this library more than year, even converted to TypeScript and converted to ES6 again as this name sayed wrong (Typescript !== ES6).

This project still live and peoples using it on projects, but there i think if this project get minimum 1 star per day, but not :( Maybe it's because of my bad language and not worked well on this project (maybe?)

So, you can make PR and i merge always, to NPM can give to access if possible, because i have full control over npm package, not part of tweenjs organization.

But keep in mind, you should always try to add something new and maintain library alive, you can DO that and you have enough FREE time?