Hello everyone!

First time posting here on GD, although definitely not my first visit to these forums

Been studying C# (+ .NET framework) give or take 5 months now, done some basic PHP/HTML/Javascript in the past.

I figured, now that I know a lot more about OO design principles, and being a gamer at heart, I'd take a more serious look at game development.

So after recreating an ugly (codewise) but functional Pong in MonoGame, i thought I'd start looking at writing code I can recycle in future projects.

Decided to start with the inputs (keyboard, gamepads, mouse), seeing as it's a core part of any game, and a fairly general-purpose one at that.

I googled the topic for a few hours, and most people seemed to favour a 3-layer approach. But seeing as XNA/MonoGame takes care of all but the last layer I've focused on that.

So basicly, my motive behind this post is to get your feedback and ideas on how to improve / expand upon what I've come up with so far.

Everything from broader design ideas to small performance optimizations are of value and greatly appreciated. (I'm a sucker for smart "best practice" code)

So here's my InputHandler class:

    // rough, unfinished input handler class
    //
    // primary focus (so far) has been:
    // - learning C#, XNA/MonoGame and general design choices in game development. :)
    // - centralized handling of all inputs.
    // - event-driven top layer "abstraction"
    // - reusable code.
    //
    // obviously there's lots of room for improvement, and a couple of ideas are:
    // - custom collection class which incorporates all relevant bindings for a specific playerIndex.
    // - expand upon above idea using inputs (Keyboard, GamePad, Mouse) as component objects referenced in a List
    //   and loop through this List in the Update() method.
    // - adding more modularity & abstraction in general, perhaps allowing for actions/events being added dynamicly.
    //   i.e. making the handler a better fit for a general-purpose top layer game framework.
    // - gamestate-aware/intelligent bindings, some configurable and some static.

    class InputHandler
    {
        private KeyboardState _keyboardStateCurrent, _keyboardStatePrevious;
        private Dictionary<InputActions, Keys> _keyboardBindings;

        private GamePadState _gamePadStateCurrent, _gamePadStatePrevious;
        private Dictionary<InputActions, Buttons> _gamePadBindings;

        private List<PlayerIndex> _playerIndexes;
        public List<PlayerIndex> PlayerIndexes
        {
            get { return _playerIndexes; }
        }

        public delegate void InputEvent();

        // define events
        // ("dead" default subscribers to avoid having to null check before invokes)
        public event InputEvent onMoveLeft = delegate { };
        public event InputEvent onMoveLeftHold = delegate { };
        public event InputEvent onMoveLeftRelease = delegate { };
        public event InputEvent onMoveRight = delegate { };
        public event InputEvent onMoveRightHold = delegate { };
        public event InputEvent onMoveRightRelease = delegate { };
        public event InputEvent onJump = delegate { };
        public event InputEvent onJumpHold = delegate { };
        public event InputEvent onJumpRelease = delegate { };
        public event InputEvent onNavigateBack = delegate { };

        public InputHandler()
        {
            _keyboardStatePrevious = Keyboard.GetState();
            _playerIndexes = new List<PlayerIndex>() { PlayerIndex.One };
            _gamePadStatePrevious = GamePad.GetState(_playerIndexes[0]);
            // if config file exist then load it here, else...
            ResetBindings();
        }

        private void PollKeyboard()
        {
            _keyboardStateCurrent = Keyboard.GetState();

            foreach (KeyValuePair<InputActions, Keys> keyBinding in _keyboardBindings)
            {
                // fire events when key pressed
                if (_keyboardStateCurrent.IsKeyDown(keyBinding.Value) && _keyboardStatePrevious.IsKeyUp(keyBinding.Value))
                {
                    switch (keyBinding.Key)
                    {
                        case InputActions.MovementLeft:
                            onMoveLeft();
                            break;

                        case InputActions.MovementRight:
                            onMoveRight();
                            break;

                        case InputActions.MovementJump:
                            onJump();
                            break;
                        case InputActions.NavigationBack:
                            onNavigateBack();
                            break;
                    }
                }

                // fire events key held
                else if (_keyboardStateCurrent.IsKeyDown(keyBinding.Value) && _keyboardStatePrevious.IsKeyDown(keyBinding.Value))
                {
                    switch (keyBinding.Key)
                    {
                        case InputActions.MovementLeft:
                            onMoveLeftHold();
                            break;

                        case InputActions.MovementRight:
                            onMoveRightHold();
                            break;

                        case InputActions.MovementJump:
                            onJumpHold();
                            break;
                    }
                }

                // fire events when key released
                else if (_keyboardStateCurrent.IsKeyUp(keyBinding.Value) && _keyboardStatePrevious.IsKeyDown(keyBinding.Value))
                {
                    switch (keyBinding.Key)
                    {
                        case InputActions.MovementLeft:
                            onMoveLeftRelease();
                            break;

                        case InputActions.MovementRight:
                            onMoveRightRelease();
                            break;

                        case InputActions.MovementJump:
                            onJumpRelease();
                            break;
                    }
                }
            }
            _keyboardStatePrevious = _keyboardStateCurrent;
        }

        private void PollGamePad(PlayerIndex playerIndex)
        {
            _gamePadStateCurrent = GamePad.GetState(playerIndex);

            foreach (KeyValuePair<InputActions, Buttons> buttonBinding in _gamePadBindings)
            {
                // fire events when button pressed
                if (_gamePadStateCurrent.IsButtonDown(buttonBinding.Value) && _gamePadStatePrevious.IsButtonUp(buttonBinding.Value))
                {
                    switch (buttonBinding.Key)
                    {
                        case InputActions.MovementLeft:
                            onMoveLeft();
                            break;

                        case InputActions.MovementRight:
                            onMoveRight();
                            break;

                        case InputActions.MovementJump:
                            onJump();
                            break;
                        case InputActions.NavigationBack:
                            onNavigateBack();
                            break;
                    }
                }

                // fire events when button held
                else if (_gamePadStateCurrent.IsButtonDown(buttonBinding.Value) && _gamePadStatePrevious.IsButtonDown(buttonBinding.Value))
                {
                    switch (buttonBinding.Key)
                    {
                        case InputActions.MovementLeft:
                            onMoveLeftHold();
                            break;

                        case InputActions.MovementRight:
                            onMoveRightHold();
                            break;

                        case InputActions.MovementJump:
                            onJumpHold();
                            break;
                    }
                }

                // fire events when button released
                else if (_gamePadStateCurrent.IsButtonUp(buttonBinding.Value) && _gamePadStatePrevious.IsButtonDown(buttonBinding.Value))
                {
                    switch (buttonBinding.Key)
                    {
                        case InputActions.MovementLeft:
                            onMoveLeftRelease();
                            break;

                        case InputActions.MovementRight:
                            onMoveRightRelease();
                            break;

                        case InputActions.MovementJump:
                            onJumpRelease();
                            break;
                    }
                }
            }
            _gamePadStatePrevious = _gamePadStateCurrent;
        }

        // update method to call in gameloop
        public void Update()
        {
            PollKeyboard();

            foreach (PlayerIndex playerIndex in _playerIndexes)
            {
                if (GamePad.GetState(playerIndex).IsConnected)
                {
                    PollGamePad(playerIndex);
                }
            }
        }

        // add plauyerIndex to list of gamepads to poll
        public void AddPlayerIndex(PlayerIndex playerIndex)
        {
            if (!_playerIndexes.Contains(playerIndex))
            {
                _playerIndexes.Add(playerIndex);
            }
        }

        // remove playerIndex from list of gamepads to poll
        public void RemovePlayerIndex(PlayerIndex playerIndex)
        {
            if (_playerIndexes.Contains(playerIndex))
            {
                _playerIndexes.Remove(playerIndex);
            }
        }

        // set default bindings
        // (future: add more bindings obviously; hardcoded defaults or move them to a default config file?)
        public void ResetBindings()
        {
            _keyboardBindings = new Dictionary<InputActions, Keys>() {
                { InputActions.MovementLeft, Keys.Left },
                { InputActions.MovementRight, Keys.Right },
                { InputActions.MovementJump, Keys.Space },
                { InputActions.NavigationBack, Keys.Escape }
            };

            _gamePadBindings = new Dictionary<InputActions, Buttons>() {
                { InputActions.MovementLeft, Buttons.DPadLeft },
                { InputActions.MovementRight, Buttons.DPadRight },
                { InputActions.MovementJump, Buttons.A },
                { InputActions.NavigationBack, Buttons.Y }
            };
        }

        public void ChangeBinding(InputActions inputAction, Keys key)
        {
            if (_keyboardBindings.ContainsKey(inputAction))
            {
                _keyboardBindings[inputAction] = key;
            }
        }

        public void ChangeBinding(InputActions inputAction, Buttons button)
        {
            if (_gamePadBindings.ContainsKey(inputAction))
            {
                _gamePadBindings[inputAction] = button;
            }
        }

        // various enumerated actions
        // (future: maybe seperate input actions for each gamestate into seperate enums?)
        enum InputActions
        {
            MovementLeft,
            MovementRight,
            MovementJump,
            NavigationBack
        }
    }

I'm using it by creating an instance of InputHandler in my Game1 class, calling InputHandler.Update() in the gameloop, passing it around and hooking up game entity movement methods to the InputHandler events.

Aside from the class itself, any ideas on how to improve the design in terms of where I should instanciate it?

Perhaps instanciate it in a StateHandler?

Or use seperate instances of InputHandler for each Player?

Or should I rather make the InputHandler a singleton? (I keep reading that singletons are evil )

Thanks in advance for taking the time to help a newbie out!

Cheers!

Edit:

Anyone know if there are any drawbacks to using events for inputs?

Does it scale well performancewise?

Do I need to worry about timing issues?

Any specific buffering techniques I could/should implement?

First of all, nice code

About your question, it depends on the game itself, really. If you're making a game which has a lot of objects moving at the same time (like a platformer, which I assume is the kind of game you're making, judging by your movement enum), then obviously you shouldn't make your movement handler class a singleton. If, however, your game is the kind of game where only one object would move in any given time, then I think singleton is good enough (and yes, there are games like this. Turn-based strategy for example).

