From e63b72fe110a3e81f653309f02767babfa3af9c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 11 Oct 2026 14:37:01 +0200 Subject: [PATCH 01/14] [Client] Manual deck collection sync with Sync Now action --- cockatrice/CMakeLists.txt | 1 + .../interface/deck_sync/deck_sync_manager.cpp | 510 ++++++++++++++++++ .../interface/deck_sync/deck_sync_manager.h | 133 +++++ .../widgets/tabs/tab_deck_storage.cpp | 80 +++ .../interface/widgets/tabs/tab_deck_storage.h | 29 + 5 files changed, 753 insertions(+) create mode 100644 cockatrice/src/interface/deck_sync/deck_sync_manager.cpp create mode 100644 cockatrice/src/interface/deck_sync/deck_sync_manager.h diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index d76d89211..5519d2f3b 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -28,6 +28,7 @@ set(cockatrice_SOURCES src/interface/deck_loader/deck_loader.cpp src/interface/deck_sync/deck_sync_profile.cpp src/interface/deck_sync/deck_sync_sidecar.cpp + src/interface/deck_sync/deck_sync_manager.cpp src/interface/widgets/dialogs/dlg_connect.cpp src/interface/widgets/dialogs/dlg_convert_deck_to_cod_format.cpp src/interface/widgets/dialogs/dlg_create_game.cpp diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp new file mode 100644 index 000000000..9fe76445b --- /dev/null +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -0,0 +1,510 @@ +#include "deck_sync_manager.h" + +#include "../deck_loader/deck_loader.h" +#include "deck_sync_sidecar.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace +{ +constexpr const char *SYNC_META_DIR = ".sync"; +constexpr const char *BACKUP_SUFFIX = ".sync-backup-"; + +QString serverPathFor(const QString &relativePath) +{ + QString path = relativePath; + if (path.endsWith(".cod", Qt::CaseInsensitive)) { + path.chop(4); + } + return path; +} +} // namespace + +DeckSyncManager::DeckSyncManager(QObject *parent) : QObject(parent) +{ +} + +void DeckSyncManager::setClient(AbstractClient *_client) +{ + client = _client; +} + +void DeckSyncManager::setProfile(const DeckSyncProfile &_profile) +{ + profile = _profile; +} + +void DeckSyncManager::setDeckPath(const QString &path) +{ + deckPath = path; +} + +void DeckSyncManager::setColorIdentityProvider(ColorIdentityProvider provider) +{ + colorIdentityProvider = std::move(provider); +} + +bool DeckSyncManager::isSyncing() const +{ + return syncing; +} + +void DeckSyncManager::syncNow() +{ + if (syncing) { + return; + } + if (client == nullptr || !profile.isValid() || deckPath.isEmpty()) { + emit syncFinished(0, 0, 0, 0, tr("Not connected to a server.")); + return; + } + QDir deckDir(deckPath); + if (!deckDir.exists()) { + emit syncFinished(0, 0, 0, 0, tr("Deck folder not found.")); + return; + } + + syncing = true; + uploadedCount = 0; + downloadedCount = 0; + unchangedCount = 0; + skippedCount = 0; + pendingUploads.clear(); + pendingDownloads.clear(); + ensuredDirs.clear(); + ensuredDirs.insert(QString()); + localFiles.clear(); + emit syncStarted(); + emit statusMessage(tr("Scanning local decks...")); + + sidecarPath = deckDir.filePath(SYNC_META_DIR + QString("/") + profile.key() + ".json"); + DeckSyncSidecar sidecar(sidecarPath); + if (!sidecar.load()) { + failSync(tr("Could not read sync state.")); + return; + } + + QDirIterator it(deckPath, QDir::Files | QDir::NoDotAndDotDot, QDirIterator::Subdirectories); + while (it.hasNext()) { + const QString absolutePath = it.next(); + const QString relativePath = deckDir.relativeFilePath(absolutePath); + if (relativePath.startsWith(SYNC_META_DIR + QString("/")) || + !absolutePath.endsWith(".cod", Qt::CaseInsensitive)) { + continue; + } + std::optional loaded = DeckLoader::loadFromFile(absolutePath, DeckFileFormat::Cockatrice); + if (!loaded) { + ++skippedCount; + continue; + } + LocalFile file; + file.absolutePath = absolutePath; + file.hash = loaded->deckList.getDeckHash(); + file.modified = QFileInfo(absolutePath).lastModified(); + localFiles.insert(relativePath, file); + } + + Command_DeckList cmd; + PendingCommand *pend = AbstractClient::prepareSessionCommand(cmd); + connect(pend, &PendingCommand::finished, this, &DeckSyncManager::onListFinished); + client->sendCommand(pend); +} + +void DeckSyncManager::collectServerFiles(const ServerInfo_DeckStorage_Folder &folder, + const QString &prefix, + QMap &files, + QSet &dirs) +{ + for (const auto &item : folder.items()) { + const QString name = QString::fromStdString(item.name()); + if (item.has_folder()) { + const QString path = prefix.isEmpty() ? name : prefix + "/" + name; + dirs.insert(path); + collectServerFiles(item.folder(), path, files, dirs); + } else if (item.has_file()) { + const QString path = prefix.isEmpty() ? name : prefix + "/" + name; + const ServerInfo_DeckStorage_File &file = item.file(); + ServerFile entry; + entry.id = item.id(); + entry.guid = QString::fromStdString(file.guid()); + entry.hash = QString::fromStdString(file.hash()); + entry.lastModifiedSecs = file.has_last_modified() ? static_cast(file.last_modified()) : 0; + files.insert(path, entry); + } + } +} + +void DeckSyncManager::onListFinished(const Response &response, const CommandContainer & /*commandContainer*/) +{ + if (!syncing) { + return; + } + if (response.response_code() != Response::RespOk) { + failSync(tr("Could not read server decks.")); + return; + } + + QMap serverFiles; + QSet serverDirs; + const Response_DeckList &list = response.GetExtension(Response_DeckList::ext); + collectServerFiles(list.root(), QString(), serverFiles, serverDirs); + ensuredDirs.unite(serverDirs); + emit statusMessage(tr("Comparing %1 local and %2 server decks...").arg(localFiles.size()).arg(serverFiles.size())); + + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + const qint64 nowSecs = QDateTime::currentDateTime().toSecsSinceEpoch(); + + for (auto it = localFiles.begin(); it != localFiles.end(); ++it) { + const QString serverPath = serverPathFor(it.key()); + auto serverIt = serverFiles.find(serverPath); + if (serverIt == serverFiles.end()) { + UploadOp op; + op.relativePath = it.key(); + op.serverPath = serverPath; + op.deckId = -1; + op.guid = sidecar.contains(it.key()) && !sidecar.get(it.key()).guid.isEmpty() + ? sidecar.get(it.key()).guid + : QUuid::createUuid().toString(QUuid::WithoutBraces); + pendingUploads.append(op); + continue; + } + if (it.value().hash == serverIt.value().hash) { + DeckSyncRecord record = sidecar.get(it.key()); + if (record.guid.isEmpty()) { + record.guid = serverIt.value().guid; + } + record.remoteId = serverIt.value().id; + record.baseHash = it.value().hash; + record.lastSyncSecs = nowSecs; + record.deleted = false; + sidecar.set(it.key(), record); + ++unchangedCount; + continue; + } + const qint64 localSecs = it.value().modified.toSecsSinceEpoch(); + // A server row without last_modified predates sync: the local file wins. + // Otherwise strictly newer wins; ties go to the server. + if (serverIt.value().lastModifiedSecs == 0 || localSecs > serverIt.value().lastModifiedSecs) { + UploadOp op; + op.relativePath = it.key(); + op.serverPath = serverPath; + op.deckId = serverIt.value().id; + op.guid = serverIt.value().guid; + pendingUploads.append(op); + } else { + DownloadOp op; + op.relativePath = it.key(); + op.deckId = serverIt.value().id; + op.guid = serverIt.value().guid; + op.hash = serverIt.value().hash; + op.lastModifiedSecs = serverIt.value().lastModifiedSecs; + pendingDownloads.append(op); + } + } + + for (auto it = serverFiles.begin(); it != serverFiles.end(); ++it) { + const QString wantedPath = it.key().endsWith(".cod", Qt::CaseInsensitive) ? it.key() : it.key() + ".cod"; + if (localFiles.contains(wantedPath) || QFileInfo(deckPath + "/" + wantedPath).exists()) { + continue; + } + const QString relativePath = uniqueLocalPath(wantedPath); + if (relativePath.isEmpty()) { + ++skippedCount; + continue; + } + DownloadOp op; + op.relativePath = relativePath; + op.deckId = it.value().id; + op.guid = it.value().guid; + op.hash = it.value().hash; + op.lastModifiedSecs = it.value().lastModifiedSecs; + pendingDownloads.append(op); + } + + sidecar.save(); + processNext(); +} + +QString DeckSyncManager::parentDir(const QString &path) +{ + const int slash = path.lastIndexOf('/'); + return slash == -1 ? QString() : path.left(slash); +} + +void DeckSyncManager::processNext() +{ + if (!syncing) { + return; + } + if (!pendingUploads.isEmpty()) { + processNextUpload(); + } else if (!pendingDownloads.isEmpty()) { + sendDownload(pendingDownloads.first()); + } else { + finishSync(); + } +} + +void DeckSyncManager::processNextUpload() +{ + const UploadOp op = pendingUploads.first(); + const QString dir = parentDir(op.serverPath); + if (!ensuredDirs.contains(dir)) { + sendMkDirFor(dir); + return; + } + pendingUploads.removeFirst(); + sendUpload(op); +} + +void DeckSyncManager::sendMkDirFor(const QString &dir) +{ + QStringList parts = dir.split('/'); + QString made; + for (const QString &part : parts) { + const QString candidate = made.isEmpty() ? part : made + "/" + part; + if (!ensuredDirs.contains(candidate)) { + Command_DeckNewDir cmd; + cmd.set_path(made.toStdString()); + cmd.set_dir_name(part.toStdString()); + PendingCommand *pend = AbstractClient::prepareSessionCommand(cmd); + pend->setExtraData(dir); + connect(pend, &PendingCommand::finished, this, &DeckSyncManager::onEnsureDirFinished); + client->sendCommand(pend); + return; + } + made = candidate; + } + processNextUpload(); +} + +void DeckSyncManager::onEnsureDirFinished(const Response &response, + const CommandContainer & /*commandContainer*/, + const QVariant &extraData) +{ + if (!syncing) { + return; + } + if (response.response_code() != Response::RespOk) { + failSync(tr("Could not create server folder.")); + return; + } + const QString dir = extraData.toString(); + QStringList parts = dir.split('/'); + QString made; + for (const QString &part : parts) { + made = made.isEmpty() ? part : made + "/" + part; + if (!ensuredDirs.contains(made)) { + ensuredDirs.insert(made); + break; + } + } + processNextUpload(); +} + +void DeckSyncManager::sendUpload(const UploadOp &op) +{ + emit statusMessage(tr("Uploading %1...").arg(op.relativePath)); + std::optional loaded = + DeckLoader::loadFromFile(localFiles.value(op.relativePath).absolutePath, DeckFileFormat::Cockatrice); + if (!loaded) { + ++skippedCount; + processNext(); + return; + } + const QString deckString = loaded->deckList.writeToString_Native(); + if (deckString.length() > MAX_FILE_LENGTH) { + ++skippedCount; + processNext(); + return; + } + + Command_DeckUpload cmd; + if (op.deckId == -1) { + cmd.set_path(parentDir(op.serverPath).toStdString()); + } else { + cmd.set_deck_id(op.deckId); + } + cmd.set_deck_list(deckString.toStdString()); + cmd.set_guid(op.guid.toStdString()); + if (colorIdentityProvider) { + cmd.set_color_identity(colorIdentityProvider(loaded->deckList).toStdString()); + } + PendingCommand *pend = AbstractClient::prepareSessionCommand(cmd); + pend->setExtraData(op.relativePath); + connect(pend, &PendingCommand::finished, this, &DeckSyncManager::onUploadFinished); + client->sendCommand(pend); +} + +void DeckSyncManager::onUploadFinished(const Response &response, + const CommandContainer & /*commandContainer*/, + const QVariant &extraData) +{ + if (!syncing) { + return; + } + const QString relativePath = extraData.toString(); + if (response.response_code() != Response::RespOk) { + ++skippedCount; + processNext(); + return; + } + + const Response_DeckUpload &upload = response.GetExtension(Response_DeckUpload::ext); + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + DeckSyncRecord record = sidecar.get(relativePath); + if (upload.has_new_file()) { + const auto &file = upload.new_file(); + if (!file.file().guid().empty()) { + record.guid = QString::fromStdString(file.file().guid()); + } + if (!file.file().hash().empty()) { + record.baseHash = QString::fromStdString(file.file().hash()); + } + if (file.file().has_last_modified()) { + record.lastSyncSecs = static_cast(file.file().last_modified()); + } else { + record.lastSyncSecs = QDateTime::currentDateTime().toSecsSinceEpoch(); + } + record.remoteId = file.id(); + } else if (localFiles.contains(relativePath)) { + record.baseHash = localFiles.value(relativePath).hash; + record.lastSyncSecs = QDateTime::currentDateTime().toSecsSinceEpoch(); + } + record.deleted = false; + sidecar.set(relativePath, record); + sidecar.save(); + ++uploadedCount; + processNext(); +} + +QString DeckSyncManager::uniqueLocalPath(const QString &relativePath) const +{ + if (!QFileInfo(deckPath + "/" + relativePath).exists()) { + return relativePath; + } + QString base = relativePath; + if (base.endsWith(".cod", Qt::CaseInsensitive)) { + base.chop(4); + } + for (int i = 2; i < 1000; ++i) { + const QString candidate = base + " (" + QString::number(i) + ").cod"; + if (!QFileInfo(deckPath + "/" + candidate).exists()) { + return candidate; + } + } + return QString(); +} + +void DeckSyncManager::sendDownload(const DownloadOp &op) +{ + emit statusMessage(tr("Downloading %1...").arg(op.relativePath)); + Command_DeckDownload cmd; + cmd.set_deck_id(op.deckId); + PendingCommand *pend = AbstractClient::prepareSessionCommand(cmd); + pend->setExtraData(op.relativePath); + connect(pend, &PendingCommand::finished, this, &DeckSyncManager::onDownloadFinished); + client->sendCommand(pend); +} + +void DeckSyncManager::onDownloadFinished(const Response &response, + const CommandContainer & /*commandContainer*/, + const QVariant &extraData) +{ + if (!syncing) { + return; + } + const QString relativePath = extraData.toString(); + for (int i = 0; i < pendingDownloads.size(); ++i) { + if (pendingDownloads.at(i).relativePath == relativePath) { + pendingDownloads.removeAt(i); + break; + } + } + if (response.response_code() != Response::RespOk) { + ++skippedCount; + processNext(); + return; + } + + const Response_DeckDownload &download = response.GetExtension(Response_DeckDownload::ext); + DeckList deck; + if (!deck.loadFromString_Native(QString::fromStdString(download.deck()))) { + ++skippedCount; + processNext(); + return; + } + + const QString absolutePath = deckPath + "/" + relativePath; + if (localFiles.contains(relativePath)) { + const QString backupPath = + absolutePath + BACKUP_SUFFIX + QString::number(QDateTime::currentDateTime().toSecsSinceEpoch()); + if (!QFile::copy(absolutePath, backupPath)) { + ++skippedCount; + processNext(); + return; + } + } + QDir().mkpath(QFileInfo(absolutePath).absolutePath()); + if (!DeckLoader::saveToFile(deck, absolutePath, DeckFileFormat::Cockatrice)) { + ++skippedCount; + processNext(); + return; + } + + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + DeckSyncRecord record = sidecar.get(relativePath); + if (!download.guid().empty()) { + record.guid = QString::fromStdString(download.guid()); + } + if (!download.hash().empty()) { + record.baseHash = QString::fromStdString(download.hash()); + } else { + record.baseHash = deck.getDeckHash(); + } + record.lastSyncSecs = download.has_last_modified() ? static_cast(download.last_modified()) + : QDateTime::currentDateTime().toSecsSinceEpoch(); + record.deleted = false; + sidecar.set(relativePath, record); + sidecar.save(); + ++downloadedCount; + processNext(); +} + +void DeckSyncManager::failSync(const QString &error) +{ + syncing = false; + emit syncFinished(uploadedCount, downloadedCount, unchangedCount, skippedCount, error); +} + +void DeckSyncManager::finishSync() +{ + syncing = false; + emit statusMessage(tr("Sync complete: %1 uploaded, %2 downloaded, %3 unchanged, %4 skipped.") + .arg(uploadedCount) + .arg(downloadedCount) + .arg(unchangedCount) + .arg(skippedCount)); + emit syncFinished(uploadedCount, downloadedCount, unchangedCount, skippedCount, QString()); +} diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h new file mode 100644 index 000000000..99f2040fe --- /dev/null +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -0,0 +1,133 @@ +/** + * @file deck_sync_manager.h + * @brief One-shot two-way deck collection sync between local files and the server. + */ + +#ifndef DECK_SYNC_MANAGER_H +#define DECK_SYNC_MANAGER_H + +#include "deck_sync_profile.h" + +#include +#include +#include +#include +#include +#include +#include +#include + +class AbstractClient; +class CommandContainer; +class DeckList; +class Response; +class ServerInfo_DeckStorage_Folder; + +/** + * @class DeckSyncManager + * @brief Runs a manual whole-collection sync: local *.cod files vs server deck storage. + * + * Each syncNow() run loads the profile sidecar, scans the deck folder, fetches + * the server tree and mirrors both sides: unknown local decks upload, unknown + * server decks download, and decks changed on both sides resolve newer-wins + * (ties go to the server). Matching is path-based; a rename looks like a + * delete plus an add. Before a download overwrites a locally modified file, the + * local content is copied to a .sync-backup file next to it. + * + * Ownership: fire-and-forget per run, driven by PendingCommand round trips. + * No watcher, no auto triggers, no conflict UI — those arrive in later layers. + */ +class DeckSyncManager : public QObject +{ + Q_OBJECT +public: + /** @brief Computes the color identity string uploaded alongside a deck. May be empty. */ + using ColorIdentityProvider = std::function; + + explicit DeckSyncManager(QObject *parent = nullptr); + + void setClient(AbstractClient *_client); + void setProfile(const DeckSyncProfile &_profile); + void setDeckPath(const QString &path); + void setColorIdentityProvider(ColorIdentityProvider provider); + [[nodiscard]] bool isSyncing() const; + +public slots: + void syncNow(); + +signals: + void syncStarted(); + void statusMessage(const QString &message); + void syncFinished(int uploaded, int downloaded, int unchanged, int skipped, const QString &error); + +private slots: + void onListFinished(const Response &response, const CommandContainer &commandContainer); + void + onEnsureDirFinished(const Response &response, const CommandContainer &commandContainer, const QVariant &extraData); + void + onUploadFinished(const Response &response, const CommandContainer &commandContainer, const QVariant &extraData); + void + onDownloadFinished(const Response &response, const CommandContainer &commandContainer, const QVariant &extraData); + +private: + struct ServerFile + { + int id = -1; + QString guid; + QString hash; + qint64 lastModifiedSecs = 0; + }; + struct LocalFile + { + QString absolutePath; + QString hash; + QDateTime modified; + }; + struct UploadOp + { + QString relativePath; + QString serverPath; + int deckId = -1; + QString guid; + }; + struct DownloadOp + { + QString relativePath; + int deckId = -1; + QString guid; + QString hash; + qint64 lastModifiedSecs = 0; + }; + + void failSync(const QString &error); + void finishSync(); + void collectServerFiles(const ServerInfo_DeckStorage_Folder &folder, + const QString &prefix, + QMap &files, + QSet &dirs); + void processNext(); + void processNextUpload(); + void sendMkDirFor(const QString &dir); + void sendUpload(const UploadOp &op); + void sendDownload(const DownloadOp &op); + static QString parentDir(const QString &path); + QString uniqueLocalPath(const QString &relativePath) const; + + AbstractClient *client = nullptr; + DeckSyncProfile profile; + QString deckPath; + ColorIdentityProvider colorIdentityProvider; + bool syncing = false; + + QString sidecarPath; + QMap localFiles; + QSet ensuredDirs; + QList pendingUploads; + QList pendingDownloads; + int uploadedCount = 0; + int downloadedCount = 0; + int unchangedCount = 0; + int skippedCount = 0; +}; + +#endif // DECK_SYNC_MANAGER_H diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp index 3dfdc956b..90cd24290 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp @@ -2,6 +2,7 @@ #include "../../../client/settings/cache_settings.h" #include "../../deck_loader/deck_loader.h" +#include "../../deck_sync/deck_sync_manager.h" #include "../../pixel_map_generator.h" #include "../cards/additional_info/deck_color_identity.h" #include "../deck_share/deck_share_utils.h" @@ -29,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -177,6 +179,9 @@ TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, rightVbox->addWidget(shareBar); rightVbox->addWidget(serverDirView); rightVbox->addLayout(rightToolBarLayout); + syncStatusLabel = new QLabel(this); + syncStatusLabel->setWordWrap(true); + rightVbox->addWidget(syncStatusLabel); rightGroupBox = new QGroupBox; rightGroupBox->setLayout(rightVbox); @@ -228,6 +233,10 @@ TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, aPublishDeck->setIcon(QPixmap("theme:icons/lock")); connect(aPublishDeck, &QAction::triggered, this, &TabDeckStorage::actPublishDeck); + aSyncNow = new QAction(this); + aSyncNow->setIcon(qApp->style()->standardIcon(QStyle::SP_BrowserReload)); + connect(aSyncNow, &QAction::triggered, this, &TabDeckStorage::actSyncNow); + // Add actions to toolbars leftToolBar->addAction(aOpenLocalDeck); leftToolBar->addAction(aRenameLocal); @@ -239,11 +248,22 @@ TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, rightToolBar->addAction(aOpenRemoteDeck); rightToolBar->addAction(aDownload); + rightToolBar->addAction(aSyncNow); rightToolBar->addAction(aShareDecks); rightToolBar->addAction(aPublishDeck); rightToolBar->addAction(aNewFolder); rightToolBar->addAction(aDeleteRemoteDeck); + syncManager = new DeckSyncManager(this); + syncManager->setClient(client); + syncManager->setColorIdentityProvider( + [](const DeckList &deck) { return getDeckColorIdentity(deck, CardDatabaseManager::query()); }); + connect(syncManager, &DeckSyncManager::statusMessage, this, &TabDeckStorage::onSyncStatusMessage); + connect(syncManager, &DeckSyncManager::syncFinished, this, &TabDeckStorage::onSyncFinished); + if (currentUserInfo != nullptr) { + syncUsername = QString::fromStdString(currentUserInfo->name()); + } + retranslateUi(); QWidget *mainWidget = new QWidget(this); @@ -272,6 +292,8 @@ void TabDeckStorage::retranslateUi() aShareDecks->setText(tr("Share decks")); aPublishDeck->setText(tr("Publish/unpublish deck")); aOpenDecksFolder->setText(tr("Open decks folder")); + aSyncNow->setText(tr("Sync now")); + renderSyncStatus(); shareBar->retranslateUi(); if (shareBar->isVisible()) { onServerSelectionChanged(); @@ -299,6 +321,7 @@ QString TabDeckStorage::getTargetPath() const void TabDeckStorage::handleConnected(const ServerInfo_User &userInfo) { + syncUsername = QString::fromStdString(userInfo.name()); setRemoteEnabled(userInfo.user_level() & ServerInfo_User::IsRegistered); } @@ -316,6 +339,7 @@ void TabDeckStorage::handleConnectionChanged(ClientStatus status) void TabDeckStorage::setRemoteEnabled(bool enabled) { + remoteEnabled = enabled; aUpload->setEnabled(enabled); aOpenRemoteDeck->setEnabled(enabled); aDownload->setEnabled(enabled); @@ -323,6 +347,7 @@ void TabDeckStorage::setRemoteEnabled(bool enabled) aPublishDeck->setEnabled(enabled); aNewFolder->setEnabled(enabled); aDeleteRemoteDeck->setEnabled(enabled); + aSyncNow->setEnabled(enabled && !syncManager->isSyncing()); if (enabled) { serverDirView->refreshTree(); @@ -526,6 +551,61 @@ void TabDeckStorage::actOpenDecksFolder() QtUtils::openPathInFileManager(dir); } +void TabDeckStorage::actSyncNow() +{ + DeckSyncProfile profile(client->serverName(), client->serverPort(), syncUsername); + syncManager->setProfile(profile); + syncManager->setDeckPath(SettingsCache::instance().paths().getDeckPath()); + aSyncNow->setEnabled(false); + syncManager->syncNow(); +} + +void TabDeckStorage::onSyncStatusMessage(const QString &message) +{ + syncStatusType = SyncStatusType::Working; + syncStatusDetail = message; + renderSyncStatus(); +} + +void TabDeckStorage::onSyncFinished(int uploaded, int downloaded, int unchanged, int skipped, const QString &error) +{ + aSyncNow->setEnabled(remoteEnabled); + if (!error.isEmpty()) { + syncStatusType = SyncStatusType::Failed; + syncStatusDetail = error; + } else { + syncStatusType = SyncStatusType::Finished; + syncStatusDetail.clear(); + lastSyncSummary = {uploaded, downloaded, unchanged, skipped}; + } + renderSyncStatus(); + if (error.isEmpty()) { + serverDirView->refreshTree(); + } +} + +void TabDeckStorage::renderSyncStatus() +{ + switch (syncStatusType) { + case SyncStatusType::Working: + syncStatusLabel->setText(syncStatusDetail); + break; + case SyncStatusType::Failed: + syncStatusLabel->setText(tr("Sync failed: %1").arg(syncStatusDetail)); + break; + case SyncStatusType::Finished: + syncStatusLabel->setText(tr("Sync complete: %1 uploaded, %2 downloaded, %3 unchanged, %4 skipped.") + .arg(lastSyncSummary.uploaded) + .arg(lastSyncSummary.downloaded) + .arg(lastSyncSummary.unchanged) + .arg(lastSyncSummary.skipped)); + break; + case SyncStatusType::Idle: + syncStatusLabel->setText(tr("Not synced yet")); + break; + } +} + void TabDeckStorage::actRemoteDoubleClick(const QModelIndex &curRight) { if (dynamic_cast(serverDirView->getNode(curRight))) { diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h index 35a4649c7..f24f07ac7 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h @@ -25,9 +25,11 @@ class QFileSystemModel; class QToolBar; class QGroupBox; class QTimer; +class QLabel; class CommandContainer; class Response; class ShareBarWidget; +class DeckSyncManager; class QAction; class QModelIndex; class TabSupervisor; @@ -50,6 +52,29 @@ private: QAction *aOpenLocalDeck, *aRenameLocal, *aUpload, *aNewLocalFolder, *aDeleteLocalDeck; QAction *aOpenDecksFolder; QAction *aOpenRemoteDeck, *aDownload, *aShareDecks, *aPublishDeck, *aNewFolder, *aDeleteRemoteDeck; + QAction *aSyncNow; + QLabel *syncStatusLabel; + DeckSyncManager *syncManager; + QString syncUsername; + bool remoteEnabled = false; + enum class SyncStatusType + { + Idle, + Working, + Finished, + Failed + }; + struct SyncSummary + { + int uploaded = 0; + int downloaded = 0; + int unchanged = 0; + int skipped = 0; + }; + SyncStatusType syncStatusType = SyncStatusType::Idle; + QString syncStatusDetail; + SyncSummary lastSyncSummary; + void renderSyncStatus(); bool visibilityRefreshStarted = false; QTimer *visibilityRefreshTimer; QStringList visibilityFailures; @@ -108,6 +133,10 @@ private slots: void deleteFolderFinished(const Response &response, const CommandContainer &commandContainer); void deleteDeckFinished(const Response &response, const CommandContainer &commandContainer); + void actSyncNow(); + void onSyncStatusMessage(const QString &message); + void onSyncFinished(int uploaded, int downloaded, int unchanged, int skipped, const QString &error); + public: TabDeckStorage(TabSupervisor *_tabSupervisor, AbstractClient *_client, const ServerInfo_User *currentUserInfo); void retranslateUi() override; From ad1cb8f1333cb9fcfdc57b2c5c37a1ac094ce534 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Tue, 13 Oct 2026 07:18:43 +0200 Subject: [PATCH 02/14] Fix ZeldaZach review for PR 7493: match uploads by sidecar remoteId/guid, validate server paths --- .../interface/deck_sync/deck_sync_manager.cpp | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 9fe76445b..941126e48 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -135,6 +135,9 @@ void DeckSyncManager::collectServerFiles(const ServerInfo_DeckStorage_Folder &fo { for (const auto &item : folder.items()) { const QString name = QString::fromStdString(item.name()); + if (name.isEmpty() || name == "." || name == ".." || name.contains('/') || name.contains('\\')) { + continue; + } if (item.has_folder()) { const QString path = prefix.isEmpty() ? name : prefix + "/" + name; dirs.insert(path); @@ -173,8 +176,37 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont sidecar.load(); const qint64 nowSecs = QDateTime::currentDateTime().toSecsSinceEpoch(); + QMap serverPathById; + QMap serverPathByGuid; + for (auto sit = serverFiles.begin(); sit != serverFiles.end(); ++sit) { + serverPathById.insert(sit.value().id, sit.key()); + if (!sit.value().guid.isEmpty()) { + serverPathByGuid.insert(sit.value().guid, sit.key()); + } + } + QSet matchedServerPaths; + for (auto it = localFiles.begin(); it != localFiles.end(); ++it) { - const QString serverPath = serverPathFor(it.key()); + const DeckSyncRecord sidecarRecord = sidecar.get(it.key()); + QString serverPath; + if (sidecarRecord.remoteId != -1 && serverPathById.contains(sidecarRecord.remoteId)) { + serverPath = serverPathById.value(sidecarRecord.remoteId); + } else if (!sidecarRecord.guid.isEmpty() && serverPathByGuid.contains(sidecarRecord.guid)) { + serverPath = serverPathByGuid.value(sidecarRecord.guid); + } else { + serverPath = serverPathFor(it.key()); + if (!serverFiles.contains(serverPath)) { + UploadOp op; + op.relativePath = it.key(); + op.serverPath = serverPath; + op.deckId = -1; + op.guid = sidecar.contains(it.key()) && !sidecar.get(it.key()).guid.isEmpty() + ? sidecar.get(it.key()).guid + : QUuid::createUuid().toString(QUuid::WithoutBraces); + pendingUploads.append(op); + continue; + } + } auto serverIt = serverFiles.find(serverPath); if (serverIt == serverFiles.end()) { UploadOp op; @@ -187,6 +219,7 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont pendingUploads.append(op); continue; } + matchedServerPaths.insert(serverIt.key()); if (it.value().hash == serverIt.value().hash) { DeckSyncRecord record = sidecar.get(it.key()); if (record.guid.isEmpty()) { @@ -222,6 +255,9 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont } for (auto it = serverFiles.begin(); it != serverFiles.end(); ++it) { + if (matchedServerPaths.contains(it.key())) { + continue; + } const QString wantedPath = it.key().endsWith(".cod", Qt::CaseInsensitive) ? it.key() : it.key() + ".cod"; if (localFiles.contains(wantedPath) || QFileInfo(deckPath + "/" + wantedPath).exists()) { continue; @@ -455,7 +491,13 @@ void DeckSyncManager::onDownloadFinished(const Response &response, return; } - const QString absolutePath = deckPath + "/" + relativePath; + const QString cleanDeckPath = QDir::cleanPath(deckPath); + const QString absolutePath = QDir::cleanPath(deckPath + "/" + relativePath); + if (absolutePath != cleanDeckPath && !absolutePath.startsWith(cleanDeckPath + "/")) { + ++skippedCount; + processNext(); + return; + } if (localFiles.contains(relativePath)) { const QString backupPath = absolutePath + BACKUP_SUFFIX + QString::number(QDateTime::currentDateTime().toSecsSinceEpoch()); From e1a9f7766d79871a096d7d17b96912fc8dec9f04 Mon Sep 17 00:00:00 2001 From: BruebachL Date: Tue, 13 Oct 2026 11:45:29 +0200 Subject: [PATCH 03/14] [DeckSync] Move SyncSummary into DeckSyncManager per review --- cockatrice/src/interface/deck_sync/deck_sync_manager.h | 9 +++++++++ .../src/interface/widgets/tabs/tab_deck_storage.h | 10 ++-------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h index 99f2040fe..7f759c313 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -44,6 +44,15 @@ public: /** @brief Computes the color identity string uploaded alongside a deck. May be empty. */ using ColorIdentityProvider = std::function; + /** @brief Outcome counts for a finished sync run; mirrors syncFinished() parameters. */ + struct SyncSummary + { + int uploaded = 0; + int downloaded = 0; + int unchanged = 0; + int skipped = 0; + }; + explicit DeckSyncManager(QObject *parent = nullptr); void setClient(AbstractClient *_client); diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h index f24f07ac7..bd53d5073 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h @@ -11,6 +11,7 @@ #include "../interface/widgets/server/remote/remote_decklist_tree_widget.h" #include "tab.h" +#include "../deck_sync/deck_sync_manager.h" #include #include #include @@ -29,7 +30,6 @@ class QLabel; class CommandContainer; class Response; class ShareBarWidget; -class DeckSyncManager; class QAction; class QModelIndex; class TabSupervisor; @@ -64,13 +64,7 @@ private: Finished, Failed }; - struct SyncSummary - { - int uploaded = 0; - int downloaded = 0; - int unchanged = 0; - int skipped = 0; - }; + using SyncSummary = DeckSyncManager::SyncSummary; SyncStatusType syncStatusType = SyncStatusType::Idle; QString syncStatusDetail; SyncSummary lastSyncSummary; From 8a541531dab7ab4554eac6ee61e75610f72ad217 Mon Sep 17 00:00:00 2001 From: BruebachL Date: Tue, 13 Oct 2026 11:45:44 +0200 Subject: [PATCH 04/14] [DeckSync] Prefer ctor params for set-once client/provider per review --- cockatrice/src/interface/deck_sync/deck_sync_manager.cpp | 5 +++++ cockatrice/src/interface/deck_sync/deck_sync_manager.h | 1 + cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp | 6 ++---- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 941126e48..464849eec 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -42,6 +42,11 @@ DeckSyncManager::DeckSyncManager(QObject *parent) : QObject(parent) { } +DeckSyncManager::DeckSyncManager(AbstractClient *_client, ColorIdentityProvider provider, QObject *parent) + : QObject(parent), client(_client), colorIdentityProvider(std::move(provider)) +{ +} + void DeckSyncManager::setClient(AbstractClient *_client) { client = _client; diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h index 7f759c313..2bf12f72f 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -54,6 +54,7 @@ public: }; explicit DeckSyncManager(QObject *parent = nullptr); + DeckSyncManager(AbstractClient *client, ColorIdentityProvider provider, QObject *parent = nullptr); void setClient(AbstractClient *_client); void setProfile(const DeckSyncProfile &_profile); diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp index 90cd24290..74154f3da 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp @@ -254,10 +254,8 @@ TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, rightToolBar->addAction(aNewFolder); rightToolBar->addAction(aDeleteRemoteDeck); - syncManager = new DeckSyncManager(this); - syncManager->setClient(client); - syncManager->setColorIdentityProvider( - [](const DeckList &deck) { return getDeckColorIdentity(deck, CardDatabaseManager::query()); }); + syncManager = new DeckSyncManager( + client, [](const DeckList &deck) { return getDeckColorIdentity(deck, CardDatabaseManager::query()); }, this); connect(syncManager, &DeckSyncManager::statusMessage, this, &TabDeckStorage::onSyncStatusMessage); connect(syncManager, &DeckSyncManager::syncFinished, this, &TabDeckStorage::onSyncFinished); if (currentUserInfo != nullptr) { From 0757d3d8674f151650f6ec4ed876e2d2da196439 Mon Sep 17 00:00:00 2001 From: BruebachL Date: Tue, 13 Oct 2026 11:45:54 +0200 Subject: [PATCH 05/14] [DeckSync] Use designated initializer for ServerFile per review --- .../src/interface/deck_sync/deck_sync_manager.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 464849eec..49be7fac0 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -150,11 +150,12 @@ void DeckSyncManager::collectServerFiles(const ServerInfo_DeckStorage_Folder &fo } else if (item.has_file()) { const QString path = prefix.isEmpty() ? name : prefix + "/" + name; const ServerInfo_DeckStorage_File &file = item.file(); - ServerFile entry; - entry.id = item.id(); - entry.guid = QString::fromStdString(file.guid()); - entry.hash = QString::fromStdString(file.hash()); - entry.lastModifiedSecs = file.has_last_modified() ? static_cast(file.last_modified()) : 0; + ServerFile entry{ + .id = item.id(), + .guid = QString::fromStdString(file.guid()), + .hash = QString::fromStdString(file.hash()), + .lastModifiedSecs = file.has_last_modified() ? static_cast(file.last_modified()) : 0, + }; files.insert(path, entry); } } From c0a62a24864345aeaa4db147ed9cb6033b907c25 Mon Sep 17 00:00:00 2001 From: BruebachL Date: Tue, 13 Oct 2026 11:50:40 +0200 Subject: [PATCH 06/14] [Format] Fix include order in tab_deck_storage.h --- cockatrice/src/interface/widgets/tabs/tab_deck_storage.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h index bd53d5073..ade10e951 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h @@ -8,10 +8,10 @@ #ifndef TAB_DECK_STORAGE_H #define TAB_DECK_STORAGE_H +#include "../deck_sync/deck_sync_manager.h" #include "../interface/widgets/server/remote/remote_decklist_tree_widget.h" #include "tab.h" -#include "../deck_sync/deck_sync_manager.h" #include #include #include From ad0da98a86576ebc8a55b2161f14ef2b0e463df7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 1 Nov 2026 12:41:00 +0100 Subject: [PATCH 07/14] [DeckSync] Pass the sync summary through the signal and tidy the manager API - Emit SyncSummary from syncFinished() instead of four positional ints, and build it from a single currentSummary() helper, so the counts cannot drift between the status message and the signal. - Use a designated initializer for LocalFile, like the other records. - Drop setClient() and setColorIdentityProvider(): both fields are already constructor parameters and the setters had no callers. setProfile() and setDeckPath() stay, since the tab re-reads them before every run. - Fix the include paths in tab_deck_storage.h and the narrowing conversion of the tree item id, so the branch builds again. --- .../interface/deck_sync/deck_sync_manager.cpp | 47 +++++++++---------- .../interface/deck_sync/deck_sync_manager.h | 7 ++- .../widgets/tabs/tab_deck_storage.cpp | 4 +- .../interface/widgets/tabs/tab_deck_storage.h | 6 +-- 4 files changed, 31 insertions(+), 33 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 49be7fac0..0ff162ee1 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -47,11 +47,6 @@ DeckSyncManager::DeckSyncManager(AbstractClient *_client, ColorIdentityProvider { } -void DeckSyncManager::setClient(AbstractClient *_client) -{ - client = _client; -} - void DeckSyncManager::setProfile(const DeckSyncProfile &_profile) { profile = _profile; @@ -62,11 +57,6 @@ void DeckSyncManager::setDeckPath(const QString &path) deckPath = path; } -void DeckSyncManager::setColorIdentityProvider(ColorIdentityProvider provider) -{ - colorIdentityProvider = std::move(provider); -} - bool DeckSyncManager::isSyncing() const { return syncing; @@ -78,12 +68,12 @@ void DeckSyncManager::syncNow() return; } if (client == nullptr || !profile.isValid() || deckPath.isEmpty()) { - emit syncFinished(0, 0, 0, 0, tr("Not connected to a server.")); + emit syncFinished(SyncSummary{}, tr("Not connected to a server.")); return; } QDir deckDir(deckPath); if (!deckDir.exists()) { - emit syncFinished(0, 0, 0, 0, tr("Deck folder not found.")); + emit syncFinished(SyncSummary{}, tr("Deck folder not found.")); return; } @@ -120,11 +110,9 @@ void DeckSyncManager::syncNow() ++skippedCount; continue; } - LocalFile file; - file.absolutePath = absolutePath; - file.hash = loaded->deckList.getDeckHash(); - file.modified = QFileInfo(absolutePath).lastModified(); - localFiles.insert(relativePath, file); + localFiles.insert(relativePath, LocalFile{.absolutePath = absolutePath, + .hash = loaded->deckList.getDeckHash(), + .modified = QFileInfo(absolutePath).lastModified()}); } Command_DeckList cmd; @@ -151,7 +139,7 @@ void DeckSyncManager::collectServerFiles(const ServerInfo_DeckStorage_Folder &fo const QString path = prefix.isEmpty() ? name : prefix + "/" + name; const ServerInfo_DeckStorage_File &file = item.file(); ServerFile entry{ - .id = item.id(), + .id = static_cast(item.id()), .guid = QString::fromStdString(file.guid()), .hash = QString::fromStdString(file.hash()), .lastModifiedSecs = file.has_last_modified() ? static_cast(file.last_modified()) : 0, @@ -543,16 +531,27 @@ void DeckSyncManager::onDownloadFinished(const Response &response, void DeckSyncManager::failSync(const QString &error) { syncing = false; - emit syncFinished(uploadedCount, downloadedCount, unchangedCount, skippedCount, error); + emit syncFinished(currentSummary(), error); } void DeckSyncManager::finishSync() { syncing = false; + const SyncSummary summary = currentSummary(); emit statusMessage(tr("Sync complete: %1 uploaded, %2 downloaded, %3 unchanged, %4 skipped.") - .arg(uploadedCount) - .arg(downloadedCount) - .arg(unchangedCount) - .arg(skippedCount)); - emit syncFinished(uploadedCount, downloadedCount, unchangedCount, skippedCount, QString()); + .arg(summary.uploaded) + .arg(summary.downloaded) + .arg(summary.unchanged) + .arg(summary.skipped)); + emit syncFinished(summary, QString()); +} + +DeckSyncManager::SyncSummary DeckSyncManager::currentSummary() const +{ + return SyncSummary{ + .uploaded = uploadedCount, + .downloaded = downloadedCount, + .unchanged = unchangedCount, + .skipped = skippedCount, + }; } diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h index 2bf12f72f..8ecbaf74c 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -44,7 +44,7 @@ public: /** @brief Computes the color identity string uploaded alongside a deck. May be empty. */ using ColorIdentityProvider = std::function; - /** @brief Outcome counts for a finished sync run; mirrors syncFinished() parameters. */ + /** @brief Outcome counts for a finished sync run, carried by syncFinished(). */ struct SyncSummary { int uploaded = 0; @@ -56,10 +56,8 @@ public: explicit DeckSyncManager(QObject *parent = nullptr); DeckSyncManager(AbstractClient *client, ColorIdentityProvider provider, QObject *parent = nullptr); - void setClient(AbstractClient *_client); void setProfile(const DeckSyncProfile &_profile); void setDeckPath(const QString &path); - void setColorIdentityProvider(ColorIdentityProvider provider); [[nodiscard]] bool isSyncing() const; public slots: @@ -68,7 +66,7 @@ public slots: signals: void syncStarted(); void statusMessage(const QString &message); - void syncFinished(int uploaded, int downloaded, int unchanged, int skipped, const QString &error); + void syncFinished(const SyncSummary &summary, const QString &error); private slots: void onListFinished(const Response &response, const CommandContainer &commandContainer); @@ -111,6 +109,7 @@ private: void failSync(const QString &error); void finishSync(); + [[nodiscard]] SyncSummary currentSummary() const; void collectServerFiles(const ServerInfo_DeckStorage_Folder &folder, const QString &prefix, QMap &files, diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp index 74154f3da..2ca46d4f4 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp @@ -565,7 +565,7 @@ void TabDeckStorage::onSyncStatusMessage(const QString &message) renderSyncStatus(); } -void TabDeckStorage::onSyncFinished(int uploaded, int downloaded, int unchanged, int skipped, const QString &error) +void TabDeckStorage::onSyncFinished(const SyncSummary &summary, const QString &error) { aSyncNow->setEnabled(remoteEnabled); if (!error.isEmpty()) { @@ -574,7 +574,7 @@ void TabDeckStorage::onSyncFinished(int uploaded, int downloaded, int unchanged, } else { syncStatusType = SyncStatusType::Finished; syncStatusDetail.clear(); - lastSyncSummary = {uploaded, downloaded, unchanged, skipped}; + lastSyncSummary = summary; } renderSyncStatus(); if (error.isEmpty()) { diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h index ade10e951..c7456877e 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h @@ -8,8 +8,8 @@ #ifndef TAB_DECK_STORAGE_H #define TAB_DECK_STORAGE_H -#include "../deck_sync/deck_sync_manager.h" -#include "../interface/widgets/server/remote/remote_decklist_tree_widget.h" +#include "../../deck_sync/deck_sync_manager.h" +#include "../server/remote/remote_decklist_tree_widget.h" #include "tab.h" #include @@ -129,7 +129,7 @@ private slots: void actSyncNow(); void onSyncStatusMessage(const QString &message); - void onSyncFinished(int uploaded, int downloaded, int unchanged, int skipped, const QString &error); + void onSyncFinished(const SyncSummary &summary, const QString &error); public: TabDeckStorage(TabSupervisor *_tabSupervisor, AbstractClient *_client, const ServerInfo_User *currentUserInfo); From 2916fa45afdaf05580e45f6633b5295ec0c4674e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 1 Nov 2026 12:43:02 +0100 Subject: [PATCH 08/14] [DeckSync] Match server rows by identity and hash the whole deck Sync keyed local files to server rows by path, but the server names a row after the deck, not after the file, so any deck whose file name and deck name differ uploaded again on every run. A second pass then brought the server copy back as an extra local file. - Match by the sidecar's row id, then guid, then content, and only then by the server path, so renaming either side no longer duplicates a deck. - Key the server tree by row id: the server allows several rows with the same name in one folder, and keying by path silently dropped all but one. - Stop discarding rows whose name contains '/' ("U/W Control"). They stay matchable, and only the download file name is sanitised. - Build the download path from the folder plus a sanitised row name, including every folder segment, so a hostile server name cannot escape the deck folder. - Download a colliding server row under a unique name and count it, instead of skipping it without a trace. - Hash the whole deck instead of the card hash: SHA-1 over the native deck string covers name, printings, banner, tokens and sideboard plans. The client and the server hash the exact bytes they exchange, so both agree. Adds deck_content_hash_test for the client/server hashing invariant and for the name-only change the card hash cannot see. --- .../interface/deck_sync/deck_sync_manager.cpp | 195 +++++++++++------- .../interface/deck_sync/deck_sync_manager.h | 16 +- .../libcockatrice/deck_list/deck_list.cpp | 11 + .../libcockatrice/deck_list/deck_list.h | 10 + servatrice/src/serversocketinterface.cpp | 4 +- tests/CMakeLists.txt | 9 + tests/deck_content_hash_test.cpp | 51 +++++ 7 files changed, 215 insertions(+), 81 deletions(-) create mode 100644 tests/deck_content_hash_test.cpp diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 0ff162ee1..dd4dd6f69 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -36,6 +37,35 @@ QString serverPathFor(const QString &relativePath) } return path; } + +// Rows are named after the deck, not after the file, so a row name may contain +// characters a file name cannot ("U/W Control"). Downloads therefore rebuild the +// local path from the folder plus a sanitised row name, and never from the raw +// server-supplied path. +QString sanitizedSegment(const QString &segment) +{ + QString safe = segment; + safe.replace('/', '-'); + safe.replace('\\', '-'); + safe = safe.trimmed(); + if (safe.isEmpty() || safe == QStringLiteral(".") || safe == QStringLiteral("..")) { + return QStringLiteral("deck"); + } + return safe; +} + +QString localPathFor(const QString &dir, const QString &name) +{ + // Every segment is server-supplied, so each one is sanitised before it can + // become part of a local path. + QStringList segments; + for (const QString &segment : dir.split('/', Qt::SkipEmptyParts)) { + segments << sanitizedSegment(segment); + } + segments << sanitizedSegment(name); + const QString path = segments.join('/'); + return path.endsWith(".cod", Qt::CaseInsensitive) ? path : path + QStringLiteral(".cod"); +} } // namespace DeckSyncManager::DeckSyncManager(QObject *parent) : QObject(parent) @@ -111,7 +141,7 @@ void DeckSyncManager::syncNow() continue; } localFiles.insert(relativePath, LocalFile{.absolutePath = absolutePath, - .hash = loaded->deckList.getDeckHash(), + .hash = loaded->deckList.getContentHash(), .modified = QFileInfo(absolutePath).lastModified()}); } @@ -123,28 +153,30 @@ void DeckSyncManager::syncNow() void DeckSyncManager::collectServerFiles(const ServerInfo_DeckStorage_Folder &folder, const QString &prefix, - QMap &files, + QMap &files, QSet &dirs) { for (const auto &item : folder.items()) { const QString name = QString::fromStdString(item.name()); - if (name.isEmpty() || name == "." || name == ".." || name.contains('/') || name.contains('\\')) { - continue; - } + const QString path = prefix.isEmpty() ? name : prefix + "/" + name; if (item.has_folder()) { - const QString path = prefix.isEmpty() ? name : prefix + "/" + name; dirs.insert(path); collectServerFiles(item.folder(), path, files, dirs); } else if (item.has_file()) { - const QString path = prefix.isEmpty() ? name : prefix + "/" + name; const ServerInfo_DeckStorage_File &file = item.file(); + const int id = static_cast(item.id()); ServerFile entry{ - .id = static_cast(item.id()), + .id = id, + .path = path, + .dir = prefix, + .name = name, .guid = QString::fromStdString(file.guid()), .hash = QString::fromStdString(file.hash()), .lastModifiedSecs = file.has_last_modified() ? static_cast(file.last_modified()) : 0, }; - files.insert(path, entry); + // Keyed by id: the server allows several rows with the same name in a + // folder, and keying by path silently dropped all but one of them. + files.insert(id, entry); } } } @@ -159,7 +191,7 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont return; } - QMap serverFiles; + QMap serverFiles; QSet serverDirs; const Response_DeckList &list = response.GetExtension(Response_DeckList::ext); collectServerFiles(list.root(), QString(), serverFiles, serverDirs); @@ -169,57 +201,78 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont DeckSyncSidecar sidecar(sidecarPath); sidecar.load(); const qint64 nowSecs = QDateTime::currentDateTime().toSecsSinceEpoch(); + // Reuse the sidecar's guid so the server keeps recognizing the row, and mint + // one only for a deck that has never been synced. + auto guidFor = [&sidecar](const QString &relativePath) { + const DeckSyncRecord record = sidecar.get(relativePath); + return record.guid.isEmpty() ? QUuid::createUuid().toString(QUuid::WithoutBraces) : record.guid; + }; - QMap serverPathById; - QMap serverPathByGuid; + QMap serverIdByPath; + QMap serverIdByGuid; for (auto sit = serverFiles.begin(); sit != serverFiles.end(); ++sit) { - serverPathById.insert(sit.value().id, sit.key()); + serverIdByPath.insert(sit.value().path, sit.key()); if (!sit.value().guid.isEmpty()) { - serverPathByGuid.insert(sit.value().guid, sit.key()); + serverIdByGuid.insert(sit.value().guid, sit.key()); } } - QSet matchedServerPaths; + QSet matchedServerIds; + + // A local file without a usable sidecar record still usually has a server + // counterpart: rows are named after the deck, so compare content rather than + // file names, and only upload when no unmatched row has the same content. + auto unmatchedByHash = [&serverFiles, &matchedServerIds](const QString &hash) { + if (hash.isEmpty()) { + return -1; + } + for (auto sit = serverFiles.begin(); sit != serverFiles.end(); ++sit) { + if (!matchedServerIds.contains(sit.key()) && sit.value().hash == hash) { + return sit.key(); + } + } + return -1; + }; for (auto it = localFiles.begin(); it != localFiles.end(); ++it) { const DeckSyncRecord sidecarRecord = sidecar.get(it.key()); - QString serverPath; - if (sidecarRecord.remoteId != -1 && serverPathById.contains(sidecarRecord.remoteId)) { - serverPath = serverPathById.value(sidecarRecord.remoteId); - } else if (!sidecarRecord.guid.isEmpty() && serverPathByGuid.contains(sidecarRecord.guid)) { - serverPath = serverPathByGuid.value(sidecarRecord.guid); + int serverId = -1; + if (sidecarRecord.remoteId != -1 && serverFiles.contains(sidecarRecord.remoteId)) { + serverId = sidecarRecord.remoteId; + } else if (!sidecarRecord.guid.isEmpty() && serverIdByGuid.contains(sidecarRecord.guid)) { + serverId = serverIdByGuid.value(sidecarRecord.guid); } else { - serverPath = serverPathFor(it.key()); - if (!serverFiles.contains(serverPath)) { - UploadOp op; - op.relativePath = it.key(); - op.serverPath = serverPath; - op.deckId = -1; - op.guid = sidecar.contains(it.key()) && !sidecar.get(it.key()).guid.isEmpty() - ? sidecar.get(it.key()).guid - : QUuid::createUuid().toString(QUuid::WithoutBraces); - pendingUploads.append(op); - continue; + // Content first: a row with the same deck is unambiguous, while a + // matching file name may belong to a different deck entirely. + serverId = unmatchedByHash(it.value().hash); + if (serverId == -1) { + const QString wantedPath = serverPathFor(it.key()); + if (serverIdByPath.contains(wantedPath)) { + const int pathId = serverIdByPath.value(wantedPath); + if (!matchedServerIds.contains(pathId)) { + serverId = pathId; + } + } } } - auto serverIt = serverFiles.find(serverPath); - if (serverIt == serverFiles.end()) { - UploadOp op; - op.relativePath = it.key(); - op.serverPath = serverPath; - op.deckId = -1; - op.guid = sidecar.contains(it.key()) && !sidecar.get(it.key()).guid.isEmpty() - ? sidecar.get(it.key()).guid - : QUuid::createUuid().toString(QUuid::WithoutBraces); - pendingUploads.append(op); + + if (serverId == -1) { + const QString serverPath = serverPathFor(it.key()); + pendingUploads.append(UploadOp{.relativePath = it.key(), + .serverPath = serverPath, + .serverDir = parentDir(serverPath), + .deckId = -1, + .guid = guidFor(it.key())}); continue; } - matchedServerPaths.insert(serverIt.key()); - if (it.value().hash == serverIt.value().hash) { + + const ServerFile &server = serverFiles.value(serverId); + matchedServerIds.insert(serverId); + if (it.value().hash == server.hash) { DeckSyncRecord record = sidecar.get(it.key()); if (record.guid.isEmpty()) { - record.guid = serverIt.value().guid; + record.guid = server.guid; } - record.remoteId = serverIt.value().id; + record.remoteId = server.id; record.baseHash = it.value().hash; record.lastSyncSecs = nowSecs; record.deleted = false; @@ -230,44 +283,36 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont const qint64 localSecs = it.value().modified.toSecsSinceEpoch(); // A server row without last_modified predates sync: the local file wins. // Otherwise strictly newer wins; ties go to the server. - if (serverIt.value().lastModifiedSecs == 0 || localSecs > serverIt.value().lastModifiedSecs) { - UploadOp op; - op.relativePath = it.key(); - op.serverPath = serverPath; - op.deckId = serverIt.value().id; - op.guid = serverIt.value().guid; - pendingUploads.append(op); + if (server.lastModifiedSecs == 0 || localSecs > server.lastModifiedSecs) { + pendingUploads.append(UploadOp{.relativePath = it.key(), + .serverPath = server.path, + .serverDir = server.dir, + .deckId = server.id, + .guid = server.guid}); } else { - DownloadOp op; - op.relativePath = it.key(); - op.deckId = serverIt.value().id; - op.guid = serverIt.value().guid; - op.hash = serverIt.value().hash; - op.lastModifiedSecs = serverIt.value().lastModifiedSecs; - pendingDownloads.append(op); + pendingDownloads.append(DownloadOp{.relativePath = it.key(), + .deckId = server.id, + .guid = server.guid, + .hash = server.hash, + .lastModifiedSecs = server.lastModifiedSecs}); } } for (auto it = serverFiles.begin(); it != serverFiles.end(); ++it) { - if (matchedServerPaths.contains(it.key())) { - continue; - } - const QString wantedPath = it.key().endsWith(".cod", Qt::CaseInsensitive) ? it.key() : it.key() + ".cod"; - if (localFiles.contains(wantedPath) || QFileInfo(deckPath + "/" + wantedPath).exists()) { + if (matchedServerIds.contains(it.key())) { continue; } + const QString wantedPath = localPathFor(it.value().dir, it.value().name); const QString relativePath = uniqueLocalPath(wantedPath); if (relativePath.isEmpty()) { ++skippedCount; continue; } - DownloadOp op; - op.relativePath = relativePath; - op.deckId = it.value().id; - op.guid = it.value().guid; - op.hash = it.value().hash; - op.lastModifiedSecs = it.value().lastModifiedSecs; - pendingDownloads.append(op); + pendingDownloads.append(DownloadOp{.relativePath = relativePath, + .deckId = it.value().id, + .guid = it.value().guid, + .hash = it.value().hash, + .lastModifiedSecs = it.value().lastModifiedSecs}); } sidecar.save(); @@ -297,7 +342,7 @@ void DeckSyncManager::processNext() void DeckSyncManager::processNextUpload() { const UploadOp op = pendingUploads.first(); - const QString dir = parentDir(op.serverPath); + const QString dir = op.serverDir; if (!ensuredDirs.contains(dir)) { sendMkDirFor(dir); return; @@ -370,7 +415,7 @@ void DeckSyncManager::sendUpload(const UploadOp &op) Command_DeckUpload cmd; if (op.deckId == -1) { - cmd.set_path(parentDir(op.serverPath).toStdString()); + cmd.set_path(op.serverDir.toStdString()); } else { cmd.set_deck_id(op.deckId); } @@ -416,7 +461,7 @@ void DeckSyncManager::onUploadFinished(const Response &response, } else { record.lastSyncSecs = QDateTime::currentDateTime().toSecsSinceEpoch(); } - record.remoteId = file.id(); + record.remoteId = static_cast(file.id()); } else if (localFiles.contains(relativePath)) { record.baseHash = localFiles.value(relativePath).hash; record.lastSyncSecs = QDateTime::currentDateTime().toSecsSinceEpoch(); @@ -517,7 +562,7 @@ void DeckSyncManager::onDownloadFinished(const Response &response, if (!download.hash().empty()) { record.baseHash = QString::fromStdString(download.hash()); } else { - record.baseHash = deck.getDeckHash(); + record.baseHash = deck.getContentHash(); } record.lastSyncSecs = download.has_last_modified() ? static_cast(download.last_modified()) : QDateTime::currentDateTime().toSecsSinceEpoch(); diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h index 8ecbaf74c..c891cbb58 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -30,9 +30,11 @@ class ServerInfo_DeckStorage_Folder; * Each syncNow() run loads the profile sidecar, scans the deck folder, fetches * the server tree and mirrors both sides: unknown local decks upload, unknown * server decks download, and decks changed on both sides resolve newer-wins - * (ties go to the server). Matching is path-based; a rename looks like a - * delete plus an add. Before a download overwrites a locally modified file, the - * local content is copied to a .sync-backup file next to it. + * (ties go to the server). A local file is matched to its server row by the + * sidecar's row id, then guid, then the server path, and finally by content, so + * renaming either side does not duplicate a deck. Before a download overwrites a + * locally modified file, the local content is copied to a .sync-backup file next + * to it. * * Ownership: fire-and-forget per run, driven by PendingCommand round trips. * No watcher, no auto triggers, no conflict UI — those arrive in later layers. @@ -81,6 +83,9 @@ private: struct ServerFile { int id = -1; + QString path; ///< Server-side address: folder prefix plus row name. + QString dir; ///< Folder prefix, empty in the root. + QString name; ///< Row name exactly as the server stores it. QString guid; QString hash; qint64 lastModifiedSecs = 0; @@ -94,7 +99,8 @@ private: struct UploadOp { QString relativePath; - QString serverPath; + QString serverPath; ///< Target address of a brand-new row. + QString serverDir; ///< Folder that must exist before the upload. int deckId = -1; QString guid; }; @@ -112,7 +118,7 @@ private: [[nodiscard]] SyncSummary currentSummary() const; void collectServerFiles(const ServerInfo_DeckStorage_Folder &folder, const QString &prefix, - QMap &files, + QMap &files, QSet &dirs); void processNext(); void processNextUpload(); diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp index 821b53dff..6f1a358f2 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp @@ -9,6 +9,7 @@ #include "tree/inner_deck_list_node.h" #include +#include #include #include #include @@ -453,6 +454,16 @@ void DeckList::refreshDeckHash() cachedDeckHash = QString(); } +QString DeckList::contentHash(const QString &nativeDeck) +{ + return QString::fromLatin1(QCryptographicHash::hash(nativeDeck.toUtf8(), QCryptographicHash::Sha1).toHex()); +} + +QString DeckList::getContentHash() const +{ + return contentHash(writeToString_Native()); +} + /** * Calls a given function on each card in the deck. */ diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h index 6796e9c33..9a8b54c24 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h @@ -289,6 +289,16 @@ public: /** @name Deck identity */ ///@{ QString getDeckHash() const; + /** + * @brief SHA-1 of a native deck string, covering everything the card hash + * ignores (name, printings, banner, tokens, comments, sideboard plans). + * + * Deck-collection sync hashes the exact string it sends or receives, so both + * peers compare the same bytes. + */ + static QString contentHash(const QString &nativeDeck); + /** @brief contentHash() of this deck's native serialisation. */ + QString getContentHash() const; void refreshDeckHash(); ///@} diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index e21b3b10f..f9e602f82 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -1114,7 +1114,9 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckUpload(const Comman const QString bannerCardProvider = deck.getBannerCard().providerId.left(32); const QString tagsJson = serializeDeckTags(deck.getTags()); const QString colorIdentity = sanitizeColorIdentity(nameFromStdString(cmd.color_identity())); - const QString deckHash = deck.getDeckHash(); + // Hash the exact bytes the client sent: both peers must compare the same + // string, not each side's re-serialisation of it. + const QString deckHash = DeckList::contentHash(deckStr); QString guid = cmd.has_guid() ? nameFromStdString(cmd.guid()).left(36) : QString(); if (QUuid::fromString(guid).isNull()) { guid = QUuid::createUuid().toString(QUuid::WithoutBraces); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 32ee9b7f1..5163db067 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -416,6 +416,15 @@ cockatrice_add_test( libcockatrice_deck_list libcockatrice_utility ) +cockatrice_add_test( + NAME + deck_content_hash_test + SOURCES + deck_content_hash_test.cpp + LIBS + libcockatrice_deck_list + libcockatrice_utility +) cockatrice_add_test( NAME deck_sync_profile_test diff --git a/tests/deck_content_hash_test.cpp b/tests/deck_content_hash_test.cpp new file mode 100644 index 000000000..192abb007 --- /dev/null +++ b/tests/deck_content_hash_test.cpp @@ -0,0 +1,51 @@ +#include "gtest/gtest.h" +#include +#include + +namespace +{ +QString deckWithName(const QString &name) +{ + return QStringLiteral(R"()" + R"(%1)" + R"()" + R"()" + R"()") + .arg(name); +} +} // namespace + +TEST(DeckContentHashTest, MatchesNativeSerialisation) +{ + DeckList deck; + ASSERT_TRUE(deck.loadFromString_Native(deckWithName(QStringLiteral("Burn")))); + EXPECT_EQ(deck.getContentHash(), DeckList::contentHash(deck.writeToString_Native())); +} + +TEST(DeckContentHashTest, IsStableAcrossLoads) +{ + DeckList first; + DeckList second; + ASSERT_TRUE(first.loadFromString_Native(deckWithName(QStringLiteral("Burn")))); + ASSERT_TRUE(second.loadFromString_Native(deckWithName(QStringLiteral("Burn")))); + EXPECT_EQ(first.getContentHash(), second.getContentHash()); +} + +TEST(DeckContentHashTest, CoversDeckName) +{ + DeckList burn; + DeckList renamed; + ASSERT_TRUE(burn.loadFromString_Native(deckWithName(QStringLiteral("Burn")))); + ASSERT_TRUE(renamed.loadFromString_Native(deckWithName(QStringLiteral("Burn v2")))); + + // The card hash only looks at card names and counts, so it cannot tell the + // two decks apart; the content hash must. + EXPECT_EQ(burn.getDeckHash(), renamed.getDeckHash()); + EXPECT_NE(burn.getContentHash(), renamed.getContentHash()); +} + +int main(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} From e43862c3bc3cc7f012e38cd164d474a2f30b3b61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 11 Oct 2026 16:05:22 +0200 Subject: [PATCH 09/14] [Server+Client] Tombstone deck deletes with purge and base snapshots --- .../interface/deck_sync/deck_sync_manager.cpp | 117 +++++++++++++++++- .../interface/deck_sync/deck_sync_manager.h | 9 ++ servatrice/servatrice.ini.example | 7 ++ servatrice/src/servatrice.cpp | 11 ++ servatrice/src/servatrice.h | 2 + .../src/servatrice_database_interface.cpp | 17 ++- .../src/servatrice_database_interface.h | 5 + servatrice/src/serversocketinterface.cpp | 33 ++--- 8 files changed, 180 insertions(+), 21 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index dd4dd6f69..7771c050a 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -114,6 +115,7 @@ void DeckSyncManager::syncNow() skippedCount = 0; pendingUploads.clear(); pendingDownloads.clear(); + pendingDeletes.clear(); ensuredDirs.clear(); ensuredDirs.insert(QString()); localFiles.clear(); @@ -254,8 +256,21 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont } } } - if (serverId == -1) { + if (sidecarRecord.remoteId != -1) { + // The server no longer lists this deck: it was deleted elsewhere. + // Apply the delete locally, keeping a backup of the file. + const QString absolutePath = deckPath + "/" + it.key(); + const QString backupPath = + absolutePath + BACKUP_SUFFIX + QString::number(QDateTime::currentDateTime().toSecsSinceEpoch()); + if (!QFile::copy(absolutePath, backupPath) || !QFile::remove(absolutePath)) { + ++skippedCount; + continue; + } + sidecar.remove(it.key()); + ++downloadedCount; + continue; + } const QString serverPath = serverPathFor(it.key()); pendingUploads.append(UploadOp{.relativePath = it.key(), .serverPath = serverPath, @@ -277,6 +292,7 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont record.lastSyncSecs = nowSecs; record.deleted = false; sidecar.set(it.key(), record); + snapshotLocalFile(it.key(), record.guid); ++unchangedCount; continue; } @@ -315,6 +331,18 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont .lastModifiedSecs = it.value().lastModifiedSecs}); } + const QMap knownRecords = sidecar.records(); + for (auto it = knownRecords.begin(); it != knownRecords.end(); ++it) { + if (localFiles.contains(it.key())) { + continue; + } + if (it.value().remoteId == -1) { + sidecar.remove(it.key()); + continue; + } + pendingDeletes.append(it.key()); + } + sidecar.save(); processNext(); } @@ -330,7 +358,12 @@ void DeckSyncManager::processNext() if (!syncing) { return; } - if (!pendingUploads.isEmpty()) { + if (!pendingDeletes.isEmpty()) { + const QString relativePath = pendingDeletes.first(); + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + sendDelete(relativePath, sidecar.get(relativePath).remoteId); + } else if (!pendingUploads.isEmpty()) { processNextUpload(); } else if (!pendingDownloads.isEmpty()) { sendDownload(pendingDownloads.first()); @@ -425,7 +458,12 @@ void DeckSyncManager::sendUpload(const UploadOp &op) cmd.set_color_identity(colorIdentityProvider(loaded->deckList).toStdString()); } PendingCommand *pend = AbstractClient::prepareSessionCommand(cmd); - pend->setExtraData(op.relativePath); + QVariantMap context; + context.insert("path", op.relativePath); + context.insert("serverPath", op.serverPath); + context.insert("deckId", op.deckId); + context.insert("guid", op.guid); + pend->setExtraData(context); connect(pend, &PendingCommand::finished, this, &DeckSyncManager::onUploadFinished); client->sendCommand(pend); } @@ -437,8 +475,22 @@ void DeckSyncManager::onUploadFinished(const Response &response, if (!syncing) { return; } - const QString relativePath = extraData.toString(); + const QVariantMap context = extraData.toMap(); + const QString relativePath = context.value("path").toString(); + const int deckId = context.value("deckId", -1).toInt(); if (response.response_code() != Response::RespOk) { + if (response.response_code() == Response::RespNameNotFound && deckId != -1) { + // The server row is gone (purged tombstone or removed elsewhere): + // upload the deck as new instead of updating a dead id. + UploadOp retry; + retry.relativePath = relativePath; + retry.serverPath = context.value("serverPath").toString(); + retry.deckId = -1; + retry.guid = context.value("guid").toString(); + pendingUploads.prepend(retry); + processNext(); + return; + } ++skippedCount; processNext(); return; @@ -469,6 +521,7 @@ void DeckSyncManager::onUploadFinished(const Response &response, record.deleted = false; sidecar.set(relativePath, record); sidecar.save(); + snapshotLocalFile(relativePath, record.guid); ++uploadedCount; processNext(); } @@ -569,10 +622,66 @@ void DeckSyncManager::onDownloadFinished(const Response &response, record.deleted = false; sidecar.set(relativePath, record); sidecar.save(); + snapshotLocalFile(relativePath, record.guid); ++downloadedCount; processNext(); } +void DeckSyncManager::sendDelete(const QString &relativePath, int deckId) +{ + emit statusMessage(tr("Deleting %1 from server...").arg(relativePath)); + Command_DeckDel cmd; + cmd.set_deck_id(deckId); + PendingCommand *pend = AbstractClient::prepareSessionCommand(cmd); + pend->setExtraData(relativePath); + connect(pend, &PendingCommand::finished, this, &DeckSyncManager::onDeleteFinished); + client->sendCommand(pend); +} + +void DeckSyncManager::onDeleteFinished(const Response &response, + const CommandContainer & /*commandContainer*/, + const QVariant &extraData) +{ + if (!syncing) { + return; + } + const QString relativePath = extraData.toString(); + for (int i = 0; i < pendingDeletes.size(); ++i) { + if (pendingDeletes.at(i) == relativePath) { + pendingDeletes.removeAt(i); + break; + } + } + // Both outcomes converge the record: pushed deletes and server-side + // deletes (already tombstoned, answering NameNotFound) are done. + if (response.response_code() == Response::RespOk || response.response_code() == Response::RespNameNotFound) { + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + sidecar.remove(relativePath); + sidecar.save(); + } else { + ++skippedCount; + } + processNext(); +} + +void DeckSyncManager::snapshotLocalFile(const QString &relativePath, const QString &guid) +{ + if (guid.isEmpty()) { + return; + } + QFile source(deckPath + "/" + relativePath); + if (!source.open(QIODevice::ReadOnly)) { + return; + } + QDir().mkpath(deckPath + "/" + SYNC_META_DIR + "/bases"); + QFile snapshot(deckPath + "/" + SYNC_META_DIR + "/bases/" + guid + ".cod"); + if (!snapshot.open(QIODevice::WriteOnly | QIODevice::Truncate)) { + return; + } + snapshot.write(source.readAll()); +} + void DeckSyncManager::failSync(const QString &error) { syncing = false; diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h index c891cbb58..4d61c0d0a 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -38,6 +38,10 @@ class ServerInfo_DeckStorage_Folder; * * Ownership: fire-and-forget per run, driven by PendingCommand round trips. * No watcher, no auto triggers, no conflict UI — those arrive in later layers. + * + * Deletes are tombstones, never silent: a locally deleted deck is pushed with + * Command_DeckDel, a server-side delete is applied locally only after copying + * the file to a .sync-backup, and resurrection reuses the stored deck id. */ class DeckSyncManager : public QObject { @@ -78,6 +82,8 @@ private slots: onUploadFinished(const Response &response, const CommandContainer &commandContainer, const QVariant &extraData); void onDownloadFinished(const Response &response, const CommandContainer &commandContainer, const QVariant &extraData); + void + onDeleteFinished(const Response &response, const CommandContainer &commandContainer, const QVariant &extraData); private: struct ServerFile @@ -125,6 +131,8 @@ private: void sendMkDirFor(const QString &dir); void sendUpload(const UploadOp &op); void sendDownload(const DownloadOp &op); + void sendDelete(const QString &relativePath, int deckId); + void snapshotLocalFile(const QString &relativePath, const QString &guid); static QString parentDir(const QString &path); QString uniqueLocalPath(const QString &relativePath) const; @@ -139,6 +147,7 @@ private: QSet ensuredDirs; QList pendingUploads; QList pendingDownloads; + QList pendingDeletes; int uploadedCount = 0; int downloadedCount = 0; int unchangedCount = 0; diff --git a/servatrice/servatrice.ini.example b/servatrice/servatrice.ini.example index 4a26ba1d9..08046a489 100644 --- a/servatrice/servatrice.ini.example +++ b/servatrice/servatrice.ini.example @@ -598,3 +598,10 @@ max_shares_per_day=50 ; Minimum enforced: 1. ; Default: 20 max_versions=20 + +; How many days a deleted deck stays restorable as a tombstone before the +; cleanup clock purges it (with its versions) for good. Re-uploading the +; same deck within this window resurrects it instead of creating a copy. +; A value of 0 purges tombstones on the next cleanup run. +; Default: 30 +tombstone_days=30 diff --git a/servatrice/src/servatrice.cpp b/servatrice/src/servatrice.cpp index feb178b08..f5a1f0e93 100644 --- a/servatrice/src/servatrice.cpp +++ b/servatrice/src/servatrice.cpp @@ -622,6 +622,7 @@ bool Servatrice::initServer() deckShareCleanupClock = new QTimer(this); connect(deckShareCleanupClock, SIGNAL(timeout()), this, SLOT(cleanupExpiredDeckShares())); + connect(deckShareCleanupClock, SIGNAL(timeout()), this, SLOT(purgeDeckTombstones())); const int deckShareCleanupInterval = getDeckShareCleanupInterval(); if (deckShareCleanupInterval > 0) { qDebug() << "Starting deck share cleanup clock, interval" << deckShareCleanupInterval << "ms"; @@ -856,6 +857,11 @@ void Servatrice::cleanupExpiredDeckShares() servatriceDatabaseInterface->cleanupExpiredDeckShares(); } +void Servatrice::purgeDeckTombstones() +{ + servatriceDatabaseInterface->purgeDeckTombstones(getDeckSyncTombstoneDays()); +} + void Servatrice::statusUpdate() { if (!servatriceDatabaseInterface->checkSql()) { @@ -1401,6 +1407,11 @@ int Servatrice::getDeckSyncMaxVersions() const return qMax(1, settingsCache->value("deck_sync/max_versions", 20).toInt()); } +int Servatrice::getDeckSyncTombstoneDays() const +{ + return qMax(0, settingsCache->value("deck_sync/tombstone_days", 30).toInt()); +} + int Servatrice::getNumberOfTCPPools() const { return settingsCache->value("server/number_pools", 1).toInt(); diff --git a/servatrice/src/servatrice.h b/servatrice/src/servatrice.h index 3be364aa3..f925c0984 100644 --- a/servatrice/src/servatrice.h +++ b/servatrice/src/servatrice.h @@ -166,6 +166,7 @@ private slots: void sweepLegacyBackups(); void shutdownTimeout(); void cleanupExpiredDeckShares(); + void purgeDeckTombstones(); protected: void doSendIslMessage(const IslMessage &msg, int _serverId) override; @@ -321,6 +322,7 @@ public: int getDeckShareMaxDecksPerShare() const; int getDeckShareMaxSharesPerDay() const; int getDeckSyncMaxVersions() const; + int getDeckSyncTombstoneDays() const; int getMaxUsersPerAddress() const; /// Raw security/trusted_sources list; match addresses against it with isTrustedSource(). QString getTrustedSources() const; diff --git a/servatrice/src/servatrice_database_interface.cpp b/servatrice/src/servatrice_database_interface.cpp index b5c45065c..8d5459a88 100644 --- a/servatrice/src/servatrice_database_interface.cpp +++ b/servatrice/src/servatrice_database_interface.cpp @@ -1370,8 +1370,8 @@ DeckList *Servatrice_DatabaseInterface::getDeckFromDatabase(int deckId, int user { checkSql(); - QSqlQuery *query = - prepareQuery("select content from {prefix}_decklist_files where id = :id and id_user = :id_user"); + QSqlQuery *query = prepareQuery( + "select content from {prefix}_decklist_files where id = :id and id_user = :id_user and deleted = 0"); query->bindValue(":id", deckId); query->bindValue(":id_user", userId); execSqlQuery(query); @@ -1522,6 +1522,19 @@ void Servatrice_DatabaseInterface::cleanupExpiredDeckShares() execSqlQuery(query); } +void Servatrice_DatabaseInterface::purgeDeckTombstones(int olderThanDays) +{ + checkSql(); + + // DB-native clock: tombstones are stamped with NOW() on this same + // database, so the grace window is immune to app/db clock skew. + // The null arm only catches manually flagged rows. + QSqlQuery *query = prepareQuery("delete from {prefix}_decklist_files where deleted = 1 and (last_modified < " + "DATE_SUB(NOW(), INTERVAL :days DAY) or last_modified is null)"); + query->bindValue(":days", qMax(0, olderThanDays)); + execSqlQuery(query); +} + bool Servatrice_DatabaseInterface::getDeckSharesForUser(int userId, QList &shares) { checkSql(); diff --git a/servatrice/src/servatrice_database_interface.h b/servatrice/src/servatrice_database_interface.h index d0fd8bd12..04005e160 100644 --- a/servatrice/src/servatrice_database_interface.h +++ b/servatrice/src/servatrice_database_interface.h @@ -152,6 +152,11 @@ public: bool getDeckShareItem(const QString &token, int itemId, QString &content); /** @brief Deletes all expired share bundles (cascades to their items). */ void cleanupExpiredDeckShares(); + /** + * @brief Hard-deletes tombstoned decks older than the grace period (cascades to versions). + * @param olderThanDays Tombstones older than this many days are purged; 0 purges all tombstones. + */ + void purgeDeckTombstones(int olderThanDays); int getNextGameId() override; int getNextReplayId() override; diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index f9e602f82..36099e394 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -709,7 +709,8 @@ bool AbstractServerSocketInterface::deckListHelper(int folderId, query = sqlInterface->prepareQuery("select id, name, upload_time, is_public, banner_card_name, " "banner_card_provider, color_identity, tags, guid, hash, last_modified from " - "{prefix}_decklist_files where id_folder = :id_folder and id_user = :id_user"); + "{prefix}_decklist_files where id_folder = :id_folder and id_user = :id_user " + "and deleted = 0"); query->bindValue(":id_folder", folderId); query->bindValue(":id_user", userId); if (!sqlInterface->execSqlQuery(query)) { @@ -854,8 +855,8 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckSetVisibility(const sqlInterface->checkSql(); if (cmd.has_deck_id()) { - QSqlQuery *query = - sqlInterface->prepareQuery("select 1 from {prefix}_decklist_files where id = :id and id_user = :id_user"); + QSqlQuery *query = sqlInterface->prepareQuery( + "select 1 from {prefix}_decklist_files where id = :id and id_user = :id_user and deleted = 0"); query->bindValue(":id", cmd.deck_id()); query->bindValue(":id_user", userInfo->id()); sqlInterface->execSqlQuery(query); @@ -965,7 +966,8 @@ void AbstractServerSocketInterface::deckDelDirHelper(int basePathId) deckDelDirHelper(query->value(0).toInt()); } - query = sqlInterface->prepareQuery("delete from {prefix}_decklist_files where id_folder = :id_folder"); + query = sqlInterface->prepareQuery("update {prefix}_decklist_files set deleted = 1, last_modified = NOW() " + "where id_folder = :id_folder and deleted = 0"); query->bindValue(":id_folder", basePathId); sqlInterface->execSqlQuery(query); @@ -1016,18 +1018,17 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckDel(const Command_D sqlInterface->checkSql(); QSqlQuery *query = - sqlInterface->prepareQuery("select id from {prefix}_decklist_files where id = :id and id_user = :id_user"); + sqlInterface->prepareQuery("update {prefix}_decklist_files set deleted = 1, last_modified = NOW() where id = " + ":id and id_user = :id_user and deleted = 0"); query->bindValue(":id", cmd.deck_id()); query->bindValue(":id_user", userInfo->id()); - sqlInterface->execSqlQuery(query); - if (!query->next()) { + if (!sqlInterface->execSqlQuery(query)) { + return Response::RespContextError; + } + if (query->numRowsAffected() == 0) { return Response::RespNameNotFound; } - query = sqlInterface->prepareQuery("delete from {prefix}_decklist_files where id = :id"); - query->bindValue(":id", cmd.deck_id()); - sqlInterface->execSqlQuery(query); - return Response::RespOk; } @@ -1197,9 +1198,11 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckUpload(const Comman } const QString oldHash = currentQuery->value(1).toString(); - QString updateQuery = "update {prefix}_decklist_files set name=:name, upload_time=NOW(), content=:content, " - "banner_card_name=:banner_card_name, banner_card_provider=:banner_card_provider, " - "color_identity=:color_identity, tags=:tags, guid=:guid, hash=:hash, last_modified=NOW()"; + QString updateQuery = + "update {prefix}_decklist_files set name=:name, upload_time=NOW(), content=:content, " + "banner_card_name=:banner_card_name, banner_card_provider=:banner_card_provider, " + "color_identity=:color_identity, tags=:tags, guid=:guid, hash=:hash, last_modified=NOW(), " + "deleted=0"; if (cmd.has_is_public()) { updateQuery += ", is_public=:is_public"; } @@ -1401,7 +1404,7 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckShareCreate(const C // issues its own query on the same cached statement set. QSqlQuery *query = sqlInterface->prepareQuery("select id, color_identity from {prefix}_decklist_files where id_folder = " - ":id_folder and id_user = :id_user"); + ":id_folder and id_user = :id_user and deleted = 0"); query->bindValue(":id_folder", folderId); query->bindValue(":id_user", userInfo->id()); if (!sqlInterface->execSqlQuery(query)) { From 157e63b17995861aa9fc6f289f472aac3a14d58e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Tue, 13 Oct 2026 07:19:32 +0200 Subject: [PATCH 10/14] Fix ZeldaZach review for PR 7494: match by remoteId/guid before remote-delete, only tombstone when file truly missing --- cockatrice/src/interface/deck_sync/deck_sync_manager.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 7771c050a..43a0c10e4 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -336,6 +336,9 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont if (localFiles.contains(it.key())) { continue; } + if (QFileInfo::exists(deckPath + "/" + it.key())) { + continue; + } if (it.value().remoteId == -1) { sidecar.remove(it.key()); continue; From de3468cc336d8a1a43e710222bc70710315028bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 1 Nov 2026 13:21:05 +0100 Subject: [PATCH 11/14] [DeckSync] Address tombstone review: remote deletes, revival and purge clock - Treat a record whose server row is gone as a remote delete only when the local file still matches the sidecar's base hash. An edit made after the remote delete is newer than the delete, so it uploads and revives the row instead of being removed. - Revive a tombstone with the same user and guid in cmdDeckUpload instead of inserting a second row next to it, so the deck keeps its id and version history and the promise in servatrice.ini.example holds. - Run tombstone purging on its own clock, driven by the new [deck_sync] purge_interval option, so disabling share cleanup no longer disables purging silently. - Write the base snapshot through QSaveFile and log a failed commit, so a crash or a full disk cannot leave a truncated base for DeckDiff::merge(). --- .../interface/deck_sync/deck_sync_manager.cpp | 22 ++++- servatrice/servatrice.ini.example | 12 ++- servatrice/src/servatrice.cpp | 17 +++- servatrice/src/servatrice.h | 2 + servatrice/src/serversocketinterface.cpp | 93 ++++++++++++++----- 5 files changed, 112 insertions(+), 34 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 43a0c10e4..126bbca77 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -3,10 +3,12 @@ #include "../deck_loader/deck_loader.h" #include "deck_sync_sidecar.h" +#include #include #include #include #include +#include #include #include #include @@ -257,9 +259,14 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont } } if (serverId == -1) { - if (sidecarRecord.remoteId != -1) { - // The server no longer lists this deck: it was deleted elsewhere. - // Apply the delete locally, keeping a backup of the file. + // The sidecar knows a server row that the listing no longer has: it was + // deleted elsewhere. Only mirror that delete when the local file is + // still exactly what was synced - an edit made after the remote delete + // is newer than the delete and gets uploaded again. + const bool knownToServer = sidecarRecord.remoteId != -1 || !sidecarRecord.guid.isEmpty(); + const bool unchangedSinceSync = + !sidecarRecord.baseHash.isEmpty() && it.value().hash == sidecarRecord.baseHash; + if (knownToServer && unchangedSinceSync) { const QString absolutePath = deckPath + "/" + it.key(); const QString backupPath = absolutePath + BACKUP_SUFFIX + QString::number(QDateTime::currentDateTime().toSecsSinceEpoch()); @@ -678,11 +685,16 @@ void DeckSyncManager::snapshotLocalFile(const QString &relativePath, const QStri return; } QDir().mkpath(deckPath + "/" + SYNC_META_DIR + "/bases"); - QFile snapshot(deckPath + "/" + SYNC_META_DIR + "/bases/" + guid + ".cod"); - if (!snapshot.open(QIODevice::WriteOnly | QIODevice::Truncate)) { + // QSaveFile: a crash or a full disk must not leave a truncated base behind, + // because DeckDiff::merge() against a partial base re-adds or drops cards. + QSaveFile snapshot(deckPath + "/" + SYNC_META_DIR + "/bases/" + guid + ".cod"); + if (!snapshot.open(QIODevice::WriteOnly)) { return; } snapshot.write(source.readAll()); + if (!snapshot.commit()) { + qWarning() << "DeckSync: could not write the base snapshot for" << relativePath; + } } void DeckSyncManager::failSync(const QString &error) diff --git a/servatrice/servatrice.ini.example b/servatrice/servatrice.ini.example index 08046a489..5edb38695 100644 --- a/servatrice/servatrice.ini.example +++ b/servatrice/servatrice.ini.example @@ -600,8 +600,14 @@ max_shares_per_day=50 max_versions=20 ; How many days a deleted deck stays restorable as a tombstone before the -; cleanup clock purges it (with its versions) for good. Re-uploading the -; same deck within this window resurrects it instead of creating a copy. -; A value of 0 purges tombstones on the next cleanup run. +; purge clock removes it (with its versions) for good. Re-uploading the +; same deck within this window revives the tombstone instead of creating a copy. +; A value of 0 purges tombstones on the next purge run. ; Default: 30 tombstone_days=30 + +; How often, in minutes, the tombstone purge runs. This is independent of +; [deck_share] cleanup_interval, so disabling share cleanup does not disable +; purging. A value of 0 disables purging entirely. +; Default: 60 +purge_interval=60 diff --git a/servatrice/src/servatrice.cpp b/servatrice/src/servatrice.cpp index f5a1f0e93..34e5573c0 100644 --- a/servatrice/src/servatrice.cpp +++ b/servatrice/src/servatrice.cpp @@ -622,7 +622,6 @@ bool Servatrice::initServer() deckShareCleanupClock = new QTimer(this); connect(deckShareCleanupClock, SIGNAL(timeout()), this, SLOT(cleanupExpiredDeckShares())); - connect(deckShareCleanupClock, SIGNAL(timeout()), this, SLOT(purgeDeckTombstones())); const int deckShareCleanupInterval = getDeckShareCleanupInterval(); if (deckShareCleanupInterval > 0) { qDebug() << "Starting deck share cleanup clock, interval" << deckShareCleanupInterval << "ms"; @@ -633,6 +632,16 @@ bool Servatrice::initServer() connect(legacyBackupSweepClock, &QTimer::timeout, this, &Servatrice::sweepLegacyBackups); legacyBackupSweepClock->start(kLegacyBackupSweepIntervalMs); + // Tombstone purging runs on its own clock: turning share cleanup off must not + // silently disable it as well. + deckTombstoneClock = new QTimer(this); + connect(deckTombstoneClock, SIGNAL(timeout()), this, SLOT(purgeDeckTombstones())); + const int deckSyncPurgeInterval = getDeckSyncPurgeInterval(); + if (deckSyncPurgeInterval > 0) { + qDebug() << "Starting deck tombstone purge clock, interval" << deckSyncPurgeInterval << "ms"; + deckTombstoneClock->start(deckSyncPurgeInterval); + } + // SOCKET SERVER if (getNumberOfTCPPools() > 0) { gameServer = @@ -1412,6 +1421,12 @@ int Servatrice::getDeckSyncTombstoneDays() const return qMax(0, settingsCache->value("deck_sync/tombstone_days", 30).toInt()); } +int Servatrice::getDeckSyncPurgeInterval() const +{ + // default: every 60 minutes, independent of deck_share/cleanup_interval + return qMax(0, settingsCache->value("deck_sync/purge_interval", 60).toInt()) * 60000; +} + int Servatrice::getNumberOfTCPPools() const { return settingsCache->value("server/number_pools", 1).toInt(); diff --git a/servatrice/src/servatrice.h b/servatrice/src/servatrice.h index f925c0984..ed8d82ddb 100644 --- a/servatrice/src/servatrice.h +++ b/servatrice/src/servatrice.h @@ -182,6 +182,7 @@ private: QTimer *pingClock, *statusUpdateClock; QTimer *deckShareCleanupClock; QTimer *legacyBackupSweepClock; + QTimer *deckTombstoneClock; Servatrice_GameServer *gameServer; Servatrice_WebsocketGameServer *websocketGameServer; Servatrice_IslServer *islServer; @@ -323,6 +324,7 @@ public: int getDeckShareMaxSharesPerDay() const; int getDeckSyncMaxVersions() const; int getDeckSyncTombstoneDays() const; + int getDeckSyncPurgeInterval() const; int getMaxUsersPerAddress() const; /// Raw security/trusted_sources list; match addresses against it with isTrustedSource(). QString getTrustedSources() const; diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index 36099e394..e88c74527 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -1129,31 +1129,71 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckUpload(const Comman return Response::RespNameNotFound; } - QSqlQuery *query = - sqlInterface->prepareQuery("insert into {prefix}_decklist_files (id_folder, id_user, name, upload_time, " - "content, is_public, banner_card_name, banner_card_provider, color_identity, " - "tags, guid, hash, last_modified) values(:id_folder, :id_user, :name, NOW(), " - ":content, :is_public, :banner_card_name, :banner_card_provider, " - ":color_identity, :tags, :guid, :hash, NOW())"); - query->bindValue(":id_folder", folderId); - query->bindValue(":id_user", userInfo->id()); - query->bindValue(":name", deckName); - query->bindValue(":content", deckStr); - query->bindValue(":is_public", cmd.has_is_public() && cmd.is_public() ? 1 : 0); - query->bindValue(":banner_card_name", bannerCardName); - query->bindValue(":banner_card_provider", bannerCardProvider); - query->bindValue(":color_identity", colorIdentity); - query->bindValue(":tags", tagsJson); - query->bindValue(":guid", guid); - query->bindValue(":hash", deckHash); - if (!sqlInterface->execSqlQuery(query)) { + // A deck that was deleted elsewhere leaves a tombstone behind. Revive it + // instead of inserting a second row with the same guid, which would split + // the deck's version history and leave the tombstone to be purged. + int deckId = -1; + bool inserted = false; + QSqlQuery *tombstoneQuery = + sqlInterface->prepareQuery("select id from {prefix}_decklist_files where id_user = :id_user and guid = " + ":guid and deleted = 1 order by last_modified desc limit 1"); + tombstoneQuery->bindValue(":id_user", userInfo->id()); + tombstoneQuery->bindValue(":guid", guid); + if (!sqlInterface->execSqlQuery(tombstoneQuery)) { return Response::RespContextError; } - const int deckId = query->lastInsertId().toInt(); + if (tombstoneQuery->next()) { + deckId = tombstoneQuery->value(0).toInt(); + QSqlQuery *reviveQuery = sqlInterface->prepareQuery( + "update {prefix}_decklist_files set id_folder=:id_folder, name=:name, upload_time=NOW(), " + "content=:content, is_public=:is_public, banner_card_name=:banner_card_name, " + "banner_card_provider=:banner_card_provider, color_identity=:color_identity, tags=:tags, " + "hash=:hash, last_modified=NOW(), deleted=0 where id=:id and id_user=:id_user"); + reviveQuery->bindValue(":id_folder", folderId); + reviveQuery->bindValue(":id_user", userInfo->id()); + reviveQuery->bindValue(":id", deckId); + reviveQuery->bindValue(":name", deckName); + reviveQuery->bindValue(":content", deckStr); + reviveQuery->bindValue(":is_public", cmd.has_is_public() && cmd.is_public() ? 1 : 0); + reviveQuery->bindValue(":banner_card_name", bannerCardName); + reviveQuery->bindValue(":banner_card_provider", bannerCardProvider); + reviveQuery->bindValue(":color_identity", colorIdentity); + reviveQuery->bindValue(":tags", tagsJson); + reviveQuery->bindValue(":hash", deckHash); + if (!sqlInterface->execSqlQuery(reviveQuery)) { + return Response::RespContextError; + } + } else { + QSqlQuery *query = sqlInterface->prepareQuery( + "insert into {prefix}_decklist_files (id_folder, id_user, name, upload_time, " + "content, is_public, banner_card_name, banner_card_provider, color_identity, " + "tags, guid, hash, last_modified) values(:id_folder, :id_user, :name, NOW(), " + ":content, :is_public, :banner_card_name, :banner_card_provider, " + ":color_identity, :tags, :guid, :hash, NOW())"); + query->bindValue(":id_folder", folderId); + query->bindValue(":id_user", userInfo->id()); + query->bindValue(":name", deckName); + query->bindValue(":content", deckStr); + query->bindValue(":is_public", cmd.has_is_public() && cmd.is_public() ? 1 : 0); + query->bindValue(":banner_card_name", bannerCardName); + query->bindValue(":banner_card_provider", bannerCardProvider); + query->bindValue(":color_identity", colorIdentity); + query->bindValue(":tags", tagsJson); + query->bindValue(":guid", guid); + query->bindValue(":hash", deckHash); + if (!sqlInterface->execSqlQuery(query)) { + return Response::RespContextError; + } + deckId = query->lastInsertId().toInt(); + inserted = true; + } if (!storeDeckVersion(deckId, deckStr, deckHash)) { - QSqlQuery *cleanupQuery = sqlInterface->prepareQuery("delete from {prefix}_decklist_files where id = :id"); - cleanupQuery->bindValue(":id", deckId); - sqlInterface->execSqlQuery(cleanupQuery); + if (inserted) { + QSqlQuery *cleanupQuery = + sqlInterface->prepareQuery("delete from {prefix}_decklist_files where id = :id"); + cleanupQuery->bindValue(":id", deckId); + sqlInterface->execSqlQuery(cleanupQuery); + } return Response::RespContextError; } @@ -1163,9 +1203,12 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckUpload(const Comman stampQuery->bindValue(":id", deckId); stampQuery->bindValue(":id_user", userInfo->id()); if (!sqlInterface->execSqlQuery(stampQuery) || !stampQuery->next()) { - QSqlQuery *cleanupQuery = sqlInterface->prepareQuery("delete from {prefix}_decklist_files where id = :id"); - cleanupQuery->bindValue(":id", deckId); - sqlInterface->execSqlQuery(cleanupQuery); + if (inserted) { + QSqlQuery *cleanupQuery = + sqlInterface->prepareQuery("delete from {prefix}_decklist_files where id = :id"); + cleanupQuery->bindValue(":id", deckId); + sqlInterface->execSqlQuery(cleanupQuery); + } return Response::RespContextError; } From d2e2e722e79bce1a0924f368bf39e0aeb70cb421 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 11 Oct 2026 16:46:46 +0200 Subject: [PATCH 12/14] [Client] Sync conflict dialog, onboarding and conflict default --- cockatrice/CMakeLists.txt | 2 + .../src/client/settings/cache_settings.cpp | 7 + .../src/client/settings/cache_settings.h | 3 + .../interface/deck_sync/deck_sync_conflict.h | 50 +++ .../interface/deck_sync/deck_sync_manager.cpp | 401 ++++++++++++++++-- .../interface/deck_sync/deck_sync_manager.h | 68 ++- .../interface/deck_sync/deck_sync_sidecar.cpp | 9 + .../interface/deck_sync/deck_sync_sidecar.h | 9 +- .../widgets/dialogs/dlg_sync_conflict.cpp | 178 ++++++++ .../widgets/dialogs/dlg_sync_conflict.h | 63 +++ .../widgets/dialogs/dlg_sync_onboarding.cpp | 59 +++ .../widgets/dialogs/dlg_sync_onboarding.h | 43 ++ .../settings_page/general_settings_page.cpp | 29 ++ .../settings_page/general_settings_page.h | 4 + .../widgets/tabs/tab_deck_storage.cpp | 38 ++ .../interface/widgets/tabs/tab_deck_storage.h | 4 + libcockatrice_settings/CMakeLists.txt | 2 + .../settings/deck_sync_settings.cpp | 21 + .../settings/deck_sync_settings.h | 38 ++ tests/deck_sync_sidecar_test.cpp | 20 + tests/settings/settings_defaults_test.cpp | 20 + 21 files changed, 1020 insertions(+), 48 deletions(-) create mode 100644 cockatrice/src/interface/deck_sync/deck_sync_conflict.h create mode 100644 cockatrice/src/interface/widgets/dialogs/dlg_sync_conflict.cpp create mode 100644 cockatrice/src/interface/widgets/dialogs/dlg_sync_conflict.h create mode 100644 cockatrice/src/interface/widgets/dialogs/dlg_sync_onboarding.cpp create mode 100644 cockatrice/src/interface/widgets/dialogs/dlg_sync_onboarding.h create mode 100644 libcockatrice_settings/libcockatrice/settings/deck_sync_settings.cpp create mode 100644 libcockatrice_settings/libcockatrice/settings/deck_sync_settings.h diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index 5519d2f3b..7ffe97994 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -58,6 +58,8 @@ set(cockatrice_SOURCES src/interface/widgets/dialogs/dlg_shared_decks_preview.cpp src/interface/widgets/dialogs/dlg_settings.cpp src/interface/widgets/dialogs/dlg_startup_card_check.cpp + src/interface/widgets/dialogs/dlg_sync_conflict.cpp + src/interface/widgets/dialogs/dlg_sync_onboarding.cpp src/interface/widgets/dialogs/dlg_tip_of_the_day.cpp src/interface/widgets/dialogs/dlg_update.cpp src/interface/widgets/dialogs/dlg_view_log.cpp diff --git a/cockatrice/src/client/settings/cache_settings.cpp b/cockatrice/src/client/settings/cache_settings.cpp index 08eccf1e4..6848e7b04 100644 --- a/cockatrice/src/client/settings/cache_settings.cpp +++ b/cockatrice/src/client/settings/cache_settings.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -159,6 +160,7 @@ SettingsCache::SettingsCache() cardsDisplaySettings = new CardsDisplaySettings(settingsPath, this); interfaceSettings = new InterfaceSettings(settingsPath, this); deckEditorSettings = new DeckEditorSettings(settingsPath, this); + deckSyncSettings = new DeckSyncSettings(settingsPath, this); pathsSettings = new PathsSettings(settingsPath, this); visualDeckStorageSettings = new VisualDeckStorageSettings(settingsPath, this); appearanceSettings = new AppearanceSettings(settingsPath, this); @@ -460,6 +462,11 @@ DeckEditorSettings &SettingsCache::deckEditor() const return *deckEditorSettings; } +DeckSyncSettings &SettingsCache::deckSync() const +{ + return *deckSyncSettings; +} + VisualDeckStorageSettings &SettingsCache::visualDeckStorage() const { return *visualDeckStorageSettings; diff --git a/cockatrice/src/client/settings/cache_settings.h b/cockatrice/src/client/settings/cache_settings.h index 9aeb88a18..97198466f 100644 --- a/cockatrice/src/client/settings/cache_settings.h +++ b/cockatrice/src/client/settings/cache_settings.h @@ -32,6 +32,7 @@ class ChatSettings; class CommanderBracketSettings; class DebugSettings; class DeckEditorSettings; +class DeckSyncSettings; class DownloadSettings; class GameFiltersSettings; class GameSettings; @@ -81,6 +82,7 @@ private: CardsDisplaySettings *cardsDisplaySettings = nullptr; InterfaceSettings *interfaceSettings = nullptr; DeckEditorSettings *deckEditorSettings = nullptr; + DeckSyncSettings *deckSyncSettings = nullptr; PathsSettings *pathsSettings = nullptr; VisualDeckStorageSettings *visualDeckStorageSettings = nullptr; AppearanceSettings *appearanceSettings = nullptr; @@ -149,6 +151,7 @@ public: [[nodiscard]] CardsDisplaySettings &cardsDisplay() const; [[nodiscard]] InterfaceSettings &userInterface() const; [[nodiscard]] DeckEditorSettings &deckEditor() const; + [[nodiscard]] DeckSyncSettings &deckSync() const; [[nodiscard]] PathsSettings &paths() const; [[nodiscard]] VisualDeckStorageSettings &visualDeckStorage() const; [[nodiscard]] AppearanceSettings &appearance() const; diff --git a/cockatrice/src/interface/deck_sync/deck_sync_conflict.h b/cockatrice/src/interface/deck_sync/deck_sync_conflict.h new file mode 100644 index 000000000..a5b272ecd --- /dev/null +++ b/cockatrice/src/interface/deck_sync/deck_sync_conflict.h @@ -0,0 +1,50 @@ +/** + * @file deck_sync_conflict.h + * @brief Conflict payload between the sync engine and the conflict dialog. + */ + +#ifndef DECK_SYNC_CONFLICT_H +#define DECK_SYNC_CONFLICT_H + +#include + +/** @brief How a both-sides-changed deck is resolved. */ +enum DeckSyncResolutionAction +{ + DeckSyncKeepMine, ///< Upload the local deck, discarding the server version. + DeckSyncKeepServer, ///< Download the server deck, discarding local changes. + DeckSyncKeepBoth, ///< Keep the local deck and store the server version aside. + DeckSyncUseMerge, ///< Upload the auto-merged deck (only when mergeable). + DeckSyncSkipConflict, ///< Leave both sides untouched for this run. +}; + +/** @brief Sync direction for one run. */ +enum class DeckSyncMode +{ + TwoWay, ///< Mirror both sides (default). + UploadOnly, ///< Push local content; never download or propagate deletes. + DownloadOnly, ///< Take server content; never upload or push deletes. + Separate, ///< First-sync verdict only: keep this profile out of sync. +}; + +/** @brief One deck that changed both locally and on the server. */ +struct DeckSyncConflict +{ + QString relativePath; ///< Local deck path relative to the deck folder. + QString localAbsolutePath; ///< Absolute local file path. + QString serverDeckString; ///< Server deck content in native format. + QString baseDeckString; ///< Last-synced content, empty when no snapshot exists. + bool hasBase = false; ///< False when the base snapshot is missing. + qint64 localSecs = 0; ///< Local modification time, seconds since epoch. + qint64 serverSecs = 0; ///< Server modification time, seconds since epoch. +}; + +/** @brief The dialog verdict for one DeckSyncConflict. */ +struct DeckSyncResolution +{ + DeckSyncResolutionAction action = DeckSyncKeepMine; + bool rememberSession = false; ///< Reuse this verdict for later conflicts this run. + bool rememberAlways = false; ///< Store this verdict as the conflict default setting. +}; + +#endif // DECK_SYNC_CONFLICT_H diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 126bbca77..566138141 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -90,6 +91,16 @@ void DeckSyncManager::setDeckPath(const QString &path) deckPath = path; } +void DeckSyncManager::setConflictDefault(int _conflictDefault) +{ + conflictDefault = _conflictDefault; +} + +void DeckSyncManager::setSyncMode(DeckSyncMode mode) +{ + syncMode = mode; +} + bool DeckSyncManager::isSyncing() const { return syncing; @@ -118,18 +129,29 @@ void DeckSyncManager::syncNow() pendingUploads.clear(); pendingDownloads.clear(); pendingDeletes.clear(); + conflictPaths.clear(); + stashedServerFiles.clear(); + activeConflict = ActiveConflict(); ensuredDirs.clear(); ensuredDirs.insert(QString()); localFiles.clear(); + sessionDefault = -1; + syncMode = DeckSyncMode::TwoWay; + firstSyncRun = false; emit syncStarted(); emit statusMessage(tr("Scanning local decks...")); - sidecarPath = deckDir.filePath(SYNC_META_DIR + QString("/") + profile.key() + ".json"); + sidecarPath = DeckSyncSidecar::pathForDeckPath(deckPath, profile.key()); DeckSyncSidecar sidecar(sidecarPath); if (!sidecar.load()) { failSync(tr("Could not read sync state.")); return; } + if (sidecar.mode == DeckSyncSidecar::MODE_SEPARATE) { + failSync(tr("Sync is disabled for this server.")); + return; + } + firstSyncRun = !QFileInfo(sidecarPath).exists(); QDirIterator it(deckPath, QDir::Files | QDir::NoDotAndDotDot, QDirIterator::Subdirectories); while (it.hasNext()) { @@ -200,6 +222,47 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont const Response_DeckList &list = response.GetExtension(Response_DeckList::ext); collectServerFiles(list.root(), QString(), serverFiles, serverDirs); ensuredDirs.unite(serverDirs); + + if (firstSyncRun && !localFiles.isEmpty() && !serverFiles.isEmpty()) { + stashedServerFiles = serverFiles; + emit firstSyncDetected(localFiles.size(), serverFiles.size()); + return; + } + classify(serverFiles); +} + +void DeckSyncManager::continueFirstSync(DeckSyncMode mode) +{ + if (!syncing || !firstSyncRun) { + return; + } + firstSyncRun = false; + if (mode == DeckSyncMode::Separate) { + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + sidecar.mode = DeckSyncSidecar::MODE_SEPARATE; + sidecar.save(); + stashedServerFiles.clear(); + failSync(tr("Sync is disabled for this server.")); + return; + } + syncMode = mode; + classify(stashedServerFiles); + stashedServerFiles.clear(); +} + +void DeckSyncManager::cancelFirstSync() +{ + if (!syncing || !firstSyncRun) { + return; + } + firstSyncRun = false; + stashedServerFiles.clear(); + failSync(tr("Sync cancelled.")); +} + +void DeckSyncManager::classify(const QMap &serverFiles) +{ emit statusMessage(tr("Comparing %1 local and %2 server decks...").arg(localFiles.size()).arg(serverFiles.size())); DeckSyncSidecar sidecar(sidecarPath); @@ -267,6 +330,9 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont const bool unchangedSinceSync = !sidecarRecord.baseHash.isEmpty() && it.value().hash == sidecarRecord.baseHash; if (knownToServer && unchangedSinceSync) { + if (syncMode == DeckSyncMode::UploadOnly) { + continue; + } const QString absolutePath = deckPath + "/" + it.key(); const QString backupPath = absolutePath + BACKUP_SUFFIX + QString::number(QDateTime::currentDateTime().toSecsSinceEpoch()); @@ -278,6 +344,9 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont ++downloadedCount; continue; } + if (syncMode == DeckSyncMode::DownloadOnly) { + continue; + } const QString serverPath = serverPathFor(it.key()); pendingUploads.append(UploadOp{.relativePath = it.key(), .serverPath = serverPath, @@ -304,53 +373,57 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont continue; } const qint64 localSecs = it.value().modified.toSecsSinceEpoch(); + const int effective = sessionDefault >= 0 ? sessionDefault : conflictDefault; // A server row without last_modified predates sync: the local file wins. // Otherwise strictly newer wins; ties go to the server. - if (server.lastModifiedSecs == 0 || localSecs > server.lastModifiedSecs) { - pendingUploads.append(UploadOp{.relativePath = it.key(), - .serverPath = server.path, - .serverDir = server.dir, - .deckId = server.id, - .guid = server.guid}); + if (syncMode == DeckSyncMode::UploadOnly || effective == DeckSyncConflictKeepMine) { + applyUpload(it.key(), server.path, server.id, server.guid); + } else if (syncMode == DeckSyncMode::DownloadOnly || effective == DeckSyncConflictKeepServer) { + applyDownload(it.key(), server.id, server.guid, server.hash, server.lastModifiedSecs); + } else if (effective == DeckSyncConflictKeepNewer && + (server.lastModifiedSecs == 0 || localSecs > server.lastModifiedSecs)) { + applyUpload(it.key(), server.path, server.id, server.guid); + } else if (effective == DeckSyncConflictKeepNewer) { + applyDownload(it.key(), server.id, server.guid, server.hash, server.lastModifiedSecs); } else { - pendingDownloads.append(DownloadOp{.relativePath = it.key(), - .deckId = server.id, - .guid = server.guid, - .hash = server.hash, - .lastModifiedSecs = server.lastModifiedSecs}); + queueConflict(it.key(), server.path, server.id, server.guid, server.hash, server.lastModifiedSecs); } } - for (auto it = serverFiles.begin(); it != serverFiles.end(); ++it) { - if (matchedServerIds.contains(it.key())) { - continue; + if (syncMode != DeckSyncMode::UploadOnly) { + for (auto it = serverFiles.begin(); it != serverFiles.end(); ++it) { + if (matchedServerIds.contains(it.key())) { + continue; + } + const QString wantedPath = localPathFor(it.value().dir, it.value().name); + const QString relativePath = uniqueLocalPath(wantedPath); + if (relativePath.isEmpty()) { + ++skippedCount; + continue; + } + pendingDownloads.append(DownloadOp{.relativePath = relativePath, + .deckId = it.value().id, + .guid = it.value().guid, + .hash = it.value().hash, + .lastModifiedSecs = it.value().lastModifiedSecs}); } - const QString wantedPath = localPathFor(it.value().dir, it.value().name); - const QString relativePath = uniqueLocalPath(wantedPath); - if (relativePath.isEmpty()) { - ++skippedCount; - continue; - } - pendingDownloads.append(DownloadOp{.relativePath = relativePath, - .deckId = it.value().id, - .guid = it.value().guid, - .hash = it.value().hash, - .lastModifiedSecs = it.value().lastModifiedSecs}); } - const QMap knownRecords = sidecar.records(); - for (auto it = knownRecords.begin(); it != knownRecords.end(); ++it) { - if (localFiles.contains(it.key())) { - continue; + if (syncMode != DeckSyncMode::DownloadOnly) { + const QMap knownRecords = sidecar.records(); + for (auto it = knownRecords.begin(); it != knownRecords.end(); ++it) { + if (localFiles.contains(it.key())) { + continue; + } + if (QFileInfo::exists(deckPath + "/" + it.key())) { + continue; + } + if (it.value().remoteId == -1) { + sidecar.remove(it.key()); + continue; + } + pendingDeletes.append(it.key()); } - if (QFileInfo::exists(deckPath + "/" + it.key())) { - continue; - } - if (it.value().remoteId == -1) { - sidecar.remove(it.key()); - continue; - } - pendingDeletes.append(it.key()); } sidecar.save(); @@ -365,7 +438,7 @@ QString DeckSyncManager::parentDir(const QString &path) void DeckSyncManager::processNext() { - if (!syncing) { + if (!syncing || activeConflict.active) { return; } if (!pendingDeletes.isEmpty()) { @@ -382,6 +455,220 @@ void DeckSyncManager::processNext() } } +void DeckSyncManager::applyUpload(const QString &relativePath, + const QString &serverPath, + int deckId, + const QString &guid) +{ + UploadOp op; + op.relativePath = relativePath; + op.serverPath = serverPath; + op.deckId = deckId; + op.guid = guid; + pendingUploads.append(op); +} + +void DeckSyncManager::applyDownload(const QString &relativePath, + int deckId, + const QString &guid, + const QString &hash, + qint64 lastModifiedSecs) +{ + DownloadOp op; + op.relativePath = relativePath; + op.deckId = deckId; + op.guid = guid; + op.hash = hash; + op.lastModifiedSecs = lastModifiedSecs; + pendingDownloads.append(op); +} + +QString DeckSyncManager::readBaseSnapshot(const QString &guid) const +{ + if (guid.isEmpty()) { + return {}; + } + QFile snapshot(deckPath + "/" + SYNC_META_DIR + "/bases/" + guid + ".cod"); + if (!snapshot.open(QIODevice::ReadOnly)) { + return {}; + } + return QString::fromUtf8(snapshot.readAll()); +} + +void DeckSyncManager::queueConflict(const QString &relativePath, + const QString &serverPath, + int deckId, + const QString &serverGuid, + const QString &serverHash, + qint64 serverLastModifiedSecs) +{ + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + const QString guid = !serverGuid.isEmpty() ? serverGuid : sidecar.get(relativePath).guid; + if (readBaseSnapshot(guid).isEmpty()) { + emit statusMessage(tr("No base snapshot for %1, keeping the newer version.").arg(relativePath)); + const qint64 localSecs = localFiles.value(relativePath).modified.toSecsSinceEpoch(); + if (serverLastModifiedSecs == 0 || localSecs > serverLastModifiedSecs) { + applyUpload(relativePath, serverPath, deckId, serverGuid); + } else { + applyDownload(relativePath, deckId, serverGuid, serverHash, serverLastModifiedSecs); + } + return; + } + ConflictDownload ctx; + ctx.serverPath = serverPath; + ctx.deckId = deckId; + ctx.guid = serverGuid; + ctx.hash = serverHash; + ctx.lastModifiedSecs = serverLastModifiedSecs; + pendingConflicts.insert(relativePath, ctx); + conflictPaths.insert(relativePath); + applyDownload(relativePath, deckId, serverGuid, serverHash, serverLastModifiedSecs); +} + +void DeckSyncManager::handleConflictDownload(const QString &relativePath, const QString &serverDeckString) +{ + const ConflictDownload ctx = pendingConflicts.take(relativePath); + if (sessionDefault == DeckSyncConflictKeepMine) { + applyUpload(relativePath, ctx.serverPath, ctx.deckId, ctx.guid); + processNext(); + return; + } + if (sessionDefault == DeckSyncConflictKeepServer) { + applyDownloadedDeck(relativePath, ctx.deckId, serverDeckString, ctx.guid, ctx.hash, ctx.lastModifiedSecs, + ctx.lastModifiedSecs != 0); + return; + } + std::optional loaded = + DeckLoader::loadFromFile(localFiles.value(relativePath).absolutePath, DeckFileFormat::Cockatrice); + DeckList serverDeck; + if (!loaded || !serverDeck.loadFromString_Native(serverDeckString)) { + ++skippedCount; + processNext(); + return; + } + + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + const QString guid = !ctx.guid.isEmpty() ? ctx.guid : sidecar.get(relativePath).guid; + activeConflict.active = true; + activeConflict.relativePath = relativePath; + activeConflict.serverPath = ctx.serverPath; + activeConflict.deckId = ctx.deckId; + activeConflict.serverGuid = ctx.guid; + activeConflict.serverHash = ctx.hash; + activeConflict.serverLastModifiedSecs = ctx.lastModifiedSecs; + activeConflict.serverDeckString = serverDeckString; + + DeckSyncConflict conflict; + conflict.relativePath = relativePath; + conflict.localAbsolutePath = localFiles.value(relativePath).absolutePath; + conflict.serverDeckString = serverDeckString; + conflict.baseDeckString = readBaseSnapshot(guid); + conflict.hasBase = !conflict.baseDeckString.isEmpty(); + conflict.localSecs = localFiles.value(relativePath).modified.toSecsSinceEpoch(); + conflict.serverSecs = ctx.lastModifiedSecs; + emit conflictRequest(conflict); +} + +void DeckSyncManager::resolveConflict(const DeckSyncResolution &resolution) +{ + if (!syncing || !activeConflict.active) { + return; + } + if (resolution.rememberSession && + (resolution.action == DeckSyncKeepMine || resolution.action == DeckSyncKeepServer)) { + sessionDefault = resolution.action == DeckSyncKeepMine ? DeckSyncConflictKeepMine : DeckSyncConflictKeepServer; + } + const ActiveConflict conflict = activeConflict; + activeConflict.active = false; + + switch (resolution.action) { + case DeckSyncKeepMine: + applyUpload(conflict.relativePath, conflict.serverPath, conflict.deckId, conflict.serverGuid); + break; + case DeckSyncKeepServer: + applyDownload(conflict.relativePath, conflict.deckId, conflict.serverGuid, conflict.serverHash, + conflict.serverLastModifiedSecs); + break; + case DeckSyncKeepBoth: { + // Persist the already-fetched server bytes synchronously: queuing a + // download here would race the upload below and fetch back Mine. + // The aside copy is a new local file, so it gets a fresh guid and + // uploads as its own deck on the next run. + QString asideBase = conflict.relativePath; + if (asideBase.endsWith(".cod", Qt::CaseInsensitive)) { + asideBase.chop(4); + } + const QString aside = uniqueLocalPath(asideBase + " (server).cod"); + DeckList serverDeck; + if (aside.isEmpty() || !serverDeck.loadFromString_Native(conflict.serverDeckString)) { + emit statusMessage(tr("Could not store the server version of %1 aside, keeping the local deck.") + .arg(conflict.relativePath)); + ++skippedCount; + } else { + const QString asideAbsolute = deckPath + "/" + aside; + QDir().mkpath(QFileInfo(asideAbsolute).absolutePath()); + if (!DeckLoader::saveToFile(serverDeck, asideAbsolute, DeckFileFormat::Cockatrice)) { + ++skippedCount; + } else { + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + DeckSyncRecord asideRecord; + asideRecord.guid = QUuid::createUuid().toString(QUuid::WithoutBraces); + asideRecord.baseHash = serverDeck.getDeckHash(); + sidecar.set(aside, asideRecord); + sidecar.save(); + snapshotLocalFile(aside, asideRecord.guid); + } + } + applyUpload(conflict.relativePath, conflict.serverPath, conflict.deckId, conflict.serverGuid); + break; + } + case DeckSyncUseMerge: { + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + const QString guid = + !conflict.serverGuid.isEmpty() ? conflict.serverGuid : sidecar.get(conflict.relativePath).guid; + DeckList localDeck; + DeckList serverDeck; + DeckList baseDeck; + std::optional loaded = DeckLoader::loadFromFile( + localFiles.value(conflict.relativePath).absolutePath, DeckFileFormat::Cockatrice); + if (!loaded || !serverDeck.loadFromString_Native(conflict.serverDeckString) || + !baseDeck.loadFromString_Native(readBaseSnapshot(guid))) { + emit statusMessage( + tr("Could not build the merge for %1, keeping the local deck.").arg(conflict.relativePath)); + applyUpload(conflict.relativePath, conflict.serverPath, conflict.deckId, conflict.serverGuid); + break; + } + localDeck = loaded->deckList; + const DeckDiffResult localChanges = DeckDiff::diff(baseDeck, localDeck); + const DeckDiffResult remoteChanges = DeckDiff::diff(baseDeck, serverDeck); + const DeckList merged = DeckDiff::merge(baseDeck, localChanges, remoteChanges); + const QString absolutePath = deckPath + "/" + conflict.relativePath; + const QString backupPath = + absolutePath + BACKUP_SUFFIX + QString::number(QDateTime::currentDateTime().toSecsSinceEpoch()); + if (!QFile::copy(absolutePath, backupPath) || + !DeckLoader::saveToFile(merged, absolutePath, DeckFileFormat::Cockatrice)) { + ++skippedCount; + break; + } + std::optional reloaded = DeckLoader::loadFromFile(absolutePath, DeckFileFormat::Cockatrice); + if (reloaded) { + localFiles[conflict.relativePath].hash = reloaded->deckList.getDeckHash(); + localFiles[conflict.relativePath].modified = QFileInfo(absolutePath).lastModified(); + } + applyUpload(conflict.relativePath, conflict.serverPath, conflict.deckId, conflict.serverGuid); + break; + } + case DeckSyncSkipConflict: + ++skippedCount; + break; + } + processNext(); +} + void DeckSyncManager::processNextUpload() { const UploadOp op = pendingUploads.first(); @@ -573,8 +860,10 @@ void DeckSyncManager::onDownloadFinished(const Response &response, return; } const QString relativePath = extraData.toString(); + int finishedDeckId = -1; for (int i = 0; i < pendingDownloads.size(); ++i) { if (pendingDownloads.at(i).relativePath == relativePath) { + finishedDeckId = pendingDownloads.at(i).deckId; pendingDownloads.removeAt(i); break; } @@ -586,8 +875,26 @@ void DeckSyncManager::onDownloadFinished(const Response &response, } const Response_DeckDownload &download = response.GetExtension(Response_DeckDownload::ext); + if (conflictPaths.contains(relativePath)) { + conflictPaths.remove(relativePath); + handleConflictDownload(relativePath, QString::fromStdString(download.deck())); + return; + } + applyDownloadedDeck(relativePath, finishedDeckId, QString::fromStdString(download.deck()), + QString::fromStdString(download.guid()), QString::fromStdString(download.hash()), + static_cast(download.last_modified()), download.has_last_modified()); +} + +void DeckSyncManager::applyDownloadedDeck(const QString &relativePath, + int deckId, + const QString &deckString, + const QString &guid, + const QString &hash, + qint64 lastModifiedSecs, + bool hasLastModified) +{ DeckList deck; - if (!deck.loadFromString_Native(QString::fromStdString(download.deck()))) { + if (!deck.loadFromString_Native(deckString)) { ++skippedCount; processNext(); return; @@ -619,16 +926,18 @@ void DeckSyncManager::onDownloadFinished(const Response &response, DeckSyncSidecar sidecar(sidecarPath); sidecar.load(); DeckSyncRecord record = sidecar.get(relativePath); - if (!download.guid().empty()) { - record.guid = QString::fromStdString(download.guid()); + if (deckId != -1) { + record.remoteId = deckId; } - if (!download.hash().empty()) { - record.baseHash = QString::fromStdString(download.hash()); + if (!guid.isEmpty()) { + record.guid = guid; + } + if (!hash.isEmpty()) { + record.baseHash = hash; } else { record.baseHash = deck.getContentHash(); } - record.lastSyncSecs = download.has_last_modified() ? static_cast(download.last_modified()) - : QDateTime::currentDateTime().toSecsSinceEpoch(); + record.lastSyncSecs = hasLastModified ? lastModifiedSecs : QDateTime::currentDateTime().toSecsSinceEpoch(); record.deleted = false; sidecar.set(relativePath, record); sidecar.save(); diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h index 4d61c0d0a..389763243 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -6,6 +6,7 @@ #ifndef DECK_SYNC_MANAGER_H #define DECK_SYNC_MANAGER_H +#include "deck_sync_conflict.h" #include "deck_sync_profile.h" #include @@ -16,6 +17,7 @@ #include #include #include +#include class AbstractClient; class CommandContainer; @@ -37,7 +39,10 @@ class ServerInfo_DeckStorage_Folder; * to it. * * Ownership: fire-and-forget per run, driven by PendingCommand round trips. - * No watcher, no auto triggers, no conflict UI — those arrive in later layers. + * Both-sides-changed decks pause the queue for a verdict via conflictRequest / + * resolveConflict (or the conflict default setting); the first sync of an + * unknown profile pauses for an onboarding verdict via firstSyncDetected / + * continueFirstSync. No watcher and no auto triggers — those arrive later. * * Deletes are tombstones, never silent: a locally deleted deck is pushed with * Command_DeckDel, a server-side delete is applied locally only after copying @@ -64,15 +69,27 @@ public: void setProfile(const DeckSyncProfile &_profile); void setDeckPath(const QString &path); + void setConflictDefault(int conflictDefault); + void setSyncMode(DeckSyncMode mode); [[nodiscard]] bool isSyncing() const; public slots: void syncNow(); + /** @brief Continues a paused first sync after the onboarding verdict. */ + void continueFirstSync(DeckSyncMode mode); + /** @brief Aborts a paused first sync without writing any verdict. */ + void cancelFirstSync(); + /** @brief Applies the dialog verdict for the active conflict and resumes. */ + void resolveConflict(const DeckSyncResolution &resolution); signals: void syncStarted(); void statusMessage(const QString &message); void syncFinished(const SyncSummary &summary, const QString &error); + /** @brief First sync with content on both sides: the tab must ask how to proceed. */ + void firstSyncDetected(int localCount, int serverCount); + /** @brief A both-sides-changed deck needs a verdict; the queue pauses until resolveConflict. */ + void conflictRequest(const DeckSyncConflict &conflict); private slots: void onListFinished(const Response &response, const CommandContainer &commandContainer); @@ -118,6 +135,25 @@ private: QString hash; qint64 lastModifiedSecs = 0; }; + struct ActiveConflict + { + bool active = false; + QString relativePath; + QString serverPath; + int deckId = -1; + QString serverGuid; + QString serverHash; + qint64 serverLastModifiedSecs = 0; + QString serverDeckString; + }; + struct ConflictDownload + { + QString serverPath; + int deckId = -1; + QString guid; + QString hash; + qint64 lastModifiedSecs = 0; + }; void failSync(const QString &error); void finishSync(); @@ -133,6 +169,28 @@ private: void sendDownload(const DownloadOp &op); void sendDelete(const QString &relativePath, int deckId); void snapshotLocalFile(const QString &relativePath, const QString &guid); + QString readBaseSnapshot(const QString &guid) const; + void classify(const QMap &serverFiles); + void queueConflict(const QString &relativePath, + const QString &serverPath, + int deckId, + const QString &serverGuid, + const QString &serverHash, + qint64 serverLastModifiedSecs); + void handleConflictDownload(const QString &relativePath, const QString &serverDeckString); + void applyDownloadedDeck(const QString &relativePath, + int deckId, + const QString &deckString, + const QString &guid, + const QString &hash, + qint64 lastModifiedSecs, + bool hasLastModified); + void applyUpload(const QString &relativePath, const QString &serverPath, int deckId, const QString &guid); + void applyDownload(const QString &relativePath, + int deckId, + const QString &guid, + const QString &hash, + qint64 lastModifiedSecs); static QString parentDir(const QString &path); QString uniqueLocalPath(const QString &relativePath) const; @@ -148,6 +206,14 @@ private: QList pendingUploads; QList pendingDownloads; QList pendingDeletes; + QSet conflictPaths; + QMap pendingConflicts; + QMap stashedServerFiles; + ActiveConflict activeConflict; + DeckSyncMode syncMode = DeckSyncMode::TwoWay; + int conflictDefault = DeckSyncConflictAsk; + int sessionDefault = -1; + bool firstSyncRun = false; int uploadedCount = 0; int downloadedCount = 0; int unchangedCount = 0; diff --git a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp index 2e3a5ba49..6c5a65e73 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp @@ -16,9 +16,15 @@ DeckSyncSidecar::DeckSyncSidecar(const QString &_filePath) : filePath(_filePath) { } +QString DeckSyncSidecar::pathForDeckPath(const QString &deckPath, const QString &profileKey) +{ + return QDir(deckPath).filePath(".sync/" + profileKey + ".json"); +} + bool DeckSyncSidecar::load() { recordMap.clear(); + mode = MODE_LINKED; QFile file(filePath); if (!file.exists()) { @@ -31,6 +37,8 @@ bool DeckSyncSidecar::load() if (!document.isObject() || !document.object().value("records").isObject()) { return false; } + const QJsonValue modeValue = document.object().value("mode"); + mode = modeValue.isString() ? modeValue.toString() : MODE_LINKED; const QJsonObject records = document.object().value("records").toObject(); for (auto it = records.begin(); it != records.end(); ++it) { if (!it.value().isObject()) { @@ -62,6 +70,7 @@ bool DeckSyncSidecar::save() const } QJsonObject root; root.insert("version", SIDECAR_FORMAT_VERSION); + root.insert("mode", mode); root.insert("records", records); QDir().mkpath(QFileInfo(filePath).absolutePath()); diff --git a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.h b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.h index 7eb2db7ae..2266922bc 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.h @@ -30,10 +30,17 @@ struct DeckSyncRecord class DeckSyncSidecar { public: - QString filePath; ///< Full path of the sidecar JSON file. + static constexpr const char *MODE_LINKED = "linked"; + static constexpr const char *MODE_SEPARATE = "separate"; + + QString filePath; ///< Full path of the sidecar JSON file. + QString mode = MODE_LINKED; ///< Linked normally; separate keeps this profile out of sync. explicit DeckSyncSidecar(const QString &_filePath); + /** @brief Sidecar file path for a deck folder and profile key. */ + static QString pathForDeckPath(const QString &deckPath, const QString &profileKey); + /** @brief Loads records; a missing file loads as empty and returns true. */ bool load(); /** @brief Stores records, creating parent folders as needed. */ diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_sync_conflict.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_sync_conflict.cpp new file mode 100644 index 000000000..7d548993d --- /dev/null +++ b/cockatrice/src/interface/widgets/dialogs/dlg_sync_conflict.cpp @@ -0,0 +1,178 @@ +#include "dlg_sync_conflict.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace +{ +QString formatTime(qint64 secsSinceEpoch) +{ + return QLocale().toString(QDateTime::fromSecsSinceEpoch(secsSinceEpoch), QLocale::ShortFormat); +} + +int zoneCardCount(const DeckList &deck, const QString &zone) +{ + int count = 0; + deck.forEachCard([&count, &zone](InnerDecklistNode *zoneNode, DecklistCardNode *card) { + if (zoneNode->getName() == zone) { + count += card->getNumber(); + } + }); + return count; +} +} // namespace + +DlgSyncConflict::DlgSyncConflict(const DeckSyncConflict &_conflict, QWidget *parent) + : QDialog(parent), conflict(_conflict) +{ + auto *layout = new QVBoxLayout(this); + + titleLabel = new QLabel(this); + detailLabel = new QLabel(this); + detailLabel->setWordWrap(true); + keepMineRadio = new QRadioButton(this); + keepServerRadio = new QRadioButton(this); + keepBothRadio = new QRadioButton(this); + mergeRadio = new QRadioButton(this); + changesList = new QListWidget(this); + mergeSummaryLabel = new QLabel(this); + mergeSummaryLabel->setWordWrap(true); + rememberSessionBox = new QCheckBox(this); + rememberAlwaysBox = new QCheckBox(this); + + layout->addWidget(titleLabel); + layout->addWidget(detailLabel); + layout->addWidget(keepMineRadio); + layout->addWidget(keepServerRadio); + layout->addWidget(keepBothRadio); + layout->addWidget(mergeRadio); + layout->addWidget(changesList); + layout->addWidget(mergeSummaryLabel); + layout->addWidget(rememberSessionBox); + layout->addWidget(rememberAlwaysBox); + + auto *buttons = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel, this); + connect(buttons, &QDialogButtonBox::accepted, this, &QDialog::accept); + connect(buttons, &QDialogButtonBox::rejected, this, &QDialog::reject); + layout->addWidget(buttons); + + DeckList localDeck; + DeckList serverDeck; + QFile localFile(conflict.localAbsolutePath); + decksParsed = localFile.open(QIODevice::ReadOnly) && localDeck.loadFromFile_Native(&localFile) && + serverDeck.loadFromString_Native(conflict.serverDeckString); + localIsNewer = conflict.localSecs >= conflict.serverSecs; + + if (decksParsed) { + shownChanges = DeckDiff::diff(localDeck, serverDeck); + if (conflict.hasBase) { + DeckList baseDeck; + if (baseDeck.loadFromString_Native(conflict.baseDeckString)) { + const DeckDiffResult localChanges = DeckDiff::diff(baseDeck, localDeck); + const DeckDiffResult remoteChanges = DeckDiff::diff(baseDeck, serverDeck); + mergeable = DeckDiff::canAutoMerge(localChanges, remoteChanges); + if (mergeable) { + const DeckList merged = DeckDiff::merge(baseDeck, localChanges, remoteChanges); + mergedMainCount = zoneCardCount(merged, "main"); + mergedSideCount = zoneCardCount(merged, "side"); + } + } + } + } + + if (localIsNewer) { + keepMineRadio->setChecked(true); + } else { + keepServerRadio->setChecked(true); + } + mergeRadio->setEnabled(mergeable); + + connect(keepMineRadio, &QRadioButton::toggled, this, &DlgSyncConflict::updateRememberBoxes); + connect(keepServerRadio, &QRadioButton::toggled, this, &DlgSyncConflict::updateRememberBoxes); + connect(keepBothRadio, &QRadioButton::toggled, this, &DlgSyncConflict::updateRememberBoxes); + connect(mergeRadio, &QRadioButton::toggled, this, &DlgSyncConflict::updateRememberBoxes); + updateRememberBoxes(); + retranslateUi(); +} + +void DlgSyncConflict::retranslateUi() +{ + setWindowTitle(tr("Sync conflict")); + titleLabel->setText(conflict.relativePath); + if (!decksParsed) { + detailLabel->setText(tr("This deck changed on both sides, but one version could not be read.")); + } else { + detailLabel->setText(tr("This deck changed both on this device and on the server.")); + } + + keepMineRadio->setText( + tr("Keep my deck (%1%2)").arg(formatTime(conflict.localSecs), localIsNewer ? tr(", newest") : QString())); + keepServerRadio->setText(tr("Keep the server deck (%1%2)") + .arg(formatTime(conflict.serverSecs), localIsNewer ? QString() : tr(", newest"))); + keepBothRadio->setText(tr("Keep both (store the server version aside)")); + mergeRadio->setText(mergeable ? tr("Use the auto-merge") : tr("Use the auto-merge (not available)")); + + changesList->clear(); + for (const DeckCardChange &change : shownChanges.changes) { + changesList->addItem( + tr("%1: %2 %3 → %4").arg(change.zone).arg(change.cardName).arg(change.oldCount).arg(change.newCount)); + } + + if (mergeable) { + mergeSummaryLabel->setText(tr("Auto-merge proposal: union of both sides (%1 main, %2 side).") + .arg(mergedMainCount) + .arg(mergedSideCount)); + } else if (!conflict.hasBase) { + mergeSummaryLabel->setText(tr("No base snapshot exists, so the versions cannot be merged automatically.")); + } else { + mergeSummaryLabel->setText(tr("The changes overlap, so they cannot be merged automatically.")); + } + + rememberSessionBox->setText(tr("Remember for the rest of this sync")); + rememberAlwaysBox->setText(tr("Always use this rule")); +} + +void DlgSyncConflict::updateRememberBoxes() +{ + const bool single = keepMineRadio->isChecked() || keepServerRadio->isChecked(); + rememberSessionBox->setEnabled(single); + rememberAlwaysBox->setEnabled(single); + if (!single) { + rememberSessionBox->setChecked(false); + rememberAlwaysBox->setChecked(false); + } +} + +DeckSyncResolutionAction DlgSyncConflict::selectedAction() const +{ + if (keepServerRadio->isChecked()) { + return DeckSyncKeepServer; + } + if (keepBothRadio->isChecked()) { + return DeckSyncKeepBoth; + } + if (mergeRadio->isChecked()) { + return DeckSyncUseMerge; + } + return DeckSyncKeepMine; +} + +DeckSyncResolution DlgSyncConflict::resolution() const +{ + DeckSyncResolution result; + result.action = selectedAction(); + result.rememberSession = rememberSessionBox->isEnabled() && rememberSessionBox->isChecked(); + result.rememberAlways = rememberAlwaysBox->isEnabled() && rememberAlwaysBox->isChecked(); + return result; +} diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_sync_conflict.h b/cockatrice/src/interface/widgets/dialogs/dlg_sync_conflict.h new file mode 100644 index 000000000..0a969f53e --- /dev/null +++ b/cockatrice/src/interface/widgets/dialogs/dlg_sync_conflict.h @@ -0,0 +1,63 @@ +/** + * @file dlg_sync_conflict.h + * @brief Dialog for resolving a deck that changed both locally and on the server. + */ + +#ifndef DLG_SYNC_CONFLICT_H +#define DLG_SYNC_CONFLICT_H + +#include "../../deck_sync/deck_sync_conflict.h" + +#include +#include +#include + +class QCheckBox; +class QLabel; +class QListWidget; +class QRadioButton; + +/** + * @class DlgSyncConflict + * @brief Shows both versions of a conflicted deck and returns the verdict. + * + * Radio choice between Keep mine / Keep server (newest preselected), Keep both + * and Use merge (enabled only when the changes touch disjoint cards). The + * change list shows card-count deltas between the two versions; the merge + * summary describes the auto-merge proposal. Rejected dialogs resolve as Skip. + */ +class DlgSyncConflict : public QDialog +{ + Q_OBJECT +public: + explicit DlgSyncConflict(const DeckSyncConflict &conflict, QWidget *parent = nullptr); + [[nodiscard]] DeckSyncResolution resolution() const; + void retranslateUi(); + +private slots: + void updateRememberBoxes(); + +private: + DeckSyncResolutionAction selectedAction() const; + + DeckSyncConflict conflict; + bool decksParsed = false; + bool localIsNewer = true; + DeckDiffResult shownChanges; + bool mergeable = false; + int mergedMainCount = 0; + int mergedSideCount = 0; + + QLabel *titleLabel; + QLabel *detailLabel; + QRadioButton *keepMineRadio; + QRadioButton *keepServerRadio; + QRadioButton *keepBothRadio; + QRadioButton *mergeRadio; + QListWidget *changesList; + QLabel *mergeSummaryLabel; + QCheckBox *rememberSessionBox; + QCheckBox *rememberAlwaysBox; +}; + +#endif // DLG_SYNC_CONFLICT_H diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_sync_onboarding.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_sync_onboarding.cpp new file mode 100644 index 000000000..ae0e9446b --- /dev/null +++ b/cockatrice/src/interface/widgets/dialogs/dlg_sync_onboarding.cpp @@ -0,0 +1,59 @@ +#include "dlg_sync_onboarding.h" + +#include +#include +#include +#include + +DlgSyncOnboarding::DlgSyncOnboarding(int _localCount, int _serverCount, QWidget *parent) + : QDialog(parent), localCount(_localCount), serverCount(_serverCount) +{ + auto *layout = new QVBoxLayout(this); + + detailLabel = new QLabel(this); + detailLabel->setWordWrap(true); + mergeRadio = new QRadioButton(this); + uploadRadio = new QRadioButton(this); + downloadRadio = new QRadioButton(this); + separateRadio = new QRadioButton(this); + mergeRadio->setChecked(true); + + layout->addWidget(detailLabel); + layout->addWidget(mergeRadio); + layout->addWidget(uploadRadio); + layout->addWidget(downloadRadio); + layout->addWidget(separateRadio); + + auto *buttons = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel, this); + connect(buttons, &QDialogButtonBox::accepted, this, &QDialog::accept); + connect(buttons, &QDialogButtonBox::rejected, this, &QDialog::reject); + layout->addWidget(buttons); + + retranslateUi(); +} + +void DlgSyncOnboarding::retranslateUi() +{ + setWindowTitle(tr("Sync decks to this server")); + detailLabel->setText(tr("You have %1 local decks and %2 decks stored on this server. How should they be combined?") + .arg(localCount) + .arg(serverCount)); + mergeRadio->setText(tr("Merge: keep everything, sync both ways (recommended)")); + uploadRadio->setText(tr("Upload mine: push local decks to the server")); + downloadRadio->setText(tr("Download server: take the server decks")); + separateRadio->setText(tr("Keep separate: do not sync with this server")); +} + +DeckSyncMode DlgSyncOnboarding::selectedMode() const +{ + if (uploadRadio->isChecked()) { + return DeckSyncMode::UploadOnly; + } + if (downloadRadio->isChecked()) { + return DeckSyncMode::DownloadOnly; + } + if (separateRadio->isChecked()) { + return DeckSyncMode::Separate; + } + return DeckSyncMode::TwoWay; +} diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_sync_onboarding.h b/cockatrice/src/interface/widgets/dialogs/dlg_sync_onboarding.h new file mode 100644 index 000000000..9d811a28e --- /dev/null +++ b/cockatrice/src/interface/widgets/dialogs/dlg_sync_onboarding.h @@ -0,0 +1,43 @@ +/** + * @file dlg_sync_onboarding.h + * @brief First-sync choice when local decks and server decks meet. + */ + +#ifndef DLG_SYNC_ONBOARDING_H +#define DLG_SYNC_ONBOARDING_H + +#include "../../deck_sync/deck_sync_conflict.h" + +#include + +class QLabel; +class QRadioButton; + +/** + * @class DlgSyncOnboarding + * @brief Asks how to combine a non-empty local collection with a non-empty server collection. + * + * Merge (two-way) is the default. Upload and download restrict the run to one + * direction; keeping separate records the profile as sync-disabled. + * Rejected dialogs cancel the sync without recording anything. + */ +class DlgSyncOnboarding : public QDialog +{ + Q_OBJECT +public: + DlgSyncOnboarding(int _localCount, int _serverCount, QWidget *parent = nullptr); + [[nodiscard]] DeckSyncMode selectedMode() const; + void retranslateUi(); + +private: + int localCount = 0; + int serverCount = 0; + + QLabel *detailLabel; + QRadioButton *mergeRadio; + QRadioButton *uploadRadio; + QRadioButton *downloadRadio; + QRadioButton *separateRadio; +}; + +#endif // DLG_SYNC_ONBOARDING_H diff --git a/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp b/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp index 7b89b744d..6b6678467 100644 --- a/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp +++ b/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -359,6 +360,25 @@ GeneralSettingsPage::GeneralSettingsPage() pathsGroupBox = new QGroupBox; pathsGroupBox->setLayout(pathsGrid); + // deck sync settings + conflictDefaultSelector.addItem("", DeckSyncConflictAsk); + conflictDefaultSelector.addItem("", DeckSyncConflictKeepMine); + conflictDefaultSelector.addItem("", DeckSyncConflictKeepServer); + conflictDefaultSelector.addItem("", DeckSyncConflictKeepNewer); + int conflictIndex = conflictDefaultSelector.findData(settings.deckSync().getConflictDefault()); + if (conflictIndex >= 0) { + conflictDefaultSelector.setCurrentIndex(conflictIndex); + } + connect(&conflictDefaultSelector, qOverload(&QComboBox::currentIndexChanged), this, [this](int index) { + SettingsCache::instance().deckSync().setConflictDefault(conflictDefaultSelector.itemData(index).toInt()); + }); + + auto *deckSyncGrid = new QGridLayout; + deckSyncGrid->addWidget(&conflictDefaultLabel, 0, 0); + deckSyncGrid->addWidget(&conflictDefaultSelector, 0, 1); + deckSyncGroupBox = new QGroupBox; + deckSyncGroupBox->setLayout(deckSyncGrid); + auto *mainLayout = new QVBoxLayout; mainLayout->addWidget(languageGroupBox); mainLayout->addWidget(versionGroupBox); @@ -366,6 +386,7 @@ GeneralSettingsPage::GeneralSettingsPage() mainLayout->addWidget(cardDatabaseGroupBox); mainLayout->addWidget(startupGroupBox); mainLayout->addWidget(pathsGroupBox); + mainLayout->addWidget(deckSyncGroupBox); mainLayout->addStretch(); GeneralSettingsPage::retranslateUi(); @@ -647,5 +668,13 @@ void GeneralSettingsPage::retranslateUi() cardDatabasePathLabel.setText(tr("Card database:")); customCardDatabasePathLabel.setText(tr("Custom database directory:")); tokenDatabasePathLabel.setText(tr("Token database:")); + deckSyncGroupBox->setTitle(tr("Deck sync")); + conflictDefaultLabel.setText(tr("When a deck changed on both sides:")); + conflictDefaultSelector.setItemText(conflictDefaultSelector.findData(DeckSyncConflictAsk), tr("Ask me every time")); + conflictDefaultSelector.setItemText(conflictDefaultSelector.findData(DeckSyncConflictKeepMine), tr("Keep my deck")); + conflictDefaultSelector.setItemText(conflictDefaultSelector.findData(DeckSyncConflictKeepServer), + tr("Keep the server deck")); + conflictDefaultSelector.setItemText(conflictDefaultSelector.findData(DeckSyncConflictKeepNewer), + tr("Keep the newer deck")); resetAllPathsButton->setText(tr("Reset all paths")); } diff --git a/cockatrice/src/interface/widgets/settings_page/general_settings_page.h b/cockatrice/src/interface/widgets/settings_page/general_settings_page.h index 9451db434..92d46c265 100644 --- a/cockatrice/src/interface/widgets/settings_page/general_settings_page.h +++ b/cockatrice/src/interface/widgets/settings_page/general_settings_page.h @@ -52,6 +52,7 @@ private: QGroupBox *cardDatabaseGroupBox; QGroupBox *startupGroupBox; QGroupBox *pathsGroupBox; + QGroupBox *deckSyncGroupBox; QLabel languageLabel; QComboBox languageBox; @@ -104,6 +105,9 @@ private: QLineEdit *tokenDatabasePathEdit; QPushButton *resetAllPathsButton; QLabel *allPathsResetLabel; + + QLabel conflictDefaultLabel; + QComboBox conflictDefaultSelector; }; #endif // COCKATRICE_GENERAL_SETTINGS_PAGE_H diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp index 2ca46d4f4..cc68f9fc1 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp @@ -2,11 +2,15 @@ #include "../../../client/settings/cache_settings.h" #include "../../deck_loader/deck_loader.h" +#include "../../deck_sync/deck_sync_conflict.h" #include "../../deck_sync/deck_sync_manager.h" +#include "../../deck_sync/deck_sync_profile.h" #include "../../pixel_map_generator.h" #include "../cards/additional_info/deck_color_identity.h" #include "../deck_share/deck_share_utils.h" #include "../deck_share/share_bar_widget.h" +#include "../dialogs/dlg_sync_conflict.h" +#include "../dialogs/dlg_sync_onboarding.h" #include "../interface/widgets/server/remote/remote_decklist_tree_widget.h" #include "../interface/widgets/utility/get_text_with_max.h" #include "abstract_client.h" @@ -63,6 +67,7 @@ #include #include #include +#include #include #include #include @@ -256,8 +261,13 @@ TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, syncManager = new DeckSyncManager( client, [](const DeckList &deck) { return getDeckColorIdentity(deck, CardDatabaseManager::query()); }, this); + syncManager->setConflictDefault(SettingsCache::instance().deckSync().getConflictDefault()); + connect(&SettingsCache::instance().deckSync(), &DeckSyncSettings::conflictDefaultChanged, syncManager, + &DeckSyncManager::setConflictDefault); connect(syncManager, &DeckSyncManager::statusMessage, this, &TabDeckStorage::onSyncStatusMessage); connect(syncManager, &DeckSyncManager::syncFinished, this, &TabDeckStorage::onSyncFinished); + connect(syncManager, &DeckSyncManager::firstSyncDetected, this, &TabDeckStorage::onFirstSyncDetected); + connect(syncManager, &DeckSyncManager::conflictRequest, this, &TabDeckStorage::onConflictRequest); if (currentUserInfo != nullptr) { syncUsername = QString::fromStdString(currentUserInfo->name()); } @@ -558,6 +568,34 @@ void TabDeckStorage::actSyncNow() syncManager->syncNow(); } +void TabDeckStorage::onFirstSyncDetected(int localCount, int serverCount) +{ + DlgSyncOnboarding dialog(localCount, serverCount, this); + if (dialog.exec() == QDialog::Accepted) { + syncManager->continueFirstSync(dialog.selectedMode()); + } else { + syncManager->cancelFirstSync(); + } +} + +void TabDeckStorage::onConflictRequest(const DeckSyncConflict &conflict) +{ + DlgSyncConflict dialog(conflict, this); + if (dialog.exec() != QDialog::Accepted) { + DeckSyncResolution skip; + skip.action = DeckSyncSkipConflict; + syncManager->resolveConflict(skip); + return; + } + const DeckSyncResolution resolution = dialog.resolution(); + if (resolution.rememberAlways && + (resolution.action == DeckSyncKeepMine || resolution.action == DeckSyncKeepServer)) { + SettingsCache::instance().deckSync().setConflictDefault( + resolution.action == DeckSyncKeepMine ? DeckSyncConflictKeepMine : DeckSyncConflictKeepServer); + } + syncManager->resolveConflict(resolution); +} + void TabDeckStorage::onSyncStatusMessage(const QString &message) { syncStatusType = SyncStatusType::Working; diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h index c7456877e..610cce891 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h @@ -20,6 +20,8 @@ #include struct LoadedDeck; +struct DeckSyncConflict; +struct DeckSyncResolution; class ServerInfo_User; class QTreeView; class QFileSystemModel; @@ -128,6 +130,8 @@ private slots: void deleteDeckFinished(const Response &response, const CommandContainer &commandContainer); void actSyncNow(); + void onFirstSyncDetected(int localCount, int serverCount); + void onConflictRequest(const DeckSyncConflict &conflict); void onSyncStatusMessage(const QString &message); void onSyncFinished(const SyncSummary &summary, const QString &error); diff --git a/libcockatrice_settings/CMakeLists.txt b/libcockatrice_settings/CMakeLists.txt index d23fb3350..7772d812a 100644 --- a/libcockatrice_settings/CMakeLists.txt +++ b/libcockatrice_settings/CMakeLists.txt @@ -12,6 +12,7 @@ set(HEADERS libcockatrice/settings/commander_bracket_settings.h libcockatrice/settings/debug_settings.h libcockatrice/settings/deck_editor_settings.h + libcockatrice/settings/deck_sync_settings.h libcockatrice/settings/download_settings.h libcockatrice/settings/game_filters_settings.h libcockatrice/settings/game_settings.h @@ -43,6 +44,7 @@ add_library( libcockatrice/settings/chat_settings.cpp libcockatrice/settings/debug_settings.cpp libcockatrice/settings/deck_editor_settings.cpp + libcockatrice/settings/deck_sync_settings.cpp libcockatrice/settings/download_settings.cpp libcockatrice/settings/game_filters_settings.cpp libcockatrice/settings/game_settings.cpp diff --git a/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.cpp b/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.cpp new file mode 100644 index 000000000..0b86d3c84 --- /dev/null +++ b/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.cpp @@ -0,0 +1,21 @@ +#include "deck_sync_settings.h" + +#include "libcockatrice/settings/settings_manager.h" + +#include + +DeckSyncSettings::DeckSyncSettings(const QString &settingPath, QObject *parent) + : SettingsManager(settingPath + "deck_sync.ini", "decksync", QString(), parent) +{ +} + +int DeckSyncSettings::getConflictDefault() const +{ + return getValue("conflictDefault", QString(), QString(), DeckSyncConflictAsk).toInt(); +} + +void DeckSyncSettings::setConflictDefault(int _conflictDefault) +{ + setValue(_conflictDefault, "conflictDefault"); + emit conflictDefaultChanged(_conflictDefault); +} diff --git a/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.h b/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.h new file mode 100644 index 000000000..f984292f3 --- /dev/null +++ b/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.h @@ -0,0 +1,38 @@ +#ifndef DECK_SYNC_SETTINGS_H +#define DECK_SYNC_SETTINGS_H + +#include "settings_manager.h" + +#include +#include + +class QObject; + +enum DeckSyncConflictDefault +{ + DeckSyncConflictAsk = 0, ///< Ask every time a deck changed on both sides. + DeckSyncConflictKeepMine, ///< Silently keep the local deck. + DeckSyncConflictKeepServer, ///< Silently keep the server deck. + DeckSyncConflictKeepNewer, ///< Silently keep whichever side changed last. +}; + +class DeckSyncSettings : public SettingsManager +{ + Q_OBJECT + friend class SettingsCache; + +public: + [[nodiscard]] int getConflictDefault() const; + void setConflictDefault(int _conflictDefault); + +signals: + void conflictDefaultChanged(int conflictDefault); + +public: + explicit DeckSyncSettings(const QString &settingPath, QObject *parent = nullptr); + +private: + DeckSyncSettings(const DeckSyncSettings & /*other*/); +}; + +#endif // DECK_SYNC_SETTINGS_H diff --git a/tests/deck_sync_sidecar_test.cpp b/tests/deck_sync_sidecar_test.cpp index 4bd04be33..76e18d797 100644 --- a/tests/deck_sync_sidecar_test.cpp +++ b/tests/deck_sync_sidecar_test.cpp @@ -119,6 +119,26 @@ TEST(DeckSyncSidecarTest, RemoveDropsRecord) EXPECT_FALSE(reloaded.contains("a.cod")); } +TEST(DeckSyncSidecarTest, ModeRoundTripsAndDefaultsToLinked) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + DeckSyncSidecar sidecar(sidecarPath(dir)); + EXPECT_EQ(sidecar.mode, DeckSyncSidecar::MODE_LINKED); + sidecar.mode = DeckSyncSidecar::MODE_SEPARATE; + EXPECT_TRUE(sidecar.save()); + + DeckSyncSidecar reloaded(sidecarPath(dir)); + EXPECT_TRUE(reloaded.load()); + EXPECT_EQ(reloaded.mode, DeckSyncSidecar::MODE_SEPARATE); +} + +TEST(DeckSyncSidecarTest, PathForDeckPathBuildsSidecarPath) +{ + QString path = DeckSyncSidecar::pathForDeckPath("/decks", "abc123"); + EXPECT_EQ(path, QString("/decks/.sync/abc123.json")); +} + int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv); diff --git a/tests/settings/settings_defaults_test.cpp b/tests/settings/settings_defaults_test.cpp index aea7f1d54..0a2fb31e0 100644 --- a/tests/settings/settings_defaults_test.cpp +++ b/tests/settings/settings_defaults_test.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -586,6 +587,25 @@ TEST_F(SettingsDefaultsTest, DeckEditor_VdeStartupTab_SetAndGet) ASSERT_EQ(s.getVdeStartupTab(), VdeStartupTabContext); } +// --- DeckSyncSettings --- + +TEST_F(SettingsDefaultsTest, DeckSync_ConflictDefault_Default) +{ + DeckSyncSettings s(settingsPath, nullptr); + ASSERT_EQ(s.getConflictDefault(), DeckSyncConflictAsk); +} + +TEST_F(SettingsDefaultsTest, DeckSync_ConflictDefault_SetAndGet) +{ + DeckSyncSettings s(settingsPath, nullptr); + s.setConflictDefault(DeckSyncConflictKeepNewer); + ASSERT_EQ(s.getConflictDefault(), DeckSyncConflictKeepNewer); + s.setConflictDefault(DeckSyncConflictKeepMine); + ASSERT_EQ(s.getConflictDefault(), DeckSyncConflictKeepMine); + s.setConflictDefault(DeckSyncConflictAsk); + ASSERT_EQ(s.getConflictDefault(), DeckSyncConflictAsk); +} + // --- NetworkSettings --- TEST_F(SettingsDefaultsTest, Network_ClientID_Default) From 9e4d734373a2f1fec146f83353e355551a0845b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Tue, 13 Oct 2026 07:19:55 +0200 Subject: [PATCH 13/14] Fix ZeldaZach review for PR 7495: three-way baseHash check before conflict --- .../interface/deck_sync/deck_sync_manager.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 566138141..ebed690a0 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -372,6 +372,24 @@ void DeckSyncManager::classify(const QMap &serverFiles) ++unchangedCount; continue; } + const DeckSyncRecord baseRecord = sidecar.get(it.key()); + const bool localChanged = baseRecord.baseHash.isEmpty() || baseRecord.baseHash != it.value().hash; + const bool serverChanged = baseRecord.baseHash.isEmpty() || baseRecord.baseHash != serverIt.value().hash; + if (localChanged && !serverChanged) { + if (syncMode == DeckSyncMode::DownloadOnly) { + continue; + } + applyUpload(it.key(), serverPath, serverIt.value().id, serverIt.value().guid); + continue; + } + if (!localChanged && serverChanged) { + if (syncMode == DeckSyncMode::UploadOnly) { + continue; + } + applyDownload(it.key(), serverIt.value().id, serverIt.value().guid, serverIt.value().hash, + serverIt.value().lastModifiedSecs); + continue; + } const qint64 localSecs = it.value().modified.toSecsSinceEpoch(); const int effective = sessionDefault >= 0 ? sessionDefault : conflictDefault; // A server row without last_modified predates sync: the local file wins. From dcccb0f3321ed7bece6093cbe38d2c37d713a1cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 1 Nov 2026 05:08:27 +0100 Subject: [PATCH 14/14] Fix remaining review threads on the conflict classification - Remove the duplicated download loop and the stray '<' plus extra brace that left the head unbuildable. - Skip the fallback upload under DownloadMode so a local-only deck is not pushed when the user asked for downloads only. - Treat a server row without a hash as unknown rather than changed, so pre-migration decks are not re-downloaded on every sync. --- .../src/interface/deck_sync/deck_sync_manager.cpp | 12 +++++++----- .../src/interface/deck_sync/deck_sync_manager.h | 4 ++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index ebed690a0..60dd34879 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -261,7 +261,7 @@ void DeckSyncManager::cancelFirstSync() failSync(tr("Sync cancelled.")); } -void DeckSyncManager::classify(const QMap &serverFiles) +void DeckSyncManager::classify(const QMap &serverFiles) { emit statusMessage(tr("Comparing %1 local and %2 server decks...").arg(localFiles.size()).arg(serverFiles.size())); @@ -374,20 +374,22 @@ void DeckSyncManager::classify(const QMap &serverFiles) } const DeckSyncRecord baseRecord = sidecar.get(it.key()); const bool localChanged = baseRecord.baseHash.isEmpty() || baseRecord.baseHash != it.value().hash; - const bool serverChanged = baseRecord.baseHash.isEmpty() || baseRecord.baseHash != serverIt.value().hash; + // A server row without a hash predates sync: treat it as unknown rather than changed, + // otherwise every sync re-downloads the deck. + const bool serverChanged = + !server.hash.isEmpty() && (baseRecord.baseHash.isEmpty() || baseRecord.baseHash != server.hash); if (localChanged && !serverChanged) { if (syncMode == DeckSyncMode::DownloadOnly) { continue; } - applyUpload(it.key(), serverPath, serverIt.value().id, serverIt.value().guid); + applyUpload(it.key(), server.path, server.id, server.guid); continue; } if (!localChanged && serverChanged) { if (syncMode == DeckSyncMode::UploadOnly) { continue; } - applyDownload(it.key(), serverIt.value().id, serverIt.value().guid, serverIt.value().hash, - serverIt.value().lastModifiedSecs); + applyDownload(it.key(), server.id, server.guid, server.hash, server.lastModifiedSecs); continue; } const qint64 localSecs = it.value().modified.toSecsSinceEpoch(); diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h index 389763243..eecfceb7d 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -170,7 +170,7 @@ private: void sendDelete(const QString &relativePath, int deckId); void snapshotLocalFile(const QString &relativePath, const QString &guid); QString readBaseSnapshot(const QString &guid) const; - void classify(const QMap &serverFiles); + void classify(const QMap &serverFiles); void queueConflict(const QString &relativePath, const QString &serverPath, int deckId, @@ -208,7 +208,7 @@ private: QList pendingDeletes; QSet conflictPaths; QMap pendingConflicts; - QMap stashedServerFiles; + QMap stashedServerFiles; ActiveConflict activeConflict; DeckSyncMode syncMode = DeckSyncMode::TwoWay; int conflictDefault = DeckSyncConflictAsk;