Unwanted destructor call occurs with stack allocated class - C++

Question

In a Game class function I am allocating a Boundary class to the stack

void Game::loadContent()
{
    Boundary b(this, body); 
}

The boundary class has a pointer to the main Game class and a pointer to a rigid body. I'm not certain whether I should use a reference for each though? Some clarity here would be helpful for reasons explained later.

class Boundary : public DynamicEntity
{
public:
    Boundary(Game *game, btRigidBody *body);
    ~Boundary(void);

    // Override functions
    virtual void draw(float dt);
    virtual glm::vec3 getPosition();
    virtual void update(float dt);
};

The DynamicEntity class assigns the body and handles the pointer deletion in its destructor.

class DynamicEntity : public Entity
{
public:
    virtual ~DynamicEntity(void);

    virtual void draw(float dt) = 0;
    btRigidBody* getBody();
    glm::vec3 getPosition() = 0;
    virtual void update(float dt) = 0;

protected:
    explicit DynamicEntity(Game *game, btRigidBody *body);
    btRigidBody *m_body;
};

DynamicEntity.cpp Destructor

DynamicEntity::~DynamicEntity(void)
{
    m_game->m_dynamicsWorld->removeRigidBody(m_body);

    delete m_body;
}

The DynamicEntity derives from the base class of all game objects called Entity

Entity.h

class Entity
{
public:
    // Make destructor virtual as this is a base class
    virtual ~Entity(void);

    virtual void draw(float dt) = 0;
    int getID();                                
    virtual glm::vec3 getPosition() = 0;
    virtual void update(float dt) = 0;          

protected:
    explicit Entity(Game *game);                // Abstract base constructor

    Game *m_game;
    int m_id;                                   // Unique ID
};

I can't call delete on the Game class pointer in this class' destructor though which is why I am not sure whether passing as a pointer is the correct method (instead of a reference)?

Entity::~Entity(void)
{
    // Derived class destructors are called first
    delete m_game;  // ERROR
}

The Entity class adds a pointer to itself which can be accessed via a list in the Game class (useful for iterating and calling Entity functions in the main Game class).

Entity::Entity(Game *game) 
    : m_game(game),                                         // Initialise members
    m_id(m_game->g_idGenerator->generateNewID())            // Generate unique ID                       
{
    m_game->m_entities.push_back(std::shared_ptr<Entity>(this));
}

The main problem I am having is that once the Game::loadContent() method has finished the destructor is called for the Entity class. This ruins the *shared_ptr* stored in the list and errors occur when trying to call any of the virtual methods.

I would like the Boundary pointer to persist until I say delete. Is there any way of doing this without allocating the Boundary to the heap?

EDIT

In response to the suggestion for using const& Game

It would appear that I have to change my Entity header to the following

Entity.h

#pragma once

#include <glm\glm\glm.hpp>

#include "Game.h"

// Forward declarations
class Game;

class Entity
{
public:
    // Make destructor virtual as this is a base class
    virtual ~Entity(void);

    // '= 0' means pure virtual function (like 'abstract' in C#)
    // This means they do not have to be declared in the source file '.cpp'
    virtual void draw(float dt) = 0;
    int getID();                                
    virtual glm::vec3 getPosition() = 0;
    virtual void update(float dt) = 0;          

protected:
    explicit Entity(const Game &game);          // Abstract base constructor

    Game m_game;
    int m_id;                                   // Unique ID
};

Doesn't the Game m_game allocate an instance of the Game class to the stack? How should it be declared in the header if it to represent a reference?

EDIT 2

If I store a protected reference to the Game class in the base Entity class const Game &m_game I cannot seem to access a global member of the Game class g_wireShapeDrawer in derived classes.

class Game
{
public:
    GL_WireShapeDrawer g_wireShapeDrawer;

    Game(void);
    ~Game(void);

    void init();
    void draw(float dt);
    void handleInput(float dt);
    void loadContent();
    void update(float dt);
};

For example I get the following error when trying to access a global member in the draw method of the derived Boundary class source

void Boundary::draw(float dt)
{
    m_game.g_wireShapeDrawer.drawPlane(glm::vec3(0, 1, 0), 0.0f, glm::vec4(1, 1, 1, 1));
}

error C2662: 'GL_WireShapeDrawer::drawPlane' : cannot convert 'this' pointer from 'const GL_WireShapeDrawer' to 'GL_WireShapeDrawer &

Why is this?

Answer 1

The Game object should never be deleted from any Entity or derived class. It should be one of the last things to be deallocated before the application shuts down.

You should pass it to your Entity classes as a Game&. Why? Because you only have one instance of the Game, so there is no need to reset what it points to, and it always should be valid (since the game will exist before the Entity objects do).

Another option is to implement the Singleton Pattern in your Game class, and access it like this Game::GetInstance().m_dynamicsWorld->removeRigidBody(m_body);

As per your edit, you can create an Entity using the initializer list. This way you can store const members, like so:

class Entity
{
  protected:
    explicit Entity(Game &game) : m_game(game) {}
  private:
    Game& m_game;
}

Answer 2

Your design is flawed. You need to clearly state (through your design) who owns the pointer. If Entity owns the pointer then it should deallocate it in its destructor as you are doing (better yet; just wrap it in a std::unique_ptr). If it does not own the pointer then simply don't deallocate it.

You cannot have it both ways. You're using a shared_ptr, so that implies multiple "owners" and, once the last owner is done with it, the memory will be deallocated. Again, you needs to clearly design around who owns this memory.

Judging from your code, it seems like Entity does not really own the Game*. It needs it for implementation reasons, but should not be responsible for its deallocation.

---

on a side note, you are violating The Rule of Three.