godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.15k stars 97 forks source link

Rework the Tween system to allow more flexibility and readability #3515

Open samdze opened 2 years ago

samdze commented 2 years ago

Describe the project you are working on

Not relevant.

Describe the problem or limitation you are having in your project

The current version of the Tween system in 4.0 is surely an improvement over the 3.x one. But in my opinion it has a few pitfalls that could be fixed in time with the final release and would bring improved usability for intermediate to advanced users, still keeping it simple for basic stuff.

Here's an example for a tween configuration:

The current implementation provides several approaches to get this result.

Await approach:

var tween1 = create_tween()

tween1.tween_property(...) # 1

tween1.tween_property(...) # 2
tween1.parallel().tween_property(...) # 3

tween1.tween_property(...) # 4
await tween1.parallel().tween_property(...).finished # 5

var tween2 = create_tween()
tween2.tween_property(...) # 6

Callable approach:

var tween := create_tween()

tween.tween_property(...) # 1
tween.tween_property(...) # 2
tween.parallel().tween_property(...) # 3
tween.tween_property(...) # 4
tween.parallel().tween_property(...).finished.connect(func():  create_tween().tween_property(...)) # 5, 6

Mix:

var tween1 = create_tween()
var tween2 = create_tween()
tween2.tween_property(...) # 6
tween2.stop()

tween1.tween_property(...) # 1

tween1.tween_property(...) # 2
tween1.parallel().tween_property(...) # 3

tween1.tween_property(...) # 4
tween1.parallel().tween_property(...).finished.connect(tween2.start) # 5 + 6

There is a recurring problem with each approach: declaring parallel tweeners is often confusing, error prone or requires the use of several language features that could be avoided.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

A more flexible way of declaring complex tweens is preferable in my opinion.

Tween objects would be containers of Tweeners. Tweeners can be simple PropertyTweeners, MethodTweeners, etc. or group of tweeners: SequenceTweener, ParallelTweener.

Tweens and groups of tweeners can create and append other tweeners to themselves. When they do, the new tweener is returned, and can be used to further create and append nested tweeners.

If a non-group tweener tries to generate a new tweener, the newly created tweener is requested to be added to the parent tweener until a tweener group is found.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

This is the approach this proposal proposes, here illustrated in length:

# By default, create_tween creates a sequential group of tweeners.
# create_tween(true) could also be called to create a parallel group
# using the signature create_tween(parallel = false)
var tween := create_tween() 

tween.tween_property(...) # 1
# A new parallel group of tweeners is appended to the main tween and returned.
var parallel1 = tween.parallel()
# Each tweener added to parallel1 will be executed in parallel.
parallel1.tween_property(...) # 2
parallel1.tween_property(...) # 3

# Appending another parallel group to the main tween, it will run after parallel1 is finished.
var parallel2 = tween.parallel()
parallel2.tween_property(...) # 4
# Adding a sequential group of tweeners `inner_seq` to parallel2.
var inner_seq = parallel2.sequence()
# 5 and 6 will be executed sequentially, but in parallel with 4.
inner_seq.tween_property(...) # 5
inner_seq.tween_property(...) # 6

The one above is the lengthy and verbose approach that provides the highest flexibility. E.g. groups of tweeners could be passed around and configured independently from the other steps of the tween.

For simpler or more strainghforward tween configurations, here's what the proposed API would allow:

var tween := create_tween()
tween.tween_property(...) # 1
tween.parallel()\ # Appending a parallel group, using it immediately to add tweeners.
    .tween_property(...)\ # 2
    .tween_property(...)\ # 3
tween.parallel()\ # Appending another parallel group that will be executed after the first.
    .tween_property(...).set_trans(...)\ # 4
    .sequence()\ # New inner sequential group of tweeners (5, 6) executed in parallel with 4.
        .tween_property(...)\ # 5
        .tween_property(...)\ # 6
    .end()\ # A method is necessary to end groups of tweeners
            # if other tweeners are being declared below.

This version is much more readable and doesn't involve using other GDScript features in a creative way.

If this enhancement will not be used often, can it be worked around with a few lines of script?

Maybe a GDScript wrapper around the current implementation is possible but it's not very convenient.

Is there a reason why this should be core and not an add-on in the asset library?

Tweens should provide flexibility out of the box, it would be a shame to have almost all the components needed to bring their usability to the next level, just to lack a few of them.

NathanLovato commented 2 years ago

To me, the current API, with the current names, is relatively intuitive. The factory methods like parallel() change the following tween animation or affect the following sequence. That's at least how I expect factory methods to work.

