Skip to content

feat(input): Implement SDL3 input and window management#2639

Open
githubawn wants to merge 18 commits intoTheSuperHackers:mainfrom
githubawn:feature/sdl3-input-backport
Open

feat(input): Implement SDL3 input and window management#2639
githubawn wants to merge 18 commits intoTheSuperHackers:mainfrom
githubawn:feature/sdl3-input-backport

Conversation

@githubawn
Copy link
Copy Markdown

@githubawn githubawn commented Apr 20, 2026

SDL3 Input Backend
This PR implements an SDL3-based input and windowing backend as a modern alternative to DirectInput and Win32 window creation. By utilizing SDL3, we bypass DirectInput emulation layers on modern systems, providing a lower-latency pipeline for Windows 11 and Wine/Linux users.

Input handling
Unified event manager that centralizes keyboard, mouse, and gamepad events into a thread-safe buffer
Animated cursor support via .ani (RIFF) file loading.

Gamepad Support (v0.1)
This is an initial baseline implementation focused on providing functional out-of-the-box playability. Feedback is encouraged regarding the ergonomics and logic of these default mappings.

Input Action
Left Stick Virtual mouse cursor
Right Stick Camera pan (arrow keys)
LT (hold) Precision Mode
RT (hold) Force Attack (LCTRL)
LB Select All (Q)
RB Queue Orders (LSHIFT)
A Left click
B Right click
X Attack Move (A)
Y Stop (S)
D-Pad Control groups 1–4
Start Menu (ESC)
Back Last radar event (SPACE)
L3 Next idle worker
R3 Snap to command center

Scope Note: This implementation provides a hardcoded default layout to establish core functionality. Advanced features, such as input remapping, radial menus, adjustable deadzones, are currently out of scope for this PR and may be addressed in future iterations.

The foundation of this backend was built using the SDL3 input work from the generalsX fork by fbraz3.

Todo: replicate to generals

@githubawn githubawn force-pushed the feature/sdl3-input-backport branch from a785545 to 7cc0861 Compare April 20, 2026 15:00
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 20, 2026

Greptile Summary

This PR introduces an SDL3-based input and windowing backend as a modern alternative to DirectInput/Win32, with unified keyboard, mouse, and gamepad support (including a hardcoded default controller layout) and animated .ani cursor loading via SDL3's native animated cursor API.

Several bugs flagged in previous review rounds appear resolved in the current code: the m_SDLWindow compile error, missing windowID on synthetic gamepad events, SDL_WarpMouseInWindow(nullptr, ...) coordinate mismatch, the headless-mode GameEngine::init() skip, and the SDL3 init-failure fall-through. Remaining findings are P2 style/compliance issues.

Confidence Score: 5/5

Safe to merge; all previously flagged P0/P1 issues appear resolved in this revision, with only P2 style violations remaining.

Previously reported critical bugs (missing windowID on synthetic events, SDL_WarpMouseInWindow nullptr, headless GameEngine::init() skip, SDL_Init fall-through, m_SDLWindow compile error) are all fixed in the current diff. Remaining findings are style-rule violations (NULL vs nullptr, single-line if bodies) that do not affect correctness or safety.

GeneralsMD/Code/Main/WinMain.cpp has two NULL→nullptr style violations and a hardcoded window title. Core/GameEngineDevice/Source/SDL3Device/GameClient/SDL3Cursor.cpp has single-line if-body style violations.

Important Files Changed

