Skip to content

feat(windows): implement SDL3 backend, DLL packaging, and deterministic replay CI - #348

Open
fbraz3 wants to merge 75 commits into
mainfrom
feat/windows-sdl3
Open

fbraz3 wants to merge 75 commits into
mainfrom
feat/windows-sdl3

Conversation

@fbraz3

@fbraz3 fbraz3 commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Description

Implements the modern SDL3 windowing and input backend on Windows (windows64-deploy preset), brings self-contained runtime DLL packaging, and integrates native Windows headless deterministic replay testing into CI, achieving full architectural parity across Linux, macOS, and Windows while strictly preserving retail VC6 / 32-bit compatibility.

Context & Motivation

Previously, modern windowing and input (SDL3 + DXVK + OpenAL) were active on Linux and macOS, whereas Windows builds remained tethered to legacy Win32/DirectInput stubs or VC6 32-bit builds. This PR brings native SDL3 windowing, mouse, keyboard, and self-contained packaging to Windows 64-bit builds while isolating modern code behind if(SAGE_USE_SDL3).

Upstream TheSuperHackers PR TheSuperHackers#2639 was reviewed as an informative reference and adapted to fit GeneralsX's cross-platform conventions.

Changes

1. Windowing & Display Backend

  • SDL3 Win32 Bridge: Extracted native Win32 HWND from SDL3 properties (SDL_PROP_WINDOW_WIN32_HWND_POINTER) in SDL3Main.cpp so DXVK / DirectX 8 binds properly to the window.
  • Window Management: Conditioned window visibility on SAGE_USE_SDL3 across all platforms in W3DDisplay.cpp.
  • Mode Switching: Added SWP_NOSENDCHANGING to SetWindowPos in dx8wrapper.cpp to prevent message reentrancy during display mode switches.
  • Busy State: Included Win32OSDisplay.cpp when targeting Windows with SDL3 to handle OSDisplaySetBusyState() without Linux stubs.

2. Input Subsystem

  • Cross-Platform Cursors & Mouse: Cleaned up #ifdef __linux__ in SDL3Mouse.cpp so ANI animated cursors, hotspot calculation, and surface boundary clamping work identically across Windows, Linux, and macOS.
  • Routing: Routed createKeyboard() and createMouse() to instantiate SDL3Keyboard and SDL3Mouse whenever SAGE_USE_SDL3 is active, maintaining legacy DirectInput as fallback when disabled.

3. Build & Packaging (Self-Contained Bundles)

  • Bundle Packaging: Updated deploy-windows-zh.ps1 and deploy-windows-generals.ps1 to detect and package all runtime DLLs:
    • SDL3.dll & SDL3_image.dll (natively built via FetchContent)
    • OpenAL32.dll (OpenAL Soft)
    • d3d8.dll, dxgi.dll, and d3d11.dll (DXVK runtime)
    • MinGW CRT runtimes (libgcc_s_seh-1.dll, libstdc++-6.dll, libwinpthread-1.dll)
    • Compression/image libraries (libpng16-16.dll, libzlib1.dll, zlib1.dll)
  • CMake & Build Scripts: Enabled SAGE_USE_SDL3 on windows64-deploy preset in CMakePresets.json. Enforced MinGW vcpkg triplets in build scripts to avoid manifest collisions during rebuilds.

4. Deterministic Replay Tests in CI

  • Deterministic Replay Workflow (replay-tests-windows.yml):
    • Added dedicated Windows replay testing workflow for the SDL3 MinGW x64 bundle.
    • Added Windows bundle preparation, asset extraction (generalszh.7z with ASSETS_KEY), and user data/map cache setup.
    • Added replay fallback logic (uses windows_*.rep if recorded, or linux_*.rep as cross-platform deterministic baseline).
    • Added diagnostic fingerprint logging (GAME_BIN_SHA256, INIZH.big, MapsZH.big, and replay checksums).
    • Added headless deterministic execution (.\GeneralsXZH.exe -headless -replay <rep>) with dummy video/audio drivers (SDL_VIDEODRIVER=dummy, SDL_AUDIODRIVER=dummy, DXVK_LOG_LEVEL=none), 180s timeout, error/CRC/MISMATCH regex diagnostics, and per-replay GitHub Step Summary reporting.
  • CI Pipeline Integration (ci.yml):
    • Added build-windows-sdl3 invocation via build-windows-sdl3.yml.
    • Added replay-test-windows-sdl3 job invoking replay-tests-windows.yml.
    • Added Windows SDL3 build and replay test statuses into ci-summary matrix and failure gates.

