[SOLVED]SDL previous keystate system (Only works sometimes?)

[MadPumpkin]

The issue I'm having is interesting. The function KeyHit() works now, but it appears to not always get the key input for some reason. By pressing q, it should negate the boolean renderQuuad. It does this, but only about half the time that I press q..

SDLKeys.h

Code: Select all

#pragma once
#ifndef SDLKeys_h
#define SDLKeys_h

#include <SDL.h>

class SDLKeys
{
private:
	Uint8 *mPreviousKeys;
	Uint8 *mCurrentKeys;

	int *mMouseX;
	int *mMouseY;

public:
	SDLKeys(void);
	~SDLKeys(void);

	void UpdateKeys();
	void UpdateCursor();

	int GetMouseX() const;
	int GetMouseY() const;

	bool KeyHit( int index ) const;
	bool KeyDown( int index ) const;
};

#endif // SDLKeys_h

SDLKeys.cpp

Code: Select all

#include "SDLKeys.h"


SDLKeys::SDLKeys(void)
{
	mPreviousKeys = new Uint8[SDLK_LAST];
	mCurrentKeys = SDL_GetKeyState( NULL );
	mMouseX = NULL;
	mMouseY = NULL;
}

SDLKeys::~SDLKeys(void)
{
	delete mMouseX;
	delete mMouseY;
}

void SDLKeys::UpdateCursor()
{
	SDL_PumpEvents();
	SDL_GetMouseState( mMouseX, mMouseY );
}

void SDLKeys::UpdateKeys()
{
	Uint8* tempKeys = NULL;
	SDL_PumpEvents();

	tempKeys = mPreviousKeys;
	mPreviousKeys = mCurrentKeys;
	mCurrentKeys = tempKeys;
	tempKeys = SDL_GetKeyState( NULL );
	memcpy( mCurrentKeys, tempKeys, SDLK_LAST );
}

int SDLKeys::GetMouseX() const
{
	return *mMouseX;
}

int SDLKeys::GetMouseY() const
{
	return *mMouseY;
}

bool SDLKeys::KeyHit(int index) const
{
	return mCurrentKeys[index] && !mPreviousKeys[index];
}

bool SDLKeys::KeyDown(int index) const
{
	return mCurrentKeys[index] && mPreviousKeys[index];
}

Main.cpp

Code: Select all

#include <SDL.h>
#include <SDL_opengl.h>

#include "SDLTimer.h"
#include "SDLKeys.h"

const int SCREEN_WIDTH = 640;
const int SCREEN_HEIGHT = 480;
const int SCREEN_BPP = 32;

const int FPS = 60;

SDL_Event event;

bool renderQuad = true;

bool InitGL()
{
	//Initialize Projection Matrix
	glMatrixMode( GL_PROJECTION );
	glLoadIdentity();

	//Initialize Modelview Matrix
	glMatrixMode( GL_MODELVIEW );
	glLoadIdentity();

	//Initialize clear color
	glClearColor( 0.f, 0.f, 0.f, 1.f );

	//Check for error
	GLenum error = glGetError();
	if( error != GL_NO_ERROR )
	{
		printf( "Error initializing OpenGL! %s\n", gluErrorString( error ) );
		return false;
	}

	return true;
}

bool Init()
{
	//Initialize SDL
	if( SDL_Init( SDL_INIT_EVERYTHING ) < 0 )
		return false;

	if( SDL_SetVideoMode( SCREEN_WIDTH, SCREEN_HEIGHT, SCREEN_BPP, SDL_OPENGL ) == NULL )
		return false;

	SDL_EnableUNICODE( SDL_TRUE );

	// Init GL
	if( InitGL() == false )
		return false;

	SDL_WM_SetCaption( "Archaic: The Chaos Gauntlets", NULL );
	return true;
}

void Update()
{
}

void Render()
{
	glClear( GL_COLOR_BUFFER_BIT );

	if( renderQuad == true )
	{
		glBegin( GL_QUADS );
		glVertex2f( -0.5f, -0.5f );
		glVertex2f(  0.5f, -0.5f );
		glVertex2f(  0.5f,  0.5f );
		glVertex2f( -0.5f,  0.5f );
		glEnd();
	}

	SDL_GL_SwapBuffers();
}

void Clean()
{
	SDL_Quit();
}

