Coroutine constantly updating animator transition

Question

I have a problem for a while now and I am running out of ideas: I have a made a coroutine used to implement animations in the attacks, something like this

private IEnumerator FirstAttack()
    {

        comboCounter = 1;
        myAnimator.SetInteger("comboSequence", comboCounter);
        currentState = PlayerState.attack;
        yield return null;

        yield return new WaitForSeconds(AttackTemplate.SetDuration(0.9f));
        comboCounter = 0;
        myAnimator.SetInteger("comboSequence", comboCounter);

        currentState = PlayerState.walk;

    }

This coroutine works perfectly fine. The problem occurs when I try to concatenate this with another coroutine to perform something like a combo sequence; to do so I basically add a while cycle that checks if I am pressing the attack button while there is a time opening for the second attack to connect. The result is this:

private IEnumerator FirstAttack() {

        comboCounter = 1;

        myAnimator.SetInteger("comboSequence", comboCounter);

        currentState = PlayerState.attack;
        yield return new WaitForSeconds(AttackTemplate.SetDuration(0.6f) - comboTimer);//Problem: if i reduce the time below the animation time of the second animation, the second animation won't go until the end

        StartCoroutine(IntervalCounter());
        comboCounter = 0;
        myAnimator.SetInteger("comboSequence", comboCounter);
        currentState = PlayerState.walk;

    }

    private IEnumerator SecondAttack()
    {

        comboCounter = 2;
        myAnimator.SetInteger("comboSequence", comboCounter);
        currentState = PlayerState.attack;
        yield return null;

        yield return new WaitForSeconds(AttackTemplate.SetDuration(0.9f));
        comboCounter = 0;
        myAnimator.SetInteger("comboSequence", comboCounter);

        currentState = PlayerState.walk;

    }


    private IEnumerator IntervalCounter()
    {
        comboTimer = AttackTemplate.SetComboTimer(0.4f);
        while (comboTimer >= 0)
        {
            Debug.Log(comboTimer);
            comboTimer -= Time.deltaTime;
            if (Input.GetKeyDown(KeyCode.Mouse0))
            {
                Debug.Log("Chained");
                StartCoroutine(SecondAttack());
                yield break;
            }
            yield return null;
        }

    }

My problem is that after adding the while loop the transition to the first animation keeps being updated, to be precise the one going from Any State to Basic Attack Tree in the following image.

If I press the attack button this transition will be repeated as long as the coroutine working. I tried to wrap around the first two lines in which i set the animator to do nothing if the integer managing the combo sequence, comboCounter is already set to 1 (which is the value related to the first attack), like this

        if (comboCounter != 1)
        {
            comboCounter = 1;
            myAnimator.SetInteger("comboSequence", comboCounter);
        }else if(comboCounter == 1) {

        }

But it keeps repeating. I wanted to use something like WaitUntil, but I tried just to put it and having a bool always false, in order to suspend the coroutine indefinitely, but in that period it keeps updating the animator. Do you have any ideas why is that? How do I prevent a coroutine from running all the time?

Answer 1

I find that OO and state machines tend to produce spaghetti code and blocks like this can be quite hard to understand. Rewriting it as pseudocode in a functional style, we can architect the flow of states a bit better - An input should trigger the first attack which should trigger waiting for a second attack window which should allow for the combo to continue:

        bool canCombo = false;
        void onAttackInput()
        {

            //if we should begin the first attack
            if (comboCounter == 0)
            {
                myAnimator.SetInteger("comboSequence", comboCounter);
                currentState = PlayerState.attack;

                //in 0.6 seconds, will be able to combo
                wait(0.6f, () =>
                 {
                     canCombo = true;
                     //in 0.3 seconds, combo window could be gone
                     wait(0.3f, () =>
                     {
                         //if no combo input happened, reset
                         if (comboCounter == 1)
                         {
                             canCombo = false;
                             comboCounter = 0;
                             myAnimator.SetInteger("comboSequence", comboCounter);
                             currentState = PlayerState.walk;
                         }
                     });

                 });
            }
            //if we should begin the second attack
            if (comboCounter == 1 && canCombo)
            {
                myAnimator.SetInteger("comboSequence", comboCounter);
                currentState = PlayerState.attack;

                //in 0.9 seconds, attack ends
                wait(0.9f, () =>
                {
                    comboCounter = 0;
                    myAnimator.SetInteger("comboSequence", comboCounter);
                    currentState = PlayerState.walk;
                });
            }

            //on any input, check if an input is valid and increment the combo counter
            if (canCombo)
            {
                canCombo = false;
                comboCounter++;
            }
        }

But in C# you can't use yield with lambdas ()=>, so this mythical wait() function really needs to be StartCoroutine() and the lambda should be its own function with a yield WaitForSeconds() at the top:

        bool canCombo = false;
        private IEnumerator onAttackInput()
        {

            //if we should begin the first attack
            if (comboCounter == 0)
            {
                myAnimator.SetInteger("comboSequence", comboCounter);
                currentState = PlayerState.attack;

                StartCoroutine(FirstAttack());
            }
            //if we should begin the second attack
            if (comboCounter == 1 && canCombo)
            {
                myAnimator.SetInteger("comboSequence", comboCounter);
                currentState = PlayerState.attack;

                StartCoroutine(SecondAttack());
            }

            //check if an input is valid and increment the combo counter
            if (canCombo)
            {
                canCombo = false;
                comboCounter++;
            }
        }
        private IEnumerator FirstAttack()
        {
            //in 0.6 seconds, will be able to combo
            yield return new WaitForSeconds(0.6f);

            //0.6 seconds later, open the combo window
            canCombo = true;
            StartCoroutine(FirstAttackWindow());
        }
        private IEnumerator FirstAttackWindow()
        {
            //in 0.3 seconds, end the combo window
            yield return new WaitForSeconds(0.3f);

            //0.3 seconds later, reset if not in next attack
            if (comboCounter == 1)
            {
                canCombo = false;
                comboCounter = 0;
                myAnimator.SetInteger("comboSequence", comboCounter);
                currentState = PlayerState.walk;
            }
        }

        private IEnumerator SecondAttack()
        {
            //in 0.9 seconds, attack ends
            yield return new WaitForSeconds(0.9f);

            //0.9 seconds later
            comboCounter = 0;
            myAnimator.SetInteger("comboSequence", comboCounter);
            currentState = PlayerState.walk;

        }

Comparing the two sets of code, it looks like your analysis of if(comboCounter != 1) is correct and it's not stopping the rest of the function(including comboCounter = 0;) from happening. You can't really stop coroutines indefinitely, it's better to make sure that they only do what they need to do, and run only when needed, instead.