If people struggle to understand or use it overall, then why not change the way it works. But that should be the starting condition. I personally haven't had trouble trying the new system for concrete tween animations.

If others struggle with the new system and feel the proposed one's more intuitive, I'd just like to recommend naming changes. To me, "parallel()" doesn't suggest I'm opening a group that I have to close. I prefer explicit names like: create_parallel_group() or make_parallel_group() instead of parallel() and close_group() instead of end(). That way, the intended usage is clearer.

I'd also personally rather use a data structure to represent this kind of group/layer structure rather than factory methods. It would better fit the proposed uses (a bit like how flutter and other web frameworks let you define structures or animations). Although nowhere else in GDScript you have something like this so I don't think it fits well into Godot's API.

Anyway, the most important aspect for me is to first see if more people have had issues working with the current API in practice.

dalexeev commented 2 years ago

Another option:

var tween = create_tween()

tween.tween_property(...).play() # 1

tween.parallel(
    tween.tween_property(...), # 2
    tween.tween_property(...)  # 3
).play()

tween.parallel(
    tween.tween_property(...), # 4
    tween.chain(
        tween.tween_property(...), # 5
        tween.tween_property(...)  # 6
    )
).play()
Or even ```gdscript var tween = create_tween() tween.chain( tween.tween_property(...), # 1 tween.parallel( tween.tween_property(...), # 2 tween.tween_property(...) # 3 ), tween.parallel( tween.tween_property(...), # 4 tween.chain( tween.tween_property(...), # 5 tween.tween_property(...) # 6 ) ) ).play() ```
KoBeWi commented 2 years ago

Tween objects would be containers of Tweeners.

This is actually how it works now.

I think it might be possible to implement what you suggest without any changes to the API. We could add a GroupTweener, that would act as a "sub-Tween" and allow more advanced mixing. Although with this we would have 2 ways of sequencing/paralleling animations and the system would be more complex/confusing for users.

Another option is adding what I thought about initially - CustomTweener, a Tweener that exposes step() as virtual method that can be overriden in script. This way you could create animations however you want. Internally the Tween just calls step(delta) on each Tweener and they either return true if they are still active or false if finished etc., so there are lots of possibilities.

AhmedElTabarani commented 2 years ago

If this #623, #3490 implemented

we can do something like this

 var tween1 = create_tween()

tween1.tween_property(...) # 1

with tween1.parallel():
    tween_property(...) # 2
    tween_property(...) # 3

tween1.tween_property(...) # 4

with tween1.parallel():
    await tween_property(...).finished # 5
    var tween2 = create_tween()
    tween2.tween_property(...) # 6

this is more readable, I think

samdze commented 2 years ago

If others struggle with the new system and feel the proposed one's more intuitive, I'd just like to recommend naming changes. To me, "parallel()" doesn't suggest I'm opening a group that I have to close. I prefer explicit names like: create_parallel_group() or make_parallel_group() instead of parallel() and close_group() instead of end(). That way, the intended usage is clearer.

The methods naming is surely subject to change, I just borrowed a few names that were already in use in the previous implementation.

I'd also personally rather use a data structure to represent this kind of group/layer structure rather than factory methods. It would better fit the proposed uses (a bit like how flutter and other web frameworks let you define structures or animations). Although nowhere else in GDScript you have something like this so I don't think it fits well into Godot's API.

I'm not familiar with how Flutter works in this regard but factory methods like parallel() or create_parallel_group() return an object that can be handled and configured however you like. They are factory methods right now because the proposal expects tweeners to always be owned by a single Tween and can't live without one. Although, this can be changed, and would result in an even more flexible system.

I think it might be possible to implement what you suggest without any changes to the API. We could add a GroupTweener, that would act as a "sub-Tween" and allow more advanced mixing. Although with this we would have 2 ways of sequencing/paralleling animations and the system would be more complex/confusing for users.

Exactly, having two ways to declare tweens is not ideal.

Another option is adding what I thought about initially - CustomTweener, a Tweener that exposes step() as virtual method that can be overriden in script. This way you could create animations however you want. Internally the Tween just calls step(delta) on each Tweener and they either return true if they are still active or false if finished etc., so there are lots of possibilities.

To me, that is just another complementary tool that doesn't supersede this proposal.

MewPurPur commented 2 years ago

I hope something like this gets implemented. I was about to open a similar proposal until I found this.

In my opinion, Tweens should be entirely composed of Chain and Parallel "containers" with Tweeners in them. Entirely remove chain(), parallel(), and set_parallel() in favor of this. It'd be awkward for one tweener, but anything upwards of 2 is an improvement, as the tween's name and mode wouldn't need to be repeated.

