Reputation: 1429
I have a TileMap
class that has a std::vector<Tile>
. While just generating the vector, i notice that the Tile
s are getting deleted shortly after creation, thus not letting the TileMap
class do anything with them.
TileMap
is a kind of information class that will be used by a Stage
class for various things, so it will need to access TileMap.tiles()
(which returns the mTiles_
TileMap
.
TileMap constructor:
TileMap::TileMap(std::vector<int> pTiles, int pWidth):mWidth_(pWidth)
{
for(int i = 0; i < pTiles.size(); i++)
{
int x = (i % mWidth_);
int y = floorf(i / mWidth_);
Tile tile((Tile::TileType)pTiles[i]);
tile.x = x;
tile.y = y;
tile.position(sf::Vector2f(x * Tile::TILE_WIDTH, y * Tile::TILE_HEIGHT));
mTiles_.push_back(tile);
}
}
Previously it was a std::vector<std::shared_ptr<Tile>>
but i was seeing if i could get around using pointers. Is there a way to do this?
EDIT: Tile definition added -
class Tile : public SquareCollidableObject
{
public:
enum TileType {
TILE_GRASS,
TILE_OUTSIDE_WALL_TOP_LEFT_OUTER,
TILE_OUTSIDE_WALL_TOP,
TILE_OUTSIDE_WALL_TOP_RIGHT_OUTER,
TILE_OUTSIDE_WALL_LEFT,
TILE_OUTSIDE_WALL_RIGHT,
TILE_OUTSIDE_WALL_BOTTOM_RIGHT_INNER,
TILE_OUTSIDE_WALL_BOTTOM_LEFT_INNER,
TILE_OUTSIDE_WALL_BOTTOM_LEFT_OUTER,
TILE_OUTSIDE_WALL_BOTTOM,
TILE_OUTSIDE_WALL_TOP_RIGHT_INNER,
TILE_OUTSIDE_WALL_TOP_LEFT_INNER,
TILE_OUTSIDE_WALL_BOTTOM_RIGHT_OUTER,
TILE_WALL,
TILE_INSIDE_WALL_TOP_LEFT_INNER,
TILE_INSIDE_WALL_TOP,
TILE_INSIDE_WALL_TOP_RIGHT_INNER,
TILE_INSIDE_WALL_LEFT,
TILE_INSIDE_WALL_RIGHT,
TILE_INSIDE_WALL_BOTTOM_RIGHT_OUTER,
TILE_INSIDE_WALL_BOTTOM_LEFT_OUTER,
TILE_INSIDE_WALL_BOTTOM_LEFT_INNER,
TILE_INSIDE_WALL_BOTTOM,
TILE_INSIDE_WALL_TOP_RIGHT_OUTER,
TILE_INSIDE_WALL_TOP_LEFT_OUTER,
TILE_INSIDE_WALL_BOTTOM_RIGHT_INNER,
TILE_FLOOR
};
Tile(TileType);
virtual ~Tile();
virtual void update(float);
virtual void draw(sf::RenderWindow&, sf::Vector2f);
TileType tileType;
static int TILE_WIDTH;
static int TILE_HEIGHT;
int x;
int y;
// pathfinding
std::shared_ptr<Tile> previousTile;
float g; // cost to tile (total cost from previous tiles + cost to this tile)
float h; // cost to next tile
float f; // g + h
bool walkable;
};
Upvotes: 1
Views: 890
Reputation: 37488
First of all, your TileMap constructor calls .position
which isn't a member of the Tile
class.
Secondly, @tmlen's answer looks like it works as expected to me. If I run this code:
#include <stdlib.h>
#include <memory>
#include <vector>
#include <iostream>
using namespace std;
class Tile
{
public:
enum TileType {
TILE_GRASS,
TILE_OUTSIDE_WALL_TOP_LEFT_OUTER,
TILE_OUTSIDE_WALL_TOP,
TILE_OUTSIDE_WALL_TOP_RIGHT_OUTER,
TILE_OUTSIDE_WALL_LEFT,
TILE_OUTSIDE_WALL_RIGHT,
TILE_OUTSIDE_WALL_BOTTOM_RIGHT_INNER,
TILE_OUTSIDE_WALL_BOTTOM_LEFT_INNER,
TILE_OUTSIDE_WALL_BOTTOM_LEFT_OUTER,
TILE_OUTSIDE_WALL_BOTTOM,
TILE_OUTSIDE_WALL_TOP_RIGHT_INNER,
TILE_OUTSIDE_WALL_TOP_LEFT_INNER,
TILE_OUTSIDE_WALL_BOTTOM_RIGHT_OUTER,
TILE_WALL,
TILE_INSIDE_WALL_TOP_LEFT_INNER,
TILE_INSIDE_WALL_TOP,
TILE_INSIDE_WALL_TOP_RIGHT_INNER,
TILE_INSIDE_WALL_LEFT,
TILE_INSIDE_WALL_RIGHT,
TILE_INSIDE_WALL_BOTTOM_RIGHT_OUTER,
TILE_INSIDE_WALL_BOTTOM_LEFT_OUTER,
TILE_INSIDE_WALL_BOTTOM_LEFT_INNER,
TILE_INSIDE_WALL_BOTTOM,
TILE_INSIDE_WALL_TOP_RIGHT_OUTER,
TILE_INSIDE_WALL_TOP_LEFT_OUTER,
TILE_INSIDE_WALL_BOTTOM_RIGHT_INNER,
TILE_FLOOR
};
Tile(TileType t):
tileType(t)
{
cout << "Constructing tile\n";
}
virtual ~Tile()
{
cout << "Destructing tile\n";
}
TileType tileType;
static int TILE_WIDTH;
static int TILE_HEIGHT;
int x;
int y;
// pathfinding
std::shared_ptr<Tile> previousTile;
float g; // cost to tile (total cost from previous tiles + cost to this tile)
float h; // cost to next tile
float f; // g + h
bool walkable;
};
class TileMap
{
int mWidth_;
std::vector<Tile> mTiles_;
public:
TileMap(const std::vector<int>& pTiles, int pWidth) : mWidth_(pWidth)
{
mTiles_.reserve(pTiles.size());
for (int i = 0; i != pTiles.size(); ++i)
{
const int x = i % mWidth_;
const int y = i / mWidth_;
mTiles_.emplace_back(static_cast<Tile::TileType>(pTiles[i]));
Tile& tile = mTiles_.back();
tile.x = x;
tile.y = y;
//tile.position(sf::Vector2f(x * Tile::TILE_WIDTH, y * Tile::TILE_HEIGHT));
}
}
};
int _tmain(int argc, _TCHAR* argv[])
{
std::vector<int> tiles;
tiles.push_back(Tile::TileType::TILE_GRASS);
cout << "Creating tilemap\n";
TileMap t(tiles, tiles.size());
cout << "Tilemap created\n";
cout << "Exiting\n";
return 0;
}
I get the following result:
Creating tilemap
Constructing tile
Tilemap created
Exiting
Destructing tile
Upvotes: 0
Reputation: 217293
There is 2 reasons of Tile destruction in your code:
The local variable that you copy inside vector, and the internal copy when vector resizes internal memory.
To avoid the former, you have to emplace back the new element; for the later, you have to reserve place in vector. It results in something like:
TileMap::TileMap(const std::vector<int>& pTiles, int pWidth) : mWidth_(pWidth)
{
mTiles_.reserve(pTiles.size());
for(int i = 0; i != pTiles.size(); ++i)
{
const int x = i % mWidth_;
const int y = i / mWidth_;
mTiles_.emplace_back(static_cast<Tile::TileType>(pTiles[i]));
Tile& tile = mTiles_.back();
tile.x = x;
tile.y = y;
tile.position(sf::Vector2f(x * Tile::TILE_WIDTH, y * Tile::TILE_HEIGHT));
}
}
Upvotes: 1
Reputation: 9090
Tile
needs to have a copy (or move) constructor and assignment operator for use with std::vector
.
nTiles_.push_back(tile)
copy-constructs a new Tile
object from the local tile
.
In that for loop, at each iteration, the local object tile
gets constructed, then a copy gets pushed into the vector, and then the local tile
gets destructed. This is why destructors get called during the for loop.
One way to avoid this and instead only construct the Tile
object that will be in the vector
, you could write
TileMap::TileMap(std::vector<int> pTiles, int pWidth):mWidth_(pWidth)
{
for(int i = 0; i < pTiles.size(); i++)
{
int x = (i % mWidth_);
int y = floorf(i / mWidth_);
mTiles_.emplace_back( (Tile::TileType)pTiles[i] );
Tile& tile = mTiles_.back();
tile.x = x;
tile.y = y;
tile.position(sf::Vector2f(x * Tile::TILE_WIDTH, y * Tile::TILE_HEIGHT));
}
}
emplace_back
takes the arguments of the Tile
constructor, and constructs an object in-place at the end of the vector
. back
returns a reference to the last item.
If Tile
objects are heavy-weight (i.e. copying them is expensive), it may be better to use pointers instead as before, or implement move-constructor and move-assignment operator. std::vector
will copy (or move) its items if new items get inserted/erased, or when the vector gets resized.
Also the tiles()
function needs to return the vector
by reference.
Upvotes: 1