Mouse and keyboard event system

Question

I wrote a mouse and keyboard event system in C++, using the GLFW library to get mouse and keyboard states.

I've been working on a 3D game engine of my own for a long time (about 9 months) and wanted to have a way to handle input independent of having to intertwine GLFW calls randomly in my code (thought this would be good design and would modularize input). This also uses an entity-component system designed by Austin Morlan and I like it a lot, it makes thinking about objects much easier, kind of like building blocks. I'm working on collision detection/response soon, but since I had the time I wondered how I did with this event system, since its mostly work of my own.

I'm not sure what design pattern this would be called (maybe inversion of control?), as I'm still an amateur in software engineering, but it uses callback functions and "managers" to subscribe to certain keys and the mouse in general, something I've seen vaguely while modding games.

Things I've noticed myself, but could use some clarification/justification/agreement:

• KeySubscription feels oddly redundant. Can this just be removed?
• The MouseState struct feels as though it is containing too much information and violates some principle of object oriented design. Maybe I could pass around 2 MouseState objects instead to show the currentMouseState and the previousMouseState? It would be annoying to pass around two states all the time, so maybe I could convert it to a Mouse (containing generic mouse data) and then contain a MouseState with two Mouse objects?
• Theres some globals around that I didn't want to pass around in every method that needs them, like mCamera (what if I want multiple cameras in the world, like with portals?), or gameWindow (multiple windows in the future?). How can I avoid this? Do I need a gigantic Engine class that keeps track of everything?

keyboard_manager.hpp

#include <glad/glad.h>
#include <GLFW/glfw3.h>
#include <memory>
#include <functional>
#include "key_subscription.hpp"

namespace Input {
    // Function pointer to a keyboard callback function.
    using key_callback_fn = std::function<void(const GLFWKey key, const bool state)>;

    class KeyboardManager {
    public:
        KeyboardManager();
        size_t subscribe(key_callback_fn Function, std::array<GLFWKey, MAX_KEYS> Keys, size_t keysLen);
        void unsubscribe(size_t indexToRemove);
        void callbackAll(GLFWwindow* handle);
    private:
        // Array of key callbacks.
        std::array<key_callback_fn, MAX_KEY_CALLBACKS> keyCallbacks{};

        // Map of callback functions to their key subscriptions.
        std::array<std::unique_ptr<KeySubscription>, MAX_KEY_CALLBACKS> callbackToSubscription{};
        size_t usedCallbacks{};
    };
}

keyboard_manager.cpp

#include "keyboard_manager.hpp"

using namespace Input;

KeyboardManager::KeyboardManager() : keyCallbacks(), usedCallbacks(0) {
}

size_t KeyboardManager::subscribe(key_callback_fn Function, std::array<GLFWKey, MAX_KEYS> Keys, size_t keysLen) {
    size_t newIndex = usedCallbacks;
    keyCallbacks[newIndex] = Function;
    callbackToSubscription[newIndex] = std::make_unique<KeySubscription>(Keys, keysLen);
    ++usedCallbacks;
    return newIndex;
}

void KeyboardManager::unsubscribe(size_t indexToRemove) {
    size_t lastIndex = usedCallbacks;
    keyCallbacks[indexToRemove] = std::move(keyCallbacks[lastIndex]);
    callbackToSubscription[indexToRemove] = std::move(callbackToSubscription[lastIndex]);
    callbackToSubscription[lastIndex].release();
    --usedCallbacks;
}

void KeyboardManager::callbackAll(GLFWwindow* handle) {
    for (size_t i = 0; i < usedCallbacks; i++) {
        const auto& Function = keyCallbacks[i];
        auto& sub = *callbackToSubscription[i].get();
        const auto& list = sub.getKeyList();
        for (size_t j = 0; j < sub.size(); j++) {
            const auto& key = list[j];
            const auto& state = glfwGetKey(handle, key);
            sub.setKeyState(key, state);
        }
        for (const auto& key_state : sub.getKeyStates()) {
            Function(key_state.first, key_state.second);
        }
    }
}

key_subscription.hpp

#pragma once
#include "types.hpp"
#include <GLFW/glfw3.h>
#include <array>
#include <algorithm>
#include <unordered_map>
#include <format>
#include <assert.h>