Validation

  • Successfully compiled and linked GeneralsXZH.exe (z_generals) with SDL3, OpenAL, and DXVK.
  • Successfully compiled and linked GeneralsX.exe (g_generals) with SDL3, OpenAL, and DXVK.
  • Verified deployed bundles in build/bundles/windows-generalsxzh-windows64-deploy and build/bundles/windows-generalsx-windows64-deploy containing all 12 necessary DLLs alongside executables.
  • Tested deterministic replay workflow integration across replay-tests-windows.yml, ci.yml, and build-windows-sdl3.yml.
  • Zero Hour and Generals base build paths both clean without regressions to Linux or macOS.
  • Updated monthly diary in docs/WORKLOG/2026-09-DIARY.md.

fbraz3 and others added 30 commits May 18, 2026 14:19
…election

Phase 2 - CMake Feature Gates:
- Gate Miles for legacy-only (cmake/miles.cmake)
- Gate Bink for legacy-only (cmake/bink.cmake)
- Modern Windows64 path uses SDL3, DXVK, OpenAL, FFmpeg
- Remove hard dependencies on 32-bit checks from modern path

Phase 3 - Entry Point and Engine Selection:
- Audit WinMain.cpp and SDL3Main.cpp entry points
- Define SAGE_USE_SDL3 feature flag for backend selection
- Modern path: SAGE_USE_SDL3=ON => SDL3-based engine
- Legacy path: SAGE_USE_SDL3=OFF => Win32-based engine
- Document Phase 2 and Phase 3 deliverables

See docs/WORKDIR/planning/PHASE2_WINDOWS64_CMAKE_FEATURE_GATES.md
See docs/WORKDIR/planning/PHASE3_WINDOWS64_ENTRY_POINT_ENGINE_SELECTION.md
…election

Phase 2 - CMake Feature Gates:
- Gate Miles for legacy-only (cmake/miles.cmake)
- Gate Bink for legacy-only (cmake/bink.cmake)
- Modern Windows64 path uses SDL3, DXVK, OpenAL, FFmpeg
- Remove hard dependencies on 32-bit checks from modern path

Phase 3 - Entry Point and Engine Selection:
- Audit WinMain.cpp and SDL3Main.cpp entry points
- Define SAGE_USE_SDL3 feature flag for backend selection
- Modern path: SAGE_USE_SDL3=ON => SDL3-based engine
- Legacy path: SAGE_USE_SDL3=OFF => Win32-based engine
- Document Phase 2 and Phase 3 deliverables

See docs/WORKDIR/planning/PHASE2_WINDOWS64_CMAKE_FEATURE_GATES.md
See docs/WORKDIR/planning/PHASE3_WINDOWS64_ENTRY_POINT_ENGINE_SELECTION.md
Phase 4 - DXVK Runtime on Windows:
- Audit existing DXVK integration via dxvk_adapter.h
- Define d3d8.dll bundling and loading strategy
- Gate SAGE_USE_DXVK already exists
- DXVK already integrated for cross-platform graphics
- Create Phase 4 documentation and session report

