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

Complete rewrite of Tweens #41794

Merged
merged 1 commit into from
Jun 19, 2021
Merged

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Sep 5, 2020

EDIT: Scroll a bit further for interesting stuff.

This is meant to close godotengine/godot-proposals#514

Note that THIS IS HEAVILY WIP. The implementation is incredibly hacked together for now, full of dead and commented out code. Also it's mostly unfinished. I wanted to open the PR early to make sure that this is going in the right direction.

In its current state you can do this:

var tween := get_tree().create_tween()
tween.tween_property($icon, "position", Vector2(340, 100), 4)
tween.tween_property($icon, "modulate", Color.red, 1)

which will result in
kaJpR0TdMP

So onto details:
Tweens are no longer nodes. They are References, with very similar logic to SceneTreeTimers, designed in a "fire and forget" manner. You create a Tween with get_tree().create_tween(), which will result an empty Tween. To do something with it, you then call one of the tweening methods. These are:
tween_property - tweens a property between given values (equivalent of current interpolate property)
tween_interval - waits for given time (equivalent of a timer)
tween_callback - calls a method on an object (equivalent of current interpolate callback)
tween_method - calls a method on an object with a tweening argument that goes from initial to final value (equivalent of current interpolate method)

You can call as many tweening methods on the Tween as you want, they will be sequenced one after another, i.e. there's support for chaining out of the box. Parallel tweening will be done explicitly with a dedicated method.

As for the implementation, there two main classes: Tween and Tweener. Tween is basically a collection of Tweeners. When you call tweening method, it actually creates a Tweener. Tweeners are packed in a List. Tween will go through the list and process every Tweener and when it finishes, it goes onto another step in the sequence. The Lists of Tweeners for each step are contained in a Vector.

Internally, SceneTree calls _step() on all active Tweens. Each Tween then calls _step() on each current Tweener. _step() has a return value. When a Tweener returns false, it's considered finished (but doesn't get freed, in case you want to loop the Tween). If Tween returns false, it's removed from the list of tweens in SceneTree (and freed if you don't keep a reference). Tweener is also removed when the targeted object gets removed (whether this should happen silently or needs to be handled manually by user is up to discussion).

These are the basics. The new Tweens are going to work mostly like in the proposal (see the TweenSequence class linked in there). I yeeted the old Tween code, save for the interpolation (the _interpolate method in PropertyTweener, which is going to get cleaned up). The _calculate_interpolation method defined in easing_equations.cpp will stay there and since I need it in another class, there's a chance to expose it (I remember there was a request for it).

The thing that I'm the most unsure of is whether Tweens should be handled directly by SceneTree. But I don't have other idea how they could be handled, aside from maybe TweenServer.


EDIT:
Ok, the feature implementation is complete. What's left is error handling and documentation.

Here's a brief summary of the features:
(scroll below for actual usage examples)

Tween

  • Tweens are now Reference type handled by SceneTree, which makes them super cheap. They are intended for fire-and-forget use
  • You create a Tween by using get_tree().create_tween() or create_tween() inside Node (the latter will automatically bind the Tween, see below)
  • You can also use get_tree().get_processed_tweens() to get the list of all existing Tweens
  • Tweens have process mode and pause mode, so they can process either in idle frame or physics frame and can be paused
  • Tweens can also be bound to a Node, by using bind_node(node). The Tween will not process if the node is not inside tree and will be automatically removed when the node is freed
  • You can loop the Tween with set_loops(loops) and 0 will make infinite loop
  • You can stop (reset), pause, resume and kill a Tween
  • To actually Tween something, you need to use a tweening method (see below), which will return a Tweener
  • You can call as many tweening methods as you want and they will be executed in a sequence
  • If you want tweening method to run parallely, you can use tween.parallel()
  • Using Tween.set_parallel(true) will make all subsequent Tweeners parallel
  • You can also use Tween.set_ease() or Tween.set_trans() to set default easing and transition for a Tween
  • Alternatively, there's a method tween.interpolate_value(initial_value, delta_value, time, duration, transition, easing) which allows you to manually interpolate a value with easing (supersedes Add Tween.interpolate() as a low-level tweening option #36997)