namespace Input {
    // Defines the maximum number of keys a KeySubscription can subscribe to.
    constexpr size_t MAX_KEYS = 8;

    // Defines the maximum number of callback functions that can be stored in the KeyboardManager.
    constexpr size_t MAX_KEY_CALLBACKS = 64;

    // Enumeration of the GLFW keys used in this program.
    enum GLFWKey {
        Space = GLFW_KEY_SPACE,
        A = GLFW_KEY_A,
        D = GLFW_KEY_D,
        F = GLFW_KEY_F,
        Q = GLFW_KEY_Q,
        S = GLFW_KEY_S,
        W = GLFW_KEY_W,
        Escape = GLFW_KEY_ESCAPE
    };

    class KeySubscription {
    public:
        KeySubscription();
        KeySubscription(std::array<GLFWKey, MAX_KEYS> _keysList, size_t _keySize) {
            keySize = _keySize;
            assert(keySize <= MAX_KEYS && std::format("Maximum number of keys is {}, provided {}", MAX_KEYS, keySize).c_str());

            std::copy(std::begin(_keysList), std::end(_keysList), std::begin(keysList));

            for (size_t i = 0; i < keySize; i++) {
                keyStates.insert({ keysList[i], GLFW_RELEASE });
            }
        }

        // Get the keys list.
        const std::array<GLFWKey, MAX_KEYS>& getKeyList() const {
            return keysList;
        }

        // Get the key states.
        std::unordered_map<GLFWKey, bool> getKeyStates() const {
            return keyStates;
        }

        // Sets the key state.
        void setKeyState(GLFWKey key, bool state);

        // Get the number of keys used.
        size_t size() const { return keySize; }
    private:
        // The list of keys subscribed.
        std::array<GLFWKey, MAX_KEYS> keysList{};

        // The key states.
        std::unordered_map<GLFWKey, bool> keyStates{};

        // The number of keys subscribed.
        size_t keySize;
    };
}

key_subscription.cpp

#include "key_subscription.hpp"

using namespace Input;

KeySubscription::KeySubscription() : keysList(), keyStates() {
    keySize = 0;
}

void KeySubscription::setKeyState(GLFWKey key, bool state) {
    keyStates.at(key) = state;
}

mouse_manager.hpp

#pragma once
#include <glad/glad.h>
#include <GLFW/glfw3.h>
#include <functional>
#include <memory>
#include <array>

namespace Input {
    // The mouse state.
    using MouseState =
        struct {
        double mouseXo;
        double mouseYo;
        double lastMouseX;
        double lastMouseY;
        double deltaX;
        double deltaY;
    };

    // Function pointer to a mouse callback function.
    using mouse_callback_fn = std::function<void(const MouseState& mouse)>;

    // Defines the maximum number of callback functions that can be stored in the MouseManager.
    constexpr size_t MAX_MOUSE_CALLBACKS = 64;

    class MouseManager {
    public:
        MouseManager();
        // Subscribe to the mouse manager with a function.
        void subscribe(mouse_callback_fn Function);

        void unsubscribe(mouse_callback_fn Function);

        // Updates the mouse state. Called automatically by glfw.
        void updateMouseState(double xpos, double ypos);
    private:
        // Array of mouse callback functions.
        std::array<std::shared_ptr<mouse_callback_fn>, MAX_MOUSE_CALLBACKS> mouseCallbacks{};
        size_t usedCallbacks{};

        // Map of mouse callback functions to their indices.
        std::unordered_map<std::shared_ptr<mouse_callback_fn>, size_t> mouseCallbackToIndex{};

        // The internal mouse state.
        MouseState mouse{};

        void callbackAll();
    };
}

mouse_manager.cpp

#include "mouse_manager.hpp"

using namespace Input;

MouseManager::MouseManager() : mouseCallbacks(), mouseCallbackToIndex(), usedCallbacks(0) {
    mouse = MouseState {};

}

void MouseManager::subscribe(mouse_callback_fn Function) {
    auto sharedFunction = std::make_shared<mouse_callback_fn>(Function);
    size_t newIndex = usedCallbacks;
    mouseCallbacks[newIndex] = sharedFunction;
    mouseCallbackToIndex.insert({ sharedFunction, newIndex });

    ++usedCallbacks;
}

