# Returning an element from an array

This topic is 2013 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

Hi there. I hope someone could help me solve this problem.

I suspect this is not the way to do it, but here's the code:

 int tile[210][210]; int Map::getTile(int a, int b) { return tile[a]; } 

When I want to return the element in a different class, it returns 0.

Also, I can't make my tilemap (tile[a]) bigger than 212 * 212. The game crashes just after it launches.

##### Share on other sites
If you want the tile to be connected to an instance of the Map class, you need to place the definition of tile inside the class { } body where you define Map. Just sticking it out in no-man's-land makes it a global variable, so every copy of Map will access the same variable.

##### Share on other sites
In addition to what ApochPiQ mentioned, you should use an std::vector if you're going to have a large amount of data, because you're putting too much data on the stack which is why it crashes. This is called a Stack Overflow, apparently (I never knew what it was called until just now ).

Using a std::vector puts the data on the heap instead, which won't crash your program with the amount of data you are using.
(Note: Just because std::vectors are resizable, doesn't mean you have to use that. They are good as for constant sized arrays also. Infact, the new C++ standard even introduced a new type of container: A non-resizable vector. )

This code should give you a good boost-start:
#include <vector> class Map { public: Map(int width, int height) : width(width), height(height) { //Resize the vector to be the size we want (width * height). this->tiles.resize((width * height), 0); //Initialize each tile to 0. } int Map::GetTile(int x, int y) { int index = _GetIndex(x,y); //Make sure this is a valid index. if(_IsValid(index)) { return this->tiles[index]; } return 0; } void Map::SetTile(int x, int y, int tileID) { int index = _GetIndex(x,y); //Make sure this is a valid index. if(_IsValid(index)) { this->tiles[index] = tileID; } } private: int width, height; std::vector<int> tiles; //Converts from a two-dimensional x,y to a one-dimensional index into our vector. int _GetIndex(int x, int y) { return (y * this->width) + x; } //Makes sure that we aren't going out of bounds, which will crash the program. bool _IsValid(int index) { if(index < this->tiles.size()) return true; return false; } }; int main() { const int NOTHING = 0; const int GRASS = 1; const int DIRT = 2; const int DIFFERENT_DIRT = 3; //... const int WATER = 211; const int LAVA = 300; Map map(20, 20); //I want a map that's 20 by 20 tiles. Map largeMap(2000, 5000); //I also want a map that's 2000 by 5000 tiles. largeMap.SetTile(50, 100, DIRT) //Access tile X:50, Y:100, and set it to DIRT. if(largeMap.GetTile(playerTileX, playerTileY) == LAVA) { //...damage the player. } } 

If you have any questions about how it works, don't hesitate to ask. Edited by Servant of the Lord

##### Share on other sites
Wow, thanks, I can make the map any size I want.

But, I create the map and do all of this inside my Map class. And when again I want to get the tile in the Player class, it returns 0.

##### Share on other sites
Could you please post the code how the Map instance is passed to the Player? I have the suspicion you are creating a new (uninitialized) Map instance in your Player class.

##### Share on other sites
 class Player { Map m; public: Player(); void Init(); //... } 

I set the width and the height inside the Map class.

##### Share on other sites
Are you sure you want every player to have her own map? Because that's what you are doing right now. What you want to do, is pass a reference to your map to the player, like this:

 class Player { Map& m; public: Player(Map& map) : m(map) { ... old constructor code ... } }; in main: set up the map as you did before, then when creating the player: Player player(map); // or largeMap 

This way you only have one map for all players, all referencing the same data. If you do it the way you did, you create an empty map for every player, and one in your main, which you never use.

##### Share on other sites

But, I create the map and do all of this inside my Map class. And when again I want to get the tile in the Player class, it returns 0.

Could you show a complete example of code? We need to see what your entire Map class looks like, and your entire Player class, if we're to figure out why your Player class is not interacting with the Map class properly. Also show where your Map or Player is created. Edited by Servant of the Lord

##### Share on other sites
I think SOAL and rnlf are correct, you need every player to have a reference not create its own (though personally I would use a pointer but its all the same)

Make the player class have a reference to a M

Are you sure you want every player to have her own map? Because that's what you are doing right now. What you want to do, is pass a reference to your map to the player, like this:

 class Player { Map& m; public: Player(Map& map) : m(map) { ... old constructor code ... } }; in main: set up the map as you did before, then when creating the player: Player player(map); // or largeMap 

This way you only have one map for all players, all referencing the same data. If you do it the way you did, you create an empty map for every player, and one in your main, which you never use.

this is pefect, should easily solve the issue

also SOAL you forgot to include a check inside the isValid function if it is less than 0. I don't know if std::vector does that check or not.

##### Share on other sites

also SOAL you forgot to include a check inside the isValid function if it is less than 0. I don't know if std::vector does that check or not.

Oops, you're right.

The function should be:
bool _IsValid(int index) { if(index < this->tiles.size() && index > 0) return true; return false; }

If it was my own code, I'd just make 'index' be unsigned or a size_t.

##### Share on other sites
Oof; not quite, Servant. 0 is also a valid element.

I try to write it as close to the mathematical notation, [0 size) or 0 <= x < size, as possible:
 bool _IsValid( int index ) const { return 0 <= index && index < tiles.size(); } 
I find it's easier to "read" the domain of the condition at a glance. Edited by fastcall22

##### Share on other sites

Oof; not quite, Servant. 0 is also a valid element.

/facepalm

I must be having one of those days. It's a good thing my build environment broke, or I'd probably be wreaking havoc on my code base right now.
Or maybe that's how my build environment broke. Edited by Servant of the Lord

##### Share on other sites

Are you sure you want every player to have her own map? Because that's what you are doing right now. What you want to do, is pass a reference to your map to the player, like this:

 class Player { Map& m; public: Player(Map& map) : m(map) { ... old constructor code ... } }; in main: set up the map as you did before, then when creating the player: Player player(map); // or largeMap 

This way you only have one map for all players, all referencing the same data. If you do it the way you did, you create an empty map for every player, and one in your main, which you never use.

I've been struggling with this for a few days now. I do all it says here, but it always returns some kind of an error. The error it returns when I write as it says is: "error: 'map' is not a type" pointing to this line "Player p(map);".

##### Share on other sites
You have to actually create the map somewhere before you can pass it into Player p(map);

Either globally, or preferably, whatever function or class creates and owns "Player p" should also own "Map map".

Example:
Map myMap; Player myPlayer(myMap);

'myMap' is an instance of the 'Map' class type.
'myPlayer' is an instance of the 'Player' class type, and we're passing the 'myMap' instance into the Player constructor to create 'myPlayer'.
Because the Player class takes a reference (the & character) to a Map, it doesn't actually copy the map, but it shares the same map - Changing one changes the other, because Player's 'm' Map is a reference to 'myMap' Map (a different variable but pointing to the same location in memory).

You just have to be sure that "Map myMap" lasts as long or longer than "Player myPlayer"'s 'm' Map reference. Edited by Servant of the Lord

##### Share on other sites
Can you post the code you are trying to compile?

 Ninja'd with actual advice :-) Edited by ApochPiQ

##### Share on other sites
I actually do have the "Map m;" line above the "Player p;" one, just didn't write it in the post. But yeah, it doesn't work.
Here's the code I'm trying to compile:

 class Game { Map map; Player p(map); public: Game(); //... }; 

... and here is the Player class header file code:

 class Player { Map& m; public: Player(Map& map) : m(map) {} //... }; 

##### Share on other sites

 class Game { Map map; Player p(map); public: Game(); //... }; 

This is the source of your problem. The compiler thinks you are trying to create a function called p which takes a parameter of type map with no name, and returns a Player.

Basically, this is because you're trying to create a variable instance inside a class declaration. This is not legal. You need to create variables inside a code block such as a function body:

class Map { /* ... */ }; class Player { public: Player(Map& map) : MyMap(map) { } private: Map& MyMap; }; int main () { Map someMap; Player somePlayer(someMap); // Do stuff! return 0; }

##### Share on other sites
Ah, thank you so much. That part works now But how does the Player constructor look like in the Player code (.cpp) file?

##### Share on other sites
Moving the constructor to a separate .cpp file is pretty simple:

// Map.h class Map { /* ... */ }; // Player.h class Player { public: Player(Map& map); private: Map& MyMap; }; // Player.cpp Player::Player (Map& map) : MyMap(map) { } // Main.cpp int main () { Map someMap; Player somePlayer(someMap); // Do stuff! return 0; }

##### Share on other sites
Hmm.. It compiles now, but the initialization, loop and rendering of the Player all got messed up. None works properly.. Anyone knows why that might have happened? Edited by Toshio

##### Share on other sites
Show us the code that is having problems.

If it's too large to post, consider taking out pieces one by one until you identify the smallest program subset that actually demonstrates the problem you're having, and post that instead.

##### Share on other sites
 void Game::OnInit() { Map map; Player p(map); p.Init(); } void Game::OnLoop() { Map map; Player p(map); p.Move(); } 

Nothing compiles as it should.
Here are those two functions:

 void Player::Init() { CSurface c; x = 100; y = 100; texture = c.loadTexture("player.png"); } void Player::Move() { x += xVel * (delta / 1000.0f); y += yVel * (delta / 1000.0f); //... } 

##### Share on other sites
If you create a Map or Player within a function, it creates a new Map or Player, not the one you used previously.

You have to create them once, by placing them in the Game declaration.
class Game { public: Game(); //...other stuff. private: Map map; Player player; };

And in the constructor, you need to initialize Player with the Map, in the Game's constructor.
Game::Game() : player(map) //Pass 'map' into the Player constructor. { }

...And then you can use 'map' and 'player' normally, without re-creating them.
void Game::OnInit() { p.Init(); } void Game::OnLoop() { p.Move(); } Edited by Servant of the Lord

##### Share on other sites

#include <vector> class Map { public: [...] private: int width, height; std::vector<int> tiles; //Converts from a two-dimensional x,y to a one-dimensional index into our vector. int _GetIndex(int x, int y) [...] //Makes sure that we aren't going out of bounds, which will crash the program. bool _IsValid(int index) [...] }; 

Just a note here so people don't get into any bad habits: identifiers starting with an underscore followed by an upper case letter should never be used, as they're reserved for the implementation.

##### Share on other sites
Wow, thank you Servant of the Lord. Everything works now
Thanks ApochPiQ and everyone else who helped