Support tab-separated deck imports (#4222) (#7267)

Third-party deck tools (e.g. Delver Lens) export decks as tab-separated rows
such as "2	Card Name	Set Name". Card names never contain tab characters,
so a leading numeric column marks a tabular record: the first two columns are read
as the quantity and card name, and any further columns are discarded as
printer/set metadata.

- deck_list: parse tab-delimited rows before whitespace collapsing
- tests: cover tab-separated mainboard and sideboard imports
- docs: document the accepted plain-text import format (fills the TODO)

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>

.ci/Ubuntu26.04/Dockerfile

@@ -9,6 +9,7 @@ RUN apt-get update && \
         cmake \
         file \
         g++ \
+        gcovr \
         git \
         libgl-dev \
         liblzma-dev \

.ci/compile.sh

@@ -8,14 +8,17 @@
 # --suffix <suffix> renames package with this suffix, requires arg
 # --server compiles servatrice
 # --test runs tests
+# --sanitize builds with AddressSanitizer and UndefinedBehaviorSanitizer, gcc/clang only
+# --coverage instruments the test run for line coverage and prints a gcovr summary, gcc only
+
 # --debug or --release sets the build type ie CMAKE_BUILD_TYPE
 # --ccache [<size>] uses ccache and shows stats, optionally provide size
 # --evict-ccache <age> runs ccache eviction based on given age after build
 # --dir <dir> sets the name of the build dir, default is "build"
 # --cmake-generator <generator> sets CMAKE_GENERATOR as used by cmake
 # --target-macos-version <version> sets the min os version - only used for macOS builds
-# 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
-# (correspond to args: --debug/--release --install --package <package type> --suffix <suffix> --server --test --ccache <ccache_size> --dir <dir>)
+# 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
+# (correspond to args: --debug/--release --install --package <package type> --suffix <suffix> --server --test --sanitize --coverage --ccache <ccache_size> --dir <dir>)
 # exitcode: 1 for failure, 3 for invalid arguments
 
 # Read arguments
@@ -57,6 +60,14 @@ while [[ $# != 0 ]]; do
       MAKE_TEST=1
       shift
       ;;
+    '--sanitize')
+      MAKE_SANITIZE=1
+      shift
+      ;;
+    '--coverage')
+      MAKE_COVERAGE=1
+      shift
+      ;;
     '--debug')
       BUILDTYPE="Debug"
       shift
@@ -131,6 +142,15 @@ if [[ ! $BUILD_DIR ]]; then
   BUILD_DIR="build"
 fi
 mkdir -p "$BUILD_DIR"
+
+if [[ $MAKE_COVERAGE ]]; 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
+  # variable. Only the coverage report needs this, so only coverage pays for it.
+  SRC_ROOT="$(cd "$(dirname "$0")/.." && pwd)"
+fi
+
 cd "$BUILD_DIR"
 
 # Set minimum CMake Version
@@ -147,6 +167,17 @@ fi
 if [[ $MAKE_TEST ]]; then
   flags+=("-DTEST=1")
 fi
+if [[ $MAKE_SANITIZE ]]; then
+  flags+=("-DENABLE_SANITIZERS=ON")
+else
+  # The release-package step re-runs cmake in the same build dir, so an
+  # ENABLE_SANITIZERS=ON left over from the debug step would otherwise leak
+  # into the shipped binaries (libasan/libubsan Depends in the DEB).
+  flags+=("-DENABLE_SANITIZERS=OFF")
+fi
+if [[ $MAKE_COVERAGE ]]; then
+  flags+=("-DENABLE_COVERAGE=ON")
+fi
 if [[ $USE_CCACHE ]]; then
   flags+=("-DUSE_CCACHE=1")
   # PCH-aware caching is required or ccache refuses to cache any TU that
@@ -306,7 +337,50 @@ fi
 
 if [[ $MAKE_TEST ]]; then
   echo "::group::Run tests"
-  ctest -C "$BUILDTYPE" --output-on-failure
+  if [[ $MAKE_SANITIZE ]]; 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
+    # and they stay fully on.
+    # abort_on_error=1: a bad access leaves the process in an unusable state, so continuing
+    # only produces a second, more confusing report from the wreckage.
+    # UBSAN halt_on_error=1: without it UBSan prints "runtime error: ..." but the
+    # process exits 0 (recover mode), so gtest/ctest report a pass and the job
+    # stays green. Combined with -fno-sanitize-recover=undefined at compile time,
+    # the first finding aborts the test process with a non-zero exit.
+    export ASAN_OPTIONS="detect_leaks=0:abort_on_error=1:print_stacktrace=1:symbolize=1"
+    export UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1:symbolize=1"
+  fi
+  # Every TEST() is its own CTest entry, so the suite is process-spawn bound rather than CPU
+  # 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 -C "$BUILDTYPE" --output-on-failure -j "${CTEST_JOBS:-4}"
+  echo "::endgroup::"
+fi
+
+if [[ $MAKE_COVERAGE ]]; 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"
+  GCOVR="$(command -v gcovr || true)"
+  if [[ -z $GCOVR ]]; then
+    echo "gcovr not found; install it to produce a coverage report"
+  else
+    "$GCOVR" --version
+    # This script configures with `cmake ..`, so the build dir it was started from is the object
+    # directory and the source root is one level up. Both are passed explicitly: gcovr otherwise
+    # walks the current directory and trips over in-tree build directories left by other tooling,
+    # whose stale .gcno files cannot be resolved. --config is named too, so the filters apply no
+    # matter what directory gcovr happens to start from.
+    GCOVR_ARGS=("." --root "$SRC_ROOT" --object-directory "." --config "$SRC_ROOT/gcovr.cfg")
+    "$GCOVR" "${GCOVR_ARGS[@]}" --txt --decisions
+    "$GCOVR" "${GCOVR_ARGS[@]}" --html-details coverage-details.html --html-title "Cockatrice line coverage"
+    "$GCOVR" "${GCOVR_ARGS[@]}" --lcov -o coverage.lcov
+    "$GCOVR" "${GCOVR_ARGS[@]}" --cobertura-pretty -o coverage.xml
+    "$GCOVR" "${GCOVR_ARGS[@]}" --json-summary-pretty -o coverage-summary.json
+    echo "Coverage artifacts written to coverage-details.html, coverage.lcov, coverage.xml and coverage-summary.json"
+  fi
   echo "::endgroup::"
 fi
 

.ci/docker.sh

@@ -13,11 +13,17 @@
 # --set-cache <location> sets the location to cache the image or for ccache
 #
 # requires: docker
-# uses env: NAME CACHE BUILD GET SAVE INTERACTIVE
+# uses env: NAME IMAGE_SUFFIX CACHE BUILD GET SAVE INTERACTIVE
 # (correspond to args: <name> --set-cache <cache> --build --get --save --interactive)
 # sets env: RUN CCACHE_DIR IMAGE_NAME RUN_ARGS RUN_OPTS BUILD_SCRIPT
 # exitcode: 1 for failure, 2 for missing dockerfile, 3 for invalid arguments
 #
+# IMAGE_SUFFIX is optional and only appended to the image name, never to NAME, so that a matrix
+# leg can have its own image and image cache without needing its own directory under .ci. Two legs
+# building and tagging the same image concurrently would race on `docker build --tag`.
+# It is deliberately not called SUFFIX: compile.sh already uses SUFFIX for output file names, and
+# a job-level SUFFIX would be silently overridden by those per-step env blocks.
+#
 # exported RUN function will run the BUILD_SCRIPT inside of the docker container.
 # note that the docker container will not inherit any environment variables!
 #
@@ -81,7 +87,7 @@ if ! [[ $NAME ]]; then
   return 3
 fi
 
-export IMAGE_NAME="${project_name,,}_${NAME,,}" # lower case
+export IMAGE_NAME="${project_name,,}_${NAME,,}${IMAGE_SUFFIX:+_${IMAGE_SUFFIX,,}}" # lower case
 
 docker_dir=".ci/$NAME"
 if ! [[ -r $docker_dir/Dockerfile ]]; then

.github/workflows/desktop-build.yml

@@ -144,17 +144,36 @@ jobs:
 
             package: DEB
 
-    name: ${{ matrix.distro }} ${{ matrix.version }}
+            # The one leg that runs the suite under AddressSanitizer and
+            # UndefinedBehaviorSanitizer. Debian 13 and Fedora 44 keep covering the
+            # uninstrumented suite, so nothing is lost by instrumenting this one.
+            sanitize: yes
+
+          - distro: Ubuntu
+            version: 26.04
+            variant: cover    # separate image and ccache, same distro
+
+            # The one leg that runs instrumented for line coverage and publishes the report as
+            # a workflow artifact. It is a separate leg rather than a second flag on the
+            # sanitizer leg because the two instruments answer different questions: the
+            # sanitizer leg reports what the run did wrong, the coverage leg reports what the
+            # run never touched at all. Combining them would also slow both down. Packaging is
+            # skipped because the leg above already publishes the DEB for this distro.
+            coverage: yes
+            package: skip
+
+    name: ${{ matrix.distro }} ${{ matrix.version }}${{ matrix.variant && format(' {0}', matrix.variant) || '' }}
     needs: configure
     runs-on: ubuntu-latest    # https://github.com/actions/runner-images
     continue-on-error: ${{ matrix.allow-failure == 'yes' }}
     timeout-minutes: 70
     env:
-      CACHE: ${{ github.workspace }}/.cache/${{ matrix.distro }}${{ matrix.version }}    # directory for caching docker image and ccache
+      CACHE: ${{ github.workspace }}/.cache/${{ matrix.distro }}${{ matrix.version }}${{ matrix.variant }}    # directory for caching docker image and ccache
       CCACHE_EVICTION_AGE: 7d
       CCACHE_SIZE: 600M    # space of all repo is 10Gi: https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#usage-limits-and-eviction-policy
       CMAKE_GENERATOR: 'Ninja'
       NAME: ${{ matrix.distro }}${{ matrix.version }}
+      IMAGE_SUFFIX: ${{ matrix.variant }}
 
     steps:
       - name: "Checkout"
@@ -164,7 +183,7 @@ jobs:
         id: restore_ccache
         uses: actions/cache/restore@v6
         with:
-          key: ccache-${{ matrix.distro }}${{ matrix.version }}
+          key: ccache-${{ matrix.distro }}${{ matrix.version }}${{ matrix.variant }}
           path: ${{ env.CACHE }}
 
       - name: "Build ${{ matrix.distro }} ${{ matrix.version }} Docker image"
@@ -174,6 +193,9 @@ jobs:
       - name: "Build debug and test"
         if: matrix.test != 'skip'
         shell: bash
+        env:
+          sanitize: '${{ matrix.sanitize }}'
+          coverage: '${{ matrix.coverage }}'
         run: |
           source .ci/docker.sh
           args=()
@@ -181,7 +203,33 @@ jobs:
           args+=(--ccache "$CCACHE_SIZE")
           args+=(--cmake-generator "$CMAKE_GENERATOR")
 
-          RUN --server --debug --test "${args[@]}"
+          extra=()
+          [[ $sanitize == yes ]] && extra+=(--sanitize)
+          [[ $coverage == yes ]] && extra+=(--coverage)
+
+          RUN --server --debug --test "${extra[@]}" "${args[@]}"
+
+      # RUN bind-mounts the workspace into the container, so the report the build wrote into
+      # build/ is already on the host and no second container is needed to collect it.
+      - name: "Collect coverage report"
+        if: matrix.coverage == 'yes'
+        shell: bash
+        run: |
+          mkdir -p coverage
+          # --html-details writes one page per source file plus a stylesheet and script, and the
+          # index links to all of them by relative name. Copying the index alone would publish a
+          # report where every link 404s, so the whole family has to travel together.
+          cp build/coverage-details* coverage/
+          cp build/coverage.lcov build/coverage.xml build/coverage-summary.json coverage/
+          ls -la coverage/
+
+      - name: "Upload coverage artifact"
+        if: matrix.coverage == 'yes'
+        uses: actions/upload-artifact@v4
+        with:
+          name: coverage-report
+          path: coverage/
+          if-no-files-found: warn
 
       - name: "Build release package"
         id: build

CMakeLists.txt

@@ -22,6 +22,10 @@ option(WITH_ORACLE "Build Cockatrice card database tool (Oracle)" ON)
 option(WITH_SERVER "Build Cockatrice server (Servatrice)" OFF)
 # Compile tests
 option(TEST "Build tests" OFF)
+# Build with AddressSanitizer and UndefinedBehaviorSanitizer
+option(ENABLE_SANITIZERS "Build with AddressSanitizer and UndefinedBehaviorSanitizer" OFF)
+# Build instrumented for GCC line coverage, reported with gcovr
+option(ENABLE_COVERAGE "Build instrumented for GCC line coverage" OFF)
 # Use vcpkg regardless of OS
 option(USE_VCPKG "Use vcpkg regardless of OS" OFF)
 
@@ -211,6 +215,12 @@ else()
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pipe")
 endif()
 
+# Always included, because they define cockatrice_disable_sanitizers_for() and
+# cockatrice_disable_coverage_for() for the vendored trees. They only touch the global flags
+# when ENABLE_SANITIZERS or ENABLE_COVERAGE is on.
+include(Sanitizers)
+include(Coverage)
+
 # GNU systems need to define the Mersenne exponent for the RNG to compile w/o warning
 if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
   add_definitions("-DSFMT_MEXP=19937")

cmake/Coverage.cmake

