Why can the variable change its value suddenly if I don't change it from the code?

Question

In my code I create 3 shots after the user presses ENTER. There are delays int NEXT_SHOT_IN_CHAIN_DELAY between each other. When the ENTER is pressed I create three objects of class Shot and add them in the vector std::vector <Shot*> shots. The class Shot changes the inside statement from: 0 to 2

WAIT_FOR_SHOT_START -> BULLET_LEFT_WEAPON -> BULLET_IN_ATTACK_ZONE

According to the current time from the start of the program, when I create an exemplar of the class shot - the number of the bullet and the start time to leave the weapon (variable startShotTime) has normal values.

Console output:

    DEBUG: Bullet by creation: 0; Start shot time: 3833; End shot time: 4583 and statement: 0; Actual time: 4583
    DEBUG: Bullet by creation: 1; Start shot time: 4163; End shot time: 4913 and statement: 0; Actual time: 4913
    DEBUG: Bullet by creation: 2; Start shot time: 4493; End shot time: 5243 and statement: 0; Actual time: 5243

On every frame I update all the shots. And in the updating function my logger tells me that the bulletNumber and startShotTime have already new (strange) values.

Console output:

    DEBUG: Bullet 258744461 Start shot time: 575665160
    DEBUG: Bullet 575663496 Start shot time: 3744914323
    DEBUG: Bullet 575663688 Start shot time: 575663624

What can change the values of my private variables?

Class Bullet is only the graphic demonstration of the bullets on the screen. It does'n affect the Shot class.

If it's important: I use VisualStudio on Windows 10 and SDL2 library.

Here is my code, BulletController.h:

    #pragma once
    #include <vector>
    #include "Timer.h"
    #include "Logger.h"
    #include <vector>
    #include "FightersController.h"
    #include "Bullet.h"
    #include "Random.h"
    #include "Shot.h"
    #include "EventsListener.h"
    
    class BulletController{
        public:      
            BulletController(FightersController* fightersController, Bullet *bullets, int bulletsNumber);
             ~ BulletController();       
             void update(long deltaTime);
    
        private:
            int bulletsNumber;
            const int BULLETS_PART_TO_BE_RENDERED_IN_PERCENTS = 60;
            Bullet* bullets;
            FightersController* fightersController = nullptr;           
            const int NOTHING_TO_DELETE =-1;
            std::vector <Shot*> shots;      
            int shotNumberToBeDeleted=NOTHING_TO_DELETE ;       
            void makeExplosion(int numberOfBullet); 
            void updateShooting();
            void startToShotIfPossible();
            void updateActualBullets();
            void setVisibilityForBullets();
            const int bulletsInLine = 3;
            const int NEXT_SHOT_IN_CHAIN_DELAY = 330;
            bool shotAlreadyPressed;
            //Mutable
            Random* mutRandomizer;
    };

