Fixing the last couple of Basic Combat section bugs

For this lecture we deliberately left the bug in so that students had the opportunity to puzzle through a solution and get a deeper understanding of the relationship between the animator states and our ActionScheduler states.

Any questions or comments about this lecture?

1 Like

Id have a bool of “hitPending”, set it when you call the attack animation, unset it in the hit… because… you only need to set the stopattack animation trigger if its actually doing an attack…

as it stands if you have resettrigger stopattack, and then stopattack if you’re walking away at that point it still has the stopattack cued up, hence you also reset it in the attack which you wouldnt need to do.

So mine goes

bool hitpending = false;

    private void AttackBehavior()
    {
        transform.LookAt(target.transform);
        timeSinceLastAttack = 0;
        hitpending = true;
        GetComponent<Animator>().SetTrigger("attack");
    }



    void Hit() // animation event
    {
        hitpending = false;   
        if (target == null) return;         
        target.TakeDamage(weaponDamage);
    }

    public void Cancel()
    {
        target = null;
        if (hitpending) 
              GetComponent<Animator>().SetTrigger("stopattacknow");
    }

So mine only gets triggered if you were starting the attack, which leaves you in a cleaner state

The issue with this approach is that we are still in the attack state for a few frames after the hit has occurred. If you cancel during this window, the animation will not be stopped.

The underlying reason that the fix is fragile is because you are trying to estimate the state of the animator by setting a boolean when you think there should be a transition. This will always be error prone as you could always add more transitions later that will make your bool out of sync with the animators true state. In general, we should rely directly on the state of the animator. As querying the state of an animator can be a messy thing, I figured the next best thing was to reset the trigger.

But you mentioned that the trigger hanging around was messy. This doesn’t bother me personally as I see it as a signal that’s only important when in a certain state. That said, you could get around it a different way. You could create a transition from locomotion to itself that is triggered on “stopAttack”. This would consume the extra trigger without changing states. I personally find that approach less appealing because if you add more state that attack could transition to, you would have to remember to add the transition to consume the trigger which is messy.

Thanks Sam,

I must admit, I dont see how Im staying in attack for more frames than the given solution, however, running around with the trigger loaded bugs me :smiley:

Imagine your animation is 10 seconds long. And the hit happens at 5 seconds. At that time the hitpending is set to False. So if you cancel between 5 seconds and 10 seconds of the animation the "stopattacknow" will not be set so the animation will continue for another 5 seconds until it is finished.

well, you can use
tbh its one of those areas where a clean answer isnt simple. I dislike the idea of running round with the trigger set.

You can test if an animation is in progress with

(!anim.IsInTransition(0) && anim.GetCurrentAnimatorStateInfo(0).IsName("") &&
anim.GetCurrentAnimatorStateInfo(0).normalizedTime <= 1)

but then if its transitioning to the attack, you’d still have an issue. even though that might a next to zero time.

Definitely tricky to find a clean solution and I think it comes down to preference in the end. It’s cool to have lots to chose from now.

True, running around with the trigger prepped is a bit weird. For another “you could” - use the timeSinceLastAttack as a float value, copy that into the animator, and trigger the transition to attack when this value is a number smaller than the length of the animation (because we always set it to zero when we do another attack).

But I think, I’m going to stick with the triggers. Rather than trying to keep track of where to reset each individual trigger, I made a method ResetTriggers() that resets all of them, and I call that first thing when the action is scheduled, and then right before the action is canceled. Tidy things up when we switch “controllers” so to speak.

What I found was that the “stopattack” trigger wasn’t going back to its default state as I expected it to. What I ended up doing was calling ResetTrigger() in my InteractWithCombat() function before I set the target. I renamed the Attack() in Fighter.cs to SetTarget() since that, to me, is the biggest thing that function does.

anim = the Animator component
fighter = the Fighter.cs script

private bool InteractWithCombat()
        {
            RaycastHit[] hits = Physics.RaycastAll(GetMouseRay());
            foreach (RaycastHit hit in hits)
            {
                CombatTarget target = hit.transform.GetComponent<CombatTarget>();
                if (target == null) continue;

                if (Input.GetMouseButtonDown(0))
                {
                    anim.ResetTrigger("stopattack");
                    fighter.SetTarget(target);
                }
                return true;
            }
            return false;
        }

Lol. Rick had basically the same solution. I prefer where I have my reset placed though, because this way, I’m fully prepped when I go to attack something.

Hi everyone,

In regards to the animation(?) bug, I suspect that under AttackBehaviour(), the animation trigger is being set and having to play out before timeSinceLastAttack is set back to zero, and perhaps it is wanting the animation to play out in full before resting it?

Not overly confident with this answer but here goes!