Base deck with 4 Mountain (DOM 250), local adds 2 Shock (M19 156), remote
adds an unrelated card. Asserts the merge keeps one Mountain node with its
printing and adds a single Shock node carrying M19/156.
ZoneViewZone::initializeCards() connected PendingCommand::finished only after ZoneViewZoneLogic::requestZoneDump() had already sent the command. Local games process the dump synchronously, so finished was emitted before the view connected and the library/sideboard view stayed empty.
Split requestZoneDump() into prepareZoneDump() and sendZoneDump(), and connect before sending, matching the TabRoom prepare/send convention.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [CardDB] Drop the CardInfo self pointer
The weak self pointer had to be assigned right after every construction,
and a missed assignment silently stopped change notifications. CardInfo
is always owned by a QSharedPointer, so it now derives from
QEnableSharedFromThis<CardInfo> and asks for sharedFromThis() instead:
nothing to initialize, and the CardSet cycle stays broken.
CardSet::newInstance() also loses the commented-out self-pointer
assignment left over from the same design.
* [CardDB] Pin the cardInfoChanged contract without the self pointer
Dropping CardInfo's self pointer moved change notifications onto
QEnableSharedFromThis, so the signal now rests on the card being owned by
a QSharedPointer. Two tests pin that down:
- A property change on a card owned by a CardDatabase still emits
cardInfoChanged() carrying that card, which is what the card database
views and models listen to.
- clone() starts out unowned, so a change to the clone is reported as the
clone's and must not reach the original's observers.
* [CardDB] Treat an unowned CardInfo as a bug, not a no-op
Every CardInfo is built inside a QSharedPointer (both factory overloads
and clone()), and no constructor notifies anyone, so sharedFromThis() is
always available by the time addToSet() or a setter runs. Assert that and
warn in a released build, instead of silently skipping the notification
or the set append.
The copy constructor keeps its explicit QEnableSharedFromThis
initializer: GCC's -Wextra warns about the uninitialized base in every
translation unit that copies a CardInfo otherwise. A comment there
records that the clone deliberately starts unowned.
* [CardDB] Break the CardInfo <-> CardSet cycle on reload
CardSet is a QList<CardInfoPtr> and CardInfo::addToSet() appends the card
to its sets, while a card keeps a CardSetPtr per printing in
setsToPrintings. The sets therefore hold strong references back to the
cards, and the previous database snapshot stayed alive after a reload, so
RSS grew by roughly a second database on every reload.
Empty the sets before the cards are dropped: CardDatabase::clear() and
swapInDatabaseData() clear the sets before replacing the containers, and
removeCard() detaches the card from its sets, so nothing keeps a card
alive on behalf of a set once the database lets go of it.
Two tests pin the behaviour: a reload and a clear() must drop the last
strong reference to a card from the previous snapshot.
* [CardDB] Keep the set detach in one place
clear() and swapInDatabaseData() both emptied every set before dropping the
cards, so share that as detachSetMembership() instead of two copies that have
to stay in sync.
removeCard() now detaches through the first printing of each set directly, the
same way CardInfo::newInstance() appends, instead of an inner loop that always
breaks after the first item.
Because clear() empties the sets up front, its per-card detach was walking
every card's printings to remove it from sets that were already empty. Split
removeCard() into the per-card detach and unindexCard(), and let clear() call
only the latter.
* [CardDB] Test that removeCard detaches the card from its set
removeCard() is the one teardown path a caller can trigger for a single card
(the token editor uses it), and it now detaches the card from its sets as
well. Without that detach the set's strong CardInfoPtr keeps the removed card
alive forever, which is exactly what the reload and clear tests pin for the
whole-snapshot paths.
The test loads the fixture, removes Cat, and requires the card to be gone from
the CAT set and to be destroyed once the test drops its own reference.
* [Servatrice] Drop the settings cache from the logger
#7602 removed #include "settingscache.h" from server_logger.cpp but left
isLoggingEnabled() reading settingsCache, so master no longer compiles: the
servatrice target fails on the undeclared global and server_logger_test, which
compiles the same file without main.cpp, fails to link it.
The same commit made reloadConfig() the single place that stores writelog and
the log filters, precisely so logging threads stop reading the global QSettings.
isLoggingEnabled() now mirrors logMessage() and reads those members instead: the
console is a sink of its own, and the logfile needs an open file plus writelog.
The mutex is mutable so the const query can take it.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Servatrice] Fix silently broken socket error connections
QAbstractSocket::error() was removed in Qt 6 in favour of errorOccurred(). The ISL interface connected to the old name through SIGNAL(), which compiles but fails at runtime, so catchSocketError never ran. The WebSocket interface used the same name on QWebSocket, where it still exists but is deprecated since 6.5.
Use errorOccurred for both. The WebSocket connect needs a lambda because catchSocketError is a protected base-class slot and a base pointer-to-member is not accessible from the derived class.
* [Servatrice] Guard the WebSocket error connect against Qt 6.4
QWebSocket gained errorOccurred in 6.5 and dropped the error signal with it, so
the typed connect cannot compile against the 6.4 floor that Debian 12 and
Ubuntu 24.04 still use. Keep the typed connect on 6.5+ and fall back to the
old-style connect below it, which is what the code did before.
Verified both branches compile: forcing the guard to the else path and
rebuilding serversocketinterface.cpp succeeds, so the 6.4 spelling is intact.
* [Servatrice] Initialize ISL serverId before authentication
catchSocketError() removes islInterfaces[serverId], but the incoming
connection constructor left serverId uninitialized until initServer()
set it after the SSL handshake and certificate check. A socket error
during waitForEncrypted() therefore read an indeterminate key.
* [Servatrice] Correct the QWebSocket error signal note
error() was deprecated in 6.5, not removed; errorOccurred() was added in
6.5.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Servatrice] Exit non-zero when server initialization fails
initServer() reported every startup failure through a zero exit code: the
database could not be opened, the server printed nothing of its own, and the
process looked like a clean shutdown. Log the two refusal reasons at critical
level and return 1 so supervisors and the Docker exit status can tell the
difference.
* [Servatrice] Keep console logging independent of the logfile
logMessage() returned before the console echo unless a logfile was open and
server/writelog was true, so --log-to-console produced nothing when
server/logfile was empty or could not be opened. That is how a refused database
connection could end in a silent exit.
Print to the console before the logfile gates, drop the duplicate echo from
flushBuffer(), and read writelog/logfilters once in main instead of calling the
non-thread-safe global QSettings from every logging thread.
* [Servatrice] Uninstall the log handler before tearing down globals
The handler reaches into the global logger and settingsCache. Leaving it
installed while main deletes those objects turns any Qt warning emitted during
QCoreApplication/QFactoryLoader teardown into a use-after-free; with
QT_DEBUG_PLUGINS=1 that warning exists, and Servatrice died with SIGSEGV in
QSettings::value() on the way out.
* [Servatrice] Buffer console logs on the logger thread
Address review feedback on the startup failure change:
- Append console lines to a buffer drained by the logger thread, so a
stalled stdout reader cannot block game threads.
- Report islServer->errorString() instead of the literal call text.
- Let the constructor reuse reloadConfig() instead of parsing the filter
list twice.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Build] Use versionless Qt CMake commands
Replace the versioned qt6_add_executable, qt6_add_resources, qt6_add_shaders,\nqt6_add_translations and qt6_wrap_cpp calls with their unversioned forms.\nAll five are documented as available at the project's 6.4 minimum and accept\nthe same arguments.\n\nVerification: full client, Oracle, server and test build passed.
* versionless qt_finalize_target and _project
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
Co-authored-by: tooomm <tooomm@users.noreply.github.com>
Add the deprecation cutoff after FindQtRuntime so the compiler rejects any
API deprecated before the project's minimum Qt version. Qt 6.5 renamed the
switch, so the fallback name is used on 6.4.
The full client, Oracle, server and test build passes with the cutoff
enabled.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
The cache action creates its directory only when a prefix matches an entry, and it reads that directory unguarded when it saves, so a scope nobody has written to yet fails the job. Create it up front on every outcome, and fall back to the workspace path when the cache step was skipped and set no path output.
Co-authored-by: opencode <opencode@local>
The 13 libraries using qt6_wrap_cpp(MOC_SOURCES ${HEADERS}) listed their\nheaders twice and depended on a generated source list. Add the headers to the\ntarget sources directly and delete the qt6_wrap_cpp call; AUTOMOC scans them\nand generates the same moc output.\n\nThe change exposed that server_card_counter_test never defined the rng global\nthat the server remote library references; every sibling server test does.\nGive the test the same definition so the target links.\n\nVerification: full client, Oracle, server and test build passed.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Refactor] Move game logic into libcockatrice_game
* [Refactor] Include card_state.h in player_actions.h
* [Game] Inject settings providers into libcockatrice_game, drop Widgets
* [Game] Follow base rename to createCardRequested after rebase
* [Game] Drop unused CardItem forward declaration
* [Game] Fix includes after moving game logic into libcockatrice_game
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Refactor] Move card item creation out of game logic
* [Game] Rename signal to createCardRequested per review
* [Game] Move attachment bookkeeping onto CardState
CardState::setAttachedTo now maintains the attachedCards graph, resets the
grid point and emits the zone events itself, so game logic no longer needs
to reach into the CardItem graphics item. CardItem::setAttachedTo delegates
to the state and keeps only the view teardown (deleteLater).
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [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>
