Feel like it can get better

hey guys i’m working on my td game and make this so the tower can attack multiple targets at once is working as expected but somehow i feel like it could get better?

void Attake()
    {
        switch (attackType)
        {
            case AttackType.one:
        if (Target == null || Target.Isdead())
        {
            if (Target != null)
            {
                if (targets.Contains(Target))
                {
                    targets.Remove(Target);
                }
                Target = null;
            }
            return;
        }
        Vector3 ToLook = Target.transform.position;
        ToLook.y = Turret.transform.position.y;
        Turret.transform.LookAt(ToLook);
                if (Time.time > NextFire)
                {                
                    GameObject projectile = PoolManager.instants.Getbullets(BulletId);
                    if (projectile == null)
                    {
                        Debug.Log("something wrong with pool");
                    }
                    Projectiles _projectile = projectile.GetComponent<Projectiles>();
                    projectile.transform.position = FirePoint.position;
                    _projectile.Target = Target;
                    _projectile.speed = attackspeed;
                    projectile.SetActive(true);
                    NextFire = Time.time + firerate;
                }
                break;
            case AttackType.two:
                for(int i = 0; i< Targets.Length; i++)
                {
                    if(Targets[i] == null || Targets[i].Isdead())
                    {
                        if (Targets[i] != null)
                        {
                            if (targets.Contains(Targets[i]))
                            {
                                targets.Remove(Targets[i]);
                            }
                            Targets[i] = null;
                        }
                        return;
                    }
                }            
                foreach(EnemyScript e in Targets)
                {
                    if (e != null)
                    {
                        Vector3 ToLookTwo = e.transform.position;
                        ToLookTwo.y = Turret.transform.position.y;
                        Turret.transform.LookAt(ToLookTwo);
                        break;
                    }
                }
                if (Time.time > NextFire)
                {                
                    foreach(EnemyScript e in Targets)
                    {
                        GameObject projectile = PoolManager.instants.Getbullets(BulletId);
                        if (projectile == null)
                        {
                            Debug.Log("something wrong with pool");
                        }
                        Projectiles _projectile = projectile.GetComponent<Projectiles>();
                        projectile.transform.position = FirePoint.position;
                        _projectile.Target = e;
                        _projectile.speed = attackspeed;
                        projectile.SetActive(true);
                        NextFire = Time.time + firerate;
                    }
                }
                break;
    }
      
    }

i’ve notice where

for(int i = 0; i< Targets.Length; i++)
                {
                    if(Targets[i] == null || Targets[i].Isdead())
                    {
                        if (Targets[i] != null)
                        {
                            if (targets.Contains(Targets[i]))
                            {
                                targets.Remove(Targets[i]);
                            }
                            Targets[i] = null;
                        }
                        return;
                    }

will cause a delay on attack type 2 i will fix that when i can …just another thing i can do ?wrap up the attack logic with the for loop ? so if is not null fire the attack before looping the next one ?

This badly needs to be broken up into multiple methods for readability. I also see a lot of duplicated code that could be consolidated.

1 Like

Will make all the duplicate code into a methods and break it all up …thanks

@PraetorBlue just have time to work on my project can you have a look at this if you could.

 void Attake()
    {
        switch (attackType)
        {
            case AttackType.one:
                if (!HaveTarget(Target))
                {
                    return;
                }
                LookAtTarget(Target);
                if (Time.time > NextFire)
                {
                    FireBullet(Target);
                }
                break;
            case AttackType.two:
                for (int i = 0; i < Targets.Length; i++)
                {
                    if (!HaveTarget(Targets[i]))
                    {
                        continue;
                    }
                    if (!LockOn)
                    {
                        Debug.Log("Not Lock On");
                        LookAtTarget(Targets[i]);
                    }               
                }
                if (Time.time > NextFire)
                {
                    foreach (EnemyScript e in Targets)
                    {
                        FireBullet(e);
                    }
                }
                break;
        }

    }
private void LookAtTarget(EnemyScript e)
    {
        Vector3 ToLook = e.transform.position;
        ToLook.y = Turret.transform.position.y;
        Turret.transform.LookAt(ToLook);
        LockOn = true;
    }

    private void FireBullet(EnemyScript e)
    {
        GameObject projectile = PoolManager.instants.Getbullets(BulletId);
        if (projectile == null)
        {
            Debug.Log("something wrong with pool");
        }
        Projectiles _projectile = projectile.GetComponent<Projectiles>();
        projectile.transform.position = FirePoint.position;
        _projectile.Target = e;
        _projectile.speed = attackspeed;
        projectile.SetActive(true);
        NextFire = Time.time + firerate;
    }
bool HaveTarget(EnemyScript e)
    {
        if (e == null || e.Isdead())
        {
            if (e != null)
            {
                if (targets.Contains(e))
                {
                    targets.Remove(e);
                }
                e = null;
                LockOn = false;
            }
            return false;
        }
        return true;
    }

is working fine and the delay issue have been fixed but somehow i feel like the logic is not good enough? even tho it does what i wanted…