See docs/WORKDIR/planning/PHASE4_WINDOWS64_DXVK_RUNTIME.md
Guard legacy Miles/Bink linkage on the modern Windows64 path and\nselect SDL3GameEngine from WinMain when SAGE_USE_SDL3 is enabled.\n\nImplement DXVK Windows runtime staging in cmake/dx8.cmake by fetching\nthe DXVK release bundle and staging d3d8.dll, dxgi.dll, and d3d11.dll\nfor build/install outputs while keeping min-dx8-sdk for compile-time\nheaders/import libs.\n\nUpdate planning docs and May dev diary with the corrected phase status\nand runtime policy details.
Refactor the Windows64 execution strategy and task files to keep\nimplementation guidance phase-local.\n\nEmbed task references directly under each phase in the main strategy\ndocument and remove dependence on a separate child-task list.\n\nUpdate task files to execution-oriented wording, explicit outputs, and\nper-phase Implementation Reading (LLM) sections.
- ✅ OpenAL Soft v1.24.2 via FetchContent (WASAPI backend)
- ✅ FFmpeg cross-platform config (pkg-config Linux/macOS, kit Windows64)
- ✅ SAGE_USE_OPENAL=ON forçado no Windows (modern path)
- ✅ mingw.cmake inclui openal32 e FFmpeg libs
- ✅ CMakeLists.txt ativa as libs no build moderno
- ✅ Preset windows64-deploy com ambas as libs
- ✅ Documentação completa (task 05 + diary + planning)

- Next: Phase 6 - Legacy Windows Cull audit
- Target: GeneralsXZH first, backport shared libs to Generals
- docs/WORKDIR:
  - PLAN-2026-05-18_PHASE5_COMPLETION.md: checklist de conclusao Fase 5

- scripts/env:
  - setup-windows64.ps1: script de setup MSYS2+MinGW+FFmpeg

Fase 5 OpenAL+FFmpeg Windows64 configurado:
- OpenAL Soft v1.24.2 via FetchContent (cmake/openal.cmake)
- FFmpeg FindFFmpeg-Windows.cmake (FetchContent pattern)
- FFmpeg FindFFmpeg.cmake (cross-platform detection)
- CMakeLists.txt inclui ambas libs

Faltam:
- Instalar MSYS2
- Smoke test completo

Next: MSYS2 install -> setup script -> smoke test -> Phase 6
Advance Windows64 MinGW bring-up with incremental compatibility and build-graph fixes.

- harden WWVegas 64-bit compatibility paths (Except/registry/systimer/thread-id usage)

- remove Windows64 dependency on PkgConfig::FFMPEG in GameEngineDevice targets

- improve SDL3 zlib/libpng discovery for MSYS2 MinGW path

- update Windows64 strategy/task plans and May dev diary with current blocker state
…/timeGetTime issues

- MinGW-w64 x64 compatibility: disables x86-only code, fixes pointer/integer casts, adapts debug/exception handling for x64 CONTEXT, disables legacy FPU dump, fixes StackWalk64 function pointer types
- All timeGetTime usage replaced with GetTickCount macro for MinGW
- Fixes for socklen_t, HKL, CANDIDATELIST, HWND, and other Win32 types
- CompatLib headers forward to system SDK on MinGW
- CMake: winmm linkage for timeGetTime
- cmake/mingw.cmake: removes _int64/__int64 macro definitions
- This commit intentionally breaks VC6/MSVC6 compatibility (per user instruction)

See: #windows64-link-notes, #windows-mingw-setup

GeneralsX @build GitHub Copilot 20/05/2026 MinGW-w64 x64 cross-compile fixes.
…enAL, and platform stubs

- Corrige fallback de FFmpeg para MSYS2
- Remove dependências de d3d8/d3dx8d do caminho MinGW
- Adiciona stubs de FrameGrab e Bink para builds sem SDK proprietário
- Corrige link de ws2_32 para FTP/WinSock
- Substitui timeGetTime por GetTickCount no MinGW
- Ajusta fallback de GLI opcional
- Corrige headers e targets para Win32 input/teclado
- Não garante compatibilidade VC6 (quebrado por padrão)

[skip ci]
chore(sync): merge thesuperhackers main 2026-05-21
Enable real GitHub Actions Windows builds for Generals and Zero Hour\nwith configure/build/bundle/artifact stages using windows64-deploy.\n\nAlign repository docs/instructions/prompts with the active Windows64\nMinGW direction and remove outdated VC6-required wording for current\nworkflows and replay guidance.
- Remove .github/copilot-instructions.md (deleted in main)
- Update AGENTS.md and README.md from main
- Keep local development diary entries (Windows64 MinGW work)

