-
Notifications
You must be signed in to change notification settings - Fork 136
Refactor of Tween classes (calling start() is mandatory) - to address is... #129
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
base: master
Are you sure you want to change the base?
Conversation
… issue useflashpunk#128: - all tween packages/classes are affected by these changes (misc, motion, sound, FP) - the call to start() has been removed from tween() - tween() returns the (specific) tween itself for chaining - Tween.start() also returns the tween itself for chaining (you'd probably have to cast it to the proper tween) Why? `addTween(_someTween_, true/false)` should now behave correctly.
|
@jacobalbano @Draknek How does this change look to you guys? |
|
Looks good to me! |
|
I have no objection to returning useful values rather than null, but I can't see the argument for removing the call to I'm personally wary of breaking backwards compatibility. |
|
As I recall the issue is that if you call tween(), then add the Tween to a Tweener, it runs instantly even if you pass false as the This is inconsistent with the Alarm class as well as being counter-intuitive. |
|
Hmm. Well there's the delay option to run them a known length of time later, but sure I can see how that'd be awkward. My inclination would still be to save this up for a time when multiple other compatibility-breaking changes need to be made all at once, but it's @zachwlewis's call these days :) |
|
What about this change is not backward compatible? I'll need a list before I pull it in so I can have complete changelog notes. If this does need a major version bump, we might want to pull other big changes in as well. |
|
The incompatibility is whether a tween will start automatically when it is added. As it is now: addTween(myTween, false); // autostart = false, tween will be started anywayThis is true for anything but an Alarm. Alarms added with |
So, there's no API change that will cause the game to just not compile? The incompatibility is that when people build their game with this change, Tweens will work the way they should've from the beginning? (I guess I'm still confused with the nature of this change.) |
|
@jacobalbano recalls correctly. The topics that brought up the issue are these:
Buuut... I'd say not too merge this one, 'cause reexaming it now it doesn't fully makes sense to me, hehe 😬. The problem still holds, but I'm not too happy of the way I handled it. |
|
Oh... so here's the thought process. Forgot about it: #128 |
|
@zachwlewis There's no API change that would break the compiling, but the behaviour would be changed a little and games could act differently, resulting in not-expected behaviours during runtime. |
|
As stated previosly: still I'm not happy with how it works as of now. @jacobalbano said:
Someone should probably look further into this and come up with a solution. |
...sue #128:
Why?
addTween(_someTween_, true/false)should now behave correctly.I've tested it... maybe not thoroughly but... Any thoughts/comments about this?