Skip to content

Commit

Permalink
processEvents after calling signals
Browse files Browse the repository at this point in the history
use better thread saftey by calling processEvents with QueuedConnection back to the main thread
  • Loading branch information
noisecode3 committed Aug 24, 2024
1 parent 95502ab commit e5c5914
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 107 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ compile_commands.json
TombRaiderLinuxLauncher
TombRaiderLinuxLauncher_autogen
cmake_install.cmake
install_manifest.txt
Makefile
build-Debug
build
Expand Down
144 changes: 67 additions & 77 deletions src/Controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,96 +17,106 @@
#include "Controller.h"
#include <QDebug>

Controller::Controller(QObject *parent) : QObject(parent), ControllerThread(nullptr)
Controller::Controller(QObject *parent)
: QObject(parent), controllerThread(new QThread(this))
{
ControllerThread = new QThread();
this->moveToThread(ControllerThread);
connect(ControllerThread, &QThread::finished, ControllerThread, &QThread::deleteLater);
ControllerThread->start();
/* this should make emits to signals from the controller and to the controller happen
* on another thread event loop so TombRaiderLinuxLauncher (view object) should have a slot for
* getting the results, update the GUI state, if the controller returns something it's just the
* state of some logic right before it calls the thread signal.
* Every method here should emit a signal to this object here again and use only quick logic
* in order to get back out again and never hold up the GUI thread, never. Even if someone have like
* the slowest hard drives in the world, so the game couldn't even run probably it will not wait for
* it and keep updating the GUI. This is the most safe and correct way to do this I think.
*/

// Those start threaded work a signal hub
connect(this, SIGNAL(checkCommonFilesThreadSignal()), this, SLOT(checkCommonFilesThread()));
connect(this, SIGNAL(setupThreadSignal(QString,QString)), this, SLOT(setupThread(QString,QString)));
connect(this, SIGNAL(setupLevelThreadSignal(int)), this, SLOT(setupLevelThread(int)));
connect(this, SIGNAL(setupGameThreadSignal(int)), this, SLOT(setupGameThread(int)));

// Those work like a signal hub for threaded work, like in between callback
connect(&Model::getInstance(), SIGNAL(modelTickSignal()), this, SLOT(passTickSignal()));
connect(&FileManager::getInstance(), SIGNAL(fileWorkTickSignal()), this, SLOT(passTickSignal()));
connect(&Downloader::getInstance(), SIGNAL(networkWorkTickSignal()), this, SLOT(passTickSignal()));

connect(&Downloader::getInstance(), SIGNAL(networkWorkErrorSignal(int)), this, SLOT(passDownloadError(int)));

connect(&Model::getInstance(), SIGNAL(askGameSignal(int)), this, SLOT(passAskGame(int)));
connect(&Model::getInstance(), SIGNAL(generateListSignal()), this, SLOT(passGenerateList()));

// The TombRaiderLinuxLauncher object have the connect and slot for last signal emit that displays
// the result, we pass back what was return before like this
// connect(&Controller::getInstance(), SIGNAL(setupCampDone(bool)), this, SLOT(checkCommonFiles(bool)));
initializeThread();
}

Controller::~Controller()
{
ControllerThread->quit();
ControllerThread->wait();
delete ControllerThread;
ControllerThread = nullptr;
controllerThread->quit();
controllerThread->wait();
}

