Sign in to follow this  
recon6

Problem with inheritance in SDL class

Recommended Posts

recon6    112
Hi. I've created and SDL application that creates an object which moves around in a circle. I'm making use of inheritance and am now trying to get the object to fire a bullet. I've used the Entity class shown below as the base class for the player and the bullet.
//Entity header file

class Entity
{
    public:

        int create(SDL_Surface* dest);
        int setSize(int);
        int setRadius(int);
        Uint8 setColour(Uint8, Uint8, Uint8, Uint8);
        int setPosition(int,int);
        int getPosition();


    protected:

        float angle;
        int xPos, yPos,rad;


    private:

        int size;
        Uint8 r,g,b,a;


};

//Entity implementation file entity.cpp
int Entity::create(SDL_Surface* dest)
{
    filledCircleRGBA( dest,  xPos,  yPos,  size, r, g, b, a);
}

int Entity::setPosition(int centreX,int centreY)
{

     xPos = rad*cos(angle) + centreX;
     yPos = rad*sin(angle) + centreY;
}


int Entity::getPosition()
{
    return xPos;
    return yPos;
}

int Entity::setRadius(int _rad)
{
    rad = _rad;
}



int Entity::setSize(int _size)
{
    size = _size;
}

Uint8 Entity::setColour(Uint8 _r, Uint8 _g, Uint8 _b, Uint8 _a)
{
    r=_r;
    g=_g;
    b=_b;
    a=_a;
}
Below is the Bullet header & implementation files

//Header file
class Bullet : public Entity
{
    public:
        void move();

};

//Implementation file
void Bullet::move()
{

    rad-=2;
}
This is how i've tried to implement it in main()
if(bulletCreated == true)
      {
        bullet.setSize(2);
        bullet.setColour(10,150,50,255);
        bullet.setPosition(centreX,centreY)==player.setPosition(centreX,centreY);
        bullet.getPosition()==player.getPosition();
        bullet.create(screen);
        bullet.move();

      }
centreX, centreY and rad are global variables. rad has been set using the setRadius function. I've used spacebar to indicate bulletCreated=true. The player moves whenever i press left or right. I've tried a few variations but can't seem to get the bullet to fire from the position that the player is in. Every time i hit spacebar the bullet just fires from the initial starting position of the player. How can i get it to fire from wherever the player's position is? Thanks [Edited by - recon6 on October 13, 2007 4:11:00 PM]

Share this post


Link to post
Share on other sites
rip-off    10976
You have several errors in your source, it should compile, let alone run.

Here is the source I used to compile (the same, just with dummy variable declarations):

int centreX, centreY, rad;

class Entity
{
public:

int create(SDL_Surface* dest);
int setSize(int);
int setRadius(int);
Uint8 setColour(Uint8, Uint8, Uint8, Uint8);
int setPosition(int,int);
int getPosition();


protected:

float angle;
int xPos, yPos,rad;


private:

int size;
Uint8 r,g,b,a;


};

//Entity implementation file entity.cpp
int Entity::create(SDL_Surface* dest)
{
//filledCircleRGBA( dest, xPos, yPos, size, r, g, b, a);
}

int Entity::setPosition(int centreX,int centreY)
{

xPos = rad*cos(angle) + centreX;
yPos = rad*sin(angle) + centreY;
}


int Entity::getPosition()
{
return xPos;
return yPos;
}

int Entity::setRadius(int _rad)
{
rad = _rad;
}



int Entity::setSize(int _size)
{
size = _size;
}

Uint8 Entity::setColour(Uint8 _r, Uint8 _g, Uint8 _b, Uint8 _a)
{
r=_r;
g=_g;
b=_b;
a=_a;
}


//Header file
class Bullet : public Entity
{
public:
void move();

};

//Implementation file
void Bullet::move()
{

rad-=2;
}

void foo()
{
// dummy variables so code compiles
bool bulletCreated = true;
Bullet bullet;
SDL_Surface *screen = 0;
Entity player;
if(bulletCreated == true)
{
bullet.setSize(2);
bullet.setColour(10,150,50,255);
bullet.setPosition(centreX,centreY)=player.setPosition(centreX,centreY);
bullet.getPosition()=player.getPosition();
bullet.create(screen);
bullet.move();
}
}



Here are the errors GCC gave me at a reasonably high warning level:

