ok so im making an fps game and im trying to add a reloading feature and its supposed to reload only once but for some reason the reload function gets called repeatedly. Any Fix?
here’s my script:
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.InteropServices;
using UnityEngine;
public class AkShooting : MonoBehaviour
{
public int MaxAmmo = 5;
private int CurrentAmmo;
public GameObject ImpactEffect;
public ParticleSystem MuzzleFlash;
public Camera FpsCam;
public float FireRate = 100f;
private float NextTimeToFire = 0f;
public float damage = 10f;
public float Range = 50f;
public Animator animator;
public bool ableToFire = true;
//AMMO
public int maxAmmo = 10;
public int currentAmmo;
public float ReloadTime = 2f;
public bool isReloading = true;
public bool ableToReload = false;
void Start()
{
currentAmmo = maxAmmo;
Cursor.lockState = CursorLockMode.Locked;
CurrentAmmo = MaxAmmo;
Cursor.visible = false;
}
// Update is called once per frame
void Update()
{
if (ableToFire = true && Input.GetButton("Fire1") && Time.time >= NextTimeToFire && CurrentAmmo > 0 )
{
NextTimeToFire = Time.time + 3f / FireRate;
Shoot();
}
if (CurrentAmmo <= 0 && ableToReload = false)
{
StartCoroutine(Reload());
return;
}
void Shoot()
{
CurrentAmmo--;
MuzzleFlash.Play();
RaycastHit Hit;
if( Physics.Raycast(FpsCam.transform.position, FpsCam.transform.forward, out Hit, Range))
{
EnemyHealth enemy = Hit.transform.GetComponent<EnemyHealth>();
if (enemy != null)
{
enemy.TakeDamage(damage);
}
GameObject impactGO = Instantiate(ImpactEffect, Hit.point, Quaternion.LookRotation(Hit.normal));
Destroy(impactGO, 0.1f);
}
}
IEnumerator Reload()
{
ableToFire = false;
UnityEngine.Debug.Log("Reloading...");
yield return new WaitForSeconds(ReloadTime);
currentAmmo = maxAmmo;
ableToFire = true;
ableToReload = false;
}
}
}
[code/]
Basically how do I use coroutines properly?
Your logic is very confusing to me, but the biggest glaring issue I see with your code is that you are repeatedly using the ASSIGNMENT operator = inside if statements instead of the EQUALITY operator ==. This is going to wreak havok on whatever your logic is suppsoed to be. Fix that first (and please fix the formatting, it’s very hard to read), then see if you’re still having issues and come back.
https://stackoverflow.com/questions/4704388/using-equal-operators-in-c-sharp
1 Like
Coroutines may not be your problem, I didn’t look that close, but what I do see is your if statements are wrong.
When doing assignments a single = will do.
When doing comparisons a == is required
So your ableToFire = true should be ableToFire == true
ableToReload = true should be ableToReload == true
Note that for bools, you can also just do
if(ableToFire && Input.GetButton.... //checks if ableToFire is true
if(!ableToFire && Input.GetButton.... //checks if ableToFire is false
as an example.
That alone might fix your coroutine issue.
@PraetorBlue beat me to it. 
1 Like
alright sorry about the formatting and I realized I only need one variable for this. anyway I Changed all the if statements from “=” to “==”. everything is the same except it waits for ReloadTime to finish before repeating the debug.log statement. Here is what I changed :
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.InteropServices;
using UnityEngine;
public class AkShooting : MonoBehaviour
{
// public EnemyHealth Enemy;
public int MaxAmmo = 5;
private int CurrentAmmo;
public GameObject ImpactEffect;
public ParticleSystem MuzzleFlash;
public Camera FpsCam;
public float FireRate = 100f;
private float NextTimeToFire = 0f;
public float damage = 10f;
public float Range = 50f;
public Animator animator;
//AMMO
public int maxAmmo = 10;
public int currentAmmo;
public float ReloadTime = 2f;
private bool Reloading = false;
void Start()
{
currentAmmo = maxAmmo;
Cursor.lockState = CursorLockMode.Locked;
CurrentAmmo = MaxAmmo;
Cursor.visible = false;
}
// Update is called once per frame
void Update()
{
if (Reloading)
return;
if (Reloading == false && Input.GetButton("Fire1") && Time.time >= NextTimeToFire && CurrentAmmo > 0 )
{
NextTimeToFire = Time.time + 3f / FireRate;
Shoot();
}
if (CurrentAmmo <= 0)
{
StartCoroutine(Reload());
return;
}
void Shoot()
{
CurrentAmmo--;
MuzzleFlash.Play();
RaycastHit Hit;
if( Physics.Raycast(FpsCam.transform.position, FpsCam.transform.forward, out Hit, Range))
{
EnemyHealth enemy = Hit.transform.GetComponent<EnemyHealth>();
if (enemy != null)
{
enemy.TakeDamage(damage);
}
GameObject impactGO = Instantiate(ImpactEffect, Hit.point, Quaternion.LookRotation(Hit.normal));
Destroy(impactGO, 0.1f);
}
}
IEnumerator Reload()
{
Reloading = true;
UnityEngine.Debug.Log("Reloading...");
yield return new WaitForSeconds(ReloadTime);
currentAmmo = maxAmmo;
Reloading = false;
}
}
}
[code/]
OK so, because you have this in Update:
if (CurrentAmmo <= 0)
{
StartCoroutine(Reload());
return;
}
It’s going to start a new reload coroutine every single frame until the first reload finishes.
Try adding a check that you’re not already reloading:
if (CurrentAmmo <= 0 && !Reloading)
{
StartCoroutine(Reload());
return;
}
Aside from that, why are all of your other functions local functions inside Update()? Move them outside of Update().
2 Likes
this didn’t seem to change anything.
Im getting the same results:
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.InteropServices;
using UnityEngine;
public class AkShooting : MonoBehaviour
{
// public EnemyHealth Enemy;
public int MaxAmmo = 5;
private int CurrentAmmo;
public GameObject ImpactEffect;
public ParticleSystem MuzzleFlash;
public Camera FpsCam;
public float FireRate = 100f;
private float NextTimeToFire = 0f;
public float damage = 10f;
public float Range = 50f;
public Animator animator;
//AMMO
public int maxAmmo = 10;
public int currentAmmo;
public float ReloadTime = 2f;
private bool Reloading = false;
void Start()
{
currentAmmo = maxAmmo;
Cursor.lockState = CursorLockMode.Locked;
CurrentAmmo = MaxAmmo;
Cursor.visible = false;
}
// Update is called once per frame
void Update()
{
if (Reloading)
return;
if (Reloading == false && Input.GetButton("Fire1") && Time.time >= NextTimeToFire && CurrentAmmo > 0 )
{
NextTimeToFire = Time.time + 3f / FireRate;
Shoot();
}
if (CurrentAmmo <= 0 && !Reloading)
{
StartCoroutine(Reload());
return;
}
}
IEnumerator Reload()
{
Reloading = true;
UnityEngine.Debug.Log("Reloading...");
yield return new WaitForSeconds(ReloadTime);
currentAmmo = maxAmmo;
Reloading = false;
}
void Shoot()
{
CurrentAmmo--;
MuzzleFlash.Play();
RaycastHit Hit;
if (Physics.Raycast(FpsCam.transform.position, FpsCam.transform.forward, out Hit, Range))
{
EnemyHealth enemy = Hit.transform.GetComponent<EnemyHealth>();
if (enemy != null)
{
enemy.TakeDamage(damage);
}
GameObject impactGO = Instantiate(ImpactEffect, Hit.point, Quaternion.LookRotation(Hit.normal));
Destroy(impactGO, 0.1f);
}
}
}
[code/]
Why do you have 2 times CurrentAmmo?
One public and one private. lowercase for the public and uppercase for the private.
Are you sure you’re changing the right current ammo variable?
In your reload method you’re changing the lower case currentAmmo
In your update loop you’re checking the upper case CurrentAmmo.
In the Shoot method you’re changing the upper case.
How about remove the currentAmmo, change CurrentAmmo public (upper case is usually public)
I went and changed it up a bit and commented lines.
So now CurrentAmmo is visible in the inspector. When you shoot, it should decrease the value of CurrentAmmo. After reloading it sets the CurrentAmmo to the MaxAmmo. Make sure you’ve set the right value for MaxAmmo in the inspector.
public class AkShooting : MonoBehaviour
{
[Header("Settings")]
public int CurrentAmmo;
public int MaxAmmo = 5;
public float ReloadTime = 2f;
public float FireRate = 100f;
public float Damage = 10f;
public float Range = 50f;
[Header("Required Components")]
public GameObject ImpactEffect;
public ParticleSystem MuzzleFlash;
public Camera FpsCam;
public Animator animator;
// Privates
private float NextTimeToFire = 0f;
private bool Reloading = false;
private void Start()
{
Cursor.lockState = CursorLockMode.Locked;
Cursor.visible = false;
}
// Update is called once per frame
private void Update()
{
// While reloading, don't update
if (Reloading)
return;
// If we're using the Fire button and the Time in between bullets has passed and the Ammo clip still has bullets left.
if (Input.GetButton("Fire1") && Time.time >= NextTimeToFire && CurrentAmmo > 0)
{
// Set the next bullet time
NextTimeToFire = Time.time + 3f / FireRate;
// Shoot the bullet
Shoot();
}
// If the Ammo clip is depleted and we aren't reloading -> Reload the ammo
if (CurrentAmmo <= 0 && !Reloading)
{
StartCoroutine(Reload());
}
}
private IEnumerator Reload()
{
// We're reloading now
Reloading = true;
// Wait until the reload time has passed
yield return new WaitForSeconds(ReloadTime);
// Set the actual ammo value
CurrentAmmo = MaxAmmo;
// Finish Reloading
Reloading = false;
}
private void Shoot()
{
// Decrease the ammo
CurrentAmmo--;
// Play some particles
MuzzleFlash.Play();
// Do some physics to check if we hit an enemy
RaycastHit Hit;
if (Physics.Raycast(FpsCam.transform.position, FpsCam.transform.forward, out Hit, Range))
{
// We did hit something
// Does it have the EnemyHealth script attached?
if (Hit.transform.TryGetComponent<EnemyHealth>(out var enemyHealth))
{
// Decrease the enemy health
enemyHealth.TakeDamage(Damage);
}
// Instantiate an impact effect
GameObject impactGO = Instantiate(ImpactEffect, Hit.point, Quaternion.LookRotation(Hit.normal));
// Destroy it in .1 seconds
Destroy(impactGO, 0.1f);
}
}
}
2 Likes
just tried it and it worked! I hadn’t noticed that I had 2 CurrentAmmo. This had been halting the progress of my game for days. I really Appreciate Everyone’s help. Thanks!