Conflicts resolved in docs/DEV_BLOG/2026-05-DIARY.md
Co-authored-by: fbraz3 <10731570+fbraz3@users.noreply.github.com>
- Temporarily disable replay test jobs for determinism stabilization
- Add build fingerprint reporting to CI workflows
- Add replay CI provenance diagnostics for CRC mismatch triage
- Harden replay CI against host-specific map cache artifacts
- Sync merge from TheSuperHackers main with conflict resolution
- Windows64 MinGW CI pipeline enabled and documentation aligned
- Temporarily disable deterministic replay jobs in CI
- Add build fingerprint steps to CI build jobs
- Add replay CI provenance diagnostics for CRC mismatch triage
- Harden replay CI against host-specific map cache artifacts
- Sync merge from TheSuperHackers main
…imeEndPeriod

MinGW already includes these WinMM API declarations via <windows.h>, causing "ambiguating new declaration" errors. The functions are used elsewhere in the file and work correctly without the redundant extern declarations.

Fixes all 6 failing CI builds (Linux Flatpak, macOS, Windows) that were blocked by compilation errors in WW3D2/ww3d.cpp.

GeneralsX @BugFix 23/05/2026
GeneralsX Developer added 23 commits September 30, 2026 19:01
…nd Windows MinGW caching

- Remove accidental systimer.h include in GameCommon.h to fix MSVC x86 build
- Link VideoToolbox, CoreMedia, CoreVideo, and AudioToolbox frameworks with SWRESAMPLE in cmake/FindFFmpeg.cmake for macOS arm64
- Update setup-msys2 with msystem MINGW64 and inherit path-type, and expand MinGW candidate search paths in cmake/sdl3.cmake and configure-windows64.ps1
- Save vcpkg binary cache immediately after CMake configure in build-windows-sdl3.yml to prevent 30-minute recompilations
- Update monthly worklog diary
…cpkg caching

- Add standard Miles U32, S32, and F32 typedefs to Core/GameEngineDevice/Include/MilesAudioDevice/mss/mss.h to fix MSVC x86 build
- Export MINGW_PREFIX and MSYSTEM_PREFIX in build-windows-sdl3.yml using msys2 shell to make them available to pwsh steps
- Add -InstallDependenciesOnly switch to configure-windows64.ps1 and isolate vcpkg cache save before CMake configure
- Prioritize MINGW_PREFIX and RUNNER_TEMP msys2 paths in all Windows build, deploy, and toolchain scripts
- Update September 2026 worklog diary
…o signatures

- Correct parameter and return types in Core/GameEngineDevice/Include/MilesAudioDevice/mss/mss.h for AIL_stream_ms_position, AIL_set_3D_user_data, AIL_3D_user_data, AIL_sample_user_data, and AIL_set_sample_user_data to resolve MSVC x86 C2664 type errors
- Pin build-windows-sdl3.yml to runs-on windows-2022 matching other Windows workflows, avoiding the Visual Studio 2026 Preview / Windows SDK resource compiler cmcldeps wrapper bug
- Use cygpath -m to export clean Windows filesystem paths for MINGW_PREFIX and MSYSTEM_PREFIX
- Remove duplicate mingw cmake from setup-msys2 package list
- Update September 2026 worklog diary
Remove unused MSS_auto_cleanup declaration and AIL_startup macro override
from Core/GameEngineDevice/Include/MilesAudioDevice/mss/mss.h. This fixes
LNK2019 unresolved external symbol _MSS_auto_cleanup when linking
GeneralsX.exe and GeneralsXZH.exe on MSVC x86.
…ility

Change _int64 to __int64 in WWProfile_Get_Ticks parameter in wwprofile.cpp,
matching all class members (StartTime, ResetTime, Time, End) and callers.
Also add a fallback _int64 typedef under __MINGW32__ in wwprofile.h.
This resolves compiler error '_int64 was not declared in this scope' on
MinGW GCC builds for Windows 64-bit.
Remove duplicate static inline declarations of strlcpy and strlcat
in Download.cpp and FTP.cpp under __MINGW32__ that conflict with
declarations provided by Utility/stringex.h.
…atibility

Do not define __debugbreak as a function-like macro on _WIN32. On Windows,
MSVC and MinGW-w64 provide __debugbreak() natively via <intrin.h>, and
defining it as a zero-argument macro conflicts with function declarations
like extern void __cdecl __debugbreak(void); in SDL3/SDL_assert.h.
- Remove invalid direct <ws2ipdef.h> include from NetworkMesh.h,
  preventing undefined SCOPE_ID and SOCKET_ADDRESS errors in MinGW-w64
  and aligning with NGMP networking header guidelines.