There are 4 available Tweeners:
PropertyTweener

  • Created with tween.tween_property(object, property, final_value, duration)
  • By default, PropertyTweener starts from current value at the time it starts tweening
  • You can use from() to set a custom starting value
  • You can also use from_current() to assign it the value it has currently (i.e. when creating the Tween)
  • And finally, you can use as_relative() to make the final value be a relative value (so e.g. tweening position to Vector(100, 0) relatively will move it by 100 pixels instead of moving it to position (100, 0))
  • PropertyTweener inherits transition and ease types from the Tween. You can set them with set_ease(ease) or set_trans(trans)
  • You can also make the PropertyTweener delayed with set_delay(delay)

IntervalTweener

  • Created with tween.tween_interval(duration)
  • It just does nothing for given time, bruh

CallbackTweener

  • Created with tween.tween_callback(callback), where callback is a Callable
  • Calls the given method
  • To bind additional arguments to the method you can use Callable.bind(args)
  • You can use set_delay(delay) to have a delay before the callback

MethodTweener

  • Created with tween.tween_method(callback, from, to, duration)
  • This is like PropertyTweener, but instead of tweening an actual property, it tweens an internal variable and calls a method with this variable
  • To bind additional arguments to the method you can use Callable.bind(args)
  • You can set easing type and transition type like in PropertyTweener
  • You can also delay it
  • This is super cool to use with lambdas

Ok, so these are features. Now on the actual usage, it basically boils down to:

var tween = get_tree.create_tween()
tween.tween_something(something args)
tween.tween_something(different args).some_settings_etc()

which will execute the sequence of 2 given tweening operations.

Some samples:

var tween = get_tree().create_tween()
tween.tween_property($Sprite, "position", Vector2(300, 100), 1).set_trans(Tween.TRANS_SINE)
tween.tween_property($Sprite, "modulate:b", 0, 1)

ezgif-2-634b3229f1bb

var tween = get_tree().create_tween().set_trans(Tween.TRANS_ELASTIC).set_ease(Tween.EASE_OUT)
tween.tween_property($Sprite, "rotation", PI/2, 1)
tween.parallel().tween_property($Sprite, "position", Vector2(200, 200), 1).as_relative()
tween.tween_property($Sprite, "modulate", Color.red, 1)

5GLRBjILLh

var sprite = $Sprite
var tween = get_tree().create_tween().bind_node(sprite)
tween.tween_property(sprite, "position", Vector2(500, 0), 3).as_relative()
	
await get_tree().create_timer(1).timeout
remove_child(sprite)
await get_tree().create_timer(1).timeout
add_child(sprite)

ezgif-2-2365f3574e52
Same as above but without bind
ezgif-2-fc313369f2bb

var tween = get_tree().create_tween().set_loops(2)
tween.tween_property($Sprite, "position", Vector2(200, 0), 0.5).as_relative()
tween.tween_interval(0.5)

ezgif-2-7e2e6499c732

var tween = get_tree().create_tween().set_loops() #-1 by default
tween.tween_interval(1)
tween.tween_callback($Sprite.hide)
tween.tween_interval(1)
tween.tween_callback($Sprite.show)

ezgif-2-b3bc92085efb

func _ready():
	var tween = get_tree().create_tween()
	tween.tween_method(set_label_text, 0, 10, 1)

func set_label_text(value: int):
	$Label.text = "Counting " + str(value)

ezgif-2-64907598dcf6
Lambda version of the above:

var tween = get_tree().create_tween()
var label = $Label
tween.tween_method(func(value: int): label.text = "Counting " + str(value), 0, 10, 1)

If you have any questions or feature requests be sure to write them in the comments.

@Xrayez
Copy link
Contributor

Xrayez commented Sep 5, 2020

Nice to see it works out, see my previous comments revolving around this kind of interface in #26529 (comment) supporting this on the high-level.

@KoBeWi
Copy link
Member Author

KoBeWi commented Sep 5, 2020

@Xrayez If you mean autostarting and handling pause, then it's already planned.
btw the code sample you given there will probably look like this:

var tween = get_tree().create_tween()
tween.tween_property($Godot, "modulate:a", 0, 5)
tween.tween_callback($Godot.queue_free) #alternative to connecting finished signal

@dalexeev
Copy link
Member

dalexeev commented Sep 6, 2020

Parallel tweening will be done explicitly with a dedicated method.

Maybe something like this:

var tween := get_tree().create_tween()

tween.tween_property($icon, "position", Vector2(340, 100), 4)

tween.group_begin()
tween.tween_property($icon, "modulate", Color.red, 1)
tween.tween_property($icon, "scale", Vector2(2, 2), 1)
tween.group_end()

