HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

C++ Keyboard Controller

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
keyboardcontrollerstackoverflow

Problem

I've written a Controller for the keyboard so that I can map certain keys to callbacks within a game. A key can only call one callback, but a callback can be called by many keys.

Currently, a Controller has two std::maps - one for when the key(s) is pressed/held, and one for when the key(s) are released. These maps are SDL_Scancode to std::string (Which I may change later to an enum rather than a string? But strings are easier to read so maybe not.) The Controller also has a ControllerContext object, which is just a wrapper around two std::maps of std::string to std::function: one map for callbacks to happen when a key is pressed, another for callbacks to happen when a key is released.

I feel like some kind of change could be made to make the code more readable, and I am unsure of if I should have the context stored in a shared_ptr, or if I should just keep a raw pointer within the Controller.

This is how it would work:

//An event union provided by SDL
//https://www.libsdl.org/release/SDL-1.2.15/docs/html/sdlevent.html
SDL_Event e;
//A controller
Controller controller_;
ControllerContext context_;

//Build the context
controller_.SetContext(&context);

//Poll all events in the event queue, feeding them to the structure e
while(SDL_PollEvent(&e))
{
   switch(e.type)
       case SDL_KEYDOWN:
           controller_.OnKeyPress(e.key);
           break;
       case SDL_KEYUP:
           controller_.OnKeyRelease(e.key);
           break;
       default: 
           break;
}


keyboardcontroller.h

```
#ifndef SHMUPPY_HEADER_KEYBOARDCONTROLLER_H_
#define SHMUPPY_HEADER_KEYBOARDCONTROLLER_H_

#include
#include
#include
#include
#include
#include

struct ControllerContext;

class Controller
{
public:
Controller();

int OnKeyPress(const SDL_Event &e);

int OnKeyRelease(const SDL_Event &e);

void SetContext(std::shared_ptr context_);

protected:
std::map keypress_action_map_;
std::map keyrelease_action_map_;
std::s

Solution

Edit: Tested implementations below.

As jason says, returning either 0 or 1, looks like boolean. Change int to bool and return true or false.

Drop the std::string layer. It doesn't add anything over enum. And enum are just as readable and there is no way typos can appear with enums.

I don't have any experience with SDL myself, but I've written a keyboard -> action controller in an attempt at a game.

I'd also like to drop the std::map in favor of a std::vector. The trick is to use continuous enums and use those as indices for the std::vector.

Something like this:

enum class KeyboardEvents : byte
{
    ON_RELEASE,
    ON_PRESS,

    ORDINAL
};

typedef std::function keyboardFunc_t;

std::vector keyboardEvents(KeyboardEvents::ORDINAL);

void bindControls()
{
    auto onReleaseLambda = [&]()
    {
        // Do stuff
    };
    auto onPressLambda = [&]()
    {
        // Do stuff
    };

    // Bind with lambdas
    keyboardEvents[KeyboardEvents::ON_RELEASE] = onReleaseLambda;
    keyboardEvents[KeyboardEvents::ON_PRESS] = onPressLambda;

    // Alternatively, use std::bind
    keyboardEvents[KeyboardEvents::ON_RELEASE] = std::bind(&ClassName::onRelease, classInstancePtr);
    keyboardEvents[KeyboardEvents::ON_PRESS] = std::bind(&ClassName::onPress, classInstancePtr);
}


I just wrote this on the top off my head. I'm not sure if it'll compile, but you get the idea.

I would also prefer lambdas in favor of std::bind.

As for storing std::shared_ptr controller_context_; in a raw or shared_ptr, I'd say neither. But DO use std::unique_ptr. Using a unique_ptr will make sure your controller class is not default copyable, which is a good thing when dealing with limited resources.

Like this std::unique_ptr controller_context_;

Edit: From here and on:

Note, don't look too closely on main.cpp, it was mainly copied from somewhere to get SDL2 running.

ControllerContext.h:

#ifndef SHMUPPY_HEADER_CONTROLLER_CONTEXT_H
#define SHMUPPY_HEADER_CONTROLLER_CONTEXT_H

#include 
#include 

struct ControllerContext
{

    ControllerContext()
        : keyPressEvents(SDL_NUM_SCANCODES)
        , keyReleaseEvents(SDL_NUM_SCANCODES)
    {
        keyPressEvents.resize(SDL_NUM_SCANCODES);
        keyReleaseEvents.resize(SDL_NUM_SCANCODES);
    }

    typedef std::function   KeyboardFunc_t;
    typedef std::vector KeyEvents;

    KeyEvents   keyPressEvents;
    KeyEvents   keyReleaseEvents;
};

typedef std::unique_ptr  ControllerContext_ptr;

#endif


keyboardcontroller.h:

#ifndef SHMUPPY_HEADER_KEYBOARDCONTROLLER_H_
#define SHMUPPY_HEADER_KEYBOARDCONTROLLER_H_

#include 
#include 
#include 
#include 
#include 
#include 

#include "ControllerContext.h"

class Controller
{
public:
    Controller();

    bool OnKeyPress(const SDL_Event &e);

    bool OnKeyRelease(const SDL_Event &e);

    // Clear by setting empty func
    void SetReleaseCallback(SDL_Scancode code, const ControllerContext::KeyboardFunc_t & func);
    void SetPressCallback(SDL_Scancode code, const ControllerContext::KeyboardFunc_t & func);

    // Clear both press and release
    void ClearCallback(SDL_Scancode code);

protected:
    ControllerContext_ptr controller_context_;
};

#endif


keyboardcontroller.cpp:

#include "KeyboardController.h"
#include "ControllerContext.h"

Controller::Controller()
    : controller_context_(new ControllerContext())
{
    //Some test initializations
    auto leftPress = [&]()
    {
        // Left press lambda
        int debug=0;
    };
    auto leftRelease = [&]()
    {
        // Left release lambda
        int debug=0;
    };

    SetPressCallback(SDL_SCANCODE_LEFT, leftPress);
    SetReleaseCallback(SDL_SCANCODE_LEFT, leftRelease);
}

bool Controller::OnKeyPress(const SDL_Event &e)
{
    const auto & scancode = e.key.keysym.scancode;

    const auto & pressFunc = controller_context_->keyPressEvents[scancode];

    if (pressFunc)
    {
        // Call callback if valid
        pressFunc();
        return true;
    }

    return false;
}

bool Controller::OnKeyRelease(const SDL_Event &e)
{
    const auto & scancode = e.key.keysym.scancode;

    const auto & pressFunc = controller_context_->keyReleaseEvents[scancode];

    if (pressFunc)
    {
        // Call callback if valid
        pressFunc();

        return true;
    }

    return false;
}

void Controller::SetReleaseCallback(SDL_Scancode code, const ControllerContext::KeyboardFunc_t & func)
{
    controller_context_->keyReleaseEvents[code] = func; 
}

void Controller::SetPressCallback(SDL_Scancode code, const ControllerContext::KeyboardFunc_t & func)
{
    controller_context_->keyPressEvents[code] = func;
}

void Controller::ClearCallback(SDL_Scancode code)
{
    controller_context_->keyPressEvents[code] = ControllerContext::KeyboardFunc_t();
    controller_context_->keyReleaseEvents[code] = ControllerContext::KeyboardFunc_t();
}


main.cpp:

```
#include
#include

