6602 commits

This branch

This branch

All branches

Author	SHA1	Message	Date
sentry[bot]	0369aaffa9	[Installer] Name only the payload programs installed in the directory (#7684) ... The running-instance lookup named every process whose image path began with the install directory, subfolders included. Installing into a folder that holds another application in a subfolder therefore reported that application as holding the previous version, and nothing the guard offers could clear it: the close asks cockatrice.exe only, and the end answer refuses anything that is not a payload program. Both answers were no-ops, so the update could only abort and tell the user to close a program that was never touching the files. Name only the programs the payload installs, and only when the image lies in the install directory itself. A program below the directory that really does hold a payload file is still named by the Restart Manager lookup, which is the lookup for that question. The Windows probe check now covers a payload-named program in a subfolder and a program the payload does not install; both have to come back as nothing. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-30 13:16:50 +01:00
sentry[bot]	ded83aaab5	[Installer] Ask a hidden instance to close, then offer to end it once (#7683) ... * [Installer] Request quit for hidden Cockatrice instances and record holders in abort telemetry * [Installer] Fix compiler warnings in holder probe and installer quit handler * [VisualDeckStorage] Order the includes the way clang-format wants them * [Installer] Ask a hidden instance to close, then offer to end it once The previous attempt had the client answer a registered window message, which cannot work for the update it was reported on: the instance being replaced is a client from before that message exists, so it never answers, and the user is left with "Cockatrice is still running" exactly as reported. Every step that can free the files has to be the installer's, so the client side is reverted. The probe now posts WM_CLOSE to the instance's own window whether that window is on screen or only in the notification area: the request does not need a visible window, an instance in the tray is the one this update has no other way to reach, and the confirmation Cockatrice answers with is a window Qt shows itself, so it is painted - unlike one restored from outside. A visible window is brought forward first, and Qt's bookkeeping windows stay excluded, because a close to the notification-area one terminates the instance without its confirmation. A question of the instance's own no longer suppresses the close. Withholding it protected nobody: the installer asked the user to close a program it had never asked, and a current client refuses the close while its confirmation is pending anyway, leaving the question where it is. The launcher mode keeps the visible-only rule, because a launcher without a window has already answered its own close and the wait ends it after a grace instead of asking a user who is not there. The guard asked the same question until the user gave up: after the automatic close failed it offered to end the programs, then re-ran the probe and showed the same dialog word for word when the answer was No, which is how an abort was reported three times over. It now asks once - Yes ends the Cockatrice, Oracle and Servatrice programs the payload installs and says what that costs, No cancels - and what survives an end is named in the abort instead of being put to the user as the same question again. The uninstaller's guard and the recovery path after a failed purge follow the same shape. Kept: the abort telemetry that records the holders, the close result and the end result as event data. --------- Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com> Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-29 21:01:16 +01:00
sentry[bot]	9e50c5ad6e	[Installer] Recover payload files when previous uninstaller is missing or fails to launch (#7682) ... Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com>	2026-10-29 12:10:18 +01:00
sentry[bot]	06da7f815a	[Installer] End a launcher that has no window to ask instead of prompting (#7681) ... * [Installer] End a launcher that has no window to ask instead of prompting The in-app updater starts this installer detached and then closes itself. A client that accepts that close hides its window before it exits, and if it then hangs in shutdown there is nothing left to ask: the holder probe deliberately never posts WM_CLOSE to an instance it cannot see a window for, so the "ask it to close again" answer only repeated itself until the user picked Cancel and the installation aborted (Sentry COCKATRICE-DESKTOP-J). The launcher wait now asks the launcher itself before asking the user. A new probe mode, --close-launcher, answers whether that one process has a window a close could reach at all; it is limited to a pid that still runs from the directory being replaced, so a reused process id cannot be hit. When there is no window, the installer gives the shutdown it already started one full grace and then ends it through the handle it waited on, which is what the dialog's Yes answer did. The prompt remains for the case it exists for: a window is up, the instance may be waiting on its own question, and No now means only "keep waiting". The client is left alone: an already-installed version runs the installer during its own update, so a change there cannot reach the run that aborts. * [libcockatrice_rng] Include immintrin.h for AVX2 builds in SFMT --------- Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com>	2026-10-29 04:00:42 +01:00
BruebachL	5cf3358c99	[Client] Stop reporting local login aborts as missing client features (#7678) ... * [Client] Stop reporting local login aborts as missing client features Local aborts (unusable saved verifier, refused authentication downgrade, failed scrypt derivation) reuse RespClientUpdateRequired with an empty feature list, and the handler discarded the reason and advised a client update. Report the abort with the cause instead, and report a strict server's challenge_response_auth answer to a credential-less login as the missing password it is. * [Client] Keep the login abort messages in one named set The local login aborts each carried their wording inline at the emit site, so changing the tone or the remedy meant editing several functions. Collect them as named constants next to RemoteClient instead, translated at the use sites with the RemoteClient context so lupdate still picks them up. * [Client] Reword the login abort explanations to match what happens The first wording sent users to a server operator to 'migrate' an account, which operators cannot do, and told them to 'save' a login the client saves by itself. It also blamed the device for a derivation failure the server's parameters can cause, and the dialog led with a line that said nothing while the reason sat in the informative slot. Say what the client actually does in each case, give the step that works, and show the reason as the dialog's main text. The unreachable fallback sentence about the login not reaching the server is gone. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-28 09:48:37 +01:00
BruebachL	d89b502342	[Client] Report password-reset rate limits instead of a generic failure (#7677) ... * [Client] Report password-reset rate limits instead of a generic failure sigForgotPasswordError carried no response code, so rate limiting, a disabled reset feature, an unknown user and a bad token all produced "contact the server operator". Users who were only rate limited retried, which kept them rate limited. * [Client] Move the reset-password warning out of the response switch --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-28 09:19:42 +01:00
sentry[bot]	a83d82b5e8	[Installer] Recover leftover payload files when previous uninstaller leaves them behind (#7676) ... * [Installer] Recover leftover payload files when previous uninstaller leaves them behind * [Installer] Fix leftover recovery on the 64-bit auto-uninstall path - Record the 64-bit uninstaller exit code after ExecWait returns. It was captured before the call (always empty), so WaitForOldPayloadToDisappear always aborted before RemoveOldPayloadLeftovers could run. - Restore the pre-uninstall user-data backup before aborting out of the wait, including the guard's cancel and probe-failure prompts, so the only copy cannot be stranded in %TEMP% for the next run to wipe. - Report the leftover removal as done only when every payload executable is gone and RetryDllPurge reports no stale DLLs, instead of checking cockatrice.exe alone. - Return directly after a successful recovery instead of reusing the poll counter, so the log stops claiming the directory "was already clear" and the normal settle delay is kept. * [Installer] Delete the old uninstaller only after recovery succeeds RemoveOldPayloadLeftovers cleared the attribute and deleted $INSTDIR\uninstall.exe on every attempt, before the attempt was known to work. On failure the installer aborts telling the user to run it again, but UninstallString then points at a missing uninstaller: ExecWait leaves the exit code empty, WaitForOldPayloadToDisappear takes the "cancelled or failed" branch, and RemoveOldPayloadLeftovers is gated on exit code 0, so no later run can ever get past it - a half-removed old install the previous revision never produced. Delete it in rop_done instead, after $OldPayloadRemoved is set. WriteUninstaller recreates it on success anyway, and nothing in the success verdict depends on it being gone. --------- Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com> Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-28 09:08:54 +01:00
sentry[bot]	bf3562a85a	[Installer] Offer to end running Cockatrice immediately when auto-close fails (#7675) ... * [Installer] Offer to end running Cockatrice immediately when auto-close fails * [Installer] Keep the way out that costs nothing in the first prompt The prompt now offers to end Cockatrice, so what the deleted close-again text said has to survive in it: an instance the probe reports as waiting for an answer to a question of its own has been brought forward, and answering that window is the way out that ends nothing. Say so, and say where to go when Yes leaves it listed - Task Manager - so an end that cannot take effect does not loop on the same dialog. * [Installer] Bring the comments back in line with the code GuardCloseTried no longer counts closes, and the file header, CloseHolders and GuardAppsNotRunning still described an escalation after a second close, as did the probe's --end-holders note. * [Installer] End Oracle and Servatrice too when the guard's end answer is confirmed The guard's end answer only ever stopped cockatrice.exe, so an Oracle holding the card database or a Servatrice service kept the payload locked and the prompt came back with nothing left that could free it. The probe now ends every program the payload installs, services included: isPayloadProgram() - the list restart_manager_holders already used for its parent walk - decides the set, so a scanner holding a DLL or a program the user keeps in the install directory is still named and left to the user. A service is terminated rather than sent a stop through the SCM, which notices the process exit and marks it stopped afterwards. The guard prompt now says what ending costs: an unsaved deck, an unwritten setting, players on a running Servatrice, and an Oracle mid-way through the card database. It also keeps the notification-area hint, which the probe's "has no window to ask" answer depends on, and drops the claim that Oracle and Servatrice had been brought forward. * [Installer] Give every running-program prompt the same end-or-retry answers Four prompts meet a running program and each offered a different set: the guard ended it, the launcher wait only kept waiting, and the recovery prompt after a failed purge could only ask Cockatrice to close. With Oracle or Servatrice holding the files, that last one had no answer that could free anything - Yes and No both retried the purge and Cancel aborted an install whose old version was already gone. All four now follow the guard's shape: one automatic attempt, then a prompt whose Yes ends the programs it names and whose No retries, with Cancel to give up. - RecoverFromHolders closes once and purges before it asks anything, then offers EndHolders - the same end answer as the guard - instead of a close the user had to trigger himself. - WaitForLauncherToExit can end the launcher by pid, the one process it holds a pid for, rather than looping 60 s waits with Cancel as the only way out. - END_COST and AFTER_CLOSING_GUIDANCE are !defines, so the cost of ending and the guidance for the answers that end nothing read the same in every prompt. * [Installer] End the launcher through the handle it was waited on The "end it" answer used to look the launcher's pid up again after a dialog that can stay open as long as the user takes, so a pid that had been reused in the meantime - or a /PID= that was never the launcher - would be terminated instead. The handle the wait already holds names that process and cannot be handed to another, so the terminate answer now acts on it. PROCESS_TERMINATE is requested along with PROCESS_SYNCHRONIZE; when only the synchronize right is granted the launcher is still waited on and the answer falls back to asking it to close, instead of reporting a live launcher as gone and writing the payload over it. * [Installer] Say what the end answer ends, and report a failed end The prompt offered "end every program listed above", but EndHolders only ends the programs the payload installs: a scanner holding a payload DLL, or a service the Restart Manager names by its long name, was listed and then silently left alone, so Yes looked like it did nothing and the same dialog came back unchanged. The wording now says which programs Yes ends and that anything else has to be closed by hand, and the round after an end that did not free the files says so, so the same dialog no longer reappears word for word. The three prompts are one macro so they cannot drift apart again. The added sentences eat into the 1024 characters one NSIS string holds, so the holder line's budget drops from 400 to 320 characters to keep the longest message (cancel line included) inside it: 977 characters, measured against the worst case the probe can produce. --------- Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com> Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-27 19:09:21 +01:00
sentry[bot]	30d3afc7b4	[Updater] Close a running Cockatrice when the installer asks (#7672) ... * [Updater] Refuse a close that arrives while one is already asking MainWindow::closeEvent() had a re-entrancy guard that returned without touching the event. A QCloseEvent is accepted unless something ignores it, so a second close request - an installer replacing our files posts one every 30 s - accepted the close while the first one was still blocked on its own confirmation. close() then hid this window, hid the confirmation dialog with it, and left the process running inside the nested loop that dialog had opened: no window, no way to answer it, and every later close request accepted into the same dead end. That is what an update looked like when it could not close a running client, and it is why the installer could name the process, bring it forward and ask it to close for minutes without anything happening. Track how far the shutdown has got instead. While a confirmation is up, a close from anywhere else is refused, which leaves the question in front of the user; once the shutdown has been accepted, a duplicate close (Qt delivers the event twice on some platforms) is accepted again so it cannot cancel the quit. * [Installer] Leave an instance that is asking a question alone An instance that answered a close by hiding rather than quitting - the state the client used to get into - has no visible window left, and the confirmation it is waiting on is hidden with the window that owns it. The close probe only ever looked at visible windows, so it restored the main window, posted WM_CLOSE into the same dead end, and reported "still running" without saying why. Show every window the instance left hidden before choosing the one to close, and skip the close when the window it would reach is disabled: a modal dialog of the instance's own is up, and posting the close then only hides the question again. Name what is left of the window in the report as well - gone, still open, hidden again, and whether the thread answers at all - so the log tells a refused close from one that was never delivered. * [Installer] Offer to end a Cockatrice that will not close A close is a request, and an instance that does not answer it - waiting on a question nobody is looking at, or stuck where a close cannot reach - leaves the user with no way forward: the files cannot be replaced while it runs, and clicking Yes again only repeats the request. After a second close that changed nothing, offer the only thing that can free the files: end it. The prompt says what it costs - an unsaved deck, a running game or a setting it has not written yet - the default button is the safe one, and it is a separate answer rather than a relabelled "Yes", so it is not reached by clicking through the close prompt. Only cockatrice.exe is ever ended: Oracle writes the card database in place and Servatrice is a service, so both are named and left to the user, exactly as they are for the close. The probe gains an --end-holders mode for it, and the uninstaller's guard gets the same offer. * [Installer] Never name the process that is doing the asking The old uninstaller is started with _?= so it runs in place, from the very directory it is emptying, and an installer can be downloaded into the folder it installs to. Both are then processes running from the directory, so the probe named them - and the guard asked the user to close the program that was asking. That question cannot be answered: a close and the end offer both skip anything that is not cockatrice.exe, so the guard repeated itself until the update or the uninstall was cancelled. The probe and the processes that started it are now never named, by either lookup. The walk up the parent chain stops at the first program the payload installs, because in an in-app update the installer's own parent is the Cockatrice instance being replaced - the one the guard exists to name. The Windows check held its locked file in the script that starts the probe, so that holder is excluded by design now and the check read it as "nothing found". It holds the file from a process of its own instead, and asserts the exclusion itself: a file that only the asking process holds comes back as nothing. * [Installer] Restore only the windows a user could answer Showing every hidden window an instance owns put Qt's own bookkeeping windows on screen: the tray icon keeps one to receive the TaskbarCreated message, styled WS_OVERLAPPED and titled QTrayIconMessageWindow. Restored, it is an empty white rectangle in the taskbar with no title bar to close it - it has no window procedure that would hide it again, and the process is busy in the very loop the restore is meant to break, so it stayed until the process was force-ended. Only framed, non-empty windows are restored and considered for the close now. That also keeps the close off Qt's tray window, which answers WM_CLOSE by asking the application to terminate - a quit that skips the confirmation the close is supposed to reach. * [Installer] Uninstall the previous version before installing over it A regular install over an existing one asked "a previous version must be uninstalled", then ran the old uninstaller interactively. That uninstaller put its confirmation page behind the installer window, where it could not be answered, and it only removed the installation after the installer had already written the new one over it. Run the previous version's uninstaller silently instead. The installer has already asked the question the user answered, so a second, hidden confirmation is pure risk: with /S there is no window to be behind, and ExecWait is then waiting for something that cannot outlive it. QuietUninstallString is used when the old installation has one, otherwise /S is appended. ExecWait only waits while the uninstaller runs in place: one that did not get a usable _?= copies itself to %TEMP% and returns at once, while the copy keeps deleting - which is how an uninstall can finish after the install it was supposed to precede. Log the exit code and wait for the old cockatrice.exe to disappear before restoring user data or writing anything, and abort with a message instead of installing into a directory that is still being emptied. Cancelling the "must be uninstalled" prompt now quits the installer; it used to skip the page and install over the old version anyway. * [Installer] Do not show windows the application has hidden The guard restored every hidden window an instance owned before asking it to close, so that a confirmation hidden together with its window would come back. Windows does as it is told and shows them; the application does not, because Qt still believes they are hidden and never paints them. What the user gets is a blank white rectangle, on screen and on the taskbar, that answers nothing - and the question it was meant to bring back is not answerable in it either. An instance with no window a user could answer is named and left to the user instead, and the prompt says what to do about it: open it from the notification area. Ending it stays the last resort, offered once a close has been asked for twice. The abort after a failed uninstall now says what it saw - the old version is still there after its uninstaller ran - instead of promising a removal that is not happening, which is the state a cancelled old uninstaller leaves behind. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-27 11:19:49 +01:00
sentry[bot]	92d082e735	[VisualDeckStorage] Guard against widget deletion during tag edit dialog (#7674) ... Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com>	2026-10-27 09:14:52 +01:00
sentry[bot]	aa42cfbccc	[VisualDeckEditor] Create menus before central frame to prevent Windows crash in setParent_sys (#7673) ... Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com>	2026-10-27 09:12:37 +01:00
sentry[bot]	d42b9607c5	[Installer] Check for running instances with the Restart Manager probe (#7671) ... * [Installer] Check for running instances with the Restart Manager probe The running-instance guard was built on powershell.exe started from the 32-bit NSIS stub, which is the 32-bit Windows PowerShell: its Get-Process cannot read the path of a 64-bit process, so a running 64-bit Cockatrice was reported as not running, the update went ahead, and the purge then failed on the DLLs that instance still had mapped. Use tools/installer-holder-probe instead - the same Restart Manager lookup the abort path already had - before anything is replaced and in the uninstaller as well. It is bitness-agnostic, so the Sysnative and WMI fallbacks are gone. The guard looks only at the payload, asks Cockatrice to close once by itself (WM_CLOSE, the same request as the window's close button) and then offers close again / retry after closing by hand / cancel for whatever the close cannot reach - Oracle writes the card database in place, Servatrice has no window - without force-stopping anything. A failed purge offers the same close-and-retry, tries one last purge when nobody is named, and every dialog shows in every mode: a silent reinstall stays silent until something goes wrong. * [Installer] Name processes running from the install directory The running-instance guard answers from the Restart Manager, which reports who has a file open. A running instance does not have to have one open: its own image and the modules it loaded are mapped into it rather than opened by it, and a mapping is not reliably reported. That is the state the probe was added for - every DLL still on disk, an instance running from the directory, and "nothing is holding them any more" as the answer. A guard built on file handles alone is blind to exactly that instance, so it passes, the purge fails, the holder lookup after it names nobody either, and the install ends in a warning that no running program was found: no close is attempted and no retry is offered. A manual install with another instance open never asks it to close at all. Name every process whose image lives in the directory being replaced as well, with the same bitness-safe QueryFullProcessImageNameW the probe already uses to label its holders. The image lookup is listed first - it is the instance the user is asked to close - and the Restart Manager entries follow with the pids already named dropped, so the close still reaches everything that holds a file. A Restart Manager call that fails only makes the probe unable to answer when the image lookup named nobody either. Ask instead of dead-ending when the probe cannot answer at all: Retry checks again, Ignore carries on, Abort cancels, in the installer and the uninstaller alike, and the abort dialog says the check failed instead of claiming no running program was found. The Windows CI probe check covers the new lookup with a process started from the directory that holds no file open. * [Installer] Offer to wait again when the launcher has not exited A client that is still closing after 60 s is usually not hung: it is showing its own "quit anyway?" confirmation for an unsaved deck, an open game or a card database update, and the user is the one who has to answer it. The installer answered that with a plain warning and stopped, so the only way forward was to close Cockatrice by hand and run the update again - which on an in-app update means finding the downloaded installer. Ask instead: Retry brings the client to the front, asks it to close again (WM_CLOSE, nothing forced) and keeps waiting on its handle, Cancel stops as before. There is no "install anyway" answer because a live process has its image and Qt DLLs mapped, so writing the payload over them is the broken install this wait exists to prevent - and the guard that runs right after names the same process by its image path and stops there, so ignoring would only move the same question a moment later. * [Installer] Ask about a launcher that has not exited after ten seconds A minute of silence is a long time to sit in front of an installer that has no window yet, and the case it is waiting for is not a slow flush: a client that started an update exits in well under a second, so a minute means it is almost always showing its own "quit anyway?" confirmation with nobody looking at it. Wait ten seconds for the first question instead, and the full grace for the ones after it. The normal path is untouched - WaitForSingleObject returns as soon as the handle signals, so a client that exits in 200 ms still costs 200 ms - and answering the first question once does not turn into a stream of prompts while the user is in Cockatrice's window. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-26 19:25:10 +01:00
sentry[bot]	4793c1fc04	[Installer] Offer to close blocking processes and retry purge on upgrade (#7664) ... * [Installer] Offer to close blocking processes and retry purge on upgrade * [Installer] Retry the purge after closing only our own applications The first version of this fix closed every Restart Manager holder through RmShutdown, forcing what refused to close. RmShutdown also stops services, and the only API that brings them back is RmRestart, which would restart Cockatrice itself into the files that are about to be replaced; the forced pass also contradicts the running-instance guard, which never force-closes Cockatrice because it may be holding an unsaved deck. The guard ran first anyway, so for the case the fix exists for - a Cockatrice restarted during the update - the probe never got to run: the guard waited 60 s and aborted. Keep the prompt and the retry, but ask only Cockatrice, Oracle and Servatrice to close, with WM_CLOSE and nothing else, and leave every other holder named by the probe to the user. Repeat the prompt while the user keeps saying Yes, refresh the names after a failed retry, and keep the /R abort message pointing at the downloaded installer. Also clear the read-only attribute before every purge attempt. Delete and RMDir leave such a file in place, so the retry could never make progress, and the diagnostics branch that named the cause had no action behind it. The holder probe goes back to being diagnostic-only, so its Restart Manager code and the --close argument are dropped. * [Installer] Make an accepted close of the running client actually land WM_CLOSE is a request, and Cockatrice answers it: while a card database update is running, a game is open or a deck is unsaved it puts up its own "are you sure?" confirmation. That dialog opened behind the installer's prompt, so the user saw nothing happen; the next Yes was posted to a main window the app's own modal dialog had disabled, so it was ignored as well, and the identical yes/no prompt came back with no hint of why. - Activate the matching window before posting WM_CLOSE, so the app's own confirmation is in front where it can be answered. - Wait up to 30 s for cockatrice.exe to exit instead of purging after a fixed 2 s, so the purge measures the app gone rather than its dialog. - On the second and later rounds ask with Retry/Cancel and say what is needed, instead of repeating the same offer that just did nothing. - Stop promising Oracle and Servatrice: they are console applications, so CloseMainWindow() has nothing to send WM_CLOSE to. They are not force-stopped either - Oracle writes the card database in place. * [Installer] Close the client from the holder probe, not PowerShell Accepting the offer to close did nothing on a real machine: the instance stayed up, never came forward, and the same prompt came back. The prompt names its holders from the Restart Manager probe, but the close itself went through powershell.exe: Get-Process -Name, a strict "path starts with $INSTDIR" filter, then Process.CloseMainWindow() and a WScript.Shell AppActivate. Every step of that is silent. A process whose path cannot be read is filtered out, a process .NET finds no main window for makes CloseMainWindow() return false without posting anything, and AppActivate is refused for a background caller - so an attempted close, a missed close and a close request the client ignored all looked the same in the log: nothing. Ask the probe instead. installer-holder-probe --close-holders closes exactly the processes Restart Manager just named as holding a file of the version being replaced, so no executable path has to be guessed and a third party application that merely shares the image name is never in the set. Per instance it restores and brings the top-level window forward (AttachThreadInput, so the client's own "really quit?" confirmation is on screen rather than behind the installer), posts WM_CLOSE to the main window, and waits up to 30 s for the process to exit. Its report names every holder with what happened to it, including the ones left to the user, and goes into the installer log as one line. That also removes the separate PowerShell polling loop that used to wait for the close: the probe holds a handle on the process it asked, so the purge that follows measures a settled directory. Oracle and Servatrice are still never closed, and nothing is force-stopped. --------- Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com> Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-26 09:45:52 +01:00
BruebachL	675e1f7447	[Translation] Name languages without reading the translation files (#7662) ... * [Translation] Name languages without reading the translation files The language selector derived every label from the language's own .qm file: it loaded the file and translated the literal "English" in the "i18n" context. lupdate could never extract that literal from the callers, which pass the DEFAULT_LANG_NAME macro, so a dummy QT_TRANSLATE_NOOP in cache_settings.cpp existed to give it something to extract. #7050 deleted that dummy, the next lupdate push dropped the string from Transifex, and the following pull rewrote the .ts files without it. lrelease only emits translated messages, so 22 of 25 .qm files returned an empty name. The names are data, not translations: a selector has to be readable before any translation is loaded, and a label must not depend on the translation of the language it names. TranslationLoader::languageDisplayName now holds them next to availableLanguages, falling back to Qt's native language name and then to the code, so a language added later still gets a label. Also select the configured language by item data instead of by matching the translated "English" string, which pointed at the wrong row whenever the lookup came back empty, and restore that English entry's own name: en_US is "English (United States)" so it can no longer be confused with the untranslated default. Oracle shared the code path, so its wizard language page is fixed too. * [Translation] Name the default language selection once Review follow-up: log the unresolvable language through the page's own category, and move the en_US entry both selectors fall back to for an unset setting into TranslationLoader::defaultLanguageCode. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-26 02:39:26 +01:00
sentry[bot]	858b270fa9	[Installer] Retry locked DLL purge before aborting upgrade (#7657) ... * [Installer] Retry locked DLL purge before aborting upgrade * [Installer] Keep the purge verdict when the portable purge runs $IsUpgrade was read from IsExistingCockatriceInstall after the portable purge, which deletes the very markers that function looks for. A purge whose only survivors were non-Qt DLLs (abseil_dll.dll, libssl-3-x64.dll) or content under Plugins therefore reported "not an install", $IsUpgrade became 0 and the abort was skipped - the mixed DLL set the retry exists to prevent. Measure the directory before either purge block, where it still describes the directory as it arrived, and let RetryDllPurge's own verdict decide the abort. A portable install into a directory without portable.dat is neither purge block, so $StaleRuntimeAfterPurge was never assigned there and the abort could not fire either. Track whether a purge ran and measure that directory with HasStaleRuntimeFiles instead, which is what every upgrade did before the retry. --------- Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com> Co-authored-by: BruebachL <44814898+BruebachL@users.noreply.github.com> Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-25 20:40:57 +01:00
ebbit1q	4d8ab54313	Revert "[Version] Bump to 3.2.0 for beta release. (#7653)" (#7660) ... This reverts commit e6b4c51585.	2026-10-25 20:19:23 +01:00
BruebachL	e6b4c51585	[Version] Bump to 3.2.0 for beta release. (#7653) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-25 20:14:21 +01:00
sentry[bot]	4ab875be48	[VisualDeckStorage] Keep databaseLoadIndicator alive when swapping in folder widget (#7651) ... Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com>	2026-10-25 17:35:46 +01:00
sentry[bot]	f3fb3331d1	[Cards] Guard against null QScreen when positioning enlarged pixmap (#7650) ... Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com>	2026-10-25 17:35:10 +01:00
BruebachL	93309e01a2	[Server/Client/Protocol] Let moderators clear banner cards (#7546) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-25 13:19:20 +01:00
BruebachL	b4e24f958a	[Network] Do not format discarded debug messages, parse without memmove (#7606) ... * [Network] Do not format discarded debug messages, parse without memmove Protocol debug logging gets the full protobuf message rendered through TextFormat and reflection. On the server this happened before every session/room/game/moderator/admin/developer command, even though the default Server_ProtocolHandler::logDebugMessage() is a no-op and even when servatrice has no log file configured. On the clients the same string was built as an argument to qCDebug(), which evaluates its arguments regardless of whether the category is enabled. Server_ProtocolHandler now exposes a virtual logsDebugMessages() hook. The base implementation returns false, so handlers without a log destination spend nothing. AbstractServerSocketInterface overrides it with ServerLogger::isLoggingEnabled(), which checks the log file and the server/writelog setting that logMessage() itself consults. All debug message call sites check the hook before formatting. The client send/receive paths check QLoggingCategory::isDebugEnabled() first, the same pattern AbstractClient already uses for its slow-command warning. ISL message decoding does the same. Byte stream decoding called inputBuffer.remove(0, n) for the length header and then again for every message. Each call shifts all remaining bytes, so a read carrying k messages cost O(k * bytes remaining). The three receive loops (remote client, TCP server interface, ISL interface) now parse at a read offset and compact the consumed prefix once, when they stop because a full message is not buffered yet. No buffering or size-limit behavior changes. Also in this batch: - Server_ProtocolHandler::processGameCommandContainer() copied the whole games map under gameListMutex just to look up one game id. A new getGameInfo() performs the single lookup and returns the room/player ids. - The TCP receive loop's `ok` flag is initialized, so a ParseFromArray exception can no longer leave it indeterminate before `if (ok)`. - The exception log prints only the message being parsed instead of hex-dumping the entire pending buffer. Full build and ctest (628 tests) pass, including the server tests. * [Network] Address review comments - Move getGameInfo() out of the header into server_abstractuserinterface.cpp. - Drop the consumed prefix once after the parse loop instead of on every early return: the two "not enough data" exits now break out of the loop. Applied to the remote client, TCP server and ISL receive loops. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-25 12:49:24 +01:00
Neel Jogani	a4c4c67674	Fix tab retranslation (#7580) ... * Retranslate all open tabs on language change * Address review: deeper Visual DB retranslation, index-based Archidekt size logic In Qt 6, setItemText() on the current item emits currentTextChanged but not currentIndexChanged, so connecting the Archidekt combos to currentTextChanged fired a search on every language change. Both combos now use currentIndexChanged, and buildSearchUrl() compares the size-logic combo by index so translated labels cannot drop sizeLogic. * Derive display mode button text in one place	2026-10-25 12:30:17 +01:00
BruebachL	f33cfcb288	[UI] Show 'Password saved' placeholder when verifier is stored (#7649) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-25 10:34:56 +01:00
BruebachL	5fbc636f93	[Models] Use QCollator for natural, cached sorting (#7633) ... * [Models] Use QCollator for natural, cached sorting QString::localeAwareCompare rebuilds its collation data for every comparison, and it compares numbers as text. Keep one QCollator per model, enable numeric mode, and use it for the card name, set and user name sorts. Add a test that sorts "Forest 1", "Forest 2", "Forest 10" through DeckListSortFilterProxyModel and checks the natural order. * [Models] Sort naturally under the C locale too Numeric mode only takes effect through a real locale: under the C locale, which headless CI processes run in, QCollator silently falls back to string order and sorts "Forest 10" before "Forest 2". That made collator_sort_test fail on every Linux leg while passing locally. Add makeNaturalSortCollator(), which enables numeric mode and pins English only when the process locale is the degenerate one, and build the models' collators through it. Users with a real locale keep their own collation. * [Models] Fix compile of natural sorting (QStringView mismatch) * nit: initialize collators in member initializer lists * nit: namespace makeNaturalSortCollator under NaturalSort --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-25 01:01:04 +02:00
BruebachL	5ddfcd7ead	[Pictures] Watch the custom picture folder instead of polling (#7635) ... The local picture loader walked the whole custom picture tree every ten seconds on the GUI thread. Watch the root and every subdirectory with QFileSystemWatcher, debounce bursts of events into a single 500 ms refresh, and keep a five-minute fallback poll for filesystems where watching fails. Add an explicit-path constructor and indexedPaths() so the indexing can be tested without SettingsCache, and emit indexChanged() after a rebuild. Verification: cockatrice built. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-25 00:36:14 +02:00
BruebachL	518c7e6125	[Network] Add a configured QNetworkAccessManager factory (#7628) ... * [Network] Add a configured QNetworkAccessManager factory Every HTTP caller built its own QNetworkAccessManager, so the system proxy and a transfer timeout were easy to omit. Add NetworkManagerFactory::create() as the single place that applies the process-wide proxy configuration and a transfer timeout, and use it at all 14 construction sites. Per-site settings such as disk caches and manual redirect policies stay with their callers. * [Tests] Link libcockatrice_network into the tests that compile its headers release_channel.cpp and card_picture_loader_worker.cpp now include libcockatrice/network/network_manager.h, but the test targets that compile them only linked libcockatrice_settings, so the include directory never reached them and every platform failed with "file not found": loader_local_matching_test and picture_loader_benchmark_test. libcockatrice_network is an INTERFACE library, so linking it costs these targets nothing but hands over the include path. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-25 00:33:43 +02:00
BruebachL	3b07b10b19	[Avatar] Respect EXIF orientation and decode at target size (#7634) ... Read the avatar with setAutoTransform(true) so camera orientation is applied, and ask QImageReader for a scaled decode at MAX_AVATAR_DIMENSION instead of decoding a full-resolution photo and scaling it afterwards. Verification: cockatrice built. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-25 00:11:09 +02:00
BruebachL	66b9338c90	[QtUtils] Use makeScrollArea() for the remaining scroll areas (#7625) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-24 22:50:25 +02:00
BruebachL	77a3875c25	[VisualDatabase] Debounce filter refreshes with QtUtils::Debouncer (#7624) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-24 22:48:11 +02:00
BruebachL	28c8474f03	[QtUtils] Add Debouncer helper (#7623) ... * [QtUtils] Add Debouncer helper * Extract long message to variable before confirm() call --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-24 22:43:10 +02:00
BruebachL	11e2bb0d2f	[QtUtils] Adopt window fade, path, bold font and confirmation helpers (#7622) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-24 21:43:50 +02:00
BruebachL	eac149656b	[QtUtils] Add window fade, path opening, bold font and confirmation helpers (#7621) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-24 21:38:54 +02:00
BruebachL	e806085681	[QtUtils] Use makeLayout() for zero-margin layouts (#7620) ... * [QtUtils] Add parent lookup, layout clearing, scroll area and monospace helpers * [QtUtils] Adopt shared Qt helpers across the client * [QtUtils] Add layout creation and relayout helpers * Swap parameter order (spacing before margin) and remove static from relayout() * [QtUtils] Use relayout() after rebuilding dynamic layouts * [QtUtils] Use makeLayout() for zero-margin layouts * Consolidate setSpacing() calls into makeLayout() using new parameter order --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-24 19:06:18 +02:00
BruebachL	06b9ca368f	[QtUtils] Add layout creation and relayout helpers (#7619) ... * [QtUtils] Add layout creation and relayout helpers * Swap parameter order (spacing before margin) and remove static from relayout() --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-24 19:01:56 +02:00
BruebachL	67a212d677	[Theme] Drop stale pixmap caches on palette-only preview (#7600) ... A palette preview repaints widgets through applyStyleAndPalette without running themeChangedSlot, so generator-owned pixmaps stayed at the previous palette until the next full theme switch. Clear them where the palette is applied. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-24 10:40:20 +02:00
BruebachL	4028606bd9	[CardDB] Clear CardInfo relations before deferred deletion (#7615) ... removeCard() and swapInDatabaseData() schedule CardRelation deletion with deleteLater(), but the owning CardInfo could outlive them through an external shared pointer and kept returning the freed pointers from getAllRelatedCards(). Add CardInfo::clearDirectRelations() and call it after scheduling deletion so the UI can no longer dereference dangling relations. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-24 10:28:24 +02:00
Neel Jogani	6e770eb7b2	Retranslate Home tab when the language changes (#7579) ... * Retranslate Home tab when the language changes TabSupervisor::retranslateUi() never included the Home tab, and HomeWidget set its button text only once, when the buttons were created. Switching languages left the Home tab in the old language until it was reopened. Store the Home buttons as members, add HomeWidget::retranslateUi() to set their text, and call it from TabHome and TabSupervisor. The connect button text still only changes for the Connecting, Disconnected and LoggedIn statuses, as before. * Address review: plain ClientStatus, drop optional and placeholder	2026-10-24 08:35:31 +02:00
Neel Jogani	e73dd2b520	Refactor display type toggler and add language change handling in Vis… (#7581) ... * Refactor display type toggler and add language change handling in VisualDeckDisplayOptionsWidget * Address review: extract updateDisplayTypeButton(), document changeEvent * Restore trailing newline in visual_deck_display_options_widget.cpp	2026-10-24 08:34:19 +02:00
BruebachL	ab13a6748f	[QtUtils] Adopt shared Qt helpers across the client (#7618) ... * [QtUtils] Add parent lookup, layout clearing, scroll area and monospace helpers * [QtUtils] Adopt shared Qt helpers across the client --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-24 08:26:21 +02:00
BruebachL	1e0fc35d94	[QtUtils] Add parent lookup, layout clearing, scroll area and monospace helpers (#7617) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-24 08:26:02 +02:00
BruebachL	bbc6003f0a	[CrashReporting] Name the files and processes behind a failed installer purge (#7590) ... * [CrashReporting] Log which files and processes blocked the installer purge A purge that fails aborts the upgrade with a message that can only guess at the cause, and the log only listed which DLLs survived. Record what Windows reports for each survivor and which processes still have the install directory mapped, then repeat the holder list in the abort message. The diagnostics are additive and run only on the abort path: they open each survivor for DELETE access and close it again without deleting, so a failed upgrade can never end up half-removed. The purge itself is unchanged. * [CrashReporting] Name the holders of a failed purge with Restart Manager The process-and-module scan could only name a holder whose image lives in the install directory or that loaded one of its DLLs, so the case this diagnostic exists for - an antivirus scanner holding a plain handle - was reported as "no holder named, only a code". Reaching the Restart Manager from the NSIS script would have meant marshalling RM_PROCESS_INFO through System::Call as a table of hand-computed offsets, which cannot be checked without a Windows toolchain, and that is why it was dropped when this diagnostics was written. Move the call into tools/installer-holder-probe: a small executable the same Windows build compiles, linking nothing from this project so it still starts when the installed runtime is what the purge could not replace. It rides inside the installer and is extracted to $PLUGINSDIR only when the purge has already failed, so the successful path is unchanged and nothing new is installed. .ci/compile.sh runs it against a file held with FileShare.None on the Windows runners, so the lookup is exercised on real Windows instead of only compiled. * [CrashReporting] Fix the holder probe check and cap the holder line The probe block ran after the package steps, where the working directory is already the build dir, so its $BUILD_DIR-prefixed paths named build/build and the PowerShell file could not be written. Move it ahead of install/package, where the build dir is still the cwd, and name both files relative to it. Guard it on MAKE_NO_CLIENT, since the probe is only built with the client. Cap the rendered holder line at HOLDER_LINE_CHAR_MAX: the count limit alone does not keep long service names inside the NSIS string that also carries the abort text and the log path. Whole entries are kept and the rest counted, with the first holder always shown. Also run cmake-format on the probe's CMakeLists so the lint job accepts it. * [CrashReporting] Report a read-only purge survivor and bound the probe check --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-24 00:46:54 +02:00
BruebachL	ad4320cd7b	[CardDB] Stream card properties from blobs (#7608) ... * [CardDB] Stream card properties from blobs Parsed and cached cards used to keep a materialized QHash<QString, QString> for their properties: LazyPropertiesHash only deferred building that hash until the first query, and the cache writer re-serialized it on save. - LazyPropertiesHash answers value(), contains(), size(), isEmpty() and keys() straight out of the serialized blob, so a property query no longer builds a hash. - serialize() is public, and the cache writer passes the stored blob through instead of serializing a hash it just rebuilt. - CardInfo and PrintingInfo expose getPropertiesBlob() and getPropertyCount(); PrintingInfo::operator== compares key by key without materializing either side. - The XML parsers build cards via newInstanceFromPropertiesBlob(), which derives the simple name and the alternate names that the parser used to keep next to the hash. The reload cycle fix stays out of this change; it depends on changing the CardInfo self pointer and is reviewed separately. * [CardDB] Address review comments on streaming properties * [CardDB] Address final review nits on property streaming Move BlobReader and the blob accessors into LazyPropertiesHash as private members so the parsing helper is declared with the class instead of hiding in the translation unit. Add LazyPropertiesHash::serializedFrom() and use it where a PrintingInfo is built from a fresh property hash, replacing the nested LazyPropertiesHash(LazyPropertiesHash::serialize(props)) construction. The pre-materialized QHash constructor stays: CardInfo's non-blob factory still uses it for the oracle import path, and the tests cover it directly. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-24 00:17:31 +02:00
BruebachL	c25a2c5c3b	[CardDB] Avoid copying printing maps and property strings (#7605) ... * [CardDB] Avoid copying printing maps and property strings Reading a card's sets was a measured hotspot: `SetToPrintingsMap setMap = card->getSets();` copies the whole QMap, and then iterating it with a non-const iterator detaches and deep-copies every PrintingInfo, including its property hash, on first use. A micro-benchmark of that exact pattern (copy plus non-const iteration) came out about 19x slower than a const reference over the same data. The querier, the cache writer, the XML writer, the picture loader and the printing/set selection widgets now take `getSets()` by const reference and only use const iterators. The tests that iterate generated data are covered by the existing card database and picture loader suites. String accessors were also returning by value where the value lives in a hash that has already been materialized: - CardInfo::getProperty() and the legacy getCardType()/getColors()/ getManaCost()/getCmc()/getPowTough()/getLoyalty() accessors, - PrintingInfo::getProperty(), getUuid(), getFlavorName(), getArtist(), and getSet(), - ExactCard::getPrinting() and getCardPtr(), - CardSet::getShortName(), getLongName() and getSetType(). getPrinting() in particular copied a PrintingInfo on every call, and a copy of an unmaterialized PrintingInfo has to take the properties mutex. Callers that need an owned copy still get one; the rest just read. Other allocation removals in the same path: - LazyPropertiesHash::ensureMaterialized() checks an atomic flag before taking the mutex, so property reads after the first one do not lock. The flag is release-stored after materialization and acquire-loaded by readers/copies, preserving the existing locking for the first read. - CardSet::getCorrectedShortName() no longer rebuilds its QSet of reserved Windows file names on every call. - CardSet::getIsKnownIgnored() no longer formats the release date into a throwaway string just to check its length. - The querier builds its card-type/format count caches without `cards.values()` or a properties key list, reserves its result lists, and picks a random card without materializing `cards.keys()`. - The querier's pinned-printing path reuses the CardInfoPtr it already has instead of looking the card up again by name. - CardDatabaseCache/XML serialization copy the alt-name QSet by reference. - The picture loader compiles its !prop!/!set! regular expressions once instead of per URL template, and walks the transform map with an iterator instead of keys() plus operator[]. Full build and ctest (628 tests) pass. * [CardDB] Address review comments PrintingInfo is a value type: it is stored in QLists and returned by value from the querier, so a const-ref getter can hand out a reference into a temporary. Return the QStrings and the CardSetPtr by value again. The constFind lookup stays, so a property read still never materializes the lazy property hash - only the last shared-data refcount is paid. * [CardDB] Undo the const-ref QString return changes QString is implicitly shared, so returning by value costs only a refcount and avoids handing out references into a lazily materialized hash. The constFind lookups stay, so a property read still never materializes the hash. * [CardDB] Use QMap::asKeyValueRange() in transformUrl The range is a view over the map's own iterators and yields references to the stored key and value, so it keeps the no-allocation property of the manual const iteration. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-23 11:58:58 +02:00
BruebachL	b85436461e	[Filters] Precompute term data instead of redoing it per card (#7603) ... * [Filters] Precompute term data instead of redoing it per card Card filters are evaluated once per card in the database, and several attributes derived constant values from the filter term on every single evaluation: - acceptColor() ran up to twelve case-insensitive string replacements and a space strip for each card, then called getColors() again for every character of the term. - acceptRarity() ran the rarity abbreviation expansion loop per card. - acceptManaCost() uppercased and sorted the term per card, before sorting the card's own cost. - acceptLoyalty() and acceptFormat() trimmed/lowercased the term per card. - relationCheck() re-parsed the numeric operator and operand per card, for every cmc/power/toughness/loyalty comparison. The term of a FilterItem never changes, so these values are now computed once in the constructor and only the card-dependent part remains in the accept path. For colors this also removes the repeat getColors() call, which materializes the printing properties hash. Additionally: - CardInfoPtr is passed by const reference through FilterItemList, FilterItem and FilterTree instead of by value, removing an atomic refcount pair per card per filter node. - Main/sub type filtering no longer allocates a QStringList via split(); it looks the " - " separator up directly and compares QStringViews. - acceptSet() and acceptPowerToughness() fetch the printing/set or the power/toughness string once instead of once per branch/accessor. No filtering behavior is intended to change. Invalid numeric terms (for example a bare "<") now return false instead of indexing trimmedTerm[1] past the end. Verified with filter_string_test (42 tests) and a full client build. * [Filters] Address review comments - Replace the main/sub type separator accessor function with a plain namespace-scope constant. - Take card type, colors, cmc, loyalty, power/toughness and the printing's set by value; those getters already return by value. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-23 11:23:17 +02:00
BruebachL	f4b4365464	[Search] Stop allocating per card in card-name completion (#7604) ... * [Search] Stop allocating per card in card-name completion CardSearchModel::updateSearchResults() runs on every textEdited() signal, walks the whole card database and runs Levenshtein against every matching name. Three costs dominated that path: 1. Every candidate built a QStringList of its searchable names, then lowercased each name into another QString, only to break out of the loop after the first hit. 2. The sort comparator called sortableName() for each comparison, which re-fetched the name (QMap lookup plus QString construction) O(n log n) times. 3. levenshteinDistance() allocated a full len1 x len2 vector of vectors. The search model now lowercases the one name it tests directly, stores the tie-breaking name length once per result, and the comparator only compares integers. levenshteinDistance() uses the standard two-row formulation: same result, O(len2) memory instead of O(len1*len2), with early exits for empty and equal strings. FilterString query closures get the same treatment: - Set queries iterate the set map directly instead of materializing keys()/values() lists for every card. - Rarity queries iterate printings in place instead of copying them into a QList<PrintingInfo> first. - Power/toughness split the string with indexOf and compare QStringViews instead of allocating a QStringList. - Field queries do one hash lookup instead of contains() followed by value(). - Color queries bind the colors string by reference instead of copying. Verified with filter_string_test (42 tests) and a full client build. * [Search] Address review comments - Use std::any_of/std::none_of over the set map's key/value iterators instead of hand-rolled loops. - Extract the name matching into matchingLowercaseName() returning std::optional<QString>. - Reuse the matched name's length for the distance tie-break instead of looking the name up again, removing one getLocalizedName() call per card. - Take the color strings by value; the getters already return by value. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-23 11:23:04 +02:00
BruebachL	4980f88a7a	[CardDB] Sort power/toughness without splitting every comparison (#7607) ... * [CardDB] Sort power/toughness without splitting every comparison When the card database is sorted by the P/T column, lessThan() runs O(n log n) times over the whole view, and each call split both strings into a QStringList ("power/toughness") and then read the two elements back. For the roughly 36k cards in the database that is millions of list and string allocations, all for a value that contains at most one separator. The comparator now locates the single '/' with indexOf and passes the two halves as QStringView to lessThanNumerically(), which is changed from QString to QStringView. The numeric parsing, the "1+*" fallback and the locale-aware tie-break are unchanged, and strings with no separator or more than one still fall through to the plain string comparison, exactly as split()-based size checks did before. lessThanNumerically() also initializes its two bool out-parameters; they were declared uninitialized, which only worked because toFloat() always assigns them. Compared old and new comparator over a corpus of 2024 power/toughness strings (4096576 ordered pairs, including malformed values): zero differences. Full build and ctest (628 tests) pass. * Extract well-formedness checks in P/T comparator --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-23 10:34:13 +02:00
BruebachL	e666a4978d	[VisualDatabase] Use scoped QSignalBlockers for filter updates (#7641) ... * [VisualDatabase] Use scoped QSignalBlockers for filter updates updateFilterModel() blocked signals on the filter model and its filter tree,\nthen unblocked each of them twice. The second pair was a no-op only because\nthe first pair had already restored the default. Use two scoped\nQSignalBlockers instead, so the models are unblocked exactly once before the\nexplicit changed()/layoutChanged() notifications.\n\nVerification: cockatrice built. * Drop stale typo note from QSignalBlocker comment --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-23 10:33:29 +02:00
tooomm	c4c7cc0253	[README] Small tweaks (#7585)	2026-10-22 20:13:43 +02:00
BruebachL	0fe4fd0d8c	[CardDB] Guard guessCard and findPrintingWithId against missing cards (#7592) ... * [CardDB] Guard guessCard and findPrintingWithId against missing cards guessCard() dereferenced a null CardInfoPtr through findPrintingWithId() when the name was unknown and a providerId was given. Return an empty ExactCard instead, and make findPrintingWithId() null-safe for every caller. Add regression tests covering both paths. * [CardDB] Rely on PrintingInfo's default null set in findPrintingWithId The explicit nullptr was redundant: the constructor already defaults the set to null. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-20 21:33:56 +02:00
BruebachL	9980c90cea	[Filter] Allow spaces around the colon in search queries (#7529) ... * [Filter] Allow spaces around the colon in search queries * [Filter] Keep whitespace out of the color-identity operator Allowing `ws?` between `color` and the identity marker made `c i: g` parse as identity green, changing the meaning of text that previously read as a generic `c` plus a field query on `i`. Whitespace belongs around the separator, not inside an operator token: `ci : g` and `ci: g` stay valid, `c i: g` keeps its old parse. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-20 21:20:10 +02:00