Fix seek end loop anim.

[rakkarage]

Closes: #85220

[AThousandShips]

Should be tested against the changes in #85193 to see if changes are needed based on this

Also what is the difference between the normal loop mode and the ping-pong loop in regards to this? The seeking for that one is unchanged
My bad misread the code 🙂

Also from the original PR #85186 (review):

Also this causes a regression where the loop does not occur when the script seeks beyond the animation time.

[rakkarage]

Because it not use mod to go back to first key time when done.

Seek to start = mod(0.1, 0.9) == 0.1: unchanged
Seek to middle = mod(0.45, 0.9) == 0.5: unchanged
Seek to end = mod(0.9, 0.9) 0: Should be 0.9

Seek pingpong to end = pingpong(0.9, 0.9) = 0.9

I cannot replicate the regression and would appreciate info on how to replicate the regression.

[AThousandShips]

I'm just quoting the comment to ensure it is recognised here, I didn't report it

You can't supersed a merged PR, and it's already closed 🙂

[TokageItLab]

The biggest problem with this implementation is that the loop animation does not loop when seeking beyond the length of the animation. If you seek 0.3 seconds from the 0.8 second point in a 0.9 length animation, it must be 0.2.

There are enough use cases like seek(current_pos + n) to seek an arbitrary number of seconds in the GDScript so this PR is unacceptable. Also you should understand that current loop behavior is important for calculation of root motion (is added to AnimationPlayer by 4.2 recently) when seeking.

Even if you do the check as in

if (next_pos > len && cd.pos < len) {
	looped_flag = Animation::LOOPED_FLAG_END;
}
if (!p_seeked && (cd.pos == 0 || cd.pos == len)) {
	next_pos = Math::fposmod(next_pos, (double)len);
}
...

it will cause the problem of not processing the first key at the point where the loop should occur due to seek.

Since it is too rare case a important key at the end of a loop animation (and it is not supposed to be), and more often a important key at the beginning, so that check would cause more problems that the beginning key not being processed than the end key not being processed.

If there are enough demands/samples for the use case of placing important keys at the end of loop animations with seeking, there may be room for improvement, but it should be in the key retrieval functions such as _track_get_key_indices_in_range(), not here, so the code here should never be changed.

[TokageItLab]

This is a PR that has already been closed once as #85186, so it is closed again. As noted above, there may be room for improvement in the handling of the last key (for Discrete/Method/Audio/AnimationPlayback Track, not Continuous) during seek,

Since it is too rare case a important key at the end of a loop animation (and it is not supposed to be), and more often a important key at the beginning, so that check would cause more problems that the beginning key not being processed than the end key not being processed.

If there are enough demands/samples for the use case of placing important keys at the end of loop animations with seeking, there may be room for improvement, but it should be in the key retrieval functions such as _track_get_key_indices_in_range(), not here, so the code here should never be changed.

but in any case to improve that, a different approach would need to be taken from this PR.