Skip to content

refactor: Move WIN32_LEAN_AND_MEAN and NOMINMAX definitions to precompiled.h - #3393

Open
xezon wants to merge 2 commits into
TheSuperHackers:mainfrom
xezon:xezon/refactor-win32-lean-mean
Open

xezon wants to merge 2 commits into
TheSuperHackers:mainfrom
xezon:xezon/refactor-win32-lean-mean

Conversation

@xezon

@xezon xezon commented Oct 1, 2026

Copy link
Copy Markdown

This change moves the WIN32_LEAN_AND_MEAN and NOMINMAX definitions to precompiled.h and deletes the duplicates.

This way all targets (except for the maxsdk which needs to undefine it for its headers to work) inherit this setup automatically.

@xezon xezon added the Refactor Edits the code with insignificant behavior changes, is never user facing label Oct 1, 2026
@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 134f9ee5-8887-49e8-b211-e302b5e0ebec

📥 Commits

Reviewing files that changed from the base of the PR and between 7a32a6e and 7154d34.

📒 Files selected for processing (3)
  • Core/Tools/Autorun/Wnd_File.h
  • Core/Tools/Autorun/Wnd_file.cpp
  • Dependencies/Precompiled/Precompiled/precompiled.h
🚧 Files skipped from review as they are similar to previous changes (1)
  • Dependencies/Precompiled/Precompiled/precompiled.h

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


Walkthrough

The changes update Windows macro configuration and platform API includes across game code, tools, and dependencies. Networking sources now include Winsock declarations directly. Autorun message declarations and non-debug stubs also change.

Changes

Platform headers and Autorun declarations

