-
-
Notifications
You must be signed in to change notification settings - Fork 22k
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 AwaitTweener
#79712
base: master
Are you sure you want to change the base?
Add AwaitTweener
#79712
Conversation
Thinking about it, the only time I use
When you use But if you finish an await tween with |
|
Currently, if the
|
Changed. 0 timeout makes the tweener finish immediately, so it's a weird use-case, but why not. |
Maybe I'm worried for nothing, but wouldn't the name |
Apparently I came up with the same idea yesterday and tried to implement it myself, and came remarkably close to this PR without even knowing it existed 😅 . There were two things I had in my implementation that I don't see here, that I thought might be worth bringing up as things to add before a merge. (I'm much less experienced with the engine than you are, though, so please let me know if there are reasons for these that I'm just not aware of!) UnbindsAs I understand, this version of I attempted to solve this problem on my version by getting the signal's parent's signal list, iterating through it to find the correct signal by name, and counting its arguments there. AThousandShips and I agreed that it feels like a rather hacky workaround, and could potentially fall apart at some point. The solution in this PR definitely feels safer in that regard, but as I said before I think it would be preferable to let the engine count the number of unbinds necessary. (Maybe this would be a reason to file a feature proposal or issue for adding a Various bits of code to match other TweenersFor compatibility/parity with the other Tweeners, I think emit_signal(SceneStringName(finished)); should be included in Overall I'm really excited that this implementation exists, and I'm hoping it can make its way to 4.4! I really like tween sequences, and I think this will be a very valuable tool for using them more extensively! 😄 |
I managed to handle with vararg methods (it's the same what
Yeah I totally forgot about finish signal.
Also forgot about it 🤦♂️ I realized the implementation was broken, thanks for the feedback. |
d0622ba
to
a27d096
Compare
a27d096
to
770ae2f
Compare
770ae2f
to
8e8cb06
Compare
Tweaked the behavior to match #96216 |
doc/classes/AwaitTweener.xml
Outdated
</brief_description> | ||
<description> | ||
[AwaitTweener] is used to await a specified signal, allowing asynchronous steps in [Tween] animation. See [method Tween.tween_await] for more usage information. | ||
The [signal Tweener.finished] signal is emitted when either the awaited signal is received or when timeout is reached. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think? This is assuming it's going to be the behavior.
The [signal Tweener.finished] signal is emitted when either the awaited signal is received or when timeout is reached. | |
The [signal Tweener.finished] signal is emitted when either the awaited signal is received, when timeout is reached, or when the target object is freed. |
You may have purposely omitted this detail as it may be noted down elsewhere. However, the way this sentence is worded makes it sound like the condition at which finished
is emitted has been overridden by this class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the last case just now. Previously it would not emit the signal.
I think we could add such note to all Tweeners (in #96216 or after).
8e8cb06
to
5955b07
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't tested but looks good and the idea is good (just needs the method declaration readded)
5955b07
to
e496f56
Compare
Looking forward to this feature. When will this be merged? |
Unfortunately we're in 4.4's feature freeze right now, so I don't think it'll be in a stable release for quite a while |
e496f56
to
1bfa3dd
Compare
Looks good. One thing, though - is there any way to make this validate the signal based on the parameter list passed along with the signal? For example, for a signal |
No, the tweener discards all arguments, just like |
1bfa3dd
to
e57ccc2
Compare
Perhaps a workaround for something like this would be to connect var something: ThingThatHasHpChangeSignal = ...
signal hp_is_zero
func _ready():
something.hp_change.connect(_check_if_hp_zero)
func _check_if_hp_zero(value: int):
if value == 0: hp_is_zero.emit()
func do_tween():
var tw := create_tween()
tw.tween_await(hp_is_zero)
tw.tween_property(self, "scale", Vector2.ZERO, 1.0) |
Closes godotengine/godot-proposals#7337
Implemented more or less as described in the proposal, see the docs for details.
This was not possible, because custom step works as if the time has passed. I could maybe add a method to AwaitTweener to force it to finish, but no other Tweener has such method (though maybe it could be in the base class?).
Example:
godot.windows.editor.dev.x86_64_gh2jrUpbxv.mp4