Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a note about SceneTree.create_tween() method #81087

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

mateuseap
Copy link
Contributor

@mateuseap mateuseap commented Aug 28, 2023

What I did

  • I've added a note about create_tween() method of SceneTree class.

Closes: #7796 (issue from godot-docs repo)

@@ -6,7 +6,8 @@
<description>
Tweens are mostly useful for animations requiring a numerical property to be interpolated over a range of values. The name [i]tween[/i] comes from [i]in-betweening[/i], an animation technique where you specify [i]keyframes[/i] and the computer interpolates the frames that appear between them. Animating something with a [Tween] is called tweening.
[Tween] is more suited than [AnimationPlayer] for animations where you don't know the final values in advance. For example, interpolating a dynamically-chosen camera zoom value is best done with a [Tween]; it would be difficult to do the same thing with an [AnimationPlayer] node. Tweens are also more light-weight than [AnimationPlayer], so they are very much suited for simple animations or general tasks that don't require visual tweaking provided by the editor. They can be used in a fire-and-forget manner for some logic that normally would be done by code. You can e.g. make something shoot periodically by using a looped [CallbackTweener] with a delay.
A [Tween] can be created by using either [method SceneTree.create_tween] or [method Node.create_tween]. [Tween]s created manually (i.e. by using [code]Tween.new()[/code]) are invalid and can't be used for tweening values.
A [Tween] can be created by using either [method SceneTree.create_tween] or [method Node.create_tween]. [Tween]s created manually (i.e. by using [code]Tween.new()[/code]) are invalid and can't be used for tweening values.
[b]Note:[/b] When creating a [Tween] by using [method SceneTree.create_tween] the [Tween] will be detached from the actual [Node] and it won't be killed when the [Node] is freed. Prefer using [method Node.create_tween] if you want the [Tween] to be automatically killed when the [Node] is freed.
Copy link
Member

Choose a reason for hiding this comment

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

Move this below to the other "Note" cases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! I've moved this note to the end of the description section, where the other notes are located.

@mateuseap mateuseap changed the title Add an alert in Tween class description Add a note in Tween class description Aug 28, 2023
@mateuseap mateuseap force-pushed the docs/tweenClassReference branch 2 times, most recently from 38a8698 to 639e527 Compare August 28, 2023 21:37
@mateuseap
Copy link
Contributor Author

@AThousandShips I've adjust the PR name and description, squashed all the commits and adjusted the commit message!

@akien-mga akien-mga changed the title Add a note in Tween class description Add a note about Tween creation in its class description Aug 28, 2023
@akien-mga
Copy link
Member

Like for #81088 (comment), I'd ask to amend the commit message to remove the Conventional Commit boilerplate which is not the format we use in this repo. This PR's title as we adjusted it would make a fine descriptive commit title.

@akien-mga akien-mga modified the milestones: 4.x, 4.2 Aug 28, 2023
@mateuseap mateuseap force-pushed the docs/tweenClassReference branch from 639e527 to 26f8207 Compare August 28, 2023 22:03
@akien-mga
Copy link
Member

Final nitpick: in the body of the commit message, you have a lot of past commit messages which are not relevant to the final state of this change:

image

I would edit it to be simply:

Add a note about Tween creation in its class description

Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>

@mateuseap mateuseap force-pushed the docs/tweenClassReference branch from 26f8207 to 99dabaf Compare August 29, 2023 08:18
@mateuseap
Copy link
Contributor Author

mateuseap commented Aug 29, 2023

@akien-mga done! I forgot to adjust it earlier, but now the commit description looks good. Thanks for the patient and all the help :)

image

@mateuseap mateuseap force-pushed the docs/tweenClassReference branch from 99dabaf to 08892d7 Compare August 29, 2023 08:31
@akien-mga
Copy link
Member

Looks good!

No need to change it further, but FYI, technically that commit message still has a redundant title both in the title field (first line) and the first line of the commit body (third line).

In the git log it will look like this:

Add a note about Tween creation in its class description

Add a note about Tween creation in its class description.

Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>