void MouseManager::unsubscribe(mouse_callback_fn Function) {
    auto sharedFunction = std::make_shared<mouse_callback_fn>(Function);

    size_t indexToRemove = mouseCallbackToIndex[sharedFunction];
    size_t lastIndex = usedCallbacks;

    mouseCallbacks[indexToRemove] = mouseCallbacks[lastIndex];
    mouseCallbacks[lastIndex] = 0;

    mouseCallbackToIndex.erase(sharedFunction);
    
    --usedCallbacks;
}

void MouseManager::updateMouseState(double xpos, double ypos) {
    if (!mouse.lastMouseX || !mouse.lastMouseY) {
        mouse.lastMouseX = xpos;
        mouse.lastMouseY = ypos;
    }

    double xo = xpos - mouse.lastMouseX;
    double yo = mouse.lastMouseY - ypos;

    mouse.lastMouseX = xpos;
    mouse.lastMouseY = ypos;
    mouse.deltaX = xo;
    mouse.deltaY = yo;

    callbackAll();
}

void MouseManager::callbackAll() {
    for (size_t i = 0; i < usedCallbacks; i++) {
        const auto& Function = mouseCallbacks[i];
        (*Function.get())(mouse);
    }
}

window.hpp

#pragma once
#include <glad/glad.h>
#include <GLFW/glfw3.h>
#include <cstdint>
#include <string>
#include "input_manager.hpp"

#define BASIC_TITLE "Physics and rendering demo {}"

class Window {
private:
    GLFWwindow* handle;
    const uint16_t width;
    const uint16_t height;
    std::string title;

    Input::KeyboardManager keyboardManager;
    Input::MouseManager mouseManager;
public:
    Window(const uint16_t _width, const uint16_t _height, std::string _title);
    ~Window();

    GLFWwindow* getHandle();
    void updateTitle(std::string title);
    uint16_t getWidth();
    uint16_t getHeight();

    void updateKeyboard();

    Input::KeyboardManager& getKeyboardManager();
    Input::MouseManager& getMouseManager();
};

window.cpp

Window::Window(const uint16_t _width, const uint16_t _height, std::string _title) : width(_width), height(_height), title(_title) {
    
    // Unrelated window stuff...

    // Prepare our input managers.
    mouseManager = Input::MouseManager();
    keyboardManager = Input::KeyboardManager();
    glfwSetWindowUserPointer(handle, this);
    auto cursorPosCallback = [](GLFWwindow* handle, double xpos, double ypos) {
        static_cast<Window*>(glfwGetWindowUserPointer(handle))->mouseManager.updateMouseState(xpos, ypos);
    };
    glfwSetCursorPosCallback(handle, cursorPosCallback);
    // We callback the keyboard manager's callbacks later with our other stuff.
}

// Unrelated window stuff...

Input::KeyboardManager& Window::getKeyboardManager() {
    return keyboardManager;
}
Input::MouseManager& Window::getMouseManager() {
    return mouseManager;
}
void Window::updateKeyboard() {
    keyboardManager.callbackAll(handle);
}

main.cpp (consumer perspective)

// Global scoped window
std::unique_ptr<Window> gameWindow;

void main_input(Input::GLFWKey key, bool state) {
    using namespace Input;
    if (key == GLFWKey::Escape && state == GLFW_PRESS) {
        glfwSetWindowShouldClose(gameWindow->getHandle(), true);
    }
}

void start() {
    // Other stuff...

    gameWindow->getKeyboardManager().subscribe(&main_input, {Input::GLFWKey::Escape}, 1);
}

// ...

render_system.hpp

// Externs
extern Coordinator gCoordinator;
extern std::unique_ptr<Window> gameWindow;

using namespace Systems;

// Global camera for now
Entity mCamera;

void camera_movement(const Input::GLFWKey key, const bool state) {
    using namespace Input;
    auto& transform = gCoordinator.getComponent<Components::Transform>(mCamera);
    const auto& orientation = gCoordinator.getComponent<Components::Orientation>(mCamera);
    auto& camera = gCoordinator.getComponent<Components::Camera>(mCamera);
    static float speed = 0.1f;
    if (key == GLFWKey::W && state == GLFW_PRESS) {
        transform.Position += speed * orientation.Front;
    } else if (key == GLFWKey::A && state == GLFW_PRESS) {
        transform.Position -= speed * orientation.Right;
    } else if (key == GLFWKey::S && state == GLFW_PRESS) {
        transform.Position -= speed * orientation.Front;
    } else if (key == GLFWKey::D && state == GLFW_PRESS) {
        transform.Position += speed * orientation.Right;
    }
}

