From 54c198c5b47f13b09109ab7000c8c31106d545f4 Mon Sep 17 00:00:00 2001 From: Tobias Markus Date: Tue, 1 Aug 2023 17:15:28 +0200 Subject: [PATCH] Use std::map for addons instead of list (#2565) * Use std::map for addons instead of list * Use C++17 stuff * More C++17 --- src/addon/addon_manager.cpp | 83 +++++++++++++++---------------------- src/addon/addon_manager.hpp | 9 ++-- src/addon/downloader.cpp | 57 +++++++++---------------- src/addon/downloader.hpp | 3 +- 4 files changed, 59 insertions(+), 93 deletions(-) diff --git a/src/addon/addon_manager.cpp b/src/addon/addon_manager.cpp index c0f0439324d..9127e11b14d 100644 --- a/src/addon/addon_manager.cpp +++ b/src/addon/addon_manager.cpp @@ -80,18 +80,14 @@ MD5 md5_from_archive(const std::string& filename) } } -static Addon& get_addon(const AddonManager::AddonList& list, const AddonId& id, +static Addon& get_addon(const AddonManager::AddonMap& list, const AddonId& id, bool installed) { - auto it = std::find_if(list.begin(), list.end(), - [&id](const std::unique_ptr& addon) - { - return addon->get_id() == id; - }); + auto it = list.find(id); if (it != list.end()) { - return **it; + return *(it->second); } else { @@ -100,15 +96,14 @@ static Addon& get_addon(const AddonManager::AddonList& list, const AddonId& id, } } -static std::vector get_addons(const AddonManager::AddonList& list) +static std::vector get_addons(const AddonManager::AddonMap& list) { // Use a map for storing sorted addon titles with their respective IDs. std::map sorted_titles; - std::for_each(list.begin(), list.end(), - [&](const std::unique_ptr& addon) - { - sorted_titles.insert({addon->get_title(), addon->get_id()}); - }); + for (const auto& [id, addon] : list) + { + sorted_titles.insert({addon->get_title(), id}); + } std::vector results; results.reserve(sorted_titles.size()); std::transform(sorted_titles.begin(), sorted_titles.end(), @@ -223,9 +218,9 @@ AddonManager::~AddonManager() { // sync enabled/disabled add-ons into the config for saving m_addon_config.clear(); - for (const auto& addon : m_installed_addons) + for (const auto& [id, addon] : m_installed_addons) { - m_addon_config.push_back({addon->get_id(), addon->is_enabled()}); + m_addon_config.push_back({id, addon->is_enabled()}); } // Delete the add-on cache directory, if it exists. @@ -307,17 +302,13 @@ TransferStatusListPtr AddonManager::request_install_addon(const AddonId& addon_id) { // remove addon if it already exists - auto it = std::find_if(m_installed_addons.begin(), m_installed_addons.end(), - [&addon_id](const std::unique_ptr& addon) - { - return addon->get_id() == addon_id; - }); + auto it = m_installed_addons.find(addon_id); if (it != m_installed_addons.end()) { log_debug << "reinstalling addon " << addon_id << std::endl; - if ((*it)->is_enabled()) + if (it->second->is_enabled()) { - disable_addon((*it)->get_id()); + disable_addon(it->first); } m_installed_addons.erase(it); } @@ -413,17 +404,13 @@ void AddonManager::install_addon(const AddonId& addon_id) { { // remove addon if it already exists - auto it = std::find_if(m_installed_addons.begin(), m_installed_addons.end(), - [&addon_id](const std::unique_ptr& addon) - { - return addon->get_id() == addon_id; - }); + auto it = m_installed_addons.find(addon_id); if (it != m_installed_addons.end()) { log_debug << "reinstalling addon " << addon_id << std::endl; - if ((*it)->is_enabled()) + if (it->second->is_enabled()) { - disable_addon((*it)->get_id()); + disable_addon(it->first); } m_installed_addons.erase(it); } @@ -489,11 +476,7 @@ AddonManager::uninstall_addon(const AddonId& addon_id) disable_addon(addon_id); } log_debug << "deleting file \"" << addon.get_install_filename() << "\"" << std::endl; - const auto it = std::find_if(m_installed_addons.begin(), m_installed_addons.end(), - [&addon](const std::unique_ptr& rhs) - { - return addon.get_id() == rhs->get_id(); - }); + const auto it = m_installed_addons.find(addon.get_id()); if (it != m_installed_addons.end()) { if (PHYSFS_delete(FileSystem::join(m_addon_directory, addon.get_filename()).c_str()) == 0) @@ -568,10 +551,10 @@ AddonManager::enable_addon(const AddonId& addon_id) { if (addon.get_type() == Addon::RESOURCEPACK) { - for (const auto& installed_addon : m_installed_addons) + for (const auto& [id, addon] : m_installed_addons) { - if (installed_addon->get_type() == Addon::RESOURCEPACK && - installed_addon->is_enabled()) + if (addon->get_type() == Addon::RESOURCEPACK && + addon->is_enabled()) { throw std::runtime_error(_("Only one resource pack is allowed to be enabled at a time.")); } @@ -662,9 +645,9 @@ AddonManager::is_old_enabled_addon(const std::unique_ptr& addon) const bool AddonManager::is_old_addon_enabled() const { auto it = std::find_if(m_installed_addons.begin(), m_installed_addons.end(), - [this](const std::unique_ptr& addon) + [this](const auto& addon) { - return is_old_enabled_addon(addon); + return is_old_enabled_addon(addon.second); }); return it != m_installed_addons.end(); @@ -673,9 +656,9 @@ AddonManager::is_old_addon_enabled() const { void AddonManager::disable_old_addons() { - for (auto& addon : m_installed_addons) { + for (auto& [id, addon] : m_installed_addons) { if (is_old_enabled_addon(addon)) { - disable_addon(addon->get_id()); + disable_addon(id); } } } @@ -684,7 +667,7 @@ void AddonManager::mount_old_addons() { std::string mountpoint; - for (auto& addon : m_installed_addons) { + for (auto& [id, addon] : m_installed_addons) { if (is_old_enabled_addon(addon)) { if (PHYSFS_mount(addon->get_install_filename().c_str(), mountpoint.c_str(), !addon->overrides_data()) == 0) { @@ -698,7 +681,7 @@ AddonManager::mount_old_addons() void AddonManager::unmount_old_addons() { - for (auto& addon : m_installed_addons) { + for (auto& [id, addon] : m_installed_addons) { if (is_old_enabled_addon(addon)) { if (PHYSFS_unmount(addon->get_install_filename().c_str()) == 0) { @@ -713,7 +696,7 @@ bool AddonManager::is_from_old_addon(const std::string& filename) const { std::string real_path = PHYSFS_getRealDir(filename.c_str()); - for (auto& addon : m_installed_addons) { + for (auto& [id, addon] : m_installed_addons) { if (is_old_enabled_addon(addon) && addon->get_install_filename() == real_path) { return true; @@ -736,11 +719,11 @@ std::vector AddonManager::get_depending_addons(const std::string& id) const { std::vector addons; - for (auto& addon : m_installed_addons) + for (auto& [id, addon] : m_installed_addons) { const auto& dependencies = addon->get_dependencies(); if (std::find(dependencies.begin(), dependencies.end(), id) != dependencies.end()) - addons.push_back(addon->get_id()); + addons.push_back(id); } return addons; } @@ -846,7 +829,7 @@ AddonManager::add_installed_archive(const std::string& archive, const std::strin // save addon title and author on stack before std::move const std::string addon_title = addon->get_title(); const std::string addon_author = addon->get_author(); - m_installed_addons.push_back(std::move(addon)); + m_installed_addons[addon_id] = std::move(addon); if(user_install) { try @@ -892,10 +875,10 @@ AddonManager::add_installed_addons() } } -AddonManager::AddonList +AddonManager::AddonMap AddonManager::parse_addon_infos(const std::string& filename) const { - AddonList m_addons; + AddonMap m_addons; try { @@ -920,7 +903,7 @@ AddonManager::parse_addon_infos(const std::string& filename) const try { std::unique_ptr addon = Addon::parse(addon_node.get_mapping()); - m_addons.push_back(std::move(addon)); + m_addons[addon->get_id()] = std::move(addon); } catch(const std::exception& e) { diff --git a/src/addon/addon_manager.hpp b/src/addon/addon_manager.hpp index 4a8ac7164c8..7e3e9a0d9e9 100644 --- a/src/addon/addon_manager.hpp +++ b/src/addon/addon_manager.hpp @@ -20,6 +20,7 @@ #include #include +#include #include #include "addon/downloader.hpp" @@ -35,7 +36,7 @@ typedef std::string AddonId; class AddonManager final : public Currenton { public: - using AddonList = std::vector >; + using AddonMap = std::map >; private: Downloader m_downloader; @@ -45,8 +46,8 @@ class AddonManager final : public Currenton std::string m_repository_url; std::vector& m_addon_config; - AddonList m_installed_addons; - AddonList m_repository_addons; + AddonMap m_installed_addons; + AddonMap m_repository_addons; bool m_initialized; bool m_has_been_updated; @@ -108,7 +109,7 @@ class AddonManager final : public Currenton std::vector scan_for_archives() const; void add_installed_addons(); - AddonList parse_addon_infos(const std::string& filename) const; + AddonMap parse_addon_infos(const std::string& filename) const; /** add \a archive, given as physfs path, to the list of installed archives */ diff --git a/src/addon/downloader.cpp b/src/addon/downloader.cpp index 2406980e068..9e71d606264 100644 --- a/src/addon/downloader.cpp +++ b/src/addon/downloader.cpp @@ -402,7 +402,7 @@ Downloader::~Downloader() #ifndef EMSCRIPTEN for (auto& transfer : m_transfers) { - curl_multi_remove_handle(m_multi_handle, transfer->get_curl_handle()); + curl_multi_remove_handle(m_multi_handle, transfer.second->get_curl_handle()); } #endif m_transfers.clear(); @@ -478,21 +478,17 @@ Downloader::download(const std::string& url, const std::string& filename) void Downloader::abort(TransferId id) { - auto it = std::find_if(m_transfers.begin(), m_transfers.end(), - [&id](const std::unique_ptr& rhs) - { - return id == rhs->get_id(); - }); + auto it = m_transfers.find(id); if (it == m_transfers.end()) { log_warning << "transfer not found: " << id << std::endl; } else { - TransferStatusPtr status = (*it)->get_status(); + TransferStatusPtr status = (it->second)->get_status(); #ifndef EMSCRIPTEN - curl_multi_remove_handle(m_multi_handle, (*it)->get_curl_handle()); + curl_multi_remove_handle(m_multi_handle, it->second->get_curl_handle()); #endif m_transfers.erase(it); @@ -540,12 +536,12 @@ Downloader::update() curl_multi_remove_handle(m_multi_handle, msg->easy_handle); auto it = std::find_if(m_transfers.begin(), m_transfers.end(), - [&msg](const std::unique_ptr& rhs) { - return rhs->get_curl_handle() == msg->easy_handle; + [&msg](const auto& rhs) { + return rhs.second->get_curl_handle() == msg->easy_handle; }); assert(it != m_transfers.end()); - TransferStatusPtr status = (*it)->get_status(); - status->error_msg = (*it)->get_error_buffer(); + TransferStatusPtr status = it->second->get_status(); + status->error_msg = it->second->get_error_buffer(); m_transfers.erase(it); if (resultfromcurl == CURLE_OK) @@ -603,44 +599,37 @@ Downloader::request_download(const std::string& url, const std::string& outfile) #ifndef EMSCRIPTEN curl_multi_add_handle(m_multi_handle, transfer->get_curl_handle()); #endif - m_transfers.push_back(std::move(transfer)); - return m_transfers.back()->get_status(); + auto transferId = transfer->get_id(); + m_transfers[transferId] = std::move(transfer); + return m_transfers[transferId]->get_status(); } #ifdef EMSCRIPTEN void Downloader::onDownloadProgress(int id, int loaded, int total) { - auto it = std::find_if(m_transfers.begin(), m_transfers.end(), - [&id](const std::unique_ptr& rhs) - { - return id == rhs->get_id(); - }); + auto it = m_transfers.find(id); if (it == m_transfers.end()) { log_warning << "transfer not found: " << id << std::endl; } else { - (*it)->on_progress(static_cast(loaded), static_cast(total), 0.0, 0.0); + (it->second)->on_progress(static_cast(loaded), static_cast(total), 0.0, 0.0); } } void Downloader::onDownloadFinished(int id) { - auto it = std::find_if(m_transfers.begin(), m_transfers.end(), - [&id](const std::unique_ptr& rhs) - { - return id == rhs->get_id(); - }); + auto it = m_transfers.find(id); if (it == m_transfers.end()) { log_warning << "transfer not found: " << id << std::endl; } else { - for (const auto& callback : (*it)->get_status()->callbacks) + for (const auto& callback : it->second->get_status()->callbacks) { try { @@ -657,18 +646,14 @@ Downloader::onDownloadFinished(int id) void Downloader::onDownloadError(int id) { - auto it = std::find_if(m_transfers.begin(), m_transfers.end(), - [&id](const std::unique_ptr& rhs) - { - return id == rhs->get_id(); - }); + auto it = m_transfers.find(id); if (it == m_transfers.end()) { log_warning << "transfer not found: " << id << std::endl; } else { - for (const auto& callback : (*it)->get_status()->callbacks) + for (const auto& callback : it->second->get_status()->callbacks) { try { @@ -685,18 +670,14 @@ Downloader::onDownloadError(int id) void Downloader::onDownloadAborted(int id) { - auto it = std::find_if(m_transfers.begin(), m_transfers.end(), - [&id](const std::unique_ptr& rhs) - { - return id == rhs->get_id(); - }); + auto it = m_transfers.find(id); if (it == m_transfers.end()) { log_warning << "transfer not found: " << id << std::endl; } else { - for (const auto& callback : (*it)->get_status()->callbacks) + for (const auto& callback : it->second->get_status()->callbacks) { try { diff --git a/src/addon/downloader.hpp b/src/addon/downloader.hpp index 181af539560..5e6bfb8de54 100644 --- a/src/addon/downloader.hpp +++ b/src/addon/downloader.hpp @@ -24,6 +24,7 @@ #include #endif #include +#include #include #include @@ -120,7 +121,7 @@ class Downloader final #ifndef EMSCRIPTEN CURLM* m_multi_handle; #endif - std::vector > m_transfers; + std::map > m_transfers; int m_next_transfer_id; float m_last_update_time;