BulletController.cpp:

    #include "BulletController.h"
    
    
    BulletController::BulletController(FightersController* fightersController, Bullet *bullets, int bulletsNumber){
        this->bullets = bullets;
        this->bulletsNumber = bulletsNumber;
        this->fightersController = fightersController;  
        mutRandomizer = new Random();
    }
    
    BulletController::~BulletController(){
        delete mutRandomizer;   
        delete bullets;
    }
    
    void BulletController::update(long deltaTime){
        updateActualBullets();
        updateShooting();   
    }
    
    void BulletController::updateShooting() {   
        std::vector <Command>* commands = EventsListener::getInstance()->getCommands();
        bool shotCommandFounded = false;
        if (commands->size() > 0) {             
            for (auto i = (commands->begin()); i != commands->end(); ++i) {
                Command command = (Command)*i;  //This copies the command
                if (command.getType() == command_constants::SHOT) {
                    Logger::debug("Player pressed shot");
                    if (!shotAlreadyPressed) {
                        shotAlreadyPressed = true;
                        startToShotIfPossible();
                    }
                    shotCommandFounded = true;
                }   
            }       
        }
        if (shotAlreadyPressed) {
            if (!shotCommandFounded) {
                shotAlreadyPressed = false;
            }
        }
    }
    
    void BulletController::updateActualBullets() {      
        if (shots.size() > 0) {
            int toBeDeleted = -1;       
            int itterator = 0;
            for (auto i : shots) {
                i->update();
    
            }       
            for (auto i : shots) {          
                if (i->isInHitZone()) {
                    
                    makeExplosion(itterator);
                    toBeDeleted = itterator;
                }
                itterator++;
            }       
            if (toBeDeleted >= 0) {         
                shots.erase(shots.begin() + toBeDeleted);   
            }
        }   
    }
    
    void BulletController::startToShotIfPossible() {    
        Shot firstShot(0);
        shots.push_back(&firstShot);
        Shot secondShot(NEXT_SHOT_IN_CHAIN_DELAY);
        shots.push_back(&secondShot);
        Shot thirdShot(NEXT_SHOT_IN_CHAIN_DELAY*2);
        shots.push_back(&thirdShot);
        setVisibilityForBullets();
        Logger::debug("Shot started");  
    }
    
    void BulletController::setVisibilityForBullets() {
        int count = 0;
        for (int i = 0; i < bulletsNumber; i++) {
            int randomValue = mutRandomizer->nextInt(100);
            if (randomValue <= BULLETS_PART_TO_BE_RENDERED_IN_PERCENTS) {
                bullets[i].activateForTime(1, 1000);
                count++;
            }
        }
        Logger::debug(std::to_string(count) + " bullets will be visible by this shot");
    }
    
    void BulletController::makeExplosion(int numberOfBullet){
        shotNumberToBeDeleted = numberOfBullet;
        fightersController->handleAttack();
    }

Shot.h:

    #pragma once
    #include <SDL.h>
    #include "Logger.h"
    #include <string>
    
    class Shot
    {
    
    public:
        Shot(int timeBeforeStart);  
        ~Shot();    
        void update();
        static const int WAIT_FOR_SHOT_START = 0;
        static const int BULLET_LEFT_WEAPON = 1;
        static const int BULLET_IN_ATTACK_ZONE = 2;
        bool isInHitZone();
    
    private:
        const int SHOT_TIME = 750;  
        int statement;  
        unsigned long int startShotTime;
        unsigned long int endShotTime;  
        int bulletNumber;       
    };

Shot.cpp:

    #include "Shot.h"
    
    static int bulletCount = 0;
    
    Shot::Shot(int timeBeforeStart) {
        bulletNumber = bulletCount;
        bulletCount++;
        startShotTime = SDL_GetTicks() + timeBeforeStart;
        endShotTime = SDL_GetTicks() + timeBeforeStart + SHOT_TIME; 
        statement = WAIT_FOR_SHOT_START;    
        Logger::debug("Bullet by creation: " + std::to_string(bulletNumber) + "; Start shot time: " + std::to_string(startShotTime) + "; End shot time: " + std::to_string(endShotTime) + " and statement: " + std::to_string(statement) + "; Actual time: " + std::to_string(endShotTime));
    }
    
    Shot::~Shot() {
    
    }
    
    void Shot::update() {
        Logger::debug("Bullet " + std::to_string(bulletNumber) + " Start shot time: " + std::to_string(startShotTime));
        if (statement == WAIT_FOR_SHOT_START) {
            if (SDL_GetTicks() >= startShotTime) {
                statement == BULLET_LEFT_WEAPON;
                Logger::debug("Bullet " + std::to_string(bulletNumber) + " left the weapon at " + std::to_string(SDL_GetTicks()));
            }
        }
        else if (statement == BULLET_LEFT_WEAPON) {
            if (SDL_GetTicks() >= endShotTime) {
                statement == BULLET_IN_ATTACK_ZONE;
                Logger::debug("Bullet " + std::to_string(bulletNumber) + " is in hit zone at " + std::to_string(SDL_GetTicks()));
            }
        }
    }
    
    bool Shot::isInHitZone() {  
        if (statement == BULLET_IN_ATTACK_ZONE) return true;
        else return false;
    }