@akien-mga akien-mga requested a review from KoBeWi August 29, 2023 08:34
@@ -91,6 +91,7 @@
[url=https://raw.githubusercontent.com/godotengine/godot-docs/master/img/tween_cheatsheet.webp]Tween easing and transition types cheatsheet[/url]
[b]Note:[/b] Tweens are not designed to be re-used and trying to do so results in an undefined behavior. Create a new Tween for each animation and every time you replay an animation from start. Keep in mind that Tweens start immediately, so only create a Tween when you want to start animating.
[b]Note:[/b] The tween is processed after all of the nodes in the current frame, i.e. node's [method Node._process] method would be called before the tween (or [method Node._physics_process] depending on the value passed to [method set_process_mode]).
[b]Note:[/b] When creating a [Tween] by using [method SceneTree.create_tween] the [Tween] will be detached from the actual [Node] and it won't be killed when the [Node] is freed. Prefer using [method Node.create_tween] if you want the [Tween] to be automatically killed when the [Node] is freed.
Copy link
Member

Choose a reason for hiding this comment

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

This is not entirely true. When a Node is freed, PropertyTweeners that target it will end automatically and Tween will kill itself if it has no Tweeners to animate. I think if Callable's get_object() return value is reliable, we could add a similar safeguard to MethodTweener and CallbackTweener to prevent errors.
By "not entirely true" I mean that Tween will continue to animate, but it will finish if there is nothing to animate (this is mostly relevant if you are also animating other objects).

Also shouldn't this note be in SceneTree.create_tween() description instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@KoBeWi I've rewritten the note and moved it to SceneTree class reference file

@mateuseap mateuseap force-pushed the docs/tweenClassReference branch from 08892d7 to efe7199 Compare August 31, 2023 18:38
@mateuseap mateuseap changed the title Add a note about Tween creation in its class description Add a note about create_tween() method of Scene class Aug 31, 2023
@mateuseap mateuseap changed the title Add a note about create_tween() method of Scene class Add a note about create_tween() method of SceneTree class Aug 31, 2023
@mateuseap mateuseap force-pushed the docs/tweenClassReference branch from efe7199 to 4447c22 Compare August 31, 2023 18:40
@@ -90,6 +90,7 @@
<return type="Tween" />
<description>
Creates and returns a new [Tween]. The Tween will start automatically on the next process frame or physics frame (depending on [enum Tween.TweenProcessMode]).
[b]Note:[/b] When creating a [Tween] using [method SceneTree.create_tween], the [Tween] will be detached from the actual [Node]. It will continue to animate even if the [Node] is freed, but it will automatically finish if there's nothing left to animate. If you want the [Tween] to be automatically killed when the [Node] is freed, prefer using [method Node.create_tween].
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[b]Note:[/b] When creating a [Tween] using [method SceneTree.create_tween], the [Tween] will be detached from the actual [Node]. It will continue to animate even if the [Node] is freed, but it will automatically finish if there's nothing left to animate. If you want the [Tween] to be automatically killed when the [Node] is freed, prefer using [method Node.create_tween].
[b]Note:[/b] When creating a [Tween] using this method, the [Tween] will be detached from the actual [Node]. It will continue to animate even if the [Node] is freed, but it will automatically finish if there's nothing left to animate. If you want the [Tween] to be automatically killed when the [Node] is freed, prefer using [method Node.create_tween].

No need to mention the method itself here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, mention removed

@mateuseap mateuseap force-pushed the docs/tweenClassReference branch from 4447c22 to c172d1f Compare August 31, 2023 18:57
@mateuseap mateuseap changed the title Add a note about create_tween() method of SceneTree class Add a note about SceneTree.create_tween() method Aug 31, 2023
doc/classes/SceneTree.xml Outdated Show resolved Hide resolved
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

One last comment, otherwise looks ok.

Add note regarding SceneTree.create_tween() method behavior.

Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: Tomek <kobewi4e@gmail.com>
@mateuseap mateuseap force-pushed the docs/tweenClassReference branch from ec33813 to 5030549 Compare August 31, 2023 20:50
@akien-mga akien-mga merged commit fa3072f into godotengine:master Sep 1, 2023
@akien-mga
Copy link
Member

Thanks!

@mateuseap mateuseap deleted the docs/tweenClassReference branch September 3, 2023 20:32
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.

Alert about the risks of using SceneTree.create_tween() instead of Node.create_tween()
4 participants