scope frustration(I think)

[OffbeatPatriot]

My project has gotten kind of big now but so I'm trying to post as little code as I can. It seems like the center of my problem is that I have a map for a game, at the start of this game the map is created and is constantly drawn on the screen. The map(class Map) is made up of many regions(class Region) and when the map is told to draw itself the map turns around and tells all the regions that would appear on the screen to draw themselves, and all the rest to free up their memory for storing the tiles that represent them. The problem I am having is that it seems like once the constructor for Map finishes all the Regions that were created expire. The 2d vector I made to contain them can be traversed and the draw function for each Region can be called but most of the time I get a Segmentation Fault on the first line in the Region draw function.

cout << x << ", " << y << '\n'; //x and y are the coordinates of the region on the map.

Sometimes I don't get a Segmentation Fault and I get output like this, note, I'm trying to make a 100 x 100 game grid.

6391408, 0

As I said I think this is a problem of the Regions simply ceasing to exist after the constructor completes but I don't know how to fix this problem. I've tried several versions of the code below but the problem never stopped.

This is the header Map.h that has the prototypes for the Map and Region classes

#ifndef MAP_H  
#define MAP_H

#include "SDL/SDL_image.h"

#include "SDL/SDL_ttf.h"
#include "SDL/SDL.h"
#include "Standard.h"
#include "Graphics.h"
#include "SDL/SDL_rotozoom.h"

#include <vector>
#include <iostream>
#include <sstream>

#define TILE_HEIGHT 100
#define TILE_WIDTH 200

class Region;

class Map
{
	private:
		vector< vector<Region> > regions;
		Graphics* graphics;
		SDL_Surface* screen;
		double scale;
		int screenX, screenY, space, w, h;
		bool created;
	public:
		Map();
		Map(int w, int h, SDL_Surface* screen, int space, Graphics* graphics);
		void draw();
		bool isCreated();
};

class Region
{
  private:
    SDL_Surface* screen;
    SDL_Surface* tile;
    SDL_Surface* tempTile;
    Graphics* graphics;
    int x, y;
    bool created;
  public:
    void draw(int x, int y, double scale, bool onMonitor);
    bool isCreated();
    Region();
    Region(SDL_Surface* screen, Graphics* graphics, Map* world, int x, int y);
};

#endif

This is the constructor for the Map class

Map::Map(int w, int h, SDL_Surface* screen, int space, Graphics* graphics)
{
	this->screen = screen;
	this->graphics = graphics;
	this->space = space;
	screenX = 0;
	screenY = 0;
	this->w = w;
	this->h = h;
	this->scale = 0.5;
	int x, y;

	for(x = 0; x < w;x++)
	{
		regions.push_back(vector<Region>());
		for(y = 0; y < h;y++)
		{
			regions[x].push_back(Region(screen, graphics, this, x, y));
		}
	}
	created = true;
}

Constructor for the Region class

Region::Region(SDL_Surface* screen, Graphics* graphics, Map* world, int x, int y)
{
	this->x = x;
	this->y = y;
	this->screen = screen;
	this->graphics = graphics;
	tile = load_image("tile.png");
	tempTile = NULL;
	if(tile == NULL)
	{
		printf("Region surface failed!\n");
		created = false;
	}
	cout << x << ", " << y << '\n';
	created = true;
}

The draw function for the Region class which the Map class calls for each region in it's draw function.

void Region::draw(int screenX, int screenY, double scale, bool onMonitor)
{
	cout << x << ", " << y << '\n';
	
	if(onMonitor)
	{
		tempTile = load_image("tile.png");
		tile = rotozoomSurface(tempTile, 0, scale, 0);
		apply_surface((scale * TILE_WIDTH) * x - screenX, (scale * TILE_HEIGHT) * y - screenY, tile, screen, NULL);
	}
	else
	{
		SDL_FreeSurface(tile);
	}
}

bool Region::isCreated()
{
	return created;
}

[ArkM]