- Move <filesystem> include under SYSTEM INCLUDES prior to USER INCLUDES
  in UserPreferences.cpp so standard library templates in <chrono> and
  <codecvt> are not polluted by GameSpy min/max macros.
Define socklen_t as int when _WIN32 is defined without restricting to
_MSC_VER, resolving undefined socklen_t compilation errors in UDP::Read,
GetInputBuffer, and GetOutputBuffer under MinGW-w64 GCC.
…3 are enabled

In 64-bit Windows SDL3 builds (windows64-deploy), MilesAudioManager.cpp
failed to compile under MinGW GCC due to 32-bit pointer-to-integer casts
on HSAMPLE and HSTREAM types. Furthermore, OpenAL and SDL3/BinkVideoPlayerStub
supersede Miles and Bink respectively for modern builds.

Guard MilesAudioManager under 'if(NOT SAGE_USE_OPENAL AND NOT SAGE_USE_MINIAUDIO)'
and BinkVideoPlayer under 'if(NOT SAGE_USE_SDL3)' in Core/GameEngineDevice/CMakeLists.txt,
preserving legacy MSVC x86 multimedia compilation while excluding them from
modern 64-bit builds.
…ol resolution

On Windows (PE/COFF), DLLs cannot have unresolved external symbols at link
time. Patches/SagePatch/CMakeLists.txt linked SDL3::SDL3 on UNIX but lacked
a WIN32 branch, causing undefined reference errors to SDL windowing and
display functions (such as SDL_GetWindowFromID, SDL_SetWindowMouseGrab,
SDL_SetWindowPosition) when linking libsage_patch.dll under MinGW.

Add an elseif(WIN32) branch linking SDL3::SDL3 to sage_patch to ensure all
symbols resolve during DLL linking.
…iscovery

- Remove invalid #include "always.h" from OpenALAudioStream.h
- Replace hardcoded MSYS2 path with dynamic MinGW prefix candidates for libavcodec in Core/GameEngineDevice/CMakeLists.txt
- Package MinGW FFmpeg runtime DLLs in Windows deployment scripts for Generals and Zero Hour
… headers

- Add mingw-w64-x86_64-ffmpeg to MSYS2 installation in build-windows-sdl3.yml
- Remove unused libavcodec and libavutil includes from OpenALAudioManager.cpp
- Resolve FFmpeg library paths using find_library and target_link_directories in Core/GameEngineDevice/CMakeLists.txt
…llback decoding

- Reintroduce libavcodec and libavutil headers in OpenALAudioManager.cpp required by setFrameCallback
- Update development worklog accordingly
…for self-contained Windows bundle

- Add recursive PE import dependency resolution to Windows deploy scripts
  (deploy-windows-zh.ps1, deploy-windows-generals.ps1) to resolve all
  transitive MinGW runtime libraries imported by FFmpeg and engine binaries.
- Copy libsage_patch.dll, libgamespy.dll, and all vcpkg dynamic runtime DLLs
  (libcurl-4.dll, GameNetworkingSockets.dll, libprotobuf.dll) into bundles.
- Assert presence of primary runtime DLLs and dependency libraries during
  bundle verification step in build-windows-sdl3.yml.
- Update 2026-09-DIARY.md worklog with root-cause analysis of 0xC0000135
  (STATUS_DLL_NOT_FOUND) loader crash and packaging improvements.
…configure cache

- Validate PE machine type (IMAGE_FILE_MACHINE_AMD64 = 0x8664) across all packaged and transitive DLLs in deploy-windows-zh.ps1 and deploy-windows-generals.ps1 to prevent STATUS_INVALID_IMAGE_FORMAT (0xC000007B) crashes during replay tests.
- Filter vcpkg dynamic runtime copying to release bin/ binaries and sweep bundle for non-x64 images.
- Add automated 64-bit architecture verification in build-windows-sdl3.yml verify step.
- Add CMake FetchContent caching in build-windows-sdl3.yml to prevent repeated dependency re-downloads.
- Remove redundant vcpkg install execution from configure-windows64.ps1 during CMake configure.
- Optimize ccache configuration with CCACHE_BASEDIR, CCACHE_SLOPPINESS, and content compiler checks.
- Add ccache statistics logging in build-windows-zh.ps1 and build-windows-generals.ps1.
- Update docs/WORKLOG/2026-10-DIARY.md.
# Conflicts:
#	docs/WORKLOG/2026-10-DIARY.md
…0000139

