Hello, I have a std::vector of pointers to my Tile class:

std::vector<Tile*> m_Map;

The tiles are added like so(depending on their id):

if(id == 0){

	Tile temp(x * 32, y * 32, solid, border);
	m_Map.push_back(&temp);
}

and when I render the tiles it does not even go to the function:

void Level::render(sf::RenderWindow& window, const sf::Vector2f& scrRegion, const sf::Vector2f& scrSize){

	//render the tile if it is on the screen
	for(auto iter = m_Map.begin(); iter != m_Map.end(); ++iter){

		(*iter)->render(scrRegion, scrSize, window); //<- does not go to the function
	}
}

I believe the problem is with (*iter)->render(scrRegion, scrSize, window). I looked online but I could not find a way to fix this. Thanks for any help.

The problem is how your adding tiles to the vector.

if(id == 0){
	//Creates a tile object located on the stack
	Tile temp(x * 32, y * 32, solid, border);
	//Adds a pointer to that tile on the stack to the vector
	m_Map.push_back(&temp);
	//The tile of the stack is getting destroyed here, so the pointer in the vector becomes invalid
}

You can either store the Tiles by value (e.g. if they are not very big, you are not going to adds types derived from Tile and copying them doesn't cause other problems).

Or you need to dynamically allocate the tiles you are putting in the vector, so they don't get destroyed when the code leaves the stack frame.

//By value
std::vector<Tile> m_valueMap;
...
if (id == 0)
{
	Tile temp(...);
	m_valueMap.push_back(temp);//Safe makes a copy
}


//By pointer. Using std::unique_ptr here so that when m_ptrMap is destroyed or you remove tiles from it does not create a memory leak,
//the unique_ptr object deletes the object automatically.
std::vector<std::unique_ptr<Tile> > m_ptrMap;
...
if (id == 0)
{
	//Dynamically allocate the tile so that it may outlive the stack frame
	std::unique_ptr<Tile> temp(new Tile(...));
	//Transfer ownership to the vector
	m_ptrMap.push_back(std::move(temp));
}

@Crusable what SyncViews says is correct and you're trying to store temporaries in a vector, but I also see something else.

Although it is harmless, you seem to be using the auto keyword in your for loop here:

	for(auto iter = m_Map.begin(); iter != m_Map.end(); ++iter){
		(*iter)->render(scrRegion, scrSize, window); //<- does not go to the function
	}

But why not go the distance, since you are already using c++11, and use a range based for loop instead of explicitly using iterators?

	for (auto tile : m_Map)
        {
		tile->render(scrRegion, scrSize, window);
	}

Isn't this the nice?

Thank you.

If you'd like to type less, simply use emplace_back member function (it's even better than moving -- it emplaces the element, i.e., constructs it directly in the container).

Note, however, that there are some caveats: http://stackoverflow.com/questions/13172888/is-it-safe-to-use-emplace-back-with-a-container-of-unique-ptrs

// That's why I simply used "reserve".

Example: http://ideone.com/2f6DNU

Code:

#include <cstddef>
#include <iostream>
#include <memory>
#include <vector>

struct X
{
    X() { ++count; std::cout << "Constructing, count = " << count << "\n"; }
    ~X() { --count; std::cout << "Destructing, count = " << count << "\n"; }
    
    static std::size_t count;
};
std::size_t X::count = 0;

int main()
{
    std::vector<std::unique_ptr<X>> vec;
    vec.reserve(2);
    
    // one way
    vec.push_back(std::unique_ptr<X>(new X));
    
    // another way
    vec.emplace_back(new X);

    return 0;
}

Output:

Constructing, count = 1
Constructing, count = 2
Destructing, count = 1
Destructing, count = 0

Does emplace_back here gain any benefit over push_back for a unique_ptr by the time you have to remember to do the reserve to be exception safe?

Mostly this: http://stackoverflow.com/questions/13172888/is-it-safe-to-use-emplace-back-with-a-container-of-unique-ptrs#comment17926086_13172922

In general, I'd say this depends on your scenario -- often, in game dev you have to preallocate anyways for performance and you know the size of your game assets in advance, so calling "reserve" is arguably already a "recommended best practice". However, there are myriad other uses of std::vector that may not fit this use case, and so you have to adjust accordingly. Such is the life of a programmer ;-)

True I guess, but every time I really needed performance out of a vector or string I generally wanted something more like "resizeButDontInitialise" or "pushBackWithoutBoundsCheck", and generally ended up using my own thing (e.g. times when push_back became 50% of an algorithms time even with a reserve, or resize then spent 50% of its time in memset)