-
Notifications
You must be signed in to change notification settings - Fork 392
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Support for custom Key Callbacks in Input class #53
base: master
Are you sure you want to change the base?
Conversation
#include <glm/glm.hpp> | ||
|
||
namespace Walnut { | ||
|
||
class Input | ||
{ | ||
public: | ||
static void InitKeysCallBack(); | ||
|
||
static void SetKeyCallback(KeyCode keycode, std::function<void()> func); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest to pass the KeyCode
to the callback, so that one function could be used for multiple keys.
@TheCherno Is there anything holding this PR back? |
@coderabbitai review |
WalkthroughThe update introduces a new system for handling key events within the Walnut project. It establishes a method to set up key callbacks, allowing specific functions to be assigned to key events. This is achieved through the introduction of a key callback map, initialization and setting mechanisms for key callbacks, enhancing the project's interactivity and responsiveness to user input. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- Walnut/src/Walnut/Input/Input.cpp (1 hunks)
- Walnut/src/Walnut/Input/Input.h (1 hunks)
Additional comments not posted (6)
Walnut/src/Walnut/Input/Input.h (3)
14-14
: Consider documenting the purpose and usage ofInitKeysCallBack
.Adding comments explaining the function's role, especially how and when it should be called, would improve code maintainability and clarity for future developers.
16-16
: EnsureSetKeyCallback
is thread-safe if used in a multi-threaded context.Given that
s_KeyCallbackMap
is a static member, modifications to it viaSetKeyCallback
could lead to race conditions if accessed from multiple threads. Consider adding synchronization mechanisms if the application is multi-threaded.
26-26
: Validate the choice ofint
as the key type ins_KeyCallbackMap
.Using
KeyCode
directly as the key type fors_KeyCallbackMap
might enhance type safety and readability. IfKeyCode
is an enum, this change would make the map's purpose clearer and prevent misuse.Walnut/src/Walnut/Input/Input.cpp (3)
11-21
: Consider handling other key actions inKeyCallback
.Currently,
KeyCallback
only processesGLFW_PRESS
andGLFW_REPEAT
actions. If the application needs to handleGLFW_RELEASE
or other actions, consider extending this function. Documenting the decision can help maintain clarity.
23-27
: Verify thatglfwSetKeyCallback
is called once per window.Ensure
InitKeysCallBack
is called in a context where it's guaranteed thatglfwSetKeyCallback
is set only once per window, or implement logic to prevent multiple bindings which could lead to unexpected behavior.
29-32
: Ensure the key code is validated before adding tos_KeyCallbackMap
.Before inserting a callback into
s_KeyCallbackMap
, validate thekeycode
to ensure it represents a valid key. This can prevent potential issues with invalid keys and improve error handling.
Existing Input class only checks for IsKeyDown, which checks the key state via glfwGetKey. This doesn't support keyboard's keypress delay and repeat events ASAP.
This PR adds support for custom key press callbacks, so that we can have a callback for every event emitted for a key press, and repeats follows the user's keyboard settings.
Reference for callbacks
https://www.glfw.org/docs/3.3/input_guide.html#input_key
Summary by CodeRabbit