Improve SceneTreeTween API

[TheDuriel]

Describe the project you are working on

Not US, a Card Based roguelike puzzler survival thingy. Almost all our Animation is done trough Tweens.

Describe the problem or limitation you are having in your project

The SceneTreeTween currently supports "chaining" as a feature. Which is way to link sequential function calls on the same object, by making those functions return the object they are operating on. Ex.: var t = tdo().do().do().do()

The API documentation heavily encourages usage of this pattern, arguing that it is a convenient way of calling many of SceneTreeTweens initialization functions.

I argue that this incentivizes poor code style practices and confuses newcomers. It also offers too many way to achieve the same thing, with no enrichment in functionality. Creating a discussion about "what is best", where each solution offered only boils down to character count.

This is additionally problematic because this is the only time the Godot API does this. Breaking consistency with similar systems (where an object is created and then configured.) like for example the RayCast parameters.

Here are some examples of how users are currently encouraged to write code:

Fully chained. I believe that this is what many less experienced or disciplined coders may write. (I have seen this in my own team and tutorials.)

var t: SceneTreeTween = get_tree().create_tween().bind_node(self).set_ease(Tween.EASE_IN).set_trans(Tween.TRANS_LINEAR).tween_property(self, "position", Vector2(69, 420), 5.0)

This is poor because it easily exceeds the available visible horizontal space in any code editor. There already exists a long history of well studied discourse about how long lines are hard to read and understand.

No extraneous chaining. This is how I currently use the system in my own project. This has the downside of, if they are enabled, creating numerous warnings about discarded return values. Necessitating doubling the line count if you wish to ignore those warnings.

var t: SceneTreeTween = get_tree().create_tween()
t.bind_node(self)
t.set_ease(Tween.EASE_IN)
t.set_trans(Tween.TRANS_LINEAR)
t.tween_property(self, "position", Vector2(69, 420), 5.0)

**Below this are several examples I have gotten on how to "better format" these API calls.

What twitter thinks you should do to avoid said warnings.

var t: SceneTreeTween = get_tree().create_tween()
var _ = t.bind_node(self)
_ = t.set_ease(Tween.EASE_IN)
_ = t.set_trans(Tween.TRANS_LINEAR)
_ = t.tween_property(self, "position", Vector2(69, 420), 5.0)

What twitter thinks you should do to make chaining pretty.

var t: SceneTreeTween = get_tree().create_tween()\
.bind_node(self).set_ease(Tween.EASE_IN).set_trans(Tween.TRANS_LINEAR)\
.tween_property(self, "position", Vector2(69, 420), 5.0)

What twitter thinks also makes it pretty.

var t: SceneTreeTween = (get_tree().create_tween()
.bind_node(self)
.set_ease(Tween.EASE_IN)
.set_trans(Tween.TRANS_LINEAR)
.tween_property(self, "position", Vector2(69, 420), 5.0))

Describe the feature / enhancement and how it helps to overcome the problem or limitation

I propose that we remove the chaining where possible, through attacking other issues.

For example, currently bind_node() set_ease() set_trans() and set_parallel() among others are function calls. However these are simply properties.

I propose that properties are exposed as such, and their setters deprecated.

This achieves two things. It makes the API more consistent with other parts of Godot. We set properties instead of calling setters. And it effectively removes/discourages chaining by removing the methods which most commonly cause that behavior.

Is this a breaking change? It can be. And an implementation that breaks existing code would thus likely never be implemented. (Or indefinitely be pushed until Godot 5.)

But it is not required to be.

Existing setter functions can be kept, including their return value. While at the same time properties can be exposed and documentation can be updated to encourage their usage.

This is not too dissimilar to past changes to the engine. (Remember when we got rid of basically all the setters and getters in 3.0 and turned everything into properties? :P)

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Compare my first code examples with this new version:

var t: SceneTreeTween = get_tree().create_tween()
t.owner = self # Formerly bind_node(self)
t.ease = Tween.EASE_IN
t.trans = Tween.TRANS_LINEAR
t.tween_property(self, "position", Vector2(69, 420), 5.0)

Another way to address this would be through (optional?) initialization parameters like this: (This was mentioned to me by an engine contributor.)

var t: SceneTreeTween = get_tree().create_tween(self, Tween.EASE_IN, Tween.TRANS_LINEAR)
t.tween_property(self, "position", Vector2(69, 420), 5.0)

This is how I would personally format the above:

var t: SceneTreeTween = get_tree().create_tween(
        self, Tween.EASE_IN, Tween.TRANS_LINEAR)
t.tween_property(self, "position", Vector2(69, 420), 5.0)

If this enhancement will not be used often, can it be worked around with a few lines of script?

Affects Godot API.

Is there a reason why this should be core and not an add-on in the asset library?

This affects core API usage. While any developer may implement their own wrapper around the Tween functions. I do not believe 'hiding' it is an effective solution overall. It does not improve tutorials, understanding, or ease of use.

[dalexeev]

This is how I currently use the system in my own project. This has the downside of, if they are enabled, creating numerous warnings about discarded return values.

In 4.0, the RETURN_VALUE_DISCARDED warning is disabled by default.

[TheDuriel]

And I want it on. For those cases where it matters.

[KoBeWi]

Well, personally I use mixed chaining:

var t: SceneTreeTween = create_tween().set_ease(Tween.EASE_IN) # TRANS_LINEAR is default
t.tween_property(self, "position", Vector2(69, 420), 5.0).set_delay(2)

(btw create_tween() is also a method of Node)

The advantage of chaining is that it avoids 8-argument methods like we had in the old system. Named arguments would be nice replacement probably, but they are not supported. (also the Tween system is inspired by DOTween, which is the main reasy why it's designed like this)

Note that Tween properties are write-only, so adding getters is rather redundant. Also setup using properties like you suggest is very verbose; a code stretched out on multiple lines can also be considered less readable. So I think chaining should still be supported (not deprecated) even if we decide to add property access.

Would it be better if GDScript supported proper multiline chaining?

var t: SceneTreeTween = get_tree().create_tween()
    .bind_node(self)
    .set_ease(Tween.EASE_IN)
    .set_trans(Tween.TRANS_LINEAR)
    .tween_property(self, "position", Vector2(69, 420), 5.0)

[TheDuriel]

(btw create_tween() is also a method of Node)

My project is using 3.x for the moment. But the get_tree() call really isn't part of the discussion here anyways :P (Also still need to do something similar if you're not using a Node.)

Would it be better if GDScript supported proper multiline chaining?

If it was a proper feature with multiple use cases across the API so it's not a one of. Sure that could work.

Note that Tween properties are write-only, so adding getters is rather redundant.

Why is this different from PhysicsRayQueryParameters2D?

[KoBeWi]

My project is using 3.x for the moment.

It exists in 3.x too.

Why is this different from PhysicsRayQueryParameters2D?

Well, I think it could also be write-only, because it's an object you create just to pass some parameters (obviously too late to change it). Although the usage is a bit different and you can use it multiple times too 🤔 Regardless of consistency, I'm just saying you never need to read these properties.