From f8c20128ef4dee6b7594a4c303eeef2257375b74 Mon Sep 17 00:00:00 2001 From: morg Date: Sat, 12 Aug 2023 08:06:09 +0200 Subject: [PATCH] Plugin manager: simplifies code, fixes a crash on exit --- PluginManager.cpp | 48 ++++++-- PluginManager.h | 2 - qt/OpenRGBDialog2/OpenRGBDialog2.cpp | 119 ++++++++----------- qt/OpenRGBPluginsPage/OpenRGBPluginsPage.cpp | 33 ++--- 4 files changed, 100 insertions(+), 102 deletions(-) diff --git a/PluginManager.cpp b/PluginManager.cpp index fe1d88cd..ed2ea5e4 100644 --- a/PluginManager.cpp +++ b/PluginManager.cpp @@ -200,7 +200,6 @@ void PluginManager::AddPlugin(const filesystem::path& path) entry.info = info; entry.plugin = plugin; entry.loader = loader; - entry.loaded = false; entry.path = path_string; entry.enabled = enabled; entry.widget = nullptr; @@ -209,7 +208,7 @@ void PluginManager::AddPlugin(const filesystem::path& path) loader->unload(); - PluginManager::ActivePlugins.push_back(entry); + ActivePlugins.push_back(entry); if(entry.enabled) { @@ -236,7 +235,6 @@ void PluginManager::AddPlugin(const filesystem::path& path) entry.info = info; entry.plugin = plugin; entry.loader = loader; - entry.loaded = false; entry.path = path_string; entry.enabled = false; entry.widget = nullptr; @@ -247,10 +245,24 @@ void PluginManager::AddPlugin(const filesystem::path& path) PluginManager::ActivePlugins.push_back(entry); - loader->unload(); + bool unloaded = loader->unload(); + LOG_WARNING("[PluginManager] Plugin %s has an incompatible API version", path.c_str()); + + if(!unloaded) + { + LOG_WARNING("[PluginManager] Plugin %s cannot be unloaded", path.c_str()); + } } } + else + { + LOG_WARNING("[PluginManager] Plugin %s cannot be casted to OpenRGBPluginInterface", path.c_str()); + } + } + else + { + LOG_WARNING("[PluginManager] Plugin %s cannot be instantiated.", path.c_str()); } } } @@ -284,7 +296,7 @@ void PluginManager::RemovePlugin(const filesystem::path& path) /*---------------------------------------------------------------------*\ | If the selected plugin is in the list and loaded, unload it | \*---------------------------------------------------------------------*/ - if(ActivePlugins[plugin_idx].loaded) + if(ActivePlugins[plugin_idx].loader->isLoaded()) { LOG_TRACE("[PluginManager] Plugin %s is active, unloading", path.c_str()); UnloadPlugin(path); @@ -330,10 +342,9 @@ void PluginManager::LoadPlugin(const filesystem::path& path) /*---------------------------------------------------------------------*\ | If the selected plugin is in the list but not loaded, load it | \*---------------------------------------------------------------------*/ - if(!ActivePlugins[plugin_idx].loaded) + if(!ActivePlugins[plugin_idx].loader->isLoaded()) { ActivePlugins[plugin_idx].loader->load(); - ActivePlugins[plugin_idx].loaded = true; QObject* instance = ActivePlugins[plugin_idx].loader->instance(); @@ -390,7 +401,7 @@ void PluginManager::UnloadPlugin(const filesystem::path& path) /*---------------------------------------------------------------------*\ | If the selected plugin is in the list and loaded, unload it | \*---------------------------------------------------------------------*/ - if(ActivePlugins[plugin_idx].loaded) + if(ActivePlugins[plugin_idx].loader->isLoaded()) { /*-------------------------------------------------*\ | Call plugin's Unload function before GUI removal | @@ -405,8 +416,20 @@ void PluginManager::UnloadPlugin(const filesystem::path& path) RemovePluginCallbackVal(RemovePluginCallbackArg, &ActivePlugins[plugin_idx]); } - ActivePlugins[plugin_idx].loader->unload(); - ActivePlugins[plugin_idx].loaded = false; + bool unloaded = ActivePlugins[plugin_idx].loader->unload(); + + if(!unloaded) + { + LOG_WARNING("[PluginManager] Plugin %s cannot be unloaded", path.c_str()); + } + else + { + LOG_TRACE("[PluginManager] Plugin %s successfully unloaded", path.c_str()); + } + } + else + { + LOG_TRACE("[PluginManager] Plugin %s was already unloaded", path.c_str()); } } @@ -414,6 +437,9 @@ void PluginManager::UnloadPlugins() { for(const OpenRGBPluginEntry& plugin_entry: ActivePlugins) { - plugin_entry.plugin->Unload(); + if(plugin_entry.loader->isLoaded()) + { + plugin_entry.plugin->Unload(); + } } } diff --git a/PluginManager.h b/PluginManager.h index 2ee2790a..5f637b61 100644 --- a/PluginManager.h +++ b/PluginManager.h @@ -1,7 +1,6 @@ #pragma once #include "OpenRGBPluginInterface.h" -#include "ResourceManager.h" #include #include @@ -16,7 +15,6 @@ typedef struct OpenRGBPluginInfo info; OpenRGBPluginInterface* plugin; QPluginLoader* loader; - bool loaded; QWidget* widget; QMenu* traymenu; std::string path; diff --git a/qt/OpenRGBDialog2/OpenRGBDialog2.cpp b/qt/OpenRGBDialog2/OpenRGBDialog2.cpp index e43982d7..df3a4117 100644 --- a/qt/OpenRGBDialog2/OpenRGBDialog2.cpp +++ b/qt/OpenRGBDialog2/OpenRGBDialog2.cpp @@ -1001,77 +1001,60 @@ void OpenRGBDialog2::AddPlugin(OpenRGBPluginEntry* plugin) void OpenRGBDialog2::RemovePlugin(OpenRGBPluginEntry* plugin) { /*-----------------------------------------------------*\ - | Place plugin as its own top level tab | + | Remove plugin's tray menu | \*-----------------------------------------------------*/ - if(plugin->info.Location == OPENRGB_PLUGIN_LOCATION_TOP) - { - for(int tab_idx = 0; tab_idx < ui->MainTabBar->count(); tab_idx++) - { - if(dynamic_cast(ui->MainTabBar->widget(tab_idx)) != nullptr) - { - if(dynamic_cast(ui->MainTabBar->widget(tab_idx))->plugin_widget == plugin->widget) - { - ui->MainTabBar->removeTab(tab_idx); - delete plugin->widget; - } - } - } - } - /*-----------------------------------------------------*\ - | Place plugin in the Devices tab | - \*-----------------------------------------------------*/ - else if(plugin->info.Location == OPENRGB_PLUGIN_LOCATION_DEVICES) - { - for(int tab_idx = 0; tab_idx < ui->DevicesTabBar->count(); tab_idx++) - { - if(dynamic_cast(ui->DevicesTabBar->widget(tab_idx)) != nullptr) - { - if(dynamic_cast(ui->DevicesTabBar->widget(tab_idx))->plugin_widget == plugin->widget) - { - ui->DevicesTabBar->removeTab(tab_idx); - delete plugin->widget; - } - } - } - } - /*-----------------------------------------------------*\ - | Place plugin in the Information tab | - \*-----------------------------------------------------*/ - else if(plugin->info.Location == OPENRGB_PLUGIN_LOCATION_INFORMATION) - { - for(int tab_idx = 0; tab_idx < ui->InformationTabBar->count(); tab_idx++) - { - if(dynamic_cast(ui->InformationTabBar->widget(tab_idx)) != nullptr) - { - if(dynamic_cast(ui->InformationTabBar->widget(tab_idx))->plugin_widget == plugin->widget) - { - ui->InformationTabBar->removeTab(tab_idx); - delete plugin->widget; - } - } - } - } - /*-----------------------------------------------------*\ - | Place plugin in the Settings tab | - \*-----------------------------------------------------*/ - else if(plugin->info.Location == OPENRGB_PLUGIN_LOCATION_SETTINGS) - { - for(int tab_idx = 0; tab_idx < ui->SettingsTabBar->count(); tab_idx++) - { - if(dynamic_cast(ui->SettingsTabBar->widget(tab_idx)) != nullptr) - { - if(dynamic_cast(ui->SettingsTabBar->widget(tab_idx))->plugin_widget == plugin->widget) - { - ui->SettingsTabBar->removeTab(tab_idx); - delete plugin->widget; - } - } - } - } - if(plugin->traymenu) { - trayIconMenu->removeAction(plugin->traymenu->menuAction()); + QWidget* plugin_tray_entry = trayIconMenu->find(plugin->traymenu->winId()); + + if(plugin_tray_entry) + { + trayIconMenu->removeAction(plugin->traymenu->menuAction()); + } + + //delete plugin->traymenu; + } + + /*-----------------------------------------------------*\ + | Find plugin's container | + \*-----------------------------------------------------*/ + QTabWidget *plugin_parent_widget = nullptr; + + switch(plugin->info.Location) + { + case OPENRGB_PLUGIN_LOCATION_TOP: + plugin_parent_widget = ui->MainTabBar; + break; + case OPENRGB_PLUGIN_LOCATION_DEVICES: + plugin_parent_widget = ui->DevicesTabBar; + break; + case OPENRGB_PLUGIN_LOCATION_INFORMATION: + plugin_parent_widget = ui->InformationTabBar; + break; + case OPENRGB_PLUGIN_LOCATION_SETTINGS: + plugin_parent_widget = ui->InformationTabBar; + break; + default: + break; + } + + /*-----------------------------------------------------*\ + | Remove plugin from its container | + \*-----------------------------------------------------*/ + if(plugin_parent_widget != nullptr) + { + for(int tab_idx = 0; tab_idx < plugin_parent_widget->count(); tab_idx++) + { + if(dynamic_cast(plugin_parent_widget->widget(tab_idx)) != nullptr) + { + if(dynamic_cast(plugin_parent_widget->widget(tab_idx))->plugin_widget == plugin->widget) + { + plugin_parent_widget->removeTab(tab_idx); + //delete plugin->widget; + break; + } + } + } } } diff --git a/qt/OpenRGBPluginsPage/OpenRGBPluginsPage.cpp b/qt/OpenRGBPluginsPage/OpenRGBPluginsPage.cpp index 8297ec68..51b7f7a5 100644 --- a/qt/OpenRGBPluginsPage/OpenRGBPluginsPage.cpp +++ b/qt/OpenRGBPluginsPage/OpenRGBPluginsPage.cpp @@ -44,25 +44,25 @@ void Ui::OpenRGBPluginsPage::RefreshList() ui->PluginsList->clear(); entries.clear(); - for(unsigned int plugin_idx = 0; plugin_idx < plugin_manager->ActivePlugins.size(); plugin_idx++) + for(const OpenRGBPluginEntry& plugin: plugin_manager->ActivePlugins) { OpenRGBPluginsEntry* entry = new OpenRGBPluginsEntry(); /*---------------------------------------------------------*\ | Fill in plugin information fields | \*---------------------------------------------------------*/ - entry->ui->NameValue->setText(QString::fromStdString(plugin_manager->ActivePlugins[plugin_idx].info.Name)); - entry->ui->DescriptionValue->setText(QString::fromStdString(plugin_manager->ActivePlugins[plugin_idx].info.Description)); - entry->ui->VersionValue->setText(QString::fromStdString(plugin_manager->ActivePlugins[plugin_idx].info.Version)); - entry->ui->CommitValue->setText(QString::fromStdString(plugin_manager->ActivePlugins[plugin_idx].info.Commit)); - entry->ui->URLValue->setText(QString::fromStdString(plugin_manager->ActivePlugins[plugin_idx].info.URL)); - entry->ui->APIVersionValue->setText(QString::number(plugin_manager->ActivePlugins[plugin_idx].api_version)); + entry->ui->NameValue->setText(QString::fromStdString(plugin.info.Name)); + entry->ui->DescriptionValue->setText(QString::fromStdString(plugin.info.Description)); + entry->ui->VersionValue->setText(QString::fromStdString(plugin.info.Version)); + entry->ui->CommitValue->setText(QString::fromStdString(plugin.info.Commit)); + entry->ui->URLValue->setText(QString::fromStdString(plugin.info.URL)); + entry->ui->APIVersionValue->setText(QString::number(plugin.api_version)); /*---------------------------------------------------------*\ | If the plugin is incompatible, highlight the API version | | in red and disable the enable checkbox | \*---------------------------------------------------------*/ - if(plugin_manager->ActivePlugins[plugin_idx].incompatible) + if(plugin.incompatible) { entry->ui->APIVersionValue->setStyleSheet("QLabel { color : red; }"); entry->ui->EnabledCheckBox->setEnabled(false); @@ -71,7 +71,7 @@ void Ui::OpenRGBPluginsPage::RefreshList() /*---------------------------------------------------------*\ | Fill in plugin icon | \*---------------------------------------------------------*/ - QPixmap pixmap(QPixmap::fromImage(plugin_manager->ActivePlugins[plugin_idx].info.Icon)); + QPixmap pixmap(QPixmap::fromImage(plugin.info.Icon)); entry->ui->IconView->setPixmap(pixmap); entry->ui->IconView->setScaledContents(true); @@ -79,12 +79,12 @@ void Ui::OpenRGBPluginsPage::RefreshList() /*---------------------------------------------------------*\ | Fill in plugin path | \*---------------------------------------------------------*/ - entry->ui->PathValue->setText(QString::fromStdString(plugin_manager->ActivePlugins[plugin_idx].path)); + entry->ui->PathValue->setText(QString::fromStdString(plugin.path)); /*---------------------------------------------------------*\ | Fill in plugin enabled status | \*---------------------------------------------------------*/ - entry->ui->EnabledCheckBox->setChecked((plugin_manager->ActivePlugins[plugin_idx].enabled)); + entry->ui->EnabledCheckBox->setChecked((plugin.enabled)); entry->RegisterEnableClickCallback(EnableClickCallbackFunction, this); @@ -261,7 +261,7 @@ void Ui::OpenRGBPluginsPage::on_EnableButton_clicked(OpenRGBPluginsEntry* entry) \*-----------------------------------------------------*/ std::string name = ""; std::string description = ""; - bool enabled = true; + bool enabled = entry->ui->EnabledCheckBox->isChecked(); bool found = false; unsigned int plugin_ct = 0; unsigned int plugin_idx = 0; @@ -270,15 +270,6 @@ void Ui::OpenRGBPluginsPage::on_EnableButton_clicked(OpenRGBPluginsEntry* entry) std::string entry_desc = entry->ui->DescriptionValue->text().toStdString(); std::string entry_path = entry->ui->PathValue->text().toStdString(); - if(entry->ui->EnabledCheckBox->isChecked()) - { - enabled = true; - } - else - { - enabled = false; - } - if(plugin_settings.contains("plugins")) { plugin_ct = plugin_settings["plugins"].size();