main_sdl.cpp: In member function `int Entity::create(SDL_Surface*)':
main_sdl.cpp:47: warning: control reaches end of non-void function

main_sdl.cpp: In member function `int Entity::setPosition(int, int)':
main_sdl.cpp:52: warning: converting to `int' from `double'
main_sdl.cpp:53: warning: converting to `int' from `double'
main_sdl.cpp:54: warning: control reaches end of non-void function

main_sdl.cpp: In member function `int Entity::setRadius(int)':

main_sdl.cpp:66: warning: control reaches end of non-void function
main_sdl.cpp: In member function `int Entity::setSize(int)':
main_sdl.cpp:73: warning: control reaches end of non-void function

main_sdl.cpp: In member function `Uint8 Entity::setColour(Uint8, Uint8, Uint8, Uint8)':
main_sdl.cpp:81: warning: control reaches end of non-void function

main_sdl.cpp: In function `void foo()':
main_sdl.cpp:108: error: non-lvalue in assignment

main_sdl.cpp:109: error: non-lvalue in assignment

Share this post


Link to post
Share on other sites
recon6    112
My code does run,it doesn't give any errors at all. The problem is i don't know how to get the bullet to fire from the position that the player is in.

Here is the code to move the player:

[code]
//H file
class Player : public Entity
{
public:
void moveLeft();
void moveRight();

};

//Player.cpp file
void Player::moveLeft()
{
angle +=0.01;
}

void Player::moveRight()
{
angle -=0.01;
}

//Implementation in main()
Player player;
player.setSize(10);
player.setColour(100,50,100,255);
player.setPosition(centreX,centreY);
player.getPosition();
player.create(screen);

Share this post


Link to post
Share on other sites
darookie    1441
Quote:
Original post by recon6
My code does run,it doesn't give any errors at all.

Set your compiler to treat warnings as errors and try to compile. You'll be surprised...

Seriously, a lot of your code is not only wrong (though the compiler will only
warn), but also doesn't make sense.

Just a few examples:

// why does the method return an int?
int Entity::setPosition(int centreX,int centreY)
{

xPos = rad*cos(angle) + centreX;
yPos = rad*sin(angle) + centreY;
// you fail to return an "int" here
}



// you return a *single* int value here
int Entity::getPosition()
{
return xPos;
// unreachable statement - the return above will cause the method to leave
return yPos;
}

// in C++ a simple solution for the problem above is to separate the method
// into two methods:

// return xPos. he const only hints the compiler that the method does
// not modify any class members
int Entity::getPositionX() const
{
return xPos;
}

// same as above for yPos
int Entity::getPositionY()
{
return yPos;
}



// now what is that supposed to mean?
bullet.setPosition(centreX,centreY)==player.setPosition(centreX,centreY);
bullet.getPosition()==player.getPosition();

// correction:
bullet.setPosition(player.getPositionX(), player.getPositionY());



I hope this is enough to get you started with fixing your code. Please have another look at your favourite C++ book, too.

Share this post


Link to post
Share on other sites
recon6    112
Thanks.
I've corrected the errors.

But i still can't wrap my head around as to how i can get the bullet firing from the players position.

I've made this correction:

bullet.setPosition(player.getPositionX(), player.getPositionY());

Now, when you press space, the bullet starts at the extreme right of the screen and moves from right to left. The bullet also moves about when i press the left and right keys. How can i fix the angle at which it's fired from and just decrease the radius?

I don't know how else to go about getting the bullet to fire.

Share this post


Link to post
Share on other sites
Zahlman    1682
A class is not simply some storage space for code and related data. It represents a data type. Except, of course, for containers - but the standard library already provides all the kinds of containers you're likely to need.

Consequently, it doesn't make sense to think in terms of instantiating one Bullet somewhere and a separate "bullet created" flag, because as soon as you instantiate the Bullet, it IS created.

What you want to do is store Bullets in a container, and when you update, draw "each bullet in the container". A std::vector is appropriate for this.

Consequently, you want the bullet to be "valid" for its entire lifetime. In C++, we use constructors to construct objects - i.e., set the initial values of all data. We don't set things one piece at a time. So in general, writing a bunch of accessor/mutator pairs like that is a big red warning flag, because it provides an interface for treating the class like a storage space for data, instead of the new data type that it's supposed to be.

What we are trying to do is to "encapsulate" the data, which is why we make things 'private'. When you write an accessor and also a mutator for the same data, and neither do any checks, your code is lying to you about this data being encapsulated. It is not.

So, what do we do?

First off, we can organize related components of the data into other classes or structs. In many of these cases, there's no need for encapsulation, but the added struct-wrapping is a powerful way to indicate "these values are related", and it also lets you simplify your function interfaces (for example, instead of taking a bunch of arguments to represent a colour, you just take a Colour object).