Here is a syntax inspired, of all things, by BoxContainers and how VBox and HBox containers are nested in them:

var TW := create_tween()
TW.chain:
   tween_property(...)   #1
   parallel:
      tween_property(...)   #2
      tween_property(...)   #3
   parallel:
      tween_property(...)   #4
      chain:
         tween_property(...)   #5
         tween_property(...)   #6
  1. Makes Tweeners with complex order easy to read. You don't have to check if the Tween has set_parallel() and then consider every chain() and parallel() tag along the way and whether the parallel/chain mode is inferred anywhere.
  2. Currently, it's easy to forget if chain() and parallel() are methods of Tweens or Tweeners. This is no longer a problem.
  3. chain() and parallel() are the only methods you call from a Tween that only affect one tweener, so the clutter is not really needed.

A real example of transition between levels in my game, just to show it in practice:

var TW = create_tween().set_trans(Tween.TRANS_QUART).set_parallel()
TW.set_pause_mode(Tween.TWEEN_PAUSE_PROCESS)

TW.tween_callback(active_chpt.pause_overlay.fake_pause.bind(true))
TW.tween_property(trans_camera, "global_position", target_pos, trans_time).from(start_pos)
TW.tween_property(active_chpt.nix, "global_position", dir, trans_time).as_relative().from_current()
TW.chain().tween_callback(active_chpt.pause_overlay.fake_pause.bind(false))

becomes

var TW = create_tween().set_trans(Tween.TRANS_QUART)
TW.set_pause_mode(Tween.TWEEN_PAUSE_PROCESS)

TW.chain:
   parallel:
      tween_callback(active_chpt.pause_overlay.fake_pause.bind(true))
      tween_property(trans_camera, "global_position", target_pos, trans_time).from(start_pos)
      tween_property(active_chpt.nix, "global_position", dir, trans_time).as_relative().from_current()
   tween_callback(active_chpt.pause_overlay.fake_pause.bind(false))
KoBeWi commented 2 years ago

@MewPurPur This requires adding a special GDScript syntax just for Tweens, so unlikely it will get added.

MewPurPur commented 2 years ago

True, I posted because all of the syntax proposed here has also been special for tweens and/xor somewhat inconsistent with GDScript, and my syntax idea was the most compact/clear by far IMO. Tbh if tweens are deemed a really important class, a unique syntax shouldn't be off the table (but still undesirable)

Meorge commented 1 day ago

I found this thread because I really enjoy using tweens in my game code, both when I was working with Unity/C# and DOTween, and now with Godot.

I think adding the ability to nest tweens within each other would go a long way towards allowing for more complex tween workflows.

# Holds the sequence of 5-then-6
var subtween_5_6 = create_tween()
subtween_5_6.tween_property(...) # 5
subtween_5_6.tween_property(...) # 6

# Holds the sub-sequence of 4 at the same time as 5-then-6
var subtween_4_5_6 = create_tween()
subtween_4_5_6.tween_property(...) # 4
subtween_4_5_6.parallel().tween_subtween(subtween_5_6) # 5-then-6

# Overall tween
var tween = create_tween()

tween.tween_property(...) # 1

tween.tween_property(...) # 2
tween.parallel().tween_property(...) # 3 at the same time as 2

tween.tween_subtween(subtween_4_5_6) # 4 and 5-then-6

When a Tween is passed to tween_subtween, it could have its "auto-run" behavior disabled, so that it is only evaluated once it is reached in its parent Tween.

This sort of behavior would also make it a little comfier to dynamically build tween sequences from various objects. Say you have a bunch of sprites you want to scale up then down in sequence. You could define a function on the sprite's script like so:

func bounce_tween() -> Tween:
    var tw = create_tween()
    tw.tween_property(self, "scale", Vector2.ONE * 2, 1.0)
    tw.tween_property(self, "scale", Vector2.ONE, 1.0)
    return tw

Then, you could have them animate in sequence:

func animate_all_sprites():
    var tween = create_tween()
    for spr in sprites:
        tween.tween_subtween(spr.bounce_tween())

[!NOTE] I know this is already possible by passing a Tween object to the function, like so:

func bounce_tween(tw: Tween):
    tw.tween_property(self, "scale", Vector2.ONE * 2, 1.0)
    tw.tween_property(self, "scale", Vector2.ONE, 1.0)

But then, if you just want to bounce one sprite on its own, you have to pass it an empty Tween object, which feels a little icky to me:

one_sprite.bounce_tween(create_tween())

And the subtween function would help with cleaner, clearer setup of sequences in other ways, so I still think it'd be a good addition.