int main( int argc, char *argv[] )
{
	SDLTimer *t = new SDLTimer();
	SDLKeys *k = new SDLKeys();

	bool quit = false;

	if( Init() == false )
		return 1;

	while( quit == false )
	{
		t->Start(); // Frame timer

		//While there are events to handle
		while( SDL_PollEvent( &event ) )
		{
			if( event.type == SDL_QUIT ) {
				quit = true;
			} // else if( event.type == SDL_KEYDOWN ) {}
		}

		k->UpdateKeys();
		k->UpdateCursor();
		if( k->KeyHit( SDLK_q ) )
		{
			renderQuad = !renderQuad;
			printf("q ");
		}

		/* Frame functions */
		Update();
		Render();

		if( t->GetTicks() < 1000/FPS )
		{
			SDL_Delay( ( 1000/FPS ) - t->GetTicks() );
		}
	}

	delete k;
	delete t;
	Clean();

	return 0;
}

[Fillius]

There are various problems with this code, the one that is propably causing your error, as far as I can see, should be this:

The pointer returned by SDL_GetKeyState points to an internal array used by SDL, it remains valid during the whole lifetime of your application and is updated at each call to SDL_PumpEvents. You should only read this array and there is no need to call SDL_GetKeyState more than once, it will return the same pointer each time. In your first call to UpdateKeys() you swap your internal array pointer with the one returned by SDL_GetKeyState() and overwrite the contents of your own array. This is more or less ok, when you call it again, however, mCurrentKeys is a pointer to your own array, mPreviousKeys belongs to SDL. You swap the two and copy the array belonging to SDL over itself. Not only is this a bad thing because you should never modify SDLs internal key array, but it will obviously not work as expected.

Another part of your code, that will undoubtedly cause some kind of a problem is your usage of SDL_GetMouseState(). This function, unlike SDL_GetKeyState, will not return something that remains valid. It expects you to pass in pointers to existing int variables and will store the current position of the cursor in those. It also accepts NULL pointers, which is the reason your program does not crash(yet).Once you call the GetMouseX() or GetMouseY() methods you will dereference a NULL pointer, which is a very very bad thing...

Furthermore, you never free the array you allocate in SDLKeys constructor and should not need to use new and delete for your SDLKeys object in main.

Some of this information might not be up to date or correct, since I havent used SDL at all for about 2 years, so if any of my explanations above are fundamentally wrong I beg your forgiveness.

[MadPumpkin]

Alright I attempted to change the code accordingly and now it doesn't work at all:

Code: Select all

SDLKeys::SDLKeys(void)
{
	mPreviousKeys = new Uint8[SDLK_LAST];
	mCurrentKeys = SDL_GetKeyState( NULL );
	mMouseX = NULL;
	mMouseY = NULL;
}

SDLKeys::~SDLKeys(void)
{
	delete[] mPreviousKeys;
	mCurrentKeys = NULL;
}

void SDLKeys::UpdateCursor()
{
	SDL_PumpEvents();
	SDL_GetMouseState( &mMouseX, &mMouseY );
/* The delcaration for mMousex are now just ints */
}

#include <utility>
void SDLKeys::UpdateKeys()
{
	SDL_PumpEvents();
	memcpy( mPreviousKeys, mCurrentKeys, SDLK_LAST );
}

I can't say I know why it doesn't work like this.

[Fillius]

Well, as far as I can see it, the contents of mPreviousKeys and mCurrentKeys seem to be the same after each call to UpdateKeys(), because you copy the contents after they are updated. You should propably put your memcpy before you call SDL_PumpEvents().

[MadPumpkin]

Hmm. Thanks for all help thus far :P. That doesn't appear to work though.

EDIT: It works! I did have to call the memcpy first, the reason it didn't work at first after changing it is because I was calling it in main, after SDL_PollEvents.

Thanks much for your help!

[Fillius]

I presume you still call your UpdateKeys() and UpdateMouse() methods after invoking SDL_PollEvent()? I was a little unsure about this point, so I checked it(http://sdl.beuc.net/sdl.wiki/SDL_PollEvent) and SDL_PollEvent calls SDL_PumpEvents, so the state of the key array is updated before you even call your UpdateKeys

[MadPumpkin]

THat's what I just changed. If I don't call UpdateKeys() BEFORE the PollEvents loop in main, then it doesn't work.