Jump to content

  • Log In with Google      Sign In   
  • Create Account


problems with initializing my map


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
6 replies to this topic

#1 Ars7c3   Members   -  Reputation: 177

Like
0Likes
Like

Posted 15 June 2013 - 11:04 AM

Hey guys,
so I am working on a simple level editor for a platform game I'm making, and everything's been going smoothly up until now.
I decided I wanted to have a 3d vector to allocate tiles for my editor instead of a 3d pointer because they are very difficult to understand( for me anyway). The level allocates correctly ONLY if the width, height, and layers of the level are all the same value, and that is what is stumping me.
I would greatly appreciate it if someone could help me out.
 
//My vector container
vector<vector<vector<Tile*> > >map;

//How I'm allocating the map... in the Level constructor
Level::Level(int tileWidth, string tilePath)
{
	AskProperties();
	AskFileName();
	sheet.Init(tileWidth, tilePath);
	
	for(int l = 0; l < layers; l++)
	{
		map.resize(layers);
		cout << "Layer " << l << endl;
	
	for(int i = 0; i < height; i++)
	{
		map[i].resize(height);
		cout << endl<< "Row " << i << endl;
		for(int t = 0; t < width; t++)
		{
			map[i][t].resize(width);
	     	map[i][t].push_back(new Tile(t * sheet.GetTileSize(), i * sheet.GetTileSize(), false, sheet));
		    cout << "X " << t << endl;
		}
	}
	}
	
	
	

}

//How i am saving the level -- using integers to represent tile types
void Level::Save()
{
	
	cout << "NOPE" << endl;
	ofstream file(fileName.c_str());
	for(int x = 0; x < layers; x++)
	{
	file << endl << endl << "Layer: " << x + 1 << endl << endl;	
		for(int y = 0; y < height; y++)
		{
			file << endl;
			for(int z = 0; z < width; z++)
			{
				file << map[x][y][z]->GetType();
			}
		}
	}
	
}


 



Sponsor:

#2 Brother Bob   Moderators   -  Reputation: 7863

Like
5Likes
Like

Posted 15 June 2013 - 11:40 AM

I see many fundamental problems:

  1. Think about your loops and where you resize the map for a second. Ignoring the fact that map is a tripple-nested vector, map itself is a vector of layers. Wouldn't it be a better idea to resize the map to the appropriate number of layers before looping over all the layers? Same thing goes for all the other dimensions; don't resize over and over again them in their own loop.

  2. The last dimension is not correctly handled (ignoring point 1, of course). First you resize map[i][t] to contain width elements, but then you proceed to push_back another width elements to it. The result is a vector of length 2*width, where the first width elements (the result from the resize) are null pointers.

  3. You're mixing the indices for the different dimensions. In the height-loop, you access map[i] with the height-loop index, even though map[i] refers to the i:th layer since it has been resized to the number of layers, whereas i is looping all the way up to height. Same goes for the width-loop and the t index.

  4. Don't use containers of containers of containers to represent a trivial 3D array structure. For example, boost provides a dedicated multi-dimensional array data structure, but just using a single contigious 1D array with manual indexing is simple enough and doesn't have the overhead of multiple nested containers.

  5. If you don't have a very good reason to store pointers to tiles, store them by value. You'll get rid of a level of memory indirection: you have effectively four nested pointers to go through just to get a single tile already, where one should be enough.

But the solution to the core of the problem, as addressed in point 1 and 2, is this:

map.resize(layers);
for(int l = 0; l < layers; l++)
{
    map[l].resize(height);
    for(int i = 0; i < height; i++)
    {
        map[l][i].reserve(width);
        for(int t = 0; t < width; t++)
        {
            map[i][t].push_back(...);
        }
    }
}

Pay attention to the reserve, and not a resize, of the last dimension. You don't want to resize and add elements to it, you either want to resize and write with operator[], or reserve and add with push_back.



#3 Ars7c3   Members   -  Reputation: 177

Like
0Likes
Like

Posted 15 June 2013 - 12:04 PM

I tried that, and it had the same result as before. It only works when the width, height, and layers are all the same. When, for example, i set the width and height to 10, and the layers to 3, it returns an error that says: vector subscript is out of range.



#4 Brother Bob   Moderators   -  Reputation: 7863

Like
1Likes
Like

Posted 15 June 2013 - 12:07 PM

Last line should be

map[l][i].push_back(...)

Copy-paste error on my part.



#5 Ars7c3   Members   -  Reputation: 177

Like
1Likes
Like

Posted 15 June 2013 - 12:17 PM

Thank You Brother Bob. I still have much to learn. rolleyes.gif But then again, we're never done learning are we?



#6 Brother Bob   Moderators   -  Reputation: 7863

Like
2Likes
Like

Posted 15 June 2013 - 12:23 PM

Nor should we be done learning.



#7 Paradigm Shifter   Crossbones+   -  Reputation: 5203

Like
1Likes
Like

Posted 15 June 2013 - 12:32 PM

If you resize in the 'i' loop, instead of reserve, you don't need to push back, you can just use the indices directly map[l][i][t];

 

Probably makes it clearer what is going on.

 

Virtual spanks all round for using lower case L as a variable name, looks too much like the number 1. (EDIT: I'd use layer, row, col instead for the index variables I think).

 

I'm not jumping for joy about naming the array vector3 'map' either, since that is also the name of a class in namespace std.


Edited by Paradigm Shifter, 15 June 2013 - 12:37 PM.

"Most people think, great God will come from the sky, take away everything, and make everybody feel high" - Bob Marley




Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS