4128 commits

Author	SHA1	Message	Date
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
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 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 20:09:34 +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	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	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	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	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
github-actions[bot]	a8e9eb3c1b	Update translation source strings (#7397)	2026-10-04 21:08:47 +02:00
tooomm	5dc5390880	Fix and rework client updater logic (#7038) ... * first version * combined logic + better regex * Simplify, we only support 64bit, installer prevents wrong installs * lint + fix Linux versions (missing declaration) * update regex * improve/add comments * comments * ReleaseChannel: parse non-numeric OS versions on Windows Server QSysInfo::productVersion() returns "Server 2022" on Windows Server, so splitting on '.' and calling toInt() made the version unparseable and disabled the in-client updater there. Extract the first numeric token instead; Server 2022 becomes 2022 and matches the Win10 assets. * ReleaseChannel: don't offer 64-bit assets on 32-bit Windows The old downloadMatchesCurrentOS() had an explicit 32-bit branch that only accepted "32bit" assets. Restore that guard: 32-bit Windows builds return no match instead of being offered the 64-bit Win10 installer that won't run. * ReleaseChannel: only query the OS version on updater platforms The in-client updater only ships assets for macOS and Windows, but the host system version was still being resolved on every platform, including Linux where it is never used. Compute it only under the Q_OS_MACOS / Q_OS_WIN guards. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-03 17:57:03 +02:00
BruebachL	0934380a5c	[Translations] Load Qt's own translations and build ours with the binary (#7384) ... * [Translations] Load Qt's own translations and build ours with the binary Three separate bugs kept translations from working. installNewTranslator() loaded the application translation into qtTranslator. QTranslator discards its previous contents on every load, so that wiped the qt_<lang>.qm loaded one line earlier and installed an empty translator instead. Qt's own strings were never translated on any platform. Give each file its own translator. QLibraryInfo::path(TranslationsPath) was the only directory searched for Qt's translations. A relocated Qt can report a path that does not exist: the KDE Flatpak runtime reports /usr/translations while its files live in /usr/share/qt6/translations. Probe the usual install layouts and only hand back the ones that exist. The .qm files were attached to the default build target only, so building the executable on its own left it without any translations and the language selector empty. Attach them to the target as well. Move the shared lookup into TranslationLoader so client and Oracle cannot drift again, and have it report a missing translation at debug level. An unset language is the default for English, which has no file to begin with, and warning about it only added noise to every fresh debug log. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> * [Tests] Define main() in translation_loader_test Without a GTest installation the fallback in tests/CMakeLists.txt links only gtest, not gtest_main, so the test has to bring its own main(). Every other test already does; only this one relied on the distribution package and failed to link on CI. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> * [Tests] Put the translation loader fixture next to the test executable A multi-config generator (MSBuild, Xcode) writes the executable to a per-config subdirectory while the .qm stays in the current binary directory, so applicationDirPath() never saw the fixture and ABuildTreeFindsTheTranslationsNextToTheExecutable failed on Windows. * [Translations] Depend on Qt's lrelease target and skip the loader test without lrelease Attaching the .qm custom commands to a second custom target races with Qt's own <Project>_lrelease target on MSBuild/Makefiles and is rejected by Xcode, so depend on Qt's target instead. Only build the translation loader test when Qt6::lrelease is available. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> Co-authored-by: ZeldaZach <zahalpern+github@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-03 15:04:30 +02:00
BruebachL	3aba1ef929	[Settings] Back up the settings folder before migrating (#7386) ... * [Settings] Back up the settings folder before migrating The per-file settings migration only preserved global.ini, as global.ini.old. Everything else the migrations write was overwritten in place with no way back: the legacy migration and the 17 per-domain migrateXxxSettings functions all write straight into the per-domain .ini files, so any of them that got a key mapping wrong silently replaced a working file. The re-migration path is the sharpest case, because migrateSettingsFromGlobalIni() runs again whenever an older build wrote non-sentinel keys back into global.ini, and those values then land on top of per-domain files the user had already edited. Snapshot the whole settings folder before either migration runs, into a timestamped sibling settings-backup-<yyyyMMdd-HHmmss>. A sibling rather than a child, so a backup is never copied into the next backup and the "Open Settings Folder" action keeps showing only live files. Keep every snapshot; settings files are kilobytes, so growth is not a concern. SettingsMigration::isMigrationPending() gates the copy so a normal start creates nothing. It reuses the same condition migrateSettingsFromGlobalIni() uses to decide whether it has work, factored into a shared helper so the two cannot drift, and it ignores the legacy migration on an empty profile where that migration only writes its sentinel. The backup itself skips a folder whose files are all empty, which covers a fresh profile whose global.ini was created but never written. A failed copy logs a warning and returns an empty path rather than aborting the migration, since a full disk or an unwritable data folder must not leave the client unable to start. global.ini.old still works as the single-file escape hatch. Document the folder layout, the automatic backups, and the fact that migrations only run forwards, so users know to copy the settings folder before downgrading to a version that cannot read the per-group files. * [Client] Match the desktop file name case for the portal app ID QGuiApplication::setDesktopFileName() was given "Cockatrice"/"Oracle" while the installed desktop entries are cockatrice.desktop and oracle.desktop. Linux paths are case-sensitive, so xdg-desktop-portal failed the app-info lookup for "Cockatrice" and Qt warned on every QDesktopServices call that it could not register with the host portal. The same string becomes the xdg-shell appId, so the window icon lookup failed on Wayland for the same reason. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-03 14:59:58 +02:00
BruebachL	b8b81170bf	[Client] Show card names in the game log and [card] tags in the chosen card language (#7387) ... The game log translates its sentences, zone names and counter names, but every card name was rendered as the raw canonical name straight from the server. The card board and the card info text already went through CardLocalization::displayName(), so the log was the odd one out. cardLink() used one string for both the card:// href and the visible label, which left no room for a localized label. Split the two: the href keeps the canonical name because ChatView::mousePressEvent feeds it straight to showCardInfoPopup for lookup, while the label goes through the display language. Same split for [card] chat tags in ChatView::appendCardTag. Sanitize the name while building the link. logUndoDraw already did this by hand and had drifted into its own non-italic copy of cardLink(); route it back through the helper. Add CardLocalization::displayNameFor() for callers that only hold a canonical name and need no CardInfoPtr of their own. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-03 13:47:07 +02:00
BruebachL	014983b5bc	[Theme] Carry Default theme directory customizations over to System (#7385) ... * [Theme] Carry Default theme directory customizations over to System The Default -> System rename rewrote the stored theme name but left the user profile's themes/Default directory behind, so anyone who had customized zone graphics, stylesheets or palettes there silently lost them. Migrate the directory alongside the name. Rename it when System does not exist yet, otherwise merge file by file without overwriting what System already has, then drop the legacy directory. Users who already ran the rename get their customizations back on the next start, and a hand-made Default folder of a user on another theme is left untouched. * [Tests] Give theme_migration_test an explicit main and gate it on WITH_CLIENT The build machines that cannot find GTest fall back to the downloaded googletest, which links the bare gtest library without gtest_main. Every other test defines its own main() for that reason; this one did not, so Debian 13, Fedora 44 and Ubuntu 26.04 failed to link it. The test compiles client sources, so -DTEST=1 -DWITH_CLIENT=0 has no way to build it. Gate the subdirectory on WITH_CLIENT like the other client-source test. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-03 13:43:25 +02:00
BruebachL	add65caae5	[Client] Hand off single-instance payloads outside the readyRead signal (#7373) ... * [Client] Hand off single-instance payloads outside the readyRead signal Opening a cockatrice:// link from outside a running client showed the "Join game ...?" confirmation and then crashed on both answers. The join reached the server first, so the other players saw the user join and immediately disconnect. SingleInstanceManager::handleNewConnection() emitted filesReceived from inside the forwarding socket's readyRead lambda and only disconnected afterwards. A joingame link makes the handler show a modal QMessageBox, which runs a nested event loop. The sending instance exits as soon as it has the acknowledgment, so the socket was disconnected and deleteLater'd inside that loop: it died in the middle of its own readyRead signal, and both this lambda and Qt's signal dispatch kept using the freed memory. Disconnect right after the acknowledgment and emit filesReceived through a queued call, so no handler and no nested event loop ever runs inside the socket's signal. Guarding the disconnect call alone is not enough, since Qt still touches the socket after the lambda returns. Add a regression test that reproduces the nested-event-loop hand-off. * [Tests] Apply cmake-format to the single-instance test wiring * [Client] Defer the single-instance socket teardown past the readyRead frame The forwarded payload is emitted synchronously from inside the forwarding socket's readyRead, which is what lets the modal "Join game?" confirmation appear. A handler can spin a nested event loop, and the sending instance exits as soon as it has the ACK, so the socket is disconnected and deleteLater'd while that loop runs. Control then returns to this frame with the socket already destroyed, and the inline disconnectFromServer() touched freed memory. Queue the teardown instead and guard it with a QPointer taken before the handlers run -- building the guard afterwards is itself a use-after-free, since QPointer dereferences the pointer it is constructed from. The payload still reaches the handlers synchronously, so the confirmation is unchanged. The regression test covers both halves: the handlers must run while the socket is still connected, and the socket must not be torn down until the readyRead frame has returned. * Lint. * [Client] Only delete the single-instance socket outside the readyRead frame The auto-delete on `disconnected` was the remaining use-after-free. A handler may spin a nested event loop (the modal "Join game?" box), the sending instance exits as soon as it has the ACK, and so the peer closes while that loop runs. deleteLater() issued from inside such a loop is not deferred to the outer loop: Qt frees the object as soon as the nested loop unwinds, which is still inside this readyRead emission, so Qt's own signal dispatch and the handler lambda both keep using a freed socket. When the notification happens to arrive late the bug hides, which is why this only reproduced on some platforms. Drop the auto-delete and let the queued teardown own the socket's lifetime: it disconnects and then deletes, once the frame has unwound. The QPointer guard was necessary but not sufficient -- it stopped us dereferencing a dead pointer, not the socket being freed underneath the frame. The test waits on the socket's destroyed signal instead of polling, so the teardown assertion does not depend on platform event timing. * [Client] Do not queue the single-instance socket teardown The previous commit moved the deleteLater() off the disconnected signal and into a queued invocation, believing that "queued" meant "after the readyRead frame returns". It does not. A queued call is delivered by whichever event loop is running, and the modal "Join game?" box runs a nested one -- so the teardown was still executed inside the readyRead emission, which is exactly the use-after-free it claimed to prevent. Queueing was never necessary. Because nothing connects deleted/close to the socket's disconnected signal any more, no nested loop can free it while the handlers run, and emit filesReceived() is synchronous: once it returns, the frame has unwound and the socket can be closed and deleted inline. The QPointer stays, covering the one case that remains -- the primary shutting down while the confirmation box is open, since QLocalServer destroys its pending connections. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-02 21:19:02 +02:00
BruebachL	4ce2e8c3d3	[UserList] Fix join/spectate from the profile hover card doing nothing (#7383) ... The games list in the profile hover card offered "Join game" and "Spectate", but UserInfoPopup::joinGameRequested was only forwarded to an identical UserListWidget::joinGameRequested signal that nothing connected to. Clicking either entry sent no command, opened no tab and showed no error. Route the request to TabSupervisor, which owns the join flow. The popup's games list spans every room the user is in, so the target room may not be open yet; IntentJoinServerGame already joins the room first and retries, which is why this goes through the intent rather than straight to GameSelector. Extract the intent construction that joinReportGame already had into a shared joinGameByIntent helper instead of duplicating it. Also stop disabling "Join game" for games that are full or already running. GameSelector::joinGame() offers to join as a spectator in that case, so a greyed-out entry read as a dead button; GameSelector's own context menu never disables it either. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-02 18:57:07 +02:00
BruebachL	750d5b3c21	[Client] Add option to use game time in game log timestamps (#7265) ... * [Client] Add option to use game time in game log timestamps (#2202) Game log timestamps showed the local clock time, which is meaningless in a replay or in a log read after the fact — nobody can tell when a play happened relative to the game. Add a "Use game time instead of local time in game logs" setting. When enabled, the message log stamps entries with the elapsed game time (HH:MM:SS since the game started); regular chat views keep the local clock. * [Game] Move formatElapsedTime helper out of public slots (#2202) The message log needs the HH:MM:SS formatter used by the game timer, but it lives in GameState's public slots, tying a pure formatting helper to the signal-slot machinery. Move formatElapsedTime into a public section of GameState so the log can call it directly, and add the missing trailing newline to game_state.cpp. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-02 17:30:48 +02:00
RickyRister	ffcb9094ca	[Replay] Fix cards accumulating in zone view when rewinding (#7357) ... * [Replay] Fix cards accumulating in zone view when rewinding * rename method	2026-10-02 17:17:34 +02:00
BruebachL	ecd955bc60	[Installer] Wait on the launching process so auto-updates cannot write over a mixed Qt runtime (#7372) ... * [Installer] Abort instead of installing over leftover runtime files The silent upgrade could leave a mix of old and new Qt DLLs next to the new executables, which is what makes the next start fail with "The procedure entry point X could not be located in the dynamic link library ...Qt6QuickWidgets.dll" or "The code execution cannot proceed because Qt6Multimedia.dll was not found". The check that was supposed to catch this probed only Qt6Core.dll and Qt6Network.dll, so it was blind to the two files the reports name. It also only warned and then installed anyway, so a caught problem was still compounded. - Scan every file the purge targets with FindFirst, which takes the same wildcards the Delete/RMDir calls use, instead of two hardcoded names - Widen IsExistingCockatriceInstall from Qt6Core.dll to Qt6*.dll so a directory left holding only some Qt modules is still recognised as an install - Gate the scan on that, so a first-time install into a directory that happens to hold unrelated Qt5 DLLs is not turned into an abort - Abort with a non-zero exit code instead of warning, so the new payload is never written next to files of the previous version - Skip the message box on an unattended /S install, which would block on it forever, matching the two existing abort sites * [Installer] Cancel a silent upgrade when the running-instance guard fails The running instance guard is what stands between a locked install directory and the mixed Qt DLL set that breaks the next start, but it failed open in two places and let the install carry on anyway: - powershell.exe could not be started: the check warned and returned - the process lookup answered "could not determine": WaitForAppToClose returned on $R0 != 1, so "unknown" was treated exactly like "not running" On a silent upgrade there is nobody at the keyboard to close the applications in response to that warning, which is precisely the state in which the launcher was still holding its Qt DLLs mapped. - Treat "could not determine" as its own state rather than folding it into "not running", in all three places WaitForAppToClose inspects the result - Cancel a silent upgrade in both cases; keep an interactive install on the existing warn-and-continue path, since refusing there would leave an installer that can never complete and the stale-runtime check still applies - Route every abort through one AbortInstall so the dialog-vs-silent decision lives in a single place, and stop the two pre-existing warning dialogs that a /S install without /R would have blocked on forever * [Installer] Wait on the process that launched the update installer The in-app updater starts the installer and only then quits itself, so the installer has to decide whether the launching process is still alive while it is running. It did that by polling powershell.exe for an image name under the install directory, which is a proxy for the real question - is the process that started me still holding its Qt DLLs mapped - and a proxy that can answer "not running" or "could not determine" while the process is very much alive. Either answer let the install carry on into the mixed Qt DLL set. Take /PID= from the command line and wait on that process handle directly: - OpenProcess with PROCESS_SYNCHRONIZE only, which is all that is needed to wait and the least likely to be refused for an unrelated reason - WaitForSingleObject for the same 60 s the name based check allows, so a client slow to flush its card database is not squeezed - Treat an unopenable handle as "already exited", which is what it means for a dead process - Cancel the upgrade if the launcher has not exited, rather than replacing files it is still using Run it before EnsureAppsNotRunning, not after: it needs no powershell.exe, so it is the one check that still works on a machine where the process lookup cannot run, and it is what lets that case keep upgrading instead of being cancelled by what was only ever a proxy for it. Keep an older client, which passes no /PID=, on the existing path. An older installer ignores /PID=, and /D= stays the last parameter either way. * [Client] Pass our process id to the update installer The updater starts the installer and only then quits itself, so the installer begins replacing files while this process still has every Qt runtime DLL mapped, and a mapped DLL cannot be deleted. It has to work out that it must wait, and until now all it had was its own image name to go on. Pass /PID= so it can wait on this process directly instead of guessing, and cancel the update rather than replace files this process is still using. /D= stays the last parameter, which NSIS requires. * [CI] Fail the build when the installer is missing a Qt runtime DLL The Windows payload is assembled by globbing every *.dll out of the build output directories, so nothing but luck decides whether a module the client links statically - Qt6Multimedia.dll, Qt6QuickWidgets.dll - reaches the installer at all. A module that is left out is invisible until a user installs an update and the client then refuses to start, with "The code execution cannot proceed because Qt6Multimedia.dll was not found" or a missing procedure entry point in a Qt6QuickWidgets.dll carried over from the previous version. Derive the required list from SERVATRICE_QT_MODULES, COCKATRICE_QT_MODULES and ORACLE_QT_MODULES, the same module lists the targets are actually built against, so it cannot drift from what the binaries need, and write it next to the CMake cache as required-qt-runtime.txt. Add the platform plugin, without which the client cannot start at all. Assert it in the existing 7-Zip pass that already rejects build-tree artifacts, comparing exact relative paths so a same-named file in the wrong directory does not satisfy a requirement, and normalizing the archive's own separators so it does not matter which one NSIS chose. Missing manifest, missing entry or missing 7-Zip-independent failures all stop the build rather than warn, which is the whole point: the previous check already failed hard for artifacts, and a warning here is what let a broken package ship. * [Client] Point the Windows qt.conf translations path at the real one The Windows qt.conf carried Translations = Resources/translations, copied from the macOS bundle block, but the Windows install has no Resources directory: the .qm files are installed to COCKATRICE_WIN32_QM_INSTALL_DIR, which is translations, and main.cpp looks for them at applicationDirPath() + "/translations". Leave the macOS value alone, which is correct for it. * [Installer] Close the /? help string so the installer compiles again The last line of the continued MessageBox string ended in $\n\" instead of $\n". NSIS reads \" as an escaped quote, so the string never terminated and makensis rejected the script, which took the CPack packaging step down with it. The Windows build failed on "Problem running NSIS command" before ever reaching the new required-runtime check. * [Installer] Give FindFirst the two arguments it takes FindFirst takes the pattern and the output variable and leaves the search handle in $0. There is no handle argument, and FindNext taking only a variable is the giveaway: the handle has to live somewhere implicit, and it is $0. The call passed three arguments, so makensis rejected the script and the CPack packaging step failed with "Problem running NSIS command" before ever reaching the required-runtime check. This was not only a compile error. The handle variable was never populated, so the stale-runtime check that commit 844a1a9b added could never see a match: it probed $StaleFindHandle, which stayed empty, and the widened wildcard scan over the purge set was a no-op. Pass the pattern, test $0, and close $0. No FindNext is needed, because $R0 is a yes/no flag rather than a list. Drops the now unused StaleFindHandle Var. * [CI] Dump the NSIS compile log when packaging fails CPack reports a failed makensis run as "Problem running NSIS command" plus a path to NSISOutput.log, and that log exists only on the runner, so the actual compile error never reaches the workflow output. An installer script that does not compile is therefore undiagnosable from CI alone, which is exactly how a three-argument FindFirst cost two build cycles here. * [CI] Run the NSIS log dump under bash The dump step used the default pwsh shell, where `find` resolves to Windows' text-search find.exe rather than a file finder, so it died with "FIND: Parameter format not correct" instead of printing anything. Pin shell: bash. * [Installer] Test the error flag with LogicLib's ${Errors} condition makensis rejected the script with "macro _If requires 4 parameter(s), passed 2" at the ClearErrors/OpenProcess pair. A ${If} condition cannot be a bare $Var: LogicLib needs either a comparison or one of its flag macros, and a lone $Errors splits into too few arguments for _If. ${Errors} is the documented flag condition and is the same mechanism as the ${Silent} test already used here. The dump step earned its keep - this took one run to locate rather than another round of guessing. * [Installer] Give FindFirst its handle back and stop blocking silent aborts makensis: "FindFirst expects 3 parameters, got 2. Usage: FindFirst $(user_var: handle output) $(user_var: filename output) filespec". The previous commit was wrong about this. FindFirst does take the handle explicitly, and FindNext finds it in $0 as a side effect of the FindFirst that set it, which is why FindNext takes only a variable. The three-argument form the stale-runtime check was written with was correct all along, and the two-argument form broke it. Restored, along with Var StaleFindHandle. That same commit inverted the interactive check in both abort paths. `${If} $ReinstallMode = 1` with `${OrIfNot} ${Silent}` shows the message box exactly when the install is the silent /R upgrade - the one case the comment says must never block, since nothing will dismiss it and the installer hangs until it is killed. Replaced with `${IfNot} ${Silent}`, so the box appears only for an interactive install and a silent abort goes straight to SetErrorLevel. Audited every construct added by this branch against master, which compiles: all 9 LogicLib condition shapes have a master precedent, FindFirst is the only FindFirst and is back to 3 arguments, Function/Section and block depth balance, and the 12 lines with an odd number of quotes are the same pre-existing string continuations master has. * [CI] Strip the CR 7-Zip leaves on its paths before comparing them The installer check reported all 14 required files missing, including Qt6Core-era basics an 82 MB package obviously contains, so the payload was fine and the comparison was broken. 7-Zip writes CRLF on Windows, sed 's/^Path = //p' kept the \r on every line, and grep -qxF matches whole lines - so every entry carried a trailing \r and nothing matched. The build-tree artifact check next to it uses an unanchored grep -E and was unaffected, which is what made the two checks disagree. Reproduced locally against a 7z archive with CRLF injected: the old pipeline matches nothing, the new one finds the present files and reports only the genuinely absent one. Also match case-insensitively. That is how the installed tree behaves - the uninstaller clears both $INSTDIR\plugins and $INSTDIR\Plugins - so a Plugins vs plugins difference in the archive is not a missing file. * [CI] Print both sides of the installer path comparison, and strip its prefixes The check still reported all 14 files missing after the CR fix, so the CR was not the cause and the real difference between 7-Zip's output and the manifest is still unknown. Inferring it again is what produced two wrong fixes in a row, so on failure the check now prints the required list and the first 40 paths 7-Zip lists. A bare "missing" line cannot distinguish a missing file from a misread listing, and twice it was the latter. Also strip $INSTDIR/, $OUTDIR/, $PLUGINSDIR/ and ./ prefixes from the listed paths. Written as separate -e expressions rather than one alternation: with | as the sed delimiter an escaped \| is a literal pipe, so the alternation version silently matched nothing at all. Verified against a 7z archive fed through seven path shapes - plain, Windows backslashes with CRLF, each prefix, lowercase plugins, and all of them combined - each reporting only the file genuinely absent from the archive. * [CI] Strip CR from the manifest side of the installer path check The dump added last run settled it: 7-Zip listed 209 paths including Qt6Gui.dll, Qt6Multimedia.dll and Plugins/platforms/qwindows.dll verbatim, as plain relative paths with no prefix - so the archive side was already clean and the previous fix was right to strip CR there but not sufficient. The trailing CR was on the other side. CMake writes required-qt-runtime.txt with the platform line ending, so on Windows $entry arrived as "Qt6Gui.dll\r", which cannot whole-line match a clean "Qt6Gui.dll". Every entry then read as missing while the listing held it, which is precisely what the two sides of the dump showed. Reproduced locally with a CRLF manifest against a 7z archive: without the strip every entry is reported missing, with it only the genuinely absent file is. Checked across plain paths, Windows backslashes with CRLF, $INSTDIR, $OUTDIR, $PLUGINSDIR, ./ and / prefixes. The literal $INSTDIR-style prefixes are written as [$] bracket expressions so shellcheck does not read them as unexpanded variables; that clears the SC2016 that failed the format job, without a disable directive. * [CI] Reword the bracket-expression comment so the linter stops parsing it A comment whose first word is shellcheck is read as a directive, so "# shellcheck does not read these as unexpanded shell variables" was parsed as a directive named "does" and failed with SC1073 and SC1072 - which cascaded into the SC1009 on the else clause above it. The SC2016 it was meant to document is already gone now that the prefixes are written as [$] bracket expressions, so the note only had to stop looking like a directive. * [CI] Indent the diagnostic path lists without sed, and undo comment drift SC2001 asks for parameter expansion instead of sed 's/^/ /'. Echoing ${manifest//$'\n'/$'\n '} prepends the two spaces directly and keeps the diagnostic block readable. Both occurrences are converted; only the one reading from a here-string was reported, but the piped one has the same shape. Also restore master's wording of the build-tree artifact comment. An earlier edit inserted "in " ("not the install)" became "not in the install)"). It has no functional effect, but it was an unintended change to a line this branch has no reason to touch, and .ci/compile.sh now differs from master only by additions. * [CI] Ship the Qt module runtimes the Windows package check requires The installer check reports Qt6Concurrent.dll and Qt6ShaderTools.dll missing from the Windows package. Both are genuinely absent, not another comparison artifact - the manifest now matches 12 of 14 entries and the dual-list diagnostic shows the other two are not in the archive at all. Nothing deploys them. fixup_bundle copies only the DLLs it finds in Cockatrice.exe's import table, and the *.dll glob only sees what was already copied next to the executable. Qt6::Concurrent is linked but never imported, because QtConcurrent::run and QFutureWatcher are header-only in Qt 6, so the linker drops the import; Qt6::ShaderTools is skipped the same way. The package has shipped without them and the client still starts, but that is luck rather than intent, and required-qt-runtime.txt asks for both. Install the runtime of every linked module explicitly so the installer contains what the check demands. An unused DLL costs a few hundred kilobytes; a missing one is what stops the client starting after an update, and stays invisible until then. Derive both the manifest and the install list from the imported target's TYPE, so a statically linked Qt module - which ships no runtime at all - is skipped by each and the two can never disagree. Scope the manifest to the client rather than the union across server, client and Oracle: only the client is packaged on Windows, and the other two module lists are subsets of the client's, so this changes nothing today. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-02 15:15:29 +02:00
BruebachL	1365f3cb49	[Avatar] Cap user avatar upload resolution at 1024x1024 (#7353) ... * [Avatar] Cap user avatar upload resolution at 1024x1024 * Remove stylesheet. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-27 21:38:50 +02:00
BruebachL	2632f8a040	[Client/Server] Keep non-deck files out of the deck storage (#7352) ... The local deck and replay browsers list every file in the folder, so a stray file (e.g. a PNG screenshot of a deck) can be parsed as a garbage plaintext deck, opened in the editor, and uploaded. The server also accepts oversized deck payloads with only silent truncation. Hide files that are not in a supported deck/replay format in the two QFileSystemModel views (directories stay visible), skip them when opening, reject them when uploading, and make the server reject deck uploads larger than MAX_FILE_LENGTH instead of truncating them. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-27 21:32:49 +02:00
BruebachL	fa8220df3c	[Client] Exit deterministically when launching the update installer (#7309) ... * [Client] Exit deterministically when launching the update installer * [Client] Only quit when the main window close is accepted Wait for MainWindow::close() to be accepted before quitting after the update installer is launched. When the close is vetoed (a running card DB update, open games, or an unsaved deck), keep running and tell the user the installer is already waiting, instead of exiting over their answer. Also fix the comment so it does not claim settings are saved on the vetoed path. * [Client] Only quit on update when the shutdown actually ran MainWindow::closeEvent has a static re-entrancy guard that returns early on a second close event, leaving it in its default accepted state. DlgUpdate reached from a nested event loop while a shutdown prompt was up could then get close() == true with no shutdown work done (no settings flush, no tab shutdown) and tear down the process behind an unanswered prompt. Add MainWindow::closeForUpdate(), which reports false when a close is already in progress or was vetoed, and gate the update-exit on it. * [Client] Use QCoreApplication::exit() to leave on update QCoreApplication::quit() only asks the application to quit and can be interrupted if a top-level window refuses its close event. On the update path the process must actually leave so its Qt DLLs are unlocked before the installer starts replacing files - apply the exit(0) variant both on the accepted-close path and the fallback where no MainWindow was ever closed. * [Client] Warn that the update installer force-closes within a minute Telling the user Cockatrice just 'stays open for now' is misleading: the installer launched by #7308 waits only about 60 s for a graceful close before it force-terminates the process, which would lose unsaved work. State the deadline and tell the user to save and close before then. * [Client] Warn that a vetoed update is cancelled, not forced The warning on the veto path promised the opposite of what the installer does: WaitForAppToClose in #7308 never force-closes cockatrice.exe ($AllowForceClose is 0), it aborts the update after 60 s so no unsaved data is lost. A user who is told Cockatrice will be terminated in a minute hurries to close it, only to find the update already given up. State the real outcome instead: the installer waits about a minute for a close, and the update is cancelled if that does not happen. * [Client] Warn about a waiting installer without blocking it CloseMatchingApps in #7308 sends the WM_CLOSE once and the wait loop only polls IsAppRunning afterwards, but QMessageBox::warning is application modal and QGuiApplicationPrivate::processCloseEvent drops spontaneous close events for windows blocked by a modal widget. The veto path therefore swallowed the installer's one and only request, so even a user who resolved the blocker and closed the app still ended in the 60 s timeout. Move the warning into warnInstallerIsWaiting() and show it modeless, so the main window stays able to answer the installer while the message is up. * [Client] Close the parent on the no-MainWindow update fallback The fallback branch exits the process without closing anything, so it skips the settings flush and tab shutdown this change exists to get in front of the installer - the opposite of the fix if a call site ever stops passing a MainWindow. Close the parent widget first, like the code before this change did, and only then leave unconditionally: the parent is the one that knows how to shut down gracefully, and the deterministic exit is what the installer needs. * Add override. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-27 20:05:41 +02:00
patricktsullivan	7c9dc303f8	[Game] Make 'Put top cards on stack until...' history case-sensitive (#7358)	2026-09-27 09:50:05 +02:00
BruebachL	e9cd9453ea	[Startup] Run the onboarding wizard before the first-run set dialog (#7349) ... On a clean install, checkUnknownSets() fired before the onboarding wizard, so the legacy 'all the sets have been enabled' welcome and the Manage Sets dialog appeared first - with no card data there are no sets to manage yet. Defer the set check until after the wizard closes, suppress its dialogs while onboarding runs, and enable newly downloaded sets silently instead. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-25 08:32:03 +02:00
BruebachL	bc26e19564	[DeckEditor] Highlight card name in VDE quick search after adding (#7350) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-25 08:31:38 +02:00
BruebachL	a9b38ec0e1	[UserList] Sort developers below judges (#7347) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-24 18:40:24 +02:00
BruebachL	712ca61a9e	[Themes] Encode home backgrounds as JPEG to slim the repo (#7346) ... The home background PNGs shipped since #7280 total ~67 MB and bloat every clone and pull for other devs. Encode them as JPEG at quality 95 (matching resolution, visually lossless at 40-48 dB PSNR since the art is noise-heavy renders PNG cannot compress) for ~79% size reduction. The scheme-variant and theme asset resolution already probe .jpg, so only the qrc entries needed updating. Refs #7327 Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-24 18:40:04 +02:00
BruebachL	eae9591a17	[Server/Client] Unify report categories (#7345) ... * [Server/Client] Unify report categories into one shared source of truth * Lont. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-24 18:37:54 +02:00
BruebachL	a1d8ce6165	[Accessibility] Bind card size sliders to Ctrl + Scroll (#7338) ... Ctrl+scroll over a card display now resizes the cards everywhere a card size slider is shown, matching the standard way of resizing content. CardSizeWidget learns to forward Ctrl+wheel events onto its slider via an event filter that is installed on the display container and, when present, on the scroll area's content widget so the resize intercepts the wheel event before the view scrolls. The existing slider valueChanged wiring then rescales the displayed cards. Applied to the visual deck editor (per card group, covering flat and overlapped layouts), visual database display, printing selector, sample hand, visual and public deck storage, Archidekt previews and EDHRec card displays. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-23 21:03:34 +02:00
BruebachL	71ef36374d	[DeckEditor] Show card counts in Visual Deck Editor banners (#7339) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-23 19:19:30 +02:00
BruebachL	7a41dfe175	[PictureLoader] Fix custom-folder pictures in subdirectories not loading (#7340) ... CardPictureLoaderLocal compared the full file name (including extension) of the custom-folder candidate against the extension-stripped base name of each directory entry, so a CUSTOM subfolder image like pics/CUSTOM/poker/1 of Hearts.png never matched and the card fell through to the network. Compare the complete base names instead. Add matcher tests covering CUSTOM subfolder resolution. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-21 21:48:34 +02:00
BruebachL	e97ef5a617	[Style] Do not offer broken windows11 (#7341) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-21 21:45:11 +02:00
BruebachL	fe83966087	[Theme] Revert default light palette and only include AppColors (#7342) ... * [Theme] Revert default light palette and only include AppColors * [Style] Also remove dark mode palette --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-21 21:45:05 +02:00
BruebachL	e01912d3c8	[Tests] Add manual picture loader benchmark against the real card hosts (#7288) ... * tests: add manual picture loader benchmark against the real card hosts (cherry picked from commit 588d71d3b9a8278ab98b847802908f322c1035b7) * tests: address review on picture loader benchmark - Sandbox via unique app/org names plus Linux-only XDG redirection, derive the warm-cache probe and data path from SettingsCache, and bail out when the temporary sandbox cannot be created. - Run each pass with a fresh worker and shut workers down before reading the 429 counters, so the redirect cache is persisted and no worker thread can outlive the stack-local counters or the installed message handler. - Make s_activeCounters atomic and always forward log output when the previous handler is the built-in (nullptr) one. - Validate --timeout-min, scale the cached-pass budget with --count, honour the first --url as the stress template, and add the missing trailing newlines. - Zero-initialise SettingsCache members so the benchmark mock cannot dereference an indeterminate pointer. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-21 18:30:04 +02:00
BruebachL	9f66a098ac	[PictureLoader] Fix worker thread shutdown and cross-thread cache clearing (#7292) ... * [PictureLoader] Fix worker thread shutdown and cross-thread cache clearing clearNetworkCache() ran directly on the UI thread while the worker thread owned the disk cache and redirect cache, racing cache reads/writes. Make it a worker-thread slot invoked via a blocking queued call when the thread is running, so the 'Cached card pictures have been reset.' message is truthful. The worker thread was also never quit()/wait()ed: both destructors only deleteLater'd their objects, so Qt warned 'QThread: Destroyed while thread is still running' and leaked a running loop at exit. Wire the worker's finished() signal to its own deleteLater() (canonical worker-object pattern), add shutdownThread() to stop the loop, and let CardPictureLoader destroy the QThread only after wait() has returned. * [PictureLoader] Guard cache teardown and stop blocking the UI thread --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-21 18:30:03 +02:00
BruebachL	14acf3bf64	[PictureLoader] Add user-configurable per-host request caps (#7287) ... * [PictureLoader] Add user-configurable per-host request caps Picture downloads were throttled to a uniform 10 requests/second per host with no way to tune a specific server. A rate-limited API host (Scryfall caps at 10 req/s) can trip 429s during bursts, and CDN hosts with no rate limit were throttled needlessly. Introduce developer-owned per-host caps that users can only ever lower, never raise, exposed in the download settings page: - DownloadSettings::DEVELOPER_HOST_CAPS sets the ceiling per host (api.scryfall.com 9, cards.scryfall.io unlimited, others 10). - A new hostRequestLimits setting stores user overrides in downloads.ini; clampHostRequestLimit() bounds them to [1, devCap] so a user can reduce api.scryfall.com to 5 but never raise it above 9. - The picture worker seeds, halves on 429, and recovers its sustained per-host allowance against the effective ceiling instead of the global maximum, and skips per-host accounting entirely for unlocked hosts (cards.scryfall.io) while global pacing and 429 backoff still apply. - The deck editor settings page gains one spinbox per known host, each clamped to its developer cap. * [PictureLoader] Let unlocked hosts skip dispatch pacing; adjust limits per URL Two refinements to the per-host request caps: - Unlocked hosts (UNLIMITED_HOST_QUOTA, e.g. cards.scryfall.io) no longer wait on the 100ms dispatch pacing or consume the global per-second quota. dispatchQueuedRequest fires their queued requests back-to-back, bounded only by their 429 backoff window and Qt's per-host connection pool, so an unthrottled CDN is not artificially slowed. - The deck editor download settings page replaces the static grid of one spinbox per known host with an "Adjust Rate Limit" toolbar action on the URL list. It picks the host out of the selected URL and clamps the entry against the developer cap table (including for user-added URLs). Also fixes a review finding: resetRequestQuota could write the UNLIMITED_HOST_QUOTA sentinel (-1) into the sustained per-host quota when a host became unlocked mid-run, permanently poisoning its allowance. Stale entries for unlocked hosts are now dropped, and the per-second seed is clamped against the effective ceiling so a lowered limit applies immediately. * [PictureLoader] Cap unlocked host bursts and adapt them to 429s * [PictureLoader] Store per-host limits readably and show them per URL * [PictureLoader] Make dispatch and rate-limit bookkeeping key on the real host Addresses ZeldaZach's round-4 review nits: - Dispatch now resolves the cached-redirect chain before the in-flight gate, so a redirect learned after a URL was queued can no longer bypass the MAX_IN_FLIGHT_PER_HOST cap and drain the whole queue onto the redirect target, which may carry its own developer cap. processSingleRequest does the same so the allowance math keys on the host that is actually hit. - The per-host in-flight slot is released when the reply is destroyed (with the worker as the connection context) rather than on a 'finished' connection bound to the work object, so an aborted reply or a work object deleted while a reply is pending can never permanently shrink the fast path's concurrency. - storeSettings only prunes limits for hosts with neither a URL nor a developer cap, so throttles on redirect targets (api.scryfall.com -> cards.scryfall.io) survive URL removal. - Unlocked hosts are offered 0..UNLOCKED_HOST_LIMIT_MAX (50) in the rate limit dialog, matching clampHostRequestLimit() and the documented hand-editable range, so values written into downloads.ini are no longer silently rewritten on the next edit. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-21 18:30:03 +02:00
BruebachL	3d5eb84d81	[PictureLoader] Seed per-host allowances on demand and skip hosts in 429 backoff (#7286) ... * [PictureLoader] Seed per-host allowances on demand and skip hosts in 429 backoff The quota reset re-filled every host's remaining allowance to a full MAX_REQUESTS_PER_SEC as soon as the queue had a request for it, so a server that was just rate limited could be hammered again at full speed immediately after (or even during) recovery. Only seed a host's allowance the first time it is dispatched in the current second, seeded from its reduced sustained quota, and skip hosts still inside their 429 backoff window entirely, handing the entry back to its worker so it can wait the backoff out or fall through to another source instead of parking in the queue with no reply pending. Deferrals wait on the host that is actually blocking the request (cached-redirect targets and the reply host of a 429) rather than the current card URL's host. Rebased onto network-requests/request-pacing, which absorbed the earlier pacing and dispatch-guard commits, and reuses its updateTimerState idle-429 recovery plumbing. * [PictureLoader] Hand backed-off entries back across the whole dispatch tick Review fixes on the 429-backoff skip: - Hand a backed-off queue entry back to its worker via scheduleDeferredRetry(host) instead of startNextPicDownload(), which was looping on the pre-redirect host every 100ms tick whenever the queued URL was a cached-redirect target whose redirect host was the one in backoff. - Keep scanning the queue after a hand-back (--i; continue) so a backed-off host at the head no longer consumes the entire dispatch tick, stalling every healthy host further down. - Emit imageRequestSucceeded(url) before makeRequest()'s cached-redirect backoff return so the status bar reclaims the deferred URL's widget instead of inflating the progress bar forever. - Only spend a per-second allowance when makeRequest() actually issues a request: it returns nullptr when the redirect target is backed off and the work is handed back, so the slot would otherwise be wasted on a no-op. - Gate the dispatch-time backoff skip on requestTouchesNetwork() so entries served straight from the disk cache (e.g. with downloads disabled) are not bounced into a 30-60s deferred retry for a host that 429'd earlier. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-21 18:30:03 +02:00
BruebachL	334a743953	[PictureLoader] Pace requests and run the throttle timers on the worker thread (#7285) ... * [PictureLoader] Pace requests and run the throttle timers on the worker thread Previously the whole backed-up queue was drained in a burst as soon as a request was enqueued, sending up to 10 requests back-to-back and then immediately re-filling the quota one second later. That hard-bursts a rate-limited API like Scryfall's (10 requests/second) into a 30 second lockout. Introduce a pacing timer that dispatches a single queue entry every 100 ms, so the per-second allowance is used smoothly instead of in spikes, and keep the quota timer at 1 second. Also fix both timers' thread affinity: they are QTimer value members and so are not QObject children, meaning moveToThread() on the worker left them on the main thread while the slot code started them from the picture thread, which was a no-op that also warned. They are moved to the worker thread explicitly and started lazily from there. * [PictureLoader] Guard dispatch timer restarts and drop dead request quota * [PictureLoader] Reconcile quota-timer lifecycle with idle 429 recovery --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-21 18:30:02 +02:00
BruebachL	7a2492ac67	[Chat] Render room chat history usernames as live user tags (#7269) ... * Render room chat history usernames as live user tags (#1595) Room chat history carries no user-level data, so history usernames were rendered as fixed, zero-level tags: the moderation context menu was missing the buddy/ignore and promote/demote entries and the stored name casing was never corrected. Resolve each history author against the online user list and, when found, build the user tag with the real user level and name so the entry behaves exactly like a live chat tag. Offline users keep the plain fallback. - chat_view: look up history authors via getOnlineUser for the real level/name * Fix offline history usernames getting a leading underscore The offline fallback used "_" as the level placeholder, producing an href of user://__NAME. The hover handler splits at the first underscore, so interactions targeted a nonexistent "_NAME" user. Use level 0 so offline history entries render as zero-level tags like before. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-21 09:13:55 +02:00
BruebachL	ef68a7bdcc	[Card] Add a setting for the language used in card search (#7314) ... * [Card] Add a setting for the language used in card search Localized card names and texts can now be searched too, controlled by a 'Language used in card search' toggle (English, selected card language, or both) on the general settings page. Untranslated cards always keep matching in English. Removed the redundant local copy of the URL templates list in the localized picture loader while here. * [Card] Bind search language per FilterString instance The peg parser rules are set up once per process, so the GenericQuery and OracleQuery rule actions could not capture per-instance state. Instead of storing the search language in a process-global that FilterString instance methods mutate, hand it to the rule actions through a thread-local parse context and copy it into the filter closures they produce. Card evaluation in FilterString::check no longer reads any process-global state, and each instance keeps the language it was built with; constructing one instance no longer changes what unrelated instances (deck filter, drop-to-hand, zone views) match against. The card database display model stores the raw query and rebuilds the FilterString when the search language changes, since the language is now bound at parse time. Add tests for the English/Selected/Both search modes, the English fallback for untranslated cards, and per-instance language independence. * [Card] Pass the card search language to deck and zone card searches Wire the two remaining FilterString consumers to the configured card search language so card-name matches respect it everywhere: - DeckFilterString now takes the search language and mode, exposes them to its [[card name]] rule action via a thread-local parse context (same pattern as FilterString), and the engine's card database uses them for content search. - ZoneViewZone reads the card language from CardsDisplaySettings when applying its search filter, and the reveal-zone widget re-applies the active search when the language setting changes. - The deck-storage search re-runs its filter against the current card language setting, including live re-application when the setting changes. Game-action targeting (DlgMoveTopCardsUntil) intentionally keeps evaluating against English card names. * [Card] Rename CardSearchLanguage to SearchLanguageMode * [Card] Restore displaced namespace doc in card_localization.h * [Filters] Pass CardSearchLanguage as a single struct * [CardSearchModel] Match English and localized names in Both mode Card names are stored in both English and localized forms, so search for matches in both during the 'Both' search mode instead of checking only the localized name. * [CreateTokenDialog] Fetch cardsDisplay settings inside the apply lambda Avoid capturing the raw settings pointer in the lambda: resolve the card language and card search language from the settings cache at call time so the values are always current when the search language is re-applied. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-21 08:50:48 +02:00
BruebachL	6823d54c1e	[DeckShare] Browse and open public decks with loading, error and accessibility states (#7245) ... * [DeckShare] Browse and open public decks with loading, error and accessibility states Add a public-decks tab that lists decks published by other users using the server's deck visibility feature, previewing each deck's banner card, color identity, tags and upload time without downloading the deck list until the user opens it. - Add a public-decks tab with a shared-settings widget and a remote model that fetches the target user's decks and refreshes both automatically and on user request, with a loading indicator and a server-error message instead of a blank tab when the fetch fails or the connection drops - Render each deck as a focusable preview tile whose banner, color identity, tags and upload time follow the existing Preview settings, with the deck name announced as the tile's accessible name and Space/Enter opening the deck, mirroring the shared-deck preview tile - Show a message box when opening a public deck fails or arrives corrupted - Publish and unpublish decks from the server storage toolbar and context menu, toggling the deck's own visibility bit (what the server persists) rather than the inherited effective state, and batch the visibility refresh until the last in-flight change is acknowledged - Add the Show Upload Time setting so the tile's upload stamp can be hidden like the other preview details - Update the retranslateUi wiring for the new public-decks tab and rename the share action tooltip from "Deck share" to "Share link" * [DeckShare] Adapt deck upload to the server-derived banner and tag protocol The server now derives the banner card and tags from the uploaded deck list itself, so Command_DeckUpload only carries the client-computed color identity. Drop the reserved banner/tag setters from the editor and storage uploads, send the color identity on remote saves, and read tags from the now-repeated ServerInfo_DeckStorage_TreeItem field. * [DeckStorage] Refresh the visibility column with a guarded timer instead of a latch counter A dropped visibility reply used to leave the pendingVisibilityChanges counter permanently positive, so the Public/Private column never refreshed again and nothing reset it on disconnect. A restartable single-shot timer with a boolean guard re-reads the tree whenever publishes quiet down and is stopped on disconnect, so a lost reply costs one stale refresh instead of killing the column for the session. * [DeckStorage] Summarize batch publish failures when the batch drains Each rejected node stacked its own modal dialog, so publishing a ten-deck selection against a rejecting server made the user dismiss ten dialogs one at a time. Failures are now collected while the batch is in flight and shown as a single summary when the visibility refresh timer fires; a reply that lands outside an active batch still reports right away. * [PublicDecks] Time out the loading state so a dropped reply cannot wedge the tab loading only cleared in decksReceived, but the ping sweep can drop a pending command without ever emitting finished, leaving the tab stuck on 'Loading public decks...' and the refresh button permanently inert. A single-shot timer started per refresh clears the latch and reports a timeout; the latch also clears when the client disconnects. * [PublicDecks] Escape remote-crafted text in tooltips and the tab title Deck names and usernames come from other users' records and Qt renders QLabel tooltips as AutoText, so a name like '<h1><table>...' parsed as markup. Escape and bound the deck-name tooltip and escape the username interpolated into the title label. * [DeckStorage] Distinguish an inherited public state in the visibility column The column reported the effective state while publishing toggles the node's own bit, so a private deck inside a public folder already read 'Public' and toggling appeared to do nothing (and toggling again silently unpublished it). The cell now shows 'Public (inherited)' for that case and the tooltip explains why. * [PublicDecks] Run retranslateUi at construction and name the refresh button retranslateUi was never called from the constructor, so the tooltips set there were absent until a language change. Call it before the first refresh, and give the icon-only refresh button an accessible name for screen readers. * [PublicDecks] Keep the empty and status variants correct across language changes retranslateUi unconditionally rewrote the empty label to the 'nothing published' variant, stomping the 'no decks match your filters' choice rebuildGrid had made, and a visible loading message stayed in the old language. Let retranslateUi pick the same variant rebuildGrid does and re-show the status so it retranslates. * [VDS] Share one color-identity match rule between the two deck grids The remote public decks model verbatim-copied updateColorMatches' switch, down to the ExactMatch normalization and the fact that Includes/Excludes do not normalize case. Extract colorIdentityMatches() next to the FilterMode enum and call it from both so the subtle rule cannot drift. * [DeckStorage] Drop the unused tree widget model accessor The accessor handed the model out past the wrapper methods that exist to keep it encapsulated, and nothing in the stack called it. * [DeckShare] End the public-decks files with a trailing newline keeps the final line's diff clean and stops clang-format CI from flagging the files. * [VDS] Reuse the shared quick settings widget for the public decks tab PublicDecksQuickSettingsWidget was VisualDeckStorageQuickSettingsWidget minus the folders, banner and tooltip controls, with identical wiring for the shared keys and a version of the near-identical file to keep in sync by hand. Fold the Show Upload Time checkbox into the shared widget, give it a setPublicDecksMode() that hides the controls that do not apply, and delete the duplicate. * [PublicDecks] Drop stale deck-list replies after the loading timeout A reply that lands after its own loading timeout (the reverse of the ping sweep dropping the command) could stop the newer request's timeout timer and repaint the grid with out-of-date data. Each refresh now captures a monotonically increasing request id, and only the newest request's reply updates the grid. * [PublicDecks] Re-show a displayed failure message on language changes The status label carries both the loading and the failure message, and retranslateUi hid it whenever the model was not loading, so a language change while a server-error or timeout message was on screen swapped it for the (empty) grid. The tab now keeps the last failure text and re-shows it when not loading, clearing it once a new refresh starts. * [DeckStorage] Keep the visibility refresh armed until replies land The single-shot drain was armed with the 500 ms delay at send time, so a round trip slower than that drained before the server applied the change, re-read the old state and never re-armed, leaving the column stale until a manual refresh. The timer is now armed with the full network timeout at send time (a lost reply still costs one stale refresh) and re-armed with the short delay every time a reply lands. * [DeckShare] Close public decks tabs when the client disconnects TabSupervisor::stop() built tabsToDelete from the room and game tabs only, so a public decks tab survived a disconnect, sitting with stale contents and a refresh button that kept hitting the dead client. Its values are now folded into the same cleanup. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-20 20:22:17 +02:00
BruebachL	8ca749c07d	[DeckShare] Open shared decks via links with a gated preview flow (#7244) ... * [DeckShare] Open shared decks via links with a gated preview flow - Serialized url-chain dispatcher in IntentUrlParser; queue-drained urlChainFinished(bool) drives the startup auto-connect fallback - Open-shared-deck intent with sequential download state machine, 15s per-item timeout, partial-success offer, livable Cancel via ApplicationModal dlg_login_prompt interactive fallback - Preview dialog: download progress label, share vocab sweep, palette-highlight selection frame, Space/Enter keyboard toggle, NoFocus checkbox, double-click tile opens immediately - Confirm-before-server-migration with one-shot restore to the previous server on failed/cancelled chains (statusChanged settle deferral), hostname-only identity comparisons - Skip credential link when already connected; arrow-key navigation in FlowWidget; card glows use palette highlight - Address code-review M1-M4 and UI/UX QA blockers 1-2 * [DeckShare] End the open-shared-deck files with a trailing newline * [DeckShare] Forward a dependency's cancellation as the owner's own * [DeckShare] Let intent chains opt into the link sign-in dialog * [DeckShare] Track link-intent chains per-run so each can restore its own session * [Settings] Match a server on the exact host and port when adding it * [DeckShare] Confirm the share link's target server before opening a deck * [DeckShare] Reformat the link sign-in intent constructor * [DeckShare] Time the share-list round trip and backstop silently-destroyed intent chains * [Client] Drain a single-instance payload before its handlers read the socket again * [Client] Treat a busy single-instance primary as alive instead of stealing its socket * [DeckShare] Keep arrow-key navigation between flow items inside a scroll area * [Client] Skip the startup connection when a macOS URL launch owns the connection * [Client] Redact share secrets from activation URL logs * [Client] Make the link-connection gates port-aware and keyboard-safe Second-pass review notes for the shared-deck link flow (Cockatrice#7244): - FlowWidget arrow-key navigation is opt-in via addNavigableWidget, so combo/spin controls on the analytics flows keep their own arrow keys - isConnectedTo and the open-deck/join-game preconditions compare the configured server port alongside the host, so a same-host/different-port link cannot resolve its share token or game id on the wrong instance - the link sign-in dialog reuses an existing server entry's saved name instead of renaming it to the raw hostname - skipStartupAutoConnect is cleared once the launch chain connects, so a later mid-session declined link cannot fire the startup fallback - the plain-launch path of SingleInstanceManager no longer blocks on the primary's ACK - link- and server-supplied text is html-escaped in the confirm prompts and shared-deck preview so markup cannot spoof the shown messages --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-20 20:22:17 +02:00
BruebachL	ba2900dcb9	[DeckShare] Create temporary share links for local and server decks (#7243) ... * [DeckShare] Create temporary share links for local and server decks * [DeckShare] Address review findings and harden the share flows Gate every share entry point on login, de-duplicate the share-link and color-identity logic behind DeckShareUtils and an injected querier, and replace the silent tray/status-bar notices with always-visible dialogs. - abstract_tab_deck_editor: explain that sharing requires a connection instead of silently doing nothing when logged out - tab_deck_storage: disable the share action on disconnect, reject folder/deck mixes and the root folder with clear warnings, re-enable Create on every entry/response so a dropped connection cannot leave the button disabled - tab_deck_storage_visual: same login gate for the context-menu entry, visible success/error dialogs, and a symmetric in-flight guard - getDeckColorIdentity now takes a CardDatabaseQuerier, dropping the CardDatabaseManager singleton access and enabling unit tests * [DeckShare] Fix share-link expiry build on the minimum-supported Qt QTimeZone::UTC (the Initialization enum) only exists since Qt 6.7, so Debian 12 and Ubuntu 24.04 (Qt 6.4) fail to compile the share-link expiry handling in the share dialog and the two deck-storage tabs. Mirror the existing games_model guard and fall back to Qt::UTC on older Qt. * [DeckShare] Use the stable server client for the visual deck storage tab * [DeckShare] Extract the share-creation response handling into DeckShareUtils * [DeckShare] Drop includes left unused by the share-response extraction * [DeckShare] Format share expiry with the locale-aware short format * [DeckShare] Build share links with QUrl and QUrlQuery for percent-encoding * [DeckShare] Replace the duplicate computeColorIdentity with the shared getDeckColorIdentity * [DeckShare] Recover the share controls when the server never answers * [DeckShare] Provide the full share hint in each plural form * [DeckShare] Join the selected-count label with a non-translatable separator * [DeckShare] Retranslate the share button tooltip with the storage widget * [DeckShare] Forward retranslateUi to the visual deck storage widget * [DeckShare] Let the share bar owners supply the hint text * [DeckShare] End the share-related headers and sources with a trailing newline * [DeckShare] Keep the settings include in the project include block * [DeckShare] Include the network settings header used by the share timeout * [DeckShare] Resolve the share theme icon through themePixmap QPixmap("theme:icons/share") has no file extension, so ThemeManager::assetPath() is bypassed and the pixmap is always null. Use themePixmap(QStringLiteral("icons/share")) like every other toolbar action, so the .svg (and dark/light variants) resolves. * [DeckShare] Keep the share selection consistent with the visible decks Filtered-out previews are hidden but kept alive, so selectedFilePaths() counted them in the share and the selection highlight. Only decks the user can see are now shared, and a deck that stops matching the filters is deselectd as the deck pass runs, keeping the %n count and the highlight in sync with the screen. * [DeckShare] Abandon an in-flight tree share on cancel Leaving share mode never stopped the timeout timer, and a late response still ran shareFromTreeFinished, copying the link and announcing success for a share the user backed out of. Stopping the timer and tracking the outstanding request by sequence number means a stale reply (or a timed-out one) after cancel is ignored, and cancelling + re-entering share mode can no longer confuse the two requests. * [DeckShare] Abandon an in-flight tile share on cancel exitShareMode() left shareTimeoutTimer running and did not abandon the pending Command_DeckShareCreate, so a timer pop or a late success still reported the share after the user cancelled. Stop the timer and ignore stale responses via a sequence number, mirroring the tree tab. * [DeckShare] Wire the status-changed handler after shareBar exists handleConnectionChanged() dereferences shareBar->isVisible(), but the connection was set up before shareBar was constructed and shareBar had no in-class initializer. On any status change delivered before construction the slot read an indeterminate pointer. Seed the connection (and the initial share availability) after shareBar exists and give shareBar a = nullptr initializer. * [DeckShare] Explain why a blank deck cannot be shared A blank deck exited the share flow silently. The menu only disables the entry via setSaveStatus(), a different predicate, so the path is reachable (e.g. add a card and remove it again). Mirror the not-logged-in branch with a short information dialog. * [DeckShare] Restore the banner-text doc comment Re-add the doc block above refreshBannerCardText() that was removed as part of the share-selection work; it documents the coupling to refreshBannerCardToolTip. * [DeckShare] Resolve the stable server client in the deck editor gate actShareDeck went through tabSupervisor->getClient(), which hands back a LocalClient while an offline game is running. LocalClient never sets its status, so a logged-in user could not share from the deck editor during a local game, and got a misleading "You must be connected" message. Expose the supervisor's stable remote client and use it for the gate and the dialog, matching the other share tabs. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-20 20:22:16 +02:00
BruebachL	a289d61765	[VDS] Decouple tag filter and fix reordered-chips crash (#7242) ... * [VDS] Decouple tag filter and fix reordered-chips crash * [VDS] Address review: dead code, chip reparenting, filter signal and sort fast-path * [VDS] Address second round of review nits --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-20 20:22:16 +02:00
BruebachL	a5e94d8a4f	[Build] Keep Windows installs free of build-tree artifacts (#7316) ... * [Build] Keep Windows installs free of build-tree artifacts Several Windows packaging gaps could leak Visual Studio CMake build output into the installed application or the NSIS installer: - The per-app DLL sweep used ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/${CMAKE_BUILD_TYPE}, which is empty on multi-config generators, collapsing the recursive DIRECTORY install into the whole build tree (containing *.dir, *_autogen, .qt, .qsb, x64, ...). Point it at the real per-config output with $<TARGET_FILE_DIR:...> and exclude build artifacts. - install(FILES ${OPENSSL_INCLUDE_DIRS} ...) tried to install OpenSSL include directories as files. CMake refuses this ("install FILES given directory"); it only slipped through CI because the vcpkg OpenSSL config leaves the variable empty. Remove it; fixup_bundle already ships the OpenSSL runtime DLLs. - The NSIS uninstaller only deleted *.exe/*.dll and a few known files, so build-tree leftovers survived an uninstall/reinstall cycle. Wipe the whole directory tree instead. - Add a Windows CI gate that lists the packaged installer with 7-Zip and fails the build if any build-tree artifact path is found. * Update .ci/compile.sh Co-authored-by: tooomm <tooomm@users.noreply.github.com> * [Build] Rework Windows installer artifact exclusions per review --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> Co-authored-by: tooomm <tooomm@users.noreply.github.com>	2026-09-20 20:21:52 +02:00