// FPS camera
void camera_look(const Input::MouseState& mouse) {
    static double sensitivity = 0.05f;
    auto& orientation = gCoordinator.getComponent<Components::Orientation>(mCamera);
    auto& camera = gCoordinator.getComponent<Components::Camera>(mCamera);
    double yawOff = mouse.deltaX * sensitivity;
    double pitchOff = mouse.deltaY * sensitivity;
    camera.Yaw += yawOff;
    camera.Pitch += pitchOff;
    if (camera.Pitch > 89.0f)
        camera.Pitch = 89.0f;
    if (camera.Pitch < -89.0f)
        camera.Pitch = -89.0f;

    camera.update(orientation);
}

void RenderSystem::init() {
    mCamera = gCoordinator.createEntity();
    gCoordinator.addComponent(mCamera,
        Components::Transform{
            .Position = glm::vec3(0.0f),
            .Rotation = glm::vec3(0.0f),
            .Scale = glm::vec3(0.0f)
        });
    gCoordinator.addComponent(mCamera,
        Components::Orientation{
            .Front = glm::vec3(0.0f, 0.0f, -1.0f),
            .Up = glm::vec3(0.0f, 1.0f, 0.0f),
            .Right = glm::vec3(1.0f, 0.0f, 0.0f)
        });
    gCoordinator.addComponent(mCamera,
        Components::Camera{
            .Projection = Components::Camera::createProjection(45.0f, 0.1f, 1000.0f, 1920.0f, 1080.0f),
            .Pitch = 0.0f,
            .Yaw = -90.0f
        });

    gameWindow->getKeyboardManager().subscribe(&camera_movement, {Input::GLFWKey::W, Input::GLFWKey::A, Input::GLFWKey::S, Input::GLFWKey::D }, 4);
    gameWindow->getMouseManager().subscribe(&camera_look);

    // Shader stuff...
}

Answer 1

Unsafe use of using namespace

Don't rely on using namespace Input to define class member functions of classes that were declared inside a namespace Input scope. Instead, use a namespace Input scope again:

#include "keyboard_manager.hpp"

namespace Input {

KeyboardManager::KeyBoardManager(): keyCallbacks(), usedCallbacks(0) {
}
…

}

This makes it explicit that you are defining that function inside namespace Input. If you merely use using namespace Input, then you are relying on name resolution finding the right namespace, which might not do the right thing if you have multiple namespaces that have a class KeyboardManager defined in them. Even if that's unlikely, it's better to avoid the possibility.

Unnecessary constructors

You didn't need to declare an explicit constructor for KeyboardManager; you already used default member initialization, so keyCallbacks and usedCallbacks get initialized even if you don't add a constructor. Just remove it. The same goes for MouseManager.

For KeySubscription you can also simplify the default constructor it if you add default member initialization for keySize:

class KeySubscription {
public:
    KeySubscription() = default;
    …
private:
    …
    size_t keySize{};
};

Here you cannot simply remove it because you have non-default constructor as well.

Use std::vector for variable sized arrays

I see you use std::array in combination with some variable to keep track of how many elements of the array are actually in use. In that case, I recommend using std::vector instead. It avoids having to worry about the maximum size of the array, and it will keep track of its size itself.

If you want to avoid memory allocations from happening while your game is running (which could affect your framerate), you can use reserve() to reserve memory for vectors.

Unnecessary use of std::unique_ptr and std::shared_ptr

You are using std::unique_ptr to store subscriptions in the array callbackToSubscription. However, you did not have to do this, you could store KeySubscriptions by value in the array. Of course, that might waste a lot of space, but that's because you were using std::array. That problem goes away if you also use std::vector:

class KeyboardManager {
    …
private:
    …
    std::vector<KeySubscription> callbackToSubscriptions;
};

This avoids all the hassle of std::make_unique(), std::move(), and keeping correct counts:

size_t KeyboardManager::subscribe(key_callback_fn Function, std::vector<GLFWKey> Keys) {
    …
    callbackToSubscription.push_back(Keys);
    …
}

