Screen manager and its screens having access over the screen manager

I've got this here screen manager system and while it works rather well, I do worry on how practical it all is.

The application creates a screen manager class then enters a message loop which calls the update, render and handle input methods of the screen manager.

The screen manager has a vector container of unique_ptr's.
The main container type is a virtual screen class. It looks like this.

I should mention that vgSreen has a forward declaration of screenManager.

std::vector> gameScreens;
this->AddScreen(std::unique_ptr(new MainMenu()));

When the screen manager adds a screen it loads the screens content and initializes it, then adds it to the stack. When it initializes a screen it passes a pointer of itself like this. From here on I'm skeptical about the performance of the program.

gameScreenToAdd.Initialize(*this);

The game screen stores the screen manager parameter as a pointer:

bool MainMenu::Initialize(ScreenManager* sManager)
{
    //manager is a ScreenManager* pointer
    this->manager = sManager;
}

Then when a screen wants to add or remove itself or another screen it can do the following:

manager->AddScreen(std::unique_ptr(new Gameplay()));

I believe the erase function may result in errors because when a unique_ptr is added to the vector, the vector size increases, but erasing the vector doesn't then resize the container.

bool ScreenManager::RemoveScreen(int index)
{
    unsigned int a = GetCount();

    gameScreen[index]->Shutdown();

    gameScreen[index].release();

    gameScreen.erase(gameScreen.begin() + index);

    return true;
}

If there's a better alternate than using a vector of unique pointers, or if I'm leaking horrendously by passing screen manager to each screen, let me know. Or if you have any thoughts on this in general.

I just happened to code my own StateMachine a week ago, which is similar to your ScreenManager, so here are my thoughts on the subject.

A vector of state can work, but I think using a stack instead saves you the index management. Also, the stack would resize itself during both pop and push operations.

A state (or a screen in your case) manages its own lifetime. As a consequence, it can remove itself from the stack during an update operation, and must survive until the next loop is processed. Thus, I choosed std::shared_ptr to work out this problem.

std::stack> _screens;


Since you're using C++11, the more you avoid manual dynamic allocations, the better. Using templates, you can use a safer way of creating your screen, and even provide parameters to them :

template
void push_screen (Args && ... args)
{
if (!_screens.empty())
_screens.top()->pause(); // Pause the current state

_screens.emplace(std::make_shared(std::forward(args)...));
// Or, if you still use std::unique_ptr
// _screens.emplace(new Screen { std::forward(args)... }); // C++11
// _screens.emplace(std::make_unique(std::forward(args)...)); // C++14
_screens.top()->enter(); // Start the new state
}


This way, you can pass a reference of your ScreenManager and avoid using a bare pointer.

push_screen(*this);


StackExchange Code Review Q#54434, answer score: 5