I found that challenge description somewhat confusing

It wasn’t clear to me that the target for CanAttack() was meant to be the potential target that we just got from the hit list of the RayCast. At first I had the issue that I couldn’t hit any enemy anymore because CanAttack() couldn’t possibly be true without having a valid target, but if it wasn’t then it would always skip to the next item of the hit list.
I solved that by first assigning and then check in CanAttack() and if not (because the potential target was dead already) skip over to the next item, but that felt wrong, too.

Now, with the solution from the lesson I would rather go one step further and, since it doesn’t depend on any state within the player’s Fighter component, turn it into a static method and call it as Fighter.CanAttack(potentialTarget); instead.

as for its implementation, I simply added a null guard to getting the Health component from the combatTarget, so if it’s null the code will never attempt to grab the health and targetToTest will be null just the same.

        public static bool CanAttack(CombatTarget combatTarget)
        {
            Health targetToTest = combatTarget?.GetComponent<Health>();
            return (null != targetToTest && !targetToTest.IsDead());
        }

It does appear, at this point, that CanAttack() can be static, but this won’t hold up a bit further down the course when we also incorporate travel distance into the formula.

        public bool CanAttack(GameObject combatTarget)
        {
            if (combatTarget == null) { return false; }
            if (!GetComponent<Mover>().CanMoveTo(combatTarget.transform.position) &&
                !GetIsInRange(combatTarget.transform)) 
            {
                return false; 
            }
            Health targetToTest = combatTarget.GetComponent<Health>();
            return targetToTest != null && !targetToTest.IsDead();
        }

With this, we need to get a reference to the current character’s Mover component, as well as have a concrete reference to the Fighter to get GetIsInRange()

I see. Well, it can always be refactored back, when needed… :grin:

1 Like

Privacy & Terms