Layer / File(s) Summary
Windows macro configuration
Core/GameEngine/..., Core/GameEngineDevice/..., Core/Libraries/..., Core/Tools/..., Dependencies/..., Generals/..., GeneralsMD/...
Many files no longer define WIN32_LEAN_AND_MEAN locally, and core_wwcommon no longer propagates it. The Windows precompiled header defines WIN32_LEAN_AND_MEAN and NOMINMAX when unset. BaseMacros.h removes its NOMINMAX definition, and max_undef.h undefines WIN32_LEAN_AND_MEAN.
Platform API and library includes
Core/GameEngine/Include/Common/crc.h, Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp, Core/Libraries/Source/WWVegas/WWLib/win.h, Core/Tools/Autorun/*, Core/Tools/Launcher/Toolkit/..., Core/Tools/PATCHGET/debug.cpp, Core/Tools/WW3D/max2w3d/dllmain.h, Generals/Code/Tools/GUIEdit/Source/*, GeneralsMD/Code/Tools/GUIEdit/Source/*
The changes add Shell API, common-dialog, standard-library, and multimedia includes. The DirectSound include moves after MilesLoader.h. The commented-out winsock2.h include is removed from crc.h.
Direct Winsock includes
Core/Tools/mangler/wnet/{field,packet}.cpp, Core/Tools/matchbot/wnet/{field,packet}.cpp
The networking sources include winsock.h directly and remove Win32_Winsock definitions. The non-Windows branch in the mangler field source changes its includes to windows.h and winsock.h.
Autorun message declarations and stubs
Core/Tools/Autorun/Wnd_File.h, Core/Tools/Autorun/Wnd_file.cpp
Msg declarations no longer specify __cdecl. Non-debug builds use inline no-op functions for Msg and Delete_Msg_File instead of macros. Disabled debug stubs are removed.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 7154d

No confirmed issue in the reviewed changes prevents merging after normal build checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 7154d

The examined changes do not show expanded privileges or newly enabled release-build logging. Risk remains low rather than minimal because the complete before-and-after build configuration could not be verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The examined diagnostic change remains within the existing local Windows process and its debug file/debugger outputs. The reviewed release stubs do not introduce a privileged sink or increase the removable-volume access requested by callers.

Trust Boundaries and Controls

  • observed — The examined volume-error diagnostics use literal format strings and pass the drive-derived character as formatting data. Debug-versus-release selection remains compile-time gating, not an authentication or authorization boundary.

Resilience and Maintainability Implications

  • inferred — The no-argument release Delete_Msg_File stub introduces no shared-state transition requiring atomicity, retry, ownership transfer, or recovery. The existing constructor retains its reset-before-message order; debug file lifecycle behavior is unchanged by the supplied implementation ranges.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: moving WIN32_LEAN_AND_MEAN and NOMINMAX definitions to precompiled.h and removing duplicates.
Description check ✅ Passed The description directly explains the macro relocation, duplicate removal, automatic inheritance, and the maxsdk exception.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Consolidates Windows preprocessor definitions to a shared header.

The PR appears safe to merge based on the changes reviewed.

Summary

The PR centralizes Windows header macros in the shared precompiled header and removes local definitions. It also adds explicit Windows includes where needed and changes Autorun’s release logging stubs. No actionable new issue was established.

Reviews (2) · Last reviewed commit: "Fix compile error in Autorun"

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
Dependencies/Precompiled/Precompiled/precompiled.h (1)

47-47: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Fix the misleading closing comment.

The #endif closes #ifdef _WIN32, but the comment says _MSC_VER. Change the comment to _WIN32.

Proposed fix
-#endif // _MSC_VER
+#endif // _WIN32

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: bb9e1ced-25f1-4670-9457-2833a6f6dd7e

📥 Commits

Reviewing files that changed from the base of the PR and between 1f27661 and 7a32a6e.

📒 Files selected for processing (51)
  • Core/GameEngine/Include/Common/crc.h
  • Core/GameEngine/Include/Precompiled/PreRTS.h
  • Core/GameEngine/Source/GameNetwork/LANAPI.cpp
  • Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp
  • Core/GameEngineDevice/Source/W3DDevice/GameClient/GUI/GUICallbacks/W3DMainMenu.cpp
  • Core/GameEngineDevice/Source/Win32Device/Common/Win32OSDisplay.cpp
  • Core/GameEngineDevice/Source/Win32Device/GameClient/Win32Mouse.cpp
  • Core/Libraries/Include/Precompiled/Precompiled/BaseMacros.h
  • Core/Libraries/Source/WWVegas/CMakeLists.txt
  • Core/Libraries/Source/WWVegas/WWLib/win.h
  • Core/Libraries/Source/debug/debug_dlg/debug_dlg.cpp
  • Core/Libraries/Source/debug/netserv/netserv.cpp
  • Core/Libraries/Source/debug/test2/StdAfx.h
  • Core/Tools/Autorun/ViewHTML.cpp
  • Core/Tools/Autorun/autorun.cpp
  • Core/Tools/Launcher/Toolkit/Debug/DebugPrint.cpp
  • Core/Tools/Launcher/Toolkit/Storage/File.cpp
  • Core/Tools/Launcher/winblows.cpp
  • Core/Tools/Launcher/winblows.h
  • Core/Tools/PATCHGET/WINBLOWS.cpp
  • Core/Tools/PATCHGET/WINBLOWS.h
  • Core/Tools/PATCHGET/debug.cpp
  • Core/Tools/PATCHGET/registry.cpp
  • Core/Tools/WW3D/max2w3d/dllmain.h
  • Core/Tools/WW3D/pluglib/win.h
  • Core/Tools/buildVersionUpdate/buildVersionUpdate.cpp
  • Core/Tools/mangler/wnet/field.cpp
  • Core/Tools/mangler/wnet/packet.cpp
  • Core/Tools/matchbot/wnet/field.cpp
  • Core/Tools/matchbot/wnet/packet.cpp
  • Core/Tools/textureCompress/textureCompress.cpp
  • Core/Tools/timingTest/StdAfx.h
  • Core/Tools/timingTest/timingTest.cpp
  • Core/Tools/versionUpdate/versionUpdate.cpp
  • Core/Tools/wolSetup/StdAfx.h
  • Core/Tools/wolSetup/verchk.h
  • Core/Tools/wolSetup/wolSetup.h
  • Dependencies/Bink/BinkLoader.cpp
  • Dependencies/DbgHelp/imagehlp_adapter.h
  • Dependencies/DbgHelp/minidump_subset.h
  • Dependencies/MaxSDK/max_undef.h
  • Dependencies/Precompiled/Precompiled/precompiled.h
  • Dependencies/Usp10/usp10_adapter.h
  • Dependencies/Usp10/usp10_subset.h
  • Dependencies/Utility/Utility/lazy_static.h
  • Generals/Code/Main/WinMain.cpp
  • Generals/Code/Tools/GUIEdit/Source/GUIEdit.cpp
  • Generals/Code/Tools/GUIEdit/Source/LayoutScheme.cpp
  • GeneralsMD/Code/Main/WinMain.cpp
  • GeneralsMD/Code/Tools/GUIEdit/Source/GUIEdit.cpp
  • GeneralsMD/Code/Tools/GUIEdit/Source/LayoutScheme.cpp
💤 Files with no reviewable changes (32)
  • Core/GameEngineDevice/Source/W3DDevice/GameClient/GUI/GUICallbacks/W3DMainMenu.cpp
  • Core/GameEngineDevice/Source/Win32Device/GameClient/Win32Mouse.cpp
  • Core/Tools/Launcher/winblows.cpp
  • GeneralsMD/Code/Main/WinMain.cpp
  • Core/Tools/PATCHGET/WINBLOWS.cpp
  • Dependencies/Bink/BinkLoader.cpp
  • Core/Tools/versionUpdate/versionUpdate.cpp
  • Dependencies/DbgHelp/minidump_subset.h
  • Core/Libraries/Source/debug/debug_dlg/debug_dlg.cpp
  • Core/GameEngine/Source/GameNetwork/LANAPI.cpp
  • Core/Tools/textureCompress/textureCompress.cpp
  • Core/Libraries/Source/debug/netserv/netserv.cpp
  • Core/Tools/timingTest/StdAfx.h
  • Dependencies/DbgHelp/imagehlp_adapter.h
  • Generals/Code/Main/WinMain.cpp
  • Core/Tools/wolSetup/wolSetup.h
  • Core/Tools/wolSetup/StdAfx.h
  • Core/Libraries/Source/debug/test2/StdAfx.h
  • Core/Tools/PATCHGET/WINBLOWS.h
  • Core/Tools/WW3D/pluglib/win.h
  • Core/Libraries/Source/WWVegas/CMakeLists.txt
  • Dependencies/Usp10/usp10_adapter.h
  • Core/GameEngineDevice/Source/Win32Device/Common/Win32OSDisplay.cpp
  • Core/Libraries/Include/Precompiled/Precompiled/BaseMacros.h
  • Core/Tools/PATCHGET/registry.cpp
  • Dependencies/Utility/Utility/lazy_static.h
  • Core/Tools/buildVersionUpdate/buildVersionUpdate.cpp
  • Core/Tools/wolSetup/verchk.h
  • Core/Tools/timingTest/timingTest.cpp
  • Dependencies/Usp10/usp10_subset.h
  • Core/GameEngine/Include/Common/crc.h
  • Core/Tools/Launcher/winblows.h

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Refactor Edits the code with insignificant behavior changes, is never user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant