godotengine / godotengine/godot

Tween.is_valid() doesn't work on a null value - but Tweens are expected to be <null>

Open
#90,281 4 comments 5 reactions 0 assignees View on GitHub
discussion topic:core
Dominant language
C++
Stars
117k
Forks
26.8k
PR merge metrics
PR metrics pending

Description

### Tested versions

- Reproducible in all Godot 4.0 builds as far as I can tell, Including specifically 4.0-stable, 4.2.1-stable, 4.3-dev5

### System information

Godot v4.2.1.stable - Void #1 (Linux)

### Issue description

We'll start here: I have a class that creates a number of instances. I would like to be able to animate each instance separately when needed. To this end, the class has a `tween` variable, so that x.tween can be animated separately from y.tween. Sometimes these animations need to be ended early.

Following the docs, it is suggested to use `if tween: tween.kill()` to stop animations if needed. This works really well if it's the first statement within the Tween's scope, because `var tween: Tween` starts as ``. It becomes tricky when this is no longer the case.

As far as I can tell: Tweens are returned to `` when the scene changes or the node is freed, but if neither happens the object just sticks around.

`tween.is_valid()` is incomplete because of this, because it will not work on a null value - which happens regularly to tweens. `if tween:` is also incomplete on its own as any tween that has been killed but not removed from scope returns `true`.

A brief example where tween is expected to be gone, but comes up true:
```GDscript
#Tween is killed but still in scope
if not tween: #tween still referenced, returns true
tween = create_tween()
tween_stuff()
#no tween has been created
await tween.finished
```
A brief example where tween is expected to not exist, but does:
```GDscript
if true:
tween.kill()
if tween: #tween still exists, so this is true
await tween.finished #nothing to wait for
#code expecting a finished tween
#Unreachable code point
```

A brief example where tween.is_valid() doesn't work:
```GDscript
var tween: Tween

func main():
if not tween.is_valid():
tween = create_tween()
```
I think either problem would be fine alone, but together they're quite bothersome.

I can think of some solutions that would work well:

1) tween.kill() also returns the variable and attached tweeners to ``. I think this fits best with the current documentation, and with the idea that Tweens aren't really supposed to be reused, so the variable is just there for the developer.

2) `tween.is_valid()` returns false for anything that's not a valid tween. As it is, I can't think of any uses-cases for `is_valid()` where you wouldn't want it to also to return false for a null value.

### Steps to reproduce

I've included a really simple MRP, but I'm sure you'd be able to reproduce without. A class with a single variable `tween`, a method to interrupt tweening, and a method to rotate said class. Then another script for the scene that consists of a _ready() function that creates two instances of the class, gives them the icon.svg and a position. Rotates both, interrupts one of the instances, tries to rotate it again, and then awaits both finishing.

The first icon will finish the complete animation. The second will not, and displays a message "Tween is already rotating" when it is not rotating, and demonstrates an unreachable codepoint by trying to `await` the killed tween. **Just press play**. And then manually end it when you get bored, because the await cannot reach the endpoint.

This MRP does not cover the `is_valid()` issue - but it can be replicated by simply using `is_valid()` before a tween is created.

### Minimal reproduction project (MRP)

[tween.zip](https://github.com/godotengine/godot/files/14890681/tween.zip)

I know this is really simple, but that should demonstrate how quickly this can cause issues

Contributor guide

Open the contributing guide

Research direction

Start with the attached tween.zip MRP and the Tween.is_valid(), Tween.kill(), and finished entry points; press Play to reproduce the stale-reference behavior. Compare the documented lifecycle with the two proposed behaviors, then confirm the selected behavior against the null, killed-tween, and await examples.

Written by the indexing model from the issue text.

Assessment

Domain
game-dev
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.