Please try to create a [mre] to show us. By minimizing the code needed to replicate the problem, it's also possible that you will find out the problem yourself. — Some programmer dude, Jul 20 '23 at 07:21
You code is full of undefined behaviours. For example `Shot firstShot(0); shots.push_back(&firstShot);` - you store a pointer to a local variable that doesn't exist after the function exits. — 273K, Jul 20 '23 at 07:21
As for your problem, when data seems to change unexpectedly there usually three common reasons: 1) You have a buffer overflow of some kind, where you modify data out of bounds of allocated memory; Or 2) You get the wrong object and modify it; Or 3) You have references or pointers to objects or variables that doesn't exist anymore. — Some programmer dude, Jul 20 '23 at 07:22
To be honest, this looks like an attempt of C++ without learning C++ properly. Things like storing addresses of local variables -- that is something that should be taught very early when learning C++. — PaulMcKenzie, Jul 20 '23 at 07:25
Looks like you have not learned c++ from a good source. Good sources to learn cpp from are : A [recent C++ book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) or have a go at https://www.learncpp.com/ (that's pretty decent, and pretty up-to-date). For C++ reference material use : [cppreference](https://en.cppreference.com/w/). And after you learned the C++ basics from those sources, look at the [C++ coreguidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines) regularely to keep up-to-date with the latest guidelines. — Pepijn Kramer, Jul 20 '23 at 07:26
What stands out is the useless use of pointers, inline logging (logging should not be used for debugging) use a debugger for that. And logic should be tested by unit tests. Logging in time critical parts of your code has too much impact on performance (leading to other bugs). Also the use of `this->` should not be necessary there are better ways of initializing your member variables. Also try to make a design not relying on global variabls, read up on dependency injection. — Pepijn Kramer, Jul 20 '23 at 07:27
@273K does it means my shots don't exist after the function ends? Should I place my shots (not pointers) in the array? — Alexander Gorodilov, Jul 20 '23 at 07:33
In addition, the excessive usage of `new` to create objects suggests you are coming from another computer language, where `new` is used to create objects. There is no need to use `new` to create objects in C++, and if you must create objects dynamically, use smart pointers such as `unique_ptr` or `shared_ptr`. — PaulMcKenzie, Jul 20 '23 at 07:33
@PaulMcKenzie you are right. I have a large Java experience and started to learn C++ — Alexander Gorodilov, Jul 20 '23 at 07:34
Well, usage of `new` and `new[]` in this day and age of C++ should be rare in a client program. Maybe if you were creating your own low-level data structure you would use it, but not in such a program you are attempting to develop. To start with `std::vector shots;` instead of `std::vector shots;` -- Start to work with that to slowly eliminate the usage of `new` — PaulMcKenzie, Jul 20 '23 at 07:35
@PaulMcKenzie Do you think I should use objects directly or smart pointers on the objects? — Alexander Gorodilov, Jul 20 '23 at 07:36
@AlexanderGorodilov That very much depends on your requirements — john, Jul 20 '23 at 07:40
@john, well, I don't transfer this vector with shots. It lays always in the ShotController class. — Alexander Gorodilov, Jul 20 '23 at 07:42
Another option for "suddenly changes" is the code in update `statement == BULLET_LEFT_WEAPON;` which **doesn't** change the value. So it then "unexpectedly" keeps its original value. — BoP, Jul 20 '23 at 07:43
@BoP Thanks, It is very useful! I fix it. But the code doesn't reach this place — Alexander Gorodilov, Jul 20 '23 at 07:49
@AlexanderGorodilov Then using objects directly seems like the right thing to do. — john, Jul 20 '23 at 09:50

score 1 · Answer 1 · answered Jul 20 '23 at 07:58

void BulletController::startToShotIfPossible() {    
    Shot firstShot(0);
    shots.push_back(&firstShot);
    Shot secondShot(NEXT_SHOT_IN_CHAIN_DELAY);
    shots.push_back(&secondShot);
    Shot thirdShot(NEXT_SHOT_IN_CHAIN_DELAY*2);
    shots.push_back(&thirdShot);
    setVisibilityForBullets();
    Logger::debug("Shot started");  
}

This function creates three Shot variables in the current stackframe. It stores the addresses of those variables in the shots vector.

However, the memory allotted for that stackframe may be reused once that function exits, and other data may be written at the addresses where those variables previously existed. However, you still have those addressed stored, even though they may no longer point to any valid data.

This leads to undefined behavior.

You are right, I thought It should work like in Java. – Alexander Gorodilov Jul 20 '23 at 09:37 — Alexander Gorodilov, Jul 20 '23 at 09:37

Why can the variable change its value suddenly if I don't change it from the code?

1 Answers1