Input Handling a Key Press

[civicdude95]

Ok so I'm implementing an input handler class to make checking for input easier in my game and I cannot seem to get the code correct to check for a key press as opposed to a key simply being held down.
I'm using a class with static methods and two static variables, currentKeyState and lastKeyState. What I'm trying to do is call this class's update method every frame, setting the lastKeyState = currentKeyState and setting currentKeyState = SDL_GetKeyState(NULL);
Then I want to run a KeyPressed() method that returns true if currentKeyState[key] && !lastKeyState[key]

I will show the code I'm using and I ask you guys to look it over and maybe offer some advice. Thanks in advance.

Code: Select all

#ifndef INPUTHANDLER_H
#define INPUTHANDLER_H

#include "SDL\SDL.h"

class InputHandler
{
public:
	static void Update();
	static bool IsKeyDown(SDLKey);
	static bool IsKeyUp(SDLKey);
	static bool KeyPressed(SDLKey);
	static bool KeyReleased(SDLKey);
private:
	InputHandler();
	~InputHandler();
	static Uint8* currentKeyState;
	static Uint8* lastKeyState;
};

#endif

Code: Select all

#include "InputHandler.h"

InputHandler::InputHandler() {}

InputHandler::~InputHandler() {}

void InputHandler::Update()
{
	lastKeyState = currentKeyState;
	currentKeyState = SDL_GetKeyState(NULL);
}

bool InputHandler::IsKeyDown(SDLKey key)
{
	return currentKeyState[key];
}

bool InputHandler::IsKeyUp(SDLKey key)
{
	return !currentKeyState[key];
}

bool InputHandler::KeyPressed(SDLKey key)
{
	return currentKeyState[key] && !lastKeyState[key];
}

bool InputHandler::KeyReleased(SDLKey key)
{
	return lastKeyState[key] && !currentKeyState[key];
}

Uint8* InputHandler::currentKeyState = SDL_GetKeyState(NULL);
Uint8* InputHandler::lastKeyState = SDL_GetKeyState(NULL);

[xiphirx]

First thing I noticed

Code: Select all

Uint8* InputHandler::currentKeyState = SDL_GetKeyState(NULL);
Uint8* InputHandler::lastKeyState = SDL_GetKeyState(NULL);

Shouldn't that be in the constructor with

Code: Select all

currentKeyState = SDL_GetKeyState(NULL);
lastKeyState = SDL_GetKeyState(NULL);

?

Your currentKeyState and lastKeyState variables are private...

[civicdude95]

Well that was the way I tried it at first, but I kept getting two LNK2001 errors:

1>InputHandler.obj : error LNK2001: unresolved external symbol "private: static unsigned char * InputHandler::lastKeyState" (?lastKeyState@InputHandler@@0PAEA)
1>InputHandler.obj : error LNK2001: unresolved external symbol "private: static unsigned char * InputHandler::currentKeyState" (?currentKeyState@InputHandler@@0PAEA)

So I did some research and ended up changing it to:

Code: Select all

Uint8* InputHandler::currentKeyState = SDL_GetKeyState(NULL);
Uint8* InputHandler::lastKeyState = SDL_GetKeyState(NULL);

I'm pretty sure the input error that I'm haveing is a logical error, because the input part itself works just fine when I'm only looking for if a key is down. The logical part that I can't figure out is when a key is pressed.

[Falco Girgis]

First thing I noticed

Code: Select all

Uint8* InputHandler::currentKeyState = SDL_GetKeyState(NULL);
Uint8* InputHandler::lastKeyState = SDL_GetKeyState(NULL);

Shouldn't that be in the constructor with

Code: Select all

currentKeyState = SDL_GetKeyState(NULL);
lastKeyState = SDL_GetKeyState(NULL);

?

Your currentKeyState and lastKeyState variables are private...

No. Those are STATIC member variables.

[xiphirx]

First thing I noticed

Code: Select all

Uint8* InputHandler::currentKeyState = SDL_GetKeyState(NULL);
Uint8* InputHandler::lastKeyState = SDL_GetKeyState(NULL);

Shouldn't that be in the constructor with

Code: Select all

currentKeyState = SDL_GetKeyState(NULL);
lastKeyState = SDL_GetKeyState(NULL);

?

Your currentKeyState and lastKeyState variables are private...

No. Those are STATIC member variables.

So what? Why can you access private static variables like that?

[civicdude95]

So what? Why can you access private static variables like that?

Because they are within the InputHandler class's scope.

[Falco Girgis]

First thing I noticed

Code: Select all

Uint8* InputHandler::currentKeyState = SDL_GetKeyState(NULL);
Uint8* InputHandler::lastKeyState = SDL_GetKeyState(NULL);

Shouldn't that be in the constructor with

Code: Select all

currentKeyState = SDL_GetKeyState(NULL);
lastKeyState = SDL_GetKeyState(NULL);

?

Your currentKeyState and lastKeyState variables are private...

No. Those are STATIC member variables.

So what? Why can you access private static variables like that?

You aren't "accessing" them, you're initializing them. That's how static member variables are initialized. How else are you going to initialize something that is not bound to any particular instance of a class? That's how it works.

[Ginto8]

okay, What exactly is the issue? Are you trying to figure out how to go about it, or is it not working? if it isn't working, what exactly is going wrong? You ask us for help, and then just throw some code at us and say "why isn't it working?" That's not very helpful to those who want to help you

[Falco Girgis]

okay, What exactly is the issue? Are you trying to figure out how to go about it, or is it not working? if it isn't working, what exactly is going wrong? You ask us for help, and then just throw some code at us and say "why isn't it working?" That's not very helpful to those who want to help you

He just asked for advice/critiques on his code. He didn't say anything anywhere about something not working or not compiling, guys.

[civicdude95]

He just asked for advice/critiques on his code. He didn't say anything anywhere about something not working or not compiling, guys.

Thanks Gyro.

I probably didn't explain it well enough in the OP but my problem is that I am trying to detect a single key press as opposed to a key being held down.

The logic that I am using involves having two sets of Uint8* keyStates (one current and one last) that I can check in my methods.

The KeyPressed(SDLKey); method is the main one I am referring to because if I press (for example) 'q' to quit to the main menu and also have 'q' setup to be the "quit game" button, there is no way for me to press and release 'q' quick enough (just using the IsKeyDown() method) to avoid quitting the entire game.

For this reason, I wrote the KeyPressed method to try and detect a single key press. The problem that I am running into with the way it is implemented in the code in the OP is that the game is detecting a keydown with my IsKeyDown method but not detecting a key press with my KeyPressed method.

So I said all of that ask, "Does my logic make sense in KeyPressed()?"

[Ginto8]

AHA! This is the issue:

Code: Select all

lastKeyState = currentKeyState;
currentKeyState = SDL_GetKeyState(NULL);

Because SDL's keystate is internal, lastKeyState and currentKeyState point to the same thing. However, you can do this instead:

Code: Select all

lastKeyState = new Uint8[SDLK_LAST];
memcpy(lastKeyState,currentKeyState,SDLK_LAST);

The first line do at the start, or better yet have its static size be SDLK_LAST. The second line, do during each update function. Keep in mind that this will notify you of the change a frame after it happens, but that shouldn't be too much of an issue.

Also, you don't need to set currentKeyState each frame. It will always point to the same data

[civicdude95]

Awesome that works perfectly now! thanks a lot for the help. I thought it might be something like that because I know the logic should work but I don't know that much of the guts of SDL yet.

[Falco Girgis]

Also, I recommend you distinguish between a key being "down" and a key being "tapped." When you move (pushing an arrow key), you are going to want to count that button as being pressed every frame. When you do something like jump, you are going to want to count that button as being pressed only one frame.

We have a getDown(KEY) for the former and a getTapped(KEY) for the latter.

[Ginto8]

Also, I recommend you distinguish between a key being "down" and a key being "tapped." When you move (pushing an arrow key), you are going to want to count that button as being pressed every frame. When you do something like jump, you are going to want to count that button as being pressed only one frame.

We have a getDown(KEY) for the former and a getTapped(KEY) for the latter.

and he has IsKeyDown and KeyPressed

[Falco Girgis]

Also, I recommend you distinguish between a key being "down" and a key being "tapped." When you move (pushing an arrow key), you are going to want to count that button as being pressed every frame. When you do something like jump, you are going to want to count that button as being pressed only one frame.

We have a getDown(KEY) for the former and a getTapped(KEY) for the latter.

and he has IsKeyDown and KeyPressed

Damn. This time I didn't read it fully. Nevermind then.