6392 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
BruebachL	fea12a74e2	[CI] Build Windows releases with Ninja (#7380) ... The A/B run measured Ninja against the Visual Studio generator across ten builds: Ninja medians sit at ~17 min against ~21 for MSBuild, and more importantly Ninja already matches the Linux and macOS legs, runs the compiler itself and parallelizes the whole graph instead of one MSBuild node at a time. Single runs on shared 4-core hosts swing by minutes either way (VS and Ninja legs even ran in different Azure regions inside one matrix), so this switches on medians and engineering coherence, not on any one timing. The Ninja leg is now the release leg: it keeps the -Win10 suffix and the vcpkg scope (named after the vs2026 toolchain, whose cl.exe is unchanged, so the warm archives keep restoring), uploads PDBs from the single-config layout and ships installers to releases. The experiment scaffolding goes away with the generator it measured.	2026-10-14 19:13:52 +02:00
BruebachL	0c10798ef4	[CI] Build the Windows solution on more than one MSBuild node (#7379) ... cmake --build only passes a job count to MSBuild when it is asked for one, and .ci/compile.sh never asked, so every Visual Studio generator build ran single-node: projects were built one after another and MTT only ever parallelized the files of whichever project happened to be building. Ask for one node per core, overridable with BUILD_PARALLEL_LEVEL, and fall back to a single node when the core count is not exposed. Alongside it set EnforceProcessCountAcrossBuilds and CL_MPCount, the knobs Microsoft's build throughput write-up pairs with MTT: without them the node count and MTT multiply into more cl.exe processes than the runner has cores for. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-14 16:15:01 +02:00
BruebachL	eefe0bd672	[CI] Collect an MSBuild performance summary and binary log on request (#7378) ... The Windows job spends about 28 minutes inside one "Build project" group, with nothing in it that says where the time goes: MSBuild runs single-node because no --parallel is passed, so projects build one after another and it is not visible which projects, tasks or phases are responsible. Add a --profile flag that makes the Windows build write -flp:PerformanceSummary and -bl, repeat the summary in the job log so the numbers need no download, and report the core count, which decides how much parallelism is worth asking for. The binary log is uploaded as an artifact only on the runs that asked for it. The workflow drives it from a build_profile matrix key that is empty by default, so no build changes behaviour until someone sets it. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-14 14:27:58 +02:00
BruebachL	25c85cf0bb	[CI] Key the vcpkg binary cache per ABI and let pull requests heal it (#7377) ... * [CI] Key the vcpkg binary cache per ABI and let pull requests heal it The cache had no prefix, and TAServers/vcpkg-cache restores every entry it finds under the prefix rather than only the hashes vcpkg asks for. With all legs sharing the single vcpkg/ namespace, each of the five of them downloads all 66 archives of all five on every run. Each restore also refreshes the entry's last access date, so a dead ABI generation stays fresh forever and GitHub's least recently used eviction can never reach it. Name the scope in the key after the platform, target and toolchain it belongs to, so a leg restores only its own archives. macOS 14, 15 and 26 on the same Xcode build identical binaries when they share the lowest deployment target, so point all three at 14 and let them share one scope instead of three. A pull request cannot write the shared cache either. Its entries are scoped to refs/pull/<n>/merge, so nothing it saves is ever readable again, and letting it write anyway only burns the repository's quota, which is what the previous attempt at this did. Keep the build read-only and hand the archives it had to build itself to a new workflow running in the default branch scope, which saves them where master can restore them. Forks stay read-only, they are the one case whose archives must not be trusted. Because the prefix changes every key, add a job that deletes the entries of retired scopes, which is the one-time clean up of the current flat namespace, and the per-run ones pull requests leave behind, which age based eviction also never reaches. Also pin the Linux ccache cache to its own subdirectory, since the directory is shared with the docker build context, and drop the clang-format install from the build images, which format nothing at runtime. * [CI] Create the vcpkg cache directory when a scope is still empty * [CI] Let a pull request write the vcpkg archives it builds * [CI] Upload the vcpkg handover as one archive instead of loose files * [CI] Hand the vcpkg cache over from one leg per scope * [CI] Gate vcpkg cache writes to master pushes --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-14 08:14:16 +02:00
BruebachL	cce606764c	[Server] Resolve a zone dump request to an in-range index span (#7401) ... Command_DumpZone.number_cards is an unbounded sint32 supplied by the client, and cmdDumpZone used it as an index without ever putting it in range. for (int i = 0; (i < cards.size()) && (i < numberCards || numberCards == -1); ++i) { const auto &findId = cmd.is_reversed() ? cards.size() - numberCards + i : i; Server_Card *card = cards[findId]; A forward request happened to be safe, because the loop stopped at the zone's own size. A reversed one was not. number_cards is documented by the client as -1 for "the whole zone", and cards.size() - numberCards is therefore cardCount + 1, one past the last card. A request larger than the zone puts the offset at a negative index instead. Both read out of bounds, then dereference the result as a Server_Card, so either adjacent heap memory or a crash. Any player can reach it: the zone only has to be a public one, or the requester's own, and both number_cards and is_reversed come straight off the wire. A reversed dump of -1 is enough on any public zone with cards in it. Resolve the request to a half-open span up front. -1 and any positive count are clamped to what the zone actually holds, any other negative count resolves to nothing, and a reversed request takes the span from the end rather than computing an offset from a value that may not be in range. cards.at() then carries the bound as a second line of defence, and qsizetype keeps the loop and the list's own indexing type aligned. Also report card_count as the zone's real size. It was numberCards < cards.size() ? cards.size() : numberCards, which agrees with the zone size for every request that was in range and reported the client's own oversized number otherwise. The client uses card_count to draw placeholders when a zone lists no cards, so overstating it invents cards that were never there. Put the resolution in its own translation unit. server_abstract_player.cpp refers to the RNG global that only servatrice's main.cpp defines, so anything linking that object needs the whole server binary behind it, which is why no test could reach this code before. server_dump_zone_test covers each request shape and then asserts the span is contained in [0, cardCount) across the edges of the request domain, INT_MIN and INT_MAX included. Reverting the arithmetic makes three of them fail, the two out-of-bounds shapes and the containment sweep. The client's own copy of this convention at view_zone.cpp:93 already clamps the way this now does. Leaving that duplication for a follow-up, since the client reaching the same answer by a different route is not a defect. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-13 11:23:13 +02:00
BruebachL	24874b10cf	[GameSelector] Persist quick-filter toolbar edits and repair filter settings migration (#7500) ... * [GameSelector] Persist quick-filter toolbar edits and repair filter settings migration - Save game-filter parameters on toolbar changes, debounced with flush on close - Repair pre-camelCase gamefilters.ini keys in place on startup - Map the legacy creator-name filter to hostNameFilter - Balance begin/endFilterChange in GamesProxyModel::refresh - Align the hideIgnoredUserGames default between model and settings - Cover the migration with regression tests * [Review] Address RickyRister comments on filter persistence - Rename game-type prefix constants to SCREAMING_SNAKE to match MIGRATION_SENTINEL_KEY and project style - Document why GameSelector destructor flushes a pending debounced filter save (toolbar path has no other writer) --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-13 07:45:08 +02:00
BruebachL	a712553c81	[CardDB] Reload after the first run enables all sets without freezing startup (#7470) ... * [CardDB] Reload when the first run enables all sets checkUnknownSets() enabled every set when none was enabled, but the database had already been parsed under that empty enablement, so the parsers had stripped every printing. It only notified the models and never reloaded, leaving the deck editor's card database display empty until the next launch. Route the all-sets-enabled case through the same reload the new-sets path uses, and skip caching a snapshot whose sets are all disabled. * [CardDB] Own checkUnknownSets after startup and fix the cache skip The loader's initialLoadComplete guard skipped checkUnknownSets() on the first successful load of the session, so a download that landed after startupConfigCheck() and the onboarding wizard had already run (e.g. skipping the wizard download and fetching later from the menu) left every set disabled and the deck editor blank until the next launch. The guard also let the check run on the worker before the queued snapshot swap, and alongside reloadCardDatabasesAndNotify()'s own check, double-emitting cardDatabaseNewSetsFound on manual reloads. Move the check to the callers that load after startup: the post-update, custom-import, and spoiler paths run it via a QFutureWatcher once the finished snapshot has been swapped in, and the loader's synchronous path-change reload runs it directly. Judge the cache skip by the parsed sets' own enablement, what the parsers actually saw, instead of getEnabledSetNames(), which reports empty for the noop controller even though it enables every set. Wait for the first-run reparse before opening Manage Sets so the dialog cannot bind to the pre-reload set instances. * [CardDB] Queue post-reload check and unblock first-run Manage Sets reloadCardDatabasesAndNotify() no longer emits cardDatabaseEnabledSetsChanged after the swap's Reset already rebuilt the models (second 36k rebuild froze startup), and its follow-up checkUnknownSets() is queued to the GUI thread so it sees swapped-in data. cardDatabaseAllNewSetsEnabled() chains Welcome/Manage Sets off a QFutureWatcher instead of waitForFinished() blocking the event loop during the reparse. * [CardDB] Batch the first-run enable-all into a single settings flush checkUnknownSets() enabled every set with ~3 QSettings rewrite cycles per set (sort key, known, enabled), and each setValue re-opens and re-syncs the whole cardDatabase.ini. With a full database (874 sets) that froze the GUI thread for ~9s after the database download (LagMonitor: event loop stalled for 9129 ms, measured offscreen with a real 59MB cards.xml), reading as a total application hang that recovers on its own. Persist the enable-all through the controller's batched saveSets() in a single flush instead, mirroring what SetsModel::save() already does: CardSetList::enableAllBatched() assigns fresh sort keys and marks every set known+enabled in memory, then writes all records at once. SetSaveData gains the isKnown flag so the batch covers all three options, and SetsModel::save() passes the sets' known state through (unchanged behavior there). Also drop the cardDatabaseEnabledSetsChanged emission from the all-sets-enabled branch: the reload chained off cardDatabaseAllNewSetsEnabled rebuilds the models from the re-parsed snapshot via cardDatabaseReset, so emitting both rebuilt the 36k-card models twice on the GUI thread. Adds a cache-invalidation test driving the real checkUnknownSets() first-run path. * [CardDB] Batch the new-sets-found enable-all the same way The first-run batching fixed the 874-set case, but the new-sets-found dialog path (enableAllUnknownSets/markAllSetsAsKnown) still wrote per set and per option, each setValue re-opening and re-syncing the whole cardDatabase.ini. Users with hundreds of custom sets hit the same multi-second GUI stall when answering the dialog. Persist both through the controller's batched saveSets() in a single flush, writing only changed sets (so the common no-op check skips the flush entirely). Removes the now-unused write-through CardSetList methods (enableAll, enableAllUnknown, markAllAsKnown, guessSortKeys) so the batched flush is the only way to persist bulk set changes. Extends the test controller mock with per-write/batched-flush counters and adds tests asserting both dialog paths persist with exactly one flush and zero individual writes. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-13 06:29:16 +02:00
BruebachL	bac2d285e3	[Versioning] Bump version number for 3.1.2 beta release. (#7501) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> 2026-10-12-Development-3.1.2-beta	2026-10-12 21:14:29 +02:00
BruebachL	96b819d7c8	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>	2026-10-11 21:19:10 +02:00
BruebachL	8968709642	[SMTP] Add reliable email delivery with retry and database-backed queue (#7240) ... * [SMTP] Add reliable email delivery with retry and database-backed queue Rework the SMTP client from fire-and-forget sending into a persistent queue with exponential backoff and per-message retry budget, backed by the existing database rows as the durable source of truth. - Keep pending emails in an in-memory queue with attempts/backoff tracking; enqueue without deleting database rows, and only remove the row once delivery is confirmed via the mailDelivered signal. - Add exponential backoff (doubling, capped at 30 min) between delivery attempts, bounded by new smtp/maxretries and smtp/retrydelay settings. - Track connection-layer failures (connect/auth/encrypt) with their own backoff; route failures to recordFailed/recordConnectionFailure so undeliverable mail eventually drops from the queue instead of retrying forever. - Treat rejected recipients/senders as permanent failures. - Rebuild the SMTP buffer from the authoritative queue before each send via a new QxtSmtp::reset() so reconnects never duplicate deliveries. - Start the idle timeout after authentication, not on TCP connect, so slow SSL/STARTTLS handshakes are not cut short. * [SMTP] Fix retry accounting, backoff reset, and token dedupe --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 20:59:32 +02:00
BruebachL	1a4d89ff8f	[Game] Don't alert game tab for spectator activity (#1848) (#7263) ... Spectator join/leave events were triggering the game tab's attention indicator (exclamation mark and taskbar alert) for players who often leave the tab open while waiting for an opponent. This made spectator activity a false alarm. Only emit the user event when an actual player joins or leaves, keeping spectator joins/leaves quiet. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 16:54:03 +02:00
BruebachL	4feb31bd1a	[Game] Show loaded deck messages to players joining a game (#7268) ... * Show loaded deck messages to players joining a game (#552) The "Player X has loaded a deck" message is only broadcast when a deck is selected, so players and spectators who join a game afterwards never see it. On game join, replay a deck-select event context privately to the joining participant for every player that already has a deck loaded, reusing the existing Event_PlayerPropertiesChanged + Context_DeckSelect mechanism so no protocol version bump is needed. - Server_AbstractPlayer: expose the sideboard-locked state - Server_Game: synthesize deck-select contexts in createGameJoinedEvent * [Server] Skip deck replay on rejoin and after start --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 16:53:26 +02:00
BruebachL	4c5a5e3e55	[Game] Invalidate view when hiding replaced card menu (#7453) ... * [Game] Invalidate view when hiding replaced card menu Hide any visible card menu before replacing and invalidate viewport. * [Game] Fix viewport update compile error in setCardMenu QWidget::update has no QRectF overload, and sceneRect is in scene coordinates rather than viewport coordinates anyway, so invalidate the whole viewport instead. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:36:11 +02:00
BruebachL	5d06bb903d	[Chat] Fix card completion after an @ mention (#7469) ... An open [[ card link now takes precedence over a space-preceded @ mention, so typing an @ before a card no longer shadows the card completer. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:35:10 +02:00
BruebachL	57a6a0d9cb	[Client] Warn when data paths point inside the program directory (#7473) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:34:49 +02:00
BruebachL	f83a61979f	[Installer] Uninstaller deletes only the installed payload, never the whole tree (#7472) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:34:49 +02:00
BruebachL	07eb9f0103	[Report] Prompt for a note on Resolve and drop the redundant option (#7445) ... - Remove the third close option (Resolve with note...) so closing a report is one of two clear actions - Make Resolve always prompt for the optional resolution note, matching Dismiss, and rename the button to Resolve... - Halve the resolveReport signature now that every caller prompts Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:33:48 +02:00
BruebachL	38acddeede	[Report] Add context menu to report queue rows (#7444) ... * [Report] Add report reopen action for closed reports - New Command_ReportReopen moderator command (extension 1206) with server handler that reopens resolved/dismissed reports, resets resolution state and notified, and records the reopening note in the audit log - Reopen... button in the report queue action bar, gated to resolved and dismissed reports, with optional note prompt and tooltip - Success confirmations for Assign, Resolve/Dismiss and Reopen now survive the list refresh via a pending status message instead of being clobbered - Dispatch/auth coverage in server_moderator_role_test * [Report] Surface resolution audit info in report details - Add resolved_by_name to ServerInfo_Report and join resolved_by users in the report list, my-list and details queries - Render resolution note and resolved-at/by in the report detail pane instead of duplicating the description into the chat log context * [Report] Add context menu to report queue rows - Open the standard UserContextMenu on right-click/Menu key for the reported user, selecting the row first so the details panel matches the menu target - Carry the reported user's level in ServerInfo_Report so the menu shows the same actions as the user list, with an IsUser fallback for deleted or unregistered users - Wire the Private chat action through TabReport to TabSupervisor and retranslate the menu on live language switches * [Server] Fix reported_user_level column indices in report queries The SELECT lists resolved_by_name at index 10, shifting room_id to 11, replay_id to 12, reported_user_id to 13 and reported_user_admin to 14. The code read 12/13, so 'user exists' meant 'a replay exists' and the admin flags came from the user id (e.g. id 5 -> admin+judge; users without a replay reported as unregistered, hiding registered-only actions). Fixed in cmdReportList and cmdReportMyList, plus the column comment. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:33:47 +02:00
BruebachL	74a6acde91	[Report] Surface resolution audit info in report details (#7443) ... - Add resolved_by_name to ServerInfo_Report and join resolved_by users in the report list, my-list and details queries - Render resolution note and resolved-at/by in the report detail pane instead of duplicating the description into the chat log context Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:29:09 +02:00
BruebachL	4a010bd17f	[Report] Add report reopen action for closed reports (#7442) ... - New Command_ReportReopen moderator command (extension 1206) with server handler that reopens resolved/dismissed reports, resets resolution state and notified, and records the reopening note in the audit log - Reopen... button in the report queue action bar, gated to resolved and dismissed reports, with optional note prompt and tooltip - Success confirmations for Assign, Resolve/Dismiss and Reopen now survive the list refresh via a pending status message instead of being clobbered - Dispatch/auth coverage in server_moderator_role_test Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:28:45 +02:00
BruebachL	315cbae345	[Build] Extend PCH to libraries and grow IWYU-driven header sets (#7456) ... The executables precompile Qt since #7235, but the libcockatrice_* static libraries still reparse the full Qt Core stack in every TU. Measured over the tree after the IWYU pass: 380 MB of header parse in the libraries, 92% of it reachable from qtcore_pch.h. - Apply qtcore_pch.h to the 14 Qt Core static library targets (skips libcockatrice_rng: its SFMT.c is C and cannot use a C++ precompiled header). - Grow qtcore_pch.h with 25 QtCore headers that IWYU shows are re-parsed across many TUs and were not already reachable from the existing list (JSON/QUuid cluster, QSettings, item models, threading, XmlStream, QTemporaryDir, ...). - Grow qtwidgets_pch.h with 30 QtGui/QtWidgets headers (dialog cluster, item views, QImageReader, ...). Only stable Qt headers go into the PCH, so it rarely invalidates. Test executables stay PCH-less: 1-3 TUs per target does not amortize the extra precompile. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:07:30 +02:00
BruebachL	bdda3dedfa	[CI] Publish a gcovr line-coverage report from the test run (#7404) ... * [Tests] Register each test case with CTest instead of each test binary Registering a test cost four edits across a dozen CMakeLists files: add_test, add_executable, a conditional add_dependencies on gtest, and target_link_libraries. The conditional branch was the trap -- it is only needed when GTest was fetched and add_subdirectory'd, and omitting it yields undefined gtest symbols at link time -- so it was easy to leave out and impossible to notice until the link failed. Fold all four steps into cockatrice_add_test() and migrate every test directory onto it, with a cockatrice_add_manual_test() wrapper for the three benchmarks that are built but deliberately kept out of ctest. The calls now read as a declaration of what a test is rather than a transcript of how to register one. While rewriting the registrations, replace add_test(NAME x COMMAND x) with gtest_discover_tests(). Naming the binary as a single CTest entry meant a CI failure reported only which binary failed, and ctest -R Suite.Case could not select an individual case. Discovery raises the suite from 35 opaque entries to 469 real ones: ctest -R ThemeMigrationTest.IsIdempotent now runs exactly that case, and ctest -L server / -LE bench work off labels applied at registration. Give every test a 60s TIMEOUT. CTest's implicit default is 1500s, so one hung socket wait would stall CI for 25 minutes, and single_instance_manager_test already does real QLocalSocket waits. Dropping per-case granularity costs wall-clock time, since the suite becomes process-spawn bound rather than CPU bound: serial ctest went from 3.8s to 30.8s. Run it with -j in CI to recover it -- 7.6s at -j4, and 3.5s at -j8, both at or below the old serial time. The binaries are parallel-safe: the only test claiming a fixed resource scopes its local socket name to its own pid, so concurrent copies cannot collide. Also delete dummy_test, which asserted 1 == 1 and only proved the harness was wired up -- something the other 468 cases already establish. No test is added, removed, or changed by this commit; the 469 cases are the same ones the 35 binaries contained. * [Tests] Make QtTest available to test targets QtTest was not among the modules test targets could link, so the Qt model and signal contracts had no way to be verified from a test: QAbstractItemModelTester and QSignalSpy both live there. Add Test to _TEST_NEEDED in cmake/FindQtRuntime.cmake. That is the single place the union of test-only Qt modules is declared, so this is the only file that needs to change to make the module reachable from every test target. Nothing uses it yet. This lands the capability ahead of the model-layer tests that need it, so those can be added as tests alone rather than as test plus build-system plumbing. * [Tests] Correct the note on the oracle test compression defines add_definitions() populates the directory's COMPILE_DEFINITIONS property, which is consumed at generate time, so it reaches every target in the directory no matter where it is called relative to add_executable(). The previous note claimed the flags stayed confined to the benchmark because the find_package block came after it; oracle_importer_test, created above that block, carries both defines too. Verified harmless rather than a live bug: HAS_ZLIB and HAS_LZMA gate code only in oracle/src/pages.cpp and oracle/src/pagetemplates.cpp, and no test target compiles either. The ordering and the leaked defines are both pre-existing, so this only corrects the explanation. * [Tests] Carry one CTest label per test instead of several gtest_discover_tests() can hand CTest exactly one label. GoogleTestAddTests.cmake accumulates its generated script with `foreach(arg ${ARGN})`, so a label list reaches the CTestTestfile as bare `LABELS ci server`, and CTest reads only the first value of a property and drops the rest without a word. The `ci` label that is appended to every registered test was therefore absent from every test that also asked for `server` or `bench`: `ctest -L ci` reported 400 of 469 cases, and `ctest --print-labels` never listed a second label anywhere in the build. Reduce each test to exactly one of `ci`, `server`, or `bench`, which is all the selections need. The three now partition the suite exactly, 400 + 64 + 5 = 469, and `ctest -LE bench` still means everything but benchmarks. A second label is now a configure-time FATAL_ERROR rather than a line that quietly means something else. 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. Also raise DISCOVERY_TIMEOUT off GoogleTestAddTests' 5s default. A plain debug binary answers --gtest_list_tests in 0.04s, but that budget is thin once several binaries discover concurrently behind a -j build, and being over it fails at the *link* step with a bare "Process terminated due to timeout" that points nowhere near the cause. * [Tests] Link gtest_main on the fetched-gtest path The downloaded-gtest branch set GTEST_BOTH_LIBRARIES to `gtest` alone, while FindGTest sets it to both halves. Every test target links that list rather than picking an entry point, so on this path nothing provides main() and any test source that does not define its own fails at link with `undefined reference to main` pointing at crt1.o, naming neither gtest nor the line responsible. That stayed latent while every test carried its own four-line main(). A test that omits it now fails, and the error points at the C runtime rather than at gtest, which is a long way from the cause. Restoring both halves makes the two paths agree and drops the boilerplate requirement on new test sources. The 18 sources that still define main() keep linking: gtest_main.cc defines nothing but main, so the linker leaves its object in the archive unused rather than reporting a duplicate. * [Build] Add an ENABLE_SANITIZERS option for ASan and UBSan Adds -DENABLE_SANITIZERS, off by default, building with AddressSanitizer and UndefinedBehaviorSanitizer. The bulk of the work is in the three ways the obvious implementation silently does the wrong thing. Probe the flags on the link step, not just the compile step. check_cxx_compiler_flag() only compiles, so every sanitizer flag comes back unsupported, because linking an ASan object without -fsanitize=address leaves __asan_init undefined. The result is not a build failure but a build that appears instrumented and is not, which is worse. Use check_cxx_source_compiles() with CMAKE_REQUIRED_LINK_OPTIONS instead, and treat an unsupported sanitizer pair as fatal since the cosmetic flags are allowed to degrade. Opt vendored sources out per source, through a cockatrice_disable_sanitizers_for() helper rather than a list of set_source_files_properties() calls at the root. The helper has to exist because CMake resolves a source-file property against the directory scope its target was declared in: setting the property from the root with absolute paths is a no-op, silently, which is why the call sites sit next to the add_library and add_executable that pull the vendored code in. SFMT, minizip, the LZMA SDK and Qxt SMTP are excluded. oracle/src/qt-json is in the history but not in the tree, removed in #7214. Also mark those sources SKIP_PRECOMPILE_HEADERS. The PCH 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, which is a fatal -Werror=invalid-pch in the servatrice and oracle builds. -fno-sanitize=vptr is not optional. Qt constructs QObject subclasses into raw storage and writes the vptr afterwards, so the vptr check reports on essentially every QObject instantiation, and it has no value here because nothing outside moc-generated code inherits QObject. Verified on the resulting build: our own translation units carry ASan and UBSan calls, SFMT.o carries neither, and the full suite of 464 cases passes clean under ASAN_OPTIONS=detect_leaks=0:abort_on_error=1 with UBSAN_OPTIONS=halt_on_error=0. The test suite needs two adjustments to make this option usable at all, both of which land here because they are consequences of COCKATRICE_SANITIZERS existing rather than independent changes. DISCOVERY_TIMEOUT rises from 60s to 300s under sanitizers. An instrumented binary takes 2.8s to answer --gtest_list_tests against 0.04s uninstrumented, and discovery runs several binaries at once behind a -j build. One target is excluded via a NO_SANITIZE flag, server_developer_role_test. Reaching google::protobuf::TextFormat::Printer's destructor aborts on an absl assertion inside system protobuf 33.4 whenever the consumer translation unit is built without NDEBUG, which a Debug sanitizer build is. This reproduces with no Cockatrice code at all, just "TextFormat::Printer p; p.PrintToString(msg, &out);" built with -fsanitize=address, so it is upstream rather than ours. It is a flag and not a fourth label because "skip this in the sanitizer leg" has to compose with a test's group rather than compete for the single label slot, and because a flag excludes by not registering the cases at all, which cannot rot the way a label silently can. The target still builds and still runs on the legs without sanitizers. * [CI] Run the suite once under ASan and UBSan Adds --sanitize to .ci/compile.sh, mirroring --test, and turns it on for one entry of the existing Linux Docker matrix: Ubuntu 26.04, which already had a test-enabled leg. That reuses the image, the ccache layer and the existing step shape, so the CI diff is a matrix key plus the argument plumbing. Debian 13 and Fedora 44 keep covering the uninstrumented suite, so nothing is lost by instrumenting one leg. Sanitizers stay off the release and package legs, which build with --release for packaging: instrumenting those would change the shipped binaries' optimization profile and build time for no benefit. macOS and Windows stay uninstrumented too, since ASan is unreliable against the Qt build there and MSVC has no equivalent without /fsanitize=address. --sanitize does not imply --test, so an instrumented build can be produced locally without paying for ctest. When both are given, ctest runs with detect_leaks=0 because Qt and the sanitizer runtimes both leak at exit by design and LeakSanitizer's default buries real findings under hundreds of process-teardown reports, and with UBSan halt_on_error=0 so one run collects many findings instead of stopping at the first. ASan keeps abort_on_error=1, since a bad access leaves the process unusable and continuing only produces a second, more confusing report from the wreckage. * [CI] Publish a gcovr line-coverage report from the test run Sanitizers tell us what a run did wrong; they say nothing about what it never reached. This adds the other half: an instrumented build whose report says which of our own lines no test executes. Add an ENABLE_COVERAGE option that compiles with --coverage -O0 -g, mirroring the ENABLE_SANITIZERS opt-in including the per-source carve-out that keeps vendored trees out of the denominator. -O0 is deliberate: at higher optimisation levels GCC merges and relocates lines, so the report would describe code that no longer exists in that form. Only GCC is supported, since Clang's intermediate format needs llvm-cov; asking for coverage under Clang is a hard error rather than a build that silently produces unreadable .gcno files. Keep the filters in a committed gcovr.cfg rather than in the invocation. The tests themselves are excluded because they are compiled with the same instrumentation and are only ever executed by the run doing the measuring, so leaving them in would report a healthier project than we have. The process entry points and the client's window chrome go too: each needs a display, an interactive terminal or a live database before it does anything a headless test could observe, so they would otherwise sit at 0% forever and bury real movement. Give coverage its own matrix leg rather than a second flag on the sanitizer leg. The two answer different questions, and combining them slows both down without telling us more. The new variant key gives the leg its own Docker image and ccache entry so the two legs cannot race on the same tag. The leg publishes an HTML report with per-file pages, LCOV, Cobertura XML and a JSON summary. gcovr is a report-time dependency rather than a build dependency, so a missing one warns and skips instead of failing an otherwise green build. * [CI] Resolve the coverage source root before the build dir cd The previous commit computed SRC_ROOT after `cd "$BUILD_DIR"`, under a comment claiming it ran before. `$0` is relative to the directory the script was invoked from, so from inside the build dir `dirname "$0"` no longer resolves and the subshell's `cd` fails. Under `set -e` that is fatal rather than an empty variable, which took down every leg in the matrix, coverage or not, including the ones this change does not otherwise touch. Move it above the cd and guard it on MAKE_COVERAGE, since the coverage report is the only consumer. Resolving it unconditionally meant a coverage-only detail was paid for by every build in the fleet, and gave every one of them a way to fail. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:05:28 +02:00
BruebachL	8822ade49d	[Tests] Register each test case with CTest instead of each test binary (#7399) ... * [Tests] Register each test case with CTest instead of each test binary Registering a test cost four edits across a dozen CMakeLists files: add_test, add_executable, a conditional add_dependencies on gtest, and target_link_libraries. The conditional branch was the trap -- it is only needed when GTest was fetched and add_subdirectory'd, and omitting it yields undefined gtest symbols at link time -- so it was easy to leave out and impossible to notice until the link failed. Fold all four steps into cockatrice_add_test() and migrate every test directory onto it, with a cockatrice_add_manual_test() wrapper for the three benchmarks that are built but deliberately kept out of ctest. The calls now read as a declaration of what a test is rather than a transcript of how to register one. While rewriting the registrations, replace add_test(NAME x COMMAND x) with gtest_discover_tests(). Naming the binary as a single CTest entry meant a CI failure reported only which binary failed, and ctest -R Suite.Case could not select an individual case. Discovery raises the suite from 35 opaque entries to 469 real ones: ctest -R ThemeMigrationTest.IsIdempotent now runs exactly that case, and ctest -L server / -LE bench work off labels applied at registration. Give every test a 60s TIMEOUT. CTest's implicit default is 1500s, so one hung socket wait would stall CI for 25 minutes, and single_instance_manager_test already does real QLocalSocket waits. Dropping per-case granularity costs wall-clock time, since the suite becomes process-spawn bound rather than CPU bound: serial ctest went from 3.8s to 30.8s. Run it with -j in CI to recover it -- 7.6s at -j4, and 3.5s at -j8, both at or below the old serial time. The binaries are parallel-safe: the only test claiming a fixed resource scopes its local socket name to its own pid, so concurrent copies cannot collide. Also delete dummy_test, which asserted 1 == 1 and only proved the harness was wired up -- something the other 468 cases already establish. No test is added, removed, or changed by this commit; the 469 cases are the same ones the 35 binaries contained. * [Tests] Make QtTest available to test targets QtTest was not among the modules test targets could link, so the Qt model and signal contracts had no way to be verified from a test: QAbstractItemModelTester and QSignalSpy both live there. Add Test to _TEST_NEEDED in cmake/FindQtRuntime.cmake. That is the single place the union of test-only Qt modules is declared, so this is the only file that needs to change to make the module reachable from every test target. Nothing uses it yet. This lands the capability ahead of the model-layer tests that need it, so those can be added as tests alone rather than as test plus build-system plumbing. * [Tests] Correct the note on the oracle test compression defines add_definitions() populates the directory's COMPILE_DEFINITIONS property, which is consumed at generate time, so it reaches every target in the directory no matter where it is called relative to add_executable(). The previous note claimed the flags stayed confined to the benchmark because the find_package block came after it; oracle_importer_test, created above that block, carries both defines too. Verified harmless rather than a live bug: HAS_ZLIB and HAS_LZMA gate code only in oracle/src/pages.cpp and oracle/src/pagetemplates.cpp, and no test target compiles either. The ordering and the leaked defines are both pre-existing, so this only corrects the explanation. * [Tests] Carry one CTest label per test instead of several gtest_discover_tests() can hand CTest exactly one label. GoogleTestAddTests.cmake accumulates its generated script with `foreach(arg ${ARGN})`, so a label list reaches the CTestTestfile as bare `LABELS ci server`, and CTest reads only the first value of a property and drops the rest without a word. The `ci` label that is appended to every registered test was therefore absent from every test that also asked for `server` or `bench`: `ctest -L ci` reported 400 of 469 cases, and `ctest --print-labels` never listed a second label anywhere in the build. Reduce each test to exactly one of `ci`, `server`, or `bench`, which is all the selections need. The three now partition the suite exactly, 400 + 64 + 5 = 469, and `ctest -LE bench` still means everything but benchmarks. A second label is now a configure-time FATAL_ERROR rather than a line that quietly means something else. 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. Also raise DISCOVERY_TIMEOUT off GoogleTestAddTests' 5s default. A plain debug binary answers --gtest_list_tests in 0.04s, but that budget is thin once several binaries discover concurrently behind a -j build, and being over it fails at the *link* step with a bare "Process terminated due to timeout" that points nowhere near the cause. * [Tests] Link gtest_main on the fetched-gtest path The downloaded-gtest branch set GTEST_BOTH_LIBRARIES to `gtest` alone, while FindGTest sets it to both halves. Every test target links that list rather than picking an entry point, so on this path nothing provides main() and any test source that does not define its own fails at link with `undefined reference to main` pointing at crt1.o, naming neither gtest nor the line responsible. That stayed latent while every test carried its own four-line main(). A test that omits it now fails, and the error points at the C runtime rather than at gtest, which is a long way from the cause. Restoring both halves makes the two paths agree and drops the boilerplate requirement on new test sources. The 18 sources that still define main() keep linking: gtest_main.cc defines nothing but main, so the linker leaves its object in the archive unused rather than reporting a duplicate. * [Build] Run the test suite under ASan and UBSan (#7400) * [Build] Add an ENABLE_SANITIZERS option for ASan and UBSan Adds -DENABLE_SANITIZERS, off by default, building with AddressSanitizer and UndefinedBehaviorSanitizer. The bulk of the work is in the three ways the obvious implementation silently does the wrong thing. Probe the flags on the link step, not just the compile step. check_cxx_compiler_flag() only compiles, so every sanitizer flag comes back unsupported, because linking an ASan object without -fsanitize=address leaves __asan_init undefined. The result is not a build failure but a build that appears instrumented and is not, which is worse. Use check_cxx_source_compiles() with CMAKE_REQUIRED_LINK_OPTIONS instead, and treat an unsupported sanitizer pair as fatal since the cosmetic flags are allowed to degrade. Opt vendored sources out per source, through a cockatrice_disable_sanitizers_for() helper rather than a list of set_source_files_properties() calls at the root. The helper has to exist because CMake resolves a source-file property against the directory scope its target was declared in: setting the property from the root with absolute paths is a no-op, silently, which is why the call sites sit next to the add_library and add_executable that pull the vendored code in. SFMT, minizip, the LZMA SDK and Qxt SMTP are excluded. oracle/src/qt-json is in the history but not in the tree, removed in #7214. Also mark those sources SKIP_PRECOMPILE_HEADERS. The PCH 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, which is a fatal -Werror=invalid-pch in the servatrice and oracle builds. -fno-sanitize=vptr is not optional. Qt constructs QObject subclasses into raw storage and writes the vptr afterwards, so the vptr check reports on essentially every QObject instantiation, and it has no value here because nothing outside moc-generated code inherits QObject. Verified on the resulting build: our own translation units carry ASan and UBSan calls, SFMT.o carries neither, and the full suite of 464 cases passes clean under ASAN_OPTIONS=detect_leaks=0:abort_on_error=1 with UBSAN_OPTIONS=halt_on_error=0. The test suite needs two adjustments to make this option usable at all, both of which land here because they are consequences of COCKATRICE_SANITIZERS existing rather than independent changes. DISCOVERY_TIMEOUT rises from 60s to 300s under sanitizers. An instrumented binary takes 2.8s to answer --gtest_list_tests against 0.04s uninstrumented, and discovery runs several binaries at once behind a -j build. One target is excluded via a NO_SANITIZE flag, server_developer_role_test. Reaching google::protobuf::TextFormat::Printer's destructor aborts on an absl assertion inside system protobuf 33.4 whenever the consumer translation unit is built without NDEBUG, which a Debug sanitizer build is. This reproduces with no Cockatrice code at all, just "TextFormat::Printer p; p.PrintToString(msg, &out);" built with -fsanitize=address, so it is upstream rather than ours. It is a flag and not a fourth label because "skip this in the sanitizer leg" has to compose with a test's group rather than compete for the single label slot, and because a flag excludes by not registering the cases at all, which cannot rot the way a label silently can. The target still builds and still runs on the legs without sanitizers. * [CI] Run the suite once under ASan and UBSan Adds --sanitize to .ci/compile.sh, mirroring --test, and turns it on for one entry of the existing Linux Docker matrix: Ubuntu 26.04, which already had a test-enabled leg. That reuses the image, the ccache layer and the existing step shape, so the CI diff is a matrix key plus the argument plumbing. Debian 13 and Fedora 44 keep covering the uninstrumented suite, so nothing is lost by instrumenting one leg. Sanitizers stay off the release and package legs, which build with --release for packaging: instrumenting those would change the shipped binaries' optimization profile and build time for no benefit. macOS and Windows stay uninstrumented too, since ASan is unreliable against the Qt build there and MSVC has no equivalent without /fsanitize=address. --sanitize does not imply --test, so an instrumented build can be produced locally without paying for ctest. When both are given, ctest runs with detect_leaks=0 because Qt and the sanitizer runtimes both leak at exit by design and LeakSanitizer's default buries real findings under hundreds of process-teardown reports, and with UBSan halt_on_error=0 so one run collects many findings instead of stopping at the first. ASan keeps abort_on_error=1, since a bad access leaves the process unusable and continuing only produces a second, more confusing report from the wreckage. * [Build] Fix sanitizer flag leak into release package and make UBSan fail CI - compile.sh always passes -DENABLE_SANITIZERS explicitly so the release-package reconfigure in the same build dir cannot inherit ON from the sanitized debug step (which shipped libasan/libubsan DEB deps). - Sanitizers.cmake resets COCKATRICE_SANITIZERS to OFF when the option is off (CACHE INTERNAL otherwise sticks forever, keeping NO_SANITIZE skips and the 300s discovery timeout). - UBSan now fails the job: -fno-sanitize-recover=undefined at compile time plus UBSAN_OPTIONS halt_on_error=1 (recover mode exits 0, so the old halt_on_error=0 could never fail). --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 14:50:09 +02:00
tooomm	7fa6234eee	[Docs] Add app logo to main page + cleanup (#7471) ... * Add app logo to main page * rename pages folder * Add link + cleanup * Cleanup file extension patterns * Update Doxygen README.md	2026-10-10 23:48:37 +02:00
ebbit1q	efc7e7a079	fix the desktop file, removing the invalid key (#7406) ... * fix the desktop file, removing the invalid key * Update cockatrice.desktop --------- Co-authored-by: BruebachL <44814898+BruebachL@users.noreply.github.com>	2026-10-10 17:15:10 +02:00
BruebachL	be2c16eca5	[UI] Fix auto-completion acceptance on Space/Tab/Enter (#7466) ... * [UI] Fix auto-completion acceptance on Space/Tab/Enter * [UI] Accept current completion when no item selected in popup * [UI] Also accept completion on focus out when none selected * [UI] Keep focus in chat on Tab completion acceptance * [UI] Prevent Tab from moving focus when no completion popup * [UI] Refocus on Tab when completing * [UI] Let focusOut handle Tab completion acceptance * [UI] Restore Tab behavior to match original (handle in focusOut) * [UI] Defer refocus on Tab to avoid focus chain issues * [UI] Auto-select first completion match in popup * [UI] Auto-select first match when completer popup resets * Revert "[UI] Auto-select first match when completer popup resets" This reverts commit c7a3de3595. * Revert "[UI] Auto-select first completion match in popup" This reverts commit ae0dfb8622. * [UI] Ensure completion selection updates on model changes * Revert "[UI] Ensure completion selection updates on model changes" This reverts commit 686bbef5b2. * [UI] Highlight the selected completion in chat popups The card completer's above-variant moved the current row with NoUpdate, so the delegate never painted the selection border, and it used Select which accumulated highlights while navigating. Use ClearAndSelect so only the current row is highlighted and the old one is cleared. The mention popup opened with no current row at all, so nothing was highlighted and there was no visual hint of what Enter would insert. Select the row currentCompletion() uses, and install a delegate that mirrors the active highlight into the inactive group so the selection stays visible when the transient popup is not the active window. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> 2026-10-10-Release-3.1.1	2026-10-10 09:45:42 +02:00
BruebachL	b8e8f2c738	[PrintingSelector] Fix crash setting all printings to preferred (#7467) ... getPreferredPrinting returns a null PrintingInfo when a deck card has no database entry or printings. SetProviderIdToPreferred dereferenced getSet() unconditionally, crashing on custom or unresolved cards. Guard against a null preferred set and leave the card's cleared printing data as is. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-10 09:36:26 +02:00
BruebachL	1e1f79db0d	[Windows] Wait for uninstaller, rework .dll bundling, add logging (#7407) ... * [Windows] Purge every runtime DLL on upgrade instead of a named few A user updating from 3.0.2 to 3.1 got "abseil_dll.dll is either not designed to run on Windows or it contains an error" and could not start the client. abseil_dll.dll comes from vcpkg, not Qt. It rides into the install directory on the "*.dll" glob in cockatrice/CMakeLists.txt, alongside libprotobuf.dll, liblzma.dll and z.dll, and none of those were named in the upgrade purge. So on an upgrade the old uninstaller's RMDir /r skipped whichever of them was locked, the purge did not look for them, HasStaleRuntimeFiles did not look for them either, and the installer reported success - having written the new executables next to the previous version's protobuf runtime. Windows then refused to load it. The mask list was also already wrong where it existed: it matches zlib*.dll while vcpkg ships z.dll. Delete every top-level DLL instead of naming library families. The payload is written after the purge, so a DLL the new build still needs is put straight back; what the purge has to guarantee is only that nothing from the previous build is left to be loaded next to the new executables. Enumerated families cannot do that - every transitive dependency that arrives between two releases is one more name to forget, and the three previous rounds of this work each reasoned about Qt because that is what the reports named at the time. HasStaleRuntimeFiles collapses to the matching *.dll probe, so the abort fires on any survivor rather than only on the families it was told about. IsExistingCockatriceInstall stays on Qt6*.dll on purpose: it decides whether a directory is ours at all, and a folder that merely holds some DLLs of its own must not be purged. Qt6*.dll is enough even when only the protobuf runtime survived, because the whole Qt set is mapped by the same process that maps abseil_dll.dll and so is locked or free with it. * [CI] Assert the whole Windows runtime, not just the Qt modules required-qt-runtime.txt listed only the Qt modules COCKATRICE_QT_MODULES names, so it could not see any other runtime go missing. The three previous rounds of installer work all reasoned about Qt*.dll because that was all the build knew how to ask about, and the one runtime that actually broke the reported upgrade - abseil_dll.dll - was outside the set entirely. - Rename it to required-runtime.txt and take the third party runtime names from the vcpkg bin directory the app-local copies come from, so a dependency bump cannot leave the list behind and a versioned name such as libcrypto-3-x64.dll becoming libcrypto-4-x64.dll cannot either - Union the three applications' module lists rather than the client's alone: Servatrice contributes Qt6Sql.dll, servatrice.exe is installed next to cockatrice.exe and cannot start without it, and nothing asserted it - Add Core to the client's module list. It is the one module nothing can do without and it only ever arrived transitively, so Qt6Core.dll was shipped and never checked - the new unknown-runtime check flagged it on the first run - Fail the build on a missing runtime, as before, and warn on a runtime nothing requires. Only the test framework is a hard failure there: "no shipped executable links gtest" is a fact, whereas a runtime missing from the manifest may be one the manifest failed to derive, and a check that has never run before should not be able to block a release The warning is meant to be read. Qt6::Quick pulls in Qml, QmlModels, QmlWorkerScript, QuickBasic, QuickTemplates2 and OpenGL transitively and none are named, so the manifest is known to be a partial list; the first Windows run reports the real one, which is evidence rather than a guess about what Qt links. Fold that back in when it lands. * [CI] Stop deriving the required runtime from the vcpkg bin directory The first Windows run of this check failed, and the seven files it named as missing are not runtime at all: gmock.dll, gmock_main.dll, legacy.dll, libprotobuf-lite.dll, libprotoc.dll and pthreadVC3/VCE3/VSE3.dll. None of them is linked by any shipped executable, so a correct package leaves them out. vcpkg_installed/x64-windows/bin is everything vcpkg installed, not everything the applications import. The protoc compiler, the GoogleMock library, a port tool and three CMake FindThreads probes sit beside the genuine runtimes, and a directory listing cannot tell them apart. Asserting it fails the build over a package that is right - which is what happened, and it is the same mistake as the one that produced this bug report: reasoning about a list of names instead of about the build. The real source of truth is the three executables' import tables, which fixup_bundle already walks to decide what to copy. Until CMake can read that result, the Qt half is what can be derived honestly, from the same module lists the targets are built against. So assert that, and report the rest rather than require it: - Drop the vcpkg bin glob, its gtest filter and the vcpkg-runtime-known marker. All three existed only to support the glob. - Report top level DLLs the manifest does not name without failing. These are expected: protobuf, Abseil, OpenSSL, liblzma, ffmpeg and the Qt debug DLLs all arrive on the build output glob because nothing in the build declares them by name. The list is a report of what the package carries, not a verdict, and it is how the missing Qt6Qml/Quick/OpenGL entries above became visible. - Report a leaked gtest or gmock instead of failing on it. "No shipped executable links this" is a fact, but a check that has never passed on a real package has not earned the right to block a release, and the first run of an enforcing version is exactly when its own wrong assumptions show up. - Skip DLLs under a subdirectory, which are platform plugins rather than imported runtimes, and the NSIS plugins, which extract to $PLUGINSDIR and so reach the comparison looking like payload. Verified against the paths 7-Zip actually listed in the failing package: the check passes on it, still fails when Qt6Sql.dll or the qwindows plugin is removed, and still reports gtest.dll and gmock.dll if either is added. The purge in cmake/NSIS.template.in covers the runtimes the manifest does not name. That fix is independent of any list and is what makes the reported update failure go away. * [Installer] Fix the /R upgrade aborting before it does anything WaitForLauncherToExit read the wrong registers. System.dll writes its .rN outputs to $N and its .RN outputs to $RN, but the wait used p.r0/i.r1 while testing $R1. $R1 therefore stayed empty, and the "did not exit" test compared "" with "0" - true every time. Every in-app update passes /PID=, so every upgrade hit AbortInstall before writing anything; because /R also sets silent, the override just vanished with exit code 1 and no message. The bug came in with #7372 and is inherited by this branch, not introduced by it. Use the uppercase .RN forms, and treat a null or plugin-failed OpenProcess as the launcher already being gone - the case the old ${Errors} check never caught, because System.dll does not set the NSIS error flag from the callee's return value. Also restore the abort dialog for the in-app updater: AbortInstall and the two inline warnings gated their message box on ${IfNot} ${Silent}, which is false for a /R upgrade, so any abort on that path was equally silent. The unattended case is a /S install without /R, not /R, so show the box for $ReinstallMode = 1 or an interactive install. The stale-runtime abort now goes through AbortInstall so that decision lives in one place. * [Installer] Log every upgrade step to %TEMP%\Cockatrice-installer.log * [Installer] Actually append to the upgrade log (FileSeek END) * [Installer] Wait for the old uninstaller before replacing files An NSIS uninstaller copies itself to %TEMP% and runs from there so that it can delete its own directory; the process the installer started exits immediately. A plain ExecWait therefore returns before the uninstall has run, and the temp copy's "RMDir /r $INSTDIR" then executes concurrently with the install section, deleting files the payload just wrote. That is why an upgrade could leave Qt6Core/Gui/Multimedia and the other alphabetically-first DLLs missing while the tail of the directory survived: a recursive delete marching from the front while the final listing ran. Pass _?=$INSTDIR to the old uninstaller so it runs in place and ExecWait actually blocks until the uninstall completes. _?= must be the last argument and unquoted, even for paths with spaces. Applied to both the silent /R path (AutoUninstallIfNeeded) and the interactive path (componentsPagePre). * [Installer] Tighten the wording of the upgrade comments Drop the dash-heavy phrasing from the comments this PR adds, and say plainly in the _?= comment that ExecWait returns before the old uninstaller has run and the temp copy then deletes the payload the section is still writing. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> 2026-10-09-Development-3.1.1-beta.2	2026-10-09 20:09:34 +02:00
BruebachL	bb61617d42	[Versioning] Bump version number for 3.1.1 beta release. (#7464) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> 2026-10-09-Development-3.1.1-beta	2026-10-09 11:35:35 +02:00
BruebachL	a44757bd11	[Client] Improve card database update visibility (#7458) ... * [Client] Improve card database update visibility Addresses Cockatrice/Cockatrice#7431. Cockatrice previously offered no in-client feedback while the card database updater ran: the only progress UI lived in the first-run wizard, stderr/stdout were not parsed in interactive runs, and starting a second update with Oracle already open or with a hidden background run active just showed a confusing 'already running' guard - even though Oracle is launched as a separate process with no lifecycle tracking. - Add a status-bar widget (CardDatabaseUpdateStatusBar) that appears while an update runs and shows the Downloading/Parsing/Importing stages with percent progress; it is also used in the quit-confirm prompt so users see an unfinished update before cancelling it. - Parse the updater's PROGRESS lines (new CardUpdateProgressParser, unit-tested) in both foreground and background runs and log stage changes to the new card_database.update category, throttled to at most one line per second; forward Oracle stderr to the logs. - Show a tray balloon when a background update starts and on finish or failure instead of staying silent. - Request a running Oracle instance to raise itself (QLocalServer CockatriceOracleRaise-<user>) when the user starts another update, and demote the 'already running' guard to an informative message only when no running instance can be found. - Emit download/scan/import progress unconditionally from Oracle so interactive and background runs behave the same. - Document the new log category in the troubleshooting guide. * [Client] Address card database update review feedback * [Client] Add setting to control card database update tray notifications * [Client] Address remaining card database update review nits - Rename card_update_progress_parser.{h,cpp} to card_update_progress.{h,cpp} - Store the CardUpdateProgress struct in the status bar widget * Lint. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-09 11:07:48 +02:00
BruebachL	6bed5d6f67	[Style] Warn about the broken Windows native styles and stop resolving System to them (#7461) ... * [Style] Fall back to the built-in windows style instead of windowsvista The Windows Vista native style is broken the same way as the Windows 11 one, so the sanitizer now catches both and falls back to the built-in QWindowsStyle instead of the Vista plugin. The style picker no longer offers either. * [Style] Keep Windows native styles selectable but warn about them --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-09 11:07:22 +02:00
BruebachL	205e1ecebf	[CardDB] Keep parse-time set filtering and bust the cache on enablement changes (#7448) ... The parsers keep dropping printings for disabled sets so that disabling a set hides them everywhere, not just in deck editor search. The cached parse output therefore depends on enablement, which the source hash did not cover: the first parse froze whatever enablement state existed at that moment and later toggles hit the stale cache. - Fold a parser schema version and the sorted enabled set names into the binary cache source hash so toggling a set or enabling newly discovered sets (including custom ones) forces a re-parse on the next load. - Harden the legacy set migration to only write keys that exist so missing legacy values can no longer clobber cardDatabase.ini and silently disable sets on upgrade. - Make CardDatabase::reloadCardDatabasesAndNotify() bypass the binary cache so an explicit reload always re-reads the files on disk, even when their size/mtime hash still matches. - Cover cache invalidation end to end: sandboxed loads assert that disabling a set, adding a custom database and enabling a fresh set each rewrite the cache, that a no-op load keeps it, and that an explicit reload picks up edits the source hash cannot see. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-09 06:09:18 +02:00
BruebachL	aaafa72c0e	[Game] Use native title bar height for ZoneViewWidget close button (#7450) ... * [Game] Use native title bar height for ZoneViewWidget close button Compute title bar height from style (PM_TitleBarHeight) instead of hardcoded 24.0 to align hit box with native close button. * [Game] Fix ZoneViewWidget title bar height compile errors Drop the redundant static on the anonymous namespace helper, stop redeclaring titleBarHeight's parameter and reuse tbh in the fallback path, and restore the position clamp that still referenced the removed kTitleBarHeight constant. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-09 06:08:15 +02:00
BruebachL	56c456414b	[Game] Use non-blocking popup for zone context menus (#7452) ... Replace QMenu::exec with popup() for zone context menus to avoid blocking the scene. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-09 06:07:39 +02:00
BruebachL	95f02ac0a1	[CardDB] Drop the rebalanced-cards setting entirely (#7457) ... * [CardDB] Drop rebalanced-card filtering from the v4 parser Rebalanced cards no longer exist, so the includeRebalancedCards preference has no data to filter. Always keep printings marked isRebalanced and remove the now-unused ICardPreferenceProvider plumbing from CockatriceXml4Parser and CardDatabaseLoader. The setting itself (CardsDisplaySettings, Manage Sets checkbox, Oracle attribute mapping) is removed in a follow-up. * [CardDB] Drop the rebalanced-cards setting entirely Rebalanced Alchemy cards no longer exist, so remove the includeRebalancedCards preference surface: the CardsDisplaySettings getter/setter/signal, the ICardPreferenceProvider and ICardsDisplaySettingsProvider methods, the Manage Sets checkbox, the legacy settings migration mapping, the isRebalanced attribute mapping in the Oracle importer, and the isRebalanced attribute in the v4 schema. Builds on #7448, which already stopped the v4 parser from filtering on the preference. * [CardDB] Drop the unused isOnlineOnly printing property The Arena-card detection switched from isOnlineOnly to isRebalanced in #5778, so nothing has read this printing property since; it only round-trips from the Oracle importer into cards.xml. Remove the importer mapping, the benchmark fixture field and the v4 schema attribute so regenerated databases stop carrying it. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-09 06:02:47 +02:00
BruebachL	d0fc27e3a8	[Security] Harden network input handling and websocket proxying (#7076) ... - TCP client connections: reject messages whose declared length is negative or exceeds a 1.5MB cap immediately instead of buffering indefinitely, preventing unbounded memory growth and the previously stuck state caused by negative lengths overflowing the int header. - ISL connections: apply the same length validation with a 4MB cap, disconnecting the peer on an implausible length. - Websockets: only trust the forwarded client-address header (e.g. X-Forwarded-For) when the websocket connection originates from an address in security/trusted_sources. Previously any client could spoof its address to bypass bans, user limits and rate limiting. Took 3 minutes Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-08 23:18:47 +02:00
BruebachL	514014dc00	[Completer] Stop line edit rewrite on popup highlight (#7447) ... * [Completer] Stop line edit rewrite on popup highlight * [Completer] Extract the completion index into a variable Make the popup selection update readable at a glance. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-08 14:38:11 +02:00
BruebachL	870ab23608	[Tooling] Add IWYU Qt6 mapping, generator, and runner script (#7454) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-08 11:17:01 +02:00
BruebachL	d603a2bb6a	[Tooling] Apply include-what-you-use across the tree (#7396) ... * [DeckList] Apply include-what-you-use to libcockatrice_deck_list * [IWYU] Apply include-what-you-use across the tree Run one whole-tree IWYU pass over the libcockatrice_* libraries, the Cockatrice client, Oracle, Servatrice and the test suite, so each translation unit includes what it uses directly and forward-declares what it does not need. IWYU only analyses real translation units, never the moc code CMake generates from Q_OBJECT headers, so it strips includes that moc output silently relies on. Where a type is passed by value (or nested in a QList/QMap) in a signal or slot signature, the generated moc code needs the complete type; those includes are kept with an explanatory `// IWYU pragma: keep`. The same holds for Q_DECLARE_METATYPE targets such as QSqlDatabase. A handful of includes are added by hand where callers brace-initialize a const-reference parameter, which also requires the complete type. Applied as a single commit on purpose: stripping includes from the libcockatrice_* headers is precisely what breaks the client, test and server translation units, and the fixes for those live in the same translation units. Splitting by component would therefore leave intermediate commits that do not compile. All 426 modified Q_OBJECT/Q_GADGET headers were verified to be moc-self-contained by compiling each generated moc_*.cpp standalone; several built only because of moc include ordering in mocs_compilation.cpp before the keep-includes were added. * [Tooling] Emit public Qt headers and fix IWYU fallout CI found Three problems the local build could not see, all because a green local build only proves the configuration it was run under. Private Qt headers. IWYU emitted private, lowercase Qt headers, and qtypes.h does not exist at all before Qt 6.9, so the Debian 12 and Ubuntu 24.04 jobs on Qt 6.4.2 failed to compile. The generator only emitted a mapping when the capital header name mirrored the private one (QAbstractAnimation for qabstractanimation.h), so qtypes.h, which is wrapped by QtTypes instead, got no mapping and kept the private spelling. Map it explicitly to <QtTypes>, which is public since Qt 5 and provides the same typedefs either way, and rewrite the 100 affected includes. Do not generalize this to "prefer any Qt* wrapper". The table is generated from whichever Qt is installed, and the wrappers are not equally old: QtLogging only appeared in Qt 6.8 and QtMinMax in 6.5, so that heuristic trades one breakage for another. Every other lowercase header still in the tree has shipped publicly since Qt 5, so those are left alone. Platform-guarded includes. IWYU only sees the preprocessor configuration it is handed, so it dropped an include that only the macOS branch of main.cpp needs, and the macOS jobs failed on an undeclared UrlSchemeEventFilter. Restore the include. Force-compiling every file that carries a platform guard under Q_OS_MAC, Q_OS_MACOS and __APPLE__ reports nothing else, so the macOS branches are covered rather than assumed. shellcheck. Drop a leftover variable and stop assigning the include-what-you-use lookup, which was only ever an existence check. * [Tooling] Map Qt 6.5+ split headers to QtGlobal for Qt 6.4 compatibility IWYU emitted QtTypes, QtPreprocessorSupport, QtVersionChecks, QtAssert, QtMinMax, QtClassHelperMacros, QtEnvironmentVariables and QtTranslation. Qt 6.5 split those declarations out of qglobal.h into separate headers, so neither the Qt* wrapper nor the q*.h private name exists on the oldest Qt we support (6.4.2), and Debian 12 / Ubuntu 24.04 fail with "No such file or directory". qtypes.h is worse still: it only appeared in Qt 6.9. QtGlobal is the one spelling that works across the whole supported range. It has been public since Qt 5, defines these declarations itself up to Qt 6.4, and includes the split headers from Qt 6.5 on, so redirect all eight private headers to it and collapse the affected includes in the tree. Curated overrides now take precedence over the name-mirroring rule, which cannot know about Qt version floors because the mapping is generated from whatever Qt is installed. That rule was mapping qtversionchecks.h to <QtVersionChecks> even with an override present, because the override was only consulted as a fallback. A curated key that matches nothing is now a hard error rather than a silent fallback, and the redirected headers are no longer advertised as the home of a symbol. Verified every Qt include in the tree resolves against the real qt6-base-dev 6.4.2 package: 15 distinct includes, 0 missing. * [Tooling] Map QOverload to QtGlobal and drop the QMetaMethodArgument include Two more headers IWYU emitted do not exist on Qt 6.4.2: QOverload was split out of qglobal.h in Qt 6.5, so on 6.4.2 it is declared in qglobal.h directly and the standalone header is a hard error. QtGlobal covers both, so map qoverload.h to it like the other 6.5+ split headers. QMetaMethodArgument is never referenced anywhere in the tree. It is a bare forwarder for qobjectdefs.h that our generator advertised as a class symbol, which is how IWYU came to add the include at all. The symbol mappings are generated from header names, so they cannot tell a forwarder that happens to be capitalised from a real class; removing the unused include is the fix, and the generic case stays a known limitation of the generator. Extending the audit to every Q* header (an earlier pass only matched headers starting with "Qt" or lowercase q*.h, so it missed all of them) against the real qt6-base-dev, qt6-multimedia-dev, qt6-declarative-dev, qt6-svg-dev and qt6-websockets-dev 6.4.2 packages: 341 distinct Qt includes, 0 missing. * [Game] Give moc the complete CardItem type in player_actions.h player_actions.h forward declares CardItem but passes QList<CardItem *> by value in actPlay/actPlayFacedown/actHide. moc instantiates the metatype for those parameters, and on Qt 6.4 that expands to sizeof(CardItem), so Ubuntu 24.04 fails with "invalid application of 'sizeof' to incomplete type 'CardItem'". Newer Qt tolerates the incomplete type, which is why only the 6.4.2 jobs failed. Include the defining header with an IWYU pragma keep, matching how the other MOC-required includes in this area are handled. * [Game] Give moc the complete ServerInfo_Card type player_logic.h, game_scene.h, view_zone.h and view_zone_widget.h forward declare ServerInfo_Card but pass QList<const ServerInfo_Card *> through signal and slot signatures. moc instantiates the metatype for those parameters, which Qt 6.4 expands to sizeof(ServerInfo_Card), so the connect in game_scene.cpp fails on Debian 12 and Ubuntu 24.04 with "invalid application of 'sizeof' to incomplete type 'const ServerInfo_Card'". Newer Qt tolerates it, so only the 6.4.2 jobs failed. Include the generated protobuf header with an IWYU pragma keep, following the pattern already used for ServerInfo_Player and ServerInfo_User in game_event_handler.h. * [Client] Complete the types Qt 6.4 needs for connect and moc Qt 6.4 expands QMetaType::fromType on the normalised argument list of a PMF connect and on moc-generated metatypes, which requires sizeof on every signal and slot parameter. Newer Qt tolerates forward declarations there, so only the Debian 12 and Ubuntu 24.04 jobs failed, with "invalid application of 'sizeof' to incomplete type" for QMouseEvent, QPrinter and GameReplay. Replace the forward declarations with the real includes: - QMouseEvent in the nine widget headers that pass it through signals - QPrinter in abstract_tab_deck_editor.cpp, for the actPrintDeck lambda - game_replay.pb.h in tab_replays.h. GameReplay is a protobuf message, so the project's game/replay.h only forward declares it and cannot supply the complete type. Found by recompiling all 1918 project TUs against the Debian Qt 6.4.2 headers, which reproduces those two CI jobs locally in one pass. * [Client] Complete Release and QMenu for Qt 6.4 moc and connect Continuing the Qt 6.4 complete-type sweep: - window_main.cpp connects ClientUpdateChecker::finishedCheck to MainWindow::checkClientUpdatesFinished, whose Release * parameter made moc 6.4 instantiate the metatype. GameReplay turned out to be a protobuf message, so Release likewise needed release_channel.h in the three headers declaring those signals: window_main.h, dlg_update.h and client_update_checker.h. - moc 6.4 also needs sizeof(QMenu) for the QList<QMenu *> carried by TabSupervisor::setMenu and MainWindow::updateTabMenu. Verified by regenerating moc output with the Debian moc 6.4.2 for all 434 Q_OBJECT/Q_GADGET classes and compiling it, plus recompiling all 1612 source TUs, against the Qt 6.4.2 headers: no remaining failures. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-08 11:13:50 +02:00
BruebachL	b1f4d1e99d	[UI] Flip tear-off menus default to off (#7449) ... Detach menus no longer default to tear-off; users can re-enable in settings. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-08 10:51:03 +02:00
BruebachL	d0991b68f6	[EDHRec] Update tab title when a card is searched (#7408) ... TabEdhRecMain never emitted tabTextChanged, so TabSupervisor::updateTabText was never called and the tab kept the title it was created with. addEdhrecMainTab() creates the tab without a card, so every search left the title reading "EDHRec: " with no card name for the rest of the tab's life. addEdhrecTab() sets the card before myAddTab(), which is why the older TabEdhRec tab was never affected. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-07 14:55:25 +02:00
tooomm	08a608992d	[Docs] Cleanup Doxygen files (#7361) ... * Table layout * table layout * table layout * Delete doc/doxygen/extra-pages/developer_documentation/network/game_event_handler.md * Delete doc/doxygen/extra-pages/developer_documentation/network/player_event_handler.md * use default value	2026-10-06 07:23:41 +02:00
tooomm	b545efb929	[Readme] Add hint on building docs (#7389) ... * add hint on building docs * Update README.md * Update README.md * Expand doxygen README	2026-10-04 21:48:10 +02:00
tooomm	a02523ee9e	Add CONTRIBUTING file to doc links (#7395)	2026-10-04 21:46:56 +02:00
github-actions[bot]	a8e9eb3c1b	Update translation source strings (#7397)	2026-10-04 21:08:47 +02:00
tooomm	327a6bfd8a	[CI] Fix permissions for automations around translations (#7394) ... * Update translations-push.yml * Update translations-pull.yml	2026-10-04 20:20:28 +02:00
BruebachL	86039d7eb6	Change release name for 3.1.0 release. (#7388) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-04 20:15:31 +02:00
Michael Hogg	0a9d6b2226	Include git metadata in docker build context (#7393) ... cmake/getversion.cmake reads the commit date and release tag from git, but .dockerignore excluded .git/, so docker builds had no git info and Servatrice reported its version as "3.1.0 ()". Keep .git/ in the build context; only /usr/local is copied into the final image, so its size is unaffected. Co-authored-by: Michael Hogg <3495076+jedinerd@users.noreply.github.com>	2026-10-04 03:48:15 +02:00
Michael Hogg	885b50de00	Allow manually publishing a release image in docker workflow (#7392) ... Add a workflow_dispatch trigger that builds an existing release tag with the current workflow and publishes it to GHCR under that tag name (optionally also as latest). This allows republishing a release whose image failed to publish, since re-running the original run reuses the workflow file from the release tag. Dispatched runs check out the given tag, publish like stable releases, and use their own concurrency group so pushes to master can't cancel them. Co-authored-by: Michael Hogg <3495076+jedinerd@users.noreply.github.com>	2026-10-04 02:44:54 +02:00
Michael Hogg	76bcc4842b	Fix digest artifact names in docker release workflow (#7391) ... With archive: false, actions/upload-artifact ignores the name input and names the artifact after the uploaded file, so the digest artifacts were named after the raw digest instead of digest-<label>. The index job's download-artifact pattern digest-* then matched nothing, leaving no digests to build the image index from. Drop archive: false so the digest-<label> names apply. Co-authored-by: Michael Hogg <3495076+jedinerd@users.noreply.github.com>	2026-10-04 02:35:24 +02:00