int Controller::checkGameDirectory(int id)
void Controller::initializeThread()
{
return model.checkGameDirectory(id);
this->moveToThread(controllerThread.data());
connect(controllerThread.data(), &QThread::finished,
controllerThread.data(), &QThread::deleteLater);
controllerThread->start();

// Using the controller thread to start model work
connect(this, &Controller::checkCommonFilesThreadSignal,
this, [this]()
{
model.checkCommonFiles();
});

connect(this, &Controller::setupThreadSignal,
this, [this](const QString& level, const QString& game)
{
model.setup(level, game);
});

connect(this, &Controller::setupLevelThreadSignal,
this, [this](int id) {
model.getGame(id);
});

connect(this, &Controller::setupGameThreadSignal,
this, [this](int id)
{
model.setupGame(id);
});

// Comming back from model or other model objects
auto tickSignal = [this](){ emit controllerTickSignal(); };
connect(&model, &Model::modelTickSignal,
this, tickSignal, Qt::QueuedConnection);

connect(&fileManager, &FileManager::fileWorkTickSignal,
this, tickSignal, Qt::QueuedConnection);

connect(&downloader, &Downloader::networkWorkTickSignal,
this, tickSignal, Qt::QueuedConnection);

connect(&downloader, &Downloader::networkWorkErrorSignal,
this, [this](int status)
{
emit controllerDownloadError(status);
}, Qt::QueuedConnection);

connect(&model, &Model::askGameSignal,
this, [this](int id)
{
emit controllerAskGame(id);
}, Qt::QueuedConnection);

connect(&model, &Model::generateListSignal,
this, [this]()
{
emit controllerGenerateList();
}, Qt::QueuedConnection);
}

void Controller::checkCommonFiles()
{
emit checkCommonFilesThreadSignal();
}
void Controller::checkCommonFilesThread()
{
model.checkCommonFiles();
}

void Controller::setup(const QString& level, const QString& game)
{
emit setupThreadSignal(level, game);
}

void Controller::setupThread(const QString& level, const QString& game)
{
model.setup(level, game);
}

void Controller::setupGame(int id)
{
emit setupGameThreadSignal(id);
}

void Controller::setupGameThread(int id)
{
model.setupGame(id);
}

void Controller::setupLevel(int id)
{
emit setupLevelThreadSignal(id);
}

void Controller::setupLevelThread(int id)
int Controller::checkGameDirectory(int id)
{
model.getGame(id);
return model.checkGameDirectory(id);
}

void Controller::getList(QVector<ListItemData>& list)
// GUI Thread
void Controller::getList(QVector<ListItemData>* list)
{
model.getList(list);
}
Expand All @@ -130,23 +140,3 @@ int Controller::getItemState(int id)
{
return model.getItemState(id);
}

void Controller::passTickSignal()
{
emit controllerTickSignal();
}

void Controller::passDownloadError(int status)
{
emit controllerDownloadError(status);
}

void Controller::passAskGame(int id)
{
emit controllerAskGame(id);
}

void Controller::passGenerateList()
{
emit controllerGenerateList();
}
39 changes: 17 additions & 22 deletions src/Controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,53 +27,48 @@
class Controller : public QObject
{
Q_OBJECT

public:
static Controller& getInstance()
{
static Controller instance;
return instance;
}

int checkGameDirectory(int id);
void checkCommonFiles();
void setup(const QString& level, const QString& game);
void setupGame(int id);
void setupLevel(int id);
void getList(QVector<ListItemData>& list);
bool link(int id);

void getList(QVector<ListItemData>* list);
const InfoData getInfo(int id);
const QString getWalkthrough(int id);
bool link(int id);
int getItemState(int id);
int checkGameDirectory(int id);
void checkCommonFiles();

public slots:
// Passed from Model to Controller between threaded work
void passAskGame(int id);
void passDownloadError(int status);
void passGenerateList();
void passTickSignal();
// Start thread work from GUI and pass it to Model
void checkCommonFilesThread();
void setupThread(const QString& level, const QString& game);
void setupGameThread(int id);
void setupLevelThread(int id);

signals:
// Signals the GUI from in between thread workd
void controllerAskGame(int id);
void controllerGenerateList(); // setup done
void controllerGenerateList();
void controllerTickSignal();
void controllerDownloadError(int status);
// Singals Thread start

void checkCommonFilesThreadSignal();
void setupThreadSignal(const QString& level, const QString& game);
void setupGameThreadSignal(int id);
void setupLevelThreadSignal(int id);

private:
Model& model = Model::getInstance();

explicit Controller(QObject *parent = nullptr);
void initializeThread();
~Controller();
QThread* ControllerThread;

Data& data = Data::getInstance();
FileManager& fileManager = FileManager::getInstance();
Model& model = Model::getInstance();
Downloader& downloader = Downloader::getInstance();
QScopedPointer<QThread> controllerThread;

Q_DISABLE_COPY(Controller)
};

