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

Bug: V3.1.2 - Tween.onStart fires immediately - as opposed to waiting for tweens delay. #3330

Closed
wtravO opened this issue Mar 5, 2018 · 6 comments

Comments

@wtravO
Copy link
Contributor

wtravO commented Mar 5, 2018

This Issue is about

  • A bug in the API V3.1.2

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@photonstorm
Copy link
Collaborator

In the following how long should it wait before firing onStart?

    this.tweens.add({

        targets: image,

        x: { value: 700, duration: 4000, ease: 'Power2', delay: 500 },

        y: { value: 400, duration: 1500, ease: 'Bounce.easeOut' },
        
        scaleX: { value: 1.5, duration: 2000, delay: 2000, yoyo: true },

        scaleY: { value: 4, duration: 2000, delay: function (i, total, target) { return 1000 + Math.random() * 2000 }, yoyo: true },

        delay: 1000

    });

The tween has started when it's not in a paused state as the timeline is active (progress is elapsing).

The alternative is firing an onStart for every property, as they can each start at different times, or possibly just the very first one to fire (but then you could argue why did it pick that one as the progress was already running, why not the last one to indicate the whole tween is now started). This may be better served via a delayedPlay invocation.

@wtravO
Copy link
Contributor Author

wtravO commented Mar 5, 2018 via email

@wtravO
Copy link
Contributor Author

wtravO commented Mar 5, 2018 via email

@Jerenaux
Copy link

Jerenaux commented Mar 5, 2018

We discussed it on the Slack today, and having onStart() fire immediately does seem very counter-intuitive, especially for simple tweens on a single property.

I think the cleanest and most flexible solution could be to have onStart() be called for each property indeed, possibly receiving an extra argument indicating for which property it's triggering? Then, for single-property tweens, onStart() could still be used as usual and behave intuitively, while for multi-properties tweens the dev would have the flexibility to decide what to do, and elect to use the first call or the last call or whatever.

Might also be interesting to consider adding a new event triggering a onAllStart() callback that would specifically trigger only when they have all started.

@photonstorm
Copy link
Collaborator

onStart doesn't signify the start of a target property tweening, it's the start of the tween itself running, which happens the moment you call play. If this changes then it becomes impossible to sequence events in a Tween Timeline, as there's no differentiation between a paused tween pending playback and a running tween just holding out for one of potentially many delays to expire.

It will need a different callback to tell these two states apart.

Half-way through building the Tween system I removed all the callbacks in favor of using events, but put them back. In hindsight, I should have left them out and kept it event based. Actually, it may still be worth making that change. The existing callbacks can be moved to be TweenData specific and nothing to do with the Tween itself. Because this issue isn't just related to onStart - when should onRepeat fire in a Tween which changes multiple properties - where it's possible for one of those properties to have repeated lots of times before the Tween itself ever repeats at all. This is why I think moving all of the callbacks to the properties and using events for the Tween itself will work a lot better.

@photonstorm
Copy link
Collaborator

Thank you for submitting this feature request. We have implemented this and the feature has been pushed to the master branch. It will be part of the next release. If you get time to build and test it for yourself we would appreciate that.

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

No branches or pull requests

3 participants