0

I have three base Classes {Player,Weapon,Game} and one inherited Class Warrior from Player. The code I added may not be minimal but I added anything that can be relevant. Since I didn't know where to look.

This is my test for simple debugging, I add Players into vectorPlayers which is definded in game.h

int main() {
    Weapon w1("hello",STRENGTH,3);
    Game game(3);
    game.addWarrior("s22","we",STRENGTH,3,55);
    //4
    game.addWarrior("s22","we",STRENGTH,3,55);
    return 0;
}

When I reach the the 4th line, vectorPlayers[0] includes the correct inputs. But when I enter to game.cpp and call the function addPlayer in addWarrior,suddenly players_name for example switches to error: Cannot access memory at address 0x1

//inside game.cpp


GameStatus Game::addPlayer(const string playerName, const string weaponName,
                                   Target target,int hit_strength)
        {
            Weapon newWeapon(weaponName,target,hit_strength);
            Warrior newWarrior(playerName,newWeapon,false);
            if(ifNameAlreadyExists(playerName)) {
                addRegPlayer(static_cast<Player*>(&newWarrior));
            }
            return SUCCESS;
        }

        GameStatus Game::addRegPlayer(Player* player)
        {
            playersVector.push_back(player);
        }


        //base classes

            class Game {

            private:
                int maxPlayer;
                vector<Player*> playersVector;
        }

        class Player {
                string player_name;
                int level;
                int strength;
                Weapon player_weapon;
            protected:
                int place;
                int life;
            public:
            Player(const string &name, const Weapon &weapon);
            virtual ~Player();
                }


        //inherited class

        class Warrior : public Player {
        private:
            bool rider;
        public:
            Warrior ();
            Warrior (string const& name, Weapon const& weapon, bool rider);
            ~ Warrior () override = default;
            Warrior &operator=(const Warrior &warrior) ;
            Warrior ( const Warrior &warrior) = default;
            void makeStep() override;
        };


        //Constructor of Warrior
        Warrior::Warrior(string const& name, Weapon const& weapon, bool rider) :
                Player(name,weapon),rider(rider){
            if (weapon.getTarget() == LEVEL){
                throw IllegalWeapon();
            }
        }
        //constructor of player
        Player::Player(const string& name, const Weapon& weapon) :
                level(1),life(1),strength(1),player_weapon(weapon),player_name(name),place(0){
        }
Sam12
  • 1,662
  • 2
  • 11
  • 17
  • 2
    ProTip: If you find yourself using static_cast, you are probably doing something wrong. newWarrior needs to be declared on the heap using new. You are caching pointers to dead autovariables. – Kyle Huff Jun 21 '18 at 23:37
  • I just removed it and it still worked:) @KyleHuff – Sam12 Jun 21 '18 at 23:41
  • 2
    The big problem I see is you're declaring a _local_ instance of Warrior, then pushing that pointer into the vector. That instance is going to be destructed when you leave the function and you'll have a dangling pointer to garbage. Accessing that is going to any number of bad things. If you're on linux try running valgrind on it to see if it gives you any other warnings. – gct Jun 21 '18 at 23:41
  • If I pass newWarrior to addRegPlayer by reference it would be better? @SeanMcAllister – Sam12 Jun 21 '18 at 23:45
  • 2
    No. An automatic variable gets destroyed when it goes out of scope. You need to allocate it on the heap so it will only get destroyed when you destroy it explicitly (e.g., in a `deletePlayer` or `removePlayer`). – Jerry Coffin Jun 21 '18 at 23:46
  • Can you give me an example please? on how to allocate it on the heap @JerryCoffin – Sam12 Jun 21 '18 at 23:48
  • 3
    Usually, `std::unique_ptr f = std::make_unique();` Rarely, std::shared_ptr f = std::make_shared();`. For those, you should have a `vector>` or `vector>` respectively. With a definition of `vector`, you can use something like: `Player *p = new Player("Thor", "Hammer"); playerVector.push_back(p);`. I'd recommend learning to use `std::unique_ptr` instead though, if at all possible. – Jerry Coffin Jun 21 '18 at 23:53
  • 3
    When Jerry Coffin says to try to use std::unique_ptr, that is preferable because there is one-and-only-one owner, which is a more common scenario. The std::shared_ptr is for shared ownership, which does happen, but ought to only be selected when it is really needed rather than "just in case". – Eljay Jun 21 '18 at 23:57
  • It worked!! Thank you so much all. – Sam12 Jun 21 '18 at 23:59
  • It worked without unique pointers though, is it crucial to use them? – Sam12 Jun 22 '18 at 00:00
  • 1
    unique_ptr will delete the underlying object when the unique_ptr is destructed. If you use raw pointers you have to clean them up yourself. Read about memory management in C++, it doesn't hold your hand like eg: Java. Be careful or you'll leak memory all over the place. – gct Jun 22 '18 at 00:09
  • 1
    `unique_ptr` is one of [what's called a Smart Pointer](https://stackoverflow.com/questions/106508/what-is-a-smart-pointer-and-when-should-i-use-one). Smart pointers build in a number of safety features that prevent common errors when dealing with pointers and enforce ownership (who is responsible for the maintenance and ultimately release) of a resource. It is not crucial to use them, but you're a fool if you don't take advantage of them when you can. They can make your life orders of magnitude easier and your code much, much more robust. – user4581301 Jun 22 '18 at 00:13
  • Semi-related: I'm trying to find a good link that explains the concept of Ownership. If any of you out there have one, please share it. – user4581301 Jun 22 '18 at 00:14

0 Answers0