Include libssl-3-x64.dll and libcrypto-3-x64.dll from MinGW MSYS2
in core runtime DLLs across deploy scripts to avoid missing OpenSSL
QUIC entrypoints imported by libcurl-4 and ngtcp2.

Also remove unused vcpkg libcurl.dll and stray debug DLLs, and add
OpenSSL runtime DLLs to CI bundle validation.
MinGW-w64 GeneralsX console logging writes status diagnostics to
stderr, leaving standard output empty during headless replay execution.
Checking stdout exclusively caused completed replays to fail evaluation
with 'missing completed Game Time marker'.

Update replay-tests-windows.yml to search for completion markers across
combined console streams ($output), and refine error regex filtering.
…lism

Remove Cache CMake FetchContent step which spent ~6 minutes decompressing >40,000 files on NTFS while causing stale build trees.

Add pch_defines and include_file_ctime to CCACHE_SLOPPINESS so ccache properly caches GCC compilations utilizing precompiled headers.

Guard cmake/ccache.cmake to avoid overwriting external CCACHE_SLOPPINESS.

Normalize CCACHE_BASEDIR and CCACHE_DIR to forward slashes for MinGW.

Bump ccache key prefix to ccache-windows-sdl3-v2- for a clean cache baseline.

Uncap Ninja parallelism in build scripts and enable verbose ccache logging.

@fbraz3 fbraz3 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Review summary (CodeRabbit emulation, chill profile)

Note

