Ok, So I Feel Like An Idiot


PSyMastR

\m/O__O\m/
Joined
Sep 14, 2005
Messages
2,968
Website
Visit site
Coding in C++, using SDL.

Hey everyone, so I decided to clean up my code and make it neater (using classes for sprites, instead of just a crapton of variables and lines of code in main). I somehow lost control of my player, where I get the data from an event poll. Before I had one event poll in a main loop, with bools for each key (false if not pressed, true if pressed). I then moved a seperate control code to my player_sprite class, and it no longer worked. I want to be able to call player.handleInput(), and have it detect key presses, but with the same code as before (worked totally fine) it did not. Just wondering if anyone here can help. In the main loop, I am doing it like this:
CODE

while(!game_ended)
{
player.handleInput();
player.draw();
}



Here is my handleInput function from my player_sprite class:

CODE

void handleInput()
{
bool key_up, key_down, key_left, key_right, key_enter;
SDL_Event event;
while (SDL_PollEvent(&event))
{
switch (event.type)
{
case SDL_KEYDOWN:
{
//IF ESCAPE is pressed, then quit
if(event.key.keysym.sym == SDLK_DOWN)
{
key_down = true;
}
else if(event.key.keysym.sym == SDLK_UP)
{
key_up = true;
}
else if(event.key.keysym.sym == SDLK_LEFT)
{
key_left = true;
}
else if(event.key.keysym.sym == SDLK_RIGHT)
{
key_right = true;
}
else if(event.key.keysym.sym == SDLK_RETURN)
{
key_enter = true;
}
break;
}
case SDL_KEYUP:
{
if(event.key.keysym.sym == SDLK_DOWN)
{
key_down = false;
}
else if(event.key.keysym.sym == SDLK_UP)
{
key_up = false;
}
else if(event.key.keysym.sym == SDLK_LEFT)
{
key_left = false;
}
else if(event.key.keysym.sym == SDLK_RIGHT)
{
key_right = false;
}
else if(event.key.keysym.sym == SDLK_RETURN)
{
key_enter = false;
}
}
}
}
//Player X Value Controls
if(key_left)
{
velocity_x -= ACCEL_CONST;
if(velocity_x < -1.0 * ACCEL_MAX)
{
velocity_x = -1.0 * ACCEL_MAX;
}
else if(velocity_x > 0)
{
velocity_x = 0;
}
}
else if(key_right)
{
velocity_x += ACCEL_CONST;
if(velocity_x > ACCEL_MAX)
{
velocity_x = ACCEL_MAX;
}
else if(velocity_x < 0)
{
velocity_x = 0;
}
}
if(!key_left && !key_right)
{
if(velocity_x != 0)
{
if(velocity_x > 0)
{
velocity_x -= ACCEL_CONST;
}
else if(velocity_x < -0)
{
velocity_x += ACCEL_CONST;
}
}
}
//Player Y Value Controls
if(key_up)
{
velocity_y -= ACCEL_CONST;
if(velocity_y < -1.0 * ACCEL_MAX)
{
velocity_y = -1.0 * ACCEL_MAX;
}
else if(velocity_y > 0)
{
velocity_y = 0;
}
}
else if(key_down)
{
velocity_y += ACCEL_CONST;
if(velocity_y > ACCEL_MAX)
{
velocity_y = ACCEL_MAX;
}
else if(velocity_y < 0)
{
velocity_y = 0;
}
}
if(!key_up && !key_down)
{
if(velocity_y != 0)
{
if(velocity_y > 0)
{
velocity_y -= ACCEL_CONST;
}
else if(velocity_y < 0)
{
velocity_y += ACCEL_CONST;
}
}
}
x_coord += velocity_x;
y_coord += velocity_y;

//Check screen bounds for x
if(x_coord < 0)
{
x_coord = 0;
}
else if(x_coord > SCREEN_WIDTH)
{
x_coord = SCREEN_WIDTH;
}
//Check screen bounds for y
if(y_coord < 0)
{
y_coord = 0;
}
else if(y_coord > SCREEN_HEIGHT)
{
y_coord = SCREEN_HEIGHT;
}
}



Is this the correct method, or is there a much better way to do this. (It worked fine until I classed my character)
 
I would think you'd want to make these all static so they keep their values inbetween calls to handleInput():
bool key_up, key_down, key_left, key_right, key_enter;

The way they are declared now, they're just temporary variables on the stack that probably contain garbage values. If it worked before like this (inside a function, that is), it was simply chance that the stack frame of the procedure got positioned into the same place on the stack call after call.
 
How many of "while (SDL_PollEvent(&event))" do you have in the whole code base?
 
Thanks for all the responses :D

WhiteFalcon said:
And what about using SDL_GetKeyState() ?

CODE

unsigned char *keystates = SDL_GetKeyState(NULL);
Yeah, I'll Probably do that. That seems like a much better solution. Thank you.

yaustar said:
How many of "while (SDL_PollEvent(&event))" do you have in the whole code base?
I have 3 in total, but they are never accessed at the same time. The way I setup different parts of the game is inside my main loop I have:

If (mainmenu)

else if(highscores)

else if(game)

else if(about)

etc... and inside those is the polling events.

EDIT: I have now change my code around to using SDL_GetKeyState, but still with no luck. All my other functions with input work perfectly, but still not the character movement. I changed the handleInput to be:

CODE
keystates = SDL_GetKeyState(NULL);
if(keystates[SDLK_ESCAPE])
{
game = false;
main_menu = true;
key_escape = false;
Mix_PlayMusic(menu_music, -1);
}
player.handleInput(keystates);



