Efficiently checking input and firing events

Question

I'm writing an InputHandler class in XNA, and there are several different keys considered valid input (all of type Microsoft.XNA.Framework.Input.Keys). For each key, I have three events:

internal event InputEvent XYZPressed;
internal event InputEvent XYZHeld;
internal event InputEvent XYZReleased;

where XYZ is the name of the Keys object representing that key. To fire these events, I have the following for each key:

if (Keyboard.GetState().IsKeyDown(XYZ))
{
  if (PreviousKeyState.IsKeyDown(XYZ))
  {
    if (XYZHeld != null)
      XYZHeld();
  }
  else
  {
    if (XYZPressed != null)
      XYZPressed();
  }
}
else if (PreviousKeyState.IsKeyDown(XYZ))
{
  if (XYZReleased != null)
    XYZReleased();
}

However, this is a lot of repeated code (the above needs to be repeated for each input key). Aside from being a hassle to write, if any keys are added to/removed from the keys (if functionality is added/removed), a new section needs to be added (or an existing one removed).

Is there a cleaner way to do this? Perhaps something along the lines of

foreach key
  check which state it's in
  fire this key's event for that state

where the code does the foreach (automatically checking exactly those keys that "exist") rather than the coder?

Answer 1

I would definitely avoid having an event for each key, and certainly not an event for each key and state change. Worse, a 'held' event is very ambiguous as to when it should fire: once a frame? Once every X seconds?

To keep full control of your input, it's better to poll at a specific point in your update loop. In that polling function (which I've called CheckForKeyPresses below), you check the current state against the previous state to watch for state changes.

When that function is called, iterate over the list of keys you are interested in, and check if any of them are pressed or released now. If you identify that a key that is pressed now that wasn't before, or a key that is released now when it was pressed before, then you have a state change for that key. Once you've identified that is the case, call a 'dispatcher' function to do some work. E.g.

List<Key> myKeys;
Dictionary<Key, bool> keyPressedState;

private void CheckForKeyPresses()
{
    foreach (Key key in myKeys)
    {
        if (Key.IsPressed(key) && !keyPressedState[key])
        {
            keyPressedState = true;
            HandleKeyPressed(key);
        }
        else if (!Key.IsPressed(key) && keyPressedState[key])
        {
            keyPressedState = false;
            HandleKeyReleased(key);
        }
    }
}

private void HandleKeyPressed(Key key)
{
    if (key == Keys.W)
    {
        GoForward();
    }
    //else if ...
}

Held keys you want to handle slightly differently: in your update loop, at some obvious point when you're checking inputs, call a function to see if any keys which were held are still held, and handle them there.

private void CheckForHeldKeys()
{
    foreach (Key key in myKeys)
    {
        //is it still pressed?
        if (keyPressedState[key] && Key.IsPressed(key))
        {
            HandleKeyHeld(key);
        }
    }
}

Now, the dispatcher logic is probably the thing you want to code as events. I.e. in HandleKeyPressed, if you detect a press of the W key, then you raise the 'go forward' event. That way multiple listeners can react to a single event. This dispatching through events forms your control mapping. The rest of your game doesn't have to know that it's the W key that maps to going forward, because it's bound to a logical control event instead of a physical one. That allows you to rebind W to some arbitrary key, and not have to change a bunch of code elsewhere that assumes that forward is always W.

Answer 2

There's a way if you don't mind using a bit of reflection, but I'm not sure I'd recommend it, especially since the title mentions efficiency. But I'll describe it anyway. I'll assume there's a subset of keys you want to cover, and not the entire Keys enumeration. If you want to cover every key, iterate directly over the enumeration values instead of using the array below:

1. Add every key you're interested in to an array:

   public Keys[] keys = {Keys.A, Keys.B, Keys.C};
   

2. Create the events for these keys. I'll use a simple Action delegate which has no parameters and no return value (add other types of events).

   public event Action APressed;
   public event Action BPressed;
   public event Action CPressed;
   

3. Next we need an utility method to call an event by name. This is where the reflection comes in. Once again I'll assume that there are no parameters to the events, and I'll place the method in the same class that stores the events:

   private void RaiseEventByName(string eventName)
   {
       var eventDelegate = (MulticastDelegate)GetType().GetField(eventName, BindingFlags.Instance | BindingFlags.NonPublic).GetValue(this);
       if (eventDelegate != null)
           foreach (var handler in eventDelegate.GetInvocationList())
               handler.Method.Invoke(handler.Target, null);
   }
   

4. Finally our Update loop becomes something like (add logic for other states):

   foreach(Keys key in keys)
       if (currentKeyboardState.IsKeyDown(key) && previousKeyboardState.IsKeyUp(key))
           RaiseEventByName(key.ToString() + "Pressed");
   

Answer 3

So I have been thinking about this a lot, and as this question is quite old, I decided to help future generations, even though XNA is discontinued it may be useful with other programming languages. The InputHelper that I wanted to design would be able to handle additions and modifications easily, therefore I don't want to program that the W key specifically is pressed, instead I'd like to state that the 'Forward' key has been pressed, or perhaps a list of keys. The way I wanted to accomplish this is having a class KeyBind. This class holds names (for internal use and for KeyBind modifications the player can relate to), a list of keys and certain events. The structure would be as follows:

class KeyHelper {
    public string Name;
    public string GraphicalName;
    public ICollection<Keys> Keys;

    public event EventHandler keyPressed;
    public event EventHandler keyReleased;

    public KeyHelper(string Name, string GraphicalName, ICollection<Keys> Keys)
    {
        this.Name = Name;
        this.GraphicalName = GraphicalName;
        this.Keys = Keys;
    }

    public void OnKeyPress(EventArgs e)
    {
        keyPressed(this, e);
    }

    public void OnKeyRelease(EventArgs e)
    {
        keyReleased(this, e);
    }
}

This way, you can just iterate over every keybind in the InputHelper, check if the key has been pressed for this keybind and fire the according event. I'm sure you can figure the details out though, as I just programmed this in the editor for this site.

Anyway, I have to be in the same line with MrCranky, because you want to check for held keys manually, by polling. Unless of course you state that, when you want the Held Key fire every 100 milliseconds. Then it can be done.