The real CodeRabbit run on this PR was skipped ("Too many files": 335 changed files vs. a limit of 100). This is a manual emulation based on .coderabbit.yaml (path instructions, platform isolation, determinism, retail compatibility, CMake rules). references/**, assets/** and WIKI_PAGES/** were skipped per path_filters/scope.

Walkthrough — Enables the SDL3 + DXVK + OpenAL stack on Windows x64 (MinGW): SDL3Main HWND bridge, SDL3 mouse/keyboard wiring, MinGW compat shims, Win64 exception-handler variants, FFmpeg/SDL3/DXVK CMake plumbing, deploy scripts and Windows replay CI.

Findings

Severity Area Summary
🔴 Critical cmake/FindFFmpeg-Windows.cmake Placeholder SHA256=PENDING_HASH_CHECK + unused FetchContent; breaks configure when included
🟠 Major Except.cpp, debug_except.cpp 64-bit RIP truncated to 32 bits (unsigned long/unsigned) on Win64
🟠 Major Core/GameEngineDevice/CMakeLists.txt Win32DIKeyboardStub.cpp replaces the real DI keyboard on the legacy (non-SDL3) path and duplicates symbols with the ZH else() branch
🟠 Major cmake/dx8.cmake DXVK Windows runtime downloaded without URL_HASH; upstream DXVK vs. GeneralsX fork divergence; OR WIN32 affects legacy presets
🟡 Minor SDL3Main.cpp (ZH + Generals) No null check on extracted HWND
🟡 Minor FramePacer.cpp, W3DWater.cpp Timer resolution/GetTickCount regressions for Windows
🟡 Minor SlavedUpdate.cpp Unrelated GameLogic edit (determinism-sensitive)
🟡 Minor GeneralsMD/.../CMakeLists.txt MiniAudio guard dropped (OpenAL/MiniAudio parity)
🟡 Minor mss.h, types_compat.h, 2026-05-DIARY.md Attribution/upstream annotation, windows.h macro pollution, historical diary rewritten

Pre-merge checks

  • Title: ✅ Conventional Commits, no @.
  • Description: ⚠️ Mentions docs/WORKLOG/2026-09-DIARY.md, but diaries for 2026-05, 2026-09 and 2026-10 are all modified; the PR is also very large (335 files) — consider splitting (build tooling / compat shims / SDL3 wiring / CI / wiki).
  • Backport: ✅ Generals + ZH are changed in parallel; keep the HWND null-check and other fixes mirrored.

Positives

  • Legacy Win32 path kept behind SAGE_USE_SDL3 for engine/mouse/keyboard factories.
  • setenv/ExitProcess abstractions in SDL3Main are clean and platform-isolated.
  • Deterministic replay workflow on Windows is a valuable addition.

#auto-review

Comment thread Core/Libraries/Source/WWVegas/WWLib/Except.cpp Outdated
Comment thread Core/Libraries/Source/debug/debug_except.cpp Outdated
Comment thread Core/GameEngineDevice/CMakeLists.txt
Comment thread GeneralsMD/Code/GameEngineDevice/CMakeLists.txt Outdated
Comment thread cmake/FindFFmpeg-Windows.cmake Outdated
Comment thread Core/GameEngine/Source/Common/FramePacer.cpp Outdated
Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/Water/W3DWater.cpp Outdated
Comment thread GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/SlavedUpdate.cpp Outdated
Comment thread Core/GameEngineDevice/Include/MilesAudioDevice/mss/mss.h
Comment thread GeneralsMD/Code/CompatLib/Include/types_compat.h
…acy paths

Address CodeRabbit review feedback on PR #348:

- Revert float cast drop in SlavedUpdate.cpp to preserve GameLogic determinism.

- Add HWND validation and fail-fast handling in SDL3Main.cpp (GeneralsMD and Generals).

- Wrap windows.h with WIN32_LEAN_AND_MEAN and NOMINMAX in types_compat.h.

- Delete dead and unreferenced cmake/FindFFmpeg-Windows.cmake.

- Pin SHA256 checksum for dxvk_windows release archive in cmake/dx8.cmake.

- Restore 1ms timer resolution in FramePacer.cpp via winmm on all Windows targets.

- Restore high-precision timeGetTime in W3DWater.cpp (lines 1024, 2087).

- Gate Win32DIKeyboardStub behind RTS_BUILD_OPTION_ISOLATE_LEGACY_WININPUT to fix MSVC keyboard input and prevent duplicate symbols.

- Restore AND NOT SAGE_USE_MINIAUDIO guard in GeneralsMD GameEngineDevice CMakeLists.txt.

- Add upstream attribution annotation to MilesAudioDevice/mss/mss.h.
@fbraz3

fbraz3 commented Oct 3, 2026

Copy link
Copy Markdown
Owner Author

Re-review of 328e94c (follow-up fixes)

Verified: DXVK URL_HASH matches the real dxvk-2.6.tar.gz (sha256 0d762c33…7f01, contains x64/d3d8|dxgi|d3d11.dll); winmm is linked in cmake/mingw.cmake, so timeBeginPeriod/timeGetTime are safe on MinGW; SlavedUpdate.cpp is back to the main expression; HWND null-check present in both SDL3Main files.

10 of 12 threads resolved.

Still open

  • Except.cpp and debug_except.cpp: 64-bit RIP still truncated to 32 bits (see thread replies).
  • docs/WORKLOG/2026-05-DIARY.md: the 580-line rewrite of a closed month was never addressed (no inline thread since it was not anchorable).

Minor nits on the new commit

  • Core/GameEngineDevice/CMakeLists.txt: with RTS_BUILD_OPTION_ISOLATE_LEGACY_WININPUT=ON and SDL3 off, the stub is added while the else() branch in GeneralsMD/.../CMakeLists.txt still adds the real Win32DIKeyboard.cpp — same duplicate-symbol risk, only for that option combination.
  • dx8.cmake: @security is not an allowed annotation keyword (@bugfix/@feature/@performance/@refactor/@tweak/@build); use @build.
  • mss.h: the // Upstream reference lines sit inside a /** */ block, so they render as literal text; harmless.

Not ready to merge until the two RIP threads are resolved or explicitly deferred.

#auto-review