@KoBeWi
Copy link
Member Author

KoBeWi commented Sep 6, 2020

Maybe something like this:

Nah, it will be simpler. It's not mentioned in the OP, but the Tweens will make heavy use of chaining (every Tween/Tweener method returns that Tween/Tweener to allow multiple calls).

That said, your code will look like this:

var tween := get_tree().create_tween()

tween.tween_property($icon, "position", Vector2(340, 100), 4)

tween.tween_property($icon, "modulate", Color.red, 1)
tween.join().tween_property($icon, "scale", Vector2(2, 2), 1)

Parallel Tweening is going to be achieved with parallel() and join() methods followed by a Tweener method. The difference between the two is that join() will result in an error if there was no previous tweening command.

@dalexeev
Copy link
Member

dalexeev commented Sep 6, 2020

We must try to make this not only simple, but also as versatile as possible. How to get the following using your way:

I had a thought about await:

await tween.tween_property( 1 )

tween.tween_property( 3 )
await tween.tween_property( 2 )

tween.tween_property( 4 )
await tween.tween_property( 5 )

tween.tween_property( 6 )

But this is too GDScript specific.

@KoBeWi
Copy link
Member Author

KoBeWi commented Sep 6, 2020

How to get the following using your way:

By using two Tweens. You'd need to add CallbackTweener 5b with a delay equal to duration of 5, which would create a new Tween that does 6. Tweens are cheap in the new implementation, so you don't have to worry about creating many of them.

Allowing asynchronous steps would make the implementation much more complicated. Although maybe I could add at least a finished signal for Tweeners to make your await code possible.

@dalexeev
Copy link
Member

dalexeev commented Sep 6, 2020

I got it. Something like this:

tween1.tween_property( 1 )
tween1.tween_property( 2 )
tween1.tween_property( 5 )
tween1.tween_property( 6 )

tween2.tween_interval( ... )
tween2.tween_property( 3 )
tween2.tween_interval( ... )
tween2.tween_property( 4 )

That is, you can always do without .join() if you add another Tween. But that's a little unintuitive.

@KoBeWi
Copy link
Member Author

KoBeWi commented Sep 7, 2020

Ok, I completed the features. Check the added section in the OP.

@kleonc
Copy link
Member

kleonc commented Sep 7, 2020

var tween = get_tree().create_tween().set_loops(2)
tween.tween_property($Sprite, "position", Vector2(200, 0), 0.5).as_relative()
tween.tween_interval(0.5)
gifExampleLoops

I don't think set_loops(n) should make tween execute n + 1 times. I find it unintuitive.

@KoBeWi
Copy link
Member Author

KoBeWi commented Sep 7, 2020

I don't think set_loops(n) should make tween execute n + 1 times. I find it unintuitive.

set_loop(n) means "loop n times". Maybe it should be called repeat?

@reduz
Copy link
Member

reduz commented Sep 7, 2020

While I agree tween should be a reference, I think it should be something created in Node, not in SceneTree (even though it is processed in SceneTree), so you simply use it like

tween("property",values)
# or
$node.tween("property",values)

I would do the same change for timers

@dominiks
Copy link
Contributor

dominiks commented Sep 7, 2020

Hi I just saw your rewrite on Twitter and I very much agree with the 'fire and forget' idea of tweens and many of the other changes and improvements you have made. In the past I found myself always creating tween objects on demand and never really figured out how to properly restart or reuse them.

Creating tweens

Is there a specific reason that the create_tween function must be on the SceneTree? I have the feeling it should be a function of Node with the tweens automatically binding to that node that "owns" them. Just throwing them at the tree without an "owner" probably works in 99% of the cases but it might be a source of confusing bugs where tweens run through pause or show unexpected behavour differing from the "owner" node because I forgot to bind the tween after creation. I would probably just bind them always to prevent this.

Also it fits a bit better into the composition philosophy of Godot when the tween is created on the current Node instead of the tree which feels a bit more global.

Auto start tweens

If I understand correctly tweens automatically start on the next frame after their creation? And if you want to delay the start you would add a delay with set_delay or inserting an interval tween in the front? I cannot remember if I ever actually wanted to not start a tween immediately after creation but preventing the auto start to start them manually at an arbitrary point could be useful in some scenarios?

Parallel tweens

