Sign in to follow this  
peeptheflix

help! demonic c++ function driving me insane

Recommended Posts

Hello GameDev, I'm making a side-scroller in SDL in which enemies shoot projectiles at you. So far so good, right? So I made a projectile class, instantiated 7 (global variable MAX_PROJECTILES) projectiles in an array, and made a function to display and initialize the position of these projectiles. This function is called every time the player presses SPACE.
void addProjectile(enum color pC) {
     for(int i=0; i<MAX_PROJECTILES; i++) {
         if(projectiles[i].getDrawn()==0) {
             projectiles[i].changeColor(pC);
             projectiles[i].xset(enemy.getx()-PROJECTILE_SIZE);
             projectiles[i].yset(enemy.gety()+(enemy.geth()/2)-(PROJECTILE_SIZE/2));
             projectiles[i].setDrawn(1);
             projectiles[i].setDir(-1);
             return;
         }
     }
}
The way I see this function, it should be called by main(), loop through all the projectiles in the for() loop, find a projectile object that is not drawn (if statement), call some member functions, and then RETURN, terminating the activity of the function. What happens when I compile this sucker is that the function loops 7 times like it's supposed to, but also calls the member functions once for each loop, instead of terminating after the first go. This, of course, makes the enemy instantly fire a series of 7 projectiles in succession, which is very bad for the player. And now for the craziest part. I tried commenting out the return statement just to see what happens, and got what I expected, a perfect superimposed stack of 7 projectiles. So, how is it possible that the return statement only slightly delays the creation of a new projectile instead of completely terminating the function? Somebody deliver me from this torment!

Share this post


Link to post
Share on other sites
I'm guessing this entirely depends on how you are handling your input. Say I hold down space, and every frame your game checks to see if space is held down....

1) With return statement: addPRojectile gets called, fills in data for a projectile and returns, next loop through the game it calls it again and fills in the next. End result is 7 projectiles 1 frame apart.

2) Without return statement: addProjectile gets called, all 7 projectiles get filled out on the same frame and end up on top of each other.

This looks pretty much like your situation.

Share this post


Link to post
Share on other sites
Quote:
Original post by Amvi88
You should use a break instead of a return, the return is just making an overhead but is actually doing nothing

Err, break and return end up doing the exact same thing here, and your compiler is almost certain to compile both into the exact same machine code here.

Share this post


Link to post
Share on other sites
I would suggest you are calling addProjectile() every update once space is pressed. You only want to call addProjectile() once when the space bar is first pressed and not whilst it is held.

Depending on how your handling input from the keyboard the simplest fix would be to have a variable at the top of main() defined like: bool spacePressed = FALSE;

Then set spacePressed to TRUE when you detect the space key in your input handling code.

Then call
if(spacePressed) { addProjectile(...); }


Finally in the input handler wrap your
spacePressed = TRUE;
code that you added earlier, and put it in
if(spacePressed) {
spaceHeld = TRUE;
spacePressed = FALSE;
} else { spacePressed = TRUE; }


That if statement basically says "if space pressed previously then say space is now held, otherwise space has only just been pressed."


Hope that makes sense and is the solution to your issue.

Share this post


Link to post
Share on other sites
Try to think of your operations here in more atomic terms.

First, you want to find an item in the list that is usable.

Second, you want to use the item in the list and initialize it with data.

You may wish to add functions to do these common tasks, like so:


Projectile* findUndrawnProjectile(){
Projectile* result = 0;
for(int i=0; result==0 && i<MAX_PROJECTILES; i++) {
if(projectiles[i].getDrawn()==0) {
result = &projectiles[i];
}
}
return result;
}

void initializeProjectile(Projectile* theProjectile,color pc,int x,int y,int dir){
if(!theProjectile) return;
theProjectile->changeColor(pc);
theProjectile->xset(x);
theProjectile->yset(y);
theProjectile->setDrawn(1);
theProjectile->setDir(dir);

}

void addProjectile(enum color pC) {
if(Projectile* projectile = findUndrawnProjectile()){
initializeProjectile(projectile,pC,enemy.getx()-PROJECTILE_SIZE,enemy.gety()+(enemy.geth()/2)-(PROJECTILE_SIZE/2),-1);
}
}



At this point, its a little more organize and clearly differentiated between different operations.

Of course, now the question is: why isn't there a member of Projectile that will do this initialization for you? Also, why isn't there a class that represents a collection of projectiles, rather than a global list?

But, as always, we take it a step at a time.

Share this post


Link to post
Share on other sites
thanks to everyone for the quick responses! I think I have a better idea of what's going on now.

For good measure, here's the beginning of my game loop, including how I did my input:


