Firing is inconsistent despite using time

[Dinipants]

I'm using the Clock class to get elapsed time and using that to try and standardise everything in my game based on time so it's consistent. However I'm having an issue with a gun Class (specifically cooldown for its firing)

This is the time code:

sf::Time elapsed = clock.getElapsedTime();

Then in my gun class:

void fire(float angle, Vector2f playerPos) {
        if (this->gunReady == true) {
            this->gunReady = false;
            // Play fire sound, create bullet, set position, etc.
        }
    }

    void updateBullets(float elapsed) {
        this->cooldownTime += elapsed;
        if (this->cooldownTime >= SHOT_COOLDOWN) {
            this->cooldownTime = 0.0f;
            this->gunReady = true;
        }
        for (auto &bullet : this->sprites) {
            // Move all the bullets
        }
    }

then in my firing function, i just check to see if this->gunReady is true before doing anything. There are two problems:

• There is a noticeable inconsistency between shots (looks like it's changing based on FPS or some other non-static variable). sometimes it's the full cooldown time, other times it feels like two thirds the value, or longer
• Secondly (and less importantly) the very first time I start spamming the mouse button, it fires a double shot (i.e. two shots within like 200ms of each other), then after that it starts the above behaviour, where the cooldown is only roughly the value I sent and is constantly undulating

Am I using the time functionality the wrong way?

Thanks 

[eXpl0it3r]

Sounds like the problem is somewhere in a different part of your code.
If update isn't called consistently, the elapsed time might be checked weirdly as well.
Make sure you're update code is not withing the event processing loop, but directly in the game loop.

[Dinipants]

Thanks for your feedback/

Well my code isn't very well structured since I'm just practicing with each different module rather than trying to actually build something properly. So my main loop has this:

while (window.isOpen()) {
        Event event;
        while (window.pollEvent(event)) {
            if (event.type == Event::Closed) {
                window.close();
                game.exit(&window);
            }
            game.parseEvent(event, &window);
        }

        sf::Time elapsed = clock.getElapsedTime();
        clock.restart();

        game.update(&window, elapsed.asSeconds());
    }

game.parseEvent() checks if the keyboard/mouse buttons are being pressed (and set booleans accordingly). Then in game.update() I do all the logic, like moving the player, firing a bullet, checking collision between player and other objects, and finally moving the existing bullets (stored in a vector).

In this case I do the event processing (assuming some event has happened), THEN run the update code. So you're saying I should separate event processing and game updating? How would this be done?

[Hapax]

Shouldn't you be checking to see if gun is not ready?

[Dinipants]

Well if the gun is ready I want to fire, else I don't want to do anything. So what does inverting the boolean condition achieve?

[Laurent]

Well if the gun is ready I want to fire, else I don't want to do anything. So what does inverting the boolean condition achieve?

The code that you show is not about firing, it detects when the cool down time is over to make the gun ready again. So yes, it should be "if (gun not ready)".

[Hapax]

The giveaway part that there is something off in the logic is:

if (gunReady)
{
    // ...
    gunReady = true;
}

[Dinipants]

Woops, look like I copied my gun code wrong, sorry:

void fire(float angle, Vector2f playerPos) {
        if (this->gunReady == true) {
            this->gunReady = false;
            // Play fire sound, create bullet, set position, etc.
        }
    }

    void updateBullets(float elapsed) {
        this->cooldownTime += elapsed;
        if (this->cooldownTime >= SHOT_COOLDOWN) {
            this->cooldownTime = 0.0f;
            this->gunReady = true;
        }
        for (auto &bullet : this->sprites) {
            // Move all the bullets
        }
    }

I've fixed it in the post

[Laurent]

Looks like the cooldownTime is running on its own, independently of the gun actually firing. You should:
- reset it to 0 in fire()
- stop updating it when gun is ready

[Mario]

You're not taking into account instances where the time passed is not just exact for one shot.

Let's assume you're allowed to shoot once every 10ms.

So VSync is enabled, your time between frames is 16.67ms. Using this approach, you're firing just once every 16.67ms. If one frame is slower, like 22ms, you're still only firing one shot, despite being able to fire two shots.

So in the end it's just another instance of fix your timestep.

While this won't fix the time between shots, it would fix the number of shots:

while (this->cooldownTime >= SHOT_COOLDOWN) {
    this->cooldownTime -= SHOT_COOLDOWN;
    // do something
}