Next, we give the class a constructor which replaces the work of the mutators. After all, a Bullet's colour isn't going to change while in mid-air, right? So, we can set it at the beginning, and forget about it. We also don't need to "set" its position; that's made redundant by the ability to "move".

Then, we make the behaviour of functions match their names. Another word for behaviour is functionality, oddly enough :) For example, that "creation" function doesn't really create a bullet (that's what a constructor does); it draws the bullet. So, we should name it that way - i.e. draw().

Similarly, When a bullet "move"s, what changes? Its radius? No, its position. So, we update the bullet's postion in there, according to its speed. Hold on - it doesn't have a speed? Oh, we need to think more about how we model the object, then. The Entity setPosition allows for - oh, I get it now; 'rad' is not an object radius but a turning radius. That's another point: use meaningful names; it doesn't cost you anything to lengthen them except your own typing effort, because they "don't exist" in the compiled version.

Anyway, bullets don't turn about some circle while they move, do they? So, that stuff doesn't belong in the Entity class, because it isn't stuff that makes sense for every Entity.

That follows from the Liskov substitution principle (google it). What we want is to make sure that a derived object (Bullet) could be used anywhere a base object (Entity) could. Consequently, the Entity should not "be able to do" things that a Bullet can't.

So, we take the angle and turning radius out of the Entity, and move it to the Player (which is the thing that can move in a circle) and give the Bullet additionally a 'velocity'.

Finally, we can clean up that accessor, by thinking about responsibility. You must be wondering how we're going to construct a Bullet, since we have to set its starting location in the constructor... we want to use the player's location, and now I'm telling you you can't "ask" the player for its location? Yep, that's right. Because we aren't going to construct the Bullet - the player is. After all, if you were describing what happens in natural language, you wouldn't say that a bullet comes into existence at the player's location - you'd say the player fires a bullet. So that's exactly what we'll do.

I'll write only the declarations; if you've been following the discussion, you should be able to implement the classes and make use of them.


// First, some structures for wrapping up related data together.
// For now, we will let these be containers, since they don't have obvious
// functionality of their own.
struct Position { int x, y; };
struct Velocity { int dx, dy; };
// Yes, it's basically the same structure - right now - but there are benefits
// to this kind of duplication: we get more type information. We know that
// adding Position + Velocity or Velocity + Position or Velocity + Velocity
// makes sense, but Position + Position doesn't. That helps us avoid logical
// errors when we write the code, by looking at the data types. Later, we'll
// use better techniques, but I don't want to show too much at once.
struct Colour { Uint8 r, g, b, a; };

// Now, our classes look like:
class Entity {
public:
void draw(SDL_Surface* dest);
// I *will* implement the constructor, because the above discussion doesn't
// explain enough about what to do...
Entity(const Position& p, int size, const Colour& colour) :
position(p), size(size), colour(colour) {}

protected:
Position position;

private:
int size;
Colour colour;
};

class Player : public Entity {
public:
// We'll need to "forward" the constructor - i.e. explain how to construct the
// "entity part of" a Player for a given Player object.
Player(const Position& p, int size, const Colour& colour, int radius, int angle):
Entity(p, size, colour), turning_radius(radius), angle(angle) {}

// The Player's position will change iteratively (i.e. once per game loop)
// just as any Bullet's position does.
void move();

// Think *carefully* about how you are going to implement move() for this
// class. You can't *just* add the vector specified by the turning radius
// and angle, because that just gives a straight line motion in that
// direction. You'll want to *change* the angle over time, and also... well,
// I'll give you some room for trial and error :)

// Also, the player needs to be able to fire a bullet:
Bullet fire();
// Think about how you are going to decide what you pass to the Bullet
// constructor. You have access to the needed position (i.e. the current
// position of 'this' object). Size and colour will presumably always be some
// constant "bullet" values. What about the velocity?

private:
int turning_radius;
float angle;
};

class Bullet : public Entity {
public:
// This should look familiar...
Player(const Position& p, int size, const Colour& colour, const Velocity& v):
Entity(p, size, colour), velocity(v) {}

void move();

// This version of move() should be a lot easier to implement: just "add" the
// velocity to the position.

private:
Velocity velocity;
};

// Above, I said:
// Think about how you are going to decide what you pass to the Bullet
// constructor.... Size and colour will presumably always be some
// constant "bullet" values.