class TestMain
{
public:

Code Snippets

enum class KeyboardEvents : byte
{
    ON_RELEASE,
    ON_PRESS,

    ORDINAL
};

typedef std::function<void(void)> keyboardFunc_t;

std::vector<keyboardFunc_t> keyboardEvents(KeyboardEvents::ORDINAL);

void bindControls()
{
    auto onReleaseLambda = [&]()
    {
        // Do stuff
    };
    auto onPressLambda = [&]()
    {
        // Do stuff
    };

    // Bind with lambdas
    keyboardEvents[KeyboardEvents::ON_RELEASE] = onReleaseLambda;
    keyboardEvents[KeyboardEvents::ON_PRESS] = onPressLambda;

    // Alternatively, use std::bind
    keyboardEvents[KeyboardEvents::ON_RELEASE] = std::bind(&ClassName::onRelease, classInstancePtr);
    keyboardEvents[KeyboardEvents::ON_PRESS] = std::bind(&ClassName::onPress, classInstancePtr);
}
#ifndef SHMUPPY_HEADER_CONTROLLER_CONTEXT_H
#define SHMUPPY_HEADER_CONTROLLER_CONTEXT_H


#include <vector>
#include <functional>

struct ControllerContext
{

    ControllerContext()
        : keyPressEvents(SDL_NUM_SCANCODES)
        , keyReleaseEvents(SDL_NUM_SCANCODES)
    {
        keyPressEvents.resize(SDL_NUM_SCANCODES);
        keyReleaseEvents.resize(SDL_NUM_SCANCODES);
    }

    typedef std::function<void(void)>   KeyboardFunc_t;
    typedef std::vector<KeyboardFunc_t> KeyEvents;

    KeyEvents   keyPressEvents;
    KeyEvents   keyReleaseEvents;
};


typedef std::unique_ptr<ControllerContext>  ControllerContext_ptr;

#endif
#ifndef SHMUPPY_HEADER_KEYBOARDCONTROLLER_H_
#define SHMUPPY_HEADER_KEYBOARDCONTROLLER_H_

#include <map>
#include <SDL.h>
#include <iostream>
#include <exception>
#include <string>
#include <memory>


#include "ControllerContext.h"



class Controller
{
public:
    Controller();