Expand Down
2 changes: 2 additions & 0 deletions src/FileManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ bool FileManager::extractZip(
j <= currentPercent; j++)
{
emit this->fileWorkTickSignal();
QCoreApplication::processEvents();
}
lastPrintedPercent = currentPercent;
}
Expand All @@ -159,6 +160,7 @@ bool FileManager::extractZip(
for (unsigned int j = lastPrintedPercent + 1; j <= gotoPercent; j++)
{
emit this->fileWorkTickSignal();
QCoreApplication::processEvents();
}

// Clean up
Expand Down
9 changes: 5 additions & 4 deletions src/Model.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#include <QDebug>
#include <assert.h>
#include "Model.h"

// Those lambda should be in another header file
Expand Down Expand Up @@ -75,10 +73,12 @@ void Model::checkCommonFiles()
{
checkCommonFilesIndex_m = i+1;
emit askGameSignal(i);
QCoreApplication::processEvents();
return;
}
}
emit generateListSignal();
QCoreApplication::processEvents();
}

QString Model::getGameDirectory(int id)
Expand Down Expand Up @@ -110,9 +110,9 @@ int Model::checkGameDirectory(int id)
return -1;
}

void Model::getList(QVector<ListItemData>& list)
void Model::getList(QVector<ListItemData>* list)
{
list = data.getListItems();
*list = data.getListItems();
}

int Model::getItemState(int id)
Expand Down Expand Up @@ -220,6 +220,7 @@ bool Model::getGame(int id)
for (int i=0; i < 50; i++)
{
emit this->modelTickSignal();
QCoreApplication::processEvents();
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/Model.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
#include <QObject>
#include <QMap>
#include <QDebug>
#include <QtCore>
#include <assert.h>
#include "Data.h"
#include "FileManager.h"
#include "Network.h"
Expand Down Expand Up @@ -62,11 +64,10 @@ class Model : public QObject
static Model instance;
return instance;
};
//int checkAllGames(int id);
void checkCommonFiles();
int checkGameDirectory(int id);
int checkLevelDirectory(int id);
void getList(QVector<ListItemData>& list);
void getList(QVector<ListItemData>* list);
int getItemState(int id);
bool setLink(int id);
QString getGameDirectory(int id);
Expand Down
4 changes: 4 additions & 0 deletions src/Network.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ int Downloader::progress_callback(
{
static Downloader& instance = Downloader::getInstance();
emit instance.networkWorkTickSignal();
QCoreApplication::processEvents();
lastEmittedProgress = static_cast<int>(progress);
}
}
Expand Down Expand Up @@ -145,14 +146,17 @@ void Downloader::run()
if(res == 6 || res == 7 || res == 28 || res == 35)
{
emit this->networkWorkErrorSignal(1);
QCoreApplication::processEvents();
}
else if(res == CURLE_PEER_FAILED_VERIFICATION)
{
emit this->networkWorkErrorSignal(2);
QCoreApplication::processEvents();
}
else
{
emit this->networkWorkErrorSignal(3);
QCoreApplication::processEvents();
}
}
else
Expand Down
1 change: 1 addition & 0 deletions src/Network.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <QUrl>
#include <QFile>
#include <QDir>
#include <QtCore>
#include <QDebug>
#include <curl/curl.h>

Expand Down
3 changes: 1 addition & 2 deletions src/TombRaiderLinuxLauncher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@

#include "ui_TombRaiderLinuxLauncher.h"


TombRaiderLinuxLauncher::TombRaiderLinuxLauncher(QWidget *parent)
:QMainWindow(parent),
ui(new Ui::TombRaiderLinuxLauncher)
Expand Down Expand Up @@ -144,7 +143,7 @@ void TombRaiderLinuxLauncher::generateList()
}

QVector<ListItemData> list;
controller.getList(list);
controller.getList(&list);
const size_t s = list.size();
for (int i = 0; i < s; i++)
{
Expand Down

0 comments on commit e5c5914

Please sign in to comment.