Demigiant / dotween

A Unity C# animation engine. HOTween v2
http://dotween.demigiant.com
Other
2.34k stars 351 forks source link

Callback called twice #15

Open alryazanov opened 9 years ago

alryazanov commented 9 years ago

Hi! For some tweens callback called twice. I noticed that in this case depends on the ease for sequence (OutBounce).

DOTween v1.0.665 release Unity 4.6.4 p4

public class Test : MonoBehaviour
{
    public Transform ObjTransform1;
    public Transform ObjTransform2;

    private readonly string Id = "test";
    private readonly Ease Ease = Ease.OutBounce;
    private readonly float Duration = 1f;
    private readonly float StartDelay = 0f;
    private readonly float Delay = 0.25f;

    void Start()
    {
        Sequence tweenSequence = DOTween.Sequence();
        tweenSequence.SetId(Id);
        tweenSequence.SetEase(Ease);
        tweenSequence.OnStart(() => Debug.Log("Started"));
        tweenSequence.OnComplete(() => Debug.Log("Completed"));

        Tween firstMoveTween = ObjTransform1.DOLocalMove(ObjTransform2.localPosition, Duration);
        firstMoveTween.OnStart(() => Debug.Log("Transform started " + ObjTransform1.name));
        firstMoveTween.OnComplete(() => Debug.Log("Transform completed " + ObjTransform1.name));

        Tween secondMoveTween = ObjTransform2.DOLocalMove(ObjTransform1.localPosition, Duration);
        secondMoveTween.OnStart(() => Debug.Log("Transform started " + ObjTransform2.name));
        secondMoveTween.OnComplete(() => Debug.Log("Transform completed " + ObjTransform2.name));

        tweenSequence.Insert(StartDelay, firstMoveTween);
        tweenSequence.Insert(StartDelay + Delay, secondMoveTween);
    }
}

Log (cube1 completed called twice):

test

Thanks!

Demigiant commented 9 years ago

Hi!

Woah weird! Ease is calculated in a completely independent way from the duration, so it shouldn't matter at all with callbacks. Could you try with this last version I uploaded (which fixes a bug with callbacks) and let me know if it still happens?

Cheers!

alryazanov commented 9 years ago

Hi!

You did not try to reproduce the example?

I updated version, but callback still called twice.

Repro project: http://s000.tinyupload.com/?file_id=71881774889752584613

Thanks!

Demigiant commented 9 years ago

Ouch, I apologize! I was slammed after completing the new DOTween update yesterday, and I went over your issue too quickly.

Exclusively in the case of Sequences, ease does influence the time position and thus the internal callbacks too. This is not a bug though, because with Bounce (or Elastic) eases some Sequenced tween might actually complete twice. That's because, when the whole Sequence is bouncing, a nested tween might complete, then rewind back a little, then complete again (depending on the position where it was inserted).

alryazanov commented 9 years ago

I understand what you mean, but in practice it is strange and not intuitive. In practice, user does not expect it and waiting for a single OnComplete callback.

Demigiant commented 9 years ago

I'm thinking that I might disallow Bounce/Elastic ease with Sequences, which would prevent this confusion, but I would rather not. Uhmmm. By the way, you know you can apply easings to nested Tweens directly (in which case this issue wouldn't happen) right?

alryazanov commented 9 years ago

Now I am planning to move away from the use of sequences (I will use separate tweens with delays and durations), because the profiler shows certain amount of GC Alloc for sequences (with no callbacks). Besides, I still think how get around closures (generates lot of GC Alloc for my custom data). For example 100 callback for start, 100 for complete with custom data as args. Maybe using of separate custom timers or timeline for triggers.

Demigiant commented 9 years ago

Ouch, Sequences generating GC Alloc is very shameful. Totally my bad, I introduced that recently while working around an issue concerning floating point imprecision. I am on it, and hope to remove those allocations asap.

I've thought about possible solutions in order to get around your closure problem, but can't find anything satisfying API-wise and KB-wise. Will ponder more, and if you think of something let me know.

Demigiant commented 9 years ago

P.S. opened a new issue about Sequences GC alloc here

Demigiant commented 8 years ago

UPDATE: this is an open issue just so people can read about it, but it's actually a NON issue. It's a matter of how the logic of easing applies to whole Sequences. Here's a summary:

Exclusively in the case of Sequences, ease does influence the time position and thus the internal callbacks too. This is not a bug though, because with Bounce (or Elastic) eases some Sequenced tween might actually complete twice. That's because, when the whole Sequence is bouncing, a nested tween might complete, then rewind back a little, then complete again (depending on the position where it was inserted).