You are also using std::shared_ptr in MouseManager. I guess it's to work around the fact that you can't use a std::function directly as a key for std::unordered_map. However, this doesn't do what you think it does. It won't cause the function itself to be compared, rather just a pointer to the std::function object referencing to the actual callback function. When you do this in MouseManager::unsubscribe():

auto sharedFunction = std::make_shared<mouse_callback_fn>(Function);

You are creating a brand new std::function object with its own address. Comparing sharedFunction with any other std::shared_ptr<mouse_callback_fn> in mouseCallbackToIndex will return false. So in:

size_t indexToRemove = mouseCallbackToIndex[sharedFunction];

On the right hand side, mouseCallbackToIndex[sharedFunction] will try to look up sharedFunction, finds that it doesn't exist, and will create a new entry in the std::unordered_map. The value of that entry will be zero. So indexToRemove will always be zero.

There is unfortunately no safe way to compare std::functions with each other. So instead you should do something like you did in KeyboardManager, where you let subscribe() return an index, and pass that index to unsubscribe(). However, that brings me to:

Your unsubscribe() functions are completely wrong

Apart from the problem with MouseManager::unsubscribe() which I mentioned above, there are also several problem with KeyboardManager::unsubscribe(). Let's start with lastIndex = usedCallbacks. Please note that usedCallbacks is the number of active elements in the arrays, however C++ starts indexing at zero. So the index of the last element is actually usedCallbacks - 1. At best you are thus accessing an unused element, at worst you are reading past the end of the array.

Second, while moving the last element into the place of an earlier erased element is a nice way to get an array without any holes, the problem is that you just invalidated the index of the last element. So if someone subscribes two functions, gets indexes 0 and 1. It then unsubscribes index 0, which means whatever was at index 1 gets moved into index 0. But it doesn't know that, so when it wants to unsubscribe the second function it will call unsubscribe(1), which doesn't do what you want.

A third issue is that you called release() on an element that you already move-assigned from in the previous line. This is unnecessary, and in general you should never access any values after you've std::move()d them, as their state might be undefined.

A typical solution to the second problem is to never shuffle entries around; this way you avoid invalidating indexes. However, that will create holes in your array or vector. You want to be able to reuse those efficiently, so you can have another vector keep track of the unused indexes in the other vectors.

Consider listening for events instead of polling state

Your code seems to be geared towards polling the state of all the keys you are interested in, every time callbackAll() is called. I assume that will be once per frame. However, most frames nothing will change, and if something changes it most likely be one or two keys at a time. However, with your approach you have to call glfwGetKey() for all keys you are interested in for every frame, and then you also call all registered callback functions every frame, regardless of any activity. All these function calls do have some cost.

Instead, consider having your KeyboardManager call glfwSetKeyCallback() so it will get notified when a key's state has changed. Then it can look up if any of the subscribers are interested in that key, and only then call the registered callback functions of those subscribers. A similar thing can be done with mouse events. Once a frame you have to call glfwPollEvents(), probably at the point where you called the callbackAll() functions before.

Answers to your questions

• KeySubscription feels oddly redundant. Can this just be removed?

Yes, I think you can just replace this with a std::vector<GLFWKey>.

• The MouseState struct feels as though it is containing too much information and violates some principle of object oriented design. Maybe I could pass around 2 MouseState objects instead to show the currentMouseState and the previousMouseState?

You could do that. But maybe some subscribers don't even care about the previous state? A subscriber can always store the previous state themselves if they are interested in that.

It would be annoying to pass around two states all the time, so maybe I could convert it to a Mouse (containing generic mouse data) and then contain a MouseState with two Mouse objects?

If you wanted to pass the current and previous states, that would indeed be a nice way of doing it.

Theres some globals around that I didn't want to pass around in every method that needs them, like mCamera (what if I want multiple cameras in the world, like with portals?), or gameWindow (multiple windows in the future?). How can I avoid this? Do I need a gigantic Engine class that keeps track of everything?

Putting everything in a class is a good way of doing it. That class doesn't necessarily have to be "gigantic", and even if it is, it's not worse than having that same gigantic amount of state as global variables.

Also, it doesn't all have to go into the same class. Only if you have several functions that need access to all the information in your program does it make sense to put everything into such a class. Also consider that some of the state can be moved into existing classes. For example, since the camera is mostly important to the rendering, maybe move mCamera into RenderSystem?