Just creating multiple tweens to run at the same time will probably enough in most cases but you are right that there needs to be some sort of join mechanism for more complex tweening. Calling this process join is a good fit as the terminology and thought process behind this is similar to multithreading. Intuitively I would expect it to work something like this:

var tween := create_tween()
tween.tween_property(...) #A

var parallel := tween.join()
parallel.tween_property(...) # B
parallel.tween_property(...) # C

tween.tween_property(...) # D

This should run tween A, then B and C parallel and after that tween D. This underlines the idea that calling the tween_* functions on the tween chains them one after the other and a group of parallel tweens is just chained in.

I think the names of the tween_* functions could be changed into something that underlines that they are added instead of overwritten (it is possible to think a created tween can only run one tween function) and that they run in sequence. But I don't have a good name for that at the moment. Something like add_interpolate hints that multiple tweens are possible but does not indicate that they are a sequence.

@eon-s
Copy link
Contributor

eon-s commented Sep 7, 2020

I don't like it accessed via SceneTree, it makes the tween access a bit dirty, while it can use a Node like it is currently and expose configuration via inspector which would be great for many workflows and more godot-like.

@API-Beast
Copy link

API-Beast commented Sep 7, 2020

Usually you would combine tweens with one or multiple AnimationPlayer to get most of the functionality in this pull request. I think it is overdoing it a little, trying to do too much with tweens alone.

Edit: the relative movement and the fixed amount of loops is something I would like to see in the AnimationPlayer instead.

@MagellanicGames
Copy link

The idea seems solid and very much an improvement, removing it from being a node and only accessible through the SceneTree just seems a bit....off. It's probably just me. I certainly like occasionally using "get_tree().create_timer" in an in-line way for yields but I will often pull that back into a node composed within a scene so it's a bit cleaner and self-contained. I know it's handled now, so that in case a yield returns to a now dead node/scene it wont crash but this would have to be handled as well with tweens.
I'm well aware though that this could just be me, but I thought I would voice the concern in case what I'm talking about has any relevance.

@sketchyfun
Copy link
Contributor

sketchyfun commented Sep 7, 2020

Maybe it's just me, but I feel like the default behaviour of a tween should be to process in parallel, which is how it works currently. The join() function doesn't really make sense in terms of doing things in parallel. I'd probably do something like:

parallel:
tween.interpolate_property(blah)
tween.interpolate_property(other_blah)
tween.start()

sequence:
tween.interpolate_property(blah)
tween.chain().interpolate_property(other_blah)
tween.start()

(not sure if tween.start() is still a thing in this PR?)

Not sure if 'chain' is the best word (maybe append?), but I think something like that would better describe the idea of creating/adding to a sequence rather than 'join'

@KoBeWi
Copy link
Member Author

KoBeWi commented Sep 7, 2020

I have the feeling it should be a function of Node with the tweens automatically binding to that node that "owns" them.

I actually had this idea already. Seeing how some people requested it I'll add it as an alternative way of creating Tweens.

Auto start tweens [...]

If you don't want the Tween to start immediately, you can call stop() or pause() after it is created. This will be mentioned in the docs.

This should run tween A, then B and C parallel and after that tween D. This underlines the idea that calling the tween_* functions on the tween chains them one after the other and a group of parallel tweens is just chained in.

This can be already achieved with

var tween = get_tree().create_tween()
tween.tween_property(...) #A
tween.tween_property(...) # B
tween.join().tween_property(...) # C
tween.tween_property(...) # D

When you use join() or parallel() method, any tweening method called immediately after will run in parallel with the previous one. Tween will go to next step only when all parallel Tweeners have finished.

I think the names of the tween_* functions could be changed into something

In my prototype they were called append_*. I could rename this probably.

Usually you would combine tweens with one or multiple AnimationPlayer to get most of the functionality in this pull request. I think it is overdoing it a little, trying to do too much with tweens alone.

Long before the PR I made a prototype in GDScipt and have been using it in my project for few months. Tweens made this way are fun to abuse for lots of different things. I never needed to combine them with AnimationPlayer, this is only useful when you want to visually design some animation.

I know it's handled now, so that in case a yield returns to a now dead node/scene it wont crash but this would have to be handled as well with tweens.

This is irrelevant. You don't need to use yield/away with Tweens, you just tell them to animate something and they animate it. If the animated object is removed midway, the Tween will automatically stop and get deleted.

Maybe it's just me, but I feel like the default behaviour of a tween should be to process in parallel, which is how it works currently.

