Rework AnimatedTexture's fps into speed_scale

[Mickeon]

Closes godotengine/godot-proposals#5311.
Alternative to #65182.

As lightly mentioned in #64657 while updating the description, the fps property in AnimatedTexture has somewhat confusing behaviour. Mainly, it is expressed in frames per seconds, yet each individual frame's delay is expressed in seconds, making it a confusing soup to work with, and explain in the documentation, despite being such a simple concept.
This PR attempts to address that, but as a more major overhaul.

fps has been turned into speed_scale. It now affects the speed scale of the entire animation, exactly like AnimatedSprite2D, and AnimationPlayer's playback_speed.
As a bonus, very akin to #65148 If speed_scale is a negative value, the animation is played in reverse!

Showcase	Showcase (reverse)

Each individual frame's delay_sec has been renamed to duration (prefixes and documentation exist, making the previous name redundant). Furthermore, it's defaulted to 1.

Setters and getters, documentation updated, too.

Bugsquad edit:
Closes godotengine/godot-proposals#4742
Closes #64657
Closes #65182

[Zireael07]

I prefer this one to the other, mainly because it gets rid of the confusing "delay" naming and also handily allows playing in reverse

[filipworksdev]

I would personally make it so the duration of the individual frames are adjusted proportionally.

For ex an animation with 3 frames: 0.5s 0.75s and 1.25s totalling 2.45s when you set duration on the animation itself to say 4s it will automatically stretch each frame to 0.8s 1.2s and 2.0 totalling 4s respectively so that the character of the original animation is not altered. Right now it sets all frames to the same exact timing 1.33 1.33 1.33. This is my main issue with fps. It destroys the character of the animation. There is zero point in using fps when all my individually set timings are overriden and made equal accross the board.

I like the animation in reverse and the new naming though. Makes more sense.

I think it still would need a proposal regarding how to handle all frame times being forced to the same value.

[filipworksdev]

The final, visualised duration is adjusted proportional to the speed_scale, as you can see.

Yes but all frames will be set to that same value.

It's better not "dynamically adjusting" each, individual frame, because it would not just needlessly be computationally expensive, but the value would be subject to float inaccuracies as the slider is dragged back and forth.

I don't follow why it would be computationally expensive. The calculation is a simple loop and happens only once. You gonna have a dozen to a few hundred frames at most per AnimatedTexture. Nobody would export 1 million images and turn them into an AnimatedTexture.

...That is, unless both get_frame_duration and set_frame_duration were to return the duration, affected by speed_scale, already.

I don't know about the inner workings but based on how it works right now fps or any other rename is pointless. If I select individual times (and I often do) fps value that is over 0 will override them and make all my frames the exact same time. They give you the option to select individual frame times but they don't tell you fps ignores all your frame times and instead forces them all to the same value.

I think fps was used to test out the feature and was only useful for developers. It was left as a positive value since they were testing out that feature but for most people fps might as well not exist since it serves no practical purpose for helping the animation.

I still think it needs a proposal of sorts so we can discuss these things there instead of over here in PRs .

[filipworksdev]

Yes but all frames will be set to that same value.

I feel like there's a misunderstanding. The individual frame duration does contribute to the final playback time individually, and it's not automatically "set" to be the same value across the board behind the scenes.

With speed_scale set to 2, first frame lasting 2 seconds and the second frame lasting 6 seconds, the first frame displays for 1 second, while the second displays for 3 seconds.

Based on my tests it does actually force all the individual frames to the same value which is something like frames/fps or something like that. I don't think I found anything in the docs about this.

The logic is here see what you make of it

godot/scene/resources/texture.cpp

Line 2625 in f02134a

limit = 1.0 / fps;

Notice that when fps != 0 it forces a limit for all frame delays to 1/fps. Therefore it ignores your individually set frame delays. Is been a while so is not so fresh in memory but I remember some major flaws with how fps was designed and not being able to find much help in the docs.

Either way this is offtopic for this PR. Let's move the discussion to my proposal godotengine/godot-proposals#4742 ? 😃

[TokageItLab]

The behavior looks good.

As far as I see in the code, the problem mentioned by @filipworksdev should also be removed. With this change, all frames will progress depending on the duration set for each frames. That is, this should be equivalent to the fps = 0 state of the previous behavior. And with the progression completely dependent on the duration, then speed_scale only changes the playback rate.

[Mickeon]

Rebased,

[TokageItLab]

I think fps was used to test out the feature and was only useful for developers. It was left as a positive value since they were testing out that feature but for most people fps might as well not exist since it serves no practical purpose for helping the animation.

I guess it was probably expected that the delay of all frames would be set to 0 and the fps would be used to control the entire process, but it was not so convenient and it was mixed up with the different units of fps and seconds.

With this PR, the fps item has been removed, but it is still possible to set the duration of all frames to 1 and then set the SpeedScale to a value such as 60 or 120. However, since the AnimationTexture cannot choose whether or not to use Process or PhysicsProcess for processing, so there is not much need to be concerned about perfectly precise fps in the first place :)

[Mickeon]

In this PR each frame's duration also defaults to 1, so for most use cases the user would only need to change the speed_scale, which is super-convenient, and I believe it means it saves on Resource file size, as well?

[dalexeev]

In this PR each frame's duration also defaults to 1, so for most use cases the user would only need to change the speed_scale

It seems to me that this would be more obvious if the s suffix was removed from individual frames (not 1.0 s, but just 1.0, like Control.size_flags_stretch_ratio), and instead of speed_scale there was a general frame_duration property.

[akien-mga]

The commit message should likely be amended, speed_duration had me surprised for a bit as that's two quantities typically using different units (m/s vs. s).

[Mickeon]

The commit message should likely be amended, speed_duration had me surprised for a bit as that's two quantities typically using different units (m/s vs. s).

I honestly simply did not realise the naming mistake.

[YuriSizov]

Thanks!

[filipworksdev]

Thanks everyone. This worked even better than I expected! 🙏