WIP: Fix all warnings

.github/workflows/cibuild.yml

@@ -32,6 +32,11 @@ jobs:
       image: 'registry.gitlab.steamos.cloud/steamrt/sniper/sdk:latest'
 
     steps:
+      # Silence a git warning emitted from actions/checkout.
+      # For context: https://git-scm.com/docs/git-init#Documentation/git-init.txt---initial-branchbranch-name
+    - name: Set initial default branch name
+      run: git config --global init.defaultBranch master
+
     - uses: actions/checkout@v4
       with:
         fetch-depth: 0
@@ -111,31 +116,36 @@ jobs:
       with:
         name: ${{ env.libraries }}
         path: ${{ env.install_dir }}/${{ env.libraries }}
+        if-no-files-found: error
 
     - name: Upload libraries debug information
       uses: actions/upload-artifact@v4
       with:
         name: ${{ env.libraries_debuginfo }}
         path: ${{ env.install_dir }}/${{ env.libraries_debuginfo }}
+        if-no-files-found: error
 
     - name: Upload dedicated library
       uses: actions/upload-artifact@v4
       with:
         name: ${{ env.dedicated }}
         path: ${{ env.install_dir }}/${{ env.dedicated }}
+        if-no-files-found: error
 
     - name: Upload dedicated library debug information
       uses: actions/upload-artifact@v4
       with:
         name: ${{ env.dedicated_debuginfo }}
         path: ${{ env.install_dir }}/${{ env.dedicated_debuginfo }}
+        if-no-files-found: error
 
     - name: Upload SourceMod gamedata
       if: ${{ matrix.preset_build_type.name == 'release' }}
       uses: actions/upload-artifact@v4
       with:
         name: ${{ env.gamedata }}
         path: ${{ env.install_dir }}/${{ env.gamedata }}
+        if-no-files-found: error
 
   windows-build:
     name: ${{ matrix.platform.display_name }} ${{ matrix.preset_build_type.display_name }}
@@ -151,7 +161,7 @@ jobs:
 
       matrix:
         platform:
-        - { display_name: 'Windows Native Libraries', os: windows-latest, preset_os: windows }
+        - { display_name: 'Windows Native Libraries', os: windows-2025, preset_os: windows }
 
         preset_build_type:
         - { display_name: 'Debug', name: debug }
@@ -174,10 +184,6 @@ jobs:
         echo "install_dir=$install_dir" >> "$GITHUB_ENV"
         echo "preset_name=$preset_name" >> "$GITHUB_ENV"
 
-
-    - name: Install packages
-      run: choco install -y ninja
-
     - uses: ilammy/msvc-dev-cmd@v1
       with:
         arch: x64
@@ -244,29 +250,33 @@ jobs:
       with:
         name: ${{ env.libraries }}
         path: ${{ env.install_dir }}/${{ env.libraries }}
+        if-no-files-found: error
 
     - name: Upload libraries debug information
       uses: actions/upload-artifact@v4
       with:
         name: ${{ env.libraries_debuginfo }}
         path: ${{ env.install_dir }}/${{ env.libraries_debuginfo }}
+        if-no-files-found: error
 
     - name: Upload dedicated library
       uses: actions/upload-artifact@v4
       with:
         name: ${{ env.dedicated }}
         path: ${{ env.install_dir }}/${{ env.dedicated }}
+        if-no-files-found: error
 
     - name: Upload dedicated library debug information
       uses: actions/upload-artifact@v4
       with:
         name: ${{ env.dedicated_debuginfo }}
         path: ${{ env.install_dir }}/${{ env.dedicated_debuginfo }}
+        if-no-files-found: error
 
   pack-resources:
     name: Windows Native Resources
 
-    runs-on: windows-latest
+    runs-on: windows-2025
 
     defaults:
       run:
@@ -290,9 +300,6 @@ jobs:
     - name: Git fetch tags
       run: git fetch origin --tags --force
 
-    - name: Install packages
-      run: choco install -y ninja
-
     - uses: ilammy/msvc-dev-cmd@v1
       with:
         arch: x64
@@ -322,6 +329,7 @@ jobs:
       with:
         name: ${{ env.resources }}
         path: ${{ env.install_dir }}/${{ env.resources }}
+        if-no-files-found: error
 
   upload-latest-build:
     name: Upload Latest Build

CONTRIBUTING.md

@@ -116,13 +116,59 @@ This project uses the [CMake](https://cmake.org/) build system to generate ninja
 ### Preprocessor definitions
 In shared code, clientside code can be differentiated with CLIENT_DLL, vs. serverside's GAME_DLL. In more general engine files, Neotokyo specific code can be marked with a NEO ifdef.
 
-### Code style
+## Supported compilers
+
+Only the x86-64 architecture is supported.
+
+### Windows
+* MSVC v143 - VS 2022 C++ x64/x86 build tools (Latest)
+* MSVC version used by the latest `windows-2025` [runner image](https://github.com/actions/runner-images/blob/main/images/windows/Windows2025-Readme.md) (Microsoft.VisualStudio.Component.VC.Tools.x86.x64)
 
-No big restrictions on general code format, just try to more or less match the other SDK code style.
+### Linux
+* GCC 10 [steamrt3 'sniper'](https://gitlab.steamos.cloud/steamrt/sniper/sdk#toolchains)
+* GCC 14 [steamrt3 'sniper'](https://gitlab.steamos.cloud/steamrt/sniper/sdk#toolchains)
+* GCC 14 [steamrt4](https://gitlab.steamos.cloud/steamrt/steamrt4/sdk#toolchains)
+* Clang 19 [steamrt4](https://gitlab.steamos.cloud/steamrt/steamrt4/sdk#toolchains)
+
+### Code style
 
-* C++20 within GCC 10+ and MSVC v143+ support.
-* STL generally shouldn't be included in as it may conflicts with existing similar functions.
-* Valve likes to ( space ) their arguments, especially with macros, but it's not necessary to strictly follow everywhere.
+* C++20, within the [supported compilers'](#supported-compilers) capabilities.
+* Formatting:
+  * No big restrictions on general format, but try to more or less match the surrounding SDK code style for consistency. For example this typically means using tabs for whitespace, but generally go with what the file you're editing is doing stylistically.
+* Warnings are treated as errors.
+  * You may choose to suppress a warning with compiler-specific preprocessing directives if it is a false positive, but **please do not suppress valid warnings**; instead resolve it by fixing the underlying issue.
+  * For local development, you may disable warnings-as-errors by modifying your `CMAKE_COMPILE_WARNING_AS_ERROR` option, eg. by modifying the entry in your `CMakeCache.txt` file.
+  * For the CI runners, the following cases are exempt from the *warnings-as-errors* rule:
+    * Tagged release builds (`v<major>.<minor>`...)
+    * Branch name or its latest commit message containing the phrase `nwae` (acronym for: *"no warnings as errors"*)
+      * The `nwae` phrase also works locally, but you may need to refresh the CMake cache for it to take effect.
+  * Please note that the above methods of disabling warnings-as-errors are intended as temporary workarounds to allow devs to resolve exceptional situations (eg. the un-pinned MSVC compiler version changes unexpectedly changing warnings behaviour), and it should *not* be regularly relied upon to dodge warnings. The preferred long-term solution is almost always to properly fix the warning! 
+* STL and platform-specific headers may be used when needed, but generally the SDK libraries should be preferred. If you do use such includes, be careful with not bringing in conflicting macro definitions or performance-costly code.
+* The `min` and `max` from the SDK macros have been disabled, because they were polluting namespaces and making things like like `::max` awkward to use. You may use the templated `Min` and `Max` functions from `basetypes.h` instead.
+* Some nonstandard casting helpers are available:
+  * `assert_cast`:
+    * The preferred cast for pointer conversion where an incompatible cast may occur by accident
+      * If you know for a fact it is always a safe cast, you can use `static_cast` instead
+    * For debug builds, will runtime-validate the cast with `ptr == dynamic_cast<T>(ptr)`
+    * For release builds, it is identical to `static_cast`
+  * `narrow_cast`:
+    * The preferred cast for narrowing conversions where loss of information may occur
+      * Use to mark narrowing conversions where the input could overflow
+      * If you know for a fact it is always a safe cast, you can use `static_cast` instead
+    * For debug builds, will runtime-validate narrowing conversions for:
+      * Roundtrip equality: `value v of type A equals itself after A->B->A type conversion`
+      * Signed/unsigned conversion overflow
+    * For release builds, it is identical to `static_cast`
+  * `neo::bit_cast`:
+    * The preferred bit cast function, for performing a well-formed type pun
+    * Mostly used to avoid strict aliasing rule violations in the SDK code
+    * Acts as a wrapper for `std::bit_cast` when it's available, else will fall back to `memcpy` based conversions (for example on the steamrt3 default GCC compiler)
+    * For debug builds, a runtime assertion test is available as an additional parameter:
+      * `auto output = neo::bit_cast<T>(BC_TEST(input, expectedOutput));`
+      * When replacing ill-formed type puns, this test syntax can be used to ensure the output of `neo::bit_cast<T>(input)` remains identical to `expectedOutput`
+    * For release builds, the above test optimizes away, into:
+      * `auto output = neo::bit_cast<T>(input);`
+* Valve likes to `( space )` indent their parentheses, especially with macros, but it's not necessary to strictly follow everywhere.
 * Tabs are preferred for indentation, to be consistent with the SDK code.
-* When using a TODO/FIXME/HACK... style comment, use the format "// NEO TODO (Your-username): Example comment." to make it easier to search NEO specific todos/fixmes (opposed to Valve ones), and at a glance figure out who has written them.
+* When using a TODO/FIXME/HACK... style comment, use the format `// NEO TODO (Your-username): Example comment.` to make it easier to search for `NEO` specific todos/fixmes (opposed to Valve ones of the original SDK), and at a glance figure out who has written them.
 * For classes running on both client and server, you should generally follow Valve's <i>C_Thing</i> (client) -- <i>CThing</i> (server) convention. On shared files, this might mean #defining serverclass for client, or vice versa. There's plenty of examples of this pattern in Valve's classes for reference, [for example here](https://github.com/NeotokyoRebuild/neo/blob/f749c07a4701d285bbb463686d5a5a50c20b9528/mp/src/game/shared/hl2mp/weapon_357.cpp#L20).

src/CMakeLists.txt

@@ -22,6 +22,7 @@ if(NOT OS_WINDOWS AND NOT OS_LINUX AND NOT OS_MACOS)
     message(FATAL_ERROR "Unknown OS")
 endif()
 
+option(CMAKE_COMPILE_WARNING_AS_ERROR "Treat compile warnings as errors" ${DEFAULT_WARN_AS_ERR})
 option(NEO_CI_BUILD "CI build mode" OFF)
 option(NEO_USE_CCACHE "Use ccache" ${OS_LINUX})
 option(NEO_STAGING_ONLY "Staging mode (STAGING_ONLY)" OFF)
@@ -49,6 +50,7 @@ option(NEO_ENABLE_CPACK "Enable CPack" OFF)
 option(NEO_USE_MEM_DEBUG "Enable USE_MEM_DEBUG defines" OFF)
 option(NEO_GENERATE_GAMEDATA "Generate SourceMod gamedata" ${NEO_DEDICATED})
 
+message(STATUS "Treat compile warnings as errors: ${CMAKE_COMPILE_WARNING_AS_ERROR}")
 message(STATUS "CI build mode: ${NEO_CI_BUILD}")
 message(STATUS "Use ccache: ${NEO_USE_CCACHE}")
 message(STATUS "Staging (STAGING_ONLY): ${NEO_STAGING_ONLY}")
@@ -107,14 +109,22 @@ elseif(CMAKE_BUILD_TYPE STREQUAL "Release" OR CMAKE_BUILD_TYPE STREQUAL "RelWith
         NDEBUG
     )
 
-    add_compile_options(
-        #-fno-strict-aliasing
-        -ffast-math
-        -fno-omit-frame-pointer
-        -ftree-vectorize
-    )
+    if (NOT COMPILER_MSVC)
+        add_compile_options(
+            #-fno-strict-aliasing
+            -ffast-math
+            -fno-omit-frame-pointer
+            -ftree-vectorize
+        )
+    endif()
 endif()
 
+add_compile_definitions(
+	# Whether to validate narrowing conversions done with narrow_cast<T>(...)
+	# If not, will simply alias narrow_cast to static_cast
+	$<$<CONFIG:Debug>:PARANOID_NARROWING>
+)
+
 if(NEO_RAD_TELEMETRY_DISABLED)
     add_compile_definitions(RAD_TELEMETRY_DISABLED)
 endif()
@@ -142,6 +152,10 @@ add_compile_definitions(
     # NEO NOTE (nullsystem): New defines since 2025-02-18 TF2 SDK Update
     PLATFORM_64BITS
     INCLUDED_STEAM2_USERID_STRUCTS
+
+    # NEO NOTE (Rain): Needed to remove platform specific min/max C macro pollution;
+    # instead you can use the Min/Max templated functions from basetypes.h.
+    NOMINMAX
 )
 
 if(OS_WINDOWS)
@@ -215,7 +229,6 @@ if(OS_WINDOWS)
             /Zc:forScope
             /Zc:inline
             /Zc:forScope
-            /Oy-
             /MTd
         )
 
@@ -235,17 +248,16 @@ if(OS_WINDOWS)
         add_compile_options(
             #/Z7 # DebugInformationFormat - C7 Compatible
             #/Wp64 # Detect64bitPortabilityIssues
-            #/O2 # Optimization - Maximize Speed
-            /Ob2 # InlineFunctionExpansion - Any Suitable
-            /Oi # EnableIntrinsicFunctions
-            /Ot # FavorSizeOrSpeed - Favor Fast Code
-            /GF # EnableStringPooling
-            /Gy # EnableFunctionLevelLinking
+            /O2 # Optimization - Maximize Speed
             #/Zo
-            /Oy-
             /MT
         )
 
+        if(NOT CMAKE_SIZEOF_VOID_P EQUAL 8)
+            # In x64 compilers, /Oy and /Oy- are not available.
+            add_compile_options(/Oy-)
+        endif()
+
         add_link_options(
             /NODEFAULTLIB:libcmtd
             /NODEFAULTLIB:libcpmtd
@@ -300,14 +312,15 @@ if(OS_LINUX OR OS_MACOS)
         #-Wextra # if STEAM_BRANCH
         #-Wshadow # if STEAM_BRANCH
 
-        $<$<CXX_COMPILER_ID:GNU>:-mfpmath=sse>
+        # Required for SourceMod gamedata generation
         $<$<COMPILE_LANGUAGE:CXX>:-Wno-invalid-offsetof>
+
+        $<$<CXX_COMPILER_ID:GNU>:-mfpmath=sse>
         $<$<COMPILE_LANGUAGE:CXX>:-Wno-overloaded-virtual>
         $<$<COMPILE_LANGUAGE:CXX>:-Wno-reorder>
         -Wno-multichar
         -Wno-write-strings
         -Wno-unused-variable
-        -Wno-unused-but-set-variable
         -Wno-unused-function
         -Wno-unknown-pragmas
         -Wno-unused-parameter
@@ -317,8 +330,12 @@ if(OS_LINUX OR OS_MACOS)
         -Wno-float-equal
         -Wno-unused-local-typedefs
         -Wno-switch
-        -Wno-strict-aliasing
-        -Wno-address
+        #-Wno-strict-aliasing
+        #-Wno-address
+        $<$<CXX_COMPILER_ID:Clang>:-Wno-inconsistent-missing-override>
+
+        # Unsupported by the steamrt3 Clang compiler
+        $<$<NOT:$<AND:$<COMPILE_LANG_AND_ID:CXX,Clang>,$<CXX_COMPILER_VERSION:11.0.1>>>:-Wno-unused-but-set-variable>
 
         -Werror=return-type
         -fdiagnostics-show-option
@@ -329,30 +346,27 @@ if(OS_LINUX OR OS_MACOS)
         -Ustrncpy
 
         # Fix for non-clickable vgui buttons when using -O2
-        -fno-devirtualize-speculatively
+        $<$<CXX_COMPILER_ID:GNU>:-fno-devirtualize-speculatively>
 
         # Fix for non-clickable vgui buttons when using -O3
-        -fno-ipa-cp-clone
+        $<$<CXX_COMPILER_ID:GNU>:-fno-ipa-cp-clone>
 
         # Fix for "error: cast from pointer to smaller type 'unsigned int' loses information"
         #-fms-extensions
-
-        # Required for SourceMod gamedata generation
-        -Wno-invalid-offsetof
     )
 
     # Disable option for older compiler in docker
     if(NOT CMAKE_CXX_COMPILER_VERSION STREQUAL "4.8.4")
         add_compile_options(
             $<$<COMPILE_LANG_AND_ID:CXX,GNU>:-fabi-compat-version=2>
-            $<$<COMPILE_LANGUAGE:CXX>:-Wno-class-memaccess>
-            $<$<COMPILE_LANGUAGE:CXX>:-Wno-template-id-cdtor>
-            -Wno-nonnull-compare
+            $<$<COMPILE_LANG_AND_ID:CXX,GNU>:-Wno-class-memaccess>
+            $<$<COMPILE_LANG_AND_ID:CXX,GNU>:-Wno-template-id-cdtor>
+            $<$<CXX_COMPILER_ID:GNU>:-Wno-nonnull-compare>
         )
     endif()
 
     # We should always specify -Wl,--build-id, as documented at:
-    # http://linux.die.net/man/1/ld and http://fedoraproject.org/wiki/Releases/FeatureBuildId.http://fedoraproject.org/wiki/Releases/FeatureBuildId
+    # http://linux.die.net/man/1/ld and http://fedoraproject.org/wiki/Releases/FeatureBuildId
     add_link_options(
         -Wl,--build-id
         -static-libgcc
@@ -374,10 +388,15 @@ if(OS_LINUX)
     )
 endif()
 
+# Clang may define GCC or MSVC values as some kind of a compatibility thing,
+# but this can break code that relies on accurately identifying the used compiler.
+# The "ACTUALLY_..." defs here can be used for succintly determining the compiler for real.
 if(COMPILER_MSVC)
     add_compile_definitions(COMPILER_MSVC)
+    add_compile_definitions(ACTUALLY_COMPILER_MSVC)
 elseif(COMPILER_GCC)
     add_compile_definitions(COMPILER_GCC)
+    add_compile_definitions(ACTUALLY_COMPILER_GCC)
 elseif(COMPILER_CLANG)
     add_compile_definitions(COMPILER_CLANG)
 endif()