@@ -0,0 +1,75 @@
+# Line-coverage instrumentation opt-in, plus per-source opt-out
+# Input: ENABLE_COVERAGE
+# Defines: cockatrice_disable_coverage_for
+#
+# Included once from the root CMakeLists.txt, immediately after the per-compiler flag
+# block, so add_compile_options() reaches every library and executable defined below.
+#
+# Note the name deliberately shadows CMake's own built-in Modules/Coverage.cmake. It resolves
+# to this file only because the root CMakeLists.txt puts cmake/ at the front of
+# CMAKE_MODULE_PATH; if that ever moves, include(Coverage) would silently pick up the built-in
+# module, which only defines a `COVERAGE` custom target and touches no global flags, so
+# instrumentation would quietly stop happening rather than erroring.
+
+include(CheckCXXSourceCompiles)
+
+# Opt a set of vendored sources out of instrumentation.
+#
+# cockatrice_disable_coverage_for(<source> ...)
+#
+# Third-party code drags the total down with lines no test can ever reach, and a total that
+# cannot move is not a useful signal. This is the same carve-out the sanitizer build uses.
+#
+# The sources must be named relative to the directory that compiles them. CMake resolves a
+# source-file property against the directory scope the target was declared in, so calling
+# this from the root with absolute paths silently does nothing. That is why the call sites
+# live next to the add_library/add_executable that pull the vendored code in.
+#
+# SKIP_PRECOMPILE_HEADERS mirrors the sanitizer helper. The precompiled header carries the
+# instrumentation flags, and mixing an instrumented PCH with an uninstrumented translation
+# unit makes GCC reject it.
+function(cockatrice_disable_coverage_for)
+  if(NOT ENABLE_COVERAGE)
+    return()
+  endif()
+  # There is no -fno-coverage: --coverage is shorthand for -fprofile-arcs -ftest-coverage, so both
+  # halves have to be negated. CC rejects "-fno-coverage" outright, which is how the vendored C
+  # files below hit it first.
+  set_source_files_properties(
+    ${ARGN} PROPERTIES COMPILE_OPTIONS "-fno-profile-arcs;-fno-test-coverage" SKIP_PRECOMPILE_HEADERS ON
+  )
+endfunction()
+
+if(ENABLE_COVERAGE)
+  # Line coverage needs the compiler's own toolchain, so only GCC is supported. Clang's
+  # equivalent is llvm-cov, which reads a different intermediate format and would need a
+  # separate report path; rejecting it is better than emitting .gcno files that gcovr cannot
+  # read.
+  if(NOT CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+    message(FATAL_ERROR "ENABLE_COVERAGE requires GCC, but the compiler is ${CMAKE_CXX_COMPILER_ID}. "
+                        "Clang coverage needs llvm-cov and is not wired up."
+    )
+  endif()
+
+  # Probe the way the flag will actually be used: compile AND link. A .gcno file comes from the
+  # compile step, but the runtime that writes .gcda files comes from the link step, so probing
+  # only the compile step would pass on a toolchain that cannot actually produce coverage.
+  set(_cockatrice_saved_required_flags "${CMAKE_REQUIRED_FLAGS}")
+  set(_cockatrice_saved_required_link_options "${CMAKE_REQUIRED_LINK_OPTIONS}")
+  set(CMAKE_REQUIRED_FLAGS "-O0 -g")
+  set(CMAKE_REQUIRED_LINK_OPTIONS -fprofile-arcs -ftest-coverage)
+  check_cxx_source_compiles("int main() { return 0; }" COCKATRICE_HAVE_COVERAGE_FLAGS)
+  set(CMAKE_REQUIRED_FLAGS "${_cockatrice_saved_required_flags}")
+  set(CMAKE_REQUIRED_LINK_OPTIONS "${_cockatrice_saved_required_link_options}")
+
+  if(NOT COCKATRICE_HAVE_COVERAGE_FLAGS)
+    message(FATAL_ERROR "ENABLE_COVERAGE is on, but this toolchain rejected the coverage flags.")
+  endif()
+
+  # -O0 keeps the line mapping honest: at higher optimisation levels the compiler merges and
+  # relocates lines, and the report describes code that no longer exists in that form.
+  add_compile_options(--coverage -O0 -g)
+  add_link_options(--coverage)
+
+  message(STATUS "Coverage instrumentation enabled (--coverage). Report with gcovr over the build dir.")
+endif()

cmake/FindQtRuntime.cmake

@@ -40,7 +40,9 @@ endif()
 if(TEST)
   # Union of Qt modules required across all test targets (independent of application targets).
   # When adding a new test that needs additional Qt modules, add them here rather than in the test's CMakeLists.txt.
-  set(_TEST_NEEDED Concurrent Network Svg Widgets)
+  # Test is QtTest: it brings QAbstractItemModelTester and QSignalSpy, which are how the Qt model
+  # and signal contracts get verified without standing up a GUI.
+  set(_TEST_NEEDED Concurrent Network Svg Widgets Test)
 endif()
 
 set(REQUIRED_QT_COMPONENTS ${REQUIRED_QT_COMPONENTS} ${_SERVATRICE_NEEDED} ${_COCKATRICE_NEEDED} ${_ORACLE_NEEDED}

cmake/NSIS.template.in

@@ -304,10 +304,11 @@ SetShellVarContext all
 ; _?=$INSTDIR keeps the old uninstaller running in place. An NSIS uninstaller
 ; normally copies itself to %TEMP% and runs from there so that it can delete its
 ; own directory, and the process we started exits at once. ExecWait then returns
-; before the uninstall has run, and the temp copy's "RMDir /r $INSTDIR" executes
-; while the section below is still writing the new payload. _?= disables the copy
-; and makes ExecWait block until the uninstall finishes. It must be the last
-; argument and must not be quoted, even when the path contains spaces.
+; before the uninstall has run, and the temp copy keeps deleting files - from
+; 3.1.0 and 3.1.1 on disk a blanket "RMDir /r $INSTDIR" - while the section
+; below is still writing the new payload. _?= disables the copy and makes
+; ExecWait block until the uninstall finishes. It must be the last argument
+; and must not be quoted, even when the path contains spaces.
 ;
 ; --- 32-bit uninstall ---
 SetRegView 32
@@ -1107,9 +1108,14 @@ ${AndIf} ${FileExists} "$INSTDIR\portable.dat"
 	; upgrade portable mode
 	!insertmacro CockLog "section: portable upgrade purge"
 	!insertmacro CockLogListing "before-portable-purge" "*.dll"
+	; plugins and translations are payload and are rewritten by the payload
+	; below, so removing them here is safe. sounds and themes are payload
+	; too - the default packs are rewritten below as well - but a portable
+	; user's custom sounds and themes live in exactly those folders, so a
+	; recursive purge here would delete user content along with the defaults.
+	; Stale default files from an older release are overwritten or replaced
+	; by the payload, never by a purge.
 	RMDir /r "$INSTDIR\plugins"
-	RMDir /r "$INSTDIR\sounds"
-	RMDir /r "$INSTDIR\themes"
 	RMDir /r "$INSTDIR\translations"
 	Delete "$INSTDIR\uninstall.exe"
 	Delete "$INSTDIR\cockatrice.exe"
@@ -1263,16 +1269,32 @@ Section "un.Application" UnSecApplication
 	SetShellVarContext all
 	StrCpy $R1 0
 
-	; Record whether this was a portable install before the directory (and the
-	; portable.dat marker inside it) is removed below.
+	; Record whether this was a portable install before portable.dat is
+	; removed below.
 	IfFileExists "$INSTDIR\portable.dat" 0 +2
 		StrCpy $R1 1
 
-	; Remove the entire application directory so any file that is not part of
-	; the installed payload (e.g. build-tree artifacts such as *.dir folders,
-	; *_autogen and *.tlog files from a build) cannot survive between an
-	; uninstall and a fresh reinstall.
-	RMDir /r "$INSTDIR"
+	; Remove only what the installer wrote. The two lists below are generated
+	; by CPack from the staged install tree: one Delete per installed file and
+	; one non-recursive RMDir per installed directory, children first.
+	; Anything the user put in $INSTDIR - decks, pictures, portable data
+	; folders - is not on those lists and survives, and $INSTDIR itself is
+	; only removed when nothing else is left in it. An earlier version of this
+	; section ran a blanket "RMDir /r $INSTDIR" to keep build-tree artifacts
+	; out of a fresh reinstall; that also deleted user decks whenever a user
+	; had pointed their data folders into the install directory, so payload
+	; purity may never be bought at that price again.
+@CPACK_NSIS_DELETE_FILES@
+@CPACK_NSIS_DELETE_DIRECTORIES@
+
+	; Written by the installer itself, so absent from the staged payload
+	; above; qdebug.txt is app-generated log output next to the executables.
+	Delete "$INSTDIR\uninstall.exe"
+	Delete "$INSTDIR\portable.dat"
+	Delete "$INSTDIR\qdebug.txt"
+
+	; Succeeds only when no foreign files remain.
+	RMDir "$INSTDIR"
 
 	RMDir /r "$SMPROGRAMS\Cockatrice"
 

cmake/Sanitizers.cmake

@@ -0,0 +1,109 @@
+# AddressSanitizer + UndefinedBehaviorSanitizer opt-in, plus per-source opt-out
+# Input: ENABLE_SANITIZERS
+# Defines: cockatrice_disable_sanitizers_for
+#
+# Included once from the root CMakeLists.txt, immediately after the per-compiler flag
+# block, so add_compile_options() reaches every library and executable defined below.
+
+include(CheckCXXSourceCompiles)
+
+# Opt a set of vendored sources out of instrumentation.
+#
+# cockatrice_disable_sanitizers_for(<source> ...)
+#
+# Sanitizer findings inside vendored code are not actionable for us, and letting them
+# through buries our own bugs in noise until everyone learns to skip the job.
+#
+# The sources must be named relative to the directory that compiles them. CMake resolves a
+# source-file property against the directory scope the target was declared in, so calling
+# this from the root with absolute paths silently does nothing. That is why the call sites
+# live next to the add_library/add_executable that pull the vendored code in.
+#
+# SKIP_PRECOMPILE_HEADERS is not optional. The precompiled header is built with the
+# sanitizer flags, so it defines __SANITIZE_ADDRESS__; a translation unit compiled with
+# -fno-sanitize=all does not, and GCC then refuses the PCH with
+# "not used because __SANITIZE_ADDRESS__ not defined", which is a fatal -Werror=invalid-pch
+# in the servatrice and oracle builds.
+function(cockatrice_disable_sanitizers_for)
+  if(NOT ENABLE_SANITIZERS)
+    return()
+  endif()
+  set_source_files_properties(${ARGN} PROPERTIES COMPILE_OPTIONS "-fno-sanitize=all" SKIP_PRECOMPILE_HEADERS ON)
+endfunction()
+
+if(ENABLE_SANITIZERS)
+  # Probe a flag the way it will actually be used: on the compile step AND the link step.
+  # check_cxx_compiler_flag() only probes the compile step, which makes every sanitizer
+  # flag look unsupported because linking an ASan object without -fsanitize=address leaves
+  # __asan_init undefined. That is a silent misconfiguration rather than an error, so the
+  # probe has to link.
+  function(cockatrice_probe_flag FLAG VARNAME)
+    set(CMAKE_REQUIRED_FLAGS "${FLAG}")
+    set(CMAKE_REQUIRED_LINK_OPTIONS "${FLAG}")
+    check_cxx_source_compiles("int main() { return 0; }" ${VARNAME})
+    unset(CMAKE_REQUIRED_FLAGS)
+    unset(CMAKE_REQUIRED_LINK_OPTIONS)
+  endfunction()
+
+  set(SANITIZER_FLAGS
+      -fsanitize=address,undefined
+      -fno-omit-frame-pointer
+      # UBSan defaults to recover mode: it prints "runtime error: ..." but the
+      # process exits 0, so gtest/ctest report a pass and CI stays green.
+      # -fno-sanitize-recover makes the first finding abort instead. This is
+      # probed like the other cosmetic flags, but CI also sets
+      # UBSAN_OPTIONS=halt_on_error=1 as the runtime-side guarantee.
+      -fno-sanitize-recover=undefined
+      # Qt builds QObject subclasses into raw storage and writes the vptr afterwards, which
+      # the vptr check reports on essentially every QObject instantiation. The check has no
+      # value for us because nothing outside moc-generated code inherits QObject, so it is
+      # off outright rather than suppressed piecemeal.
+      -fno-sanitize=vptr
+  )
+
+  # MSVC has no ASan/UBSan equivalent, and clang-cl would need /fsanitize=address plus a
+  # matching runtime link, which is a separate piece of work.
+  if(MSVC)
+    message(FATAL_ERROR "ENABLE_SANITIZERS is not supported with MSVC")
+  endif()
+
+  # Probe rather than assume, so an older toolchain gets the sanitizers it can support
+  # instead of failing the build on a flag it does not recognize.
+  set(SANITIZER_SUPPORTED_FLAGS)
+  foreach(FLAG ${SANITIZER_FLAGS})
+    string(MAKE_C_IDENTIFIER "COCKATRICE_HAS${FLAG}" _FLAG_VAR)
+    cockatrice_probe_flag("${FLAG}" ${_FLAG_VAR})
+    if(${_FLAG_VAR})
+      list(APPEND SANITIZER_SUPPORTED_FLAGS ${FLAG})
+    else()
+      message(STATUS "Sanitizers: ${FLAG} unsupported by ${CMAKE_CXX_COMPILER_ID}, skipping")
+    endif()
+  endforeach()
+
+  # The whole point of the option is the sanitizers, so an unsupported pair is fatal even
+  # though the cosmetic flags above are allowed to degrade.
+  if(NOT "-fsanitize=address,undefined" IN_LIST SANITIZER_SUPPORTED_FLAGS)
+    message(FATAL_ERROR "ENABLE_SANITIZERS requires -fsanitize=address,undefined, which "
+                        "${CMAKE_CXX_COMPILER_ID} does not accept"
+    )
+  endif()
+
+  add_compile_options(${SANITIZER_SUPPORTED_FLAGS})
+  add_link_options(-fsanitize=address,undefined)
+
+  # Recorded so the test tree can label its entries and a plain `make` at the same time
+  # cannot quietly disagree with what CI ran.
+  set(COCKATRICE_SANITIZERS
+      ON
+      CACHE INTERNAL "AddressSanitizer and UBSan are enabled"
+  )
+else()
+  # ENABLE_SANITIZERS is OFF, but COCKATRICE_SANITIZERS is CACHE INTERNAL, so a
+  # reused build dir (e.g. CI's release-package step reconfiguring the same dir
+  # after the sanitized debug step) would otherwise keep skipping NO_SANITIZE
+  # tests and keep the 300s discovery timeout. Reset it explicitly.
+  set(COCKATRICE_SANITIZERS
+      OFF
+      CACHE INTERNAL "AddressSanitizer and UBSan are enabled" FORCE
+  )
+endif()

cmake/pch/qtcore_pch.h

@@ -3,22 +3,53 @@
  *
  * Safe for every target that links Qt Core, including the headless
  * Servatrice binary. Keep this header free of any widget/gui types.
+ *
+ * Candidates are chosen from IWYU include statistics: a header earns a
+ * spot when it is not already reachable from the rest of this list and
+ * enough translation units re-parse it. Qt headers are stable across
+ * builds, so the precompiled header rarely invalidates.
  */
 
 #include <QBasicTimer>
 #include <QByteArray>
+#include <QCommandLineParser>
+#include <QCoreApplication>
+#include <QCryptographicHash>
 #include <QDateTime>
 #include <QDebug>
+#include <QDir>
+#include <QElapsedTimer>
 #include <QFile>
+#include <QFileInfo>
 #include <QHash>
+#include <QItemSelectionModel>
+#include <QJsonArray>
+#include <QJsonDocument>
+#include <QJsonObject>
+#include <QJsonValue>
 #include <QList>
 #include <QLoggingCategory>
 #include <QMap>
+#include <QMetaEnum>
 #include <QMetaObject>
+#include <QMutex>
+#include <QMutexLocker>
 #include <QObject>
+#include <QPointer>
+#include <QPropertyAnimation>
 #include <QRandomGenerator>
+#include <QReadWriteLock>
+#include <QRegularExpression>
+#include <QSet>
+#include <QSettings>
 #include <QSharedPointer>
+#include <QSortFilterProxyModel>
 #include <QString>
 #include <QStringList>
+#include <QTemporaryDir>
+#include <QThread>
 #include <QTimer>
+#include <QUuid>
 #include <QVariant>
+#include <QXmlStreamReader>
+#include <QXmlStreamWriter>

cmake/pch/qtwidgets_pch.h

@@ -4,27 +4,56 @@
  * Includes the Qt Core precompiled header plus the heavy Gui, Widgets and
  * Network layers that virtually every client translation unit re-parses.
  * Do not use on Servatrice (headless, QT_DONT_USE_QTGUI).
+ *
+ * Candidates are chosen from IWYU include statistics: a header earns a
+ * spot when it is not already reachable from the rest of this list and
+ * enough translation units re-parse it. Qt headers are stable across
+ * builds, so the precompiled header rarely invalidates.
  */
 
 #include "qtcore_pch.h"
 
 #include <QAction>
 #include <QApplication>
+#include <QCheckBox>
+#include <QComboBox>
+#include <QCompleter>
+#include <QDialogButtonBox>
+#include <QDockWidget>
+#include <QFileDialog>
+#include <QFormLayout>
 #include <QFrame>
 #include <QGraphicsItem>
 #include <QGraphicsScene>
+#include <QGraphicsSceneMouseEvent>
 #include <QGraphicsView>
+#include <QGridLayout>
+#include <QGroupBox>
+#include <QHBoxLayout>
+#include <QHeaderView>
 #include <QImage>
+#include <QImageReader>
+#include <QInputDialog>
 #include <QLabel>
 #include <QLayout>
+#include <QListWidget>
 #include <QMainWindow>
 #include <QMenu>
+#include <QMessageBox>
 #include <QNetworkAccessManager>
 #include <QNetworkReply>
 #include <QPainter>
+#include <QPlainTextEdit>
 #include <QPushButton>
 #include <QScrollArea>
+#include <QSpinBox>
+#include <QSplitter>
+#include <QStandardItemModel>
 #include <QTabWidget>
+#include <QTableWidget>
+#include <QTableWidgetItem>
 #include <QToolBar>
+#include <QToolButton>
 #include <QTreeWidget>
+#include <QVBoxLayout>
 #include <QWidget>

cockatrice/CMakeLists.txt

@@ -59,6 +59,7 @@ set(cockatrice_SOURCES
     src/interface/widgets/dialogs/dlg_update.cpp
     src/interface/widgets/dialogs/dlg_view_log.cpp
     src/interface/widgets/dialogs/override_printing_warning.cpp
+    src/interface/widgets/dialogs/path_safety_warning.cpp
     src/interface/widgets/dialogs/tip_of_the_day.cpp
     src/interface/widgets/deck_share/deck_share_utils.cpp
     src/interface/widgets/deck_share/shared_deck_preview_widget.cpp

cockatrice/src/game/game_event_handler.cpp

@@ -277,8 +277,6 @@ void GameEventHandler::eventSpectatorLeave(const Event_Leave &event,
     emit spectatorLeft(eventPlayerId);
 
     game->getPlayerManager()->removeSpectator(eventPlayerId);
-
-    emitUserEvent();
 }
 
 void GameEventHandler::eventGameStateChanged(const Event_GameStateChanged &event,
@@ -465,9 +463,9 @@ void GameEventHandler::eventJoin(const Event_Join &event, int /*eventPlayerId*/,
         PlayerLogic *newPlayer = game->getPlayerManager()->addPlayer(playerId, playerInfo.user_info());
         emit logJoinPlayer(newPlayer);
         emit playerJoined(playerInfo);
-    }
 
-    emitUserEvent();
+        emitUserEvent();
+    }
 }
 
 QString GameEventHandler::getLeaveReason(Event_Leave::LeaveReason reason)

cockatrice/src/interface/widgets/dialogs/path_safety_warning.cpp

@@ -0,0 +1,34 @@
+#include "path_safety_warning.h"
+
+#include "../../client/settings/cache_settings.h"
+
+#include <QCoreApplication>
+#include <QMessageBox>
+#include <QObject>
+#include <QString>
+#include <libcockatrice/settings/paths_settings.h>
+
+class QWidget;
+
+bool PathSafetyWarning::confirmOutsideProgramDir(QWidget *parent, const QString &path)
+{
+    if (SettingsCache::instance().getIsPortableBuild()) {
+        return true;
+    }
+
+    if (!PathsSettings::isInsideDir(path, QCoreApplication::applicationDirPath())) {
+        return true;
+    }
+
+    QMessageBox box(QMessageBox::Warning, QObject::tr("Path inside the program directory"),
+                    QObject::tr("'%1' is inside the Cockatrice program directory.\n\n"
+                                "Files stored there can be deleted by an update or by uninstalling "
+                                "Cockatrice, so this change was not applied. Choose a location outside "
+                                "the program directory.")
+                        .arg(path),
+                    QMessageBox::Ok, parent);
+    box.setTextFormat(Qt::PlainText);
+    box.exec();
+
+    return false;
+}

cockatrice/src/interface/widgets/dialogs/path_safety_warning.h

@@ -0,0 +1,25 @@
+#ifndef COCKATRICE_PATH_SAFETY_WARNING_H
+#define COCKATRICE_PATH_SAFETY_WARNING_H
+
+class QString;
+class QWidget;
+
+namespace PathSafetyWarning
+{
+
+/**
+ * @brief Warns when the user tries to configure a data path inside the program directory.
+ *
+ * Shows a message box explaining that files kept there can be deleted by an
+ * update or by uninstalling Cockatrice. Portable builds are always exempt:
+ * their data lives beside the executables by design.
+ *
+ * @param parent The parent widget
+ * @param path The path the user is trying to configure
+ * @return true when the path is outside the program directory and safe to apply
+ */
+bool confirmOutsideProgramDir(QWidget *parent, const QString &path);
+
+} // namespace PathSafetyWarning
+
+#endif // COCKATRICE_PATH_SAFETY_WARNING_H

cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp

@@ -2,6 +2,7 @@
 
 #include "../../../client/settings/cache_settings.h"
 #include "../../pixel_map_generator.h"
+#include "../dialogs/path_safety_warning.h"
 #include "update/card_spoiler/spoiler_background_updater.h"
 
 #include <QAbstractItemModel>
@@ -363,6 +364,9 @@ void DeckEditorSettingsPage::spoilerPathButtonClicked()
     if (lsPath.isEmpty()) {
         return;
     }
+    if (!PathSafetyWarning::confirmOutsideProgramDir(this, lsPath + "/spoiler.xml")) {
+        return;
+    }
 
     mpSpoilerSavePathLineEdit->setText(lsPath + "/spoiler.xml");
     SettingsCache::instance().paths().setSpoilerDatabasePath(lsPath + "/spoiler.xml");

cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp

@@ -1,6 +1,7 @@
 #include "general_settings_page.h"
 
 #include "../../../client/settings/cache_settings.h"
+#include "../dialogs/path_safety_warning.h"
 #include "../interface/card_picture_loader/card_picture_loader.h"
 #include "../main.h"
 #include "../server/user/user_info_connection.h"
@@ -378,6 +379,9 @@ void GeneralSettingsPage::deckPathButtonClicked()
     if (path.isEmpty()) {
         return;
     }
+    if (!PathSafetyWarning::confirmOutsideProgramDir(this, path)) {
+        return;
+    }
 
     deckPathEdit->setText(path);
     SettingsCache::instance().paths().setDeckPath(path);
@@ -389,6 +393,9 @@ void GeneralSettingsPage::filtersPathButtonClicked()
     if (path.isEmpty()) {
         return;
     }
+    if (!PathSafetyWarning::confirmOutsideProgramDir(this, path)) {
+        return;
+    }
 
     filtersPathEdit->setText(path);
     SettingsCache::instance().paths().setFiltersPath(path);
@@ -400,6 +407,9 @@ void GeneralSettingsPage::replaysPathButtonClicked()
     if (path.isEmpty()) {
         return;
     }
+    if (!PathSafetyWarning::confirmOutsideProgramDir(this, path)) {
+        return;
+    }
 
     replaysPathEdit->setText(path);
     SettingsCache::instance().paths().setReplaysPath(path);
@@ -411,6 +421,9 @@ void GeneralSettingsPage::picsPathButtonClicked()
     if (path.isEmpty()) {
         return;
     }
+    if (!PathSafetyWarning::confirmOutsideProgramDir(this, path)) {
+        return;
+    }
 
     picsPathEdit->setText(path);
     SettingsCache::instance().paths().setPicsPath(path);
@@ -422,6 +435,9 @@ void GeneralSettingsPage::cardDatabasePathButtonClicked()
     if (path.isEmpty()) {
         return;
     }
+    if (!PathSafetyWarning::confirmOutsideProgramDir(this, path)) {
+        return;
+    }
 
     cardDatabasePathEdit->setText(path);
     SettingsCache::instance().paths().setCardDatabasePath(path);
@@ -433,6 +449,9 @@ void GeneralSettingsPage::customCardDatabaseButtonClicked()
     if (path.isEmpty()) {
         return;
     }
+    if (!PathSafetyWarning::confirmOutsideProgramDir(this, path)) {
+        return;
+    }
 
     customCardDatabasePathEdit->setText(path);
     SettingsCache::instance().paths().setCustomCardDatabasePath(path);
@@ -444,6 +463,9 @@ void GeneralSettingsPage::tokenDatabasePathButtonClicked()
     if (path.isEmpty()) {
         return;
     }
+    if (!PathSafetyWarning::confirmOutsideProgramDir(this, path)) {
+        return;
+    }
 
     tokenDatabasePathEdit->setText(path);
     SettingsCache::instance().paths().setTokenDatabasePath(path);

cockatrice/src/interface/widgets/tabs/tab_game.cpp

@@ -1060,6 +1060,12 @@ void TabGame::setCardMenu(CardMenu *menu)
         return;
     }
 
+    if (aCardMenu->menu() && aCardMenu->menu()->isVisible()) {
+        aCardMenu->menu()->hide();
+        if (gameView && gameView->viewport()) {
+            gameView->viewport()->update();
+        }
+    }
     if (menu) {
         aCardMenu->setMenu(menu);
     } else {

cockatrice/src/interface/widgets/tabs/tab_report.cpp

@@ -1,5 +1,6 @@
 #include "tab_report.h"
 
+#include "../server/user/user_context_menu.h"
 #include "../utility/report_utils.h"
 #include "abstract_client.h"
 #include "libcockatrice/protocol/pb/response.pb.h"
@@ -33,11 +34,13 @@
 #include <QVariant>
 #include <QWidget>
 #include <QtGlobal>
+#include <libcockatrice/network/server/remote/user_level.h>
 #include <libcockatrice/protocol/pb/command_replay_download_by_game_id.pb.h>
 #include <libcockatrice/protocol/pb/command_report_add_comment.pb.h>
 #include <libcockatrice/protocol/pb/command_report_assign.pb.h>
 #include <libcockatrice/protocol/pb/command_report_details.pb.h>
 #include <libcockatrice/protocol/pb/command_report_list.pb.h>
+#include <libcockatrice/protocol/pb/command_report_reopen.pb.h>
 #include <libcockatrice/protocol/pb/command_report_resolve.pb.h>
 #include <libcockatrice/protocol/pb/command_report_stats.pb.h>
 #include <libcockatrice/protocol/pb/command_report_user_info.pb.h>
@@ -120,6 +123,10 @@ TabReport::TabReport(TabSupervisor *_tabSupervisor, AbstractClient *_client) : T
     table->horizontalHeader()->setSectionResizeMode(COL_REPORTED, QHeaderView::Stretch);
     table->horizontalHeader()->setSectionResizeMode(COL_REPORTER, QHeaderView::ResizeToContents);
     connect(table, &QTableWidget::itemSelectionChanged, this, &TabReport::onSelectionChanged);
+    table->setContextMenuPolicy(Qt::CustomContextMenu);
+    connect(table, &QTableWidget::customContextMenuRequested, this, &TabReport::showTableContextMenu);
+    userContextMenu = new UserContextMenu(tabSupervisor, this);
+    connect(userContextMenu, &UserContextMenu::openMessageDialog, this, &TabReport::openMessageDialog);
 
     descGroup = new QGroupBox;
     descriptionEdit = new QTextEdit;
@@ -231,14 +238,14 @@ TabReport::TabReport(TabSupervisor *_tabSupervisor, AbstractClient *_client) : T
 
     assignButton = new QPushButton;
     resolveButton = new QPushButton;
-    resolveWithNoteButton = new QPushButton;
     dismissButton = new QPushButton;
+    reopenButton = new QPushButton;
     viewReplayButton = new QPushButton;
     joinGameButton = new QPushButton;
     connect(assignButton, &QPushButton::clicked, this, &TabReport::assignReport);
-    connect(resolveButton, &QPushButton::clicked, this, [this]() { resolveReport(false, false); });
-    connect(resolveWithNoteButton, &QPushButton::clicked, this, [this]() { resolveReport(false, true); });
-    connect(dismissButton, &QPushButton::clicked, this, [this]() { resolveReport(true, true); });
+    connect(resolveButton, &QPushButton::clicked, this, [this]() { resolveReport(false); });
+    connect(dismissButton, &QPushButton::clicked, this, [this]() { resolveReport(true); });
+    connect(reopenButton, &QPushButton::clicked, this, &TabReport::reopenReport);
     connect(viewReplayButton, &QPushButton::clicked, this, &TabReport::viewReplay);
     connect(joinGameButton, &QPushButton::clicked, this, &TabReport::joinGame);
 
@@ -247,8 +254,8 @@ TabReport::TabReport(TabSupervisor *_tabSupervisor, AbstractClient *_client) : T
     auto *actionBar = new QHBoxLayout;
     actionBar->addWidget(assignButton);
     actionBar->addWidget(resolveButton);
-    actionBar->addWidget(resolveWithNoteButton);
     actionBar->addWidget(dismissButton);
+    actionBar->addWidget(reopenButton);
     actionBar->addSpacing(20);
     actionBar->addWidget(viewReplayButton);
     actionBar->addWidget(joinGameButton);
@@ -272,6 +279,7 @@ TabReport::TabReport(TabSupervisor *_tabSupervisor, AbstractClient *_client) : T
 
 void TabReport::retranslateUi()
 {
+    userContextMenu->retranslateUi();
     searchEdit->setPlaceholderText(tr("Search by username, category..."));
     statusFilter->clear();
     statusFilter->addItem(tr("All Statuses"), "");
@@ -312,9 +320,10 @@ void TabReport::retranslateUi()
     statsGroup->setTitle(tr("Report Statistics"));
 
     assignButton->setText(tr("Assign to Me"));
-    resolveButton->setText(tr("Resolve"));
-    resolveWithNoteButton->setText(tr("Resolve with note..."));
+    resolveButton->setText(tr("Resolve..."));
     dismissButton->setText(tr("Dismiss..."));
+    reopenButton->setText(tr("Reopen..."));
+    reopenButton->setToolTip(tr("Available for resolved or dismissed reports."));
     viewReplayButton->setText(tr("View Replay"));
     joinGameButton->setText(tr("Join Game"));
 }
@@ -346,6 +355,7 @@ void TabReport::reportListResponse(const Response &response)
     refreshButton->setEnabled(true);
 
     if (response.response_code() != Response::RespOk) {
+        pendingStatusMessage.clear();
         statusLabel->setText(tr("Failed to load reports."));
         return;
     }
@@ -359,6 +369,11 @@ void TabReport::reportListResponse(const Response &response)
     applyFilters();
     updateStats();
 
+    if (!pendingStatusMessage.isEmpty()) {
+        statusLabel->setText(pendingStatusMessage);
+        pendingStatusMessage.clear();
+    }
+
     if (selectedReportIdBeforeRefresh >= 0) {
         bool found = false;
         for (int row = 0; row < table->rowCount(); ++row) {
@@ -494,6 +509,36 @@ void TabReport::onSelectionChanged()
     updateActionStates();
 }
 
+void TabReport::showTableContextMenu(const QPoint &pos)
+{
+    QTableWidgetItem *clickedItem = table->itemAt(pos);
+    if (!clickedItem || clickedItem->row() < 0) {
+        return;
+    }
+
+    const int row = clickedItem->row();
+    const QTableWidgetItem *reportedItem = table->item(row, COL_REPORTED);
+    const QTableWidgetItem *idItem = table->item(row, COL_ID);
+    if (!reportedItem || !idItem || reportedItem->text().isEmpty()) {
+        return;
+    }
+
+    int userLevel = ServerInfo_User::IsUser;
+    const int reportId = idItem->data(Qt::UserRole).toInt();
+    for (const ServerInfo_Report &r : filteredReports) {
+        if (r.report_id() == reportId) {
+            if (r.reported_user_level() != 0) {
+                userLevel = r.reported_user_level();
+            }
+            break;
+        }
+    }
+
+    table->selectRow(row);
+    userContextMenu->showContextMenu(table->viewport()->mapToGlobal(pos), reportedItem->text(),
+                                     UserLevelFlags(userLevel));
+}
+
 void TabReport::loadReportDetails(int reportId)
 {
     detailsRequestedReportId = reportId;
@@ -577,8 +622,8 @@ void TabReport::updateActionStates()
     const QString status = QString::fromStdString(report.status());
     assignButton->setEnabled(status == "open");
     resolveButton->setEnabled(status == "open" || status == "assigned");
-    resolveWithNoteButton->setEnabled(status == "open" || status == "assigned");
     dismissButton->setEnabled(status == "open" || status == "assigned");
+    reopenButton->setEnabled(status == "resolved" || status == "dismissed");
 
     const bool hasGameId = report.game_id() > 0;
     const bool hasReplay = hasGameId && report.has_replay_id() && report.replay_id() > 0;
@@ -590,8 +635,8 @@ void TabReport::setActionsEnabled(bool enabled)
 {
     assignButton->setEnabled(enabled);
     resolveButton->setEnabled(enabled);
-    resolveWithNoteButton->setEnabled(enabled);
     dismissButton->setEnabled(enabled);
+    reopenButton->setEnabled(enabled);
     viewReplayButton->setEnabled(false);
     joinGameButton->setEnabled(false);
     commentButton->setEnabled(enabled);
@@ -645,7 +690,7 @@ void TabReport::assignReport()
 void TabReport::assignResponse(const Response &response)
 {
     if (response.response_code() == Response::RespOk) {
-        statusLabel->setText(tr("Assigned."));
+        pendingStatusMessage = tr("Assigned.");
         refreshList();
     } else {
         statusLabel->setText(tr("Assignment failed."));
@@ -653,22 +698,19 @@ void TabReport::assignResponse(const Response &response)
     }
 }
 
-void TabReport::resolveReport(bool dismissed, bool promptNote)
+void TabReport::resolveReport(bool dismissed)
 {
     const int reportId = selectedReportId();
     if (reportId < 0) {
         return;
     }
 
-    QString note;
-    if (promptNote) {
-        bool ok;
-        note = QInputDialog::getText(this, dismissed ? tr("Dismiss Report") : tr("Resolve Report"),
-                                     dismissed ? tr("Optional note:") : tr("Resolution note (optional):"),
-                                     QLineEdit::Normal, QString(), &ok);
-        if (!ok) {
-            return;
-        }
+    bool ok;
+    QString note = QInputDialog::getText(this, dismissed ? tr("Dismiss Report") : tr("Resolve Report"),
+                                         dismissed ? tr("Optional note:") : tr("Resolution note (optional):"),
+                                         QLineEdit::Normal, QString(), &ok);
+    if (!ok) {
+        return;
     }
 
     setActionsEnabled(false);
@@ -689,7 +731,46 @@ void TabReport::resolveReport(bool dismissed, bool promptNote)
 void TabReport::resolveResponse(const Response &response)
 {
     if (response.response_code() == Response::RespOk) {
-        statusLabel->setText(tr("Done."));
+        pendingStatusMessage = tr("Done.");
+        refreshList();
+    } else {
+        statusLabel->setText(tr("Action failed."));
+        updateActionStates();
+    }
+}
+
+void TabReport::reopenReport()
+{
+    const int reportId = selectedReportId();
+    if (reportId < 0) {
+        return;
+    }
+
+    bool ok;
+    QString note = QInputDialog::getText(this, tr("Reopen Report"), tr("Reopening note (optional):"), QLineEdit::Normal,
+                                         QString(), &ok);
+    if (!ok) {
+        return;
+    }
+
+    setActionsEnabled(false);
+    statusLabel->setText(tr("Reopening..."));
+
+    Command_ReportReopen cmd;
+    cmd.set_report_id(reportId);
+    if (!note.isEmpty()) {
+        cmd.set_reopen_note(note.toStdString());
+    }
+
+    PendingCommand *pend = client->prepareModeratorCommand(cmd);
+    connect(pend, &PendingCommand::finished, this, &TabReport::reopenResponse);
+    client->sendCommand(pend);
+}
+
+void TabReport::reopenResponse(const Response &response)
+{
+    if (response.response_code() == Response::RespOk) {
+        pendingStatusMessage = tr("Reopened.");
         refreshList();
     } else {
         statusLabel->setText(tr("Action failed."));

cockatrice/src/interface/widgets/tabs/tab_report.h

@@ -4,6 +4,7 @@
 #include "tab.h"
 
 #include <QList>
+#include <QPoint>
 #include <QString>
 #include <libcockatrice/protocol/pb/serverinfo_report.pb.h>
 #include <qtmetamacros.h>
@@ -22,6 +23,7 @@ class QTimer;
 class GameReplay;
 class Response;
 class TabSupervisor;
+class UserContextMenu;
 
 class TabReport : public Tab
 {
@@ -37,15 +39,17 @@ public:
 signals:
     void openReplay(GameReplay *replay);
     void requestJoinGame(int gameId, int roomId);
+    void openMessageDialog(const QString &userName, bool focus);
 
 private slots:
     void refreshList();
     void reportListResponse(const Response &response);
     void assignReport();
     void assignResponse(const Response &response);
-    void resolveReport(bool dismissed, bool promptNote);
+    void resolveReport(bool dismissed);
     void resolveResponse(const Response &response);
     void onSelectionChanged();
+    void showTableContextMenu(const QPoint &pos);
     void viewReplay();
     void viewReplayResponse(const Response &response);
     void joinGame();
@@ -56,6 +60,8 @@ private slots:
     void reportDetailsResponse(const Response &response);
     void requestStats();
     void statsResponse(const Response &response);
+    void reopenReport();
+    void reopenResponse(const Response &response);
 
 private:
     int selectedReportId() const;
@@ -85,12 +91,13 @@ private:
     QPushButton *commentButton;
     QPushButton *assignButton;
     QPushButton *resolveButton;
-    QPushButton *resolveWithNoteButton;
     QPushButton *dismissButton;
+    QPushButton *reopenButton;
     QPushButton *viewReplayButton;
     QPushButton *joinGameButton;
     QLabel *statusLabel;
     QTimer *refreshTimer;
+    UserContextMenu *userContextMenu;
 
     QGroupBox *userContextGroup;
     QLabel *userContextName;
@@ -108,6 +115,7 @@ private:
     QLabel *userContextNotesLabel;
     QLabel *userContextRecentReportsLabel;
     QString lastRequestedUser;
+    QString pendingStatusMessage;
     QGroupBox *statsGroup;
     QLabel *statsTotalLabel;
     QLabel *statsTrendLabel;

cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp

@@ -919,6 +919,7 @@ void TabSupervisor::openTabReport()
     myAddTab(tabReport, aTabReport);
     connect(tabReport, &TabReport::openReplay, this, &TabSupervisor::openReplay);
     connect(tabReport, &TabReport::requestJoinGame, this, &TabSupervisor::joinReportGame);
+    connect(tabReport, &TabReport::openMessageDialog, this, &TabSupervisor::addMessageTab);
     connect(tabReport, &QObject::destroyed, this, [this] {
         tabReport = nullptr;
         aTabReport->setChecked(false);

cockatrice/src/interface/widgets/utility/line_edit_completer.cpp

@@ -130,10 +130,19 @@ void LineEditCompleter::keyPressEvent(QKeyEvent *event)
     CompleterInfo *active = nullptr;
     QString prefix;
 
+    // An open "[[" card link takes precedence over a mention: while the cursor is
+    // inside it, an earlier "@" must not shadow the card completer.
+    const int openCardPos = textValue.lastIndexOf("[[", cursorPos - 1);
+    const int closeCardPos = openCardPos == -1 ? -1 : textValue.indexOf("]]", openCardPos + 2);
+    const bool insideCardLink = openCardPos != -1 && (closeCardPos == -1 || closeCardPos >= cursorPos);
+
     for (auto &info : completers) {
         bool triggered = false;
         switch (info.trigger) {
             case CompleterTrigger::Mention: {
+                if (insideCardLink) {
+                    break;
+                }
                 int triggerPos = textValue.lastIndexOf("@", cursorPos - 1);
                 if (triggerPos != -1 && (triggerPos == 0 || textValue[triggerPos - 1].isSpace())) {
                     triggered = true;

cockatrice/src/interface/widgets/utility/report_utils.cpp

@@ -100,10 +100,27 @@ void renderReportDetails(QTextEdit *chatLogEdit,
                          const QString &moderatorPrefix,
                          const QString &nonModeratorPrefix)
 {
+    QString details;
+    if (report.has_resolution_note() && !report.resolution_note().empty()) {
+        details = QObject::tr("Resolution note: %1").arg(QString::fromStdString(report.resolution_note()));
+    }
+    if (report.has_resolution_time()) {
+        QString resolvedBy = QObject::tr("unknown");
+        if (report.has_resolved_by_name() && !report.resolved_by_name().empty()) {
+            resolvedBy = QString::fromStdString(report.resolved_by_name());
+        }
+        if (!details.isEmpty()) {
+            details += "\n";
+        }
+        details += QObject::tr("Resolved at %1 by %2").arg(formatReportTime(report.resolution_time()), resolvedBy);
+    }
+
     if (report.has_chat_log() && !report.chat_log().empty()) {
-        chatLogEdit->setPlainText(QString::fromStdString(report.chat_log()));
+        const QString chatLog = QString::fromStdString(report.chat_log());
+        chatLogEdit->setPlainText(
+            details.isEmpty() ? chatLog : details + "\n\n" + QObject::tr("=== Chat Log ===") + "\n" + chatLog);
     } else {
-        chatLogEdit->clear();
+        chatLogEdit->setPlainText(details);
     }
 
     commentsEdit->clear();

cockatrice/src/interface/window_main.cpp

@@ -58,7 +58,9 @@
 #include <QAction>
 #include <QApplication>
 #include <QButtonGroup>
+#include <QCheckBox>
 #include <QCloseEvent>
+#include <QCoreApplication>
 #include <QDateTime>
 #include <QDebug>
 #include <QDesktopServices>
@@ -603,8 +605,39 @@ MainWindow::MainWindow(QWidget *parent)
     QTimer::singleShot(0, this, &MainWindow::applyStartupDestination);
 }
 
+void MainWindow::warnAboutDataPathsInProgramDir()
+{
+    auto &paths = SettingsCache::instance().paths();
+    if (SettingsCache::instance().getIsPortableBuild() || paths.getAppDirWarningAcknowledged()) {
+        return;
+    }
+
+    const QStringList offenders = paths.pathsInsideDir(QCoreApplication::applicationDirPath());
+    if (offenders.isEmpty()) {
+        return;
+    }
+
+    QMessageBox box(QMessageBox::Warning, tr("Data paths inside the program directory"),
+                    tr("These paths are configured inside the Cockatrice program directory:\n\n%1\n\n"
+                       "Cockatrice keeps its own data outside the program directory by default. Files "
+                       "stored inside it can be deleted by an update or by uninstalling Cockatrice. "
+                       "Change them in the Settings dialog - most are on the General page - to keep "
+                       "them safe.")
+                        .arg(offenders.join(QLatin1Char('\n'))),
+                    QMessageBox::Ok, this);
+    box.setTextFormat(Qt::PlainText);
+    auto *dontShowAgain = new QCheckBox(tr("Don't show this again"));
+    box.setCheckBox(dontShowAgain);
+    box.exec();
+    if (dontShowAgain->isChecked()) {
+        paths.setAppDirWarningAcknowledged(true);
+    }
+}
+
 void MainWindow::startupConfigCheck()
 {
+    warnAboutDataPathsInProgramDir();
+
     const bool isCleanInstall = SettingsCache::instance().network().getClientVersion() == CLIENT_INFO_NOT_SET;
 
     // checkUnknownSets() is intentionally deferred from the card database load

cockatrice/src/interface/window_main.h

@@ -149,6 +149,9 @@ private:
 
     void runFirstRunWizard(bool firstRun = false);
 
+    /** @brief Warns once when a non-portable install keeps its data paths inside the program directory. */
+    void warnAboutDataPathsInProgramDir();
+
     inline QString getCardUpdaterBinaryName()
     {
         return "oracle";

doc/doxygen/pages/user_documentation/deck_management/importing_decks.md

@@ -38,7 +38,18 @@ Selecting this action will open a new text editor dialog with the contents of yo
 
 The import dialog expects each line to be a card with the following format:
 
-TODO
+`[quantity] Card Name [options]`
+
+- `quantity` is the number of copies and can be written as `2`, `2x` or `2X`. If omitted, a single copy is imported.
+- `options` are appended after the card name and are understood on a best-effort basis:
+  - a set code and collector number, either as `Card Name (SET) 123` or `Card Name (SET) 123-1`
+  - a foil marker, `Card Name *F*`
+- Lines can also carry a `SB:` prefix or a set code prefix (`[SET]`) — Cockatrice tries to stay compatible with the most
+  common deck formats.
+
+Tab-separated exports from third-party apps (such as Delver Lens) are also accepted: any line whose first column is a
+plain quantity treats the following columns as the card name and printer/set metadata (which is discarded), for example
+`2\tCard Name\tSet Name`.
 
 Each card should be on a separate line and there should be no empty lines between cards. The first empty line between
 two blocks of cards will be considered as the divider between mainboard and sideboard.

gcovr.cfg

@@ -0,0 +1,63 @@
+# gcovr configuration for Cockatrice line coverage.
+#
+# Used by .ci/compile.sh --coverage and by hand:
+#
+#   cmake -S . -B build/cover -DCMAKE_BUILD_TYPE=Debug -DWITH_SERVER=1 -DTEST=1 -DENABLE_COVERAGE=ON
+#   cmake --build build/cover -j 24
+#   ctest --test-dir build/cover -LE bench -j 24
+#   gcovr build/cover --root . --object-directory build/cover --txt --decisions
+#
+# The point of most of this file is subtraction. A raw report is dominated by code no test can
+# reach: vendored third-party trees, build directories left over from earlier experiments, and
+# the client's main() and window classes, which have no headless test. Those all dilute the one
+# number worth watching, so they are filtered out here rather than in every invocation.
+
+# root is deliberately not set here. This file is normally invoked from the build directory
+# (compile.sh runs there, because it configures with `cmake ..`), so a `root` of `.` would
+# resolve to the build tree and every filter below would exclude everything. compile.sh passes
+# --root explicitly instead.
+
+# Our own code only. This keeps generated Qt MOC output, protobuf output and anything CMake
+# built under the source tree out of the report.
+filter = ^cockatrice/
+filter = ^libcockatrice_[^/]+/
+filter = ^servatrice/
+filter = ^oracle/
+
+# The tests are the vehicle, not the subject. They are compiled with the same instrumentation,
+# so leaving them in would add their near-perfect coverage to the numerator and quietly report a
+# healthier project than we have: test code is only ever executed by the run that is measuring
+# it. Helpers under tests/ are excluded on the same grounds.
+exclude = ^tests/
+
+# Vendored third-party code is not ours to cover and is never going to move the number.
+exclude = ^vcpkg/
+exclude = ^doc/doxygen/theme/
+exclude = ^libcockatrice_rng/libcockatrice/rng/sfmt/
+exclude = ^libcockatrice_utility/libcockatrice/utility/peglib\.h$
+exclude = ^oracle/src/lzma/
+exclude = ^oracle/src/qt-json/
+exclude = ^oracle/src/zip/
+exclude = ^servatrice/src/smtp/
+
+# Qt's own moc/pch/uic output is generated into the build tree, not the source tree, so the
+# filters above already keep it out. These cover CLion's directories, which sit in-tree and are
+# picked up by the object-directory walk.
+exclude = .*cmake-build-.*
+exclude = .*\.cache/
+
+# Process entry points and the client's window chrome. Each of these needs a display, an
+# interactive terminal, or a live database before it does anything a test could observe, so no
+# headless run will ever move them. Left in, they contribute thousands of lines at 0% and drag
+# the total down by a fixed amount that hides movement in the code we can actually test. The
+# window header is excluded with its .cpp because gcovr attributes its inline template code
+# there, which is where most of its line count lives.
+exclude = ^cockatrice/src/main\.cpp$
+exclude = ^cockatrice/src/interface/window_main\.
+exclude = ^oracle/src/main\.cpp$
+exclude = ^servatrice/src/main\.cpp$
+
+exclude-throw-branches = yes
+exclude-function-lines = yes
+
+print-summary = yes

libcockatrice_card/CMakeLists.txt

@@ -60,3 +60,5 @@ target_link_libraries(
   PUBLIC libcockatrice_utility
   PUBLIC ${QT_CORE_MODULE}
 )
+
+target_precompile_headers(libcockatrice_card PRIVATE "${CMAKE_SOURCE_DIR}/cmake/pch/qtcore_pch.h")

libcockatrice_deck_list/CMakeLists.txt

@@ -41,3 +41,5 @@ target_link_libraries(
   libcockatrice_deck_list PUBLIC libcockatrice_interfaces libcockatrice_protocol libcockatrice_utility
                                  ${QT_CORE_MODULE}
 )
+
+target_precompile_headers(libcockatrice_deck_list PRIVATE "${CMAKE_SOURCE_DIR}/cmake/pch/qtcore_pch.h")

libcockatrice_deck_list/libcockatrice/deck_list/deck_list_plain_text_parser.cpp

@@ -33,6 +33,7 @@ bool parse(QTextStream &in,
 
     // Regex for advanced card parsing
     static const QRegularExpression reMultiplier(R"(^[xX\(\[]*(\d+)[xX\*\)\]]* ?(.+))");
+    static const QRegularExpression reTabQuantity(R"(^\d+[xX]?$)");
 
     // Regex for extracting set code and collector number with attached symbols
     static const QRegularExpression reHyphenFormat(R"(\((\w{3,})\)\s+(\w{3,})-(\d+[^\w\s]*))");
@@ -110,8 +111,34 @@ bool parse(QTextStream &in,
             continue;
         }
 
-        QString cardName = match.captured().simplified();
+        const QString rawLine = match.captured();
+        QString cardName = rawLine.simplified();
         bool sideboard = false;
+        int amount = 1;
+
+        // Card names never contain tab characters, so a leading numeric field
+        // marks a tabular record emitted by third-party deck tools (e.g. Delver
+        // Lens "2\tCard Name\tSet Name"). The first two columns are the quantity
+        // and the card name; any further columns are printer/set metadata and
+        // are discarded.
+        bool hasTabQuantity = false;
+        const auto tabFields = rawLine.split('\t');
+        int firstField = 0;
+        while (firstField < tabFields.size() && tabFields.at(firstField).trimmed().isEmpty()) {
+            ++firstField;
+        }
+        if (firstField + 1 < tabFields.size()) {
+            const QString quantity = tabFields.at(firstField).trimmed();
+            if (reTabQuantity.match(quantity).hasMatch()) {
+                hasTabQuantity = true;
+                QString digits = quantity;
+                if (digits.endsWith('x') || digits.endsWith('X')) {
+                    digits.chop(1);
+                }
+                amount = digits.toInt();
+                cardName = tabFields.at(firstField + 1).simplified();
+            }
+        }
 
         // Sideboard detection
         if (sBStart < 0) {
@@ -156,11 +183,12 @@ bool parse(QTextStream &in,
         }
 
         // check if a specific amount is mentioned
-        int amount = 1;
-        match = reMultiplier.match(cardName);
-        if (match.hasMatch()) {
-            amount = match.captured(1).toInt();
-            cardName = match.captured(2);
+        if (!hasTabQuantity) {
+            match = reMultiplier.match(cardName);
+            if (match.hasMatch()) {
+                amount = match.captured(1).toInt();
+                cardName = match.captured(2);
+            }
         }
 
         // Normalize the card name

libcockatrice_filters/CMakeLists.txt

@@ -18,3 +18,5 @@ add_dependencies(libcockatrice_filters libcockatrice_card)
 target_include_directories(libcockatrice_filters PUBLIC ${CMAKE_CURRENT_SOURCE_DIR})
 
 target_link_libraries(libcockatrice_filters PUBLIC libcockatrice_card ${QT_CORE_MODULE})
+
+target_precompile_headers(libcockatrice_filters PRIVATE "${CMAKE_SOURCE_DIR}/cmake/pch/qtcore_pch.h")

libcockatrice_interfaces/CMakeLists.txt

@@ -31,3 +31,5 @@ add_library(libcockatrice_interfaces STATIC ${MOC_SOURCES})
 target_include_directories(libcockatrice_interfaces PUBLIC ${CMAKE_CURRENT_SOURCE_DIR})
 
 target_link_libraries(libcockatrice_interfaces PUBLIC libcockatrice_utility ${QT_CORE_MODULE})
+
+target_precompile_headers(libcockatrice_interfaces PRIVATE "${CMAKE_SOURCE_DIR}/cmake/pch/qtcore_pch.h")

libcockatrice_models/libcockatrice/models/database/CMakeLists.txt

@@ -29,3 +29,5 @@ add_library(
 target_include_directories(libcockatrice_models_database PUBLIC ${CMAKE_CURRENT_SOURCE_DIR})
 
 target_link_libraries(libcockatrice_models_database PUBLIC libcockatrice_card libcockatrice_filters ${QT_CORE_MODULE})
+
+target_precompile_headers(libcockatrice_models_database PRIVATE "${CMAKE_SOURCE_DIR}/cmake/pch/qtcore_pch.h")

libcockatrice_models/libcockatrice/models/deck_list/CMakeLists.txt

@@ -16,3 +16,5 @@ target_include_directories(libcockatrice_models_deck_list PUBLIC ${CMAKE_CURRENT
 target_link_libraries(
   libcockatrice_models_deck_list PUBLIC libcockatrice_card libcockatrice_deck_list ${QT_CORE_MODULE}
 )
+
+target_precompile_headers(libcockatrice_models_deck_list PRIVATE "${CMAKE_SOURCE_DIR}/cmake/pch/qtcore_pch.h")

libcockatrice_network/libcockatrice/network/client/abstract/CMakeLists.txt

@@ -19,3 +19,5 @@ target_link_libraries(
   PUBLIC ${COCKATRICE_QT_VERSION_NAME}::Network ${COCKATRICE_QT_VERSION_NAME}::WebSockets libcockatrice_protocol
          libcockatrice_network_server_remote
 )
+
+target_precompile_headers(libcockatrice_network_client_abstract PRIVATE "${CMAKE_SOURCE_DIR}/cmake/pch/qtcore_pch.h")

libcockatrice_network/libcockatrice/network/client/local/CMakeLists.txt

@@ -19,3 +19,5 @@ target_link_libraries(
   PUBLIC ${COCKATRICE_QT_VERSION_NAME}::Network ${COCKATRICE_QT_VERSION_NAME}::WebSockets
          libcockatrice_network_client_abstract
 )
+
+target_precompile_headers(libcockatrice_network_client_local PRIVATE "${CMAKE_SOURCE_DIR}/cmake/pch/qtcore_pch.h")

libcockatrice_network/libcockatrice/network/client/remote/CMakeLists.txt

@@ -19,3 +19,5 @@ target_link_libraries(
   PUBLIC ${COCKATRICE_QT_VERSION_NAME}::Network ${COCKATRICE_QT_VERSION_NAME}::WebSockets
          libcockatrice_network_client_abstract libcockatrice_interfaces libcockatrice_utility libcockatrice_protocol
 )
+
+target_precompile_headers(libcockatrice_network_client_remote PRIVATE "${CMAKE_SOURCE_DIR}/cmake/pch/qtcore_pch.h")

libcockatrice_network/libcockatrice/network/server/local/CMakeLists.txt

@@ -15,3 +15,5 @@ add_dependencies(libcockatrice_network_server_local libcockatrice_protocol)
 target_include_directories(libcockatrice_network_server_local PUBLIC .)
 
 target_link_libraries(libcockatrice_network_server_local PUBLIC ${COCKATRICE_QT_MODULES} libcockatrice_protocol)
+
+target_precompile_headers(libcockatrice_network_server_local PRIVATE "${CMAKE_SOURCE_DIR}/cmake/pch/qtcore_pch.h")

libcockatrice_network/libcockatrice/network/server/remote/CMakeLists.txt

@@ -62,3 +62,5 @@ target_link_libraries(
   libcockatrice_network_server_remote PUBLIC libcockatrice_protocol libcockatrice_utility libcockatrice_rng
                                              libcockatrice_deck_list ${COCKATRICE_QT_MODULES}
 )
+
+target_precompile_headers(libcockatrice_network_server_remote PRIVATE "${CMAKE_SOURCE_DIR}/cmake/pch/qtcore_pch.h")

libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_player.h

@@ -56,6 +56,10 @@ public:
     {
         return deck;
     }
+    bool getSideboardLocked() const
+    {
+        return sideboardLocked;
+    }
     bool getReadyStart() const
     {
         return readyStart;

libcockatrice_network/libcockatrice/network/server/remote/game/server_game.cpp

@@ -61,6 +61,8 @@
 #include <QTimer>
 #include <google/protobuf/descriptor.h>
 #include <google/protobuf/message.h>
+#include <libcockatrice/deck_list/deck_list.h>
+#include <libcockatrice/protocol/pb/context_deck_select.pb.h>
 #include <libcockatrice/protocol/pb/context_ping_changed.pb.h>
 #include <libcockatrice/protocol/pb/event_delete_arrow.pb.h>
 #include <libcockatrice/protocol/pb/event_game_closed.pb.h>
@@ -825,6 +827,48 @@ void Server_Game::createGameJoinedEvent(Server_AbstractParticipant *joiningParti
     }
 
     rc.enqueuePostResponseItem(ServerMessage::GAME_EVENT_CONTAINER, prepareGameEvent(event2, -1));
+
+    // A newly joined player never saw the "Player X has loaded a deck" messages
+    // (they are only broadcast when a deck is selected), so replay a deck-select
+    // context for every player that already has a deck loaded. It is delivered
+    // privately to the joining participant, whose client renders it as a log
+    // entry in the same way as a fresh deck selection. Skipped on reconnect
+    // (resuming) — the player already saw these — and once the game has started,
+    // when the message is no longer useful and would only spam the log.
+    if (!resuming && !gameStarted) {
+        for (auto *participant : participants.values()) {
+            if (participant == joiningParticipant) {
+                continue;
+            }
+            auto *deckOwner = qobject_cast<Server_Player *>(participant);
+            if (deckOwner == nullptr) {
+                continue;
+            }
+            const DeckList *deckList = deckOwner->getDeckList();
+            if (deckList == nullptr) {
+                continue;
+            }
+
+            Event_PlayerPropertiesChanged event;
+            event.mutable_player_properties()->set_sideboard_locked(deckOwner->getSideboardLocked());
+            event.mutable_player_properties()->set_deck_hash(deckList->getDeckHash().toStdString());
+
+            Context_DeckSelect deckSelect;
+            deckSelect.set_deck_hash(deckList->getDeckHash().toStdString());
+            deckSelect.set_sideboard_size(deckList->getSideboardSize());
+            if (getShareDecklistsOnLoad()) {
+                deckSelect.set_deck_list(deckList->writeToString_Native().toStdString());
+            }
+
+            GameEventContext context;
+            context.GetReflection()
+                ->MutableMessage(&context, deckSelect.GetDescriptor()->FindExtensionByName("ext"))
+                ->CopyFrom(deckSelect);
+
+            rc.enqueuePostResponseItem(ServerMessage::GAME_EVENT_CONTAINER,
+                                       prepareGameEvent(event, deckOwner->getPlayerId(), &context));
+        }
+    }
 }
 
 void Server_Game::sendGameEventContainer(GameEventContainer *cont,

libcockatrice_protocol/CMakeLists.txt

@@ -24,3 +24,5 @@ target_include_directories(
   libcockatrice_protocol PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}
                                 ${CMAKE_CURRENT_BINARY_DIR} # points to the generated headers
 )
+
+target_precompile_headers(libcockatrice_protocol PRIVATE "${CMAKE_SOURCE_DIR}/cmake/pch/qtcore_pch.h")

libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt

@@ -55,6 +55,7 @@ set(PROTO_FILES
     command_report_details.proto
     command_report_list.proto
     command_report_my_list.proto
+    command_report_reopen.proto
     command_report_resolve.proto
     command_report_stats.proto
     command_report_user_info.proto

libcockatrice_protocol/libcockatrice/protocol/pb/command_report_reopen.proto

@@ -0,0 +1,10 @@
+syntax = "proto2";
+import "moderator_commands.proto";
+
+message Command_ReportReopen {
+    extend ModeratorCommand {
+        optional Command_ReportReopen ext = 1206;
+    }
+    required int32 report_id = 1;
+    optional string reopen_note = 2;
+}

libcockatrice_protocol/libcockatrice/protocol/pb/moderator_commands.proto

@@ -26,6 +26,7 @@ message ModeratorCommand {
         REPLAY_DOWNLOAD_BY_GAME_ID = 1203;
         REPORT_USER_INFO = 1204;
         REPORT_STATS = 1205;
+        REPORT_REOPEN = 1206;
     }
     extensions 100 to max;
 }

libcockatrice_protocol/libcockatrice/protocol/pb/serverinfo_report.proto

@@ -33,4 +33,7 @@ message ServerInfo_Report {
     optional string chat_log = 14;
 
     optional string resolution_note = 15;
+    optional string resolved_by_name = 16;
+
+    optional int32 reported_user_level = 17;
 }

libcockatrice_rng/CMakeLists.txt

@@ -11,6 +11,10 @@ add_library(
                            libcockatrice/rng/sfmt/SFMT.c
 )
 
+# SFMT is vendored and its own headers trip UBSan's alignment and vptr checks.
+cockatrice_disable_sanitizers_for(libcockatrice/rng/sfmt/SFMT.c)
+cockatrice_disable_coverage_for(libcockatrice/rng/sfmt/SFMT.c)
+
 target_include_directories(libcockatrice_rng PUBLIC ${CMAKE_CURRENT_SOURCE_DIR})
 
 target_link_libraries(libcockatrice_rng PUBLIC ${QT_CORE_MODULE})

libcockatrice_settings/CMakeLists.txt

@@ -71,3 +71,5 @@ target_include_directories(
 )
 
 target_link_libraries(libcockatrice_settings PUBLIC libcockatrice_card libcockatrice_utility ${QT_CORE_MODULE})
+
+target_precompile_headers(libcockatrice_settings PRIVATE "${CMAKE_SOURCE_DIR}/cmake/pch/qtcore_pch.h")

libcockatrice_settings/libcockatrice/settings/paths_settings.cpp

@@ -2,6 +2,8 @@
 
 #include "libcockatrice/settings/settings_manager.h"
 
+#include <QDir>
+#include <QStringList>
 #include <QVariant>
 
 class QObject;
@@ -121,3 +123,52 @@ void PathsSettings::setSpoilerDatabasePath(const QString &_spoilerDatabasePath)
     setValue(_spoilerDatabasePath, "spoilerDatabase");
     emit cardDatabasePathChanged();
 }
+
+QStringList PathsSettings::pathsInsideDir(const QString &baseDir) const
+{
+    QStringList offenders;
+    const auto check = [&offenders, &baseDir](const QString &path) {
+        if (!path.isEmpty() && isInsideDir(path, baseDir)) {
+            offenders.append(QDir::cleanPath(path));
+        }
+    };
+
+    check(getDeckPath());
+    check(getFiltersPath());
+    check(getReplaysPath());
+    check(getPicsPath());
+    check(getCustomPicsPath());
+    check(getThemesPath());
+    check(getCardDatabasePath());
+    check(getCustomCardDatabasePath());
+    check(getTokenDatabasePath());
+    check(getSpoilerCardDatabasePath());
+    check(getRedirectCachePath());
+
+    offenders.removeDuplicates();
+    return offenders;
+}
+
+bool PathsSettings::isInsideDir(const QString &path, const QString &baseDir)
+{
+#if defined(Q_OS_WIN) || defined(Q_OS_MACOS)
+    // Windows and default macOS volumes are case-insensitive filesystems.
+    constexpr Qt::CaseSensitivity caseSensitivity = Qt::CaseInsensitive;
+#else
+    constexpr Qt::CaseSensitivity caseSensitivity = Qt::CaseSensitive;
+#endif
+    const QString base = QDir::cleanPath(baseDir);
+    const QString cleaned = QDir::cleanPath(path);
+    const QString prefix = base.endsWith(QLatin1Char('/')) ? base : base + QLatin1Char('/');
+    return cleaned == base || cleaned.startsWith(prefix, caseSensitivity);
+}
+
+bool PathsSettings::getAppDirWarningAcknowledged() const
+{
+    return getValue("appDirWarningAcknowledged").toBool();
+}
+
+void PathsSettings::setAppDirWarningAcknowledged(bool _appDirWarningAcknowledged)
+{
+    setValue(_appDirWarningAcknowledged, "appDirWarningAcknowledged");
+}

libcockatrice_settings/libcockatrice/settings/paths_settings.h

@@ -4,6 +4,7 @@
 #include "settings_manager.h"
 
 #include <QString>
+#include <QStringList>
 #include <libcockatrice/interfaces/interface_paths_settings_provider.h>
 #include <qtmetamacros.h>
 
@@ -27,6 +28,24 @@ public:
     [[nodiscard]] QString getSpoilerCardDatabasePath() const override;
     [[nodiscard]] QString getRedirectCachePath() const override;
 
+    /**
+     * @brief Returns the configured paths that resolve inside baseDir.
+     *
+     * Comparison is lexical (QDir::cleanPath); symlinks are not resolved, so a
+     * path that only reaches baseDir through a symlink is not reported.
+     */
+    [[nodiscard]] QStringList pathsInsideDir(const QString &baseDir) const;
+
+    /**
+     * @brief Whether path resolves inside baseDir, as a single-path check.
+     *
+     * A path counts as inside when it equals baseDir or is nested below it;
+     * a sibling sharing the name prefix (baseDir + "2") does not.
+     */
+    [[nodiscard]] static bool isInsideDir(const QString &path, const QString &baseDir);
+
+    [[nodiscard]] bool getAppDirWarningAcknowledged() const;
+
     void setDeckPath(const QString &_deckPath);
     void setFiltersPath(const QString &_filtersPath);
     void setReplaysPath(const QString &_replaysPath);
@@ -37,14 +56,17 @@ public:
     void setCustomCardDatabasePath(const QString &_customCardDatabasePath);
     void setTokenDatabasePath(const QString &_tokenDatabasePath);
     void setSpoilerDatabasePath(const QString &_spoilerDatabasePath);
+    void setAppDirWarningAcknowledged(bool _appDirWarningAcknowledged);
 
 signals:
     void cardDatabasePathChanged();
     void picsPathChanged();
     void themeChanged();
 
-private:
+public:
     explicit PathsSettings(const QString &settingPath, QObject *parent = nullptr);
+
+private:
     PathsSettings(const PathsSettings & /*other*/);
 };
 

libcockatrice_utility/CMakeLists.txt

@@ -49,3 +49,5 @@ target_link_libraries(libcockatrice_utility PUBLIC libcockatrice_rng OpenSSL::Cr
 set(ORACLE_LIBS)
 
 include_directories(${${COCKATRICE_QT_VERSION_NAME}Core_INCLUDE_DIRS})
+
+target_precompile_headers(libcockatrice_utility PRIVATE "${CMAKE_SOURCE_DIR}/cmake/pch/qtcore_pch.h")

oracle/CMakeLists.txt

@@ -79,6 +79,9 @@ if(ZLIB_FOUND)
   include_directories(${ZLIB_INCLUDE_DIRS})
   add_definitions("-DHAS_ZLIB")
   list(APPEND oracle_SOURCES src/zip/unzip.cpp src/zip/zipglobal.cpp)
+  # minizip and zlib are vendored.
+  cockatrice_disable_sanitizers_for(src/zip/unzip.cpp src/zip/zipglobal.cpp)
+  cockatrice_disable_coverage_for(src/zip/unzip.cpp src/zip/zipglobal.cpp)
 else()
   message(STATUS "Oracle: zlib not found; ZIP support disabled")
 endif()
@@ -89,6 +92,9 @@ if(LIBLZMA_FOUND)
   include_directories(${LIBLZMA_INCLUDE_DIRS})
   add_definitions("-DHAS_LZMA")
   list(APPEND oracle_SOURCES src/lzma/decompress.cpp)
+  # The LZMA SDK is vendored.
+  cockatrice_disable_sanitizers_for(src/lzma/decompress.cpp)
+  cockatrice_disable_coverage_for(src/lzma/decompress.cpp)
 else()
   message(STATUS "Oracle: LibLZMA not found; xz support disabled")
 endif()

servatrice/CMakeLists.txt

@@ -27,6 +27,14 @@ set(servatrice_SOURCES
 
 set(servatrice_RESOURCES servatrice.qrc)
 
+# Qxt SMTP is vendored.
+cockatrice_disable_sanitizers_for(
+  src/smtp/qxthmac.cpp src/smtp/qxtmailattachment.cpp src/smtp/qxtmailmessage.cpp src/smtp/qxtsmtp.cpp
+)
+cockatrice_disable_coverage_for(
+  src/smtp/qxthmac.cpp src/smtp/qxtmailattachment.cpp src/smtp/qxtmailmessage.cpp src/smtp/qxtsmtp.cpp
+)
+
 if(WIN32)
   set(servatrice_SOURCES ${servatrice_SOURCES} servatrice.rc)
 endif(WIN32)

servatrice/servatrice.ini.example

@@ -250,6 +250,16 @@ email=root@localhost
 ; Sender email name
 name="Cockatrice server"
 
+; Maximum number of attempts to deliver a given e-mail before giving up.
+; If the smtp server is unreachable or repeatedly rejects a message, the server
+; will retry with an exponentially growing delay up to this many times, then drop
+; the message from the queue and log the failure. Default: 5
+maxretries=5
+
+; Initial retry delay in seconds between delivery attempts. Doubles with each
+; failed attempt (capped at 30 minutes). Default: 60
+retrydelay=60
+
 ; Email subject
 subject="Cockatrice server account activation token"
 

servatrice/src/servatrice.cpp

@@ -38,6 +38,7 @@
 #include <QDebug>
 #include <QFile>
 #include <QIODevice>
+#include <QLoggingCategory>
 #include <QMapIterator>
 #include <QMetaObject>
 #include <QMutableListIterator>
@@ -69,6 +70,8 @@
 #include <server_room.h>
 #include <string>
 
+inline Q_LOGGING_CATEGORY(ServatriceLog, "servatrice");
+
 Servatrice_GameServer::Servatrice_GameServer(Servatrice *_server,
                                              int _numberPools,
                                              const QSqlDatabase &_sqlDatabase,
@@ -335,6 +338,11 @@ bool Servatrice::initServer()
         servatriceDatabaseInterface->clearSessionTables();
     }
 
+    // Connect the SMTP client's delivery-confirmation signals to the database cleanup handlers.
+    // Emails are removed from the queue tables only after delivery is confirmed (see statusUpdate()).
+    connect(smtpClient, &SmtpClient::mailDelivered, this, &Servatrice::onEmailDelivered);
+    connect(smtpClient, &SmtpClient::mailPermanentlyFailed, this, &Servatrice::onEmailPermanentlyFailed);
+
     if (getRoomsMethodString() == "sql") {
         QSqlQuery *query = servatriceDatabaseInterface->prepareQuery(
             "select id, name, descr, permissionlevel, privlevel, auto_join, join_message, chat_history_size from "
@@ -737,41 +745,36 @@ void Servatrice::statusUpdate()
                 return;
             }
 
-            auto *queryDelete =
-                servatriceDatabaseInterface->prepareQuery("delete from {prefix}_activation_emails where name = :name");
-
+            // Rows are intentionally NOT deleted here: the durable source of truth for a pending
+            // activation e-mail is the database row, which is only removed once delivery has been
+            // confirmed by the SMTP client (see onEmailDelivered). This makes sending resilient to
+            // transient SMTP failures and server restarts.
             while (servDbSelQuery->next()) {
                 const QString userName = servDbSelQuery->value(0).toString();
                 const auto emailAddress = EmailParser::getParsedEmailAddress(servDbSelQuery->value(1).toString());
                 const QString token = servDbSelQuery->value(2).toString();
 
-                if (smtpClient->enqueueActivationTokenMail(userName, emailAddress, token)) {
-                    queryDelete->bindValue(":name", userName);
-                    servatriceDatabaseInterface->execSqlQuery(queryDelete);
-                }
+                smtpClient->enqueueActivationTokenMail(userName, emailAddress, token);
             }
         }
 
         if (getEnableForgotPassword()) {
             auto *forgotPwQuery = servatriceDatabaseInterface->prepareQuery(
-                "select a.name, b.email, b.token from {prefix}_forgot_password a left join {prefix}_users b on a.name "
-                "= b.name where a.emailed = 0");
+                "select a.id, a.name, b.email, b.token from {prefix}_forgot_password a left join {prefix}_users b on "
+                "a.name = b.name where a.emailed = 0");
             if (!servatriceDatabaseInterface->execSqlQuery(forgotPwQuery)) {
                 return;
             }
 
-            QSqlQuery *queryDelete = servatriceDatabaseInterface->prepareQuery(
-                "update {prefix}_forgot_password set emailed = 1 where name = :name");
-
+            // Rows are intentionally left with emailed = 0 here; they are marked emailed only once
+            // delivery is confirmed by the SMTP client (see onEmailDelivered).
             while (forgotPwQuery->next()) {
-                const QString userName = forgotPwQuery->value(0).toString();
-                const auto emailAddress = EmailParser::getParsedEmailAddress(forgotPwQuery->value(1).toString());
-                const QString token = forgotPwQuery->value(2).toString();
+                const qint64 rowId = forgotPwQuery->value(0).toLongLong();
+                const QString userName = forgotPwQuery->value(1).toString();
+                const auto emailAddress = EmailParser::getParsedEmailAddress(forgotPwQuery->value(2).toString());
+                const QString token = forgotPwQuery->value(3).toString();
 
-                if (smtpClient->enqueueForgotPasswordTokenMail(userName, emailAddress, token)) {
-                    queryDelete->bindValue(":name", userName);
-                    servatriceDatabaseInterface->execSqlQuery(queryDelete);
-                }
+                smtpClient->enqueueForgotPasswordTokenMail(userName, emailAddress, token, rowId);
             }
         }
 
@@ -779,6 +782,69 @@ void Servatrice::statusUpdate()
     }
 }
 
+void Servatrice::onEmailDelivered(const QString &userName, EmailType type, const QString &token, qint64 rowId)
+{
+    if (cleanupDatabaseForEmail(userName, type, token, rowId)) {
+        qCDebug(ServatriceLog) << "E-mail delivered to" << userName;
+    } else {
+        qCWarning(ServatriceLog) << "Failed to clean up database row for delivered e-mail to" << userName
+                                 << "- row may be re-enqueued on next status update";
+    }
+}
+
+void Servatrice::onEmailPermanentlyFailed(const QString &userName,
+                                          EmailType type,
+                                          FailureReason reason,
+                                          const QString &token,
+                                          qint64 rowId)
+{
+    switch (reason) {
+        case FailureReason::RetryExhausted:
+            qCWarning(ServatriceLog) << "E-mail to" << userName << "permanently failed after max retries";
+            break;
+        case FailureReason::RecipientRejected:
+            qCWarning(ServatriceLog) << "E-mail to" << userName << "permanently failed: recipient rejected";
+            break;
+        case FailureReason::SenderRejected:
+            qCWarning(ServatriceLog) << "E-mail to" << userName << "permanently failed: sender rejected";
+            break;
+    }
+
+    if (!cleanupDatabaseForEmail(userName, type, token, rowId)) {
+        qCWarning(ServatriceLog) << "Failed to clean up database row for permanently failed e-mail to" << userName
+                                 << "- row may be re-enqueued on next status update";
+    }
+}
+
+bool Servatrice::cleanupDatabaseForEmail(const QString &userName, EmailType type, const QString &token, qint64 rowId)
+{
+    Q_UNUSED(token);
+    switch (type) {
+        case EmailType::Activation: {
+            QSqlQuery *query =
+                servatriceDatabaseInterface->prepareQuery("delete from {prefix}_activation_emails where name = :name");
+            query->bindValue(":name", userName);
+            return servatriceDatabaseInterface->execSqlQuery(query);
+        }
+        case EmailType::ForgotPassword: {
+            // Scope the update to the row that was actually sent: the table
+            // holds one row per request, and marking by name would also mark
+            // rows for newer tokens that were never delivered.
+            if (rowId >= 0) {
+                QSqlQuery *query = servatriceDatabaseInterface->prepareQuery(
+                    "update {prefix}_forgot_password set emailed = 1 where id = :id");
+                query->bindValue(":id", rowId);
+                return servatriceDatabaseInterface->execSqlQuery(query);
+            }
+            QSqlQuery *query = servatriceDatabaseInterface->prepareQuery(
+                "update {prefix}_forgot_password set emailed = 1 where name = :name");
+            query->bindValue(":name", userName);
+            return servatriceDatabaseInterface->execSqlQuery(query);
+        }
+    }
+    return false;
+}
+
 SessionEvent *Servatrice::makeShutdownEvent() const
 {
     Event_ServerShutdown event;

servatrice/src/servatrice.h

@@ -21,6 +21,7 @@
 #define SERVATRICE_H
 
 #include "metrics_registry.h"
+#include "smtpclient.h"
 
 #include <QByteArray>
 #include <QDateTime>
@@ -181,6 +182,7 @@ private:
     QMap<QString, bool> serverRequiredFeatureList;
     QString officialWarnings;
     Servatrice_DatabaseInterface *servatriceDatabaseInterface;
+    bool cleanupDatabaseForEmail(const QString &userName, EmailType type, const QString &token, qint64 rowId);
     int serverId;
     int uptime;
     QMutex txBytesMutex, rxBytesMutex;
@@ -231,6 +233,12 @@ public slots:
     void scheduleShutdown(const QString &reason, int minutes);
     void updateLoginMessage();
     void setRequiredFeatures(const QString &featureList);
+    void onEmailDelivered(const QString &userName, EmailType type, const QString &token, qint64 rowId);
+    void onEmailPermanentlyFailed(const QString &userName,
+                                  EmailType type,
+                                  FailureReason reason,
+                                  const QString &token,
+                                  qint64 rowId);
 
 public:
     explicit Servatrice(QObject *parent = nullptr);

servatrice/src/serversocketinterface.cpp

@@ -110,6 +110,7 @@
 #include <libcockatrice/protocol/pb/command_report_details.pb.h>
 #include <libcockatrice/protocol/pb/command_report_list.pb.h>
 #include <libcockatrice/protocol/pb/command_report_my_list.pb.h>
+#include <libcockatrice/protocol/pb/command_report_reopen.pb.h>
 #include <libcockatrice/protocol/pb/command_report_resolve.pb.h>
 #include <libcockatrice/protocol/pb/command_report_stats.pb.h>
 #include <libcockatrice/protocol/pb/command_report_user_info.pb.h>
@@ -434,6 +435,8 @@ Response::ResponseCode AbstractServerSocketInterface::processExtendedModeratorCo
             return cmdReportAssign(cmd.GetExtension(Command_ReportAssign::ext), rc);
         case ModeratorCommand::REPORT_RESOLVE:
             return cmdReportResolve(cmd.GetExtension(Command_ReportResolve::ext), rc);
+        case ModeratorCommand::REPORT_REOPEN:
+            return cmdReportReopen(cmd.GetExtension(Command_ReportReopen::ext), rc);
         case ModeratorCommand::VIEWLOG_HISTORY:
             return cmdGetLogHistory(cmd.GetExtension(Command_ViewLogHistory::ext), rc, true);
         case ModeratorCommand::GRANT_REPLAY_ACCESS:
@@ -1983,6 +1986,29 @@ Response::ResponseCode AbstractServerSocketInterface::cmdBanFromServer(const Com
     return Response::RespOk;
 }
 
+namespace
+{
+int reportUserLevel(bool userExists, int adminFlags)
+{
+    int userLevel = ServerInfo_User::IsUser;
+    if (userExists) {
+        userLevel |= ServerInfo_User::IsRegistered;
+        if (adminFlags & 1) {
+            userLevel |= ServerInfo_User::IsAdmin | ServerInfo_User::IsModerator;
+        } else if (adminFlags & 2) {
+            userLevel |= ServerInfo_User::IsModerator;
+        }
+        if (adminFlags & 4) {
+            userLevel |= ServerInfo_User::IsJudge;
+        }
+        if (adminFlags & 8) {
+            userLevel |= ServerInfo_User::IsDeveloper;
+        }
+    }
+    return userLevel;
+}
+} // namespace
+
 Response::ResponseCode AbstractServerSocketInterface::cmdReportList(const Command_ReportList &cmd,
                                                                     ResponseContainer &rc)
 {
@@ -1996,8 +2022,9 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReportList(const Comman
 
     // Columns: 0=id, 1=reporter_name, 2=reported_user_name, 3=game_id,
     //          4=category, 5=description, 6=created_at, 7=status,
-    //          8=resolution_note, 9=assigned_mod_name,
-    //          10=room_id, 11=replay_id
+    //          8=resolution_note, 9=assigned_mod_name, 10=resolved_by_name,
+    //          11=room_id, 12=replay_id,
+    //          13=reported_user_id, 14=reported_user_admin
     QString whereClause;
     if (unresolvedOnly) {
         whereClause = "WHERE r.status = 'open' OR r.status = 'assigned' ";
@@ -2012,11 +2039,13 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReportList(const Comman
 
     QString queryStr = "SELECT r.id, r.reporter_name, r.reported_user_name, r.game_id, "
                        "r.category, r.description, r.created_at, r.status, "
-                       "r.resolution_note, u.name AS assigned_mod_name, r.room_id, "
+                       "r.resolution_note, u.name AS assigned_mod_name, ru.name AS resolved_by_name, r.room_id, "
                        "(SELECT id FROM {prefix}_replays WHERE id_game = r.game_id ORDER BY id DESC LIMIT 1) "
-                       "AS replay_id "
+                       "AS replay_id, u2.id AS reported_user_id, u2.admin AS reported_user_admin "
                        "FROM {prefix}_reports r "
-                       "LEFT JOIN {prefix}_users u ON r.assigned_to = u.id ";
+                       "LEFT JOIN {prefix}_users u ON r.assigned_to = u.id "
+                       "LEFT JOIN {prefix}_users ru ON r.resolved_by = ru.id "
+                       "LEFT JOIN {prefix}_users u2 ON u2.name = r.reported_user_name ";
 
     if (unresolvedOnly) {
         queryStr += "WHERE r.status = 'open' OR r.status = 'assigned' ";
@@ -2057,14 +2086,19 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReportList(const Comman
         if (!query->value(9).isNull()) {
             info->set_assigned_mod_name(query->value(9).toString().toStdString());
         }
-
         if (!query->value(10).isNull()) {
-            info->set_room_id(query->value(10).toInt());
+            info->set_resolved_by_name(query->value(10).toString().toStdString());
         }
 
         if (!query->value(11).isNull()) {
-            info->set_replay_id(query->value(11).toInt());
+            info->set_room_id(query->value(11).toInt());
         }
+
+        if (!query->value(12).isNull()) {
+            info->set_replay_id(query->value(12).toInt());
+        }
+
+        info->set_reported_user_level(reportUserLevel(!query->value(13).isNull(), query->value(14).toInt()));
     }
 
     rc.setResponseExtension(re);
@@ -2737,11 +2771,13 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReportMyList(const Comm
 
     QString queryStr = "SELECT r.id, r.reporter_name, r.reported_user_name, r.game_id, "
                        "r.category, r.description, r.created_at, r.status, "
-                       "r.resolution_note, u.name AS assigned_mod_name, r.room_id, "
+                       "r.resolution_note, u.name AS assigned_mod_name, ru.name AS resolved_by_name, r.room_id, "
                        "(SELECT id FROM {prefix}_replays WHERE id_game = r.game_id ORDER BY id DESC LIMIT 1) "
-                       "AS replay_id "
+                       "AS replay_id, u2.id AS reported_user_id, u2.admin AS reported_user_admin "
                        "FROM {prefix}_reports r "
                        "LEFT JOIN {prefix}_users u ON r.assigned_to = u.id "
+                       "LEFT JOIN {prefix}_users ru ON r.resolved_by = ru.id "
+                       "LEFT JOIN {prefix}_users u2 ON u2.name = r.reported_user_name "
                        "WHERE r.reporter_id = :reporter_id "
                        "ORDER BY r.created_at DESC LIMIT 200";
 
@@ -2777,20 +2813,61 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReportMyList(const Comm
         if (!query->value(9).isNull()) {
             info->set_assigned_mod_name(query->value(9).toString().toStdString());
         }
-
         if (!query->value(10).isNull()) {
-            info->set_room_id(query->value(10).toInt());
+            info->set_resolved_by_name(query->value(10).toString().toStdString());
         }
 
         if (!query->value(11).isNull()) {
-            info->set_replay_id(query->value(11).toInt());
+            info->set_room_id(query->value(11).toInt());
         }
+
+        if (!query->value(12).isNull()) {
+            info->set_replay_id(query->value(12).toInt());
+        }
+
+        info->set_reported_user_level(reportUserLevel(!query->value(13).isNull(), query->value(14).toInt()));
     }
 
     rc.setResponseExtension(re);
     return Response::RespOk;
 }
 
+Response::ResponseCode AbstractServerSocketInterface::cmdReportReopen(const Command_ReportReopen &cmd,
+                                                                      ResponseContainer & /*rc*/)
+{
+    if (!sqlInterface->checkSql()) {
+        return Response::RespInternalError;
+    }
+
+    int reportId = cmd.report_id();
+    QString note = textFromStdString(cmd.reopen_note());
+
+    QSqlQuery *query = sqlInterface->prepareQuery("UPDATE {prefix}_reports "
+                                                  "SET status = 'open', resolution_note = NULL, "
+                                                  "resolution_time = NULL, resolved_by = NULL, assigned_to = NULL, "
+                                                  "notified = 0 "
+                                                  "WHERE id = :id AND status IN ('resolved', 'dismissed')");
+
+    query->bindValue(":id", reportId);
+
+    if (!sqlInterface->execSqlQuery(query)) {
+        return Response::RespInternalError;
+    }
+
+    if (query->numRowsAffected() == 0) {
+        return Response::RespInvalidData;
+    }
+
+    QString auditDetails = QString("Report #%1 reopened").arg(reportId);
+    if (!note.isEmpty()) {
+        auditDetails += QString(". Reason: %1").arg(note.left(200));
+    }
+    sqlInterface->addAuditRecord(QString::number(reportId), this->getAddress(),
+                                 QString::fromStdString(userInfo->clientid()), "REPORT_REOPENED", auditDetails, true);
+
+    return Response::RespOk;
+}
+
 Response::ResponseCode AbstractServerSocketInterface::cmdReportDetails(const Command_ReportDetails &cmd,
                                                                        ResponseContainer &rc)
 {
@@ -2806,17 +2883,19 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReportDetails(const Com
 
     // Columns: 0=id, 1=reporter_id, 2=reporter_name, 3=reported_user_name, 4=game_id,
     //          5=category, 6=description, 7=created_at, 8=status,
-    //          9=resolution_note, 10=assigned_mod_name, 11=chat_log,
-    //          12=room_id, 13=resolution_time, 14=replay_id
-    QString queryStr = "SELECT r.id, r.reporter_id, r.reporter_name, r.reported_user_name, r.game_id, "
-                       "r.category, r.description, r.created_at, r.status, "
-                       "r.resolution_note, u.name AS assigned_mod_name, r.chat_log, r.room_id, "
-                       "r.resolution_time, "
-                       "(SELECT id FROM {prefix}_replays WHERE id_game = r.game_id ORDER BY id DESC LIMIT 1) "
-                       "AS replay_id "
-                       "FROM {prefix}_reports r "
-                       "LEFT JOIN {prefix}_users u ON r.assigned_to = u.id "
-                       "WHERE r.id = :id";
+    //          9=resolution_note, 10=assigned_mod_name, 11=resolved_by_name,
+    //          12=chat_log, 13=room_id, 14=resolution_time, 15=replay_id
+    QString queryStr =
+        "SELECT r.id, r.reporter_id, r.reporter_name, r.reported_user_name, r.game_id, "
+        "r.category, r.description, r.created_at, r.status, "
+        "r.resolution_note, u.name AS assigned_mod_name, ru.name AS resolved_by_name, r.chat_log, r.room_id, "
+        "r.resolution_time, "
+        "(SELECT id FROM {prefix}_replays WHERE id_game = r.game_id ORDER BY id DESC LIMIT 1) "
+        "AS replay_id "
+        "FROM {prefix}_reports r "
+        "LEFT JOIN {prefix}_users u ON r.assigned_to = u.id "
+        "LEFT JOIN {prefix}_users ru ON r.resolved_by = ru.id "
+        "WHERE r.id = :id";
     QSqlQuery *query = sqlInterface->prepareQuery(queryStr);
     query->bindValue(":id", reportId);
 
@@ -2855,21 +2934,24 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReportDetails(const Com
     if (!query->value(10).isNull()) {
         info->set_assigned_mod_name(query->value(10).toString().toStdString());
     }
-
     if (!query->value(11).isNull()) {
-        info->set_chat_log(query->value(11).toString().toStdString());
+        info->set_resolved_by_name(query->value(11).toString().toStdString());
     }
 
     if (!query->value(12).isNull()) {
-        info->set_room_id(query->value(12).toInt());
+        info->set_chat_log(query->value(12).toString().toStdString());
     }
 
     if (!query->value(13).isNull()) {
-        info->set_resolution_time(query->value(13).toDateTime().toSecsSinceEpoch());
+        info->set_room_id(query->value(13).toInt());
     }
 
     if (!query->value(14).isNull()) {
-        info->set_replay_id(query->value(14).toInt());
+        info->set_resolution_time(query->value(14).toDateTime().toSecsSinceEpoch());
+    }
+
+    if (!query->value(15).isNull()) {
+        info->set_replay_id(query->value(15).toInt());
     }
 
     QSqlQuery *commentQuery =

servatrice/src/serversocketinterface.h

@@ -96,6 +96,7 @@ class Command_ReportAssign;
 class Command_ReportDetails;
 class Command_ReportList;
 class Command_ReportMyList;
+class Command_ReportReopen;
 class Command_ReportResolve;
 class Command_ReportStats;
 class Command_ReportUserInfo;
@@ -181,6 +182,7 @@ private:
     Response::ResponseCode cmdUpdateServerMessage(const Command_UpdateServerMessage &cmd, ResponseContainer &rc);
     Response::ResponseCode cmdReportAssign(const Command_ReportAssign &cmd, ResponseContainer &);
     Response::ResponseCode cmdReportResolve(const Command_ReportResolve &cmd, ResponseContainer &);
+    Response::ResponseCode cmdReportReopen(const Command_ReportReopen &cmd, ResponseContainer &);
     Response::ResponseCode cmdReportUserInfo(const Command_ReportUserInfo &cmd, ResponseContainer &rc);
     Response::ResponseCode cmdReportStats(const Command_ReportStats &cmd, ResponseContainer &rc);
     Response::ResponseCode cmdRegisterAccount(const Command_Register &cmd, ResponseContainer &rc);

servatrice/src/smtp/qxtsmtp.cpp

@@ -94,6 +94,38 @@ int QxtSmtp::pendingMessages() const
     return qxt_d().pending.count();
 }
 
+/*!
+ * \brief Drops all queued (not yet transmitted) messages without sending them.
+ *
+ * Any message still awaiting delivery is discarded. This is used by callers that
+ * keep their own authoritative queue and wish to rebuild the SMTP buffer from scratch
+ * (e.g. after a reconnect) without risking duplicates.
+ */
+void QxtSmtp::clearPending()
+{
+    qxt_d().pending.clear();
+}
+
+/*!
+ * \brief Clears the outgoing queue and resets the SMTP state machine.
+ *
+ * Unlike clearPending(), this also resets the internal state to Disconnected so
+ * that subsequent send() calls will not fire sendNext() prematurely on a
+ * not-yet-connected socket.  Use this when rebuilding the queue from scratch
+ * before a fresh connectToHost() / connectToSecureHost() cycle.
+ */
+void QxtSmtp::reset()
+{
+    qxt_d().pending.clear();
+    qxt_d().extensions.clear();
+    qxt_d().buffer.clear();
+    qxt_d().recipients.clear();
+    qxt_d().rcptNumber = 0;
+    qxt_d().rcptAck = 0;
+    qxt_d().mailAck = false;
+    qxt_d().state = QxtSmtpPrivate::Disconnected;
+}
+
 QTcpSocket *QxtSmtp::socket() const
 {
     return qxt_d().socket;

servatrice/src/smtp/qxtsmtp.h

@@ -66,6 +66,8 @@ public:
 
     int send(const QxtMailMessage& message);
     int pendingMessages() const;
+    void clearPending();
+    void reset();
 
     QTcpSocket* socket() const;
     void connectToHost(const QString& hostName, quint16 port = 25);

servatrice/src/smtpclient.cpp

@@ -7,17 +7,56 @@
 #include <QAbstractSocket>
 #include <QByteArray>
 #include <QDebug>
+#include <QLoggingCategory>
 #include <QSharedPointer>
 #include <QSslSocket>
+#include <QString>
 #include <QTcpSocket>
 #include <QVariant>
 #include <qlogging.h>
 #include <qobjectdefs.h>
 
+static const qint64 DEFAULT_MAX_RETRIES = 5;
+static const qint64 DEFAULT_RETRY_DELAY = 60;
+static const qint64 MAX_RETRY_DELAY = 1800;  // 30 minutes
+static const qint64 IDLE_TIMEOUT_MS = 60000; // 60 seconds
+// Maximum bit-shift exponent used in the exponential back-off formula.
+// With a base delay of 60 s, shift 10 gives 60 * 1024 ≈ 17 h before the
+// MAX_RETRY_DELAY cap is applied, preventing unbounded intermediate values.
+static const int MAX_BACKOFF_SHIFT = 10;
+
+inline Q_LOGGING_CATEGORY(SmtpClientLog, "smtp_client")
+
+static QString pendingKey(const QString &userName, EmailType type, const QString &token)
+{
+    const char *typeName = (type == EmailType::Activation) ? "activation" : "forgotpassword";
+    // The token is part of the key: a regenerated token (forgot-password issues a
+    // fresh one per request) must not be suppressed by a stale pending entry.
+    return userName + '/' + typeName + '/' + token;
+}
+
+// True only for permanent 5xx replies. Anything else (4xx greylisting/rate
+// limiting, or an unparsable reply) must be retried, never treated as final:
+// destroying the durable queue row on a transient failure loses the mail.
+static bool isPermanentSmtpReply(const QByteArray &msg)
+{
+    for (char c : msg) {
+        if (c == ' ' || c == '\t' || c == '\r' || c == '\n' || c == '-') {
+            continue;
+        }
+        return c == '5';
+    }
+    return false;
+}
+
 SmtpClient::SmtpClient(QObject *parent) : QObject(parent)
 {
     smtp = new QxtSmtp(this);
 
+    idleTimer = new QTimer(this);
+    idleTimer->setSingleShot(true);
+    connect(idleTimer, SIGNAL(timeout()), this, SLOT(idleTimeout()));
+
     connect(smtp, SIGNAL(authenticated()), this, SLOT(authenticated()));
     connect(smtp, SIGNAL(authenticationFailed(const QByteArray &)), this,
             SLOT(authenticationFailed(const QByteArray &)));
@@ -40,104 +79,151 @@ SmtpClient::~SmtpClient()
 {
     if (smtp) {
         delete smtp;
-        smtp = 0;
+        smtp = nullptr;
     }
 }
 
+int SmtpClient::maxRetries() const
+{
+    // Bounded like retryDelaySeconds(): an unset/blank value must not turn the
+    // first transient failure into a permanent one (attempts >= 0 is immediate).
+    return qBound(1, settingsCache->value("smtp/maxretries", DEFAULT_MAX_RETRIES).toInt(), 100);
+}
+
+int SmtpClient::retryDelaySeconds() const
+{
+    return qBound(1, settingsCache->value("smtp/retrydelay", DEFAULT_RETRY_DELAY).toInt(),
+                  static_cast<int>(MAX_RETRY_DELAY));
+}
+
+QDateTime SmtpClient::nextRetryTime(int attempts, int retryDelay) const
+{
+    // Exponential backoff, capped to avoid hammering an unreachable server.
+    const int shift = qMax(0, qMin(attempts - 1, MAX_BACKOFF_SHIFT));
+    const qint64 delay = qMin<qint64>(retryDelay * qint64(1) << shift, MAX_RETRY_DELAY);
+    return QDateTime::currentDateTime().addSecs(delay);
+}
+
+SmtpClient::PendingEmail SmtpClient::buildEmail(const QString &nickname,
+                                                EmailType type,
+                                                const QString &subject,
+                                                const QString &body,
+                                                const QString &recipient,
+                                                const QString &token,
+                                                const QString &email,
+                                                qint64 rowId)
+{
+    QString name = settingsCache->value("smtp/name", "").toString();
+
+    PendingEmail pending;
+    pending.userName = nickname;
+    pending.type = type;
+    pending.token = token;
+    pending.rowId = rowId;
+    pending.maxAttempts = maxRetries();
+    pending.retryDelay = retryDelaySeconds();
+    pending.nextAttempt = QDateTime::currentDateTime();
+
+    QString emailBody = body;
+    emailBody.replace("%username", nickname).replace("%token", token);
+
+    pending.message.setSender(name + " <" + email + ">");
+    pending.message.addRecipient(recipient);
+    pending.message.setSubject(subject);
+    pending.message.setBody(emailBody);
+
+    return pending;
+}
+
+bool SmtpClient::enqueue(const PendingEmail &email)
+{
+    const QString key = pendingKey(email.userName, email.type, email.token);
+    if (pendingEmails.contains(key)) {
+        qCDebug(SmtpClientLog) << "Email already pending for" << email.userName << "- not duplicating";
+        return false;
+    }
+
+    // A fresher database row for the same user/type supersedes a stale pending
+    // entry: drop the stale one so only the current token is ever delivered.
+    // (Any in-flight mapping for the dropped key is removed as well; the
+    // mailSent/recordFailed lookups tolerate missing entries.)
+    for (auto it = pendingEmails.begin(); it != pendingEmails.end();) {
+        if (it.value().userName == email.userName && it.value().type == email.type) {
+            for (auto mapIt = mailIdToUser.begin(); mapIt != mailIdToUser.end();) {
+                if (mapIt.value() == it.key()) {
+                    mapIt = mailIdToUser.erase(mapIt);
+                } else {
+                    ++mapIt;
+                }
+            }
+            qCDebug(SmtpClientLog) << "Replacing stale pending email for" << email.userName;
+            it = pendingEmails.erase(it);
+        } else {
+            ++it;
+        }
+    }
+
+    pendingEmails.insert(key, email);
+    qCDebug(SmtpClientLog) << "Enqueued" << (email.type == EmailType::Activation ? "activation" : "password reset")
+                           << "mail to" << email.userName;
+    return true;
+}
+
 bool SmtpClient::enqueueActivationTokenMail(const QString &nickname, const QString &recipient, const QString &token)
 {
-    QString email = settingsCache->value("smtp/email", "").toString();
-    QString name = settingsCache->value("smtp/name", "").toString();
+    if (pendingEmails.contains(pendingKey(nickname, EmailType::Activation, token))) {
+        qCDebug(SmtpClientLog) << "Activation email already pending for" << nickname << "- skipping";
+        return false;
+    }
+
     QString subject = settingsCache->value("smtp/subject", "").toString();
     QString body = settingsCache->value("smtp/body", "").toString();
+    QString email = settingsCache->value("smtp/email", "").toString();
 
-    if (email.isEmpty()) {
-        qDebug() << "[MAIL] Missing sender email in configuration";
+    if (!validateEmailFields(email, subject, body, recipient, token, nickname)) {
         return false;
     }
 
-    if (subject.isEmpty()) {
-        qDebug() << "[MAIL] Missing subject field in configuration";
-        return false;
-    }
-
-    if (body.isEmpty()) {
-        qDebug() << "[MAIL] Missing body field in configuration";
-        return false;
-    }
-
-    if (recipient.isEmpty()) {
-        qDebug() << "[MAIL] Missing recipient field for user " << nickname;
-        return false;
-    }
-
-    if (token.isEmpty()) {
-        qDebug() << "[MAIL] Missing token field for user " << nickname;
-        return false;
-    }
-
-    QxtMailMessage message;
-    message.setSender(name + " <" + email + ">");
-    message.addRecipient(recipient);
-    message.setSubject(subject);
-    message.setBody(body.replace("%username", nickname).replace("%token", token));
-
-    int id = smtp->send(message);
-    qDebug() << "[MAIL] Enqueued mail to" << recipient << "as" << id;
-    return true;
+    return enqueue(buildEmail(nickname, EmailType::Activation, subject, body, recipient, token, email));
 }
 
-bool SmtpClient::enqueueForgotPasswordTokenMail(const QString &nickname, const QString &recipient, const QString &token)
+bool SmtpClient::enqueueForgotPasswordTokenMail(const QString &nickname,
+                                                const QString &recipient,
+                                                const QString &token,
+                                                qint64 rowId)
 {
-    QString email = settingsCache->value("smtp/email", "").toString();
-    QString name = settingsCache->value("smtp/name", "").toString();
+    if (pendingEmails.contains(pendingKey(nickname, EmailType::ForgotPassword, token))) {
+        qCDebug(SmtpClientLog) << "Password reset email already pending for" << nickname << "- skipping";
+        return false;
+    }
+
     QString subject = settingsCache->value("forgotpassword/subject", "").toString();
     QString body = settingsCache->value("forgotpassword/body", "").toString();
+    QString email = settingsCache->value("smtp/email", "").toString();
 
-    if (email.isEmpty()) {
-        qDebug() << "[MAIL] Missing sender email in configuration";
+    if (!validateEmailFields(email, subject, body, recipient, token, nickname)) {
         return false;
     }
 
-    if (subject.isEmpty()) {
-        qDebug() << "[MAIL] Missing subject field in configuration";
-        return false;
-    }
-
-    if (body.isEmpty()) {
-        qDebug() << "[MAIL] Missing body field in configuration";
-        return false;
-    }
-
-    if (recipient.isEmpty()) {
-        qDebug() << "[MAIL] Missing recipient field for user " << nickname;
-        return false;
-    }
-
-    if (token.isEmpty()) {
-        qDebug() << "[MAIL] Missing token field for user " << nickname;
-        return false;
-    }
-
-    QxtMailMessage message;
-    message.setSender(name + " <" + email + ">");
-    message.addRecipient(recipient);
-    message.setSubject(subject);
-    message.setBody(body.replace("%username", nickname).replace("%token", token));
-
-    int id = smtp->send(message);
-    qDebug() << "[MAIL] Enqueued mail to" << recipient << "as" << id;
-    return true;
+    return enqueue(buildEmail(nickname, EmailType::ForgotPassword, subject, body, recipient, token, email, rowId));
 }
 
 void SmtpClient::sendAllEmails()
 {
     // still connected from the previous round
-    if (smtp->socket()->state() == QAbstractSocket::ConnectedState) {
+    if (smtp->socket()->state() == QAbstractSocket::ConnectedState ||
+        smtp->socket()->state() == QAbstractSocket::ConnectingState ||
+        smtp->socket()->state() == QAbstractSocket::HostLookupState ||
+        smtp->socket()->state() == QAbstractSocket::ClosingState) {
         return;
     }
 
-    if (smtp->pendingMessages() == 0) {
+    if (pendingEmails.isEmpty()) {
+        return;
+    }
+
+    // Respect connection-level backoff after auth/conn/encryption failures.
+    if (connectionAttempts > 0 && QDateTime::currentDateTime() < connectionBackoffUntil) {
         return;
     }
 
@@ -151,6 +237,16 @@ void SmtpClient::sendAllEmails()
     smtp->setUsername(username);
     smtp->setPassword(password);
 
+    // Push due emails into QxtSmtp *before* connecting so they are already
+    // queued when the authenticated() signal fires and QxtSmtp's internal
+    // sendNext() runs.
+    relayoutForSending();
+
+    // Nothing due right now — skip the SMTP round-trip.
+    if (mailIdToUser.isEmpty()) {
+        return;
+    }
+
     // Connect
     if (connectionType == "ssl") {
         if (acceptAllCerts) {
@@ -162,64 +258,286 @@ void SmtpClient::sendAllEmails()
     }
 }
 
-void SmtpClient::authenticated()
+void SmtpClient::relayoutForSending()
 {
-    qDebug() << "[MAIL] authenticated";
-}
+    // Rebuild the SMTP buffer exclusively from our authoritative set, so a reconnect
+    // after a partial transmission never results in duplicate deliveries.
+    // reset() also clears the internal state machine so that send() does not
+    // fire sendNext() prematurely on a not-yet-connected socket.
+    smtp->reset();
+    mailIdToUser.clear();
 
-void SmtpClient::authenticationFailed(const QByteArray &msg)
-{
-    qDebug() << "[MAIL] authenticationFailed" << QString(msg);
-}
-
-void SmtpClient::connected()
-{
-    qDebug() << "[MAIL] connected";
-}
-
-void SmtpClient::connectionFailed(const QByteArray &msg)
-{
-    qDebug() << "[MAIL] connectionFailed" << QString(msg);
-}
-
-void SmtpClient::disconnected()
-{
-    qDebug() << "[MAIL] disconnected";
-}
-
-void SmtpClient::encrypted()
-{
-    qDebug() << "[MAIL] encrypted";
-}
-
-void SmtpClient::encryptionFailed(const QByteArray &msg)
-{
-    qDebug() << "[MAIL] encryptionFailed" << QString(msg);
-    qDebug() << "[MAIL] Try enabling the \"acceptallcerts\" option in servatrice.ini";
+    const QDateTime now = QDateTime::currentDateTime();
+    for (auto it = pendingEmails.begin(); it != pendingEmails.end(); ++it) {
+        PendingEmail &pending = it.value();
+        if (pending.nextAttempt > now) {
+            continue; // not yet due; honours the backoff schedule
+        }
+        int id = smtp->send(pending.message);
+        mailIdToUser.insert(id, pendingKey(pending.userName, pending.type, pending.token));
+    }
 }
 
 void SmtpClient::finished()
 {
-    qDebug() << "[MAIL] finished";
+    qCDebug(SmtpClientLog) << "finished";
     smtp->disconnectFromHost();
 }
 
+void SmtpClient::authenticated()
+{
+    qCDebug(SmtpClientLog) << "authenticated";
+    // The session has actually succeeded (TCP connect alone proves nothing:
+    // TLS/auth may still fail), so the connection-level backoff resets here.
+    connectionAttempts = 0;
+    // Start the idle timer here, once the connection is fully ready to send,
+    // rather than on TCP connect: SSL/STARTTLS handshake and auth may take a
+    // while and must not be cut short by the idle timeout.
+    idleTimer->start(IDLE_TIMEOUT_MS);
+}
+
+void SmtpClient::authenticationFailed(const QByteArray &msg)
+{
+    qCWarning(SmtpClientLog) << "authenticationFailed" << QString::fromUtf8(msg);
+    idleTimer->stop();
+    recordConnectionFailure();
+    mailIdToUser.clear();
+    smtp->disconnectFromHost();
+}
+
+void SmtpClient::connected()
+{
+    qCDebug(SmtpClientLog) << "connected";
+    // NOTE: connectionAttempts is deliberately not reset here. This fires on
+    // TCP connect, before the TLS handshake and AUTH; resetting here would pin
+    // the connection-level backoff at its base delay for exactly the failures
+    // (bad password/cert/STARTTLS) it is meant to cover. It resets in
+    // authenticated() and on the first successful mailSent instead.
+}
+
+void SmtpClient::connectionFailed(const QByteArray &msg)
+{
+    qCWarning(SmtpClientLog) << "connectionFailed" << QString::fromUtf8(msg);
+    idleTimer->stop();
+    recordConnectionFailure();
+    mailIdToUser.clear();
+    smtp->disconnectFromHost();
+}
+
+void SmtpClient::disconnected()
+{
+    qCDebug(SmtpClientLog) << "disconnected - emails still pending:" << pendingEmails.count();
+    idleTimer->stop();
+    // A mid-session drop (peer close/RST past the connect phase, or the idle
+    // timeout firing mid-batch) emits no mailFailed/connectionFailed signal, so
+    // route every in-flight mail through recordFailed(): otherwise attempts and
+    // backoff are bypassed and the mails requeue with stale state forever.
+    const QList<int> inFlight = mailIdToUser.keys();
+    for (int mailID : inFlight) {
+        recordFailed(mailID);
+    }
+    mailIdToUser.clear();
+}
+
+void SmtpClient::encrypted()
+{
+    qCDebug(SmtpClientLog) << "encrypted";
+}
+
+void SmtpClient::encryptionFailed(const QByteArray &msg)
+{
+    qCWarning(SmtpClientLog) << "encryptionFailed" << QString::fromUtf8(msg);
+    qCWarning(SmtpClientLog) << "Try enabling the \"acceptallcerts\" option in servatrice.ini";
+    idleTimer->stop();
+    recordConnectionFailure();
+    mailIdToUser.clear();
+    smtp->disconnectFromHost();
+}
+
+void SmtpClient::idleTimeout()
+{
+    qCDebug(SmtpClientLog) << "idle timeout - disconnecting";
+    smtp->disconnectFromHost();
+}
+
+void SmtpClient::recordFailed(int mailID)
+{
+    auto it = mailIdToUser.find(mailID);
+    if (it == mailIdToUser.end()) {
+        return;
+    }
+    const QString key = it.value();
+    mailIdToUser.erase(it);
+
+    auto pendingIt = pendingEmails.find(key);
+    if (pendingIt == pendingEmails.end()) {
+        return;
+    }
+
+    PendingEmail &pending = pendingIt.value();
+    pending.attempts++;
+
+    if (pending.attempts >= pending.maxAttempts) {
+        const QString userName = pending.userName;
+        const EmailType type = pending.type;
+        const QString token = pending.token;
+        const qint64 rowId = pending.rowId;
+        qCWarning(SmtpClientLog) << "Email to" << userName << "permanently failed after" << pending.attempts
+                                 << "attempts - dropping from pending set";
+        pendingEmails.erase(pendingIt);
+        emit mailPermanentlyFailed(userName, type, FailureReason::RetryExhausted, token, rowId);
+        return;
+    }
+
+    pending.nextAttempt = nextRetryTime(pending.attempts, pending.retryDelay);
+    qCWarning(SmtpClientLog) << "Email to" << pending.userName << "failed (attempt" << pending.attempts << "of"
+                             << pending.maxAttempts << "), retrying" << pending.nextAttempt.toString(Qt::ISODate);
+}
+
+void SmtpClient::recordConnectionFailure()
+{
+    ++connectionAttempts;
+    connectionBackoffUntil = nextRetryTime(connectionAttempts, retryDelaySeconds());
+    qCWarning(SmtpClientLog) << "Connection-layer failure #" << connectionAttempts << "- next attempt after"
+                             << connectionBackoffUntil.toString(Qt::ISODate);
+
+    // Connection failures count toward the per-email retry budget so that
+    // undeliverable mail eventually gives up rather than retrying forever.
+    QList<QString> keysToRemove;
+    for (auto it = pendingEmails.begin(); it != pendingEmails.end(); ++it) {
+        PendingEmail &pending = it.value();
+        pending.attempts++;
+        if (pending.attempts >= pending.maxAttempts) {
+            qCWarning(SmtpClientLog) << "Email to" << pending.userName << "permanently failed after" << pending.attempts
+                                     << "connection failures - dropping from pending set";
+            keysToRemove.append(it.key());
+            emit mailPermanentlyFailed(pending.userName, pending.type, FailureReason::RetryExhausted, pending.token,
+                                       pending.rowId);
+        }
+    }
+    for (const QString &key : keysToRemove) {
+        pendingEmails.remove(key);
+    }
+}
+
+bool SmtpClient::validateEmailFields(const QString &email,
+                                     const QString &subject,
+                                     const QString &body,
+                                     const QString &recipient,
+                                     const QString &token,
+                                     const QString &nickname)
+{
+    if (email.isEmpty()) {
+        qCCritical(SmtpClientLog) << "Missing sender email in configuration";
+        return false;
+    }
+    if (subject.isEmpty()) {
+        qCCritical(SmtpClientLog) << "Missing subject field in configuration";
+        return false;
+    }
+    if (body.isEmpty()) {
+        qCCritical(SmtpClientLog) << "Missing body field in configuration";
+        return false;
+    }
+    if (recipient.isEmpty()) {
+        qCCritical(SmtpClientLog) << "Missing recipient field for user" << nickname;
+        return false;
+    }
+    if (token.isEmpty()) {
+        qCCritical(SmtpClientLog) << "Missing token field for user" << nickname;
+        return false;
+    }
+    return true;
+}
+
 void SmtpClient::mailFailed(int mailID, int errorCode, const QByteArray &msg)
 {
-    qDebug() << "[MAIL] mailFailed id=" << mailID << " errorCode=" << errorCode << "msg=" << QString(msg);
+    qCWarning(SmtpClientLog) << "mailFailed id=" << mailID << " errorCode=" << errorCode
+                             << "msg=" << QString::fromUtf8(msg);
+    // Progress was made (the server answered), so the idle timer measures from
+    // here rather than acting as a cap on the whole session.
+    idleTimer->start(IDLE_TIMEOUT_MS);
+    recordFailed(mailID);
 }
 
 void SmtpClient::mailSent(int mailID)
 {
-    qDebug() << "[MAIL] mailSent" << mailID;
+    qCDebug(SmtpClientLog) << "mailSent" << mailID;
+    // A delivered mail proves the session works: reset the connection-level
+    // backoff and restart the idle timer from this point of progress.
+    connectionAttempts = 0;
+    idleTimer->start(IDLE_TIMEOUT_MS);
+
+    auto it = mailIdToUser.find(mailID);
+    if (it == mailIdToUser.end()) {
+        return;
+    }
+    const QString key = it.value();
+    mailIdToUser.erase(it);
+
+    auto pendingIt = pendingEmails.find(key);
+    if (pendingIt == pendingEmails.end()) {
+        return;
+    }
+
+    const QString userName = pendingIt.value().userName;
+    const EmailType type = pendingIt.value().type;
+    const QString token = pendingIt.value().token;
+    const qint64 rowId = pendingIt.value().rowId;
+    pendingEmails.erase(pendingIt);
+    qCDebug(SmtpClientLog) << "Delivered email to" << userName;
+    emit mailDelivered(userName, type, token, rowId);
 }
 
 void SmtpClient::recipientRejected(int mailID, const QString &address, const QByteArray &msg)
 {
-    qDebug() << "[MAIL] recipientRejected id=" << mailID << " address=" << address << "msg=" << QString(msg);
+    qCWarning(SmtpClientLog) << "recipientRejected id=" << mailID << " address=" << address
+                             << "msg=" << QString::fromUtf8(msg);
+    if (!isPermanentSmtpReply(msg)) {
+        // Transient 4xx (e.g. greylisting): consume one attempt and reschedule
+        // with backoff instead of destroying the durable queue row.
+        recordFailed(mailID);
+        return;
+    }
+    // A rejected recipient is a permanent condition; consider the mail undeliverable.
+    auto it = mailIdToUser.find(mailID);
+    if (it != mailIdToUser.end()) {
+        const QString key = it.value();
+        if (auto pendingIt = pendingEmails.find(key); pendingIt != pendingEmails.end()) {
+            const QString userName = pendingIt.value().userName;
+            const EmailType type = pendingIt.value().type;
+            const QString token = pendingIt.value().token;
+            const qint64 rowId = pendingIt.value().rowId;
+            pendingEmails.erase(pendingIt);
+            qCWarning(SmtpClientLog) << "Email to" << userName << "rejected - dropping from pending set";
+            emit mailPermanentlyFailed(userName, type, FailureReason::RecipientRejected, token, rowId);
+        }
+        mailIdToUser.erase(it);
+    }
 }
 
 void SmtpClient::senderRejected(int mailID, const QString &address, const QByteArray &msg)
 {
-    qDebug() << "[MAIL] senderRejected id=" << mailID << " address=" << address << "msg=" << QString(msg);
+    qCWarning(SmtpClientLog) << "senderRejected id=" << mailID << " sender=" << address
+                             << "msg=" << QString::fromUtf8(msg);
+    if (!isPermanentSmtpReply(msg)) {
+        // Transient 4xx on MAIL FROM (e.g. rate limiting): retry with backoff.
+        recordFailed(mailID);
+        return;
+    }
+    // A rejected sender is a permanent condition; consider the mail undeliverable.
+    auto it = mailIdToUser.find(mailID);
+    if (it != mailIdToUser.end()) {
+        const QString key = it.value();
+        if (auto pendingIt = pendingEmails.find(key); pendingIt != pendingEmails.end()) {
+            const QString userName = pendingIt.value().userName;
+            const EmailType type = pendingIt.value().type;
+            const QString token = pendingIt.value().token;
+            const qint64 rowId = pendingIt.value().rowId;
+            pendingEmails.erase(pendingIt);
+            qCWarning(SmtpClientLog) << "Email to" << userName << "sender rejected - dropping from pending set";
+            emit mailPermanentlyFailed(userName, type, FailureReason::SenderRejected, token, rowId);
+        }
+        mailIdToUser.erase(it);
+    }
 }

servatrice/src/smtpclient.h

@@ -1,26 +1,67 @@
 #ifndef SMTPCLIENT_H
 #define SMTPCLIENT_H
 
+#include "smtp/qxtmailmessage.h"
+
+#include <QDateTime>
+#include <QHash>
 #include <QObject>
 #include <QString>
+#include <QTimer>
 #include <qtmetamacros.h>
 
 class QxtSmtp;
 class QByteArray;
 
+/**
+ * @brief Types of e-mail the SMTP client is able to deliver.
+ */
+enum class EmailType
+{
+    Activation,
+    ForgotPassword
+};
+
+Q_DECLARE_METATYPE(EmailType)
+
+/**
+ * @brief Why a permanent e-mail delivery failure occurred.
+ */
+enum class FailureReason
+{
+    RetryExhausted,
+    RecipientRejected,
+    SenderRejected
+};
+
+Q_DECLARE_METATYPE(FailureReason)
+
 class SmtpClient : public QObject
 {
     Q_OBJECT
 public:
-    SmtpClient(QObject *parent = 0);
-    ~SmtpClient();
+    SmtpClient(QObject *parent = nullptr);
+    ~SmtpClient() override;
+
+public slots:
+    bool enqueueActivationTokenMail(const QString &nickname, const QString &recipient, const QString &token);
+    bool enqueueForgotPasswordTokenMail(const QString &nickname,
+                                        const QString &recipient,
+                                        const QString &token,
+                                        qint64 rowId = -1);
+    void sendAllEmails();
+
+signals:
+    void mailDelivered(const QString &userName, EmailType type, const QString &token, qint64 rowId);
+    void mailPermanentlyFailed(const QString &userName,
+                               EmailType type,
+                               FailureReason reason,
+                               const QString &token,
+                               qint64 rowId);
 
 protected:
     QxtSmtp *smtp;
-public slots:
-    bool enqueueActivationTokenMail(const QString &nickname, const QString &recipient, const QString &token);
-    bool enqueueForgotPasswordTokenMail(const QString &nickname, const QString &recipient, const QString &token);
-    void sendAllEmails();
+
 protected slots:
     void authenticated();
     void authenticationFailed(const QByteArray &msg);
@@ -30,10 +71,55 @@ protected slots:
     void encrypted();
     void encryptionFailed(const QByteArray &msg);
     void finished();
+    void idleTimeout();
     void mailFailed(int mailID, int errorCode, const QByteArray &msg);
     void mailSent(int mailID);
     void recipientRejected(int mailID, const QString &address, const QByteArray &msg);
     void senderRejected(int mailID, const QString &address, const QByteArray &msg);
+
+private:
+    struct PendingEmail
+    {
+        QString userName;
+        EmailType type;
+        QString token;
+        // Database row id for tables that allow multiple rows per user
+        // ({prefix}_forgot_password); -1 when the name alone identifies the row.
+        qint64 rowId = -1;
+        QxtMailMessage message;
+        int attempts = 0;
+        int maxAttempts = 0;
+        int retryDelay = 0;
+        QDateTime nextAttempt;
+    };
+
+    PendingEmail buildEmail(const QString &nickname,
+                            EmailType type,
+                            const QString &subject,
+                            const QString &body,
+                            const QString &recipient,
+                            const QString &token,
+                            const QString &email,
+                            qint64 rowId = -1);
+    bool enqueue(const PendingEmail &email);
+    void relayoutForSending();
+    QDateTime nextRetryTime(int attempts, int retryDelay) const;
+    void recordFailed(int mailID);
+    void recordConnectionFailure();
+    bool validateEmailFields(const QString &email,
+                             const QString &subject,
+                             const QString &body,
+                             const QString &recipient,
+                             const QString &token,
+                             const QString &nickname);
+    int maxRetries() const;
+    int retryDelaySeconds() const;
+
+    QHash<QString, PendingEmail> pendingEmails;
+    QHash<int, QString> mailIdToUser;
+    QTimer *idleTimer;
+    int connectionAttempts = 0;
+    QDateTime connectionBackoffUntil;
 };
 
-#endif
+#endif

tests/CMakeLists.txt

@@ -2,109 +2,10 @@
 # If a new test needs additional Qt modules, add them there — not in individual test CMakeLists.txt files.
 enable_testing()
 
-add_test(NAME dummy_test COMMAND dummy_test)
-add_test(NAME expression_test COMMAND expression_test)
-add_test(NAME clamped_arithmetic_test COMMAND clamped_arithmetic_test)
-add_test(NAME test_age_formatting COMMAND test_age_formatting)
-add_test(NAME password_hash_test COMMAND password_hash_test)
-add_test(NAME playmat_resolver_test COMMAND playmat_resolver_test)
-add_test(NAME server_card_counter_test COMMAND server_card_counter_test)
-add_test(NAME server_counter_test COMMAND server_counter_test)
-add_test(NAME server_rate_limiter_test COMMAND server_rate_limiter_test)
-add_test(NAME server_developer_role_test COMMAND server_developer_role_test)
-add_test(NAME server_game_join_test COMMAND server_game_join_test)
-add_test(NAME warning_categories_test COMMAND warning_categories_test)
-# The translation loader test compiles a .qm fixture, which needs lrelease from Qt Linguist.
-if(TARGET Qt6::lrelease)
-  add_test(NAME translation_loader_test COMMAND translation_loader_test)
-endif()
-add_test(NAME report_categories_test COMMAND report_categories_test)
-add_test(NAME lag_monitor_test COMMAND lag_monitor_test)
-add_test(NAME latency_tracker_test COMMAND latency_tracker_test)
-add_test(NAME metrics_registry_test COMMAND metrics_registry_test)
-add_test(NAME card_update_progress_test COMMAND card_update_progress_test)
-add_test(NAME loader_local_matching_test COMMAND loader_local_matching_test)
-
-add_test(NAME deck_hash_performance_test COMMAND deck_hash_performance_test)
-set_tests_properties(deck_hash_performance_test PROPERTIES TIMEOUT 15)
+include(GoogleTest)
 
 # Find GTest
 
-add_executable(dummy_test dummy_test.cpp)
-add_executable(expression_test expression_test.cpp)
-add_executable(clamped_arithmetic_test clamped_arithmetic_test.cpp)
-add_executable(test_age_formatting test_age_formatting.cpp)
-add_executable(password_hash_test password_hash_test.cpp)
-add_executable(playmat_resolver_test playmat_resolver_test.cpp)
-add_executable(deck_hash_performance_test deck_hash_performance_test.cpp)
-add_executable(server_card_counter_test server_card_counter_test.cpp)
-add_executable(server_counter_test server_counter_test.cpp)
-add_executable(server_rate_limiter_test server_rate_limiter_test.cpp)
-add_executable(server_developer_role_test server_developer_role_test.cpp)
-add_executable(server_game_join_test server_game_join_test.cpp)
-add_executable(warning_categories_test warning_categories_test.cpp)
-if(TARGET Qt6::lrelease)
-  add_executable(translation_loader_test translation_loader_test.cpp)
-
-  # QTranslator only accepts real .qm files, so the translation loader test gets one compiled from its
-  # own .ts fixture rather than a stub.
-  set(TRANSLATION_LOADER_TEST_QM "${CMAKE_CURRENT_BINARY_DIR}/translation_loader_test_de.qm")
-  add_custom_command(
-    OUTPUT "${TRANSLATION_LOADER_TEST_QM}"
-    COMMAND Qt6::lrelease "${CMAKE_CURRENT_SOURCE_DIR}/translation_loader_test.ts" -qm "${TRANSLATION_LOADER_TEST_QM}"
-    DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/translation_loader_test.ts"
-    COMMENT "Compiling translation_loader_test.ts"
-  )
-  add_custom_target(translation_loader_test_qm DEPENDS "${TRANSLATION_LOADER_TEST_QM}")
-  add_dependencies(translation_loader_test translation_loader_test_qm)
-  target_compile_definitions(translation_loader_test PRIVATE TRANSLATION_LOADER_TEST_DIR="${CMAKE_CURRENT_BINARY_DIR}")
-
-  # A multi-config generator (MSBuild, Xcode) puts the executable in a per-config subdirectory while
-  # the .qm stays in the current binary directory, so copy the fixture next to the binary: that is the
-  # layout ABuildTreeFindsTheTranslationsNextToTheExecutable is about, and the only one it finds on
-  # those generators.
-  add_custom_command(
-    TARGET translation_loader_test
-    POST_BUILD
-    COMMAND ${CMAKE_COMMAND} -E copy_if_different "${TRANSLATION_LOADER_TEST_QM}"
-            "$<TARGET_FILE_DIR:translation_loader_test>/"
-    VERBATIM
-  )
-endif()
-add_executable(report_categories_test report_categories_test.cpp)
-add_executable(lag_monitor_test ${CMAKE_SOURCE_DIR}/cockatrice/src/client/lag_monitor.cpp lag_monitor_test.cpp)
-target_include_directories(lag_monitor_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src)
-add_executable(latency_tracker_test latency_tracker_test.cpp)
-add_executable(metrics_registry_test ../servatrice/src/metrics_registry.cpp metrics_registry_test.cpp)
-add_executable(
-  card_update_progress_test ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/card_database_update/card_update_progress.cpp
-                            card_update_progress_test.cpp
-)
-target_include_directories(card_update_progress_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src)
-add_executable(
-  loader_local_matching_test
-  ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/card_picture_loader/card_picture_loader_local.cpp
-  ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/cache_settings.cpp
-  ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/card_counter_settings.cpp
-  ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/shortcuts_settings.cpp
-  ${CMAKE_SOURCE_DIR}/cockatrice/src/client/network/update/client/release_channel.cpp
-  ${VERSION_STRING_CPP}
-  loader_local_matching_test.cpp
-)
-target_include_directories(loader_local_matching_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src)
-
-# Guard must match the condition for src/single_instance_manager.cpp in cockatrice/CMakeLists.txt.
-if(WITH_CLIENT)
-  add_executable(
-    single_instance_manager_test ${CMAKE_SOURCE_DIR}/cockatrice/src/single_instance_manager.cpp
-                                 single_instance_manager_test.cpp
-  )
-  target_include_directories(single_instance_manager_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src)
-  target_include_directories(single_instance_manager_test PRIVATE ${CMAKE_SOURCE_DIR}/libcockatrice_utility)
-
-  add_test(NAME single_instance_manager_test COMMAND single_instance_manager_test)
-endif()
-
 find_package(GTest)
 
 if(NOT GTEST_FOUND)
@@ -129,93 +30,342 @@ if(NOT GTEST_FOUND)
   target_include_directories(gtest INTERFACE "$<BUILD_INTERFACE:${CMAKE_BINARY_DIR}/gtest-src/include>")
 
   set(GTEST_INCLUDE_DIRS "${CMAKE_BINARY_DIR}/gtest-src/include")
-  set(GTEST_BOTH_LIBRARIES gtest)
-  add_dependencies(dummy_test gtest)
-  add_dependencies(expression_test gtest)
-  add_dependencies(clamped_arithmetic_test gtest)
-  add_dependencies(test_age_formatting gtest)
-  add_dependencies(password_hash_test gtest)
-  add_dependencies(playmat_resolver_test gtest)
-  add_dependencies(deck_hash_performance_test gtest)
-  add_dependencies(server_card_counter_test gtest)
-  add_dependencies(server_counter_test gtest)
-  add_dependencies(server_rate_limiter_test gtest)
-  add_dependencies(server_developer_role_test gtest)
-  add_dependencies(server_game_join_test gtest)
-  add_dependencies(warning_categories_test gtest)
-  if(TARGET translation_loader_test)
-    add_dependencies(translation_loader_test gtest)
-  endif()
-  add_dependencies(report_categories_test gtest)
-  add_dependencies(lag_monitor_test gtest)
-  add_dependencies(latency_tracker_test gtest)
-  add_dependencies(metrics_registry_test gtest)
-  add_dependencies(card_update_progress_test gtest)
-  add_dependencies(loader_local_matching_test gtest)
-  add_dependencies(single_instance_manager_test gtest)
+  # gtest_main is not optional here. FindGTest sets GTEST_BOTH_LIBRARIES to both halves, so this
+  # has to match it or the two paths disagree about what provides main(). A test source that
+  # defines its own main() gets away with the omission; one that does not fails at link with
+  # `undefined reference to main` pointing at crt1.o, which names neither gtest nor this line.
+  set(GTEST_BOTH_LIBRARIES gtest gtest_main)
 endif()
 
+# Directory-scoped on purpose: the test subdirectories below add targets that include the
+# fetched gtest headers and do not repeat this path themselves.
 include_directories(${GTEST_INCLUDE_DIRS})
-target_link_libraries(dummy_test Threads::Threads ${GTEST_BOTH_LIBRARIES})
-target_link_libraries(expression_test libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES})
-target_link_libraries(
-  clamped_arithmetic_test libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
-)
-target_link_libraries(
-  test_age_formatting libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
-)
-target_link_libraries(
-  password_hash_test libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
-)
-target_link_libraries(
-  playmat_resolver_test libcockatrice_deck_list libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES}
-  ${TEST_QT_MODULES}
-)
-target_link_libraries(
-  deck_hash_performance_test libcockatrice_deck_list libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES}
-  ${TEST_QT_MODULES}
-)
-target_link_libraries(
-  server_card_counter_test libcockatrice_network Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
-)
-target_link_libraries(
-  server_counter_test libcockatrice_network Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
-)
-target_link_libraries(
-  server_rate_limiter_test libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
-)
-target_link_libraries(
-  server_developer_role_test libcockatrice_network libcockatrice_rng Threads::Threads ${GTEST_BOTH_LIBRARIES}
-  ${TEST_QT_MODULES}
-)
-target_link_libraries(
-  server_game_join_test libcockatrice_network_server_remote libcockatrice_rng Threads::Threads ${GTEST_BOTH_LIBRARIES}
-  ${TEST_QT_MODULES}
-)
-target_link_libraries(
-  warning_categories_test libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
-)
-if(TARGET translation_loader_test)
+
+# --- Test registration ----------------------------------------------------------
+#
+# Every test target needs the same four steps: build the executable, depend on gtest when it
+# was fetched from source, link the shared test libraries, and hand the cases to CTest. The
+# `if(NOT GTEST_FOUND)` branch is the easy one to forget — omit it and the link fails with
+# undefined gtest symbols — so it lives here once instead of being repeated per test.
+#
+# This also replaces `add_test(NAME x COMMAND x)`. Naming the binary as a single CTest entry
+# hides which case failed and makes `ctest -R Suite.Case` impossible; discovery exposes each
+# TEST() as its own CTest entry instead.
+#
+# Labels: exactly one per test, because gtest_discover_tests() cannot carry more than one.
+# GoogleTestAddTests.cmake accumulates the generated script with `foreach(arg ${ARGN})`, so a
+# label list serialises as bare `LABELS ci server` in the CTestTestfile, and CTest reads only
+# the first value of a property and silently discards the rest. A label therefore classifies a
+# test into exactly one group, and the three groups cover every selection worth making:
+#
+#   ci      the default hermetic set, so `ctest -LE bench` runs everything but benchmarks
+#   server  servatrice-facing tests, so `ctest -L server` works
+#   bench   the slow suites, kept out of the default run
+#
+# A second label is a configure-time FATAL_ERROR rather than a line CTest quietly reads as one.
+# Anything orthogonal to a test's kind belongs in a helper flag beside the label, so it
+# composes with the group instead of competing for the single slot. NO_SANITIZE is such a flag.
+#
+# TIMEOUT is bounded per test because CTest's implicit default is 1500s: one hung socket wait
+# would otherwise stall CI for 25 minutes.
+#
+# EXTRA_LIBS is appended after the shared libraries so a caller adding a static archive (zlib,
+# lzma) keeps it last, which is where the linker needs it.
+
+function(cockatrice_add_test)
+  cmake_parse_arguments(
+    PARSE_ARGV 0 ARG "MANUAL;NO_SANITIZE" "NAME" "SOURCES;LIBS;LABELS;DEFINITIONS;INCLUDE_DIRS;EXTRA_LIBS"
+  )
+
+  if(NOT ARG_NAME OR NOT ARG_SOURCES)
+    message(FATAL_ERROR "cockatrice_add_test requires at least NAME and SOURCES")
+  endif()
+
+  add_executable(${ARG_NAME} ${ARG_SOURCES})
+
+  if(ARG_INCLUDE_DIRS)
+    target_include_directories(${ARG_NAME} PRIVATE ${ARG_INCLUDE_DIRS})
+  endif()
+
+  if(ARG_DEFINITIONS)
+    target_compile_definitions(${ARG_NAME} PRIVATE ${ARG_DEFINITIONS})
+  endif()
+
   target_link_libraries(
-    translation_loader_test libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
+    ${ARG_NAME} ${ARG_LIBS} Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES} ${ARG_EXTRA_LIBS}
+  )
+
+  # Only needed when gtest was downloaded and add_subdirectory'd above. An imported gtest
+  # target already carries its own dependency, so this must stay conditional.
+  if(NOT GTEST_FOUND)
+    add_dependencies(${ARG_NAME} gtest gtest_main)
+  endif()
+
+  # MANUAL targets are built but deliberately kept out of ctest: benchmarks that need a card
+  # database, hit the network, or measure RAM. Leaving them unregistered keeps CI hermetic
+  # and fast. They still link gtest so they can use TEST() without a second code path.
+  if(ARG_MANUAL)
+    return()
+  endif()
+
+  # NO_SANITIZE marks a target the sanitizer leg cannot run. Excluded by not handing its cases
+  # to CTest at all, rather than by a label, so the exclusion cannot rot into a line CTest
+  # quietly reads as something else. The target still builds, and still runs on the other legs.
+  if(ARG_NO_SANITIZE AND COCKATRICE_SANITIZERS)
+    return()
+  endif()
+
+  # One label only, per the note above. Rejecting a second loudly beats emitting a
+  # CTestTestfile line that CTest will quietly read as one label.
+  list(LENGTH ARG_LABELS _label_count)
+  if(_label_count GREATER 1)
+    message(FATAL_ERROR "${ARG_NAME}: pass at most one label, got '${ARG_LABELS}'")
+  endif()
+  if(NOT ARG_LABELS)
+    set(ARG_LABELS ci)
+  endif()
+
+  # GoogleTestAddTests defaults DISCOVERY_TIMEOUT to 5s, which is ample for a plain debug
+  # binary (--gtest_list_tests costs 0.04s) but not for an instrumented one (~2.8s cold, and
+  # discovery runs several binaries concurrently behind a -j build). Left at the default, a
+  # sanitizer build fails at the *link* step with a bare "Process terminated due to timeout"
+  # and no hint that discovery, not the test, is what is slow.
+  if(COCKATRICE_SANITIZERS)
+    set(_discovery_timeout 300)
+  else()
+    set(_discovery_timeout 60)
+  endif()
+
+  gtest_discover_tests(
+    ${ARG_NAME} DISCOVERY_MODE POST_BUILD DISCOVERY_TIMEOUT ${_discovery_timeout}
+    PROPERTIES LABELS "${ARG_LABELS}"
+    TIMEOUT 60
+  )
+endfunction()
+
+# Same wiring for targets that are built but deliberately kept out of ctest: benchmarks that
+# need a card database, hit the network, or measure RAM. A separate name rather than a MANUAL
+# flag spelled out at each call site, so "this is not in the CI suite" is visible in the CMake
+# file. Note the unquoted expansions: an unset multi-value keyword expands to zero arguments,
+# which is what lets this forward a caller's keywords verbatim without tripping the FATAL_ERROR
+# guard above on an empty list.
+function(cockatrice_add_manual_test)
+  cmake_parse_arguments(PARSE_ARGV 0 ARG "" "NAME" "SOURCES;LIBS;DEFINITIONS;INCLUDE_DIRS;EXTRA_LIBS")
+
+  cockatrice_add_test(
+    NAME
+    ${ARG_NAME}
+    SOURCES
+    ${ARG_SOURCES}
+    LIBS
+    ${ARG_LIBS}
+    DEFINITIONS
+    ${ARG_DEFINITIONS}
+    INCLUDE_DIRS
+    ${ARG_INCLUDE_DIRS}
+    EXTRA_LIBS
+    ${ARG_EXTRA_LIBS}
+    MANUAL
+  )
+endfunction()
+
+# --- Unit tests ------------------------------------------------------------------
+
+cockatrice_add_test(NAME expression_test SOURCES expression_test.cpp LIBS libcockatrice_utility)
+cockatrice_add_test(NAME clamped_arithmetic_test SOURCES clamped_arithmetic_test.cpp LIBS libcockatrice_utility)
+cockatrice_add_test(NAME test_age_formatting SOURCES test_age_formatting.cpp LIBS libcockatrice_utility)
+cockatrice_add_test(NAME password_hash_test SOURCES password_hash_test.cpp LIBS libcockatrice_utility)
+cockatrice_add_test(NAME report_categories_test SOURCES report_categories_test.cpp LIBS libcockatrice_utility)
+cockatrice_add_test(NAME warning_categories_test SOURCES warning_categories_test.cpp LIBS libcockatrice_utility)
+cockatrice_add_test(
+  NAME
+  playmat_resolver_test
+  SOURCES
+  playmat_resolver_test.cpp
+  LIBS
+  libcockatrice_deck_list
+  libcockatrice_utility
+)
+cockatrice_add_test(
+  NAME
+  server_rate_limiter_test
+  SOURCES
+  server_rate_limiter_test.cpp
+  LIBS
+  libcockatrice_utility
+  LABELS
+  server
+)
+cockatrice_add_test(
+  NAME
+  server_card_counter_test
+  SOURCES
+  server_card_counter_test.cpp
+  LIBS
+  libcockatrice_network
+  LABELS
+  server
+)
+cockatrice_add_test(
+  NAME
+  server_counter_test
+  SOURCES
+  server_counter_test.cpp
+  LIBS
+  libcockatrice_network
+  LABELS
+  server
+)
+cockatrice_add_test(
+  NAME
+  server_developer_role_test
+  NO_SANITIZE
+  SOURCES
+  server_developer_role_test.cpp
+  LIBS
+  libcockatrice_network
+  libcockatrice_rng
+  LABELS
+  server
+)
+cockatrice_add_test(
+  NAME
+  server_moderator_role_test
+  NO_SANITIZE
+  SOURCES
+  server_moderator_role_test.cpp
+  LIBS
+  libcockatrice_network
+  libcockatrice_rng
+  LABELS
+  server
+)
+cockatrice_add_test(
+  NAME
+  server_game_join_test
+  SOURCES
+  server_game_join_test.cpp
+  LIBS
+  libcockatrice_network_server_remote
+  libcockatrice_rng
+  LABELS
+  server
+)
+cockatrice_add_test(
+  NAME
+  latency_tracker_test
+  SOURCES
+  latency_tracker_test.cpp
+  LIBS
+  libcockatrice_network
+  LABELS
+  server
+)
+cockatrice_add_test(
+  NAME
+  lag_monitor_test
+  SOURCES
+  ${CMAKE_SOURCE_DIR}/cockatrice/src/client/lag_monitor.cpp
+  lag_monitor_test.cpp
+  INCLUDE_DIRS
+  ${CMAKE_SOURCE_DIR}/cockatrice/src
+)
+cockatrice_add_test(
+  NAME
+  metrics_registry_test
+  SOURCES
+  ${CMAKE_SOURCE_DIR}/servatrice/src/metrics_registry.cpp
+  metrics_registry_test.cpp
+  INCLUDE_DIRS
+  ${CMAKE_SOURCE_DIR}/servatrice/src
+  LABELS
+  server
+)
+cockatrice_add_test(
+  NAME
+  card_update_progress_test
+  SOURCES
+  ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/card_database_update/card_update_progress.cpp
+  card_update_progress_test.cpp
+  INCLUDE_DIRS
+  ${CMAKE_SOURCE_DIR}/cockatrice/src
+)
+cockatrice_add_test(
+  NAME
+  loader_local_matching_test
+  SOURCES
+  ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/card_picture_loader/card_picture_loader_local.cpp
+  ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/cache_settings.cpp
+  ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/card_counter_settings.cpp
+  ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/shortcuts_settings.cpp
+  ${CMAKE_SOURCE_DIR}/cockatrice/src/client/network/update/client/release_channel.cpp
+  ${VERSION_STRING_CPP}
+  loader_local_matching_test.cpp
+  LIBS
+  libcockatrice_settings
+  INCLUDE_DIRS
+  ${CMAKE_SOURCE_DIR}/cockatrice/src
+)
+cockatrice_add_test(
+  NAME
+  deck_hash_performance_test
+  SOURCES
+  deck_hash_performance_test.cpp
+  LIBS
+  libcockatrice_deck_list
+  libcockatrice_utility
+  LABELS
+  bench
+)
+
+# The translation loader test compiles a .qm fixture, which needs lrelease from Qt Linguist.
+if(TARGET Qt6::lrelease)
+  set(TRANSLATION_LOADER_TEST_QM "${CMAKE_CURRENT_BINARY_DIR}/translation_loader_test_de.qm")
+  add_custom_command(
+    OUTPUT "${TRANSLATION_LOADER_TEST_QM}"
+    COMMAND Qt6::lrelease "${CMAKE_CURRENT_SOURCE_DIR}/translation_loader_test.ts" -qm "${TRANSLATION_LOADER_TEST_QM}"
+    DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/translation_loader_test.ts"
+    COMMENT "Compiling translation_loader_test.ts"
+  )
+  add_custom_target(translation_loader_test_qm DEPENDS "${TRANSLATION_LOADER_TEST_QM}")
+
+  cockatrice_add_test(
+    NAME
+    translation_loader_test
+    SOURCES
+    translation_loader_test.cpp
+    LIBS
+    libcockatrice_utility
+    DEFINITIONS
+    TRANSLATION_LOADER_TEST_DIR="${CMAKE_CURRENT_BINARY_DIR}"
+  )
+  add_dependencies(translation_loader_test translation_loader_test_qm)
+
+  # A multi-config generator (MSBuild, Xcode) puts the executable in a per-config subdirectory while
+  # the .qm stays in the current binary directory, so copy the fixture next to the binary: that is the
+  # layout ABuildTreeFindsTheTranslationsNextToTheExecutable is about, and the only one it finds on
+  # those generators.
+  add_custom_command(
+    TARGET translation_loader_test
+    POST_BUILD
+    COMMAND ${CMAKE_COMMAND} -E copy_if_different "${TRANSLATION_LOADER_TEST_QM}"
+            "$<TARGET_FILE_DIR:translation_loader_test>/"
+    VERBATIM
   )
 endif()
-target_link_libraries(
-  report_categories_test libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
-)
-target_link_libraries(lag_monitor_test Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES})
-target_link_libraries(
-  latency_tracker_test libcockatrice_network Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
-)
-target_include_directories(metrics_registry_test PRIVATE ${CMAKE_SOURCE_DIR}/servatrice/src)
-target_link_libraries(metrics_registry_test ${TEST_QT_MODULES} Threads::Threads ${GTEST_BOTH_LIBRARIES})
-target_link_libraries(card_update_progress_test Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES})
-target_link_libraries(
-  loader_local_matching_test libcockatrice_settings Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
-)
 
+# Guard must match the condition for src/single_instance_manager.cpp in cockatrice/CMakeLists.txt.
 if(WITH_CLIENT)
-  target_link_libraries(single_instance_manager_test Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES})
+  cockatrice_add_test(
+    NAME
+    single_instance_manager_test
+    SOURCES
+    ${CMAKE_SOURCE_DIR}/cockatrice/src/single_instance_manager.cpp
+    single_instance_manager_test.cpp
+    INCLUDE_DIRS
+    ${CMAKE_SOURCE_DIR}/cockatrice/src
+    ${CMAKE_SOURCE_DIR}/libcockatrice_utility
+  )
 endif()
 
 add_subdirectory(card_zone_algorithms)

tests/card_zone_algorithms/CMakeLists.txt

@@ -1,15 +1,4 @@
-add_executable(card_zone_algorithms_test card_zone_algorithms_test.cpp)
-
-target_include_directories(card_zone_algorithms_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src/game/zones)
-
-target_link_libraries(
-  card_zone_algorithms_test
-  PRIVATE Threads::Threads
-  PRIVATE ${GTEST_BOTH_LIBRARIES}
+cockatrice_add_test(
+  NAME card_zone_algorithms_test SOURCES card_zone_algorithms_test.cpp INCLUDE_DIRS
+  ${CMAKE_SOURCE_DIR}/cockatrice/src/game/zones
 )
-
-add_test(NAME card_zone_algorithms_test COMMAND card_zone_algorithms_test)
-
-if(NOT GTEST_FOUND)
-  add_dependencies(card_zone_algorithms_test gtest)
-endif()

tests/carddatabase/CMakeLists.txt

@@ -9,61 +9,46 @@ add_definitions("-DCARDDB_DATADIR=\"${CMAKE_CURRENT_SOURCE_DIR}/data/\"")
 # ------------------------
 # Card Database Test
 # ------------------------
-add_executable(carddatabase_test ${MOCKS_SOURCES} ${VERSION_STRING_CPP} carddatabase_test.cpp mocks.cpp)
-
-target_link_libraries(
+cockatrice_add_test(
+  NAME
   carddatabase_test
-  PRIVATE libcockatrice_card
-  PRIVATE Threads::Threads
-  PRIVATE ${GTEST_BOTH_LIBRARIES}
-  PRIVATE ${TEST_QT_MODULES}
+  SOURCES
+  ${VERSION_STRING_CPP}
+  carddatabase_test.cpp
+  mocks.cpp
+  LIBS
+  libcockatrice_card
 )
 
-add_test(NAME carddatabase_test COMMAND carddatabase_test)
-
-# ------------------------
-# Load Benchmark (manual, not run in CI)
-# ------------------------
-add_executable(load_benchmark ${MOCKS_SOURCES} ${VERSION_STRING_CPP} load_benchmark.cpp mocks.cpp)
-
-target_link_libraries(
-  load_benchmark
-  PRIVATE libcockatrice_card
-  PRIVATE libcockatrice_models
-  PRIVATE Threads::Threads
-  PRIVATE ${GTEST_BOTH_LIBRARIES}
-  PRIVATE ${TEST_QT_MODULES}
-)
-
-if(NOT GTEST_FOUND)
-  add_dependencies(load_benchmark gtest)
-endif()
-
 # ------------------------
 # Filter String Test
 # (guard must match the condition for libcockatrice_filters in the root CMakeLists.txt)
 # ------------------------
 if(WITH_ORACLE OR WITH_CLIENT)
-  add_executable(filter_string_test ${MOCKS_SOURCES} ${VERSION_STRING_CPP} filter_string_test.cpp mocks.cpp)
-
-  target_link_libraries(
+  cockatrice_add_test(
+    NAME
     filter_string_test
-    PRIVATE libcockatrice_filters
-    PRIVATE Threads::Threads
-    PRIVATE ${GTEST_BOTH_LIBRARIES}
-    PRIVATE ${TEST_QT_MODULES}
+    SOURCES
+    ${VERSION_STRING_CPP}
+    filter_string_test.cpp
+    mocks.cpp
+    LIBS
+    libcockatrice_filters
   )
-
-  add_test(NAME filter_string_test COMMAND filter_string_test)
-
-  if(NOT GTEST_FOUND)
-    add_dependencies(filter_string_test gtest)
-  endif()
 endif()
 
 # ------------------------
-# Dependencies on gtest
+# Load Benchmark (manual, not run in CI)
 # ------------------------
-if(NOT GTEST_FOUND)
-  add_dependencies(carddatabase_test gtest)
-endif()
+cockatrice_add_manual_test(
+  NAME
+  load_benchmark
+  SOURCES
+  ${VERSION_STRING_CPP}
+  load_benchmark.cpp
+  mocks.cpp
+  LIBS
+  libcockatrice_card
+  libcockatrice_models
+  MANUAL
+)

tests/deck_list_model/CMakeLists.txt

@@ -1,33 +1,23 @@
-add_executable(deck_list_model_custom_zones_test ${VERSION_STRING_CPP} deck_list_model_custom_zones_test.cpp)
-
-if(NOT GTEST_FOUND)
-  add_dependencies(deck_list_model_custom_zones_test gtest)
-endif()
-
-target_link_libraries(
+cockatrice_add_test(
+  NAME
   deck_list_model_custom_zones_test
+  SOURCES
+  ${VERSION_STRING_CPP}
+  deck_list_model_custom_zones_test.cpp
+  LIBS
   libcockatrice_models
   libcockatrice_card
   libcockatrice_deck_list
-  Threads::Threads
-  ${GTEST_BOTH_LIBRARIES}
-  ${TEST_QT_MODULES}
 )
-add_test(NAME deck_list_model_custom_zones_test COMMAND deck_list_model_custom_zones_test)
 
-add_executable(deck_list_model_zone_integration_test ${VERSION_STRING_CPP} deck_list_model_zone_integration_test.cpp)
-
-if(NOT GTEST_FOUND)
-  add_dependencies(deck_list_model_zone_integration_test gtest)
-endif()
-
-target_link_libraries(
+cockatrice_add_test(
+  NAME
   deck_list_model_zone_integration_test
+  SOURCES
+  ${VERSION_STRING_CPP}
+  deck_list_model_zone_integration_test.cpp
+  LIBS
   libcockatrice_models
   libcockatrice_card
   libcockatrice_deck_list
-  Threads::Threads
-  ${GTEST_BOTH_LIBRARIES}
-  ${TEST_QT_MODULES}
 )
-add_test(NAME deck_list_model_zone_integration_test COMMAND deck_list_model_zone_integration_test)

tests/deck_list_zones/CMakeLists.txt

@@ -1,10 +1 @@
-add_executable(deck_list_zones_test deck_list_zones_test.cpp)
-
-if(NOT GTEST_FOUND)
-  add_dependencies(deck_list_zones_test gtest)
-endif()
-
-target_link_libraries(
-  deck_list_zones_test libcockatrice_deck_list Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
-)
-add_test(NAME deck_list_zones_test COMMAND deck_list_zones_test)
+cockatrice_add_test(NAME deck_list_zones_test SOURCES deck_list_zones_test.cpp LIBS libcockatrice_deck_list)

tests/dummy_test.cpp

@@ -1,20 +0,0 @@
-#include "gtest/gtest.h"
-#include <string>
-
-namespace
-{
-class FooTest : public ::testing::Test
-{
-};
-
-TEST(DummyTest, Works)
-{
-    ASSERT_EQ(1, 1) << "One is not equal to one";
-}
-} // namespace
-
-int main(int argc, char **argv)
-{
-    ::testing::InitGoogleTest(&argc, argv);
-    return RUN_ALL_TESTS();
-}

tests/loading_from_clipboard/CMakeLists.txt

@@ -1,12 +1,13 @@
 add_definitions("-DCARDDB_DATADIR=\"${CMAKE_CURRENT_SOURCE_DIR}/data/\"")
-add_executable(loading_from_clipboard_test ${VERSION_STRING_CPP} clipboard_testing.cpp loading_from_clipboard_test.cpp)
 
-if(NOT GTEST_FOUND)
-  add_dependencies(loading_from_clipboard_test gtest)
-endif()
-
-target_link_libraries(
-  loading_from_clipboard_test libcockatrice_deck_list libcockatrice_card Threads::Threads ${GTEST_BOTH_LIBRARIES}
-  ${TEST_QT_MODULES}
+cockatrice_add_test(
+  NAME
+  loading_from_clipboard_test
+  SOURCES
+  ${VERSION_STRING_CPP}
+  clipboard_testing.cpp
+  loading_from_clipboard_test.cpp
+  LIBS
+  libcockatrice_deck_list
+  libcockatrice_card
 )
-add_test(NAME loading_from_clipboard_test COMMAND loading_from_clipboard_test)

tests/loading_from_clipboard/loading_from_clipboard_test.cpp

@@ -80,6 +80,20 @@ TEST(LoadingFromClipboardTest, WeirdWhitespaceIsIgnored)
     testDeck(clipboard, result);
 }
 
+TEST(LoadingFromClipboardTest, TabSeparatedImport)
+{
+    QString clipboard("2\tMystic Snake\tStreets of New Capenna\n"
+                      "3x\tCounterspell\tNPH\n"
+                      "\t2\tSol Ring\tFMB\n"
+                      "1\tForest\n"
+                      "\n"
+                      "2x\tDoom Blade\tM11\n");
+
+    Result result("", "", {{"Mystic Snake", 2}, {"Counterspell", 3}, {"Sol Ring", 2}, {"Forest", 1}},
+                  {{"Doom Blade", 2}});
+    testDeck(clipboard, result);
+}
+
 TEST(LoadingFromClipboardTest, RemoveBlankEntriesFromBeginningAndEnd)
 {
     QString clipboard("\n"

tests/movecard_tests/CMakeLists.txt

@@ -1,16 +1,11 @@
-add_executable(reverse_card_move_test reverse_card_move_test.cpp)
-
-if(NOT GTEST_FOUND)
-  add_dependencies(reverse_card_move_test gtest)
-endif()
-
-target_link_libraries(
+cockatrice_add_test(
+  NAME
   reverse_card_move_test
-  PRIVATE libcockatrice_network_server_remote
-  PRIVATE libcockatrice_rng
-  PRIVATE Threads::Threads
-  PRIVATE ${GTEST_BOTH_LIBRARIES}
-  PRIVATE ${TEST_QT_MODULES}
+  SOURCES
+  reverse_card_move_test.cpp
+  LIBS
+  libcockatrice_network_server_remote
+  libcockatrice_rng
+  LABELS
+  server
 )
-
-add_test(NAME reverse_card_move_test COMMAND reverse_card_move_test)

tests/oracle/CMakeLists.txt

@@ -1,37 +1,32 @@
-add_executable(parse_cipt_test ../../oracle/src/parsehelpers.cpp parse_cipt_test.cpp)
-
-if(NOT GTEST_FOUND)
-  add_dependencies(parse_cipt_test gtest)
-endif()
-
-target_link_libraries(parse_cipt_test Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES})
-
-add_test(NAME parse_cipt_test COMMAND parse_cipt_test)
+cockatrice_add_test(NAME parse_cipt_test SOURCES ../../oracle/src/parsehelpers.cpp parse_cipt_test.cpp)
 
 # Oracle importer unit tests
-add_executable(
-  oracle_importer_test ${VERSION_STRING_CPP} ../../oracle/src/oracleimporter.cpp ../../oracle/src/parsehelpers.cpp
-                       ../../oracle/src/raw_json_scanner.cpp oracle_importer_test.cpp
+cockatrice_add_test(
+  NAME
+  oracle_importer_test
+  SOURCES
+  ${VERSION_STRING_CPP}
+  ../../oracle/src/oracleimporter.cpp
+  ../../oracle/src/parsehelpers.cpp
+  ../../oracle/src/raw_json_scanner.cpp
+  oracle_importer_test.cpp
+  LIBS
+  libcockatrice_card
+  libcockatrice_interfaces
 )
 
-if(NOT GTEST_FOUND)
-  add_dependencies(oracle_importer_test gtest)
-endif()
-
-target_link_libraries(
-  oracle_importer_test libcockatrice_card libcockatrice_interfaces Threads::Threads ${GTEST_BOTH_LIBRARIES}
-  ${TEST_QT_MODULES}
-)
-
-add_test(NAME oracle_importer_test COMMAND oracle_importer_test)
-
 # Oracle importer benchmark tests (manual, not run in CI, incl. RAM benchmark)
 # Optional compression libs, mirrored from oracle/CMakeLists.txt, so the benchmark
 # can download and decompress whatever AllPrintings format the default URL selects.
+# add_definitions() is directory-scoped and order-independent, so HAS_ZLIB/HAS_LZMA also
+# reach the two unit tests above. That is harmless: those macros gate code only in
+# oracle/src/pages.cpp and oracle/src/pagetemplates.cpp, which no test target compiles.
 find_package(ZLIB)
 if(ZLIB_FOUND)
   add_definitions("-DHAS_ZLIB")
   set(_ORACLE_BENCH_EXTRA_SOURCES ../../oracle/src/zip/unzip.cpp ../../oracle/src/zip/zipglobal.cpp)
+  cockatrice_disable_sanitizers_for(${_ORACLE_BENCH_EXTRA_SOURCES})
+  cockatrice_disable_coverage_for(${_ORACLE_BENCH_EXTRA_SOURCES})
   set(_ORACLE_BENCH_EXTRA_LIBRARIES ${ZLIB_LIBRARIES})
   include_directories(${ZLIB_INCLUDE_DIRS})
 else()
@@ -42,28 +37,28 @@ find_package(LibLZMA)
 if(LIBLZMA_FOUND)
   add_definitions("-DHAS_LZMA")
   list(APPEND _ORACLE_BENCH_EXTRA_SOURCES ../../oracle/src/lzma/decompress.cpp)
+  cockatrice_disable_sanitizers_for(../../oracle/src/lzma/decompress.cpp)
+  cockatrice_disable_coverage_for(../../oracle/src/lzma/decompress.cpp)
   list(APPEND _ORACLE_BENCH_EXTRA_LIBRARIES ${LIBLZMA_LIBRARIES})
   include_directories(${LIBLZMA_INCLUDE_DIRS})
 else()
   message(STATUS "Oracle tests: LibLZMA not found; xz download benchmark disabled")
 endif()
 
-add_executable(
-  oracle_importer_benchmark_test
-  ${VERSION_STRING_CPP} ../../oracle/src/oracleimporter.cpp ../../oracle/src/parsehelpers.cpp
-  ../../oracle/src/raw_json_scanner.cpp oracle_importer_benchmark_test.cpp ${_ORACLE_BENCH_EXTRA_SOURCES}
-)
-
-if(NOT GTEST_FOUND)
-  add_dependencies(oracle_importer_benchmark_test gtest)
-endif()
-
-target_link_libraries(
+cockatrice_add_manual_test(
+  NAME
   oracle_importer_benchmark_test
+  SOURCES
+  ${VERSION_STRING_CPP}
+  ../../oracle/src/oracleimporter.cpp
+  ../../oracle/src/parsehelpers.cpp
+  ../../oracle/src/raw_json_scanner.cpp
+  oracle_importer_benchmark_test.cpp
+  ${_ORACLE_BENCH_EXTRA_SOURCES}
+  LIBS
   libcockatrice_card
   libcockatrice_interfaces
-  Threads::Threads
-  ${GTEST_BOTH_LIBRARIES}
-  ${TEST_QT_MODULES}
+  EXTRA_LIBS
   ${_ORACLE_BENCH_EXTRA_LIBRARIES}
+  MANUAL
 )

tests/picture_loader_benchmark/CMakeLists.txt

@@ -1,8 +1,10 @@
-# Manual benchmark hitting the real card image hosts. Not registered with
-# add_test(): it requires a cards.xml, touches the network for minutes at a
-# time, and needs network access. Build it explicitly and run by hand.
-add_executable(
+# Manual benchmark hitting the real card image hosts. Not registered with CTest: it requires a
+# cards.xml, touches the network for minutes at a time, and needs network access. Build it
+# explicitly and run by hand.
+cockatrice_add_manual_test(
+  NAME
   picture_loader_benchmark_test
+  SOURCES
   ${VERSION_STRING_CPP}
   ../../cockatrice/src/interface/card_picture_loader/card_picture_loader_local.cpp
   ../../cockatrice/src/interface/card_picture_loader/card_picture_to_load.cpp
@@ -11,16 +13,12 @@ add_executable(
   ../../cockatrice/src/client/settings/cache_settings.h
   picture_loader_benchmark.cpp
   settings_cache_mock.cpp
-)
-
-target_include_directories(picture_loader_benchmark_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src)
-
-target_link_libraries(
-  picture_loader_benchmark_test
+  LIBS
   libcockatrice_card
   libcockatrice_settings
   libcockatrice_interfaces
   libcockatrice_utility
-  Threads::Threads
-  ${TEST_QT_MODULES}
+  INCLUDE_DIRS
+  ${CMAKE_SOURCE_DIR}/cockatrice/src
+  MANUAL
 )

tests/server_moderator_role_test.cpp

@@ -0,0 +1,156 @@
+/** @file server_moderator_role_test.cpp
+ *  @brief Tests for the moderator staff role authorization and dispatch.
+ *  @ingroup Tests
+ */
+
+#include "libcockatrice/protocol/pb/response.pb.h"
+#include "libcockatrice/protocol/pb/server_message.pb.h"
+
+#include <QString>
+#include <gtest/gtest.h>
+#include <libcockatrice/network/server/remote/server.h>
+#include <libcockatrice/network/server/remote/server_protocolhandler.h>
+#include <libcockatrice/protocol/pb/command_report_reopen.pb.h>
+#include <libcockatrice/protocol/pb/commands.pb.h>
+#include <libcockatrice/protocol/pb/moderator_commands.pb.h>
+#include <libcockatrice/protocol/pb/serverinfo_user.pb.h>
+#include <libcockatrice/rng/rng_abstract.h>
+#include <stdint.h>
+#include <string>
+
+// The server_remote library references the global RNG, which is normally
+// defined by the servatrice/client executable main(). Provide a stub so the
+// unit test can link against it.
+RNG_Abstract *rng = nullptr;
+
+namespace
+{
+
+class TestModeratorHandler : public Server_ProtocolHandler
+{
+public:
+    explicit TestModeratorHandler(Server *_server) : Server_ProtocolHandler(_server, nullptr)
+    {
+    }
+
+    QString getAddress() const override
+    {
+        return {};
+    }
+    QString getConnectionType() const override
+    {
+        return {};
+    }
+
+    // Buffer the last response code sent to the client so tests can assert on
+    // the outcome of processCommandContainer().
+    Response::ResponseCode lastResponseCode = Response::RespNothing;
+    int dispatchCount = 0;
+
+protected:
+    void transmitProtocolItem(const ServerMessage &item) override
+    {
+        if (item.message_type() == ServerMessage::RESPONSE) {
+            lastResponseCode = item.response().response_code();
+        }
+    }
+
+    Response::ResponseCode
+    processExtendedModeratorCommand(int cmdType, const ModeratorCommand &, ResponseContainer &) override
+    {
+        ++dispatchCount;
+        // Fail closed for anything not explicitly handled.
+        if (cmdType != ModeratorCommand::REPORT_REOPEN) {
+            return Response::RespFunctionNotAllowed;
+        }
+        return Response::RespOk;
+    }
+};
+
+class ModeratorRoleTest : public ::testing::Test
+{
+protected:
+    Server server;
+    TestModeratorHandler handler{&server};
+
+    void setUserLevel(uint32_t level)
+    {
+        ServerInfo_User user;
+        user.set_user_level(level);
+        handler.setUserInfo(user);
+    }
+};
+
+TEST_F(ModeratorRoleTest, RejectsWhenNotLoggedIn)
+{
+    CommandContainer cont;
+    cont.add_moderator_command();
+    handler.processCommandContainer(cont);
+    EXPECT_EQ(handler.lastResponseCode, Response::RespLoginNeeded);
+    EXPECT_EQ(handler.dispatchCount, 0);
+}
+
+TEST_F(ModeratorRoleTest, RejectsPlainUser)
+{
+    setUserLevel(ServerInfo_User::IsUser | ServerInfo_User::IsRegistered);
+
+    CommandContainer cont;
+    cont.add_moderator_command();
+    handler.processCommandContainer(cont);
+    EXPECT_EQ(handler.lastResponseCode, Response::RespLoginNeeded);
+    EXPECT_EQ(handler.dispatchCount, 0);
+}
+
+TEST_F(ModeratorRoleTest, DispatchesToModeratorCommandForModerator)
+{
+    setUserLevel(ServerInfo_User::IsModerator);
+
+    CommandContainer cont;
+    ModeratorCommand *cmd = cont.add_moderator_command();
+    cmd->MutableExtension(Command_ReportReopen::ext);
+    handler.processCommandContainer(cont);
+    EXPECT_EQ(handler.lastResponseCode, Response::RespOk);
+    EXPECT_EQ(handler.dispatchCount, 1);
+}
+
+TEST_F(ModeratorRoleTest, DispatchesToModeratorCommandForAdmin)
+{
+    setUserLevel(ServerInfo_User::IsModerator | ServerInfo_User::IsAdmin);
+
+    CommandContainer cont;
+    ModeratorCommand *cmd = cont.add_moderator_command();
+    cmd->MutableExtension(Command_ReportReopen::ext);
+    handler.processCommandContainer(cont);
+    EXPECT_EQ(handler.lastResponseCode, Response::RespOk);
+    EXPECT_EQ(handler.dispatchCount, 1);
+}
+
+TEST_F(ModeratorRoleTest, RejectsDeveloperThatIsNotModerator)
+{
+    setUserLevel(ServerInfo_User::IsDeveloper);
+
+    CommandContainer cont;
+    cont.add_moderator_command();
+    handler.processCommandContainer(cont);
+    EXPECT_EQ(handler.lastResponseCode, Response::RespLoginNeeded);
+    EXPECT_EQ(handler.dispatchCount, 0);
+}
+
+TEST_F(ModeratorRoleTest, FailClosedForUnknownModeratorCommand)
+{
+    setUserLevel(ServerInfo_User::IsModerator);
+
+    CommandContainer cont;
+    cont.add_moderator_command(); // no extension set -> getPbExtension() returns -1
+    handler.processCommandContainer(cont);
+    EXPECT_EQ(handler.lastResponseCode, Response::RespFunctionNotAllowed);
+    EXPECT_EQ(handler.dispatchCount, 1);
+}
+
+} // namespace
+
+int main(int argc, char **argv)
+{
+    ::testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
+}

tests/settings/CMakeLists.txt

@@ -1,26 +1,14 @@
-add_executable(settings_manager_test settings_manager_test.cpp)
-add_executable(settings_defaults_test settings_defaults_test.cpp)
-add_executable(settings_migration_test settings_migration_test.cpp)
+cockatrice_add_test(NAME settings_manager_test SOURCES settings_manager_test.cpp LIBS libcockatrice_settings)
 
-target_link_libraries(
-  settings_manager_test libcockatrice_settings Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
+cockatrice_add_test(
+  NAME
+  settings_defaults_test
+  SOURCES
+  settings_defaults_test.cpp
+  LIBS
+  libcockatrice_settings
+  DEFINITIONS
+  SETTINGS_UNIT_TEST
 )
 
-target_link_libraries(
-  settings_defaults_test libcockatrice_settings Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
-)
-target_compile_definitions(settings_defaults_test PRIVATE SETTINGS_UNIT_TEST)
-
-target_link_libraries(
-  settings_migration_test libcockatrice_settings Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
-)
-
-add_test(NAME settings_manager_test COMMAND settings_manager_test)
-add_test(NAME settings_defaults_test COMMAND settings_defaults_test)
-add_test(NAME settings_migration_test COMMAND settings_migration_test)
-
-if(NOT GTEST_FOUND)
-  add_dependencies(settings_manager_test gtest)
-  add_dependencies(settings_defaults_test gtest)
-  add_dependencies(settings_migration_test gtest)
-endif()
+cockatrice_add_test(NAME settings_migration_test SOURCES settings_migration_test.cpp LIBS libcockatrice_settings)

tests/settings/settings_defaults_test.cpp

@@ -1,6 +1,7 @@
 #include "libcockatrice/card/card_localization.h"
 
 #include "gtest/gtest.h"
+#include <QDir>
 #include <QHash>
 #include <QList>
 #include <QString>
@@ -15,6 +16,7 @@
 #include <libcockatrice/settings/game_settings.h>
 #include <libcockatrice/settings/interface_settings.h>
 #include <libcockatrice/settings/network_settings.h>
+#include <libcockatrice/settings/paths_settings.h>
 #include <libcockatrice/settings/personal_settings.h>
 #include <libcockatrice/settings/sound_settings.h>
 #include <libcockatrice/settings/tabs_settings.h>
@@ -695,6 +697,55 @@ TEST_F(SettingsDefaultsTest, VisualDeckStorage_ShowUploadTime_SetAndGet)
     ASSERT_EQ(s.getVisualDeckStorageShowUploadTime(), false);
 }
 
+TEST_F(SettingsDefaultsTest, Paths_AppDirWarningAcknowledged_Default)
+{
+    PathsSettings s(settingsPath, nullptr);
+    ASSERT_EQ(s.getAppDirWarningAcknowledged(), false);
+}
+
+TEST_F(SettingsDefaultsTest, Paths_AppDirWarningAcknowledged_SetAndGet)
+{
+    PathsSettings s(settingsPath, nullptr);
+    s.setAppDirWarningAcknowledged(true);
+    ASSERT_EQ(s.getAppDirWarningAcknowledged(), true);
+}
+
+TEST_F(SettingsDefaultsTest, Paths_PathsInsideDir_DetectsNestedAndIgnoresSiblings)
+{
+    PathsSettings s(settingsPath, nullptr);
+    const QString base = QDir::cleanPath(settingsPath);
+
+    s.setDeckPath(base + "/decks/");
+    s.setPicsPath(base + "/pics");
+    s.setReplaysPath(base + "2"); // sibling sharing the name prefix, not nested
+
+    const QStringList offenders = s.pathsInsideDir(base);
+    ASSERT_EQ(offenders.size(), 2);
+    ASSERT_TRUE(offenders.contains(base + "/decks"));
+    ASSERT_TRUE(offenders.contains(base + "/pics"));
+}
+
+TEST_F(SettingsDefaultsTest, Paths_PathsInsideDir_EmptyWhenNothingMatches)
+{
+    PathsSettings s(settingsPath, nullptr);
+    s.setDeckPath("/somewhere/else/decks");
+    ASSERT_TRUE(s.pathsInsideDir(QDir::cleanPath(settingsPath)).isEmpty());
+}
+
+TEST_F(SettingsDefaultsTest, Paths_PathsInsideDir_MatchesExactBaseAndNormalizesDots)
+{
+    PathsSettings s(settingsPath, nullptr);
+    const QString base = QDir::cleanPath(settingsPath);
+
+    s.setCustomPicsPath(base);                // exactly the base directory
+    s.setThemesPath(base + "/pics/../decks"); // needs cleanPath normalization
+
+    const QStringList offenders = s.pathsInsideDir(base);
+    ASSERT_EQ(offenders.size(), 2);
+    ASSERT_TRUE(offenders.contains(base));
+    ASSERT_TRUE(offenders.contains(base + "/decks"));
+}
+
 } // namespace
 
 int main(int argc, char **argv)

tests/theme_migration/CMakeLists.txt

@@ -1,5 +1,7 @@
-add_executable(
+cockatrice_add_test(
+  NAME
   theme_migration_test
+  SOURCES
   ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/theme_manager.cpp
   ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/theme_config.cpp
   ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/pixel_map_generator.cpp
@@ -9,22 +11,11 @@ add_executable(
   ${CMAKE_SOURCE_DIR}/cockatrice/src/client/network/update/client/release_channel.cpp
   ${VERSION_STRING_CPP}
   theme_migration_test.cpp
-)
-target_include_directories(theme_migration_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src)
-
-target_link_libraries(
-  theme_migration_test
+  LIBS
   libcockatrice_settings
   libcockatrice_network
   libcockatrice_network_server_remote
   libcockatrice_protocol
-  Threads::Threads
-  ${GTEST_BOTH_LIBRARIES}
-  ${TEST_QT_MODULES}
+  INCLUDE_DIRS
+  ${CMAKE_SOURCE_DIR}/cockatrice/src
 )
-
-add_test(NAME theme_migration_test COMMAND theme_migration_test)
-
-if(NOT GTEST_FOUND)
-  add_dependencies(theme_migration_test gtest)
-endif()