[Build] Clean up CMake compiler flags and CI build script (#7219)

Compiler flags are applied once and for the right compiler:

- Detect the toolchain by CMAKE_CXX_COMPILER_ID (MSVC / GNU / Clang,
  warn on an unknown one).
- MSVC: drop /std:c++20 (CMAKE_CXX_STANDARD already adds it) and the
  outdated C4251 suppression, add /utf-8, and use /O2 instead of /Ox.
- GCC: drop the no-op -Wno-long-long and the macOS-only -std=gnu++20
  override; CMAKE_CXX_EXTENSIONS=OFF now owns the dialect.
- Use add_compile_definitions for SFMT_MEXP instead of add_definitions.

CI build script:

- Only run the schema version check for server builds.
- Accept boolean env flags as 1 and say so in the header.
- Print ccache, ctest and cpack versions next to their log groups.
- Only build the macOS vcpkg triplet when vcpkg is actually used.
- Drop the extra cmake --build verbosity.
- Install NSIS without chocolatey progress noise.

Co-authored-by: toomm <toomm@users.noreply.github.com>

.ci/compile.sh

@@ -20,6 +20,7 @@
 # --profile writes an MSBuild performance summary and binary log, Windows only
 # uses env: BUILDTYPE MAKE_INSTALL MAKE_PACKAGE PACKAGE_TYPE PACKAGE_SUFFIX MAKE_SERVER MAKE_NO_CLIENT MAKE_TEST USE_CCACHE CCACHE_SIZE CCACHE_EVICTION_AGE BUILD_DIR CMAKE_GENERATOR TARGET_MACOS_VERSION MAKE_SANITIZE MAKE_COVERAGE VCPKG_TARGET_MACOS_VERSION BUILD_PARALLEL_LEVEL BUILD_PROFILE
 # (correspond to args: --debug/--release --install --package <package type> --suffix <suffix> --server --test --sanitize --coverage --ccache <ccache_size> --dir <dir> --profile)
+# boolean env flags are enabled with the value 1 (e.g. MAKE_SERVER=1)
 # exitcode: 1 for failure, 3 for invalid arguments
 
 # Read arguments
@@ -138,17 +139,21 @@ done
 
 set -e
 
-# Setup
-./servatrice/check_schema_version.sh
-if [[ ! $BUILDTYPE ]]; then
+# Schema version consistency (only relevant to the server)
+if [[ ${MAKE_SERVER:-} == "1" ]]; then
+  ./servatrice/check_schema_version.sh
+fi
+
+# Use default values if unset
+if [[ -z $BUILDTYPE ]]; then
   BUILDTYPE=Release
 fi
-if [[ ! $BUILD_DIR ]]; then
+if [[ -z $BUILD_DIR ]]; then
   BUILD_DIR="build"
 fi
 mkdir -p "$BUILD_DIR"
 
-if [[ $MAKE_COVERAGE ]]; then
+if [[ ${MAKE_COVERAGE:-} == "1" ]]; then
   # Resolved while the working directory is still the source root. $0 is relative to the
   # directory the script was invoked from, so after the cd below it no longer resolves, and
   # under `set -e` the failed subshell would abort the build rather than just yielding an empty
@@ -163,16 +168,16 @@ export CMAKE_POLICY_VERSION_MINIMUM=3.10
 
 # Add cmake flags
 flags=("-DCMAKE_BUILD_TYPE=$BUILDTYPE")
-if [[ $MAKE_SERVER ]]; then
+if [[ ${MAKE_SERVER:-} == "1" ]]; then
   flags+=("-DWITH_SERVER=1")
 fi
-if [[ $MAKE_NO_CLIENT ]]; then
+if [[ ${MAKE_NO_CLIENT:-} == "1" ]]; then
   flags+=("-DWITH_CLIENT=0" "-DWITH_ORACLE=0")
 fi
-if [[ $MAKE_TEST ]]; then
+if [[ ${MAKE_TEST:-} == "1" ]]; then
   flags+=("-DTEST=1")
 fi
-if [[ $MAKE_SANITIZE ]]; then
+if [[ ${MAKE_SANITIZE:-} == "1" ]]; then
   flags+=("-DENABLE_SANITIZERS=ON")
 else
   # Every leg states the flag explicitly rather than relying on the CMake
@@ -180,20 +185,20 @@ else
   # never leak libasan/libubsan into a shipped binary.
   flags+=("-DENABLE_SANITIZERS=OFF")
 fi
-if [[ $MAKE_COVERAGE ]]; then
+if [[ ${MAKE_COVERAGE:-} == "1" ]]; then
   flags+=("-DENABLE_COVERAGE=ON")
 fi
-if [[ $USE_CCACHE ]]; then
+if [[ ${USE_CCACHE:-} == "1" ]]; then
   flags+=("-DUSE_CCACHE=1")
-  if [[ $CCACHE_SIZE ]]; then
-    # note, this setting persists after running the script
+  if [[ -n $CCACHE_SIZE ]]; then
+    # This setting persists after running the script
     ccache --max-size "$CCACHE_SIZE"
   fi
 fi
-if [[ $PACKAGE_TYPE ]]; then
+if [[ -n $PACKAGE_TYPE ]]; then
   flags+=("-DCPACK_GENERATOR=$PACKAGE_TYPE")
 fi
-if [[ $USE_VCPKG ]]; then
+if [[ ${USE_VCPKG:-} == "1" ]]; then
   flags+=("-DUSE_VCPKG=1")
   flags+=("-DVCPKG_INSTALL_OPTIONS=--x-abi-tools-use-exact-versions")
 fi
@@ -221,7 +226,8 @@ fi
 buildflags=(--config "$BUILDTYPE")
 
 function ccachestatsverbose() {
-  # note, verbose only works on newer ccache, discard the error
+  ccache --version
+  # Verbose only works on newer ccache, discard the error
   local got
   if got="$(ccache --show-stats --verbose 2>/dev/null)"; then
     echo "$got"
@@ -261,42 +267,44 @@ if [[ $RUNNER_OS == macOS ]]; then
   # this works independent of the qt version as there should be only one version installed on the runner at a time
   export QTDIR
 
-  if [[ $TARGET_MACOS_VERSION ]]; then
-    # CMAKE_OSX_DEPLOYMENT_TARGET is a vanilla cmake flag needed to compile to target macOS version
+  if [[ -n $TARGET_MACOS_VERSION ]]; then
+    # CMAKE_OSX_DEPLOYMENT_TARGET is a vanilla CMake flag needed to compile to the target macOS version
     flags+=("-DCMAKE_OSX_DEPLOYMENT_TARGET=$TARGET_MACOS_VERSION")
 
-    # vcpkg dependencies need a vcpkg triplet file to compile to the target macOS version
-    # an easy way is to copy the x64-osx.cmake file and modify it
-    triplets_dir="/tmp/cmake/triplets"
-    triplet_version="custom-triplet"
-    triplet_file="$triplets_dir/$triplet_version.cmake"
-    arch=$(uname -m)
-    if [[ $arch == x86_64 ]]; then
-      arch="x64"
+    if [[ ${USE_VCPKG:-} == "1" ]]; then
+      # vcpkg dependencies need a vcpkg triplet file to compile to the target macOS version
+      # An easy way is to copy the x64-osx.cmake file and modify it
+      triplets_dir="/tmp/cmake/triplets"
+      triplet_version="custom-triplet"
+      triplet_file="$triplets_dir/$triplet_version.cmake"
+      arch=$(uname -m)
+      if [[ $arch == x86_64 ]]; then
+        arch="x64"
+      fi
+      mkdir -p "$triplets_dir"
+      triplet_source="../vcpkg/triplets/$arch-osx.cmake"
+      if [[ ! -f "$triplet_source" ]]; then
+        triplet_source="../vcpkg/triplets/community/$arch-osx.cmake"
+      fi
+      cp "$triplet_source" "$triplet_file"
+      # The triplet is what vcpkg hashes into the ABI hash of every port, so two legs that
+      # agree here reuse each other's binary cache. Targets 14, 15 and 26 on the same Xcode
+      # produce identical binaries when they share the lowest deployment target, so they all
+      # build against 14 to keep one vcpkg cache scope instead of three.
+      # Keep in sync with "vcpkg_scope" and "vcpkg_deployment_target" in desktop-build.yml.
+      vcpkg_target="${VCPKG_TARGET_MACOS_VERSION:-$TARGET_MACOS_VERSION}"
+      echo "set(VCPKG_CMAKE_SYSTEM_VERSION $vcpkg_target)" >>"$triplet_file"
+      echo "set(VCPKG_OSX_DEPLOYMENT_TARGET $vcpkg_target)" >>"$triplet_file"
+      flags+=("-DVCPKG_OVERLAY_TRIPLETS=$triplets_dir")
+      flags+=("-DVCPKG_HOST_TRIPLET=$triplet_version")
+      flags+=("-DVCPKG_TARGET_TRIPLET=$triplet_version")
+      echo "::group::Generated triplet $triplet_file"
+      cat "$triplet_file"
+      echo "::endgroup::"
     fi
-    mkdir -p "$triplets_dir"
-    triplet_source="../vcpkg/triplets/$arch-osx.cmake"
-    if [[ ! -f "$triplet_source" ]]; then
-      triplet_source="../vcpkg/triplets/community/$arch-osx.cmake"
-    fi
-    cp "$triplet_source" "$triplet_file"
-    # The triplet is what vcpkg hashes into the ABI hash of every port, so two legs that
-    # agree here reuse each other's binary cache. Targets 14, 15 and 26 on the same Xcode
-    # produce identical binaries when they share the lowest deployment target, so they all
-    # build against 14 to keep one vcpkg cache scope instead of three.
-    # Keep in sync with "vcpkg_scope" and "vcpkg_deployment_target" in desktop-build.yml.
-    vcpkg_target="${VCPKG_TARGET_MACOS_VERSION:-$TARGET_MACOS_VERSION}"
-    echo "set(VCPKG_CMAKE_SYSTEM_VERSION $vcpkg_target)" >>"$triplet_file"
-    echo "set(VCPKG_OSX_DEPLOYMENT_TARGET $vcpkg_target)" >>"$triplet_file"
-    flags+=("-DVCPKG_OVERLAY_TRIPLETS=$triplets_dir")
-    flags+=("-DVCPKG_HOST_TRIPLET=$triplet_version")
-    flags+=("-DVCPKG_TARGET_TRIPLET=$triplet_version")
-    echo "::group::Generated triplet $triplet_file"
-    cat "$triplet_file"
-    echo "::endgroup::"
   fi
 
-  echo "::group::Signing Certificate"
+  echo "::group::Setup signing certificate"
   if [[ -n "$MACOS_CERTIFICATE_NAME" ]]; then
     echo "$MACOS_CERTIFICATE" | base64 --decode >"certificate.p12"
     security create-keychain -p "$MACOS_CI_KEYCHAIN_PWD" build.keychain
@@ -311,7 +319,7 @@ if [[ $RUNNER_OS == macOS ]]; then
   fi
   echo "::endgroup::"
 
-  if [[ $MAKE_PACKAGE ]]; then
+  if [[ ${MAKE_PACKAGE:-} == "1" ]]; then
     # Workaround https://github.com/actions/runner-images/issues/7522
     # have hdiutil repeat the command 10 times in hope of success.
     # The wrapper has to exit non-zero once it gives up: CPack reads that status to
@@ -342,7 +350,7 @@ elif [[ $RUNNER_OS == Windows ]]; then
     # cmake only hands MSBuild a job count when one is asked for, and without it
     # MSBuild builds a single node: one project at a time, whatever MTT does inside
     # it. Ask for a node per core.
-    if [[ ! $BUILD_PARALLEL_LEVEL ]]; then
+    if [[ -z $BUILD_PARALLEL_LEVEL ]]; then
       BUILD_PARALLEL_LEVEL="${NUMBER_OF_PROCESSORS:-1}"
     fi
     buildflags+=(--parallel "$BUILD_PARALLEL_LEVEL")
@@ -368,19 +376,20 @@ elif [[ $RUNNER_OS == Windows ]]; then
   fi
 fi
 
-if [[ $USE_CCACHE ]]; then
+if [[ ${USE_CCACHE:-} == "1" ]]; then
   echo "::group::Show ccache stats"
   ccachestatsverbose
   echo "::endgroup::"
 fi
 
-echo "::group::Configure cmake"
+# Configure CMake
+echo "::group::Generate build system"
 cmake --version
-echo "Running cmake with flags: ${flags[*]}"
+echo "Running CMake configuration with the following flags: ${flags[*]}"
 cmake .. "${flags[@]}"
 echo "::endgroup::"
 
-if [[ $USE_CCACHE ]]; then
+if [[ ${USE_CCACHE:-} == "1" ]]; then
   # PCH-aware caching is required or ccache refuses to cache any TU that consumes a
   # precompiled header, silently recompiling everything on every run. The mtime and ctime
   # entries cover files created during the build itself: the moc/uic/rcc output and the
@@ -395,9 +404,10 @@ if [[ $USE_CCACHE ]]; then
   ccache --set-config sloppiness=pch_defines,time_macros,include_file_mtime,include_file_ctime
 fi
 
+# Build
 echo "::group::Build project"
 cpuinfo
-echo "Running cmake --build with flags: ${buildflags[*]}"
+echo "Running CMake with the following build flags: ${buildflags[*]}"
 cmake --build . "${buildflags[@]}"
 echo "::endgroup::"
 
@@ -409,16 +419,16 @@ if [[ $BUILD_PROFILE && $RUNNER_OS == Windows && -f msbuild-perf.log ]]; then
   echo "::endgroup::"
 fi
 
-if [[ $USE_CCACHE ]]; then
-  if [[ $CCACHE_EVICTION_AGE ]]; then
-    echo "::group::evict ccache files older than $CCACHE_EVICTION_AGE"
+if [[ ${USE_CCACHE:-} == "1" ]]; then
+  if [[ -n $CCACHE_EVICTION_AGE ]]; then
+    echo "::group::Evict ccache files older than $CCACHE_EVICTION_AGE"
     ccache --evict-older-than "$CCACHE_EVICTION_AGE"
     echo "::endgroup::"
   fi
   echo "::group::Show ccache stats again"
   ccachestatsverbose
   echo "::endgroup::"
-elif [[ $CCACHE_EVICTION_AGE ]]; then
+elif [[ -n $CCACHE_EVICTION_AGE ]]; then
   echo "::error file=$0::ccache eviction is enabled while ccache is disabled!"
 fi
 
@@ -435,9 +445,9 @@ if [[ $RUNNER_OS == macOS ]]; then
   echo "::endgroup::"
 fi
 
-if [[ $MAKE_TEST ]]; then
+if [[ ${MAKE_TEST:-} == "1" ]]; then
   echo "::group::Run tests"
-  if [[ $MAKE_SANITIZE ]]; then
+  if [[ ${MAKE_SANITIZE:-} == "1" ]]; then
     # detect_leaks=0: Qt and the sanitizers' own runtimes both leak at exit by design, and
     # LeakSanitizer's default is aggressive enough to bury our real findings under hundreds
     # of process-teardown reports. Malloc/free misuse and use-after-free are what ASan is for
@@ -455,11 +465,12 @@ if [[ $MAKE_TEST ]]; then
   # bound and -j recovers nearly all of the per-process overhead. The test binaries are safe to
   # run concurrently: the only one that claims a fixed resource is single_instance_manager_test,
   # which already scopes its local socket name to its own pid so parallel copies cannot collide.
+  ctest --version
   ctest -C "$BUILDTYPE" --output-on-failure -j "${CTEST_JOBS:-4}"
   echo "::endgroup::"
 fi
 
-if [[ $MAKE_COVERAGE ]]; then
+if [[ ${MAKE_COVERAGE:-} == "1" ]]; then
   # gcovr is not a build dependency and is not available everywhere, so a missing one is reported
   # and skipped rather than failing an otherwise green build.
   echo "::group::Coverage report"
@@ -484,7 +495,7 @@ if [[ $MAKE_COVERAGE ]]; then
   echo "::endgroup::"
 fi
 
-if [[ $RUNNER_OS == Windows && ! $MAKE_NO_CLIENT ]]; then
+if [[ $RUNNER_OS == Windows && ${MAKE_NO_CLIENT:-} != "1" ]]; then
   echo "::group::Probe the installer's holder lookup"
   # The working directory is still the build dir here: the package steps below are what
   # leave and re-enter it, and that is what made a $BUILD_DIR-prefixed path name
@@ -684,18 +695,21 @@ POWERSHELL
   echo "::endgroup::"
 fi
 
-if [[ $MAKE_INSTALL ]]; then
+# Install
+if [[ ${MAKE_INSTALL:-} == "1" ]]; then
   echo "::group::Install"
   cmake --build . --target install --config "$BUILDTYPE"
   echo "::endgroup::"
 fi
 
-if [[ $MAKE_PACKAGE ]]; then
+# Package
+if [[ ${MAKE_PACKAGE:-} == "1" ]]; then
   echo "::group::Create package"
+  cpack --version
   cmake --build . --target package --config "$BUILDTYPE"
   echo "::endgroup::"
 
-  if [[ $PACKAGE_SUFFIX ]]; then
+  if [[ -n $PACKAGE_SUFFIX ]]; then
     echo "::group::Update package name"
     cd ..
     BUILD_DIR="$BUILD_DIR" .ci/name_build.sh "$PACKAGE_SUFFIX"

.github/workflows/desktop-build.yml

@@ -773,7 +773,7 @@ jobs:
       - name: "[Windows] Install NSIS"
         if: matrix.os == 'Windows'
         shell: bash
-        run: choco install nsis
+        run: choco install nsis --no-progress
 
       # Ninja invokes cl.exe directly and needs the MSVC environment on PATH.
       - name: "[Windows] Set up MSVC environment"

CMakeLists.txt

@@ -1,11 +1,11 @@
 # Cockatrice's main CMakeLists.txt
 #
-# This is basically a wrapper to enable/disable the compilation
-# of the different projects: servatrice, cockatrice, test
+# This is basically a wrapper to enable/disable the compilation of the different projects:
+# Cockatrice, Oracle, Servatrice, Test
 # This file sets all the variables shared between the projects
 # like the installation path, compilation flags etc..
 
-# 3.16 required for Qt6 and target_precompile_headers()
+# CMake 3.16 is required for Qt6 and target_precompile_headers()
 cmake_minimum_required(VERSION 3.16)
 
 # Use compiler cache (ccache)
@@ -40,16 +40,11 @@ if(WITH_GUI_TEST AND NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/external/qttesting/
 endif()
 
 # Default to "Release" build type
-# User-provided value for CMAKE_BUILD_TYPE must be checked before the PROJECT() call
-if(DEFINED CMAKE_BUILD_TYPE)
-  set(CMAKE_BUILD_TYPE
-      ${CMAKE_BUILD_TYPE}
-      CACHE STRING "Type of build"
-  )
-else()
+# A user-provided CMAKE_BUILD_TYPE must be checked before the PROJECT() call
+if(NOT CMAKE_BUILD_TYPE)
   set(CMAKE_BUILD_TYPE
       Release
-      CACHE STRING "Type of build"
+      CACHE STRING "Build type"
   )
 endif()
 
@@ -108,20 +103,21 @@ endif()
 # Version can be overriden by git tags, see cmake/getversion.cmake
 project("Cockatrice" VERSION 3.2.0)
 
-# Set release name if not provided via env/cmake var
+# Set release name if not provided via ENV/CMake var
 if(NOT DEFINED GIT_TAG_RELEASENAME)
   set(GIT_TAG_RELEASENAME "Reality Check")
 endif()
 
-# Use c++20 for all targets
+# Use C++20 for all targets
 set(CMAKE_CXX_STANDARD
     20
     CACHE STRING "C++ ISO Standard"
 )
-set(CMAKE_CXX_STANDARD_REQUIRED True)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
+set(CMAKE_CXX_EXTENSIONS OFF)
 
 # Set conventional loops
-set(CMAKE_ALLOW_LOOSE_LOOP_CONSTRUCTS true)
+set(CMAKE_ALLOW_LOOSE_LOOP_CONSTRUCTS ON)
 
 # Search path for cmake modules
 set(COCKATRICE_CMAKE_PATH "${PROJECT_SOURCE_DIR}/cmake")
@@ -134,31 +130,31 @@ include(createversionfile)
 
 # Define a proper install path
 if(UNIX)
-  if(APPLE)
-    # macOS
-    # Due to the special bundle structure ignore
-    # the prefix eventually set by the user.
+  if(APPLE) # macOS
+    # Due to the special bundle structure ignore the prefix eventually set by the user
     set(CMAKE_INSTALL_PREFIX ${CMAKE_BINARY_DIR}/release)
 
-    # Force ccache usage if available
-    get_property(RULE_LAUNCH_COMPILE GLOBAL PROPERTY RULE_LAUNCH_COMPILE)
-    if(RULE_LAUNCH_COMPILE)
-      message(STATUS "Force enabling CCache usage under macOS")
-      # Set up wrapper scripts
-      configure_file("${COCKATRICE_CMAKE_PATH}/launch-c.in" launch-c)
-      configure_file("${COCKATRICE_CMAKE_PATH}/launch-cxx.in" launch-cxx)
-      execute_process(COMMAND chmod a+rx "${CMAKE_BINARY_DIR}/launch-c" "${CMAKE_BINARY_DIR}/launch-cxx")
+    # Force ccache usage if requested and available
+    if(USE_CCACHE)
+      get_property(RULE_LAUNCH_COMPILE GLOBAL PROPERTY RULE_LAUNCH_COMPILE)
+      if(RULE_LAUNCH_COMPILE)
+        message(STATUS "Force enabling ccache usage under macOS")
 
-      # Set Xcode project attributes to route compilation through our scripts
-      set(CMAKE_XCODE_ATTRIBUTE_CC "${CMAKE_BINARY_DIR}/launch-c")
-      set(CMAKE_XCODE_ATTRIBUTE_CXX "${CMAKE_BINARY_DIR}/launch-cxx")
-      set(CMAKE_XCODE_ATTRIBUTE_LD "${CMAKE_BINARY_DIR}/launch-c")
-      set(CMAKE_XCODE_ATTRIBUTE_LDPLUSPLUS "${CMAKE_BINARY_DIR}/launch-cxx")
+        # Set up wrapper scripts
+        configure_file("${COCKATRICE_CMAKE_PATH}/launch-c.in" launch-c)
+        configure_file("${COCKATRICE_CMAKE_PATH}/launch-cxx.in" launch-cxx)
+        execute_process(COMMAND chmod a+rx "${CMAKE_BINARY_DIR}/launch-c" "${CMAKE_BINARY_DIR}/launch-cxx")
+
+        # Set Xcode project attributes to route compilation through our scripts
+        set(CMAKE_XCODE_ATTRIBUTE_CC "${CMAKE_BINARY_DIR}/launch-c")
+        set(CMAKE_XCODE_ATTRIBUTE_CXX "${CMAKE_BINARY_DIR}/launch-cxx")
+        set(CMAKE_XCODE_ATTRIBUTE_LD "${CMAKE_BINARY_DIR}/launch-c")
+        set(CMAKE_XCODE_ATTRIBUTE_LDPLUSPLUS "${CMAKE_BINARY_DIR}/launch-cxx")
+      endif()
     endif()
-  else()
-    # Linux / BSD
+  else() # Linux / BSD
     if(CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT)
-      #fix package build
+      # Fix package build
       if(PREFIX)
         set(CMAKE_INSTALL_PREFIX ${PREFIX})
       else()
@@ -166,47 +162,67 @@ if(UNIX)
       endif()
     endif()
   endif()
-elseif(WIN32)
+elseif(WIN32) # Windows (including 64-bit)
   set(CMAKE_INSTALL_PREFIX ${CMAKE_BINARY_DIR}/rundir/${CMAKE_BUILD_TYPE})
 endif()
 
-# Define proper compilation flags
-if(MSVC)
-  # Disable Warning C4251, C++20 compatibility, Multi-threaded Builds, Warn Detection, Unwind Semantics, Debug Symbols
-  set(CMAKE_CXX_FLAGS "/wd4251 /Zc:__cplusplus /std:c++20 /permissive- /W4 /MP /EHsc /Zi")
-  # Visual Studio: Maximum Optimization, Multi-threaded DLL
-  set(CMAKE_CXX_FLAGS_RELEASE "/Ox /MD")
-  # Visual Studio: No Optimization, Multi-threaded Debug DLL
+# Define compiler flags
+if(MSVC) # Microsoft Visual C++ compiler
+  # /EHsc              Enable standard C++ exception handling
+  # /MP                Enable parallel compilation
+  # /permissive-       Enable more standards-conforming behavior
+  # /utf-8             Set source file encoding and execution character set to UTF-8
+  # /W4                Enable warning level 4
+  # /Zc:__cplusplus    Enable correct __cplusplus reporting
+  # /Zi                Generate debugging information (Program Database, PDB)
+  set(CMAKE_CXX_FLAGS "/EHsc /MP /permissive- /utf-8 /W4 /Zc:__cplusplus /Zi")
+  # /O2                Balanced optimization
+  # /MD                Link against the multi-threaded DLL runtime library (Release CRT)
+  set(CMAKE_CXX_FLAGS_RELEASE "/O2 /MD")
+  # /Od                Disable optimization
+  # /MDd               Link against the multi-threaded Debug DLL runtime library (Debug CRT)
   set(CMAKE_CXX_FLAGS_DEBUG "/Od /MDd")
 
-  # Generate PDB, even when in release (So developers can better analyze crash logs)
+  # Generate PDBs, even in release builds, so developers can better analyze crash logs
+  # /DEBUG             Enable debug symbols and PDB generation
+  # /OPT:REF           Remove unreferenced functions/data
+  # /OPT:ICF           Fold identical COMDAT functions/data
   set(CMAKE_EXE_LINKER_FLAGS_RELEASE "${CMAKE_EXE_LINKER_FLAGS_RELEASE} /DEBUG /OPT:REF /OPT:ICF")
 
   add_compile_definitions(_SILENCE_STDEXT_ARR_ITERS_DEPRECATION_WARNING)
-elseif(CMAKE_COMPILER_IS_GNUCXX)
-  # linux/gcc, bsd/gcc, windows/mingw
+elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") # GCC compiler
   include(CheckCXXCompilerFlag)
 
-  set(CMAKE_CXX_FLAGS_RELEASE "-s -O2")
+  # -O2             Balanced optimization
+  # -s              Remove symbols from the executable
+  set(CMAKE_CXX_FLAGS_RELEASE "-O2 -s")
+
+  # -ggdb           Produce GDB debugging symbols
+  # -O0             Disable optimization
+  # -Wall           Enable a broad set of useful warnings
+  # -Wextra         Enable a set of extra warnings
+  # -Werror         Treat warnings as compilation errors
   if(WARNING_AS_ERROR)
     set(CMAKE_CXX_FLAGS_DEBUG "-ggdb -O0 -Wall -Wextra -Werror")
   else()
     set(CMAKE_CXX_FLAGS_DEBUG "-ggdb -O0 -Wall -Wextra")
   endif()
 
-  if(APPLE)
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=gnu++20")
-  endif()
-
+  # -Wcast-align                          Catch unsafe pointer casts
+  # -Wmissing-declarations                Catch defined functions without a prior declaration
+  # -Wno-error=extra                      Downgrade some -Wextra warnings from errors to warnings
+  # -Wno-error=delete-non-virtual-dtor    Downgrade delete-non-virtual-dtor from error to warning
+  # -Wno-error=sign-compare               Downgrade signed/unsigned comparison from error to warning
+  # -Wno-error=missing-declarations       Downgrade -Wmissing-declarations from error to warning
+  # -Wno-error=sfinae-incomplete          GCC 16+: Qt MOC + protobuf forward decls trigger this
   set(ADDITIONAL_DEBUG_FLAGS
       -Wcast-align
       -Wmissing-declarations
-      -Wno-long-long
       -Wno-error=extra
       -Wno-error=delete-non-virtual-dtor
       -Wno-error=sign-compare
       -Wno-error=missing-declarations
-      -Wno-error=sfinae-incomplete # GCC 16+: Qt MOC + protobuf forward decls trigger this
+      -Wno-error=sfinae-incomplete
   )
 
   foreach(FLAG ${ADDITIONAL_DEBUG_FLAGS})
@@ -218,9 +234,16 @@ elseif(CMAKE_COMPILER_IS_GNUCXX)
 
   # Reduce compiler I/O by using pipes between stages instead of temp files
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pipe")
-else()
-  # other: osx/llvm, bsd/llvm
+elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang") # Clang and AppleClang compilers
+  # -O2                      Balanced optimization
   set(CMAKE_CXX_FLAGS_RELEASE "-O2")
+
+  # -g                       Include debug information
+  # -O0                      Disable optimization
+  # -Wall                    Enable a broad set of useful warnings
+  # -Wextra                  Enable a set of extra warnings
+  # -Werror                  Treat warnings as compilation errors
+  # -Wno-unused-parameter    Suppress warnings about unused function parameters (common in Qt callbacks)
   if(WARNING_AS_ERROR)
     set(CMAKE_CXX_FLAGS_DEBUG "-g -O0 -Wall -Wextra -Werror -Wno-unused-parameter")
   else()
@@ -229,6 +252,8 @@ else()
 
   # Reduce compiler I/O by using pipes between stages instead of temp files
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pipe")
+else() # Unknown compiler
+  message(WARNING "Unknown C++ compiler: ${CMAKE_CXX_COMPILER_ID}")
 endif()
 
 # Always included, because they define cockatrice_disable_sanitizers_for() and
@@ -259,9 +284,9 @@ if(NOT COCKATRICE_DEBUG_INFO_LEVEL STREQUAL "")
   endif()
 endif()
 
-# GNU systems need to define the Mersenne exponent for the RNG to compile w/o warning
+# GNU systems need to define the Mersenne exponent for SFMT, so the RNG compiles without warnings
 if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
-  add_definitions("-DSFMT_MEXP=19937")
+  add_compile_definitions(SFMT_MEXP=19937)
 endif()
 
 find_package(Threads REQUIRED)
@@ -314,15 +339,15 @@ if(${Protobuf_VERSION} VERSION_LESS "3.21.0.0" AND NOT EXISTS "${Protobuf_PROTOC
   message(FATAL_ERROR "No protoc command found!")
 endif()
 
-#Find OpenSSL
-if(WIN32)
+# Find OpenSSL
+if(WIN32) # Windows (including 64-bit)
   find_package(OpenSSL REQUIRED)
   if(OPENSSL_FOUND)
     include_directories(${OPENSSL_INCLUDE_DIRS})
   endif()
 endif()
 
-#Find VCredist
+# Find Visual C++ Redistributable
 if(MSVC)
   find_package(VCredistRuntime)
 endif()
@@ -535,3 +560,10 @@ if(Qt6_FOUND AND Qt6_VERSION_MINOR GREATER_EQUAL 3)
   # Qt6.3+ requires project finalization to support translations
   qt_finalize_project()
 endif()
+
+# Print compiler identification at configuration time
+message(STATUS "C++ compiler: ${CMAKE_CXX_COMPILER_ID}")
+message(STATUS "C++ compiler version: ${CMAKE_CXX_COMPILER_VERSION}")
+message(STATUS "C++ compiler path: ${CMAKE_CXX_COMPILER}")
+message(STATUS "C++ standard: ${CMAKE_CXX_STANDARD}")
+message(STATUS "C++ extensions: ${CMAKE_CXX_EXTENSIONS}")

servatrice/check_schema_version.sh

@@ -10,7 +10,7 @@ declare -i schema_ver="${version_line%%)*}"
 latest_migration="$(ls -1 servatrice/migrations/ | tail -n1)"
 xtoysql="${latest_migration#servatrice_}"
 xtoy="${xtoysql%.sql}"
-declare -i old_ver="10#${xtoy%_to_*}" #declare as integer with base 10, numbers with a leading 0 are normally interpreted as base 16
+declare -i old_ver="10#${xtoy%_to_*}" # declare as integer with base 10, numbers with a leading 0 are normally interpreted as base 16
 declare -i new_ver="10#${xtoy#*_to_}"
 
 if ((old_ver >= new_ver)); then