From 293a1d615c0187e906453e7df904ffb320acc554 Mon Sep 17 00:00:00 2001 From: Mick Sayson Date: Wed, 3 Oct 2018 23:45:01 -0700 Subject: [PATCH 1/2] fix(transfer): Accurately represent pause state in UI Toxcore has a 3 state pause, us, them, or both. Currently our UI messes up if both parties pause. This changeset changes our UI behavior to show whether we're paused, or if we are waiting on the remote to unpause. --- CMakeLists.txt | 1 + src/chatlog/content/filetransferwidget.cpp | 22 +++++-- src/core/corefile.cpp | 45 ++++++------- src/core/toxfile.h | 3 + src/core/toxfilepause.h | 76 ++++++++++++++++++++++ 5 files changed, 116 insertions(+), 31 deletions(-) create mode 100644 src/core/toxfilepause.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 22f401865..12590e3e5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -268,6 +268,7 @@ set(${PROJECT_NAME}_SOURCES src/core/toxencrypt.h src/core/toxfile.cpp src/core/toxfile.h + src/core/toxfilepause.h src/core/toxid.cpp src/core/toxid.h src/core/toxlogger.cpp diff --git a/src/chatlog/content/filetransferwidget.cpp b/src/chatlog/content/filetransferwidget.cpp index c846a62f8..f38d71361 100644 --- a/src/chatlog/content/filetransferwidget.cpp +++ b/src/chatlog/content/filetransferwidget.cpp @@ -346,7 +346,7 @@ void FileTransferWidget::updateWidgetColor(ToxFile const& file) void FileTransferWidget::updateWidgetText(ToxFile const& file) { - if (lastStatus == file.status) { + if (lastStatus == file.status && file.status != ToxFile::PAUSED) { return; } @@ -360,7 +360,11 @@ void FileTransferWidget::updateWidgetText(ToxFile const& file) break; case ToxFile::PAUSED: ui->etaLabel->setText(""); - ui->progressLabel->setText(tr("Paused", "file transfer widget")); + if (file.pauseStatus.localPaused()) { + ui->progressLabel->setText(tr("Paused", "file transfer widget")); + } else { + ui->progressLabel->setText(tr("Remote Paused", "file transfer widget")); + } break; case ToxFile::TRANSMITTING: ui->etaLabel->setText(""); @@ -474,7 +478,7 @@ void FileTransferWidget::updateSignals(ToxFile const& file) void FileTransferWidget::setupButtons(ToxFile const& file) { - if (lastStatus == file.status) { + if (lastStatus == file.status && file.status != ToxFile::PAUSED) { return; } @@ -492,9 +496,15 @@ void FileTransferWidget::setupButtons(ToxFile const& file) break; case ToxFile::PAUSED: - ui->leftButton->setIcon(QIcon(Style::getImagePath("fileTransferInstance/arrow_white.svg"))); - ui->leftButton->setObjectName("resume"); - ui->leftButton->setToolTip(tr("Resume transfer")); + if (file.pauseStatus.localPaused()) { + ui->leftButton->setIcon(QIcon(Style::getImagePath("fileTransferInstance/arrow_white.svg"))); + ui->leftButton->setObjectName("resume"); + ui->leftButton->setToolTip(tr("Resume transfer")); + } else { + ui->leftButton->setIcon(QIcon(Style::getImagePath("fileTransferInstance/pause.svg"))); + ui->leftButton->setObjectName("pause"); + ui->leftButton->setToolTip(tr("Pause transfer")); + } ui->rightButton->setIcon(QIcon(Style::getImagePath("fileTransferInstance/no.svg"))); ui->rightButton->setObjectName("cancel"); diff --git a/src/core/corefile.cpp b/src/core/corefile.cpp index 8b38e1546..9365facd5 100644 --- a/src/core/corefile.cpp +++ b/src/core/corefile.cpp @@ -152,41 +152,34 @@ void CoreFile::pauseResumeFileSend(Core* core, uint32_t friendId, uint32_t fileI qWarning("pauseResumeFileSend: No such file in queue"); return; } - if (file->status == ToxFile::TRANSMITTING) { + + if (file->status != ToxFile::TRANSMITTING && file->status != ToxFile::PAUSED) { + qWarning() << "pauseResumeFileSend: File is stopped"; + return; + } + + file->pauseStatus.localPauseToggle(); + + if (file->pauseStatus.paused()) { file->status = ToxFile::PAUSED; emit core->fileTransferPaused(*file); - tox_file_control(core->tox.get(), file->friendId, file->fileNum, TOX_FILE_CONTROL_PAUSE, - nullptr); - } else if (file->status == ToxFile::PAUSED) { + } else { file->status = ToxFile::TRANSMITTING; emit core->fileTransferAccepted(*file); - tox_file_control(core->tox.get(), file->friendId, file->fileNum, TOX_FILE_CONTROL_RESUME, + } + + if (file->pauseStatus.localPaused()) { + tox_file_control(core->tox.get(), file->friendId, file->fileNum, TOX_FILE_CONTROL_PAUSE, nullptr); } else { - qWarning() << "pauseResumeFileSend: File is stopped"; + tox_file_control(core->tox.get(), file->friendId, file->fileNum, TOX_FILE_CONTROL_RESUME, + nullptr); } } void CoreFile::pauseResumeFileRecv(Core* core, uint32_t friendId, uint32_t fileId) { - ToxFile* file = findFile(friendId, fileId); - if (!file) { - qWarning("cancelFileRecv: No such file in queue"); - return; - } - if (file->status == ToxFile::TRANSMITTING) { - file->status = ToxFile::PAUSED; - emit core->fileTransferPaused(*file); - tox_file_control(core->tox.get(), file->friendId, file->fileNum, TOX_FILE_CONTROL_PAUSE, - nullptr); - } else if (file->status == ToxFile::PAUSED) { - file->status = ToxFile::TRANSMITTING; - emit core->fileTransferAccepted(*file); - tox_file_control(core->tox.get(), file->friendId, file->fileNum, TOX_FILE_CONTROL_RESUME, - nullptr); - } else { - qWarning() << "pauseResumeFileRecv: File is stopped or broken"; - } + pauseResumeFileSend(core, friendId, fileId); } void CoreFile::cancelFileSend(Core* core, uint32_t friendId, uint32_t fileId) @@ -382,6 +375,7 @@ void CoreFile::onFileControlCallback(Tox*, uint32_t friendId, uint32_t fileId, removeFile(friendId, fileId); } else if (control == TOX_FILE_CONTROL_PAUSE) { qDebug() << "onFileControlCallback: Received pause for file " << friendId << ":" << fileId; + file->pauseStatus.remotePause(); file->status = ToxFile::PAUSED; emit static_cast(core)->fileTransferRemotePausedUnpaused(*file, true); } else if (control == TOX_FILE_CONTROL_RESUME) { @@ -389,7 +383,8 @@ void CoreFile::onFileControlCallback(Tox*, uint32_t friendId, uint32_t fileId, qDebug() << "Avatar transfer" << fileId << "to friend" << friendId << "accepted"; else qDebug() << "onFileControlCallback: Received resume for file " << friendId << ":" << fileId; - file->status = ToxFile::TRANSMITTING; + file->pauseStatus.remoteResume(); + file->status = file->pauseStatus.paused() ? ToxFile::PAUSED : ToxFile::TRANSMITTING; emit static_cast(core)->fileTransferRemotePausedUnpaused(*file, false); } else { qWarning() << "Unhandled file control " << control << " for file " << friendId << ':' << fileId; diff --git a/src/core/toxfile.h b/src/core/toxfile.h index fa47e0f9c..72ab20ba8 100644 --- a/src/core/toxfile.h +++ b/src/core/toxfile.h @@ -1,6 +1,8 @@ #ifndef CORESTRUCTS_H #define CORESTRUCTS_H +#include "src/core/toxfilepause.h" + #include #include #include @@ -53,6 +55,7 @@ struct ToxFile QByteArray avatarData; QByteArray resumeFileId; std::shared_ptr hashGenerator = std::make_shared(QCryptographicHash::Sha256); + ToxFilePause pauseStatus; }; #endif // CORESTRUCTS_H diff --git a/src/core/toxfilepause.h b/src/core/toxfilepause.h new file mode 100644 index 000000000..d63951279 --- /dev/null +++ b/src/core/toxfilepause.h @@ -0,0 +1,76 @@ +/* + Copyright © 2018 by The qTox Project Contributors + + This file is part of qTox, a Qt-based graphical interface for Tox. + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + qTox is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with qTox. If not, see . +*/ + +#ifndef TOX_FILE_PAUSE_H +#define TOX_FILE_PAUSE_H + +class ToxFilePause +{ +public: + void localPause() + { + localPauseState = true; + } + + void localResume() + { + localPauseState = false; + } + + void localPauseToggle() + { + localPauseState = !localPauseState; + } + + void remotePause() + { + remotePauseState = true; + } + + void remoteResume() + { + remotePauseState = false; + } + + void remotePauseToggle() + { + remotePauseState = !remotePauseState; + } + + bool localPaused() const + { + return localPauseState; + } + + bool remotePaused() const + { + return remotePauseState; + } + + bool paused() const + { + return localPauseState || remotePauseState; + } + +private: + bool localPauseState = false; + bool remotePauseState = false; +}; + +#endif // TOX_FILE_PAUSE_H From 36154252343022366bd6be5bbf37af60d86c74dc Mon Sep 17 00:00:00 2001 From: Mick Sayson Date: Wed, 3 Oct 2018 23:50:12 -0700 Subject: [PATCH 2/2] refactor(transfer): Remove unnecessary split for pause send/recv --- src/chatlog/content/filetransferwidget.cpp | 8 ++++---- src/core/core.cpp | 11 ++--------- src/core/core.h | 3 +-- src/core/corefile.cpp | 7 +------ src/core/corefile.h | 3 +-- 5 files changed, 9 insertions(+), 23 deletions(-) diff --git a/src/chatlog/content/filetransferwidget.cpp b/src/chatlog/content/filetransferwidget.cpp index f38d71361..a236e4ff7 100644 --- a/src/chatlog/content/filetransferwidget.cpp +++ b/src/chatlog/content/filetransferwidget.cpp @@ -557,18 +557,18 @@ void FileTransferWidget::handleButton(QPushButton* btn) if (btn->objectName() == "cancel") { Core::getInstance()->cancelFileSend(fileInfo.friendId, fileInfo.fileNum); } else if (btn->objectName() == "pause") { - Core::getInstance()->pauseResumeFileSend(fileInfo.friendId, fileInfo.fileNum); + Core::getInstance()->pauseResumeFile(fileInfo.friendId, fileInfo.fileNum); } else if (btn->objectName() == "resume") { - Core::getInstance()->pauseResumeFileSend(fileInfo.friendId, fileInfo.fileNum); + Core::getInstance()->pauseResumeFile(fileInfo.friendId, fileInfo.fileNum); } } else // receiving or paused { if (btn->objectName() == "cancel") { Core::getInstance()->cancelFileRecv(fileInfo.friendId, fileInfo.fileNum); } else if (btn->objectName() == "pause") { - Core::getInstance()->pauseResumeFileRecv(fileInfo.friendId, fileInfo.fileNum); + Core::getInstance()->pauseResumeFile(fileInfo.friendId, fileInfo.fileNum); } else if (btn->objectName() == "resume") { - Core::getInstance()->pauseResumeFileRecv(fileInfo.friendId, fileInfo.fileNum); + Core::getInstance()->pauseResumeFile(fileInfo.friendId, fileInfo.fileNum); } else if (btn->objectName() == "accept") { QString path = QFileDialog::getSaveFileName(Q_NULLPTR, diff --git a/src/core/core.cpp b/src/core/core.cpp index 2d70c9e70..fb18b0454 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -715,18 +715,11 @@ void Core::sendAvatarFile(uint32_t friendId, const QByteArray& data) CoreFile::sendAvatarFile(this, friendId, data); } -void Core::pauseResumeFileSend(uint32_t friendId, uint32_t fileNum) +void Core::pauseResumeFile(uint32_t friendId, uint32_t fileNum) { QMutexLocker ml{coreLoopLock.get()}; - CoreFile::pauseResumeFileSend(this, friendId, fileNum); -} - -void Core::pauseResumeFileRecv(uint32_t friendId, uint32_t fileNum) -{ - QMutexLocker ml{coreLoopLock.get()}; - - CoreFile::pauseResumeFileRecv(this, friendId, fileNum); + CoreFile::pauseResumeFile(this, friendId, fileNum); } void Core::cancelFileSend(uint32_t friendId, uint32_t fileNum) diff --git a/src/core/core.h b/src/core/core.h index 1799c288a..f4ae6293e 100644 --- a/src/core/core.h +++ b/src/core/core.h @@ -130,8 +130,7 @@ public slots: void cancelFileRecv(uint32_t friendId, uint32_t fileNum); void rejectFileRecvRequest(uint32_t friendId, uint32_t fileNum); void acceptFileRecvRequest(uint32_t friendId, uint32_t fileNum, QString path); - void pauseResumeFileSend(uint32_t friendId, uint32_t fileNum); - void pauseResumeFileRecv(uint32_t friendId, uint32_t fileNum); + void pauseResumeFile(uint32_t friendId, uint32_t fileNum); void setNospam(uint32_t nospam); diff --git a/src/core/corefile.cpp b/src/core/corefile.cpp index 9365facd5..64949c7d0 100644 --- a/src/core/corefile.cpp +++ b/src/core/corefile.cpp @@ -145,7 +145,7 @@ void CoreFile::sendFile(Core* core, uint32_t friendId, QString filename, QString emit core->fileSendStarted(file); } -void CoreFile::pauseResumeFileSend(Core* core, uint32_t friendId, uint32_t fileId) +void CoreFile::pauseResumeFile(Core* core, uint32_t friendId, uint32_t fileId) { ToxFile* file = findFile(friendId, fileId); if (!file) { @@ -177,11 +177,6 @@ void CoreFile::pauseResumeFileSend(Core* core, uint32_t friendId, uint32_t fileI } } -void CoreFile::pauseResumeFileRecv(Core* core, uint32_t friendId, uint32_t fileId) -{ - pauseResumeFileSend(core, friendId, fileId); -} - void CoreFile::cancelFileSend(Core* core, uint32_t friendId, uint32_t fileId) { ToxFile* file = findFile(friendId, fileId); diff --git a/src/core/corefile.h b/src/core/corefile.h index c75603fd4..f45b29de3 100644 --- a/src/core/corefile.h +++ b/src/core/corefile.h @@ -51,8 +51,7 @@ private: static void sendFile(Core* core, uint32_t friendId, QString filename, QString filePath, long long filesize); static void sendAvatarFile(Core* core, uint32_t friendId, const QByteArray& data); - static void pauseResumeFileSend(Core* core, uint32_t friendId, uint32_t fileId); - static void pauseResumeFileRecv(Core* core, uint32_t friendId, uint32_t fileId); + static void pauseResumeFile(Core* core, uint32_t friendId, uint32_t fileId); static void cancelFileSend(Core* core, uint32_t friendId, uint32_t fileId); static void cancelFileRecv(Core* core, uint32_t friendId, uint32_t fileId); static void rejectFileRecvRequest(Core* core, uint32_t friendId, uint32_t fileId);