CODEWITHSUNDEEP
c#unity-game-engine
ixTec
ixTec

Reputation: 107

Counter decreasing by half when it should be decreasing by 1

I'm working on a project in the Unity engine and I've come across an issue that I cannot understand. I have an ammo counter that decreases the amount of ammo available by 1, each time the player clicks the left mouse button. Instead of doing this though, it decreases by half (rounded down). I'm not entirely sure why this happens as the only decremental value is " ammoCount -= 1; " My code for the script is below:

using UnityEngine;
using System.Collections;
using UnityEngine.UI;

public class ShootGun : MonoBehaviour {

public Text Ammo;

public int ammoCount = 30;

void FixedUpdate ()
{
    if(Input.GetButtonDown("Fire1"))
    {
        for(int i = 1; i <= ammoCount; i++)
        {
            ammoCount -= 1;
            Debug.Log ("Ammo remaining: " + ammoCount);
            Ammo.text = " " + ammoCount;

            Vector3 gunRayOrigin = transform.position;
            float gunRayDistance = 50f;

            Ray gunRay = new Ray ();
            gunRay.origin = gunRayOrigin;
            gunRay.direction = Vector3.down;

            if(Physics.Raycast(gunRayOrigin, gunRay.direction, gunRayDistance)) {
                Debug.Log("Bullet Hit");
            }

        }
    }

}

}

Any help would be greatly appreciated, as I'm still getting the hang of C#.

Upvotes: 0

Views: 415

Answers (3)

walther
walther

Reputation: 13600

FixedUpdate is an event that happens in a fixed interval and should be used mainly for scenarios when you're dealing with physics. I don't think this is the case (some may argue that you're using raycast, but I'm not sure personally if that counts). You want to react on the users button press, so in my opinion you should use Update event instead.

GetButtonDown() returns true if the user pressed and released the button. What you should be doing instead is to use GetButton() to know if the button is held down. If yes, keep firing, if not, do nothing. Otherwise you have to press the button repeatedly, which again is probably not the desired behavior.

Let's now look at what happens when you press the button. Every time the user does the action, you start to decrease the ammo count until it reaches zero. Not sure what kind of game you're creating, but that doesn't sound right. What I'd expect instead would be to decrease the count while the user keeps the button pressed. In the Update event just check if ammo count is higher than zero and if yes, do the raycast check if it hit.

It would look like this:

void Update()
{
   if(Input.GetButton("Fire1"))
   {
       if (ammoCount > 0)
       {
           ammoCount--;
           // do raycast and so on
       }
   }
}

Upvotes: 2

Donnelle
Donnelle

Reputation: 5719

You are decrementing it inside a for loop.

If we simplify down to this:

for(int i = 1; i <= ammoCount; i++)
{
     ammoCount -= 1;
}

and take an initial ammoCount of 6, the first time you enter the loop you have:

i = 1, ammoCount = 6

But then you subtract one from ammoCount, and the loop adds one to i. So the next loop you have

i = 2; ammoCount = 5

But then you subtract one from ammoCount, and the loop adds one to i. So the next loop you have

i = 3; ammoCount = 4

But then you subtract one from ammoCount, and the loop adds one to i. So the next loop you have

i = 4; ammoCount = 3

And the loop completes (because now i <= ammoCount is no longer true). And ammoCount is half what it was.

It looks like Input.GetButtonDown("Fire1") only returns true when the button has been pressed and released, so this is a "push the button and fire all ammo" scenario, rather than "fire until out of ammo or the key is released".

So you are wanting it to keep firing while there is any ammo left (hint hint).

Upvotes: 3

Nico Schertler
Nico Schertler

Reputation: 32587

You're decrementing the counter in a for loop. By decreasing the loop's upper bound and increasing the iteration variable i you basically halve the upper bound (ammoCount).

In this case, I see no reason why a loop would be necessary.

Upvotes: 3

Related Questions