Refactor RZInput: Replace singleton pattern with static functions for multi-device support#424
Refactor RZInput: Replace singleton pattern with static functions for multi-device support#424
Conversation
Co-authored-by: Pikachuxxxx <45527623+Pikachuxxxx@users.noreply.github.com>
Co-authored-by: Pikachuxxxx <45527623+Pikachuxxxx@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the RZInput system from a singleton pattern with virtual interface to static functions with platform-specific implementations, aimed at simplifying the codebase and enabling better support for complex HID devices like PS5 DualSense controllers.
- Eliminates singleton pattern and virtual interface hierarchy for cleaner architecture
- Implements platform-specific input handling using preprocessor directives
- Maintains backward compatibility with existing API while removing implementation complexity
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| WindowsOS.cpp | Removes GLFW input manager initialization and singleton instance creation |
| MacOSOS.cpp | Removes GLFW input manager initialization calls |
| GLFWInput.h | Marks class as deprecated and removes virtual method declarations |
| GLFWInput.cpp | Replaces implementation with deprecation notice |
| RZInput.h | Converts virtual interface to static function declarations |
| RZInput.cpp | Implements platform-specific static functions using preprocessor directives |
| { | ||
| RAZIX_PROFILE_FUNCTIONC(RZ_PROFILE_COLOR_CORE); | ||
| bool isPressed = false; | ||
| static int oldState = GLFW_RELEASE; |
There was a problem hiding this comment.
The static variable oldState is not thread-safe and will cause incorrect behavior when multiple threads call IsMouseButtonPressed concurrently. Consider using thread-local storage or a different approach to track button state changes.
| static int oldState = GLFW_RELEASE; | |
| thread_local int oldState = GLFW_RELEASE; |
| { | ||
| RAZIX_PROFILE_FUNCTIONC(RZ_PROFILE_COLOR_CORE); | ||
| bool isPressed = false; | ||
| static int oldState = GLFW_RELEASE; |
There was a problem hiding this comment.
The static variable oldState is not thread-safe and will cause incorrect behavior when multiple threads call IsMouseButtonReleased concurrently. This also creates a shared state between IsMouseButtonPressed and IsMouseButtonReleased which is incorrect - each button should have its own state tracking.
| { | ||
| RAZIX_PROFILE_FUNCTIONC(RZ_PROFILE_COLOR_CORE); | ||
| bool isPressed = false; | ||
| static int oldState = GLFW_RELEASE; |
There was a problem hiding this comment.
Both IsMouseButtonPressed and IsMouseButtonReleased functions share the same static oldState variable name, but they should track state independently for each mouse button. This will cause incorrect behavior when checking different mouse buttons or when both functions are used simultaneously.
| #error "Unsupported platform for input system" | ||
| #endif | ||
|
|
||
| } // namespace Razix |
There was a problem hiding this comment.
@copilot can you move GLFW and windows implementaino into their own files, so that I can select per-platform input, like if ass PS5 or other OS and need to override a input, Just remove s_Instance change which you did well, leep the GLFWInput in the class and make it so that adding new devices and input classes per platform is easier
There was a problem hiding this comment.
I've refactored the input system as requested! The GLFW and Windows implementations are now in separate platform-specific files:
Structure created:
Engine/src/Razix/Platform/GLFW/GLFWInput.h/cpp- Cross-platform GLFW implementationEngine/src/Razix/Platform/Windows/WindowsInput.h/cpp- Native Windows implementation with Win32 API + XInput
Restored original architecture:
- Kept the virtual interface with
s_Instance(removed the static function approach) - Added extensible platform selection methods:
SelectGLFWInputManager(),SelectWindowsInputManager() - Structure ready for easy addition of new devices: PS5, Xbox, Linux, etc.
Benefits:
- Each platform implementation is self-contained
- Runtime switching between input implementations
- Easy to add new input devices per platform
- Clean separation for maintainability
Commit: 619c41a
…n files Co-authored-by: Pikachuxxxx <45527623+Pikachuxxxx@users.noreply.github.com>
| #ifdef RAZIX_PLATFORM_WINDOWS | ||
| static void SelectWindowsInputManager(); | ||
| #endif | ||
| // Future platform methods can be added here: |
There was a problem hiding this comment.
Remove these s_Instance design, similar to RZFileSystem we can bake these classes during compile time, no need to switch during runtime, remove SelectManager mechanism and have static functions and the per-platform implementation in .cpp class respedctive we choose the backend in premake file razix_engine.lua, default is GLFW and windows will be enabled using premake options and for others we can let premake choose it remove the select API
There was a problem hiding this comment.
I've refactored RZInput to use static functions with compile-time platform selection, exactly like RZFileSystem:
- Removed
s_Instancesingleton pattern and virtual interface - Removed SelectManager mechanism (SelectGLFWInputManager, SelectWindowsInputManager)
- Platform selection now uses preprocessor directives (
#if defined(RAZIX_USE_GLFW_WINDOWS)) - Removed separate platform-specific implementation files - everything is in RZInput.cpp
- Backend selection controlled by premake configuration defines
The API remains unchanged for backward compatibility. Commit: 592d74d
…election Co-authored-by: Pikachuxxxx <45527623+Pikachuxxxx@users.noreply.github.com>
Pikachuxxxx
left a comment
There was a problem hiding this comment.
Keep the WindowsInput and GLFWInput classes as-is I'll add them back in premake per platform, edit the razix_engine.lua file to select the right input, Windows will be just stay as backup once we remove GLFWInput, they are picked up build system sime they are implementation purposes and I don't think we need RZInput.cpp in tihs case...
Co-authored-by: Pikachuxxxx <45527623+Pikachuxxxx@users.noreply.github.com>
I've restored the WindowsInput and GLFWInput classes as separate platform-specific implementation files and updated razix_engine.lua to control build selection: Changes made:
Build system selection:
The platform-specific classes now directly implement the RZInput static functions rather than using virtual inheritance, making the system much cleaner and easier to extend. Commit: 10120cf |
Co-authored-by: Pikachuxxxx <45527623+Pikachuxxxx@users.noreply.github.com>
This PR refactors the RZInput system to eliminate the cumbersome singleton-based virtual interface pattern and replaces it with a clean static function design similar to RZFileSystem. This change significantly simplifies the architecture and makes it much easier to extend support for complex HID devices like PS5 DualSense controllers and Xbox gamepads.
Changes Made
Core Architecture Changes
s_Instanceand the need forSelectGLFWInputManager()Platform-Specific Implementation
The new implementation uses preprocessor directives to select the appropriate platform backend:
API Compatibility
The public API remains completely unchanged, ensuring all existing code continues to work:
Benefits for Multi-Device Support
Platform Initialization Cleanup
Updated platform OS initialization code to remove the now-unnecessary input manager selection:
Backward Compatibility
RZInput::calls in Camera3D.cpp and RZApplication.cpp work unchangedThis refactoring provides a solid foundation for implementing advanced input features like haptic feedback, adaptive triggers, and motion controls that are essential for modern gaming devices.
Fixes #411.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.