* [Refactor] Retype card containers and zone logic onto CardState
* [Game] Restore attach bookkeeping via CardItem and repaint on state change
* [Refactor] Give custom zones a hidden graphics owner
Custom PileZoneLogic zones had no CardZone, so moving a card into one
left the CardItem drawn at its old spot. Create a hidden PileZone in
onCustomZoneAdded so cards are reparented and hidden.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Refactor] Move the remaining zone commands out of game_graphics/zones/
* [Game] Move zone dump request onto ZoneViewZoneLogic per review
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Refactor] Move table row mapping from TableZone into TableZoneLogic
* [Game] Use TableZoneLogic::ROW_COUNT directly per review
* [Format] Match clang-format line breaking in table_zone
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
Folds in the closed#7410 (Move AnnotationDialog from game/ logic to
game_graphics/) now that layers 1-2 are closed and this stack bottoms on
master directly:
- Add game_graphics/dialogs/dlg_annotation.{h,cpp} and move the
QInputDialog subclass there; game/ no longer declares any widget type.
- Add game/player/token_info.h and game/player/move_top_cards_until_options.h.
- Move CardMenuActionType to game/player/card_menu_action_type.h.
- Graphics dialog headers include the new game/ headers instead of
defining the structs themselves.
No behavior change.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Installer] Stop calling the payload free while an executable is locked
The guard answered "nothing is holding the payload" from two lookups that can
both miss a running instance: Restart Manager does not reliably report a
program's own image or the modules it mapped, and the image path of a process
running under another account cannot be read from the probe at all. A user log
showed exactly that: the first guard passed while cockatrice.exe was running
from the directory, the old uninstaller silently skipped the file it could not
delete, and the update aborted half an uninstall later with "cancelled by user"
after a close attempt the running window ignored.
Ask the file itself. A running image is opened without write sharing, so the
write open replacing it needs is refused with ERROR_SHARING_VIOLATION for as
long as the program runs, while a file that is merely being read still accepts
it. When neither lookup named anybody and a payload executable refuses that
open, the probe answers that the file is in use and that the program holding it
could not be identified - the guard's "could not confirm" dialog, whose Retry
works once the user has closed it - instead of passing the payload as free.
Only the payload executables are tested, not every file the purge removes: a
locked executable proves a program is running and a mapped DLL belongs to a
running executable, while a locked DLL or qt.conf can be the asking process
itself, which is the loop the guard already had. The open is retried once after
300 ms, and only for the files that refused, so a scanner holding a file for a
moment cannot stop an install for nothing.
The Windows probe check now covers that answer: a payload executable held by
the asking process, so that it cannot be named, has to come back as in use
rather than as free.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Installer] Close running clients over a local socket
The installer could only ask a running client to close with WM_CLOSE, which an instance hidden in the notification area or sitting on its own confirmation cannot answer usefully. Give every client a per-process local socket and have the holder probe ask it there first, falling back to WM_CLOSE for clients from before the socket.
* [Installer] Offer to check again or update by hand when files stay locked
Replace the two-answer guard prompt with force close, close by hand and check again, or cancel. Retry the automatic close while the set of holders changes, and point a cancelled run at the latest release so a user who does not want anything killed has a way forward that is not cancelling the update again.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
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>
* [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>
* [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>
* [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>
* [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>
* [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>
* [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>
* [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>
* [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>
* [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>
* [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>
* [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>
* [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>
* 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
* [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>
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>
* [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>
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>