Dunno, Tween chaining was requested for ages. Personally I more often tween something sequentially than parallely. Maybe I could add a method that makes paralleling default? 🤔

@kleonc
Copy link
Member

kleonc commented Sep 7, 2020

set_loop(n) means "loop n times". Maybe it should be called repeat?

The main problem here is behaviour, not naming. I think the most common use case will be to make tween execute exactly n times in total and having to call set_loop(n - 1) (or however this method will be named) will be counterintuitive and simply annoying. It will be counterintuitive because in most programming languages when you want to execute some code exactly n times you use loop (like for) where you provide template (code block) and tell how many times it should be executed. In your current tween interface you're providing a template (tween) which will be executed once and you can additionally tell how many more times it should be executed. I think it's not what programmers are used to and a common sense is to change it. Of course it's just my opinion and I might be wrong.
So I suggest changing behaviour to set_loop(total_number_of_iterations/executions) (however this method will be named).

For current behaviour: yes, repeat is better name (but it still could be misunderstood because of analogy to standard loops).

@MarcusElg
Copy link
Contributor

MarcusElg commented Sep 7, 2020

set_loop(n) means "loop n times". Maybe it should be called repeat?

The main problem here is behaviour, not naming. I think the most common use case will be to make tween execute exactly n times in total and having to call set_loop(n - 1) (or however this method will be named) will be counterintuitive and simply annoying. It will be counterintuitive because in most programming languages when you want to execute some code exactly n times you use loop (like for) where you provide template (code block) and tell how many times it should be executed. In your current tween interface you're providing a template (tween) which will be executed once and you can additionally tell how many more times it should be executed. I think it's not what programmers are used to and a common sense is to change it. Of course it's just my opinion and I might be wrong.
So I suggest changing behaviour to set_loop(total_number_of_iterations/executions) (however this method will be named).

For current behaviour: yes, repeat is better name (but it still could be misunderstood because of analogy to standard loops).

Totally agree. Iterating n + 1 times is just confusing, makes it feel like a do loop. I feel like most poeple exspect it to iterate n times and will have to go back and change their code.

@dominiks
Copy link
Contributor

dominiks commented Sep 7, 2020

I definitely support that chaining should be the default instead of parallel running but a good compromise would be to have two seperate functions to create the tween object: one creates a chaining one and the other creates one that runs all tweens at the same time.

If you don't want the Tween to start immediately, you can call stop() or pause() after it is created. This will be mentioned in the docs.

Fair enough then. If the documentation covers this all is well.

This can be already achieved with

var tween = get_tree().create_tween()
tween.tween_property(...) #A
tween.tween_property(...) # B
tween.join().tween_property(...) # C
tween.tween_property(...) # D

When you use join() or parallel() method, any tweening method called immediately after will run in parallel with the previous one. Tween will go to next step only when all parallel Tweeners have finished.

I don't know but something about doing it this way feels odd to me, maybe the empty join function that looks a bit out of place or useless. It might be just me but it would be good if the simultaneous execution of tweens B and C is not created by adding only tween C in a special way. Adding some empty lines for formatting obviously helps make B and C look like they belong together. Creating "tween-group" objects as I suggested makes it a bit easier (for me) to conceptualize the sequence and parallel relations.

@KoBeWi
Copy link
Member Author

KoBeWi commented Sep 7, 2020

After some feedback, I added a create_tween() method in Node, which will create a Tween and automatically bind it to that node. Also Tween got a new append() method, which takes a Tweener. Node got an alternative tween_property method that returns PropertyTweener created for that object. With all this combined, you can now do:

create_tween().append(tween_property("position", Vector2(100, 100), 1)).append(tween_property("position", Vector2(100, 200), 1))

This actually gives lots of possibilities for creating custom Tween-convenience methods. But creating Tweeners manually is not exposed to GDScript (right now).

Consequently, tween_* methods in Tween were renamed to append_* to make it more obvious that the Tweener created this way is automatically appended to Tween.

From other changes, I added a pause mode called TWEEN_PAUSE_BOUND, which makes the Tween pause when the bound node is paused (without bound node it works like TWEEN_PAUSE_STOP).

