Skip to content

Add Tween::has_tweeners()#92429

Merged
Repiteo merged 1 commit into
godotengine:masterfrom
Daylily-Zeleen:daylily-zeleen/add_tween_has_tweenders
Mar 12, 2026
Merged

Add Tween::has_tweeners()#92429
Repiteo merged 1 commit into
godotengine:masterfrom
Daylily-Zeleen:daylily-zeleen/add_tween_has_tweenders

Conversation

@Daylily-Zeleen
Copy link
Copy Markdown
Contributor

@Daylily-Zeleen Daylily-Zeleen commented May 27, 2024

Add Tween::has_tweeners().
So we can check and kill a tween if it has not contains any tweener before it step by scene tree and avoid print an error.

@Daylily-Zeleen Daylily-Zeleen requested a review from a team as a code owner May 27, 2024 13:37
Comment thread scene/animation/tween.cpp Outdated
Comment thread scene/animation/tween.cpp Outdated
@Daylily-Zeleen Daylily-Zeleen force-pushed the daylily-zeleen/add_tween_has_tweenders branch from 28f02cc to 951df8e Compare May 27, 2024 13:56
@Daylily-Zeleen Daylily-Zeleen requested a review from a team as a code owner May 27, 2024 13:56
Comment thread doc/classes/Tween.xml Outdated
@Daylily-Zeleen Daylily-Zeleen force-pushed the daylily-zeleen/add_tween_has_tweenders branch from 951df8e to 71eae05 Compare May 27, 2024 14:09
Comment thread doc/classes/Tween.xml Outdated
@Daylily-Zeleen Daylily-Zeleen force-pushed the daylily-zeleen/add_tween_has_tweenders branch from 71eae05 to 741ea10 Compare May 27, 2024 14:11
Comment thread doc/classes/Tween.xml Outdated
@Daylily-Zeleen Daylily-Zeleen force-pushed the daylily-zeleen/add_tween_has_tweenders branch from 741ea10 to 8915b2f Compare May 27, 2024 14:13
Comment thread doc/classes/Tween.xml Outdated
@Daylily-Zeleen Daylily-Zeleen force-pushed the daylily-zeleen/add_tween_has_tweenders branch from 8915b2f to 86e6f89 Compare May 27, 2024 14:20
Comment thread scene/animation/tween.h Outdated
@Daylily-Zeleen Daylily-Zeleen force-pushed the daylily-zeleen/add_tween_has_tweenders branch from 86e6f89 to e779320 Compare June 10, 2024 15:12
Copy link
Copy Markdown
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM code wise

@timothyqiu timothyqiu requested a review from KoBeWi June 10, 2024 17:08
@KoBeWi
Copy link
Copy Markdown
Member

KoBeWi commented Jun 10, 2024

So we can check and kill a tween if it has not contains any tweener before it step by scene tree and avoid print an error.

I think this should be mentioned in the description, possibly with some simple code sample.

Although I'm not sure how exactly is that useful. The "Tween has no Tweeners" error is most often a result of a mistake. If you add a code to safeguard against it, you might as well ensure that your logic can't lead to such situation. Unless it's meant for code that conditionally adds Tweeners and empty Tween is an expected result?

@Daylily-Zeleen
Copy link
Copy Markdown
Contributor Author

Unless it's meant for code that conditionally adds Tweeners and empty Tween is an expected result?

This is my case. Create one tween only and pass it to multiple objects, each object adds tweener conditionally.

@Daylily-Zeleen Daylily-Zeleen force-pushed the daylily-zeleen/add_tween_has_tweenders branch 2 times, most recently from ce08510 to cc645c6 Compare June 11, 2024 04:02
Comment thread doc/classes/Tween.xml Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it mention tween validity if it's not checked? (the valid variable)
Also the second sentence likely needs rewording, because it's a bit unclear.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it mention tween validity if it's not checked? (the valid variable)

This description is changed multiple time, the mention about validity seems redundant now.

Also the second sentence likely needs rewording, because it's a bit unclear.

To be honest, my English is terrible. Could you provide a suitable description to help me?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Returns [code]true[/code] if any [Tweener] has been added and the tween is valid. Useful for avoiding print an error by checking and killing the [Tween] if it has not tweeners before it is stepped by [SceneTree].
Returns [code]true[/code] if any [Tweener] has been added to the tween. Useful when tweeners are added dynamically and the tween can end up empty. Killing an empty tween before it starts will prevent errors.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks :)

Copy link
Copy Markdown
Contributor Author

@Daylily-Zeleen Daylily-Zeleen Jun 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, the validation description is can not removed.
Event if the tween has been added twenners, the vector tweeners will be cleared with valid variable, too.

@KoBeWi please check again.

@Daylily-Zeleen Daylily-Zeleen force-pushed the daylily-zeleen/add_tween_has_tweenders branch from cc645c6 to 11543c4 Compare June 11, 2024 15:07
@Daylily-Zeleen Daylily-Zeleen force-pushed the daylily-zeleen/add_tween_has_tweenders branch from 11543c4 to 5809a67 Compare June 11, 2024 15:14
@TokageItLab TokageItLab modified the milestones: 4.x, 4.7 Mar 7, 2026
@Repiteo Repiteo merged commit 020df5d into godotengine:master Mar 12, 2026
@Repiteo
Copy link
Copy Markdown
Contributor

Repiteo commented Mar 12, 2026

Thanks!

@Meorge
Copy link
Copy Markdown
Contributor

Meorge commented Mar 12, 2026

I believe this should close godotengine/godot-proposals#13266 . There is discussion on that thread about use cases that this PR wouldn't cover, but I think those can be addressed with a separate proposal.

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.

7 participants