(Note: The Escape key detection works perfectly, but when I pass off the keystates array... nope!)

and inside handleInput, I have this:

CODE
void handleInput(Uint8 *keystates)
{
//Player X Value Controls
if(keystates[SDLK_LEFT])
{
velocity_x -= ACCEL_CONST;
if(velocity_x < -1.0 * ACCEL_MAX)
{
velocity_x = -1.0 * ACCEL_MAX;
}
else if(velocity_x > 0)
{
velocity_x = 0;
}
}

ETC...
 
Last edited by a moderator:
PSyMastR said:
All my other functions with input work perfectly, but still not the character movement.
Show us your complete player class definition and also where you declare the instance of player. Also show us a class where it's working.

Does anything in the player class work?
 
Last edited by a moderator:
Everything in the player class works perfect. If I use its setCoordinates(int x, int y) function, then I can update it outside the class, just for some reason input handlnig isn't working inside the class.

I create the player:
CODE
//Create the player
ControlledPlayer player(SCREEN_WIDTH / 2, SCREEN_HEIGHT / 2, 32, 64, "images/pixel.png", 0.25, 0.75);


Right outside my main program loop.

I call the functions inside my main program loop (here is my entire game else if statement):
CODE

else if(game)
{
//Input Processing
SDL_Event event;
while (SDL_PollEvent(&event))
{
switch (event.type)
{
//If window is closed, then quit
case SDL_QUIT:
done = true;
break;
}
}
keystates = SDL_GetKeyState(NULL);
if(keystates[SDLK_ESCAPE])
{
game = false;
main_menu = true;
key_escape = false;
Mix_PlayMusic(menu_music, -1);
}
player.handleInput(keystates);

SDL_BlitSurface(game_background, 0, screen, &game_background_location);
player.draw(screen);

double xvel, yvel;
player.getVelocity(xvel, yvel);
font2.WriteBitmapFont("Acc X: " + to_string(xvel), SCREEN_WIDTH - 161, 1, screen);
font2.WriteBitmapFont("Acc Y: " + to_string(yvel), SCREEN_WIDTH - 161, 19, screen);
font1.WriteBitmapFont("You are in the game. \n To exit the game, hit ESC!", 1, 1, screen, true, 800);
}

//Finally, update the screen :)
SDL_Flip(screen);



The excape thing works perfectly, if I quit the program or hit escape.

Player handling doesn't work.

Here is a segment of my player class:
CODE

class ControlledPlayer
{
SDL_Surface* player_graphic;
SDL_Rect player_location;
int x_coord, y_coord, width, height;
double velocity_x, velocity_y;
//Acceleration and Maximum Acceleration
int ACCEL_CONST, ACCEL_MAX;

public:
ControlledPlayer(int x_start, int y_start, int w, int h, std::string image_location, double accel, double accel_max)
{
x_coord = x_start;
y_coord = y_start;
width = w;
height = h;
velocity_x = 0.0;
velocity_y = 0.0;
ACCEL_CONST = accel;
ACCEL_MAX = accel_max;
player_graphic = loadimage(image_location, true);
}
void handleInput(Uint8 *keys)
{
//Player X Value Controls
if(keys[SDLK_LEFT])
{
velocity_x -= ACCEL_CONST;
if(velocity_x < -1.0 * ACCEL_MAX)
{
velocity_x = -1.0 * ACCEL_MAX;
}
else if(velocity_x > 0)
{
velocity_x = 0;
}
}
//Etc... (Cut off to save space)
}
 
I am not sure about it, have been using SDL only for a short time, but how I am doing it is

1) I call the SDL_GetKeyState(NULL) function only once at program start

2) I have the line

CODE
while (SDL_PollEvent(&event))

only once there and split the program flow in there.

3) I have the keystates variable in the main loop, with your approach maybe make it a global instead of passing it?

4) because of your more SDL_PollEvent(&event) loops, could it be you are actually tosing away some keypresses in particular cases?
 
Are ACCEL_CONST and ACCEL_MAX really ints? Maybe I'm missing something, but won't they get rounded down to 0 given the values you pass to the constructor? And ACCEL_MAX is really max velocity, right?

You'll probably want to get familiar with whatever debugger your IDE of choice provides. Setting some breakpoints, stepping through the code and checking that values are set as expected is a great way to pick out this kind of behaviour.
 
Multiplex said:
Are ACCEL_CONST and ACCEL_MAX really ints? Maybe I'm missing something, but won't they get rounded down to 0 given the values you pass to the constructor? And ACCEL_MAX is really max velocity, right?

You'll probably want to get familiar with whatever debugger your IDE of choice provides. Setting some breakpoints, stepping through the code and checking that values are set as expected is a great way to pick out this kind of behaviour.
Yep, I second all of what Multiplex says.

Is your compiler spitting out a warning about incompatible types for these lines:

CODE

ACCEL_CONST = accel;
ACCEL_MAX = accel_max;



I think all you need to do is to change ACCEL_CONST and ACCEL_MAX to be doubles.
 
Last edited by a moderator:
OH MAN! I am an IDIOT! Haha. Thanks guys. Everything works perfect now :)

I had those originally as #defines, totally forgot they were decimal values...

Stupid me.

EDIT: Well now only left and up works, not down and right... hehe. I guess its probably something else, for ever fix, there are more bugs :p

EDIT2: Got everything :D
 
You might want to go through and clean up the code related to all warnings output by your compiler. That way, if you reintroduce an "oops!" like this (we all do that) then you'll quickly spot it.

It's a good practice to treat warnings as errors for this very reason.

Happy coding - and I look forward to seeing the final game :)
 
Back
Top