• 12
• 12
• 9
• 10
• 13

Frustrating problem

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

Recommended Posts

I'm currently working with C++ and SDL and all I want is to draw the player character on the screen, so far its proved quite a problem. At first I had player drawing look like this; -- Gfx::ApplySurface(_system.GetPlayer().GetX(), _system.GetPlayer().GetY(), _system.GetPlayer().GetImage(), _system.GetScreen()); -- However it breaks when attempting to get the players X coordinate. However when I created functions within the System class to collect the player data then it worked fine to look like this; -- Gfx::ApplySurface(_system.GetPlayerX(), _system.GetPlayerY(), _system.GetImage(), _system.GetScreen()); -- So yes the second method works, but I want to know why I cant access the player data the first way?

Share on other sites
Well to truly be able to answer that i would need to see the class that holds all the player data because in reality the Player Class uses another class function with in it so you have to class for player all and all it may have something to do with in heritance.

Ps if you use the debugger find out if the value returned by GetX is good or not and that may point you into the direction of the problem.

Regards Jouei.

Share on other sites
player.h
class Player{public:	Player();	Player(int x, int y, SDL_Surface* img);	~Player();	void Init(int x, int y, std::string filename, int screenWidth, int screenHeight);	void Events(SDL_Event event);	void Move(int x, int y);	int GetX() {return _x;}	int GetY() {return _y;}	SDL_Surface* GetImage() {return _image;}private:	int _x;	int _y;	int _screenWidth;	int _screenHeight;	SDL_Surface* _image;};

system.h <relevant code>
class System{public:Player GetPlayer() {return _player;}	int GetPlayerX()   {return _player.GetX();}	int GetPlayerY()   {return _player.GetY();}private:	Player _player;

The debugger points to this line

void Gfx::ApplySurface(int x, int y, SDL_Surface *source, SDL_Surface *destination, SDL_Rect *clip)
{
SDL_Rect offset;

offset.x = x;
offset.y = y;

--> SDL_BlitSurface(source, clip, destination, &offset);
}

as far as I can tell the x and y coordinates have been passed on.

Share on other sites
I have a feeling it is to do with your SDL_Surface rather then the X and Y getters. Try changing this line:
Player GetPlayer() {return _player;}

TO:
Player & GetPlayer() {return _player;}

And see what happens. If this fixes it then I (or someone else) can explain why.

Share on other sites
Ah yes that works, I think I know why I had at one stage put the & in but I put it in the wrong spot, thanks a bunch :)

Share on other sites
Ok the reason why it crashs is because your function for GetPlayerX does no return the Class of Player it just returns the value of where the player is located thats why it crashs here a short example that should work the way your code was execpted and why it did not.

class POINT{public:POINT(){PosX = 5; PosY = 5;int GetPointX()const {return PosX;]int GetPointY()const {return PosY;]private:int PosX;int PosY;};class BOX{public:POINT GetX(){return LeftCorner;}private:POINT LeftCorner;};void SomeFunc(){BOX Test;std::cout <<Text.GetX().GetPointX();}

See you have to return a class in order to use a function with in it in the manner you were looking at.

Ok hope that clears things up.

Regards Jouei.

Share on other sites
Quote:
 Original post by Jouei Ok the reason why it crashs is because your function for GetPlayerX does no return the Class of Player it just returns the value of where the player is located thats why it crashs here a short example that should work the way your code was execpted and why it did not.

Except that your code example works because the function returns a temporary object via the copy constructor which in this case is a shallow copy and the lifetime of that is valid till the GetPointX() function call.
#include <iostream>class POINT{public:    POINT()    {        PosX = 5;        PosY = 5;    }    int GetPointX()const    {        return PosX;    }    int GetPointY()const    {        return PosY;    }private:    int PosX;    int PosY;};class BOX{public:    POINT GetX()    {        return LeftCorner;    }private:    POINT LeftCorner;};void SomeFunc(){    BOX Test;    std::cout <<Test.GetX().GetPointX();}int main(){    SomeFunc();}

Quote:
 Original post by namingwayAh yes that works, I think I know why I had at one stage put the & in but I put it in the wrong spot, thanks a bunch :)

Despite what I said, I can't figure out for the life of me why your code was failing since it looks like it would still be valid based on the code you have shown.

It looks like the pointer to the SDL_Surface gets mangled during the copy constructor and therefore fails when you try to blit. Problem is that I can't see a reason why it would be mangled.

If possible, can you upload the entire project so I could look at it? (Yes, it is bugging me that much).

[Edited by - yaustar on February 18, 2008 7:34:27 AM]

Share on other sites
A more detailed explanation:

class Player{	~Player();private:	SDL_Surface* _image;};class System{public:Player GetPlayer() {return _player;}

When you return a member of the System (here, _player) by value, you implicitly make a copy of it. There is no copy constructor defined for the Player, so what will happen is that the copy simply has a copy of each piece of data in the Player class. One of those pieces of data is an SDL_Surface* called _image.

This is, of course, only a pointer. The pointed-at surface isn't "part of" the player; the pointer is. So now, both Players point at the same SDL_Surface.

At some point, one of the Players falls out of scope, and its destructor is called. The destructor is, presumably, assuming that the SDL_Surface "belongs" to it, and cleans up with SDL_FreeSurface(). But the other Player still has a pointer pointing at that location in memory. The next time it tries to use that pointer, BOOM. Even if it simply also gets destructed - it will try to "use" the pointer again in order to re-clean-up the already trashed memory.

Storing raw pointers to resources like this is a bad idea. Something like boost::shared_ptr will make your life a lot easier. Although it's still correct here to just return the Player by reference, in general you should be thinking about what happens any time one of your classes might get copied (that includes copy-construction and assignment).

Sometimes the correct answer is "this class should never be copied". There is precedent for that: the standard library stream classes prevent you from copying them. To do that, you can declare the copy constructor and assignment operator of your class as private, and don't implement them:

class Player{private:	Player(const Player&);	Player& operator=(const Player&);};

You can also do it by inheriting from boost::noncopyable.