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

[3.x] Backport the new Tween system as SceneTreeTween #60581

Merged
merged 1 commit into from
Apr 29, 2022

Conversation

timothyqiu
Copy link
Member

@timothyqiu timothyqiu commented Apr 28, 2022

Backports #41794 and later updates.

Like we have both Timer and SceneTreeTimer, the new Tween is backported as SceneTreeTween, so it does not break compatibility.

As it's expected to be created from get_tree().create_tween()/node.create_tween(), it does not matter what the class is called 😛

Some notes:

  • interpolate_value() was recently changed to a static function on master, it's kept as a member function here.
  • Callable is not available on 3.x, so this type of parameter is changed to a target and a method.
    • tween_callback() takes up to 8 additional args that'll be passed to the method.
    • tween_method() can only be called on methods that takes one parameter (Same as Tween.interpolate_method)
    • tween_callback() and tween_method() also takes an optional binds array for parameter binding.
  • Only the TweenPauseMode enum is added, other enums uses Tween's version directly.
  • scene/animation/easing_equations.h is copied from master.

@timothyqiu timothyqiu added this to the 3.5 milestone Apr 28, 2022
@timothyqiu timothyqiu requested review from a team as code owners April 28, 2022 09:25
@timothyqiu timothyqiu requested a review from KoBeWi April 28, 2022 09:29
@akien-mga
Copy link
Member

akien-mga commented Apr 28, 2022

scene/animation/easing_equations.h is copied from master.

3.x has it in thirdparty/misc/easing_equations.h, though possibly a bit different. This might need reworking so that the code that currently depends on thirdparty/misc/easing_equations.h switches to use scene/animation/easing_equations.h (or the other way around if compatible).

@timothyqiu
Copy link
Member Author

I searched for the filename before copying, didn't expect it's a .cpp file on 3.x 🤣

thirdparty/misc/easing_equations.cpp is an unformatted version of easing_equations.h plus some Tween code. I kept easing_equations.h and moved those Tween code back to tween.cpp.

Like the enums, SceneTreeTween now uses the static Tween::run_equations() instead of its own version.

@Mickeon
Copy link
Contributor

Mickeon commented Apr 28, 2022

I've been writing the same boilerplate code so many times, I look forward to this being in 3.x as soon as possible! A nice way to ease the transition to this new way of doing Tweens.

@KoBeWi
Copy link
Member

KoBeWi commented Apr 28, 2022

Awesome! I was thinking about doing this, but never got around to it.

tween_method() can only be called on methods that takes one parameter (Same as Tween.interpolate_method)

You could pass the binds as array. That's what 4.0 Tweens did originally before Callable.bind() was added.
Also for tween_callback(), you could pass binds as array too. It's consistent with connect() and doesn't have problems like not allowing null arguments.

(or the other way around if compatible).

4.0 easing equations work exactly the same, I just changed some code that could cause undefined behavior (e.g. #28177).

@timothyqiu timothyqiu force-pushed the neo-tween branch 2 times, most recently from ec294e4 to 4b24044 Compare April 28, 2022 13:19
@KoBeWi
Copy link
Member

KoBeWi commented Apr 29, 2022

Aside from the comment above, it should be fine now.
I'd also add a mention of SceneTreeTween somewhere to Tween.xml.

doc/classes/Tween.xml Outdated Show resolved Hide resolved
Co-authored-by: Tomasz Chabora <kobewi4e@gmail.com>
@akien-mga akien-mga merged commit 3a3dd20 into godotengine:3.x Apr 29, 2022
@akien-mga
Copy link
Member

Thanks!

@timothyqiu timothyqiu deleted the neo-tween branch April 29, 2022 12:11
Comment on lines +296 to +299
<signal name="finished">
<description>
</description>
</signal>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This description is missing for some reason.
@timothyqiu

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

Successfully merging this pull request may close these issues.

None yet

4 participants