godotengine / godotengine/godot
Tween.is_valid() doesn't work on a null value - but Tweens are expected to be <null>
- 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
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