How can I refractor my Player/PlayerRanged code?

Unity Engine
,
1

Currently I have two types of Player characters, melee and ranged. Both are on Layer: Player but their tags are different. Melee has the tag Player and Ranged has the tag PlayerRanged. I did this to be able to distinguish a Players attack as melee or ranged.

public void LightAttack(InputAction.CallbackContext context)
{
    if (context.performed)
    {
        moveVector = Vector2.zero;
        isAttackPerformed = true;
        isLightAttack = true;
        isHeavyAttack = false;
        animator.SetTrigger("LightAttack");

        if (gameObject.tag == "PlayerRanged")
        {
            Shoot();
        }

    }
}

    public void Shoot()
    {
        Ammo ammo = Instantiate(playerAmmo, firePoint.position, firePoint.rotation).GetComponent<Ammo>();
        ammo.Init(this);
    }

I recently added a state machine to all my enemies which target the “Player” tag. I was wondering if there was a better way I could call Shoot() for only the players that have ranged attack while changing their tag from PlayerRanged to Player.

Without the if statement looking for PlayerRanged, all the Player characters break because they are looking for Ammo to shoot.

Thanks!

2

You can create two different components, one for handling ranged attacks, and one for handling melee attacks.

You can also give them a shared base class, so that other systems can work with them interchangeably.

// Both RangedAttackHandler and MeleeAttackHandler can extend this same base class
public abstract class AttackHandler : MonoBehaviour
{
    [SerializeField] Player player;
    [SerializeField] string lightAttackParameter;
    [SerializeField] float lightAttackDuration;

    int lightAttackParameterId;

    // instead of three variables isAttackPerformed, isLightAttack, isHeavyAttack, can have just one
    public AttackType CurrentAttackType { get; private set; } = AttackType.None;

    // using Animator.SetTrigger(int id) has better performance than Animator.SetTrigger(string name)
    void Awake() => lightAttackParameterId = Animator.StringToHash(lightAttackParameter);

    public async void LightAttack(InputAction.CallbackContext context)
    {
        if(!context.performed || CurrentAttackType != AttackType.None)
        {
            return;
        }

        CurrentAttackType = AttackType.Light;
        animator.SetTrigger(lightAttackParameterId);
        OnLightAttackPerformed();

        await Awaitable.WaitForSecondsAsync(lightAttackDuration, destroyCancellationToken);

        CurrentAttackType = AttackType.None;
    }

    protected abstract void OnLightAttackPerformed()
}

public sealed class RangedAttackHandler : AttackHandler
{
    // No need to use GetComponent after Instantiate if you reference the component directly.
    // Also try to clearly differentiate between prefabs and instances in variable names to avoid ambiguity.
    [SerializeField] Projectile projectilePrefab;

    [SerializeField] Transform projectileLaunchPoint;

    protected override void OnLightAttackPerformed()
    {
            player.MoveSpeed = Vector2.zero;

        var projectile = Instantiate(projectilePrefab, projectileLaunchPoint.position, projectileLaunchPoint.rotation);
            projectile.Init(this);
    }
}

It would probably even make sense to attach separate handler components for light and heavy attacks. This would have the classes quite a bit shorter, and thus probably easier to read. You could also shorten a lot of the variable names, when you don’t have to add a “light” or “heavy” prefix to all of them.

However, then you would need to make sure that the two components work together so that you won’t have two overlapping attacks if the player gives both light and heavy attack inputs simultaneously.

1 Like