* [Client] Settings: extract SettingsView from DlgSettings
Move the search bar, tab bar, stacked pages and search results out of
DlgSettings into a reusable SettingsView widget. The dialog becomes a
thin frame (title, OK button, close validation) delegating to the view.
No visual or behavior change; tr() contexts preserved as DlgSettings.
* [Client] Settings: match CI clang-format-18 line breaking
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Security] Add challenge-response auth with scrypt verifiers and stop storing plaintext passwords
Challenge-response authentication: the client derives a scrypt verifier
(RFC 7914, EVP_PBE_scrypt, N=32768, r=8, p=1) and authenticates with
HMAC-SHA256(key, nonce), so neither the password nor its hash is
transmitted. The stored format becomes
"$scrypt$<n>$<r>$<p>$<salt>$<verifier>" and Response_PasswordSalt
now carries the cost parameters. Strict servers only accept scrypt
verifiers; legacy accounts are migrated after a successful login.
Fix#344 for challenge-response servers: a saved profile stores the
derived verifier under the password key instead of the plaintext password.
The connect dialog loads it without revealing it, autoconnect passes it
through, the change-password dialog no longer prefills the old password
field with it, and the client only persists the verifier when
"Save password" is checked.
Took 3 minutes
Took 1 minute
Took 10 seconds
Took 7 minutes
* [Security] Harden credential format acceptance and challenge nonce validation
* [Security] Refuse KDF downgrade; trim profile name and warn on silent credential drop
* [Security] Add SupportsAll ServerOptions enumerator to fix enum validation assert
The server advertised both SupportsPasswordHash (1) and
SupportsChallengeResponseAuth (2) via a bitwise-OR that produced the
value 3, which no enumerator declared. Protobuf's proto2 enum
validation rejects the value in _internal_set_server_options, crashing
servatrice on the debug assertion. Add a SupportsAll = 3 enumerator with
an identical wire value so the bitmask reads unchanged while the value
is now a documented, valid member of the enum.
* [Security] Normalize username when issuing challenge nonce
cmdRequestPasswordSalt bound the challenge nonce to the raw, un-simplified
username while cmdLogin verifies it against the simplified name. A user
who typed leading or trailing whitespace in the username box would be
silently rejected during challenge-response login even with the correct
password. Simplify at the nonce-issuing site so both commands agree on
the normalized name.
Also drive-by formatting: collapse the now single-expression
serverOptions initializer onto one line.
* [Security] Keep an encrypted legacy password backup for old-client logins
Migrating an account to scrypt stranded pre-challenge clients, which can
only present the legacy salt+hash: after migration the server served them
a scrypt salt they cannot use, so the only recovery was a password reset.
When security/legacy_backup_key is configured (64 hex chars,
openssl rand -hex 32), submitPasswordVerifier now seals the legacy
credential with AES-256-GCM under that key before overwriting the row,
storing it in password_legacy_backup with the time it was last used. Old
clients keep working: cmdRequestPasswordSalt serves the backup's 16-char
salt in password_salt so they can derive the legacy hash they were
always able to compute, while new clients derive the verifier from the
new scrypt_salt field, which always carries the verifier salt. The
non-challenge login path accepts the plaintext or legacy hash against
the decrypted backup and refreshes its window; a migrated row with no
usable backup fails closed.
Backups age out so a database dump taken after the window closes never
carries a crackable legacy hash: getUsableLegacyBackup purges on first
use past security/legacy_backup_ttl_days (default 90, clamped 1..3650),
and a six-hourly sweep drops backups idle accounts never touch.
changeUserPassword clears the backup since a new credential invalidates
it. Without a configured key nothing is sealed or purged, so a
temporarily missing key cannot destroy existing backups.
The AAD is the simplified user name on both sides, sealing the blob to
its row so it cannot be moved between accounts. Sealing failures
degrade to the previous lockout behaviour rather than blocking
migration. The columns extend the unmerged 0036->0037 migration, so the
schema version stays 37.
* [Security] Address review: proto compat, plaintext verify, AAD case, strict scope
Fix the two blockers and follow-ups from the 2026-10-16 review:
- Advertise challenge-response in a new supports_challenge_response
field instead of the ServerOptions enum and drop SupportsAll:
proto2 enums are closed, so already-released clients parsed
server_options = 3 as NoOptions and fell back to plaintext
passwords. server_options stays exactly SupportsPasswordHash
on the wire; old clients ignore the new field.
- Verify a plaintext password directly against a migrated scrypt
row with verifyPassword() so email changes (cmdAccountEdit)
and old clients sending password keep working without a
legacy backup key. Pre-hashed values are still checked
against the backup, and its TTL window is refreshed only
when the backup is what actually matched.
- Use the stored row name as the AES-GCM AAD on both seal
and open so a login whose case differs from the stored
name (utf8mb4_unicode_ci) still opens its backup.
- Require challenge-response in strict mode only for
registered SQL accounts (userExists gate), leaving
guest/unregistered logins and none/password auth
methods unaffected.
- Document in servatrice.ini.example that a captured
legacy hash can take over a not-yet-migrated account
and that strict only constrains registered SQL
accounts. Schema version is 38.
* [Security] Raise legacy backup TTL default to 180 days
Gives operators and slow-to-upgrade users a longer downgrade
window for pre-challenge clients before unused encrypted
legacy password backups age out.
* [Security] Fix servatrice.cpp formatting per clang-format-18
* [Security] Add missing forward declaration for Command_SubmitPasswordVerifier
The cmdSubmitPasswordVerifier declaration references the proto-generated
type, which otherwise does not name a type in this header and breaks
every C++ build.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
Co-authored-by: BruebachL <bruebachl@cockatrice.example>
* [Servatrice] Log room channel messages and bump schema to 38
* Fixup: show room_channel audit rows and keep migration nullable
The log history query returns room_channel rows, but the log tab
dropped them. Render them in the room table with an extra Channel
column. Also drop the NOT NULL on target_type from the 37->38
migration so migrated schemas match the nullable servatrice.sql
definition (and strict-mode ALTERs cannot fail on legacy NULL rows).
Update the channel tests to the flag-based privilege model: the admin
fixture now carries IsModerator alongside IsAdmin, as the database
layer assigns.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Client] Add room channel selector with per-channel chat views
* [Client] Tabbed room channels with unread states, collapse, read-only, banners
Reworks the room chat channel display: channels move out of the combobox
into a tab bar below the message input showing every channel available
to the user.
- Each tab shows its unread message count and is tinted when it holds
unreads or a mention; both colors are user-configurable chat settings
(channelUnreadColor, channelMentionColor) with a settings-page section.
- Clicking the active tab collapses the chat: message view and input
hide and the splitter section minimizes to the tab strip; expanding
restores the previous splitter sizes. Viewing a channel clears its
unread state.
- Channels the user may read but not write (per write_access_level)
show with the message input disabled and a read-only placeholder;
sends are guarded locally, the server still enforces.
- Channel banner_message strings render at the top of their channel
view on join, like the room join message on Main.
- Replayed channel history no longer inflates unread counts; moderation
redaction now applies across all channel views.
* Fixup: keep chat tab bar visible when chat is collapsed
QSplitter children are collapsible by default, so setSizes({1, 0})
hid the whole chat group box including the channel tab bar.
Mark the chat widget non-collapsible so collapsing clamps to its
minimum size (title + tab bar) instead.
* [Room] Explicitly initialize MAIN_CHANNEL per review
* [Room] Address channel tab review follow-ups
- gate @all mentions on moderator/admin sender to match ChatView,
whole-word matching for own name and highlight words
- hide single-channel tab bar so old servers do not show a lone Main
tab that collapses chat on misclick
- capture target ChatView in sendMessage so errors land in the
channel the message was sent from
- share ignore/unregistered filtering between RoomSay and
RoomChannelSay via isRoomMessageIgnored
* [Client] Fix tab_room.cpp formatting per clang-format-18
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
Co-authored-by: BruebachL <bruebachl@cockatrice.example>
* [Installer][Client] Back up user data before old uninstallers can delete it
* [Installer][Client] Fix stale backup resurrection, batch session backups, extend coverage
NSIS: clear stale %TEMP% backup and only arm restore when something was copied, directory-existence checks, back up loose .dec decks, drop downloaded pics but keep pics\CUSTOM.
Helper: single session dir per run (ms timestamps), same-basename disambiguation, partial copies still report their location.
* [Installer][Client] Fix review findings: throttle backups, NSIS old dir, portable messaging
* [Installer][Client] Fix review findings: backup markers, NSIS relocate, size cap
* [Installer][Client] Fix review findings: sounds, session coverage, NSIS wording
Client: back up data/sounds in portable mode and mention it in the
warning; reuse a recent session only when complete and covering the
current sources (source manifest), estimate size only for fresh copies,
normalize dedup via isInsideDir.
Helper: source manifest per session with sessionCoversSources(), clear
out-params on all failure paths.
NSIS: document defensive portable branch, merge backup over surviving
data/ to repair partial deletes, copy (not move) wording in relocate,
note .txt exclusion in deck notice.
* [Installer][Client] Fix review findings: stale backup, empty reuse, relocate, portable warning
NSIS: wipe TEMP backup once per run to avoid resurrecting deleted files,
check data dir existence in relocate so dir-only trees are moved.
Client: never reuse a recent session for an empty source set, only fall
back to a covering session on copy failure, warn fresh portable installs
instead of staying silent.
* [Installer][Client] Fix review findings: usable session reuse, loose decks, NSIS pics, symlinks
* [Installer][Client] Fix portable backup exclusions in offender merge
* [Client] Split at-risk data warning into focused helpers
Ricky's nit on the 200+ line warnAboutDataPathsInProgramDir: extract
collection (portable/installed), backup, message building and dialog
into named helpers. No behavior change, all tr() strings untouched.
* [Client] Read path settings inside isPortableBackupExcluded
* [Client] Make stateless at-risk helpers static
* [Client] Extract at-risk data warning out of MainWindow
* [Installer][Client] Fix review findings: subdir backup, portable warning, backup API
NSIS:
- Back up unknown top-level install subdirs (e.g. <installdir>\decks),
not just loose deck files, so the 3.1.1 -> new upgrade no longer loses
data folders the old RMDir /r deletes before the client ever runs.
Payload dirs (plugins/translations/sounds/themes) stay in place to avoid
restoring stale DLLs; restore + relocate handle the new backup set and
the one-time notice covers both decks and folders.
- Restart the %TEMP% backup when the 32/64-bit passes point at different
dirs instead of merging (which restored one dir's files into the other);
document that only the last pass relocates and the %TEMP% copy is not
a durable second copy.
Client:
- Drop the portable at-risk warning: portable installs never shipped an
uninstaller and portable updates preserve data/, so every portable user
got a spurious dialog plus daily host-machine copies.
- AtRiskDataWarning is now a plain class with Q_DECLARE_TR_FUNCTIONS
instead of QObject+Q_OBJECT; logging category renamed to
at_risk_data_warning; MainWindow wrapper inlined into startupConfigCheck.
- Lower the synchronous startup backup cap 500 -> 200 MB.
Utility/tests:
- Remove test-only backupAtRiskPath()/findRecentBackupSession() and the
no-marker/no-manifest legacy paths; isBackupSessionComplete() now
requires the complete marker; tests updated to the plural API.
* [Client] Restore collectInstalledAtRiskPaths per review
a23e02b5 removed the definition along with the portable helpers,
breaking the build (undefined reference at link time, and
isInsideAnyDir unused under -Werror on Arch/Debian 13). Restores
the 202c0d65 version (pathsInsideDir plus loose top-level deck
files) as suggested.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Completer] Highlight popup rows without rewriting the line edit
Selecting a popup row with ClearAndSelect emits selectionChanged, which
QCompleter turns into highlighted() and QLineEdit turns into setText().
Since be2c16eca5 (#7466) that rewrites the typed text on every completion
reset / arrow-key press (e.g. typing "A" becomes "Axe" and cannot be
edited), regressing the 514014dc00 (#7447) fix that used NoUpdate.
Add selectWithoutRewrite(), which blocks the selection-model signals while
applying ClearAndSelect so the highlight stays visible without touching
the line edit, then syncs the preview manually. Route both the
above-popup auto-select and Up/Down navigation through it, and handle
Up/Down for below-popups too instead of falling through to Qt's
rewriting default. Enter/click acceptance via activated() is unchanged.
* [Completer] Repaint popup highlight after silent selection change
Blocking the selection model's signals also suppresses the view's own
selectionChanged/currentChanged repaints, so force a viewport update
after restoring signals. Use EnsureVisible to match Qt's normal scroll
behaviour.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Completer] Minimum size popup, fall back to compact design if not possible.
Took 15 minutes
Took 6 seconds
Took 5 minutes
* [Completer] Fix popup going offscreen by clamping in screen coordinates
The position clamping in applyPopupWidth() was comparing popup->pos()
(parent coordinates) against screen->availableGeometry() (screen
coordinates). When the parent widget is not at the screen origin the
clamping computed a position in the wrong coordinate space, placing the
popup offscreen. Convert to global coordinates first, clamp, then map
back to parent coordinates.
* [Completer] Clamp popup from geometry() in screen coordinates
setMinimumWidth() already widens the popup synchronously, so the old
width() < minWidth guard never ran and the popup grew past the right
screen edge. The popup is also a top-level Qt::Popup window, so move()
takes global coordinates and the mapFromGlobal round-trip moved it to
the top-left of the screen. Clamp x from geometry() after
setMinimumWidth() and move(QPoint(clampedX, popup->y())) directly.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Oracle] Don't report a cancelled card database update as successful (#3470)
Took 3 minutes
* [Oracle] Only report cancel when nothing was saved yet
Rejecting the wizard after SaveSetsPage or a download page already
wrote the database to disk still replaced cards.xml on disk, but
wasCancelled() forced exit code 1 so Cockatrice skipped the reload.
Track successful saves with OracleWizard::markDatabaseSaved() and only
treat a cancel as a failure when nothing was saved.
* [Oracle] Restore updater stderr in card update error dialog
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Style] Confirm broken Windows styles with a dialog offering Fusion
* [Style] Address review nits on broken style dialog
Extract Fusion index lookup into findFusionIndex helper and add all
message box buttons before choosing the default button.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [GameSelector] Persist quick-filter toolbar edits and repair filter settings migration
- Save game-filter parameters on toolbar changes, debounced with flush on close
- Repair pre-camelCase gamefilters.ini keys in place on startup
- Map the legacy creator-name filter to hostNameFilter
- Balance begin/endFilterChange in GamesProxyModel::refresh
- Align the hideIgnoredUserGames default between model and settings
- Cover the migration with regression tests
* [Review] Address RickyRister comments on filter persistence
- Rename game-type prefix constants to SCREAMING_SNAKE to match
MIGRATION_SENTINEL_KEY and project style
- Document why GameSelector destructor flushes a pending debounced
filter save (toolbar path has no other writer)
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [CardDB] Reload when the first run enables all sets
checkUnknownSets() enabled every set when none was enabled, but the
database had already been parsed under that empty enablement, so the
parsers had stripped every printing. It only notified the models and never
reloaded, leaving the deck editor's card database display empty until the
next launch.
Route the all-sets-enabled case through the same reload the new-sets path
uses, and skip caching a snapshot whose sets are all disabled.
* [CardDB] Own checkUnknownSets after startup and fix the cache skip
The loader's initialLoadComplete guard skipped checkUnknownSets() on the
first successful load of the session, so a download that landed after
startupConfigCheck() and the onboarding wizard had already run (e.g.
skipping the wizard download and fetching later from the menu) left every
set disabled and the deck editor blank until the next launch. The guard
also let the check run on the worker before the queued snapshot swap, and
alongside reloadCardDatabasesAndNotify()'s own check, double-emitting
cardDatabaseNewSetsFound on manual reloads.
Move the check to the callers that load after startup: the post-update,
custom-import, and spoiler paths run it via a QFutureWatcher once the
finished snapshot has been swapped in, and the loader's synchronous
path-change reload runs it directly.
Judge the cache skip by the parsed sets' own enablement, what the parsers
actually saw, instead of getEnabledSetNames(), which reports empty for the
noop controller even though it enables every set. Wait for the first-run
reparse before opening Manage Sets so the dialog cannot bind to the
pre-reload set instances.
* [CardDB] Queue post-reload check and unblock first-run Manage Sets
reloadCardDatabasesAndNotify() no longer emits cardDatabaseEnabledSetsChanged after the swap's Reset already rebuilt the models (second 36k rebuild froze startup), and its follow-up checkUnknownSets() is queued to the GUI thread so it sees swapped-in data. cardDatabaseAllNewSetsEnabled() chains Welcome/Manage Sets off a QFutureWatcher instead of waitForFinished() blocking the event loop during the reparse.
* [CardDB] Batch the first-run enable-all into a single settings flush
checkUnknownSets() enabled every set with ~3 QSettings rewrite cycles per set (sort key, known, enabled), and each setValue re-opens and re-syncs the whole cardDatabase.ini. With a full database (874 sets) that froze the GUI thread for ~9s after the database download (LagMonitor: event loop stalled for 9129 ms, measured offscreen with a real 59MB cards.xml), reading as a total application hang that recovers on its own.
Persist the enable-all through the controller's batched saveSets() in a single flush instead, mirroring what SetsModel::save() already does: CardSetList::enableAllBatched() assigns fresh sort keys and marks every set known+enabled in memory, then writes all records at once. SetSaveData gains the isKnown flag so the batch covers all three options, and SetsModel::save() passes the sets' known state through (unchanged behavior there).
Also drop the cardDatabaseEnabledSetsChanged emission from the all-sets-enabled branch: the reload chained off cardDatabaseAllNewSetsEnabled rebuilds the models from the re-parsed snapshot via cardDatabaseReset, so emitting both rebuilt the 36k-card models twice on the GUI thread.
Adds a cache-invalidation test driving the real checkUnknownSets() first-run path.
* [CardDB] Batch the new-sets-found enable-all the same way
The first-run batching fixed the 874-set case, but the new-sets-found dialog path (enableAllUnknownSets/markAllSetsAsKnown) still wrote per set and per option, each setValue re-opening and re-syncing the whole cardDatabase.ini. Users with hundreds of custom sets hit the same multi-second GUI stall when answering the dialog.
Persist both through the controller's batched saveSets() in a single flush, writing only changed sets (so the common no-op check skips the flush entirely). Removes the now-unused write-through CardSetList methods (enableAll, enableAllUnknown, markAllAsKnown, guessSortKeys) so the batched flush is the only way to persist bulk set changes.
Extends the test controller mock with per-write/batched-flush counters and adds tests asserting both dialog paths persist with exactly one flush and zero individual writes.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Game] Invalidate view when hiding replaced card menu
Hide any visible card menu before replacing and invalidate viewport.
* [Game] Fix viewport update compile error in setCardMenu
QWidget::update has no QRectF overload, and sceneRect is in scene
coordinates rather than viewport coordinates anyway, so invalidate the
whole viewport instead.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
An open [[ card link now takes precedence over a space-preceded @
mention, so typing an @ before a card no longer shadows the card
completer.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
- Remove the third close option (Resolve with note...) so closing a
report is one of two clear actions
- Make Resolve always prompt for the optional resolution note, matching
Dismiss, and rename the button to Resolve...
- Halve the resolveReport signature now that every caller prompts
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Report] Add report reopen action for closed reports
- New Command_ReportReopen moderator command (extension 1206) with server
handler that reopens resolved/dismissed reports, resets resolution state
and notified, and records the reopening note in the audit log
- Reopen... button in the report queue action bar, gated to resolved and
dismissed reports, with optional note prompt and tooltip
- Success confirmations for Assign, Resolve/Dismiss and Reopen now survive
the list refresh via a pending status message instead of being clobbered
- Dispatch/auth coverage in server_moderator_role_test
* [Report] Surface resolution audit info in report details
- Add resolved_by_name to ServerInfo_Report and join resolved_by users in
the report list, my-list and details queries
- Render resolution note and resolved-at/by in the report detail pane
instead of duplicating the description into the chat log context
* [Report] Add context menu to report queue rows
- Open the standard UserContextMenu on right-click/Menu key for the
reported user, selecting the row first so the details panel matches
the menu target
- Carry the reported user's level in ServerInfo_Report so the menu
shows the same actions as the user list, with an IsUser fallback for
deleted or unregistered users
- Wire the Private chat action through TabReport to TabSupervisor and
retranslate the menu on live language switches
* [Server] Fix reported_user_level column indices in report queries
The SELECT lists resolved_by_name at index 10, shifting room_id to 11,
replay_id to 12, reported_user_id to 13 and reported_user_admin to 14.
The code read 12/13, so 'user exists' meant 'a replay exists' and the
admin flags came from the user id (e.g. id 5 -> admin+judge; users
without a replay reported as unregistered, hiding registered-only
actions). Fixed in cmdReportList and cmdReportMyList, plus the column
comment.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
- Add resolved_by_name to ServerInfo_Report and join resolved_by users in
the report list, my-list and details queries
- Render resolution note and resolved-at/by in the report detail pane
instead of duplicating the description into the chat log context
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
- New Command_ReportReopen moderator command (extension 1206) with server
handler that reopens resolved/dismissed reports, resets resolution state
and notified, and records the reopening note in the audit log
- Reopen... button in the report queue action bar, gated to resolved and
dismissed reports, with optional note prompt and tooltip
- Success confirmations for Assign, Resolve/Dismiss and Reopen now survive
the list refresh via a pending status message instead of being clobbered
- Dispatch/auth coverage in server_moderator_role_test
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [UI] Fix auto-completion acceptance on Space/Tab/Enter
* [UI] Accept current completion when no item selected in popup
* [UI] Also accept completion on focus out when none selected
* [UI] Keep focus in chat on Tab completion acceptance
* [UI] Prevent Tab from moving focus when no completion popup
* [UI] Refocus on Tab when completing
* [UI] Let focusOut handle Tab completion acceptance
* [UI] Restore Tab behavior to match original (handle in focusOut)
* [UI] Defer refocus on Tab to avoid focus chain issues
* [UI] Auto-select first completion match in popup
* [UI] Auto-select first match when completer popup resets
* Revert "[UI] Auto-select first match when completer popup resets"
This reverts commit c7a3de3595.
* Revert "[UI] Auto-select first completion match in popup"
This reverts commit ae0dfb8622.
* [UI] Ensure completion selection updates on model changes
* Revert "[UI] Ensure completion selection updates on model changes"
This reverts commit 686bbef5b2.
* [UI] Highlight the selected completion in chat popups
The card completer's above-variant moved the current row with
NoUpdate, so the delegate never painted the selection border, and it
used Select which accumulated highlights while navigating. Use
ClearAndSelect so only the current row is highlighted and the old one is
cleared.
The mention popup opened with no current row at all, so nothing was
highlighted and there was no visual hint of what Enter would insert.
Select the row currentCompletion() uses, and install a delegate that
mirrors the active highlight into the inactive group so the selection
stays visible when the transient popup is not the active window.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
getPreferredPrinting returns a null PrintingInfo when a deck card has no
database entry or printings. SetProviderIdToPreferred dereferenced
getSet() unconditionally, crashing on custom or unresolved cards.
Guard against a null preferred set and leave the card's cleared printing
data as is.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Client] Improve card database update visibility
Addresses Cockatrice/Cockatrice#7431.
Cockatrice previously offered no in-client feedback while the card
database updater ran: the only progress UI lived in the first-run
wizard, stderr/stdout were not parsed in interactive runs, and starting
a second update with Oracle already open or with a hidden background run
active just showed a confusing 'already running' guard - even though
Oracle is launched as a separate process with no lifecycle tracking.
- Add a status-bar widget (CardDatabaseUpdateStatusBar) that appears
while an update runs and shows the Downloading/Parsing/Importing
stages with percent progress; it is also used in the quit-confirm
prompt so users see an unfinished update before cancelling it.
- Parse the updater's PROGRESS lines (new CardUpdateProgressParser,
unit-tested) in both foreground and background runs and log stage
changes to the new card_database.update category, throttled to at
most one line per second; forward Oracle stderr to the logs.
- Show a tray balloon when a background update starts and on finish or
failure instead of staying silent.
- Request a running Oracle instance to raise itself (QLocalServer
CockatriceOracleRaise-<user>) when the user starts another update, and
demote the 'already running' guard to an informative message only when
no running instance can be found.
- Emit download/scan/import progress unconditionally from Oracle so
interactive and background runs behave the same.
- Document the new log category in the troubleshooting guide.
* [Client] Address card database update review feedback
* [Client] Add setting to control card database update tray notifications
* [Client] Address remaining card database update review nits
- Rename card_update_progress_parser.{h,cpp} to card_update_progress.{h,cpp}
- Store the CardUpdateProgress struct in the status bar widget
* Lint.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Style] Fall back to the built-in windows style instead of windowsvista
The Windows Vista native style is broken the same way as the Windows 11 one,
so the sanitizer now catches both and falls back to the built-in QWindowsStyle
instead of the Vista plugin. The style picker no longer offers either.
* [Style] Keep Windows native styles selectable but warn about them
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [CardDB] Drop rebalanced-card filtering from the v4 parser
Rebalanced cards no longer exist, so the includeRebalancedCards
preference has no data to filter. Always keep printings marked
isRebalanced and remove the now-unused ICardPreferenceProvider
plumbing from CockatriceXml4Parser and CardDatabaseLoader.
The setting itself (CardsDisplaySettings, Manage Sets checkbox,
Oracle attribute mapping) is removed in a follow-up.
* [CardDB] Drop the rebalanced-cards setting entirely
Rebalanced Alchemy cards no longer exist, so remove the
includeRebalancedCards preference surface: the CardsDisplaySettings
getter/setter/signal, the ICardPreferenceProvider and
ICardsDisplaySettingsProvider methods, the Manage Sets checkbox, the
legacy settings migration mapping, the isRebalanced attribute mapping
in the Oracle importer, and the isRebalanced attribute in the v4
schema.
Builds on #7448, which already stopped the v4 parser from filtering
on the preference.
* [CardDB] Drop the unused isOnlineOnly printing property
The Arena-card detection switched from isOnlineOnly to isRebalanced in
#5778, so nothing has read this printing property since; it only
round-trips from the Oracle importer into cards.xml. Remove the
importer mapping, the benchmark fixture field and the v4 schema
attribute so regenerated databases stop carrying it.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Completer] Stop line edit rewrite on popup highlight
* [Completer] Extract the completion index into a variable
Make the popup selection update readable at a glance.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [DeckList] Apply include-what-you-use to libcockatrice_deck_list
* [IWYU] Apply include-what-you-use across the tree
Run one whole-tree IWYU pass over the libcockatrice_* libraries, the
Cockatrice client, Oracle, Servatrice and the test suite, so each
translation unit includes what it uses directly and forward-declares
what it does not need.
IWYU only analyses real translation units, never the moc code CMake
generates from Q_OBJECT headers, so it strips includes that moc output
silently relies on. Where a type is passed by value (or nested in a
QList/QMap) in a signal or slot signature, the generated moc code needs
the complete type; those includes are kept with an explanatory
`// IWYU pragma: keep`. The same holds for Q_DECLARE_METATYPE targets
such as QSqlDatabase.
A handful of includes are added by hand where callers brace-initialize a
const-reference parameter, which also requires the complete type.
Applied as a single commit on purpose: stripping includes from the
libcockatrice_* headers is precisely what breaks the client, test and
server translation units, and the fixes for those live in the same
translation units. Splitting by component would therefore leave
intermediate commits that do not compile.
All 426 modified Q_OBJECT/Q_GADGET headers were verified to be
moc-self-contained by compiling each generated moc_*.cpp standalone;
several built only because of moc include ordering in
mocs_compilation.cpp before the keep-includes were added.
* [Tooling] Emit public Qt headers and fix IWYU fallout CI found
Three problems the local build could not see, all because a green local
build only proves the configuration it was run under.
Private Qt headers. IWYU emitted private, lowercase Qt headers, and
qtypes.h does not exist at all before Qt 6.9, so the Debian 12 and
Ubuntu 24.04 jobs on Qt 6.4.2 failed to compile. The generator only
emitted a mapping when the capital header name mirrored the private one
(QAbstractAnimation for qabstractanimation.h), so qtypes.h, which is
wrapped by QtTypes instead, got no mapping and kept the private
spelling. Map it explicitly to <QtTypes>, which is public since Qt 5 and
provides the same typedefs either way, and rewrite the 100 affected
includes.
Do not generalize this to "prefer any Qt* wrapper". The table is
generated from whichever Qt is installed, and the wrappers are not
equally old: QtLogging only appeared in Qt 6.8 and QtMinMax in 6.5, so
that heuristic trades one breakage for another. Every other lowercase
header still in the tree has shipped publicly since Qt 5, so those are
left alone.
Platform-guarded includes. IWYU only sees the preprocessor configuration
it is handed, so it dropped an include that only the macOS branch of
main.cpp needs, and the macOS jobs failed on an undeclared
UrlSchemeEventFilter. Restore the include. Force-compiling every file
that carries a platform guard under Q_OS_MAC, Q_OS_MACOS and __APPLE__
reports nothing else, so the macOS branches are covered rather than
assumed.
shellcheck. Drop a leftover variable and stop assigning the
include-what-you-use lookup, which was only ever an existence check.
* [Tooling] Map Qt 6.5+ split headers to QtGlobal for Qt 6.4 compatibility
IWYU emitted QtTypes, QtPreprocessorSupport, QtVersionChecks, QtAssert,
QtMinMax, QtClassHelperMacros, QtEnvironmentVariables and QtTranslation.
Qt 6.5 split those declarations out of qglobal.h into separate headers, so
neither the Qt* wrapper nor the q*.h private name exists on the oldest Qt we
support (6.4.2), and Debian 12 / Ubuntu 24.04 fail with "No such file or
directory". qtypes.h is worse still: it only appeared in Qt 6.9.
QtGlobal is the one spelling that works across the whole supported range. It
has been public since Qt 5, defines these declarations itself up to Qt 6.4, and
includes the split headers from Qt 6.5 on, so redirect all eight private
headers to it and collapse the affected includes in the tree.
Curated overrides now take precedence over the name-mirroring rule, which
cannot know about Qt version floors because the mapping is generated from
whatever Qt is installed. That rule was mapping qtversionchecks.h to
<QtVersionChecks> even with an override present, because the override was only
consulted as a fallback. A curated key that matches nothing is now a hard
error rather than a silent fallback, and the redirected headers are no longer
advertised as the home of a symbol.
Verified every Qt include in the tree resolves against the real
qt6-base-dev 6.4.2 package: 15 distinct includes, 0 missing.
* [Tooling] Map QOverload to QtGlobal and drop the QMetaMethodArgument include
Two more headers IWYU emitted do not exist on Qt 6.4.2:
QOverload was split out of qglobal.h in Qt 6.5, so on 6.4.2 it is declared in
qglobal.h directly and the standalone header is a hard error. QtGlobal covers
both, so map qoverload.h to it like the other 6.5+ split headers.
QMetaMethodArgument is never referenced anywhere in the tree. It is a bare
forwarder for qobjectdefs.h that our generator advertised as a class symbol,
which is how IWYU came to add the include at all. The symbol mappings are
generated from header names, so they cannot tell a forwarder that happens to be
capitalised from a real class; removing the unused include is the fix, and the
generic case stays a known limitation of the generator.
Extending the audit to every Q* header (an earlier pass only matched headers
starting with "Qt" or lowercase q*.h, so it missed all of them) against the real
qt6-base-dev, qt6-multimedia-dev, qt6-declarative-dev, qt6-svg-dev and
qt6-websockets-dev 6.4.2 packages: 341 distinct Qt includes, 0 missing.
* [Game] Give moc the complete CardItem type in player_actions.h
player_actions.h forward declares CardItem but passes QList<CardItem *> by
value in actPlay/actPlayFacedown/actHide. moc instantiates the metatype for
those parameters, and on Qt 6.4 that expands to sizeof(CardItem), so Ubuntu
24.04 fails with "invalid application of 'sizeof' to incomplete type
'CardItem'". Newer Qt tolerates the incomplete type, which is why only the
6.4.2 jobs failed.
Include the defining header with an IWYU pragma keep, matching how the other
MOC-required includes in this area are handled.
* [Game] Give moc the complete ServerInfo_Card type
player_logic.h, game_scene.h, view_zone.h and view_zone_widget.h forward
declare ServerInfo_Card but pass QList<const ServerInfo_Card *> through signal
and slot signatures. moc instantiates the metatype for those parameters, which
Qt 6.4 expands to sizeof(ServerInfo_Card), so the connect in game_scene.cpp
fails on Debian 12 and Ubuntu 24.04 with "invalid application of 'sizeof' to
incomplete type 'const ServerInfo_Card'". Newer Qt tolerates it, so only the
6.4.2 jobs failed.
Include the generated protobuf header with an IWYU pragma keep, following the
pattern already used for ServerInfo_Player and ServerInfo_User in
game_event_handler.h.
* [Client] Complete the types Qt 6.4 needs for connect and moc
Qt 6.4 expands QMetaType::fromType on the normalised argument list of a
PMF connect and on moc-generated metatypes, which requires sizeof on every
signal and slot parameter. Newer Qt tolerates forward declarations there,
so only the Debian 12 and Ubuntu 24.04 jobs failed, with
"invalid application of 'sizeof' to incomplete type" for QMouseEvent,
QPrinter and GameReplay.
Replace the forward declarations with the real includes:
- QMouseEvent in the nine widget headers that pass it through signals
- QPrinter in abstract_tab_deck_editor.cpp, for the actPrintDeck lambda
- game_replay.pb.h in tab_replays.h. GameReplay is a protobuf message, so
the project's game/replay.h only forward declares it and cannot supply
the complete type.
Found by recompiling all 1918 project TUs against the Debian Qt 6.4.2
headers, which reproduces those two CI jobs locally in one pass.
* [Client] Complete Release and QMenu for Qt 6.4 moc and connect
Continuing the Qt 6.4 complete-type sweep:
- window_main.cpp connects ClientUpdateChecker::finishedCheck to
MainWindow::checkClientUpdatesFinished, whose Release * parameter made
moc 6.4 instantiate the metatype. GameReplay turned out to be a protobuf
message, so Release likewise needed release_channel.h in the three
headers declaring those signals: window_main.h, dlg_update.h and
client_update_checker.h.
- moc 6.4 also needs sizeof(QMenu) for the QList<QMenu *> carried by
TabSupervisor::setMenu and MainWindow::updateTabMenu.
Verified by regenerating moc output with the Debian moc 6.4.2 for all 434
Q_OBJECT/Q_GADGET classes and compiling it, plus recompiling all 1612
source TUs, against the Qt 6.4.2 headers: no remaining failures.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
TabEdhRecMain never emitted tabTextChanged, so TabSupervisor::updateTabText
was never called and the tab kept the title it was created with.
addEdhrecMainTab() creates the tab without a card, so every search left the
title reading "EDHRec: " with no card name for the rest of the tab's life.
addEdhrecTab() sets the card before myAddTab(), which is why the older
TabEdhRec tab was never affected.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Translations] Load Qt's own translations and build ours with the binary
Three separate bugs kept translations from working.
installNewTranslator() loaded the application translation into
qtTranslator. QTranslator discards its previous contents on every load, so
that wiped the qt_<lang>.qm loaded one line earlier and installed an empty
translator instead. Qt's own strings were never translated on any platform.
Give each file its own translator.
QLibraryInfo::path(TranslationsPath) was the only directory searched for
Qt's translations. A relocated Qt can report a path that does not exist: the
KDE Flatpak runtime reports /usr/translations while its files live in
/usr/share/qt6/translations. Probe the usual install layouts and only hand
back the ones that exist.
The .qm files were attached to the default build target only, so building
the executable on its own left it without any translations and the language
selector empty. Attach them to the target as well.
Move the shared lookup into TranslationLoader so client and Oracle cannot
drift again, and have it report a missing translation at debug level. An
unset language is the default for English, which has no file to begin with,
and warning about it only added noise to every fresh debug log.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Tests] Define main() in translation_loader_test
Without a GTest installation the fallback in tests/CMakeLists.txt links
only gtest, not gtest_main, so the test has to bring its own main(). Every
other test already does; only this one relied on the distribution package
and failed to link on CI.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Tests] Put the translation loader fixture next to the test executable
A multi-config generator (MSBuild, Xcode) writes the executable to a
per-config subdirectory while the .qm stays in the current binary
directory, so applicationDirPath() never saw the fixture and
ABuildTreeFindsTheTranslationsNextToTheExecutable failed on Windows.
* [Translations] Depend on Qt's lrelease target and skip the loader test without lrelease
Attaching the .qm custom commands to a second custom target races with
Qt's own <Project>_lrelease target on MSBuild/Makefiles and is rejected
by Xcode, so depend on Qt's target instead. Only build the translation
loader test when Qt6::lrelease is available.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
Co-authored-by: ZeldaZach <zahalpern+github@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The game log translates its sentences, zone names and counter names, but every
card name was rendered as the raw canonical name straight from the server.
The card board and the card info text already went through
CardLocalization::displayName(), so the log was the odd one out.
cardLink() used one string for both the card:// href and the visible label,
which left no room for a localized label. Split the two: the href keeps the
canonical name because ChatView::mousePressEvent feeds it straight to
showCardInfoPopup for lookup, while the label goes through the display
language. Same split for [card] chat tags in ChatView::appendCardTag.
Sanitize the name while building the link. logUndoDraw already did this by
hand and had drifted into its own non-italic copy of cardLink(); route it back
through the helper.
Add CardLocalization::displayNameFor() for callers that only hold a canonical
name and need no CardInfoPtr of their own.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Theme] Carry Default theme directory customizations over to System
The Default -> System rename rewrote the stored theme name but left the
user profile's themes/Default directory behind, so anyone who had
customized zone graphics, stylesheets or palettes there silently lost them.
Migrate the directory alongside the name. Rename it when System does not
exist yet, otherwise merge file by file without overwriting what System
already has, then drop the legacy directory. Users who already ran the
rename get their customizations back on the next start, and a hand-made
Default folder of a user on another theme is left untouched.
* [Tests] Give theme_migration_test an explicit main and gate it on WITH_CLIENT
The build machines that cannot find GTest fall back to the downloaded
googletest, which links the bare gtest library without gtest_main. Every
other test defines its own main() for that reason; this one did not, so
Debian 13, Fedora 44 and Ubuntu 26.04 failed to link it.
The test compiles client sources, so -DTEST=1 -DWITH_CLIENT=0 has no way
to build it. Gate the subdirectory on WITH_CLIENT like the other
client-source test.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
The games list in the profile hover card offered "Join game" and
"Spectate", but UserInfoPopup::joinGameRequested was only forwarded to an
identical UserListWidget::joinGameRequested signal that nothing connected
to. Clicking either entry sent no command, opened no tab and showed no
error.
Route the request to TabSupervisor, which owns the join flow. The popup's
games list spans every room the user is in, so the target room may not be
open yet; IntentJoinServerGame already joins the room first and retries,
which is why this goes through the intent rather than straight to
GameSelector.
Extract the intent construction that joinReportGame already had into a
shared joinGameByIntent helper instead of duplicating it.
Also stop disabling "Join game" for games that are full or already
running. GameSelector::joinGame() offers to join as a spectator in that
case, so a greyed-out entry read as a dead button; GameSelector's own
context menu never disables it either.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Client] Add option to use game time in game log timestamps (#2202)
Game log timestamps showed the local clock time, which is meaningless in
a replay or in a log read after the fact — nobody can tell when a play
happened relative to the game.
Add a "Use game time instead of local time in game logs" setting. When
enabled, the message log stamps entries with the elapsed game time
(HH:MM:SS since the game started); regular chat views keep the local
clock.
* [Game] Move formatElapsedTime helper out of public slots (#2202)
The message log needs the HH:MM:SS formatter used by the game timer, but
it lives in GameState's public slots, tying a pure formatting helper to
the signal-slot machinery.
Move formatElapsedTime into a public section of GameState so the log can
call it directly, and add the missing trailing newline to game_state.cpp.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Installer] Abort instead of installing over leftover runtime files
The silent upgrade could leave a mix of old and new Qt DLLs next to the new
executables, which is what makes the next start fail with "The procedure entry
point X could not be located in the dynamic link library ...Qt6QuickWidgets.dll"
or "The code execution cannot proceed because Qt6Multimedia.dll was not found".
The check that was supposed to catch this probed only Qt6Core.dll and
Qt6Network.dll, so it was blind to the two files the reports name. It also only
warned and then installed anyway, so a caught problem was still compounded.
- Scan every file the purge targets with FindFirst, which takes the same
wildcards the Delete/RMDir calls use, instead of two hardcoded names
- Widen IsExistingCockatriceInstall from Qt6Core.dll to Qt6*.dll so a directory
left holding only some Qt modules is still recognised as an install
- Gate the scan on that, so a first-time install into a directory that happens to
hold unrelated Qt5 DLLs is not turned into an abort
- Abort with a non-zero exit code instead of warning, so the new payload is
never written next to files of the previous version
- Skip the message box on an unattended /S install, which would block on it
forever, matching the two existing abort sites
* [Installer] Cancel a silent upgrade when the running-instance guard fails
The running instance guard is what stands between a locked install directory
and the mixed Qt DLL set that breaks the next start, but it failed open in two
places and let the install carry on anyway:
- powershell.exe could not be started: the check warned and returned
- the process lookup answered "could not determine": WaitForAppToClose returned
on $R0 != 1, so "unknown" was treated exactly like "not running"
On a silent upgrade there is nobody at the keyboard to close the applications in
response to that warning, which is precisely the state in which the launcher was
still holding its Qt DLLs mapped.
- Treat "could not determine" as its own state rather than folding it into "not
running", in all three places WaitForAppToClose inspects the result
- Cancel a silent upgrade in both cases; keep an interactive install on the
existing warn-and-continue path, since refusing there would leave an installer
that can never complete and the stale-runtime check still applies
- Route every abort through one AbortInstall so the dialog-vs-silent decision
lives in a single place, and stop the two pre-existing warning dialogs that a
/S install without /R would have blocked on forever
* [Installer] Wait on the process that launched the update installer
The in-app updater starts the installer and only then quits itself, so the
installer has to decide whether the launching process is still alive while it is
running. It did that by polling powershell.exe for an image name under the
install directory, which is a proxy for the real question - is the process that
started me still holding its Qt DLLs mapped - and a proxy that can answer "not
running" or "could not determine" while the process is very much alive. Either
answer let the install carry on into the mixed Qt DLL set.
Take /PID= from the command line and wait on that process handle directly:
- OpenProcess with PROCESS_SYNCHRONIZE only, which is all that is needed to wait
and the least likely to be refused for an unrelated reason
- WaitForSingleObject for the same 60 s the name based check allows, so a client
slow to flush its card database is not squeezed
- Treat an unopenable handle as "already exited", which is what it means for a
dead process
- Cancel the upgrade if the launcher has not exited, rather than replacing files
it is still using
Run it before EnsureAppsNotRunning, not after: it needs no powershell.exe, so it
is the one check that still works on a machine where the process lookup cannot
run, and it is what lets that case keep upgrading instead of being cancelled by
what was only ever a proxy for it. Keep an older client, which passes no /PID=,
on the existing path.
An older installer ignores /PID=, and /D= stays the last parameter either way.
* [Client] Pass our process id to the update installer
The updater starts the installer and only then quits itself, so the installer
begins replacing files while this process still has every Qt runtime DLL mapped,
and a mapped DLL cannot be deleted. It has to work out that it must wait, and
until now all it had was its own image name to go on.
Pass /PID= so it can wait on this process directly instead of guessing, and
cancel the update rather than replace files this process is still using. /D=
stays the last parameter, which NSIS requires.
* [CI] Fail the build when the installer is missing a Qt runtime DLL
The Windows payload is assembled by globbing every *.dll out of the build output
directories, so nothing but luck decides whether a module the client links
statically - Qt6Multimedia.dll, Qt6QuickWidgets.dll - reaches the installer at
all. A module that is left out is invisible until a user installs an update and
the client then refuses to start, with "The code execution cannot proceed because
Qt6Multimedia.dll was not found" or a missing procedure entry point in a
Qt6QuickWidgets.dll carried over from the previous version.
Derive the required list from SERVATRICE_QT_MODULES, COCKATRICE_QT_MODULES and
ORACLE_QT_MODULES, the same module lists the targets are actually built against,
so it cannot drift from what the binaries need, and write it next to the CMake
cache as required-qt-runtime.txt. Add the platform plugin, without which the
client cannot start at all.
Assert it in the existing 7-Zip pass that already rejects build-tree artifacts,
comparing exact relative paths so a same-named file in the wrong directory does
not satisfy a requirement, and normalizing the archive's own separators so it
does not matter which one NSIS chose. Missing manifest, missing entry or missing
7-Zip-independent failures all stop the build rather than warn, which is the
whole point: the previous check already failed hard for artifacts, and a warning
here is what let a broken package ship.
* [Client] Point the Windows qt.conf translations path at the real one
The Windows qt.conf carried Translations = Resources/translations, copied from
the macOS bundle block, but the Windows install has no Resources directory: the
.qm files are installed to COCKATRICE_WIN32_QM_INSTALL_DIR, which is
translations, and main.cpp looks for them at applicationDirPath() +
"/translations". Leave the macOS value alone, which is correct for it.
* [Installer] Close the /? help string so the installer compiles again
The last line of the continued MessageBox string ended in $\n\" instead of $\n".
NSIS reads \" as an escaped quote, so the string never terminated and makensis
rejected the script, which took the CPack packaging step down with it. The
Windows build failed on "Problem running NSIS command" before ever reaching the
new required-runtime check.
* [Installer] Give FindFirst the two arguments it takes
FindFirst takes the pattern and the output variable and leaves the search
handle in $0. There is no handle argument, and FindNext taking only a variable
is the giveaway: the handle has to live somewhere implicit, and it is $0.
The call passed three arguments, so makensis rejected the script and the CPack
packaging step failed with "Problem running NSIS command" before ever reaching
the required-runtime check.
This was not only a compile error. The handle variable was never populated, so
the stale-runtime check that commit 844a1a9b added could never see a match: it
probed $StaleFindHandle, which stayed empty, and the widened wildcard scan over
the purge set was a no-op. Pass the pattern, test $0, and close $0. No FindNext
is needed, because $R0 is a yes/no flag rather than a list.
Drops the now unused StaleFindHandle Var.
* [CI] Dump the NSIS compile log when packaging fails
CPack reports a failed makensis run as "Problem running NSIS command" plus a
path to NSISOutput.log, and that log exists only on the runner, so the actual
compile error never reaches the workflow output. An installer script that does
not compile is therefore undiagnosable from CI alone, which is exactly how a
three-argument FindFirst cost two build cycles here.
* [CI] Run the NSIS log dump under bash
The dump step used the default pwsh shell, where `find` resolves to Windows'
text-search find.exe rather than a file finder, so it died with "FIND:
Parameter format not correct" instead of printing anything. Pin shell: bash.
* [Installer] Test the error flag with LogicLib's ${Errors} condition
makensis rejected the script with "macro _If requires 4 parameter(s), passed
2" at the ClearErrors/OpenProcess pair. A ${If} condition cannot be a bare
$Var: LogicLib needs either a comparison or one of its flag macros, and a lone
$Errors splits into too few arguments for _If. ${Errors} is the documented flag
condition and is the same mechanism as the ${Silent} test already used here.
The dump step earned its keep - this took one run to locate rather than another
round of guessing.
* [Installer] Give FindFirst its handle back and stop blocking silent aborts
makensis: "FindFirst expects 3 parameters, got 2. Usage: FindFirst
$(user_var: handle output) $(user_var: filename output) filespec".
The previous commit was wrong about this. FindFirst does take the handle
explicitly, and FindNext finds it in $0 as a side effect of the FindFirst that
set it, which is why FindNext takes only a variable. The three-argument form the
stale-runtime check was written with was correct all along, and the two-argument
form broke it. Restored, along with Var StaleFindHandle.
That same commit inverted the interactive check in both abort paths.
`${If} $ReinstallMode = 1` with `${OrIfNot} ${Silent}` shows the message box
exactly when the install is the silent /R upgrade - the one case the comment
says must never block, since nothing will dismiss it and the installer hangs
until it is killed. Replaced with `${IfNot} ${Silent}`, so the box appears only
for an interactive install and a silent abort goes straight to SetErrorLevel.
Audited every construct added by this branch against master, which compiles:
all 9 LogicLib condition shapes have a master precedent, FindFirst is the only
FindFirst and is back to 3 arguments, Function/Section and block depth balance,
and the 12 lines with an odd number of quotes are the same pre-existing string
continuations master has.
* [CI] Strip the CR 7-Zip leaves on its paths before comparing them
The installer check reported all 14 required files missing, including
Qt6Core-era basics an 82 MB package obviously contains, so the payload was fine
and the comparison was broken. 7-Zip writes CRLF on Windows, sed 's/^Path = //p'
kept the \r on every line, and grep -qxF matches whole lines - so every entry
carried a trailing \r and nothing matched. The build-tree artifact check next to
it uses an unanchored grep -E and was unaffected, which is what made the two
checks disagree.
Reproduced locally against a 7z archive with CRLF injected: the old pipeline
matches nothing, the new one finds the present files and reports only the
genuinely absent one.
Also match case-insensitively. That is how the installed tree behaves - the
uninstaller clears both $INSTDIR\plugins and $INSTDIR\Plugins - so a
Plugins vs plugins difference in the archive is not a missing file.
* [CI] Print both sides of the installer path comparison, and strip its prefixes
The check still reported all 14 files missing after the CR fix, so the CR was
not the cause and the real difference between 7-Zip's output and the manifest
is still unknown. Inferring it again is what produced two wrong fixes in a row,
so on failure the check now prints the required list and the first 40 paths
7-Zip lists. A bare "missing" line cannot distinguish a missing file from a
misread listing, and twice it was the latter.
Also strip $INSTDIR/, $OUTDIR/, $PLUGINSDIR/ and ./ prefixes from the listed
paths. Written as separate -e expressions rather than one alternation: with |
as the sed delimiter an escaped \| is a literal pipe, so the alternation
version silently matched nothing at all.
Verified against a 7z archive fed through seven path shapes - plain, Windows
backslashes with CRLF, each prefix, lowercase plugins, and all of them combined
- each reporting only the file genuinely absent from the archive.
* [CI] Strip CR from the manifest side of the installer path check
The dump added last run settled it: 7-Zip listed 209 paths including Qt6Gui.dll,
Qt6Multimedia.dll and Plugins/platforms/qwindows.dll verbatim, as plain relative
paths with no prefix - so the archive side was already clean and the previous
fix was right to strip CR there but not sufficient.
The trailing CR was on the other side. CMake writes required-qt-runtime.txt
with the platform line ending, so on Windows $entry arrived as
"Qt6Gui.dll\r", which cannot whole-line match a clean "Qt6Gui.dll". Every
entry then read as missing while the listing held it, which is precisely what
the two sides of the dump showed.
Reproduced locally with a CRLF manifest against a 7z archive: without the strip
every entry is reported missing, with it only the genuinely absent file is.
Checked across plain paths, Windows backslashes with CRLF, $INSTDIR, $OUTDIR,
$PLUGINSDIR, ./ and / prefixes.
The literal $INSTDIR-style prefixes are written as [$] bracket expressions so
shellcheck does not read them as unexpanded variables; that clears the SC2016
that failed the format job, without a disable directive.
* [CI] Reword the bracket-expression comment so the linter stops parsing it
A comment whose first word is shellcheck is read as a directive, so
"# shellcheck does not read these as unexpanded shell variables" was parsed as
a directive named "does" and failed with SC1073 and SC1072 - which cascaded into
the SC1009 on the else clause above it. The SC2016 it was meant to document is
already gone now that the prefixes are written as [$] bracket expressions, so
the note only had to stop looking like a directive.
* [CI] Indent the diagnostic path lists without sed, and undo comment drift
SC2001 asks for parameter expansion instead of sed 's/^/ /'. Echoing
${manifest//$'\n'/$'\n '} prepends the two spaces directly and keeps the
diagnostic block readable. Both occurrences are converted; only the one reading
from a here-string was reported, but the piped one has the same shape.
Also restore master's wording of the build-tree artifact comment. An earlier
edit inserted "in " ("not the install)" became "not in the install)"). It has no
functional effect, but it was an unintended change to a line this branch has no
reason to touch, and .ci/compile.sh now differs from master only by additions.
* [CI] Ship the Qt module runtimes the Windows package check requires
The installer check reports Qt6Concurrent.dll and Qt6ShaderTools.dll missing
from the Windows package. Both are genuinely absent, not another comparison
artifact - the manifest now matches 12 of 14 entries and the dual-list
diagnostic shows the other two are not in the archive at all.
Nothing deploys them. fixup_bundle copies only the DLLs it finds in
Cockatrice.exe's import table, and the *.dll glob only sees what was already
copied next to the executable. Qt6::Concurrent is linked but never imported,
because QtConcurrent::run and QFutureWatcher are header-only in Qt 6, so the
linker drops the import; Qt6::ShaderTools is skipped the same way. The package
has shipped without them and the client still starts, but that is luck rather
than intent, and required-qt-runtime.txt asks for both.
Install the runtime of every linked module explicitly so the installer contains
what the check demands. An unused DLL costs a few hundred kilobytes; a missing
one is what stops the client starting after an update, and stays invisible until
then.
Derive both the manifest and the install list from the imported target's TYPE,
so a statically linked Qt module - which ships no runtime at all - is skipped by
each and the two can never disagree. Scope the manifest to the client rather
than the union across server, client and Oracle: only the client is packaged on
Windows, and the other two module lists are subsets of the client's, so this
changes nothing today.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
The local deck and replay browsers list every file in the folder, so a
stray file (e.g. a PNG screenshot of a deck) can be parsed as a garbage
plaintext deck, opened in the editor, and uploaded. The server also
accepts oversized deck payloads with only silent truncation.
Hide files that are not in a supported deck/replay format in the two
QFileSystemModel views (directories stay visible), skip them when opening,
reject them when uploading, and make the server reject deck uploads larger
than MAX_FILE_LENGTH instead of truncating them.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Client] Exit deterministically when launching the update installer
* [Client] Only quit when the main window close is accepted
Wait for MainWindow::close() to be accepted before quitting after the
update installer is launched. When the close is vetoed (a running card DB
update, open games, or an unsaved deck), keep running and tell the user
the installer is already waiting, instead of exiting over their answer.
Also fix the comment so it does not claim settings are saved on the vetoed
path.
* [Client] Only quit on update when the shutdown actually ran
MainWindow::closeEvent has a static re-entrancy guard that returns early on a
second close event, leaving it in its default accepted state. DlgUpdate reached
from a nested event loop while a shutdown prompt was up could then get close()
== true with no shutdown work done (no settings flush, no tab shutdown) and tear
down the process behind an unanswered prompt.
Add MainWindow::closeForUpdate(), which reports false when a close is already in
progress or was vetoed, and gate the update-exit on it.
* [Client] Use QCoreApplication::exit() to leave on update
QCoreApplication::quit() only asks the application to quit and can be
interrupted if a top-level window refuses its close event. On the update path
the process must actually leave so its Qt DLLs are unlocked before the installer
starts replacing files - apply the exit(0) variant both on the accepted-close
path and the fallback where no MainWindow was ever closed.
* [Client] Warn that the update installer force-closes within a minute
Telling the user Cockatrice just 'stays open for now' is misleading: the
installer launched by #7308 waits only about 60 s for a graceful close before it
force-terminates the process, which would lose unsaved work. State the deadline
and tell the user to save and close before then.
* [Client] Warn that a vetoed update is cancelled, not forced
The warning on the veto path promised the opposite of what the installer
does: WaitForAppToClose in #7308 never force-closes cockatrice.exe
($AllowForceClose is 0), it aborts the update after 60 s so no unsaved data
is lost. A user who is told Cockatrice will be terminated in a minute hurries
to close it, only to find the update already given up.
State the real outcome instead: the installer waits about a minute for a
close, and the update is cancelled if that does not happen.
* [Client] Warn about a waiting installer without blocking it
CloseMatchingApps in #7308 sends the WM_CLOSE once and the wait loop only
polls IsAppRunning afterwards, but QMessageBox::warning is application modal
and QGuiApplicationPrivate::processCloseEvent drops spontaneous close events
for windows blocked by a modal widget. The veto path therefore swallowed the
installer's one and only request, so even a user who resolved the blocker and
closed the app still ended in the 60 s timeout.
Move the warning into warnInstallerIsWaiting() and show it modeless, so the
main window stays able to answer the installer while the message is up.
* [Client] Close the parent on the no-MainWindow update fallback
The fallback branch exits the process without closing anything, so it skips
the settings flush and tab shutdown this change exists to get in front of the
installer - the opposite of the fix if a call site ever stops passing a
MainWindow.
Close the parent widget first, like the code before this change did, and only
then leave unconditionally: the parent is the one that knows how to shut down
gracefully, and the deterministic exit is what the installer needs.
* Add override.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
On a clean install, checkUnknownSets() fired before the onboarding wizard,
so the legacy 'all the sets have been enabled' welcome and the Manage Sets
dialog appeared first - with no card data there are no sets to manage yet.
Defer the set check until after the wizard closes, suppress its dialogs
while onboarding runs, and enable newly downloaded sets silently instead.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Server/Client] Unify report categories into one shared source of truth
* Lont.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
Ctrl+scroll over a card display now resizes the cards everywhere a card
size slider is shown, matching the standard way of resizing content.
CardSizeWidget learns to forward Ctrl+wheel events onto its slider via an
event filter that is installed on the display container and, when present,
on the scroll area's content widget so the resize intercepts the wheel
event before the view scrolls. The existing slider valueChanged wiring
then rescales the displayed cards.
Applied to the visual deck editor (per card group, covering flat and
overlapped layouts), visual database display, printing selector, sample
hand, visual and public deck storage, Archidekt previews and EDHRec card
displays.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
CardPictureLoaderLocal compared the full file name (including extension)
of the custom-folder candidate against the extension-stripped base name of
each directory entry, so a CUSTOM subfolder image like
pics/CUSTOM/poker/1 of Hearts.png never matched and the card fell through
to the network. Compare the complete base names instead.
Add matcher tests covering CUSTOM subfolder resolution.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Theme] Revert default light palette and only include AppColors
* [Style] Also remove dark mode palette
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* tests: add manual picture loader benchmark against the real card hosts
(cherry picked from commit 588d71d3b9a8278ab98b847802908f322c1035b7)
* tests: address review on picture loader benchmark
- Sandbox via unique app/org names plus Linux-only XDG redirection, derive
the warm-cache probe and data path from SettingsCache, and bail out when the
temporary sandbox cannot be created.
- Run each pass with a fresh worker and shut workers down before reading the
429 counters, so the redirect cache is persisted and no worker thread can
outlive the stack-local counters or the installed message handler.
- Make s_activeCounters atomic and always forward log output when the previous
handler is the built-in (nullptr) one.
- Validate --timeout-min, scale the cached-pass budget with --count, honour the
first --url as the stress template, and add the missing trailing newlines.
- Zero-initialise SettingsCache members so the benchmark mock cannot
dereference an indeterminate pointer.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [PictureLoader] Fix worker thread shutdown and cross-thread cache clearing
clearNetworkCache() ran directly on the UI thread while the worker thread
owned the disk cache and redirect cache, racing cache reads/writes. Make it
a worker-thread slot invoked via a blocking queued call when the thread is
running, so the 'Cached card pictures have been reset.' message is truthful.
The worker thread was also never quit()/wait()ed: both destructors only
deleteLater'd their objects, so Qt warned 'QThread: Destroyed while thread
is still running' and leaked a running loop at exit. Wire the worker's
finished() signal to its own deleteLater() (canonical worker-object
pattern), add shutdownThread() to stop the loop, and let CardPictureLoader
destroy the QThread only after wait() has returned.
* [PictureLoader] Guard cache teardown and stop blocking the UI thread
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [PictureLoader] Add user-configurable per-host request caps
Picture downloads were throttled to a uniform 10 requests/second per host
with no way to tune a specific server. A rate-limited API host (Scryfall
caps at 10 req/s) can trip 429s during bursts, and CDN hosts with no rate
limit were throttled needlessly.
Introduce developer-owned per-host caps that users can only ever lower,
never raise, exposed in the download settings page:
- DownloadSettings::DEVELOPER_HOST_CAPS sets the ceiling per host
(api.scryfall.com 9, cards.scryfall.io unlimited, others 10).
- A new hostRequestLimits setting stores user overrides in downloads.ini;
clampHostRequestLimit() bounds them to [1, devCap] so a user can reduce
api.scryfall.com to 5 but never raise it above 9.
- The picture worker seeds, halves on 429, and recovers its sustained
per-host allowance against the effective ceiling instead of the global
maximum, and skips per-host accounting entirely for unlocked hosts
(cards.scryfall.io) while global pacing and 429 backoff still apply.
- The deck editor settings page gains one spinbox per known host, each
clamped to its developer cap.
* [PictureLoader] Let unlocked hosts skip dispatch pacing; adjust limits per URL
Two refinements to the per-host request caps:
- Unlocked hosts (UNLIMITED_HOST_QUOTA, e.g. cards.scryfall.io) no longer
wait on the 100ms dispatch pacing or consume the global per-second quota.
dispatchQueuedRequest fires their queued requests back-to-back, bounded
only by their 429 backoff window and Qt's per-host connection pool, so
an unthrottled CDN is not artificially slowed.
- The deck editor download settings page replaces the static grid of one
spinbox per known host with an "Adjust Rate Limit" toolbar action on the
URL list. It picks the host out of the selected URL and clamps the entry
against the developer cap table (including for user-added URLs).
Also fixes a review finding: resetRequestQuota could write the
UNLIMITED_HOST_QUOTA sentinel (-1) into the sustained per-host quota when a
host became unlocked mid-run, permanently poisoning its allowance. Stale
entries for unlocked hosts are now dropped, and the per-second seed is
clamped against the effective ceiling so a lowered limit applies immediately.
* [PictureLoader] Cap unlocked host bursts and adapt them to 429s
* [PictureLoader] Store per-host limits readably and show them per URL
* [PictureLoader] Make dispatch and rate-limit bookkeeping key on the real host
Addresses ZeldaZach's round-4 review nits:
- Dispatch now resolves the cached-redirect chain before the in-flight gate,
so a redirect learned after a URL was queued can no longer bypass the
MAX_IN_FLIGHT_PER_HOST cap and drain the whole queue onto the redirect
target, which may carry its own developer cap. processSingleRequest does
the same so the allowance math keys on the host that is actually hit.
- The per-host in-flight slot is released when the reply is destroyed (with
the worker as the connection context) rather than on a 'finished'
connection bound to the work object, so an aborted reply or a work object
deleted while a reply is pending can never permanently shrink the fast
path's concurrency.
- storeSettings only prunes limits for hosts with neither a URL nor a
developer cap, so throttles on redirect targets (api.scryfall.com ->
cards.scryfall.io) survive URL removal.
- Unlocked hosts are offered 0..UNLOCKED_HOST_LIMIT_MAX (50) in the rate
limit dialog, matching clampHostRequestLimit() and the documented
hand-editable range, so values written into downloads.ini are no longer
silently rewritten on the next edit.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [PictureLoader] Seed per-host allowances on demand and skip hosts in 429 backoff
The quota reset re-filled every host's remaining allowance to a full
MAX_REQUESTS_PER_SEC as soon as the queue had a request for it, so a server
that was just rate limited could be hammered again at full speed immediately
after (or even during) recovery.
Only seed a host's allowance the first time it is dispatched in the current
second, seeded from its reduced sustained quota, and skip hosts still inside
their 429 backoff window entirely, handing the entry back to its worker so it
can wait the backoff out or fall through to another source instead of parking
in the queue with no reply pending. Deferrals wait on the host that is
actually blocking the request (cached-redirect targets and the reply host of
a 429) rather than the current card URL's host.
Rebased onto network-requests/request-pacing, which absorbed the earlier
pacing and dispatch-guard commits, and reuses its updateTimerState idle-429
recovery plumbing.
* [PictureLoader] Hand backed-off entries back across the whole dispatch tick
Review fixes on the 429-backoff skip:
- Hand a backed-off queue entry back to its worker via
scheduleDeferredRetry(host) instead of startNextPicDownload(), which was
looping on the pre-redirect host every 100ms tick whenever the queued URL
was a cached-redirect target whose redirect host was the one in backoff.
- Keep scanning the queue after a hand-back (--i; continue) so a backed-off
host at the head no longer consumes the entire dispatch tick, stalling
every healthy host further down.
- Emit imageRequestSucceeded(url) before makeRequest()'s cached-redirect
backoff return so the status bar reclaims the deferred URL's widget instead
of inflating the progress bar forever.
- Only spend a per-second allowance when makeRequest() actually issues a
request: it returns nullptr when the redirect target is backed off and the
work is handed back, so the slot would otherwise be wasted on a no-op.
- Gate the dispatch-time backoff skip on requestTouchesNetwork() so entries
served straight from the disk cache (e.g. with downloads disabled) are not
bounced into a 30-60s deferred retry for a host that 429'd earlier.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
