C++ Syntax errors

Question

I'm trying to make a kind of screen manager in C++ but I'm getting errors. With my code below I receive

1>screenmanager.cpp(26): error C2664: 'void std::vector<_Ty>::push_back(_Ty &&)' : cannot convert parameter 1 from 'virtualGameScreen' to 'virtualGameScreen *&&'
1>          with
1>          [
1>              _Ty=virtualGameScreen *
1>          ]
1>          Reason: cannot convert from 'virtualGameScreen' to 'virtualGameScreen *'
1>          No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called
1>

Errors occur with gameScreen.push_back(gameScreenToAdd); I get access violation error when adding a reference operator with gameScreenToAdd.

ScreenManager.h

void AddScreen(virtualGameScreen);
void RemoveScreen(virtualGameScreen);

ScreenManager.cpp

std::vector<virtualGameScreen*> gameScreen;

void ScreenManager::Initialize(void)
{
    MainMenu menu = MainMenu();

    AddScreen(menu);
}

void ScreenManager::AddScreen(virtualGameScreen gameScreenToAdd)
{
    gameScreenToAdd.LoadContent();

    gameScreen.push_back(gameScreenToAdd);
}

So, I've hit a bit of a wall, any suggestions on how I might fix this?

edit game runs if I change gameScreen.push_back to gameScreen.push_back(new MainMenu()); buut that's not really how I want the function to work

Answer 1

So, the first thing the compiler did is tell you where the problem occurs:

1>screenmanager.cpp(26)

It also told you primarily what the problem is:

Reason: cannot convert from 'virtualGameScreen' to 'virtualGameScreen *'

So - something in your code is providing a "virtualGameScreen" object instance where it was expecting a pointer (denoted by the *). And it's on line 26. The other parts of the error indicate it's the call to push_back. Lets look at line 26:

    gameScreen.push_back(gameScreenToAdd);

Yep - you're calling push_back, and you're passing it gameScreenToAdd, which is of type virtualGameScreen. The push_back call is from this vector:

std::vector<virtualGameScreen*> gameScreen;

Your vector expects pointers, so the push_back expects vectors.

HOWEVER: You can't just do this:

gameScreen.push_back(&gameScreenToAdd);

because gameScreenToAdd is a temporary function variable - when you call AddScreen, the original variable is copied into a new, temporary virtualGameScreen for the lifetime of the function call. That means when the program leaves AddScreen the screen whose address you pushed will no-longer exist (the memory is still there, but it has been released and the computer will now proceed to use it for other reasons).

What you'll need to do is change AddScreen to take a pointer.

void ScreenManager::AddScreen(virtualGameScreen* gameScreenToAdd)
{
    gameScreenToAdd.LoadContent();

    gameScreen.push_back(gameScreenToAdd);
}

Unfortunately, this leaves you open to yet another problem with your code.

void ScreenManager::Initialize(void)
{
    MainMenu menu = MainMenu();

    AddScreen(menu);
}

This function creates a temporary, local MainMenu object - with a lifetime of the duration of Initialize. Then it creates a second, temporary MainMenu and copies it to menu.

If you write

    AddScreen(&menu);

it will work, but it will pass the address of a temporary instance to AddScreen.

As soon as program flow leaves the "Initialize()" function, your value goes away.

It looks like you may have some prior experience with something like Java or C# and are trying to apply previous knowledge to C++.

What you need is a member variable to store "Menu" for the life time of the instance of ScreenManager.

Option 1: Just use a class member variable.

class ScreenManager
{
    MainMenu    m_menu;

public:
    ScreenManager()
        : m_menu() // initialize menu while we are initializing.
    {}

    void Initialize()
    {
        AddScreen(&m_menu);
    }
// ...
};

If you really want to use a pointer, you might do the following:

class ScreenManager
{
    MainMenu*   m_menu;

public:
    ScreenManager()
        : m_menu(nullptr) // make sure it's null as soon as the object is created
    {}

    void Initialize()
    {
        m_menu = new MainMenu();
        AddScreen(m_menu);
    }

    // but now we have to make sure it is released when we go away

    ~ScreenManager()
    {
        if (m_menu)
        {
            delete m_menu;
            m_menu = nullptr;
        }
    }
};

Option 3: use C++ containers to manage the lifetime of the pointer for you, either std::unique_ptr or std::shared_ptr

---- EDIT ----

Seeing the edit you made while I was writing this, it's a little clearer what you're trying to do. What you probably want is something more like this:

std::vector<std::unique_ptr<virtualGameScreen>> gameScreen;

Consider the following:

Live demo: http://ideone.com/7Th2Uk

#include <iostream>
#include <vector>

class Foo {
    const char* m_name;
public:
    Foo(const char* name) : m_name(name) { std::cout << "Foo " << m_name << '\n'; }
    ~Foo() { std::cout << "~Foo " << m_name << '\n'; }
};

int main() {
    std::vector<Foo*> foos;
    Foo foo("foo");
    foos.push_back(new Foo("new"));

    return 0;
}

Note that the second foo is never released.

Foo foo
Foo new
~Foo foo

std::unique_ptr is a pointer-container object which will delete the object when the object expires. This makes it suitable for use in a container like std::vector

#include <iostream>
#include <vector>
#include <memory> // for std::unique_ptr

class Foo {
    const char* m_name;
public:
    Foo(const char* name) : m_name(name) { std::cout << "Foo " << m_name << '\n'; }
    ~Foo() { std::cout << "~Foo " << m_name << '\n'; }
};

int main() {
    std::vector<std::unique_ptr<Foo>> foos;
    Foo foo("foo");
    foos.emplace_back(new Foo("new"));

    return 0;
}

Both objects get cleaned up:

Foo foo
Foo new
~Foo foo
~Foo new

Now you don't need your m_menu at all, you can simply call AddScreen with a 'new MainMenu()' and the pointer will be added to the vector such that when the vector goes out of scope, proper cleanup will happen.

Menu* menu = new MainMenu();
AddScreen(menu);

or

AddScreen(new MainMenu());

In theory what you should really do is ensure that the allocation goes straight into a unique_ptr object so that there's no window for it to get leaked, but teaching the use of std::unique_ptr is beyond the scope of this answer. http://msdn.microsoft.com/en-us/library/hh279676.aspx, http://www.drdobbs.com/cpp/c11-uniqueptr/240002708, etc.

Answer 2

In pre-C++11 code, you might have something like this:

std::vector<virtualGameScreen*> gameScreen;

void ScreenManager::Initialize(void)
{
    AddScreen(new MainMenu);
}

void ScreenManager::AddScreen(virtualGameScreen *gameScreenToAdd)
{
    gameScreenToAdd->LoadContent();

    gameScreen.push_back(gameScreenToAdd);
}

but you would have to have some way to make sure the object got deleted.

With C++11, you would probably want to have the memory managed automatically:

std::vector<std::unique_ptr<virtualGameScreen>> gameScreen;

void ScreenManager::Initialize(void)
{
    AddScreen(std::unique_ptr<MainMenu>(new MainMenu));
}

void ScreenManager::AddScreen(std::unique_ptr<virtualGameScreen> gameScreenToAdd)
{
    gameScreenToAdd->LoadContent();

    gameScreen.emplace_back(std::move(gameScreenToAdd));
}

Answer 3

That's because you did not provide a pointer to the vector (gameScreen), and another issue about the code is that: the paramater will generate a temp object, if just put the address of it the app maybe crash.