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 if an object has running tweens before destroying it or before unloading a scene.

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?"

[NFMynster]

Is Sequence.ChainDelay missing the warnIfTargetDestroyed on purpose? Or does it not matter?

[NFMynster]

Ok, after using this more and more, it's a consistent issue. I would much rather have the flag the other way around, so if you have a business critical tween, you set warnIfTargetDestroyed: true, explicitly.

[NFMynster]

If not, I'd like a Tween.StopAll (already there) but also a sequence.stopAll

[KyryloKuzyk]

Is Sequence.ChainDelay missing the warnIfTargetDestroyed on purpose? Or does it not matter?

It doesn't matter because Sequence.ChainDelay() doesn't have the callback. 'warnIfTargetDestroyed' is only present in methods that have callbacks.

I would much rather have the flag the other way around, so if you have a business critical tween, you set warnIfTargetDestroyed: true, explicitly.

Changing the default value of 'warnIfTargetDestroyed' from true to false will make it a lot harder for users to debug problems related to destroying objects. Oftentimes, users new to Unity and C# are not aware of this potential problem at all. So I decided to go on the safe path and treat all OnComplete() callbacks as 'business critical' by default, leaving it to users to decide on a case-by-case basis if it's safe to ignore the particular callback or not.
Can you please tell me how often you encounter the need to set warnIfTargetDestroyed = false? If you don't expect the object to be destroyed while the animation is still running, you can omit the warnIfTargetDestroyed and leave it at the default 'true' value. Or if your game can potentially change scenes at arbitrary moments, you can call Tween.StopAll() before changing scenes.

If not, I'd like a Tween.StopAll (already there) but also a sequence.stopAll

Tween.StopAll() stops all tweens and sequences. Or do you wish to have a separate Sequence.StopAll() method that will stop only sequences?

[NFMynster]

Yes, after writing this I did do the Tween.StopAll(), but I worried that it didn't stop sequences. It seems to be the case, fortunately.

Or if your game can potentially change scenes at arbitrary moments,

This is exactly my case, I'm developing a multiplayer game where the host (or client) can disconnect at any moment. I did end up just doing Tween.StopAll when disconnection happens, and this solved all my issues.
But the XML summary of Tween.StopAll made me worry that it didn't stop the sequences also.

So, thank you first of all, I rest my case regarding defaulting warnIfTargetDestroyed to false - my case is solved by manually killing all my tweens.
--This wasn't viable when I used DOTween, due I had a bunch of references to sequences and such which I played forwards/backwards, and killing them all broke that, however, now it is doable.

I am also happy to share that I've ported my entire game to PrimeTween and deployed it, everything is working great.

[KyryloKuzyk]

I'm glad to hear that PrimeTween works perfectly for your game and even solves limitations present before with other tween libraries!
I'll update the XML docs in the next version to make it clear that Tween.StopAll/CompleteAll() also include all sequences.

[YuliyF]

Tween.Delay(gameObject, 4f, () => ExitToMain(), warnIfTargetDestroyed: false);
void ExitToMain()
{
 ...
Tween.StopAll(gameObject);
Tween.StopAll();
}

For me It doesn't work, I catch an error

[KyryloKuzyk]

I don't have the complete picture, but I assume you destroy the current scene at the beginning of the ExitToMain() method, right? If this is the case, the 'Tween's OnComplete callback was ignored' warning will come not from the Delay tween, but from some other tween whose target has been destroyed by the scene unload. So you should use the 'warnIfTargetDestroyed' parameter on that other tween, not on the Delay itself.

Also, unloading the scene after the Tween.StopAll() will also solve the issue without the need to use the 'warnIfTargetDestroyed' parameter at all:

void ExitToMainAfterDelay() {
    Tween.Delay(gameObject, 4f, () => ExitToMain()); // warnIfTargetDestroyed: false is not needed here if you don't destroy the 'gameObject' before the Delay completes 
}

void ExitToMain() {
    // Tween.StopAll(gameObject); // this call is not needed because the Delay tween has been already completed and we're currently in its onComplete callback 
    Tween.StopAll();
    UnloadCurrentScene(); // unload the current scene AFTER Tween.StopAll() to prevent the 'Tween's OnComplete callback was ignored' warning from destroyed tweens
}