// Think MORE about that. Do we *need* to specify a size and colour when we make
// the Bullet? Why not just let the Bullet specify them? We can pass a constant
// value in the initializer list; it doesn't have to come from a constructor
// parameter. Similarly for the player.


// Our main code will do someting like this:
#include <vector>
#include <algorithm>

int main() {
std::vector<Bullet> bullets;
Player p(/* args */);

// If the player wants to fire a bullet, we add it to the bullets like this:
bullets.push_back(p.fire());

// We update the player's position like this:
p.move();

// We update all the bullet positions like this:
std::for_each(bullets.begin(), bullets.end(), std::mem_fun_ref(&Bullet::move));

// We draw the player like this:
p.draw();

// We draw all the bullets like this:
std::for_each(bullets.begin(), bullets.end(), std::mem_fun_ref(&Bullet::draw));

// We DON'T mess around with the bullet's internal data each time around in
// order to "move" it. We certainly don't re-set its colour to the same thing
// every time ;)
}

Share this post


Link to post
Share on other sites
Chad Smith    1343
Quote:
Original post by Zahlman
A class is not simply some storage space for code and related data. It represents a data type. Except, of course, for containers - but the standard library already provides all the kinds of containers you're likely to need.

Consequently, it doesn't make sense to think in terms of instantiating one Bullet somewhere and a separate "bullet created" flag, because as soon as you instantiate the Bullet, it IS created.

What you want to do is store Bullets in a container, and when you update, draw "each bullet in the container". A std::vector is appropriate for this.

Consequently, you want the bullet to be "valid" for its entire lifetime. In C++, we use constructors to construct objects - i.e., set the initial values of all data. We don't set things one piece at a time. So in general, writing a bunch of accessor/mutator pairs like that is a big red warning flag, because it provides an interface for treating the class like a storage space for data, instead of the new data type that it's supposed to be.

What we are trying to do is to "encapsulate" the data, which is why we make things 'private'. When you write an accessor and also a mutator for the same data, and neither do any checks, your code is lying to you about this data being encapsulated. It is not.

So, what do we do?

First off, we can organize related components of the data into other classes or structs. In many of these cases, there's no need for encapsulation, but the added struct-wrapping is a powerful way to indicate "these values are related", and it also lets you simplify your function interfaces (for example, instead of taking a bunch of arguments to represent a colour, you just take a Colour object).

Next, we give the class a constructor which replaces the work of the mutators. After all, a Bullet's colour isn't going to change while in mid-air, right? So, we can set it at the beginning, and forget about it. We also don't need to "set" its position; that's made redundant by the ability to "move".

Then, we make the behaviour of functions match their names. Another word for behaviour is functionality, oddly enough :) For example, that "creation" function doesn't really create a bullet (that's what a constructor does); it draws the bullet. So, we should name it that way - i.e. draw().

Similarly, When a bullet "move"s, what changes? Its radius? No, its position. So, we update the bullet's postion in there, according to its speed. Hold on - it doesn't have a speed? Oh, we need to think more about how we model the object, then. The Entity setPosition allows for - oh, I get it now; 'rad' is not an object radius but a turning radius. That's another point: use meaningful names; it doesn't cost you anything to lengthen them except your own typing effort, because they "don't exist" in the compiled version.

Anyway, bullets don't turn about some circle while they move, do they? So, that stuff doesn't belong in the Entity class, because it isn't stuff that makes sense for every Entity.

That follows from the Liskov substitution principle (google it). What we want is to make sure that a derived object (Bullet) could be used anywhere a base object (Entity) could. Consequently, the Entity should not "be able to do" things that a Bullet can't.

So, we take the angle and turning radius out of the Entity, and move it to the Player (which is the thing that can move in a circle) and give the Bullet additionally a 'velocity'.

Finally, we can clean up that accessor, by thinking about responsibility. You must be wondering how we're going to construct a Bullet, since we have to set its starting location in the constructor... we want to use the player's location, and now I'm telling you you can't "ask" the player for its location? Yep, that's right. Because we aren't going to construct the Bullet - the player is. After all, if you were describing what happens in natural language, you wouldn't say that a bullet comes into existence at the player's location - you'd say the player fires a bullet. So that's exactly what we'll do.

I'll write only the declarations; if you've been following the discussion, you should be able to implement the classes and make use of them.

*** Source Snippet Removed ***


Wow...yet another very nice post from you. Simply amazing.



Share this post


Link to post
Share on other sites
cNoob    295
Quote:
Original post by Chad Smith
Wow...yet another very nice post from you. Simply amazing.


Agreed, Great post Zahlman.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this