Filename Overview
Core/GameEngineDevice/Source/SDL3Device/GameClient/SDL3Input.cpp Core SDL3 input manager, mouse, and keyboard implementation. Previously flagged issues (missing windowID on synthetic events, SDL_WarpMouseInWindow with nullptr) appear resolved in this diff. Ring-buffer sentinel logic using SDL_EVENT_FIRST=0 is correct. Minor: some single-line if-bodies violate project style rule.
Core/GameEngineDevice/Source/SDL3Device/GameClient/SDL3Cursor.cpp ANI/ICO cursor loader using SDL3 animated cursor API. Frame management is delegated entirely to SDL3's SDL_CreateAnimatedCursor, so the previously flagged m_frameCount OOB issue does not exist in this implementation. Several single-line if-bodies violate the project style rule.
Core/GameEngineDevice/Source/SDL3GameEngine.cpp SDL3GameEngine subclass with window/input lifecycle management. Previously flagged headless-mode early-return bug is fixed: GameEngine::init() is now always called. Text-input forwarding and minimized-loop handling look correct.
GeneralsMD/Code/Main/WinMain.cpp SDL3 window initialization wired into WinMain. SDL_Init failure now correctly returns early. Two NULL→nullptr style violations in the new SDL3 block; window title hardcodes "Generals" rather than "Zero Hour".
Core/GameEngineDevice/Include/SDL3Device/GameClient/SDL3Input.h Header for SDL3Mouse, SDL3Keyboard, and SDL3InputManager. Uses #pragma once, nullptr, and modern C++. m_window member correctly declared in SDL3InputManager, resolving the previously flagged compilation error.
cmake/sdl3.cmake SDL3/SDL3_image discovery with FetchContent fallback. SHA256 hashes are pinned for reproducibility. Static-build aliases and Windows system library linkage look correct.
GeneralsMD/Code/GameEngineDevice/Include/W3DDevice/GameClient/W3DGameClient.h SDL3/Win32 input backend switched via SAGE_USE_SDL3 preprocessor guard. createKeyboard and createMouse factory methods correctly conditionally instantiate SDL3 or DirectInput variants.
vcpkg.json sdl3 and sdl3-image added as unconditional top-level dependencies. They will be fetched even when SAGE_USE_SDL3=OFF. builtin-baseline removed and moved to vcpkg-configuration.json, which is valid.

Sequence Diagram

sequenceDiagram
    participant WinMain
    participant SDL3GameEngine
    participant SDL3InputManager
    participant SDL3Mouse
    participant SDL3Keyboard

    WinMain->>WinMain: SDL_Init()
    WinMain->>WinMain: SDL_CreateWindow() → TheSDL3Window
    WinMain->>SDL3GameEngine: CreateGameEngine()
    SDL3GameEngine->>SDL3InputManager: new SDL3InputManager(m_SDLWindow)
    SDL3InputManager->>SDL3InputManager: openFirstGamepad()

    loop Per Frame
        SDL3GameEngine->>SDL3InputManager: update()
        SDL3InputManager->>SDL3InputManager: SDL_PollEvent()
        SDL3InputManager->>SDL3InputManager: processGamepadInput()
        SDL3InputManager-->>SDL3Mouse: addMouseSDLEvent (via buffer)
        SDL3InputManager-->>SDL3Keyboard: addKeyboardSDLEvent (via buffer)
        SDL3Mouse->>SDL3InputManager: getNextMouseEvent()
        SDL3Mouse->>SDL3Mouse: translateEvent() → scaleMouseCoordinates()
        SDL3Keyboard->>SDL3InputManager: getNextKeyboardEvent()
        SDL3Keyboard->>SDL3Keyboard: translateScanCodeToKeyVal()
    end
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: GeneralsMD/Code/Main/WinMain.cpp
Line: 965

Comment:
**`NULL` used instead of `nullptr` in SDL3 block**

Two newly added SDL3 calls in this block use `NULL` rather than `nullptr`. Per the project's coding style, `nullptr` should be used for null pointer literals in C++ code:

- `SDL_GetPointerProperty(..., SDL_PROP_WINDOW_WIN32_HWND_POINTER, NULL)` — the default-value argument is a pointer, not an integer, so `nullptr` is appropriate.
- `SDL_BlitSurfaceScaled(gLoadScreenSurface, NULL, screen, &destRect, ...)` — the source-rect argument is `const SDL_Rect *`, so `nullptr` is appropriate here too.