    bool OnKeyPress(const SDL_Event &e);

    bool OnKeyRelease(const SDL_Event &e);

    // Clear by setting empty func
    void SetReleaseCallback(SDL_Scancode code, const ControllerContext::KeyboardFunc_t & func);
    void SetPressCallback(SDL_Scancode code, const ControllerContext::KeyboardFunc_t & func);

    // Clear both press and release
    void ClearCallback(SDL_Scancode code);

protected:
    ControllerContext_ptr controller_context_;
};


#endif
#include "KeyboardController.h"
#include "ControllerContext.h"


Controller::Controller()
    : controller_context_(new ControllerContext())
{
    //Some test initializations
    auto leftPress = [&]()
    {
        // Left press lambda
        int debug=0;
    };
    auto leftRelease = [&]()
    {
        // Left release lambda
        int debug=0;
    };

    SetPressCallback(SDL_SCANCODE_LEFT, leftPress);
    SetReleaseCallback(SDL_SCANCODE_LEFT, leftRelease);
}

bool Controller::OnKeyPress(const SDL_Event &e)
{
    const auto & scancode = e.key.keysym.scancode;

    const auto & pressFunc = controller_context_->keyPressEvents[scancode];

    if (pressFunc)
    {
        // Call callback if valid
        pressFunc();
        return true;
    }

    return false;
}

bool Controller::OnKeyRelease(const SDL_Event &e)
{
    const auto & scancode = e.key.keysym.scancode;

    const auto & pressFunc = controller_context_->keyReleaseEvents[scancode];

    if (pressFunc)
    {
        // Call callback if valid
        pressFunc();

        return true;
    }

    return false;
}

void Controller::SetReleaseCallback(SDL_Scancode code, const ControllerContext::KeyboardFunc_t & func)
{
    controller_context_->keyReleaseEvents[code] = func; 
}

void Controller::SetPressCallback(SDL_Scancode code, const ControllerContext::KeyboardFunc_t & func)
{
    controller_context_->keyPressEvents[code] = func;
}

void Controller::ClearCallback(SDL_Scancode code)
{
    controller_context_->keyPressEvents[code] = ControllerContext::KeyboardFunc_t();
    controller_context_->keyReleaseEvents[code] = ControllerContext::KeyboardFunc_t();
}
#include <SDL.h>
#include <iostream>

class TestMain
{
    public:
        void handlePressW()
        {
            // W press
            int debug=0;
        }
        void handleReleaseW()
        {
            // W release
            int debug=0;
        }
};

#include "keyboardcontroller.h"

int _stdcall WinMain(struct HINSTANCE__ *hInstance, struct HINSTANCE__ *hPrevInstance, char *lpszCmdLine, int nCmdShow)
{
    if (SDL_Init(SDL_INIT_EVERYTHING) < 0)
    {
        /* Handle problem */
        fprintf(stderr, "%s\n", SDL_GetError());
    }
    SDL_Window* window = SDL_CreateWindow("Window caption", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, 852, 480, 0);
    if (window == NULL)
    {
        /* Handle problem */
        fprintf(stderr, "%s\n", SDL_GetError());
        SDL_Quit();
    }
    SDL_Renderer* renderer = SDL_CreateRenderer(window, -1, 0);
    if (renderer == NULL)
    {
        /* Handle problem */
        fprintf(stderr, "%s\n", SDL_GetError());
        SDL_Quit();
    }


    Controller keycontroller;

    // Bind testmain methods
    TestMain testmain;
    keycontroller.SetPressCallback(SDL_SCANCODE_W, std::bind(&TestMain::handlePressW, testmain));
    keycontroller.SetReleaseCallback(SDL_SCANCODE_W, std::bind(&TestMain::handleReleaseW, testmain));


    bool running = true;
    while (running)
    {
        /* Clear the buffer of color, setting it to black */
        SDL_RenderClear(renderer);

        /* Draw the buffer into the window */
        SDL_RenderPresent(renderer);

        /* Handle input and events: */
        SDL_Event sdl_event;
        while (SDL_PollEvent(&sdl_event) > 0)
        {
            switch (sdl_event.type)
            {
                case(SDL_KEYDOWN):
                {
                    keycontroller.OnKeyPress(sdl_event);
                    break;
                }

                case(SDL_KEYUP):
                {
                    keycontroller.OnKeyRelease(sdl_event);
                    break;
                }


                case(SDL_QUIT):
                    running = false;
                    break;
            }

        }

        /* Run your logic. For example: */
    }

    SDL_Quit();

    return 0;
}

Context

StackExchange Code Review Q#93488, answer score: 3

Revisions (0)

No revisions yet.