Is it safe to destroy an object with a running tween on it?

[KyryloKuzyk]

Is it safe to destroy an object with a running tween on it?

The short answer is yes! It's safe to destroy an object with a running tween. If the tween's target is UnityEngine.Object, then PrimeTween will check if it's not destroyed before playing an animation further. This means you shouldn't care about running animations before destroying objects or unloading scenes.

But if a tween has an OnComplete() callback, and the tween's target gets destroyed, then what?

'warnIfTargetDestroyed' parameter

PrimeTween never silents errors or potential places where a bug can occur in a user's code. So the solution to this problem is simple - log an error that OnComplete() will not be called because of the interruption. This attracts the user's attention to the potential problem and lets them fix it correctly.

So the rule of thumb is to NOT use the OnComplete() for important actions if you expect the tween's target to be destroyed mid-way. If you're using OnComplete() only for cosmetics, then pass the warnIfTargetDestroyed: false parameter to the OnComplete() method to silence PrimeTween's warning.

Tween.PositionX(transform, endValue, duration)
    .OnComplete(transform, _transform => {
        // Cosmetic code that can be safely ignored if the transform is destroyed mid-way.
        _transform.localScale = Vector3.zero;
    }, warnIfTargetDestroyed: false);

Coin indicator example.

Let's consider the next piece of code. It animates the coinIndicator, then gives coins to the player, and updates the label:

Tween.Scale(coinIndicator, 1, 1.5f, duration, cycles: 2, cycleMode: CycleMode.Rewind)
    .OnComplete(() => {
        AddCoins(100); // if coinIndicator is destroyed mid-way, the AddCoins(100) will not be called and PrimeTween will warn you about that
        UpdateCoinIndicatorLabel();
    });

The issue with the above code is that if the coinIndicator is destroyed (or the scene is unloaded) before the animation finishes, the AddCoins(100) method will not be called, and the player will not receive their hard-earned coins. If this happens, PrimeTween will warn about that with an error to the console.

Here is a fixed version that executes only cosmetics code in OnComplete() callback:

AddCoins(100); // Call the important method independently from the animation
Tween.Scale(coinIndicator, 1, 1.5f, duration, cycles: 2, cycleMode: CycleMode.Rewind)
    .OnComplete(() => {
        UpdateCoinIndicatorLabel(); // OnComplete() callback executes only a cosmetic code that can be safely ignored if coinIndicator is destroyed
    }, warnIfTargetDestroyed: false); // disable PrimeTween's warning with 'warnIfTargetDestroyed: false' 

Notice that the important AddCoins(100) method is called independently from the animation, and the OnComplete() method only updates the label. The warnIfTargetDestroyed is set to false to tell PrimeTween that the callback is safe to ignore if coinIndicator gets destroyed mid-way.

Why not add the 'OnTargetDestroyed()' callback to the tween?

If you want to do some kind of clean-up when tween's target gets destroyed, then such a callback will not solve the problem, but will only make it worse. Let's consider a hypothetical example:

// OnTargetDestroyed() is NOT present in PrimeTween, this is just a hypothetical example
Tween.PositionX(transform, endValue, duration)
    .OnTargetDestroyed(() => DoCleanUp()); // Warning! A lot can go wrong inside the OnTargetDestroyed() callback!
// ...
Object.Destroy(gameObject);

After the transform is destroyed, it's most likely too late to do the clean-ups: (1) you can't do any operations with the transform; (2) you can't do any operations with the MonoBehaviour that started the tween because the references it holds may already be destroyed too.

The correct way to address these kinds of problems is to do a clean-up manually before you destroy the target. This way, you'll not only prevent all kinds of MissingReferenceException but will also make your code more readable and reliable.

Tween.PositionX(transform, endValue, duration);
// ...
DoCleanUp();
Object.Destroy(gameObject);

[Wokarol]

What would be the correct approach to do a coin animation that adds the coins once the animation finished in a situation where the level could finish before the end of the animation?
(For the sake of the discussion let's assume that the player's wallet is persistent because of DontDestroyOnLoad)

The solution of moving the important method outside the callback doesn't really work in this scenario as it changes behaviour.

We also cannot ignore that as a cosmetic change because then the player will lose those coins.

[KyryloKuzyk]

What would be the correct approach to do a coin animation that adds the coins once the animation finished in a situation where the level could finish before the end of the animation?

I'm not a proponent of the VMC pattern, but I'll use it to draw the analogy. Counting coins is a 'model', and it should be consistent, correct, and always up-to-date. Animating the coin indicator and displaying the coin's label is the 'controller' part that should not influence the 'model'. I'm not advocating for an MVC pattern, but I believe that separating the data from animations is a good decision.

However, if you still want to actually add the coins once the animation completes, it's possible by using a separate Tween.Delay() call. By adding coins in a separate Tween.Delay() you decouple important code from the animation (because the animation can potentially be interrupted by the scene change or target destruction). And the target-less Tween.Delay() will survive the coinIndicator destruction or scene unload.

public void AddCoinsWithAnimation() {
    Tween.Scale(coinIndicator, 1, 1.5f, duration, cycles: 2, cycleMode: CycleMode.Rewind)
        .OnComplete(() => UpdateCoinIndicatorLabel(), warnIfTargetDestroyed: false);
    Tween.Delay(duration, () => PersistentPlayerWallet.Instance.AddCoins(100));
}

However, the above code still has a problem: if the application terminates before the Tween.Delay() is completed, then the player will not receive the coins. So the completely correct solution is to still count coins separately from the animation.

The solution of moving the important method outside the callback doesn't really work in this scenario as it changes behaviour.

Yes, correctly, it changes behavior, but it fixes the potential bug. Adding important code to the OnComplete() is a bug because the animation can be interrupted by a target destruction or a scene change (or the app termination). OnComplete() is called when the animation completes (i.e. runs to completion), but scene unload and target destruction interrupt the animation, preventing its completion.

[NFMynster]

Would love for this to also be configurable globally, ie PrimeTweenConfig.warnIfTargetDestroyed

[kiroZhang1997]

I agree with you

[KyryloKuzyk]

@NFMynster @kiroZhang1997

Would love for this to also be configurable globally, ie PrimeTweenConfig.warnIfTargetDestroyed

This setting can easily be added. However, I have concerns that it will open the possibility for users to shoot themselves in a foot in some cases.

My idea with the 'warnIfTargetDestroyed' parameter was that users should make a decision on a case-by-case basis to distinguish 'cosmetic' callbacks from those that are critical and can't be safely ignored.

With the global PrimeTweenConfig.warnIfTargetDestroyed setting, it would be very easy to forget about the global setting and make a mistake. Every OnComplete() callback is different, and deciding explicitly which 'warnIfTargetDestroyed' parameter to use for each OnComplete() callback is more reliable. On the other hand, setting global PrimeTweenConfig.warnIfTargetDestroyed = false, will disable all warnings in your project (and more importantly, in all future code you will write), so you can easily miss a potential problem in your code.

What do you think?

[NFMynster]

Now yes, it is a dangerous path for sure, I agree there, though I personally don't use tweens for anything business critical - and if I do, I have my tweens stored as variable for better control.

I would maybe always have the warning enabled if editor, but not in release builds, which this flag would help with.
But I get your argumentation and won't really counter argue - it's valid points which I've considered myself. "Do I really want this?"