- Use 64-bit DbgHelp APIs and uintptr_t addresses to prevent 64-bit RIP truncation in Except.cpp and debug subsystem
- Gate Win32DIKeyboard behind NOT RTS_BUILD_OPTION_ISOLATE_LEGACY_WININPUT in GeneralsMD and Generals CMakeLists
- Standardize annotation keyword to @build in cmake/dx8.cmake
- Move upstream reference comment above Doxygen block in mss.h
- Restore docs/WORKLOG/2026-05-DIARY.md from origin/main to eliminate accidental diff on closed month
@fbraz3

fbraz3 commented Oct 3, 2026

Copy link
Copy Markdown
Owner Author

Resolution of Remaining Findings (a958d0e)

  1. 64-bit RIP Truncation:

    • Core/Libraries/Source/WWVegas/WWLib/Except.cpp: Dynamically binds and calls _SymGetSymFromAddr64 on _WIN64, using IMAGEHLP_SYMBOL64 and full 64-bit DWORD64 displacement/RIP addresses.
    • Core/Libraries/Source/debug/debug_stack.h, debug_stack.cpp, debug_stack.inl: Upgraded DebugStackwalk::Signature addresses and GetSymbol signatures to pointer-sized uintptr_t/DWORD64. Dynamically links _SymGetSymFromAddr64 and _SymGetLineFromAddr64 on x64 builds.
    • Core/Libraries/Source/debug/debug_except.cpp: Removed static_cast<unsigned>(ctx.Rip) truncations, passing static_cast<uintptr_t>(ctx.Rip) to GetSymbol.
  2. Diary Hygiene:

    • docs/WORKLOG/2026-05-DIARY.md: Restored from origin/main, reverting the accidental 580-line rewrite from a past closed month.
  3. Input Isolation Parity:

    • GeneralsMD/Code/GameEngineDevice/CMakeLists.txt & Generals/Code/GameEngineDevice/CMakeLists.txt: Guarded Win32DIKeyboard.cpp with if(NOT RTS_BUILD_OPTION_ISOLATE_LEGACY_WININPUT) when SDL3 is off, preventing duplicate symbols when Win32DIKeyboardStub.cpp is enabled.
  4. Annotation Compliance & Header Formatting:

    • cmake/dx8.cmake: Standardized @security annotation keyword to @build.
    • Core/GameEngineDevice/Include/MilesAudioDevice/mss/mss.h: Moved upstream reference attribution comment outside the Doxygen /** block.

@fbraz3

fbraz3 commented Oct 3, 2026

Copy link
Copy Markdown
Owner Author

[REVIEW BOT]

Re-review of commit a958d0e: Good to go! ✅

All remaining findings from previous review rounds have been verified and cleanly resolved:

  1. 64-bit RIP Truncation Resolved:

    • Core/Libraries/Source/WWVegas/WWLib/Except.cpp: Dynamically resolves and invokes _SymGetSymFromAddr64 on _WIN64 using DWORD64 displacement and IMAGEHLP_SYMBOL64.
    • Core/Libraries/Source/debug/debug_stack.h, debug_stack.cpp, debug_stack.inl: Upgraded address signatures from unsigned to uintptr_t, using SymGetSymFromAddr64 and SymGetLineFromAddr64 on 64-bit architectures.
    • Core/Libraries/Source/debug/debug_except.cpp: Passes static_cast<uintptr_t>(ctx.Rip) directly to GetSymbol.
  2. Diary Restored:

    • docs/WORKLOG/2026-05-DIARY.md: Fully reverted accidental changes; 2026-10 diary contains the current work notes.
  3. Input Isolation Parity:

    • GeneralsMD/Code/GameEngineDevice/CMakeLists.txt & Generals/Code/GameEngineDevice/CMakeLists.txt: Gated Win32DIKeyboard.cpp with if(NOT RTS_BUILD_OPTION_ISOLATE_LEGACY_WININPUT) when SAGE_USE_SDL3=OFF, eliminating symbol collision risks.
  4. Conventions & Comments:

    • Standardized @build keyword in cmake/dx8.cmake.
    • Repositioned upstream attribution comment in Core/GameEngineDevice/Include/MilesAudioDevice/mss/mss.h.

All 12 review threads are now resolved. Architecture, determinism, and platform isolation are preserved. Ready to merge!

#auto-review

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.

2 participants