The server list used hand-numbered saveName%1/server%1 keys and an O(n)
shift loop to delete entries. Store the profiles with beginReadArray and
beginWriteArray instead, and migrate the legacy keys once in the
constructor so existing profiles and saved passwords survive.
SettingsManager::batchWrite no longer disables atomic sync, so a batch is
still written through QSettings' atomic save. Add tests for add, update,
remove, password updates and the legacy migration.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
Qt6::Test was linked into every test but never used. Add a small RAII
QAbstractItemModelTester helper and apply it to the deck list model, the card
database model and display proxy, and the sets model. Add QSignalSpy coverage
for the card-added and download-settings signals, and assert explicitly that a
card row is a leaf with no child columns (the tester does not check that).
The contract checker immediately found four model bugs, fixed here:
- DeckListModel allowed children below non-zero columns.
- CardDatabaseModel reported every card as a child of every card row.
- CardDatabaseModel reported six columns for a valid (leaf) card index.
- CardDatabaseDisplayModel claimed more rows were fetchable while rowCount
already exposed them all, so fetchMore() inserted duplicates.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
checkUserPassword ignored force_password_change on the challenge-response
path, so a reset password stayed valid. Returning PasswordChangeRequired
was not enough on its own: the session was dropped and the client told the
user to ask the administrator for the temporary password they had just used.
The pending session now keeps its identity without registering a user
session or announcing a join, may only ping and change its password, and
the client completes the change in-session before reconnecting.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
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>
* [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>
* [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>
* [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>