std::vector is an intrusive container: it contains own copies of initialization (push_backed, for example) objects (Region class objects in that case). If you don't write a proper copy constructor for Region, default copy constructor was generated and used (copy member by member).

What happens with tile pointed object when Region destructor was called after push_back(Region(...)) was called? In that moment you have a copy of Region object in the vector (with copied tile pointer initialized by load_image call). You have two tile pointers to the same image object then Region destructor discard (?) this image object? If so you have serious troubles...

It's the only suspicious issue for me now. I don't like to place complex objects (with non-trivial destructors and copy constructor) in STL containers (prefer to contain pointers to dynamically generated objects and clear them explicitly in outer class destructor - Map in this case). Apart from anything else it is much more effective (in time and memory) approach. Of course, tastes differ...

May be, this hypothesis helps...

[OffbeatPatriot]

sorry for neglecting my own topic and thanks for the help but I'm having trouble understanding your English.

I tried changing the 2d vector to a vector of pointers to regions and I still have the same problem.

"What happens with tile pointed object when Region destructor was called after push_back(Region(...)) was called? In that moment you have a copy of Region object in the vector (with copied tile pointer initialized by load_image call). You have two tile pointers to the same image object then Region destructor discard (?) this image object? If so you have serious troubles..."

I don't quite understand this, why would the Region destructor be called after push_back?

[ArkM]

Sorry for my English. It's terrible. I hope, we are on C++ language forum, not faculty of English language and literature.

Let's simulate and trace your Map building mechanics with explicit copy constructor:

class Region;

class Map
{
  vector<vector<Region> > regions;
public:
  Map(int w, int h);
  void draw() const;
};

class Region
{
public:
  Region():x(-1),y(-1) { cout << "Region()" << endl; }
  Region(int x, int y);
  Region(const Region& r) // copy constructor
  {
     cout << "Region copy ctor" << endl;
     x = r.x;
     y = r.y;
  }
  ~Region() { cout << "~Region()" << endl; }
  void draw() const;
private:
  int x, y;
};

void Region::draw() const
{
  cout << "Region object " << x << "," << y << endl;
}
// Dangerous (error prone) naming style:
Region::Region(int x, int y) 
{
  this->x = x;
  this->y = y;
  cout << "Region(" << x << "," << y << ")" << endl;
}

Map::Map(int w, int h)
{
  for (int i = 0; i < w; ++i)
  {
    regions.push_back(vector<Region>());
    for (int j = 0; j < h; ++j)
    {
       cout << "push_back...";
       regions[i].push_back(Region(i,j));
    }
  }
}

void Map::draw() const
{
  for (size_t i = 0; i < regions.size(); ++i)
  {
    for (size_t j = 0; j < regions[i].size(); ++j)
         regions[i][j].draw();
  }
}

int main()
{
   Map m(2,2);
   cout << "Map(int,int) built object" << endl;
   m.draw();
   return 0;
}

You may run this code sceleton. See on the Map constructor backyard:

push_back...Region(0,0)
Region copy ctor
~Region()
push_back...Region(0,1)
Region copy ctor
Region copy ctor
~Region()
~Region()
push_back...Region(1,0)
Region copy ctor
~Region()
push_back...Region(1,1)
Region copy ctor
Region copy ctor
~Region()
~Region()
Map(int,int) built object
Region object 0,0
Region object 0,1
Region object 1,0
Region object 1,1
~Region()
~Region()
~Region()
~Region()

In regions[i].push_back(Region(i,j)); we create anonymous Region object (as argument) then vector::push_back create its own Region by copy constructor then (at final) the compiler generate anonymous object destructor call.

We see 10 constructor calls and 6 destructor calls. Now the vector contains 4 objects. Additional copy constructors were started by vector internal mechanics.

Of course, it's not one and only one reason for bad Region data and memory access faults in your original post. Anyway, per se vector<vector<Region> > is not a source of troubles but it looks simpler than it is (especially with complex base class).