while(Running) {
mIO.ClearScreen();
DrawScene();
mIO.UpdateScreen();

mIO.Pollkey();

keys = SDL_GetKeyState(NULL);
if(Running) {
if(keys[SDLK_RCTRL]) {
addProjectile(RED);
}
if(keys[SDLK_RSHIFT]) {
addProjectile(BLUE);
}
if(keys[SDLK_SPACE]) {
SwordDraw = 1;
} else {
SwordDraw = 0;
}
}


Also, here is my Pollkey function:


int IO::Pollkey() {
SDL_Event event;
while(SDL_PollEvent(&event)) {
switch(event.type) {
case SDL_KEYDOWN:
return event.key.keysym.sym;
case SDL_QUIT:
exit(3);
}
}
return -1;
}


I based this input (also a lot of my other code) on the Cone3D space shooter tutorial, which somehow manages to call the function only once on keypress! I even set SDL_EnableKeyRepeat to 0 to make sure that it's not just the key being held down... still no results.

Share this post


Link to post
Share on other sites
Quote:
Original post by peeptheflix
Also, here is my Pollkey function:
int IO::Pollkey() {
SDL_Event event;
while(SDL_PollEvent(&event)) {
switch(event.type) {
case SDL_KEYDOWN:
return event.key.keysym.sym;
case SDL_QUIT:
exit(3);
}
}
return -1;
}
This function is going to come back and bite you, later in development:

You may only care about keydown events at the moment, but eventually you will want to handle other types of events, and this function will transparently discard any other events.

Your switch statement should also have a default case, which just breaks/returns;

Lastly, executing a call to exit() inside this function is just plain evil. Instead you need to propagate this message upwards to your main run loop, so that everything has a chance to cleanup.

Share this post


Link to post
Share on other sites
Quote:
You may only care about keydown events at the moment, but eventually you will want to handle other types of events, and this function will transparently discard any other events.


And when that time comes, the OP can simply add those cases to the switch.

Quote:
Your switch statement should also have a default case, which just breaks/returns;


Which does nothing, except to explicitly discard unhandled events instead of implicitly discarding them.

OP: By the way, is there any particular reason to limit the projectiles to any specific number? You are aware of std::vector, yes? And how about using constructors to set up objects?

The 'drawn' member makes no sense once you are using objects properly. Just make a container which only actually contains as many projectiles as need to be drawn.


// For example, if the class currently has data like this...
class Projectile {
enum color my_color;
int x, y, dx, dy;
// int drawn; <-- we don't need this

// ... Then add a constructor:
public:
Projectile(int x, int y, enum color c) : my_color(c), x(x), y(y), dx(0), dy(-1) {}
};

// "Tell, don't ask": instead of reading data out of the enemy object to call
// the constructor, have the enemy make the projectile constructor call itself.
class Enemy {
// ...
Projectile fire(enum color c) {
return Projectile(
my_x - PROJECTILE_SIZE,
my_y + (my_h - PROJECTILE_SIZE) / 2,
c
);
}
};

// Then at the point where the projectiles are maintained:
std::vector<Projectile> projectiles; // instead of the array
void addProjectile(enum color c) {
my_projectiles.push_back(enemy.fire(c));
}

Share this post


Link to post
Share on other sites
Quote:
Original post by peeptheflix
thanks to everyone for the quick responses! I think I have a better idea of what's going on now.

For good measure, here's the beginning of my game loop, including how I did my input:


while(Running) {
mIO.ClearScreen();
DrawScene();
mIO.UpdateScreen();

mIO.Pollkey();

keys = SDL_GetKeyState(NULL);
if(Running) {
if(keys[SDLK_RCTRL]) {
addProjectile(RED);
}
if(keys[SDLK_RSHIFT]) {
addProjectile(BLUE);
}
if(keys[SDLK_SPACE]) {
SwordDraw = 1;
} else {
SwordDraw = 0;
}
}


Also, here is my Pollkey function:


int IO::Pollkey() {
SDL_Event event;
while(SDL_PollEvent(&event)) {
switch(event.type) {
case SDL_KEYDOWN:
return event.key.keysym.sym;
case SDL_QUIT:
exit(3);
}
}
return -1;
}


I based this input (also a lot of my other code) on the Cone3D space shooter tutorial, which somehow manages to call the function only once on keypress! I even set SDL_EnableKeyRepeat to 0 to make sure that it's not just the key being held down... still no results.



Using SDL_GetKeyState() is the most likely culprit, it is used for seeing if a key is being held down not if it was just pressed. Since the space bar is most likely being held down between frames that AddProjectile() function is being called each frame instead of only when the space bar is initially pressed.

To see if a key was just pressed use SDL_PollEvent() like you have in your PollKey() method. When a key is pressed it generates an event and you can use the event to determine what key is pressed.

Here is a good tutorial on how to set up the loop. It involves setting up a second switch statement in the event SDL_KEYDOWN is generated.

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
Quote:
You may only care about keydown events at the moment, but eventually you will want to handle other types of events, and this function will transparently discard any other events.
And when that time comes, the OP can simply add those cases to the switch.
My point is that this switch exists inside a function called Pollkey(), which returns the name of a single key. Adding the other events into this switch would be pointless, since as specified it has no channel to communicate anything other than a single key name to the outside world.

Share this post


Link to post
Share on other sites
Thanks again everyone... as you can tell I'm still a c++ baby so I appreciate the patience.

to Zahlman: Yes, I do know about std::vector, but I'm scared to use it when I can't even get my arrays down. That being said, I am going to try and use std::vector because in reality it does make much more sense. Giving the enemy a "fire" function seems like a good idea too.

As for the event loops, I'll check out that lazyfoo tutorial and see if I can fix this pesky problem.

Thanks fo tha help!

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