The code itself is good. I don't think there'll be a huge hit performance-wise, unless you happen to have several hundreds of players playing at the same time. Do you let your game objects handle their own movements?

You're setting your 'previous' states at the end of the poll functions, meaning that effectively the previous and current state is the exact same at all times other than inside the poll functions themselves. Consider setting previous=current just before you grab the new current state. Then you can more easily support IsTriggered and IsReleased functions at any time.

Think about data-driven design. You have a list of hard-coded actions (the delegates/events). Input is not something that happens thousands of times per frame. It's totally safe to just use strings or some other dynamic identifier in place of an enum, and to keep a map or array of these to generic delegates rather than hard-coding a list of events.

You can map any input to a value in [-1,1]. For digital buttons, 0 is released and 1 is pressed (negative and fractional values are unused). For analog sticks or pressure-sensitive buttons, 0 is "within dead zone" and 1 is "pushed all the way" and -1 is "pushed the other way"
(unused for pressure-sensitive buttons, but useful for sticks). The stick might have two inputs states: Up/Forward and Right. Up/Forward is +1 when the stick is pushed up and -1 when it is pulled down. Right is +1 when pushed right and -1 when pushed left. You can allow digital buttons to map to axis with multipliers so e.g. the A key triggers 'Right' at -1 (so your gameplay code is only looking for a single type of horizontal movement input rather than two different ones).

