Evan Ward
Evan Ward

Reputation: 1429

c++ vector of non-pointers

I have a TileMap class that has a std::vector<Tile>. While just generating the vector, i notice that the Tiles 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

Answers (3)

Jon Cage
Jon Cage

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

Jarod42
Jarod42

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

tmlen
tmlen

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

Related Questions