Sign in to follow this  
leagal4ever

key press in vector

Recommended Posts

Hello, im changing handling how keys are pressed, basicly im having number keys 1-9 and for each key same function is run.

				case SDLK_1:
					KeyPress(1);
					break;

				case SDLK_2:
					KeyPress(2);
					break;

 

it should look like this

std::vector < SDLKey > numberEnum_vec;
numberEnum_vec.push_back( SDLK_1 );
numberEnum_vec.push_back( SDLK_2 );

				for(int i=0; i<numberEnum_vec.size(); i++)
				{
					case numberEnum_vec[i+1]:

						break;
				}

Thanks for answer!

Share this post


Link to post
Share on other sites

How can there be an answer when you didn't even ask a question?

 

So instead, let's go over all the things that are wrong:

 

-You iterate over all elements in your vector and by pointlessly adding +1 to 'i' you skip the first element and access invalid memory on the last iteration

 

-There is a 'case' simply popping up in the middle of nowhere without an enclosing 'switch'

 

-'case' is using a dynamic expression, when it can only be a constant integral type

 

-the case isn't doing anything, raising the question what you're even trying to achieve

Share this post


Link to post
Share on other sites
Switch statements don't work like that. Case labels must be integral constants. It looks like you want to be able to remap keys to different actions. There are a few different ways to do that. One way is to use a std::map. Ex:
enum ActionType { ACTION_1, ACTION_2, ACTION_3 };
typedef std::map<SDLKey, ActionType> KeyActionMap;
KeyActionMap key_map;
key_map[SDLK_1] = ACTION_1;
key_map[SDLK_2] = ACTION_2;
key_map[SDLK_3] = ACTION_3;

KeyActionMap::iterator itr = key_map.find(key);
if (itr != key_map.end()) {
  switch (itr->second) {
    case ACTION_1:
      // stuff
      break;
    case ACTION_2:
      // stuff
      break;
    case ACTION_3:
      // stuff
      break;
  }
}

Share this post


Link to post
Share on other sites

My guess is that you are tying to queue up some keys then send them to some application?

 

	std::vector<SDLKey> v;
	v.push_back(SDLK_1);
	v.push_back(SDLK_2);

	for(std::vector<SDLKey>::const_iterator i = v.begin(); i != v.end(); i++)
	{
		switch (*i)
		{
			case SDLK_1:
				KeyPress(1);
				break;

			case SDLK_2:
				KeyPress(2);
				break;

			default:
				break;
		}
	}

Share this post


Link to post
Share on other sites

I'm not sure if the vector is necessary. If you can rely on the SDL_Key values not changing across platforms, or at least remaining in-order and contiguous:

[code]  SDLKey key = event.key.keysym.sym; if(key >= SDLK_0 && key >= SDLK_9) {     int number = key - SDLK_0;     KeyPress(number); }[/code]

 

If you can't or don't want to rely on that behaviour, you can still use an array (or vector if you wish):

[code]// Some constants const int NUM_KEYS = 10;   const SDLKey Keys[NUM_KEYS] = {     SDLK_0, SDLK_1, /* ... */, SDLK_9 };   // Inside your event loop SDLKey key = event.key.keysym.sym; for(int i = 0 ; i < NUM_KEYS ; ++i) {     if(Keys[i] == key) {         KeyPress(i);     } }  [/code]

Share this post


Link to post
Share on other sites

Of course, if KeyPress is then just another huge switch case that does entirely different things per key, it would make more sense to directly map the actual function. Also, why is called KeyPress and then takes a number that doesn't represent a key?

 

map<SDLKey, function<void(int)> > actionMap;

//Depending on what KeyPress does, this should probably map a more specific function per key
for (int i=1; i<=9; ++i)
    actionMap[SDLK_0+i] = bind(KeyPress, i);


//Test only to avoid adding tons of empty functions for unused keys (also, I'd have to check if it's safe to call a default constructed std/boost::function)
if (actionMap.find(key) != actionMap.end())
   actionMap[key]();

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