You can use strings to denote the actions. The code just starts listening for "ThrowGrenade" and your config file just has "ThrowGrenade: G" and no changes to the core input system are needed. There's a simple map of strings to delegate(string, float) that lets game code handle the inputs however they want. The input manager itself should have no idea what "jump" actually is or that such a concept exists; let it handle mapping input to commands and let game code deal with interpreting commands into in-game actions.

You can also use input-config strings for matching key bindings and making those configurable. For instance, you might allow "Ctrl+G" (press Control and G at the same time) or "E,F" (press E or F). This is more handy when you consider Keyboard + Gamepad (or others) all being supported simultaneously. "Key:F,Gamepad:X,Mouse:0" (F key or X button or left-most mouse button).

Trueerror:

Thanks mate!

The purpose of the class is to handle player inputs only. For multiplayer-games I guess the AI could use the input handler aswell, atleast for actions that are shared between human player and AI controlled player. Something I havn't thought off until now, so thanks for bringing it up. As for movement/actions unique to AI, I planned to handle that elsewhere.

The handler will only ever handle the inputs for 4 players max, so I guess I won't have to worry about performance.

SeanMiddleditch:

In regards to the KeyboardStates/GamepadStates, I'm not sure why I should move the assignment of the old state, wouldn't it effectively be the exact same thing having it assigned in the end of the method as in the beginning?

The rest of your post gave me a lot to think about in regards to the design, especially in terms of abstraction and dynamic bindings.

Once I get some more free time I'll have a new look at the handler and try to implement your concept.

Thanks for taking the time guys, I really appreciate it!

I'm not sure why I should move the assignment of the old state, wouldn't it effectively be the exact same thing having it assigned in the end of the method as in the beginning?

//currState declared elsewhere
currState = getCurrStateFromInput();
prevState = currState;

After this code snippet, comparing prevState and currState will always have them be equal. Basically, prevState is a copy of currState, in this case.

This makes checking anything (e.g. if a button was pressed last check, but not this one --> button released) impossible.

vs

//currState declared elsewhere
prevState = currState;
currState = getCurrStateFromInput();

In this case, we first copy the old values over, then update the new ones. This ensures that we can query prevState vs currState to see if anything changed since last time this was run.

Lactose (and SeanMiddleditch):

perhaps i'm just slow, or totally just missing something, doesnt this effectively do the same thing (only with reversed declaration)?

// prevState = getStateFromInput(); in class constructor.

pollingMethod()

{

currState = getStateFromInput();

// polling code goes here

prevState = currState;

}

i mean, the polling _does_ work in my handler, i'm checking key down, key held and key released and they all invole their respective events as they should.

Like SeanMiddleditch said, it works right now because it's all done inside the polling function. Once you've called and run the the polling function to completiong, you lose all ability to check what the previous state was.

This is information you might not need, but if you either now or later on allow currState and prevState to be accessed outside of that single entry point, the prevState variable will not contain what it sounds like it contains. I.e. if you e.g. try to access the prevState variable in your update function, after you've polled keyboard and gamepad, prevState will always be equal to currState.

Ah yes, you're right. Was actually thinking of moving them to the update later on, so i'll change it according to what you guys suggested :)

Thanks for the added explaination.