Sequence with repeat_count

[enaut]

Hey,

I was trying to have an object zoom into existence then wiggle infinitely... I tried doing that with:

Tween::new(
            EaseFunction::QuadraticInOut,
            Duration::from_millis(1000),
            TransformScaleLens {
                start: Vec3::ZERO,
                end: Vec3::ONE,
            },
        )
        .then(
            Tween::new(
                EaseFunction::QuadraticInOut,
                Duration::from_millis(200),
                TransformRotateZLens {
                    start: 0.05,
                    end: -0.05,
                },
            )
            .with_repeat_strategy(bevy_tweening::RepeatStrategy::MirroredRepeat)
            .with_repeat_count(RepeatCount::Infinite),
        )

But when Running the first animation works well, however the second one panics with:

thread 'Compute Task Pool (13)' panicked at 'overflow when subtracting durations', library/core/src/time.rs:930:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The same happens if repeat_count is set to a finite number even when set to 1.

It might be that this is "known" or "intended" but if so it should probably be in the documentation for the then method or the Sequence type.

The Behaviour I expected would have been an infinite sequence first the zoom then forever wiggling...

[djeedai]

Yes i don't think we support appending a finite and an infinite tweenables, because although that looks fine when playing forward, there's all sorts of undefined behaviors when playing backward or seeking to a particular point. I'm still perplex on the actual error though, looks like a bug.

[sibsibsib]

i ran into this recently as well. The regression was introduced here. When the mirrored tween hits the end and turns backwards, tween.elapsed will be larger than tween.duration and the result of the subtraction will be negative, causing an overflow (duration is unsigned and can't be negative). Reverting the line seems to fix the issue, but I'm not sure what implications that has, since i imagine it was changed for a good reason.

[djeedai]

I mean, we could revert that I guess, but again that doesn't solve the design question of "what do we do when the mirrored tween reaches back its start point?". If we mirror again then we're stuck on that tween, and any other tween in the Sequence will never play again. If we don't, then we break the contract of the the mirroring and/or infinite looping. Either way something goes in a potentially unexpected way for the user. I'm open to suggestions here, because I don't see a good alternative.

[enaut]

Just a thought - wouldn't it make sense to add a finally(end: Tweenable) method to the Sequence. The parameter to this method is always added after all the others and can be infinite. This would also be great for fade-out effects or similar.
If played forward the end is added after all the Tweenables in the tweens field. If played backwards the end is played after the Tweenables from the tweens list have been played backwards in reverse order. So it is always after everything has been played. So in case of a fade out. No matter if the Sequence was played backwards or forwards it would fade out in the end.

For the finite repetitions I think Sequence should handle them. When played forward and mirror their behaviour when played backwards.

[djeedai]

If I understand correctly, what you're proposing is that the Sequence struct holds two things:

• one sequence of finite tweens
• one terminal, possibly infinite, tween

Is that correct?

This is a nice idea, but I'm concerned it breaks some invariants. For example, suppose I create such a Sequence ending with an infinite loop. Then I mark the Sequence as looping (even finite looping). What happens? The Sequence contains an infinite tweenable (the terminal one) so from the point of view of the Sequence itself it never finished, and so it cannot possibly loop. Or, for the sake of discussion, I wrap that Sequence into another Sequence as the only terminal item (this is allowed since the inner Sequence is infinite and added as terminal). What happens to the outer Sequence? It also cannot loop for the same reasons.

But that made me think about an alternative. Ignore finally() and anything from above, and just allow infinite tweenable in then() at the condition that the tweenable is valid. Then we define being valid as:

• any Tween is valid, even infinite ones
• any Sequence is valid if and only if it contains finite tweenables except as the last position. If the last tweenable is infinite, then we accept the limitation that the Sequence can get "stuck" in the infinite terminal tweenable, that is playing backward will behave as if the Sequence only contained its terminal tweenable.
• any Tracks is valid if and only if all its tweenables are valid.

I think that this way the behavior is deterministic. The user can make errors still, and possibly get confused, but the mental model should be simple and clean enough that with (hopefully in a close future) some UI / tooling / Editor that user would immediately see their mistake. I'm also even considering adding introspection so that we can now what exact type a tweenable is, and emit warnings when detecting mistakes.

Any though on this @enaut and @sibsibsib ?

[djeedai]

Any the way this issue is a duplicate of #16, but the discussion here is interesting so leaving it open.

[enaut]

The finally thoughts:
first of yes that's basically what I meant. For the issue: I'd define the finally to be executed after the last element of tweenables. Meaning if the Sequence is looped infinitely it will never be played as there are always tweenables left to play. If the Sequence{twennables: [a, b], finally: c} is repeated 3 times this would result in a,b,a,b,a,b,c if it is repeated infinitely it would be a,b,a,b,… and c would never be reached.

Your second option is basically what I expected.

But maybe the cleaner solution would be to add another type. So

Sequence::finally(self, infinite:tweenable )->InfiniteSequence{…}

That InfiniteSequence type could then have only the valid methods implemented.

[sibsibsib]

I think keeping the .then() interface and maybe issuing a warning is probably fine. It fits my mental model that if i insert an infinite tweenable in the middle of a sequence that the sequence won't complete. My use case was very similar to the one enaut posted: I wanted some text to fade in and then pulse between two colors infinitely

---

Out of curiosity, I took a look at how greensock (a very mature JS tweening library) handles the situation, and it has some interesting behavior. Any infinite tweens within the sequence will start looping, but tweens that occur after will be triggered at their appropriate start point. On top of that, the parent's repeat settings are effectively ignored, since the whole sequence never reaches the end. For example, for a sequence of [A → B(∞) → C], when the playhead gets to the end of B, C will activate, but B will continue to repeat. That seems a bit strange to me but it might be useful for certain kind of UI animations (demo)