**Rule Used:** Use nullptr instead of NULL for null pointer liter... ([source](https://app.greptile.com/review/custom-context?memory=c69cf1a9-c69a-4330-a013-69bb5d6701da))

**Learned From**
[TheSuperHackers/GeneralsGameCode#2067](https://github.com/TheSuperHackers/GeneralsGameCode/pull/2067#discussion_r2703746722)

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: Core/GameEngineDevice/Source/SDL3Device/GameClient/SDL3Cursor.cpp
Line: 496-497

Comment:
**If-body placed on same line as condition**

Several guard-clauses in the new SDL3 files place the statement body on the same line as the `if` condition, which conflicts with the project's formatting rule requiring the body on its own line to allow precise debugger breakpoint placement. Examples from this file:

```cpp
if (cursor < 0 || cursor >= Mouse::NUM_MOUSE_CURSORS) return nullptr;
if (direction < 0 || direction >= MAX_2D_CURSOR_DIRECTIONS) direction = 0;
```

Also seen in `SDL3Cursor.cpp` line 505 (`if (!mouse) return;`). The preferred form is:

```cpp
if (cursor < 0 || cursor >= Mouse::NUM_MOUSE_CURSORS)
    return nullptr;
```

This pattern appears throughout the new SDL3 source files and should be applied consistently.

**Rule Used:** Always place if/else/for/while statement bodies on... ([source](https://app.greptile.com/review/custom-context?memory=16b9b669-b823-49be-ba5b-2bd30ff3ba6d))

**Learned From**
[TheSuperHackers/GeneralsGameCode#2067](https://github.com/TheSuperHackers/GeneralsGameCode/pull/2067#discussion_r2706274626)

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: GeneralsMD/Code/Main/WinMain.cpp
Line: 944

Comment:
**Window title hardcodes "Generals" instead of "Zero Hour"**

The SDL window is created with the title `"Command & Conquer Generals"`, but this `WinMain.cpp` belongs to the `GeneralsMD` (Zero Hour) project. The title should distinguish the expansion, e.g. `"Command & Conquer Generals Zero Hour"`, consistent with the rest of the Zero Hour codebase conventions.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (21): Last reviewed commit: "dont early return for headless mode" | Re-trigger Greptile

Comment thread Core/GameEngineDevice/Source/SDL3Device/GameClient/SDL3Input.cpp Outdated
Comment thread Core/GameEngineDevice/Source/SDL3Device/GameClient/SDL3Input.cpp Outdated
Comment thread Core/GameEngineDevice/Source/SDL3Device/GameClient/SDL3Input.cpp Outdated
Comment thread Core/GameEngineDevice/Source/SDL3Device/GameClient/SDL3Input.cpp
Comment thread cmake/sdl3.cmake Outdated
Comment thread cmake/sdl3.cmake
OVERRIDE_FIND_PACKAGE
)

FetchContent_Declare(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we need SDL3_image? Looks like it's used to parse .ico for animated cursors? Maybe there is already code for that we can adapt

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

directinput lets windows directly handle the cursor rendering so new code and SDL3_image was needed

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case consider putting AnimatedCursor in its own file.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SDL3_image seem overkill for handling animated cursors though as @stephanmeesters says, its just been used to decode the ico format, SDL itself already provides support for the animated cursors once the files are decoded and loaded.

Copy link
Copy Markdown
Author

@githubawn githubawn Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved AnimatedCursor into its own file and switched to the SDL_CreateAnimatedCursor API.

Regarding the dependency: this is a conscious choice. While SDL core handles the animation logic, it doesn't decode ANI or ICO files. I’d rather offload that to SDL_image than maintain bespoke binary parsing. Using the library also makes it trivial for modders to use modern formats in the future.

I think this is the most maintainable path. Does that sound reasonable?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the dependency: this is a conscious choice. While SDL core handles the animation logic, it doesn't decode ANI or ICO files. I’d rather offload that to SDL_image than maintain bespoke binary parsing. Using the library also makes it trivial for modders to use modern formats in the future.

I think this is the most maintainable path. Does that sound reasonable?

Also @bobtista ’s #1785 could then use it for jpg and png encoding

Comment thread GeneralsMD/Code/Main/WinMain.cpp
Comment thread cmake/config-build.cmake
# Enable SDL3 by default for modern builds
if(NOT IS_VS6_BUILD)
option(SAGE_USE_SDL3 "Enable SDL3 input/window backend" ON)
if(SAGE_USE_SDL3)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe call it USE_SDL3

Comment thread cmake/config-build.cmake
@@ -9,6 +9,16 @@ option(RTS_BUILD_OPTION_ASAN "Build code with Address Sanitizer." OFF)
option(RTS_BUILD_OPTION_VC6_FULL_DEBUG "Build VC6 with full debug info." OFF)
option(RTS_BUILD_OPTION_FFMPEG "Enable FFmpeg support" OFF)

# Enable SDL3 by default for modern builds
if(NOT IS_VS6_BUILD)
option(SAGE_USE_SDL3 "Enable SDL3 input/window backend" ON)
Copy link
Copy Markdown

@stephanmeesters stephanmeesters Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Options should be on top of the file and be something like RTS_BUILD_OPTION_SDL3


namespace {

Bool DecodeNextUtf8Codepoint(const char* text, size_t length, size_t& offset, UnsignedInt& outCodepoint)
Copy link
Copy Markdown

@stephanmeesters stephanmeesters Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this, maybe better placed in UnicodeString? Or some other helper if it's generic stuff

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

I looked at #2045 and #2528 which are working on UTF-8 infrastructure.
If one of those lands first, forwardTextInputEvent could use the bulk Utf8_To_Utf16Le conversion and iterate the resulting wchar_t string directly, making this function unnecessary.

Happy to refactor once there's a clear winner between those two. For now it's self-contained here.

@githubawn githubawn force-pushed the feature/sdl3-input-backport branch from e365605 to 627ef23 Compare April 20, 2026 18:24
@githubawn githubawn changed the title feat(input): Implement SDL3 input backend feat(input): Implement SDL3 input and window management Apr 20, 2026
Consolidated all work from the test/sdl3-backport branch into a single atomic commit:
- Centralized input management via SDL3InputManager.
- Hardened Ani/RIFF cursor loading with robust bounds checking.
- Native Gamepad support with analogue stick-to-mouse emulation and custom RTS mappings.
- Modernized focus and capture handling for better stability during Alt-Tab.
- Standardized and secured string operations throughout the SDL3 path.
Consolidated all work from the test/sdl3-backport branch into a single atomic commit:
- Centralized input management via SDL3InputManager.
- Hardened Ani/RIFF cursor loading with robust bounds checking.
- Native Gamepad support with analogue stick-to-mouse emulation and custom RTS mappings.
- Modernized focus and capture handling for better stability during Alt-Tab.
- Standardized and secured string operations throughout the SDL3 path.
- Updated credit attribution (fbraz3).
@githubawn githubawn force-pushed the feature/sdl3-input-backport branch from eb9908a to 534e694 Compare April 21, 2026 01:17
Comment thread GeneralsMD/Code/Main/WinMain.cpp Outdated
Comment thread Core/GameEngineDevice/Source/SDL3Device/GameClient/SDL3Input.cpp Outdated
Comment thread Core/GameEngineDevice/Source/SDL3GameEngine.cpp Outdated
}
}

AnimatedCursor* SDL3CursorManager::loadANI(const char* filepath)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libsdl-org/SDL_image#730
Ran into a bug in SDL_image. Ignore SDL3CursorManager::loadANI if you are reviewing this.

@DevGeniusCode DevGeniusCode added this to the Linux support milestone Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants