Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A snippet of Tween.update never runs #562

Closed
jonobr1 opened this issue Sep 24, 2020 · 6 comments
Closed

A snippet of Tween.update never runs #562

jonobr1 opened this issue Sep 24, 2020 · 6 comments
Labels

Comments

@jonobr1
Copy link

jonobr1 commented Sep 24, 2020

This code (link) never runs because (I think) isPlaying is meant to be invoked as a function or accessed via its private reference via underscore.

// If the tween was already finished,
if (!this.isPlaying) {
  this.start(time)
}

Switching the condition to if (!this._isPlaying) { fixes that issue, but breaks the test Unremoved tweens which have been updated past their finish time may go backward in time. This is because how valuesStart is applied in the Tween.start method changes. I would've fixed this myself, but I'm not really sure what your intention is with how valuesStart should work, so I figured I'd bring it up for discussion here.

Edit: Perhaps another solution would be to just delete it?

@trusktr
Copy link
Member

trusktr commented Sep 24, 2020

I think that's a typo! this.isPlaying will always be truthy, but I don't think that is the intent.

There's a TypeScript config option to prevent that sort of thing actually, maybe we should turn that on.

Most importantly, the intent of the project is in the unit tests. So as long as the existing tests pass (or agreed modified tests pass) then all good to change anything.

We should add a unit test for update() being called without first calling start(), which is what that intended to cover.

@trusktr
Copy link
Member

trusktr commented Oct 20, 2020

Sorry, I misread your question last time, but valuesStart is supposed to catch the initial values in the object that the Tween is tweening. This happens in the call to _setupPrperties inside the start method. It will populate the _valuesStart object with the tweened object's initial values.

I suppose we need to write a test for making tweens without having called start.

In pre-18.6, calling update before calling start would result in the animation not working. In 18.6, that check was added which was supposed to start a tween (typos aside), making calling of start be optional; however a test was not added for this.

We can either:

  1. Make calling update without first calling start throw a helpful error to the console.
  2. Fix it so that it will automatically start the tween if start is not called, making start optional
  3. Delete those lines, and the old behavior of tweens silently not working will continue like in 18.5

In the case of 1 and 2, we would need to figure how to fix the Unremoved tweens which have been updated past their finish time may go backward in time test case.

@trusktr
Copy link
Member

trusktr commented Oct 20, 2020

This is related: #271

@trusktr
Copy link
Member

trusktr commented Oct 20, 2020

Fixed in #573 (a bit of a small hack, but it works)

@trusktr trusktr closed this as completed Oct 20, 2020
@jonobr1
Copy link
Author

jonobr1 commented Oct 21, 2020

Amazing, what I was missing was making the preserve boolean trickle down. Nice work and thank you!

@trusktr
Copy link
Member

trusktr commented Oct 22, 2020

No prob! I renamed the signature to Tween#update(time, autoStart) in a newer PR, so it makes more sense in the context of when looking at Tween#update(time, autoStart) without taking Group into consideration (f.e. tweens with no groups).

Released in 18.6.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants