Open henriiquecampos opened 1 year ago
By the way...maybe just port this to core?
I don't think we should have two ways to achieve the same thing in core. This is what add-ons are suited for :slightly_smiling_face:
@Calinou
I don't think we should have two ways to achieve the same thing in core. This is what add-ons are suited for 🙂
Isn't that what we have with Timer
node and get_tree().create_timer()
?
I mean, there are plenty of situations where you can achieve the same thing in multiple, not just two, different ways. I could cite at least five out of my head:
load()
keyworddraw_texture_rect()
vs TextureRect
/Sprite
draw_string()
vs Label
draw_polygon()
vs Polygon2D
draw_line()
vs Line2D
get_node(NodePath)
vs $NodePath
...And we can keep going, but the thing is: we used to rather use Nodes than functions. This was the whole Godot Engine design.
But the thing is: in this case specifically we can just merge the two into a Node, we don´t need to keep the current Tween RefCount after porting it as a Node
Ok, I am only using tween using create_tween().
but the idea of this proposal I think is valid, due to how Timer Node works. it is better to keep consistent with other nodes in the engine.
and I think this will satisfy both who use create_tween() and Node approach.
@henriiquecampos Your first code sample almost certainly fails because the variable declaration is missing a @onready.
And your code with the proposed node will not work, because it is also missing a @onready. So there is literally no difference between these two versions.
Edit: I hope there is no user on GitHub with the name onready
@henriiquecampos Your first code sample almost certainly fails because the variable declaration is missing a @onready.
And your code with the proposed node will not work, because it is also missing a @onready. So there is literally no difference between these two versions.
@zinnschlag
Edit: I hope there is no user on GitHub with the name onready
I tried to using onready
as well, same issue.
Edit: I hope there is no user on GitHub with the name onready
There is one, otherwise the bold clickable link wouldn't appear :slightly_smiling_face:
@henriiquecampos That is odd. But it makes no difference. You can still use the original version. Declare the variable globally but don't initialise it (or initialise it to null). Then assign the tween to it in the function like in your second version. Still doesn't justify adding another node type IMO.
onready must be really annoyed with Godot by now. Sorry!
@henriiquecampos That is odd. But it makes no difference. You can still use the original version. Declare the variable globally but don't initialise it (or initialise it to null). Then assign the tween to it in the function like in your second version. Still doesn't justify adding another node type IMO.
onready must be really annoyed with Godot by now. Sorry!
Ohh man...I definitely wouldn't be wasting my time if things were working as they used to, look: https://twitter.com/pigdev/status/1707099594796982731
Note the tween is an object in the reference as seen in the debugger. It just don't process, so the Tweener that it should return from the tween_property()
returns null.
Please understand, if it was working nice and smoothly as the Tween node used too, I wouldn't be as frustrated to the point of opening this issue.
I remember that the new Terrains system was considered a bit of a Godot anti-pattern just because it having the layers as inspector variables instead of nodes. I don't understand why we should ignore the nodes system this time. Anyways, as people are already using the new way, I think we should keep both!
@henriiquecampos That is odd. But it makes no difference. You can still use the original version. Declare the variable globally but don't initialise it (or initialise it to null). Then assign the tween to it in the function like in your second version. Still doesn't justify adding another node type IMO.
onready must be really annoyed with Godot by now. Sorry!
this is what i've been doing succesfully on my project (keeping references to tweens once they're started, and then stopping them if they're valid and running), and it works fine. It's a little weird indeed compared to the old behavior, but you get accustomed to it (not all tweened animations require to be stopped).
So i'm voting against this proposal.
@francoisdlt would the implementation of this proposal prevent you from keep doing this approach?
I think having "TweenPlayer" node could be beneficial. A node for managing multiple tween refs for nodes to interact. With this way, both current refcounted Tween and node version of Tween manager can have reason to exist with eachother.
Something I just thought about is that to prevent "breaking compatibility" the create_tween()
could return a Tween node that was already added to the SceneTree(just like the current one is already added as well) without the need of using add_child()
So.. I think the whole interface of the current class could be ported to a Node. I mean, I'm struggling to understand the resistance to make this switch(not only here but the twitter's threads as well)
Having multiple ways to do the same thing leads to bloat. It increases the code size and thus maintenance costs. It increases the binary size. It increases the documentation size. It increases the cognitive load on developers who have to choose between multiple APIs. Ideally there should be one way to do it and only one way.
@zinnschlag
Having multiple ways to do the same thing leads to bloat. It increases the code size and thus maintenance costs. It increases the binary size. It increases the documentation size. It increases the cognitive load on developers who have to choose between multiple APIs. Ideally there should be one way to do it and only one way.
As shown in this reply: https://github.com/godotengine/godot-proposals/issues/7892#issuecomment-1737654277
I don't think this is a valid reason to not have this proposal implemented, but on top of that, I'm not proposing to have two ways to do the same thing, the title is clear: merge the current Tween into a Tween node. The proposal suggests to have only the Tween node. But if this is not possible, supporting the two approaches shouldn't be such a hardache because, again as shown in the reply, "having two ways to do the same thing" is just the default Godot approach, especially turning one of these ways into a Node-based approach, as suggested in this proposal.
While I do agree that we need a more reusable way for Tweens, and that the TweenNode accomplished that very well, it's not impossible to replicate it's behavior with the current system.
The docs clearly say that tweens start working as soon as you create them, so doing @onready var tween = create_tween()
will make a tween that's "running" with no tweeners.
What I do to replicate the TweenNode behavior is also in the docs, keeping the tween variable and checking if it's empty, then if I want it to reset I use stop, if not, just kill. But you have to always kill the Tween before doing a new one:
var tween: Tween
func do_anim() -> void:
if tween: tween.stop(); tween.kill()
tween = create_tween()
#do animation here
This has more or less always worked for me and it's basically the same as keeping a reference to a TweenNode, I don't get the need to do for loops on the tree to search for all the processed Tweens.
@narredev
While I do agree that we need a more reusable way for Tweens, and that the TweenNode accomplished that very well, it's not impossible to replicate it's behavior with the current system.
The docs clearly say that tweens start working as soon as you create them, so doing
@onready var tween = create_tween()
will make a tween that's "running" with no tweeners.What I do to replicate the TweenNode behavior is also in the docs, keeping the tween variable and checking if it's empty, then if I want it to reset I use stop, if not, just kill. But you have to always kill the Tween before doing a new one:
var tween: Tween func do_anim() -> void: if tween: tween.stop(); tween.kill() tween = create_tween() #do animation here
This has more or less always worked for me and it's basically the same as keeping a reference to a TweenNode, I don't get the need to do for loops on the tree to search for all the processed Tweens.
I do understand that there are "work arounds" for this issue. But take a moment and assess: that: is this intuitive? Is this as intuitive as having a node(remember the Godot's design of using nodes as building blocks to create games with Godot).
But on top of that, look at this logic:
"I need to interpolate a property and at some point I can receive a new target value, so I need to stop the current interpolation and update it to aim towards the new target value. For that I need to: create a null reference for a variable type of tween, create a custom function that will stop the tween, even if it didn't even start to begin with, just to maintain a reference to it so I can use it later on, then I can actually play the interpolation but I have to ensure that it NEVER ends, because if it does, I will lose reference to the tween I was using to interpolate the previous value, and if at any point I need to interpolate another property I have to ensure that I stop the tween, even tho I may want to have the previous interpolation going"
Previously the line of thought was: I need to interpolate something, there's this node that is specialized in interpolations, I'll add one as a child and use it, when I receive a new value to interpolate to, I can stop the current track and interpolate to it instead.
Please correct me if I'm wrong, but just because we have workarounds to achieve the same outcome, it doesn't mean we can't improve things, right?
I mean, we can create games with assembly, but we use an engine because it provides easier and higher level approaches to create these games without us having to re-invent the wheel. Besides this code you've shown works, and it probably has been working for you, imagine if instead of having a Sprite node you had to do something like:
extends CanvasItem
@export var texture
@export var position
@export var rotation
@export var scale
func _draw():
draw_texture(texture, position)
Imagine not having the Sprite node, or any of the nodes mentioned in https://github.com/godotengine/godot-proposals/issues/7892#issuecomment-1737654277
Imagine not having the SceneTree and having to implement an engine processing loop by hand. Imagine not having a user interface like the Godot Editor and having to do everything via code without visual feedback. Imagine not having a user interface and having to create animations via code.
Do you understand this proposal is not a cry saying "it's impossible to do tweens in Godot Engine 4.0"? But instead it's a proposal to improve the way we do it because as it is it's not as intuitive as it was or as it can potentially be?
@henriiquecampos It's not a "work around". It's the intended use. It's in the official documentation:
If you want to interrupt and restart an animation, consider assigning the Tween to a variable:
var tween
func animate():
if tween:
tween.kill() # Abort the previous animation.
tween = create_tween()
This is how it would look with your functions:
var tween_pos: Tween
func interpolate_position(target_position, duration_in_seconds):
if tween_pos: tween_pos.kill()
tween_pos = create_tween()
var interpolation = lerp(previous_position, target_position, 1.0)
tween_pos.set_process_mode(Tween.TWEEN_PROCESS_PHYSICS)
var tweener = tween_pos.tween_property(spaceship, "position", interpolation, duration_in_seconds)
tweener.from(previous_position)
previous_position = target_position
func synchronize_position(new_position):
if tween_pos: tween_pos.kill(); tween_pos = null
spaceship.position = new_position
previous_position = new_position
It's not much different from what we did with the TweenNode. You also had to store a reference and check if it was running or active and stop it or pause it. I don't see this argument as a need for adding a TweenNode.
You use the Sprite node as an example that doesn't require code to work, and that makes it practical and in line with Godot, but the Godot 3 Tween node is kinda opposite to that, it does need a script to be used, just like the current Tween
@henriiquecampos I would like to have a TweenNode for having more ways of doing the same, as you said here: https://github.com/godotengine/godot-proposals/issues/7892#issuecomment-1737654277
But the new tweens can still accomplish the same as the old ones. I have many scripts making use of the new tween just like the old ones, and they all work as intended. I just want to make that clear. The documentation has a pretty decent tutorial/explanation on how to use them, with common use cases addressed, it's in the tween class page
I also see how they are a bit more confusing if you're coming from Godot 3 (I was also confused when I first transitioned), but they are more intuitive if you think of them as only one animation each, and more in line with other engines. They are also designed for working quicker, since in Godot 3 they needed to have both a Node and a script, now they only need a script.
Another argument could be allowing TweenNode to work without the need of a script, which is the purpose of the Timer node when I use it or the other nodes alternatives. This also allows for them to be reused when saving scenes instead of copy pasting code
Something like a simplified animation player for only one thing? Or maybe you would need to write an expression? Then you can add that TweenNode to any scene and make the parent do the animation when a signal fires...
Not really sure how it would work but that could be a very good reason to have Tween nodes
@henriiquecampos
@narredev
What about this? You can treat it like a node if you want to thanks to the bind_node()
that's automatically called by create_tween()
in the real usecase, the TweenNode is declared in it's own script with class_name
.
@CarpenterBlue That's not really a solution. You would have to do TweenNode.tween to access it, and it still behaves the same. You would still need to kill it and re create it each time in your script. The original Tween node was made to support multiple animations, what you propose only supports one so you would need to make a new node for each.
To make a proper Godot 3 tween node, it would need to store each created tween operation in some kind of dictionary, and properly manage them. Then you would access them by the object and/or property name to stop them.
Like https://github.com/godotengine/godot-proposals/issues/7892#issuecomment-1737858301 said, a Tween node should serve the purpose of managing a group of tween animations in one place
Just to give you guys a starting point as to why this change was made I think it's worth linking to the original proposal: https://github.com/godotengine/godot-proposals/issues/514
It explains the new features, but it also explains why the node has been removed (short answer: it doesn't serve any practical purpose because nothing is exposed on it and there is no dedicated editor for it, you still need to write code — now more than ever with the advanced sequencing features).
@YuriSizov
Just to give you guys a starting point as to why this change was made I think it's worth linking to the original proposal: #514
It explains the new features, but it also explains why the node has been removed (short answer: it doesn't serve any practical purpose because nothing is exposed on it and there is no dedicated editor for it, you still need to write code — now more than ever with the advanced sequencing features).
I think then the solution would be to exposed some properties like the Timer: process mode, easing, transition type, duration, autostart(maybe implement it?), etc...
Would be way easier than revamp the whole thing and break compatibility, no? Maybe creating a cool UI like MultiplayerSynchronizer, AnimationPlayer, etc...
Would be way easier than revamp the whole thing and break compatibility, no?
I'm not sure why you're drawing a comparison like that. The rework was done to add new functionality to the class, none of which can be exposed as properties. And the properties that you are listing — nobody asked for them, so why would anyone consider exposing them? Besides, exposing them is not an alternative to the features added...
easing, transition type, duration, autostart
The first 3 are unique for each tweener, the last one is not how you normally use tweens. They just have different use cases compared to timers, even if sometimes you can use them similarly.
Would be way easier than revamp the whole thing and break compatibility, no?
I'm not sure why you're drawing a comparison like that. The rework was done to add new functionality to the class, none of which can be exposed as properties. And the properties that you are listing — nobody asked for them, so why would anyone consider exposing them? Besides, exposing them is not an alternative to the features added...
easing, transition type, duration, autostart
The first 3 are unique for each tweener, the last one is not how you normally use tweens. They just have different use cases compared to timers, even if sometimes you can use them similarly.
I'm comparing the two because this is the whole point of this proposal. So you're saying the new functionalities couldn't be implemented in a Node? If nobody asked for the functionalities I pointed why they are in the new approach? If nobody uses nodes in autostart mode why the new class literally autostarts and the whole issue is caused precisely because it starts without anyone deliberately saying so, triggering its removal and losing reference to it?
I really didn't get the whole point of your reply. Please help me see your point.
On top of all that, the new approach makes a pseudo Node already. As mentioned in the documentation, the Tween is added to the scene tree, it has process and physics processing like only nodes have in Godot, and supposedely(as mentioned in the replies above) you should create it in the _ready()
callback or using @onready
keyword...
Why making it so similar to a node yet not making it a node if making it a node wod keep compatibility and prevent the current issues?
why the new class literally autostarts and the whole issue is caused precisely because it starts without anyone deliberately saying so
you should create it in the _ready() callback or using
@onready
keyword...
The new Tweens don't "autostart" nor "start without anyone saying so", and you do not have to create them with the onready keyword. They are not meant to be just a code replacement for the old Tweens, please properly read how the new Tweens work before criticism them. The documentation is quite clear, and the original proposal is also there. What you're complaining about in the quote and other parts of this proposal comes from not understanding how they work and forcibly using them as the old ones. I already explained how to get the same result in my other comments, and it's pretty much the same amount of lines and usability as the old ones.
Think of them as a single animation that can have multiple tracks each, meant to be discarded as soon as it finishes. They are only meant to be created in a function, so you always have the code that made them. They only start when you finish creating them, so you just call the function when you want them to start. You store a reference in a variable if you want to check their progress or cancel, pause, reset them, etc.
To compare the old way vs the new, you have to understand how both work.
Do you want to keep working the old way or to have a new Tween node? Those are two different things If you want to have a Tween node with the new stuff, it will be different than the Godot 3 one, and you will run into the same problems
@henriiquecampos
I mean, there are plenty of situations where you can achieve the same thing in multiple, not just two, different ways. I could cite at least five out of my head:
* Loading with ResourceLoader vs `load()` keyword * Drawing textures with `draw_texture_rect()` vs `TextureRect`/`Sprite` * Displaying text with `draw_string()` vs `Label` * Drawing polygons with `draw_polygon()` vs `Polygon2D` * Drawing line with `draw_line()` vs `Line2D` * Accessing nodes with `get_node(NodePath)` vs `$NodePath`...
note that most of the functions are the more low level and simplified version of the same functionality without some overhead. which both alternatives needed in different use cases.
The Tween Node removed because there were no additional usability nor interesting inspector properties as stated in https://github.com/godotengine/godot-proposals/issues/514. And you still need to write code to set it up anyway.
But the thing is: in this case specifically we can just merge the two into a Node, we don´t need to keep the current Tween RefCount after porting it as a Node
why the new class literally autostarts and the whole issue is caused precisely because it starts without anyone deliberately saying so, triggering its removal and losing reference to it?
cmiiw this is the norm for tween behavior in other engines. DoTween in Unity is also auto freed when tween finished. Otherwise it will gives memory leak.
I think what you want from the old Tween Node is its reusability by not set the tween to be autokilled. And as a node, it will be freed when the node or its parent is freed.
So we need both..
But for the purpose of reusability of Tween, i think add ability to set off autokill would be better solution just like DoTween has Tween.SetAutoKill(false)
After re reading this proposal and comments, this are this options for a Tween node:
The first one can be archived with an array and functions with loops, but there could be some kind of native official way to do it (not necessarily a node)
The second one is already available with the Threen extension like it was said at the beginning, but I don't know if it has all the new stuff or only the same as Godot 3. It's as easy to use as an addon and only useful for porting Godot 3 games (if it doesn't have the new stuff)
Third one, would only be for consistency and having more ways to do the same, but I see now how it's kinda useless if you still have to do the same code in the same way.
The last one, like https://github.com/godotengine/godot-proposals/issues/7892#issuecomment-1738294679 says, the new properties can't really be exported as is. They can be used multiple times in the same tween for different things, since a tween holds multiple tweeners in sequence or parallel. It would have to be a special visual editor, in which you edit tweener by tweener. How does it compare with the regular animation player? Could it be made reusable in any scene? Will it access all the properties in a node or script, or just the exported ones? Can I do an if statement for a tweener like I can in code?
I think @mikhaelmartin has the better idea, adding a "no auto kill" option will essentially make it as persistent as a node or any other resource. It would be much easier to implement and make it more in line with the other parts of the engine
Maybe instead of doing one step back and going back to node we should list problems with current system and try to design solutions around current system. If it is hard to stop specific node right now how can we improve it (without coming back to node system)?
If problem is that tween is autokilled. Add solution to turn it off in code. If problem is that tween does not return something make it return it. If problem is that it is hard to manage multiple tweens, maybe add some kind of tween manager... etc?
I feel like that discussion is "I can't do something in new tween system, let's revert it to older version or do some frankenstein thing". Current system can be improved...
I don't think we should have two ways to achieve the same thing in core. This is what add-ons are suited for 🙂
Following this logic, we should remove every other higher level tool like Scene Tree, Nodes and stuff because we have all functionality in Servers. XD
Point of this proposal is usability. :)
Seconding @seppoday. I haven't needed a persistent tween in a while, but last time I tried, I had issues with keeping one tween always available.
Maybe we should have a set_persistent()
method to just mark the tween itself not to be auto-removed after it has no more tweeners? Would that cover everything people need from the old tween?
I don't think we should have two ways to achieve the same thing in core. This is what add-ons are suited for 🙂
This reads like we should remove ray cast nodes, because we have intersect_ray
.
No, actually we find that there is nothing wrong with having two ways to do things in core: a high level way, and a low level way. This supports the idea: https://docs.godotengine.org/en/stable/contributing/development/best_practices_for_engine_contributors.html#cater-to-common-use-cases-leave-the-door-open-for-the-rare-ones
I don't think we should have two ways to achieve the same thing in core. This is what add-ons are suited for 🙂
This reads like we should remove ray cast nodes, because we have
intersect_ray
.No, actually we find that there is nothing wrong with having two ways to do things in core: a high level way, and a low level way. This supports the idea: https://docs.godotengine.org/en/stable/contributing/development/best_practices_for_engine_contributors.html#cater-to-common-use-cases-leave-the-door-open-for-the-rare-ones
You are focusing on proving your point instead of being open minded and finding best solution for problem.
No harm in doing that. The mistake here is that tweens aren't low-level at all, they are objectively at the same level of complexity as the node. Some use cases are easier, and a few rarer use cases are harder
I don't think we should have two ways to achieve the same thing in core. This is what add-ons are suited for 🙂
This reads like we should remove ray cast nodes, because we have
intersect_ray
.No, actually we find that there is nothing wrong with having two ways to do things in core: a high level way, and a low level way. This supports the idea: https://docs.godotengine.org/en/stable/contributing/development/best_practices_for_engine_contributors.html#cater-to-common-use-cases-leave-the-door-open-for-the-rare-ones
But the current Tweens are not the low level way, they effectively replaced the Node based Tweens as the current high level way?
I agree with seppoday. A proposal should be opened with information on what's missing with the current functionality and what is needed to fix them, rather than adding more ways to do the same thing.
Just to make it clean and not create mess in this proposal I opened discussion: https://github.com/godotengine/godot-proposals/discussions/7901
@henriiquecampos
I mean, there are plenty of situations where you can achieve the same thing in multiple, not just two, different ways. I could cite at least five out of my head:
* Loading with ResourceLoader vs `load()` keyword * Drawing textures with `draw_texture_rect()` vs `TextureRect`/`Sprite` * Displaying text with `draw_string()` vs `Label` * Drawing polygons with `draw_polygon()` vs `Polygon2D` * Drawing line with `draw_line()` vs `Line2D` * Accessing nodes with `get_node(NodePath)` vs `$NodePath`...
note that most of the functions are the more low level and simplified version of the same functionality without some overhead. which both alternatives needed in different use cases.
The Tween Node removed because there were no additional usability nor interesting inspector properties as stated in #514. And you still need to write code to set it up anyway.
But the thing is: in this case specifically we can just merge the two into a Node, we don´t need to keep the current Tween RefCount after porting it as a Node
why the new class literally autostarts and the whole issue is caused precisely because it starts without anyone deliberately saying so, triggering its removal and losing reference to it?
cmiiw this is the norm for tween behavior in other engines. DoTween in Unity is also auto freed when tween finished. Otherwise it will gives memory leak.
I think what you want from the old Tween Node is its reusability by not set the tween to be autokilled. And as a node, it will be freed when the node or its parent is freed.
So we need both..
But for the purpose of reusability of Tween, i think add ability to set off autokill would be better solution just like DoTween has Tween.SetAutoKill(false)
I can think of many use cases that justify using the Inspector, but for instance: connecting signals. Please avoid being irrational, of course we can do it from code. We can do anything we do from the Godot Editor from code so please avoid using this as justification.
But there are other things that justify it as a node, as I mentioned earlier, and especially on twitter, the current Tween is treated as a Node all over the place, yet, people here as trying to make a point that, besides it is treated as a Node, and we are time and time again working against the nature of its parent class, RefCounted, since we need for a natural usage of the Tweening to, yes @narredev , workaround the RefCounted behavior, people are fighting against turning it into a Node, even tho it is treated as a Node. See one of the properties that could be exposed to the Inspector is the Process Mode.
Isn't it intriguing that a class that isn't a Node has a processing mode? Because if you look at the documentation, only nodes and inheriting classes has the _process()
and the _physics_process()
callbacks. But not only that.
Something that could be useful is to tell via Inspector is whether the Tween should pause or keep processing when we pause the scene tree. Ohh, look, there isn't a way to tell that in the current implementation, because only Nodes have processing callbacks and can inherit their parent's Pause Mode, isn't it weird. So if I pause the scene tree I have to manually pause all Tweens? If it was a Node, it would naturally and very intuitively pause as well. But wait, the Tween class has a method to tell its Pause Mode? Let's see what it does:
Tween set_pause_mode(mode: TweenPauseMode)
Determines the behavior of the Tween when the SceneTree is paused. Check TweenPauseMode for options.
Default value is TWEEN_PAUSE_BOUND.
Hmm, so it essentially does exactly what a Node does, isn't this an argument against the idea of "Having multiple ways to do the same thing"? Because it seems like the current Tween implementation is just another way to make a RefCOunted object behave as a Node, while not making it a Node. But wait, in order for it to maintain itself working as a Node, in other word, to keep it in the scene tree even if it's idling, is to bind it to a Node...
So besides not being a Node, we should treat it like if it was, while working around the fact that it isn't but ideally we bind it to a Node to prevent the drawbacks of it not being a Node, and @narredev is telling me that it's better to do all that than to just turn this into a Node?
Seriously, can you guys stop for a second and see if what you are defending makes actual sense? Let's keep going.
Let's understand this bind_node()
method, because it looks like a replacement for just considering the Tween a child of the node it animates:
Binds this Tween with the given node. Tweens are processed directly by the SceneTree, so they run independently of the animated nodes. When you bind a Node with the Tween, the Tween will halt the animation when the object is not inside tree and the Tween will be automatically killed when the bound object is freed. Also TWEEN_PAUSE_BOUND will make the pausing behavior dependent on the bound node.
Hmm, intriguing right, it does sounds like if we were just using the Tween as a child of the target Node, besides it isn't a child. But... without using the bind_node()
what happens if I Tween a node that gets freed from the SceneTree in the middle of an interpolation? Let's test this out.
Well I tried to do that:
extends Node2D
var tween
func _init():
tween = create_tween()
tween.stop()
func _ready():
var tweener = tween.tween_property($Sprite2D, "modulate", Color.BLUE, 3.0)
tweener.from(Color.RED)
func _on_timer_timeout():
queue_free()
But it gives me an error saying I can't call stop()
on a null instance, which makes me think I can't create a tween in the _init()
callback, Let's see if I can make this work in the _ready()
callback.
Well, it seems that it worked! So besides @narredev said we don't need to create tweens after the _ready callback or using @onready
Godot Engine seems to disagree with him, this code works.
extends Node2D
var tween
func _ready():
tween = create_tween()
tween.stop()
var tweener = tween.tween_property($Sprite2D, "modulate", Color.BLUE, 3.0)
tweener.from(Color.RED)
func _on_timer_timeout():
queue_free()
But we have an issue now, because it's not interpolating the color. Ohh, my bad, it is stopped right? So I should resume it.
extends Node2D
var tween
func _ready():
tween = create_tween()
tween.stop()
var tweener = tween.tween_property($Sprite2D, "modulate", Color.BLUE, 3.0)
tweener.from(Color.RED)
tween.play()
func _on_timer_timeout():
queue_free()
That works! But let's say that I want do do this tweeting every time I perform a click.
extends Node2D
var tween
func _ready():
tween = create_tween()
tween.stop()
var tweener = tween.tween_property($Sprite2D, "modulate", Color.BLUE, 3.0)
tweener.from(Color.RED)
func _unhandled_input(event):
if Input.is_mouse_button_pressed(MOUSE_BUTTON_LEFT):
tween.play()
func _on_timer_timeout():
queue_free()
What a strange thing, now I got an error saying there's no tween, where did I tell to free it from memory, why isn't it the if I explicitly told it I would use it every time I press the left mouse button, why there's no reference to it anymore? Where did my Tween go? What's happening? After the second click, I can't tween anymore? I didn't even reach the point where the $Sprite2D is freed so I don't even know if there will be an error.
Well, let's see how we could do that in 3.5:
I tried this at first:
extends Node2D
onready var tween = $Tween
func _ready():
tween.interpolate_property($Sprite, "modulate", Color.red, Color.blue, 3.0)
func _unhandled_input(event):
if Input.is_mouse_button_pressed(BUTTON_LEFT):
tween.start()
func _on_Timer_timeout():
$Sprite.queue_free()
But seems like after the second click it doesn't work. So...I think intuitively that the reason for that is that I'm not calling the interpolation method again, so let's try this out:
extends Node2D
onready var tween = $Tween
func _unhandled_input(event):
if Input.is_mouse_button_pressed(BUTTON_LEFT):
tween.interpolate_property($Sprite, "modulate", Color.red, Color.blue, 3.0)
tween.start()
func _on_Timer_timeout():
$Sprite.queue_free()
Beautiful, it works perfectly. Let's try to do the same thing on 4.0
extends Node2D
var tween
func _ready():
tween = create_tween()
tween.stop()
func _unhandled_input(event):
if Input.is_mouse_button_pressed(MOUSE_BUTTON_LEFT):
var tweener = tween.tween_property($Sprite2D, "modulate", Color.BLUE, 3.0)
tweener.from(Color.RED)
tween.play()
func _on_timer_timeout():
queue_free()
Well since I don't have a tween anymore it doesn't return a Tweener so I try to access a null instance. WHich is weird
If you note in the debugger the variable that refers to the Tween still filled with an object of type RefCounted, which I presume it's supposed to be the Tween we created?
Something even more interesting is the fact that I can call tween.tween_property()
it doesn't say that I'm calling a method on a null reference, which is weird because as we saw previously the tween doesn't exist anymore after the first tweeting. But even tho I'm technically calling the method on something, the method returns null, so I do have a reference to the original tween it is just not working as it should.
Now, besides this code is not working, let's make a comparison regarding the two versions, 3.5 and 4.0, because @narredev said there's no difference between them and they are both the same thing. I disagree, but maybe I have some difficulty trying to accurately access them. So let's use a diff tool to see if there's something different between these two codes, remember the 4.0 isn't even working:
Well, this online diff tool points out that there are MANY differences in these codes. But maybe me, Godot Engine, and all the developers involved in the developed of this tool are missing something that @narredev can see.
But anyway, I will reinforce the fact that the current Tween approach tries, poorly, to mimic as a Node should behave while trying its best to not being a Node.
Question is, and I will mention @seppoday and @produno why keep maintaining a system that tries to mimic the utility of the previous system while breaking the very design of the engine just to try to be somehow like another tool, like @mikhaelmartin suggests making a comparison with Unity, but many people also suggested that this is more "standard" with other tools forgetting that the proposal of Godot Engine is not to look like nor to follow standards of other frameworks. If I want to make things in a Unity-ish way, I can just use Unity.
Anyway, this whole discussion is getting noised by people that can't understand these three points:
Seconding @seppoday. I haven't needed a persistent tween in a while, but last time I tried, I had issues with keeping one tween always available.
Maybe we should have a
set_persistent()
method to just mark the tween itself not to be auto-removed after it has no more tweeners? Would that cover everything people need from the old tween?
What about setting process mode through inspector? Pause mode through inspector? Ease through inspector? Transition type through inspector? Connecting signals through inspector? Maybe allowing us to add tweens to groups instead of adding meta properties to them? What about the visible structure that having a Tween node provides to understand its order in the hierarchy? Why creating a work around method to make something "persistent" if the core design of its parent class(RefCount) is to not be persistent?
why keep maintaining a system that tries to mimic the utility of the previous system while breaking the very design of the engine
I think the point being is that there was already a PR and discussion regarding changing the system to how it currently is. Why do we need another PR and discussion to now change it back? Are we going to open another PR and discussion from someone else in 6 months time to change it back again to the current system? I think that ship has already sailed. Which is why it may be better to focus efforts on improving the current system and adding anything you feel is missing.
In your example above you are creating the tween in on_ready. Shouldn't you be creating it just before you use it? Then when finished you check it no longer exists and then create a new one when needed.
why keep maintaining a system that tries to mimic the utility of the previous system while breaking the very design of the engine
I think the point being is that there was already a PR and discussion regarding changing the system to how it currently is. Why do we need another PR and discussion to now change it back? Are we going to open another PR and discussion from someone else in 6 months time to change it back again to the current system? I think that ship has already sailed. Which is why it may be better to focus efforts on improving the current system and adding anything you feel is missing.
I feel like it's missing being an actual node, and the very person that implemented it felt it as well:
pause_mode
process
es and a process_mode
Binds this Tween with the given node. Tweens are processed directly by the SceneTree, so they run independently of the animated nodes. When you bind a Node with the Tween, the Tween will halt the animation when the object is not inside tree and the Tween will be automatically killed when the bound object is freed. Also TWEEN_PAUSE_BOUND will make the pausing behavior dependent on the bound node.
@henriiquecampos
I can think of many use cases that justify using the Inspector, but for instance: connecting signals.
The ability to connect tween's signal via inspector alongside its presistent lifetime should be enough reason for Tween Node. You should add it to original post.
What a strange thing, now I got an error saying there's no tween, where did I tell to free it from memory, why isn't it the if I explicitly told it I would use it every time I press the left mouse button, why there's no reference to it anymore? Where did my Tween go? What's happening? After the second click, I can't tween anymore? I didn't even reach the point where the $Sprite2D is freed so I don't even know if there will be an error.
Looking all of your examples, i think you misunderstood of how the new tween system works. Tween is auto started right after it's created and auto killed after it finished. It is very different from the old tween system works. I know it may feel weird for you, but i think this behaviour is the norm in other engines (cmiiw)
@henriiquecampos
I can think of many use cases that justify using the Inspector, but for instance: connecting signals.
The ability to connect tween's signal via inspector alongside its presistent lifetime should be enough reason for Tween Node. You should add it to original post.
What a strange thing, now I got an error saying there's no tween, where did I tell to free it from memory, why isn't it the if I explicitly told it I would use it every time I press the left mouse button, why there's no reference to it anymore? Where did my Tween go? What's happening? After the second click, I can't tween anymore? I didn't even reach the point where the $Sprite2D is freed so I don't even know if there will be an error.
Looking all of your examples, i think you misunderstood of how the new tween system works. Tween is auto started right after it's created and auto killed after it finished. It is very different from the old tween system works. I know it may feel weird for you, but i think this behaviour is the norm in other engines (cmiiw)
I didn't misunderstood at, at least not after making them work as I point out in my original proposal. @narredev said they didn't automatically start, tho.
I didn't misunderstood at, at least not after making them work as I point out in my original proposal. @narredev said they didn't automatically start, tho.
I think you and narradev are a little lost in translation. They don't auto-start in the sense that they are not controllable. They auto start as soon as they are created. That is why you create them just before you need them. Which is also why they don't, or shouldn't be created in on_ready unless you also use them in on_ready.
I didn't misunderstood at, at least not after making them work as I point out in my original proposal. @narredev said they didn't automatically start, tho.
That works! But let's say that I want do do this tweeting every time I perform a click.
This is how you do it with the new system.
extends Node2D
var tween:Tween
func _unhandled_input(event):
if Input.is_mouse_button_pressed(MOUSE_BUTTON_LEFT):
if tween:
tween.kill()
tween = create_tween()
var tweener = tween.tween_property($Sprite2D, "modulate", Color.BLUE, 3.0)
tweener.from(Color.RED)
I don't think the main problem with Tween
when moving from 3.x to 4.x is that it's not a node. Although certainly some of the changes in Tween
's design are due to the fact that it's no longer Node
. Or rather it's no longer Node
, because it makes more sense for the new design.
If we imagine that Tween
is a node in 4.x, but all other changes are applied, would you say that your main problem is solved and only minor issues remain? I think not, because:
Tween
invalidation when the animation is completed (which prevents the old object from being reused and raises the question of what happens when one property is animated by several Tween
s).Tween
. Yes, this is related to Node
vs RefCounted
, but your work with Tween
between the time it is created and deleted does not depend on whether it is a node or not. Tween
is designed for animations in code, unlike AnimationPlayer
, you do not need to change any properties in the Inspector.Godot users are divided into those who use $Node
in function bodies and those who store it in an @onready
variable. And also for those who adhere to the "everything is a node" approach (including logic/data), and those who prefer Resource
/RefCounted
. Maybe $Tween
seems simpler than variable management and calling kill()
before create_tween()
. But it is not as inconvenient as it might seem.
I didn't misunderstood at, at least not after making them work as I point out in my original proposal. @narredev said they didn't automatically start, tho.
That works! But let's say that I want do do this tweeting every time I perform a click.
This is how you do it with the new system.
extends Node2D var tween:Tween func _unhandled_input(event): if Input.is_mouse_button_pressed(MOUSE_BUTTON_LEFT): if tween: tween.kill() tween = create_tween() var tweener = tween.tween_property($Sprite2D, "modulate", Color.BLUE, 3.0) tweener.from(Color.RED)
Right, but what if I want to stop the the previous interpolation and start a new one using a new value from what the previous one stopped? Something like this(in Godot 3.5):
extends Node2D
onready var sprite = $Sprite
onready var tween = $Tween
onready var timer = $Timer
func _unhandled_input(event):
if Input.is_mouse_button_pressed(BUTTON_LEFT):
tween.stop(sprite, "modulate")
tween.interpolate_property(sprite, "modulate", Color.red, Color.green, 3.0)
tween.start()
timer.start()
func _on_Timer_timeout():
tween.stop(sprite, "modulate")
var current_color = sprite.modulate
tween.interpolate_property(sprite, "modulate", current_color, Color.blue, 3.0)
I'm using colors, but could be anything really, positions for instance.
@henriiquecampos
Right, but what if I want to stop the the previous interpolation and start a new one using a new value from what the previous one stopped?
you need to kill and create tween everytime you need it
extends Node2D
var tween:Tween
@onready var sprite = $Sprite
@onready var timer = $Timer
func _unhandled_input(event):
if Input.is_mouse_button_pressed(MOUSE_BUTTON_LEFT):
if tween:
tween.kill()
tween = create_tween()
tween.tween_property(sprite, "modulate", Color.GREEN, 3.0).from(Color.RED)
timer.start()
func _on_timer_timeout():
if tween:
tween.kill()
# killing tween does not reset tweened property's value,
# and new tween always start from current value by default
tween = create_tween()
tween.tween_property(sprite, "modulate", Color.BLUE, 3.0)
edit: oops.. forget to start timer
I strongly disagree with this proposal. There is no reason for Tween to be a Node as nothing is exposed to the editor. It is purely used in code. Same reason there is no AStar3D node, no Thread node, no Crypto node. It would serve no purpose existing on the Scene Tree instead of simply as a reference in a script.
Describe the project you are working on
I'm working on a Multiplayer Online Adventure game implementing interpolation techniques for lag compensation.
Describe the problem or limitation you are having in your project
The current Tween is way more intricate compared to the previous approach and brings up unnecessary steps to achieve things previously trivial, such as accessing and stopping the Tween. For instance, this doesn't work as it doesn't create a tweener for some reason:
But this works:
And since the tween reference "only exists" while the function runs, I can't do this:
Instead I need to do this:
Which stops other tweens not related to this behavior, so I need to add some filtering to find the actual reference to the tween I want.
On top of that, this new approach breaks the engine's design of providing NODES as building blocks for creating games with Godot Engine, so a Tween node would keep the design consistent, as it was in 3.x.
Describe the feature / enhancement and how it helps to overcome the problem or limitation
With a Tween node I could just do this instead:
Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams
The idea is to turn the current Tween into a node that would expose some properties in the Inspector, making it easier for us to configure some settings, such as the process mode, duration, ease, transition, etc...
If this enhancement will not be used often, can it be worked around with a few lines of script?
Tweens are fundamental for game development, so this would be used often. Especially because now we have access to
Tweener
s these two classes would make things very versatile and would be used often.The workaround for the current approach is to create a whole Node that performs exactly what the previous Tween node did in version 3.x, but is now implemented manually which can cause lots of issues since not every user, me included, knows the proper approach to implement this manually. You guys are the best folks to implement a feature...that you guys already implemented previously.
Is there a reason why this should be core and not an add-on in the asset library?
Yeah, it's part of the engine's core design to use Nodes, not intricate functions as if we were developing in FP. On top of that this feature was in the previous versions of Godot Engine, so technically this is a regression of the core engine's code.
Edit by the production team: added syntax highlighting to code samples and changed tabs to spaces