More bullets spawn than allowed

Question

Not sure why this happens but this code allows for more than 3 bullets to be fired after reloading. Am trying to work out why. I think it may be the time between checking which is this issues but I could be wrong.

Any help would be appreciated.

public bool isFiring;
public bool isReloading = false; 
 public BulletController bullet; // Reference another script
 public float bulletSpeed; // bullet speed
 public float timeBetweenShots; // time between shots can be fired
 private float shotCounter;
 public Transform firePoint;
 public static int ammoRemaining = 3;
 public static int maxAmmo = 3;
 public Transform ammoText;
 // Use this for initialization
 void Awake () {
     isReloading = false;
     ammoRemaining = maxAmmo;
 }

 // Update is called once per frame
 void Update () {
     if(isFiring == true )
     {
         shotCounter -= Time.deltaTime;
         if(shotCounter <= 0 && ammoRemaining > 0 && isReloading == false)
         {
             shotCounter = timeBetweenShots;
             BulletController newBullet = Instantiate(bullet, firePoint.position, firePoint.rotation) as BulletController; // creates a new instance of the bullet
             newBullet.speed = bulletSpeed;
             ammoRemaining -= 1;
             ammoText.GetComponent<Text>().text = "Ammo:" + ammoRemaining;
         }

 }
 else if (ammoRemaining == 0)
 {
     StartCoroutine(Reload());
 }
 else
 {
     shotCounter = 0;
 }
 }
 public IEnumerator Reload()
 {
     isReloading = true;
     ammoText.GetComponent<Text>().text = "REL...";
     yield return new WaitForSeconds(2);
     ammoRemaining = maxAmmo;
     isReloading = false;
     ammoText.GetComponent<Text>().text = "Ammo:" + ammoRemaining;
 }

Answer 1

This is an Off-topic, not to solve your problem, but to avoid you future issues, specially with performance. In case you are planning to develop a 3D Shooter I recommend you to don't instantiate bullets (unless you are planning to create some time lapse scenes)

The reasons are two:

• You will need to instantiate and destroy many GameObjects per second
• If you give a realistic speed to the bullet, it should not be seen in the scene

I assume the reasons to instantiate the bullets for you are mostly the following two:

• To create some visual effect of something leaving really fast the barrel when you press shoot
• To detect if you hit your target with a OnCollisionEnter() or similar

So I can give you some tips of something you can try instead of instantiating bullets.

1- To represent the shoot, you could instead attach a light at the end of your barrel which will be flashed when shooting. You can choose the colour, length, intensity... of this light in the Inspector.

In the following script you have a sample of how to activate and deactivate the light effect during the shot. It also controls elapse of time between shoots. Without any kind of coroutine.

public Light gunLight; 
public float timeBetweenBullets = 0.15f;

void Update ()
{
    // Add the time since Update was last called to the timer.
    timer += Time.deltaTime;

    // If the Fire1 button is being press and it's time to fire...
    if(Input.GetButton ("Fire1") && timer >= timeBetweenBullets)
    {
        Shoot ();
    }

    // If the timer has exceeded the proportion of timeBetweenBullets that the effects should be displayed for...
    if(timer >= timeBetweenBullets * effectsDisplayTime)
    {
        DisableEffects ();
    }
}

public void DisableEffects ()
{
    // Disable the line renderer and the light.
    gunLight.enabled = false;
}

void Shoot (){
    timer = 0f;
    // Enable the light.
    gunLight.enabled = true;

}

2- The second part is how to detect if the player has shoot in the correct direction. To solve that you need to use raycast when shooting, and analyse what was hit by the ray.

To do that, you should modify the shot method above with the following lines:

void Shoot ()
    {
        // Reset the timer.
        timer = 0f;

        // Enable the light.
        gunLight.enabled = true;

        // Set the shootRay so that it starts at the end of the gun and points forward from the barrel.
        shootRay.origin = transform.position;
        shootRay.direction = transform.forward;

        // Perform the raycast against gameobjects on the shootable layer and if it hits something...
        if(Physics.Raycast (shootRay, out shootHit, range, shootableMask))
        {
            // Try and find an EnemyHealth script on the gameobject hit.
            EnemyHealth enemyHealth = shootHit.collider.GetComponent <EnemyHealth> ();

            // If the EnemyHealth component exist...
            if(enemyHealth != null)
            {
                // ... the enemy should take damage.
                enemyHealth.TakeDamage (damagePerShot, shootHit.point);
            }

            // Set the second position of the line renderer to the point the raycast hit.
            gunLine.SetPosition (1, shootHit.point);
        }
        // If the raycast didn't hit anything on the shootable layer...
        else
        {
            // ... set the second position of the line renderer to the fullest extent of the gun's range.
            gunLine.SetPosition (1, shootRay.origin + shootRay.direction * range);
        }
    }

Now your shoot method cast a ray, in case the ray hit an enemy(that is a GameObject tagged as enemy) it will perform some actions. In this particular case, I assume the enemy has attached an script to reduce its life when it is shot.

You can make the special effects of the shot even more complex, for example: adding sounds, a line rendered, some particles system to emulate the powder...

You can, and I think you should, check this tutorial from the Unity official site, to get a better understanding about what I just mentioned in my answer, and to get some extra ideas for your game:

https://unity3d.com/learn/tutorials/projects/survival-shooter/harming-enemies?playlist=17144

Answer 2

The issue (as discussed) is that your Reload() coroutine is getting called additional times within Update() after you fire the last shot and before letting go of MouseButton(0). To avoid this, I would suggest checking to make sure you are not already reloading, before starting the coroutine:

 else if (ammoRemaining == 0 && !isReloading)
 {
     StartCoroutine(Reload());
 }