Skipping flashing dialogue causes stackoverflow exception.

[puetsua]

Describe the bug
Skipping flashing dialogue causes stackoverflow exception.

To Reproduce

1. Go to the scene 'NarrativeLog' in the FungusExample
2. Add {flash=0.5} in front of every Say commands.
3. Hit Play
4. Keep pressing space or pressing escape to skip a dialogue during a flash. (Almost always reproduce.)

Expected behavior
No exception when skipping a dialogue with flashing effect.

Screenshots

Versions & Platform (please complete the following information):

• Unity version 2019.4.24f1
• Fungus Version v.3.13.4
• Editor OS: Win 10
• Build Platform //

[breadnone]

I'm guessing, this happens because the flash command itself doesn't wait from one to another. In other words, when you're cancelling them while the previous flash still flashing then this would happen.

If my memory serves me right, cancelling dialogues (Esc), ignores WaitUntilFinished. To disable Cancellling with Esc button you can toggle off Cancel Enable on your SayDialog component OR just add a longer input delay per/second from your StandAloneInput module.

[puetsua]

I'm guessing, this happens because the flash command itself doesn't wait from one to another. In other words, when you're cancelling them while the previous flash still flashing then this would happen.

If my memory serves me right, cancelling dialogues (Esc), ignores WaitUntilFinished. To disable Cancellling with Esc button you can toggle off Cancel Enable on your SayDialog component OR just add a longer input delay per/second from your StandAloneInput module.

You're right! I tried to set flash time to 5 seconds and it triggered the bug if I put another flash while the previous hasn't finished yet.

I don't think I will disable Cancel Enable because it comes in handy when I'm testing my dialogues. I probably will try to fix it myself first. Maybe just cancelling previous the flash and run next one.

[puetsua]

Not sure if this is elegant way to fix it but at least it works now.

CameraManager.cs:

namespace Fungus
{
    public class CameraManager : MonoBehaviour
    {
        // ...

        public virtual void Fade(float targetAlpha, float fadeDuration, Action fadeAction, LeanTweenType leanTweenType = LeanTweenType.easeInOutQuad)
        {
            StopFadeTween();

            if (Mathf.Approximately(fadeDuration, 0))
            {
                fadeAlpha = targetAlpha;
                if (fadeAction != null) fadeAction();
            }
            else
            {
                fadeTween = LeanTween.value(fadeAlpha, targetAlpha, fadeDuration)
                    .setEase(leanTweenType)
                    .setOnUpdate((x) => fadeAlpha = x)
                    .setOnComplete(() =>
                    {
                        LTDescr lastTween = fadeTween;
                        
                        fadeAlpha = targetAlpha;
                        if (fadeAction != null) 
                            fadeAction();
                        
                        // Don't clear fadeTween if fadeAction created a new tween.
                        if(lastTween == fadeTween) 
                            fadeTween = null;
                    });
            }
        }

        // ...

        // Avoid recursive calls when LeanTween.cancel calls a callback that lead to StopFadeTween function.
        private LTDescr cancelingTween = null; 
        public void StopFadeTween()
        {
            // Use loop to cancel all tweens in fadeTween callbacks.
            while (fadeTween != null && cancelingTween != fadeTween)
            {
                cancelingTween = fadeTween;
                LeanTween.cancel(fadeTween.id, true);

                if(cancelingTween == fadeTween)
                    fadeTween = null;

                cancelingTween = null;
            }
        }

        // ...
    }
}

[breadnone]

There are two ways to solve this. 1. LTDescr has updateNow() function that you could use, to force update the tween regardless it's in the middle of tweening or not... 2. forcing it to cancel before triggering the next flash with LeanTween.cancel().

Based on my experience, the later is prone to fail if executed in a very tight frames, especially when you're cancelling them with Esc button

[breadnone]

@puetsua don't do that, instead do it this way..

[breadnone]

After a second thought, StopFadeTween may be accessed from something else, not just Flash, thus preventing it to be stopped early and may affect other functions that are calling StopFadeTween. I'm not sure.. I'm closing my pr for this issue for now.

My advice, just don't use Cancel (Esc) on your build. It would break anything else that has tween in it... just don't use it

[puetsua]

After a second thought, StopFadeTween may be accessed from something else, not just Flash, thus preventing it to be stopped early and may affect other functions that are calling StopFadeTween. I'm not sure.. I'm closing my pr for this issue for now.

My advice, just don't use Cancel (Esc) on your build. It would break anything else that has tween in it... just don't use it

I see, still looking forward to an official fix to this tho. Cancel (Esc) is still a nice feature to have. I haven't ran into any bug with my fix yet, but I admit it's a very ugly and confusing fix. 😛

From what I observe, the root cause is LeanTween.cancel(tween, true) can create recursive calls if the setOnComplete action ask LeanTween to cancel same tween again in order to create a new tween.

I think another way to fix the code is to change this part of code to avoid recursive calls:
Writer.cs

namespace Fungus
{
    public class Writer : MonoBehaviour, IDialogInputListener
    {
        ...
        protected virtual void Flash(float duration)
        {
            var cameraManager = FungusManager.Instance.CameraManager;

            cameraManager.ScreenFadeTexture = CameraManager.CreateColorTexture(new Color(1f, 1f, 1f, 1f), 32, 32);
            cameraManager.Fade(1f, duration, delegate
            {
                cameraManager.ScreenFadeTexture = CameraManager.CreateColorTexture(new Color(1f, 1f, 1f, 1f), 32, 32);
                cameraManager.Fade(0f, duration, null); // Do this somewhere else or wait a frame?
            });
        }
        ...
    }
}

Or maybe a fix in LeanTween, I guess. Just my two cents worth of thoughts.