I haven't touched set_loops and parallel yet. I'm not sure what to do with the latter. Maybe I should add a set_parallel() method which makes Tween parallel by default and then chain() method which chains the Tweeners when Tween is in parallel mode? (also parallel() would be removed as join() is enough, but it wouldn't have the error which is rather useless).

I yet have to update the OP.

@avedar
Copy link

avedar commented Sep 7, 2020

The ability to use custom curves for both the ease and trans would be greatly appreciated

@pixelpicosean
Copy link
Contributor

It's great to have nodes ability to create local tweens, very similar API to my JavaScript port of Godot ;) I did the same thing for performance, but have made Tween a completely self contained class that developer can add to any node when they want (a little bit hard to use because you need to update the manager yourself, but anyway as far as I know I am the only developer using that engine).

@Ansraer
Copy link
Contributor

Ansraer commented Sep 8, 2020

Consequently, tween_* methods in Tween were renamed to append_* to make it more obvious that the Tweener created this way is automatically appended to Tween.

Not sure if I am a fan of this. If I understand this correctly I would have to use create_tween().append_property(...) to tween a property. My problem with this is that "append property" doesn't make it obvious for people who haven't read the docs that the appended property is being tweened. (Is tweened a word?)

In contrast I immediately understood your create_tween().tween_properties(...) examples even without reading your explanation.
tbh I would prefer it if this change was reverted.

I am also not a huge fan of parallel() and join(). Based on my understanding of the earlier comments the two of them both run the next command in the chain concurrently with the previous command(s).
Being used to a fork/join workflow this confused me a LOT.
I would recommend parallel() = run the next tween concurrently, fork() telling the tweener to make parallel the default mode and join() being used to switch back to sequential mode.

(Edit: minor spelling correction, I typing am on my phone)

@pixelpicosean
Copy link
Contributor

The ability to use custom curves for both the ease and trans would be greatly appreciated

I also vote for custom curve support, but we may not need Godot's built in Curve because it may be better to use a Tween node with Curve and using the editor to do the editing. My suggestion is something like power based curves that may use a cubic spline for implementation, so developers can use cubic params to define the easing.

@DozyBird
Copy link

DozyBird commented Oct 21, 2021

Is it possible to find existing tweens bound to a particular node?

If I understand right, the only method right now is iterating through all the results of get_tree().get_processed_tweens().

@KoBeWi
Copy link
Member Author

KoBeWi commented Oct 21, 2021

It's not possible right now. Also get_processed_tweens() returns all Tweens and you can't get the bound node from a Tween, so the best way to have a list of Tweens bound to a node is make one yourself (i.e. put any created Tween in an Array variable).

If you think Tweens should support that normally, open a proposal.

@alexzheng
Copy link

This Tweens implementation is much better than the 3.x.
Would it be back ported to 3.x?

@KoBeWi
Copy link
Member Author

KoBeWi commented Oct 27, 2021

@alexzheng There is a very similar implementation in GDScript that you can use in 3.x: https://github.com/godot-extended-libraries/godot-next/tree/master/addons/godot-next/references/tween_sequence
There are no plans to backport new Tweens, they'd have to be a separate class, because there is too much compatibility breakage.

@alexzheng
Copy link

alexzheng commented Oct 27, 2021

This new tween may not required to be compatible with the Tween Node, It would be nice if it could be available as another option in 3.x, just keep the Tween Node as it was, personally, I really do not like the Tween node.

@alexzheng
Copy link

@KoBeWi
Check this code snippet
The position animation will jump to the final value and finished after scale animation finished.

var tween := get_tree().create_tween()
tween.tween_property($Icon, "position", Vector2(600, 900), 5.0).finished.connect(func() : printt("position finished"))
tween.parallel().tween_property($Icon, "scale", Vector2(5, 5), 1.0).finished.connect(func() : printt("scale finished"))

@KoBeWi
Copy link
Member Author

KoBeWi commented Oct 28, 2021

@alexzheng Open an issue about it.

@seppoday
Copy link

seppoday commented Jan 27, 2022

Hello @KoBeWi I don't know if it is a bug or not so I am posting this comment. I tried to queue_free() my node when there was tween in the scene looping. When I tried to do it whole game just crashed without errors or anything. Solution was to kill tween before freeing node. Maybe there is some way to auto free tween when someone free node?

Ps: Seems like creating tween under this node and not by get_tree() is also solution. When I did it I didnt need to call kill() anymore. So probably fault on my side. I will delete this comment soon. Leaving for now. Maybe in fact there is bug or not.

Godot_v4 0-alpha1_win64_LGsagz5jgN

@KoBeWi
Copy link
Member Author

KoBeWi commented Jan 27, 2022

@seppoday The Tween should auto-kill when the object gets deleted, so it sounds like a bug. Open a new issue with some minimal project

@owstetra
Copy link

owstetra commented Feb 5, 2022

@KoBeWi

@seppoday The Tween should auto-kill when the object gets deleted, so it sounds like a bug. Open a new issue with some minimal project

i guess it's a bug, because when i use a tween in a button for a menu, then close and free that menu i get this warning :

W 0:00:09:0746   start: Target object freed before starting, aborting Tweener.
  <C++ Source>   scene/animation/tween.cpp:702 @ start()

@KoBeWi
Copy link
Member Author

KoBeWi commented Feb 5, 2022

This is expected, if you free an object before animation starts, you get this warning. Although maybe it's not useful, idk 🤔

@owstetra
Copy link

owstetra commented Feb 5, 2022

This is expected, if you free an object before animation starts, you get this warning. Although maybe it's not useful, idk thinking

But the animation is already started and finished, but for some reasons i still get this warning

var ButtonTween : Tween = get_tree().create_tween()
ButtonTween.set_pause_mode(Tween.TWEEN_PAUSE_PROCESS)
ButtonTween.tween_property($BackgroundFocus, "rect_size", Vector2(0, self.rect_size.y), 0.15).set_trans(Tween.TRANS_LINEAR)

This code i used it in a button when it's pressed for a Pause menu to animate the button, but if i change the level or go to the main menu or close the game i get the warning

@KoBeWi
Copy link
Member Author

KoBeWi commented Feb 5, 2022

I tried your code and I don't get this warning. Can you share some minimal project? Also you could open a new issue.

@girng
Copy link

girng commented Feb 19, 2022

Just a bit of clarification (that maybe could be added somewhere?): What about tweens that are used over and over again, when the node doesn't get removed? I tested this, and the object count in the debugger continues to shoot up. Do those eventually get removed, or does the node need to be free'd?

@KoBeWi
Copy link
Member Author

KoBeWi commented Feb 19, 2022

All Tweens are removed automatically when not used anymore, but there is a known issue when it doesn't work: #52699

@PoisonousGame
Copy link

PoisonousGame commented Sep 26, 2022

Tween changes so much that the documentation is not yet sufficient to cover all use cases. I found a problem when using it that tween cannot be created outside the function, if this is not a bug, then it should be noted.

var tween
func _ready() -> void:
	tween = create_tween()

func transition()->void:
      tween.tween_property($Sprite, "modulate", Color.red, 1)

@sketchyfun
Copy link
Contributor

Tween changes so much that the documentation is not yet sufficient to cover all use cases. I found a problem when using it that tween cannot be created outside the function, if this is not a bug, then it should be noted.

var tween
func _ready() -> void:
	tween = create_tween()

func transition()->void:
      tween.tween_property($Sprite, "modulate", Color.red, 1)

Can confirm I had strange issues when using a tween created outside a function. The error wasn't exactly clear of what was going wrong, either

@KoBeWi
Copy link
Member Author

KoBeWi commented Sep 26, 2022

The documentation has countless examples where Tween animation is defined right after its creation, discourages re-use and tells you that you should use stop() when doing it.

But maybe the error should tell that too.

@TokisanGames
Copy link
Contributor

I am using tweens extensively in code. The key lines in the documentation are these:

Note: All Tweens will automatically start by default. To prevent a Tween from autostarting, you can call stop() immediately after it is created.

Note: Tweens are processing after all of nodes in the current frame, i.e. after Node._process() or Node._physics_process() (depending on TweenProcessMode).

That means you created an empty tween in ready, the tween begins that frame and continues looping indefinitely, then later on a subsequent frame, you're adding additional properties to change. The docs don't say what is supposed to happen when you add tweens while its running. Create it and set it up in the same frame you want it to run, or stop it upon creation for set up later as KoBeWi wrote.

@Jan-PieterInghels
Copy link

How are Tweens supposed to be created when continuously tweening something in _process()? Or is that a bad use case for them?

@TokisanGames
Copy link
Contributor

Generally, new devs should not create tweens in process. You don't want to create tweens 300 times per second. However, you can do it if you are manually checking the frequency and ensuring you're only creating them one every X seconds where X is less than or equal to your tween time.

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.

Overhaul the Tween system