From f1ddc568e02fb82ee8f61423801ccd1ecf9aacef Mon Sep 17 00:00:00 2001 From: Cvolton Date: Thu, 15 Feb 2024 20:19:24 +0100 Subject: [PATCH 1/4] add more user friendly dependency messages --- loader/include/Geode/loader/Loader.hpp | 3 ++ loader/src/loader/LoaderImpl.cpp | 53 +++++++++++++++---- .../src/ui/internal/list/ProblemsListCell.cpp | 15 +++++- 3 files changed, 60 insertions(+), 11 deletions(-) diff --git a/loader/include/Geode/loader/Loader.hpp b/loader/include/Geode/loader/Loader.hpp index 2ec88781..fa2a6743 100644 --- a/loader/include/Geode/loader/Loader.hpp +++ b/loader/include/Geode/loader/Loader.hpp @@ -38,6 +38,9 @@ namespace geode { UnzipFailed, UnsupportedVersion, UnsupportedGeodeVersion, + NeedsNewerGeodeVersion, + DisabledDependency, + OutdatedDependency, }; Type type; std::variant cause; diff --git a/loader/src/loader/LoaderImpl.cpp b/loader/src/loader/LoaderImpl.cpp index 8add5818..1601595e 100644 --- a/loader/src/loader/LoaderImpl.cpp +++ b/loader/src/loader/LoaderImpl.cpp @@ -443,7 +443,7 @@ void Loader::Impl::loadModGraph(Mod* node, bool early) { node, res.unwrapErr() }); - log::error("Unsupported game version: {}", res.unwrapErr()); + log::error("Geometry Dash version {} is required to run this mod", res.unwrapErr()); m_refreshingModCount -= 1; log::popNest(); return; @@ -451,10 +451,10 @@ void Loader::Impl::loadModGraph(Mod* node, bool early) { if (!this->isModVersionSupported(node->getMetadata().getGeodeVersion())) { this->addProblem({ - LoadProblem::Type::UnsupportedGeodeVersion, + node->getMetadata().getGeodeVersion() > this->getVersion() ? LoadProblem::Type::NeedsNewerGeodeVersion : LoadProblem::Type::UnsupportedGeodeVersion, node, fmt::format( - "Geode version {} is not supported (current: {})", + "Geode version {}\nis required to run this mod\n(installed: {})", node->getMetadata().getGeodeVersion().toString(), this->getVersion().toString() ) @@ -539,13 +539,46 @@ void Loader::Impl::findProblems() { log::warn("{} recommends {} {}", id, dep.id, dep.version); break; case ModMetadata::Dependency::Importance::Required: - this->addProblem({ - LoadProblem::Type::MissingDependency, - mod, - fmt::format("{} {}", dep.id, dep.version.toString()) - }); - log::error("{} requires {} {}", id, dep.id, dep.version); - break; + if(m_mods.find(dep.id) == m_mods.end()) { + this->addProblem({ + LoadProblem::Type::MissingDependency, + mod, + fmt::format("{}", dep.id) + }); + log::error("{} requires {} {}", id, dep.id, dep.version); + break; + } else { + auto installedDependency = m_mods.at(dep.id); + + if(!installedDependency->isEnabled()) { + this->addProblem({ + LoadProblem::Type::DisabledDependency, + mod, + fmt::format("{}", dep.id) + }); + log::error("{} requires {} {}", id, dep.id, dep.version); + break; + } else if(!dep.version.compare(installedDependency->getVersion())) { + // TODO: this also fires on major version mismatch + this->addProblem({ + LoadProblem::Type::OutdatedDependency, + mod, + fmt::format("{}", dep.id) + }); + log::error("{} requires {} {}", id, dep.id, dep.version); + break; + } else { + // this should never happen i think? + // (major mismatch should eventually fall through here though once that's fixed) + this->addProblem({ + LoadProblem::Type::MissingDependency, + mod, + fmt::format("{}", dep.id) + }); + log::error("{} requires {} {}", id, dep.id, dep.version); + break; + } + } } } diff --git a/loader/src/ui/internal/list/ProblemsListCell.cpp b/loader/src/ui/internal/list/ProblemsListCell.cpp index f409b8c8..b494d99a 100644 --- a/loader/src/ui/internal/list/ProblemsListCell.cpp +++ b/loader/src/ui/internal/list/ProblemsListCell.cpp @@ -84,7 +84,15 @@ bool ProblemsListCell::init(LoadProblem problem, ProblemsListPopup* list, CCSize break; case LoadProblem::Type::MissingDependency: icon = "info-alert.png"_spr; - message = fmt::format("{} depends on {}", cause, problem.message); + message = fmt::format("Install {} to use {}", problem.message, cause); + break; + case LoadProblem::Type::DisabledDependency: + icon = "info-alert.png"_spr; + message = fmt::format("Enable {} to use {}", problem.message, cause); + break; + case LoadProblem::Type::OutdatedDependency: + icon = "info-alert.png"_spr; + message = fmt::format("Update {} to use {}", problem.message, cause); break; case LoadProblem::Type::PresentIncompatibility: icon = "info-alert.png"_spr; @@ -105,6 +113,11 @@ bool ProblemsListCell::init(LoadProblem problem, ProblemsListPopup* list, CCSize message = fmt::format("{} is incompatible with this version of Geode", cause); m_longMessage = problem.message; break; + case LoadProblem::Type::NeedsNewerGeodeVersion: + icon = "info-alert.png"_spr; + message = fmt::format("Update Geode to use {}", cause); + m_longMessage = problem.message; + break; } m_problem = std::move(problem); From 98d572c0e5e0e666498ee72d6e6362f5f244a2b0 Mon Sep 17 00:00:00 2001 From: Cvolton Date: Thu, 15 Feb 2024 21:34:20 +0100 Subject: [PATCH 2/4] add separate message for outdated incompat --- loader/include/Geode/loader/Loader.hpp | 1 + loader/include/Geode/utils/VersionInfo.hpp | 29 ++++++++++++++----- loader/src/loader/LoaderImpl.cpp | 14 ++++----- .../src/ui/internal/list/ProblemsListCell.cpp | 6 +++- 4 files changed, 33 insertions(+), 17 deletions(-) diff --git a/loader/include/Geode/loader/Loader.hpp b/loader/include/Geode/loader/Loader.hpp index fa2a6743..44de7429 100644 --- a/loader/include/Geode/loader/Loader.hpp +++ b/loader/include/Geode/loader/Loader.hpp @@ -41,6 +41,7 @@ namespace geode { NeedsNewerGeodeVersion, DisabledDependency, OutdatedDependency, + OutdatedIncompatibility, }; Type type; std::variant cause; diff --git a/loader/include/Geode/utils/VersionInfo.hpp b/loader/include/Geode/utils/VersionInfo.hpp index 9c6827c2..ee3d53a0 100644 --- a/loader/include/Geode/utils/VersionInfo.hpp +++ b/loader/include/Geode/utils/VersionInfo.hpp @@ -16,6 +16,14 @@ namespace geode { Any }; + enum class VersionCompareResult { + TooOld, + Match, + TooNew, + MajorMismatch, + GenericMismatch + }; + /** * A version label, like v1.0.0-alpha or v2.3.4-prerelease. Limited to these * options; arbitary identifiers are not supported. Additional numbering @@ -198,30 +206,35 @@ namespace geode { static Result parse(std::string const& string); constexpr bool compare(VersionInfo const& version) const { + return compareWithReason(version) == VersionCompareResult::Match; + } + + constexpr VersionCompareResult compareWithReason(VersionInfo const& version) const { if (m_compare == VersionCompare::Any) { - return true; + return VersionCompareResult::Match; } // opposing major versions never match if (m_version.getMajor() != version.getMajor()) { - return false; + return VersionCompareResult::MajorMismatch; } // the comparison works invertedly as a version like "v1.2.0" // should return true for "<=v1.3.0" switch (m_compare) { case VersionCompare::LessEq: - return version <= m_version; + return version <= m_version ? VersionCompareResult::Match : VersionCompareResult::TooNew; case VersionCompare::MoreEq: - return version >= m_version; + return version >= m_version ? VersionCompareResult::Match : VersionCompareResult::TooOld; case VersionCompare::Less: - return version < m_version; + return version < m_version ? VersionCompareResult::Match : VersionCompareResult::TooNew; case VersionCompare::More: - return version > m_version; + return version > m_version ? VersionCompareResult::Match : VersionCompareResult::TooOld; case VersionCompare::Exact: - return version == m_version; + return version == m_version ? VersionCompareResult::Match : + (version > m_version) ? VersionCompareResult::TooOld : VersionCompareResult::TooNew; default: - return false; + return VersionCompareResult::GenericMismatch; } } diff --git a/loader/src/loader/LoaderImpl.cpp b/loader/src/loader/LoaderImpl.cpp index 1601595e..85a0c308 100644 --- a/loader/src/loader/LoaderImpl.cpp +++ b/loader/src/loader/LoaderImpl.cpp @@ -558,8 +558,7 @@ void Loader::Impl::findProblems() { }); log::error("{} requires {} {}", id, dep.id, dep.version); break; - } else if(!dep.version.compare(installedDependency->getVersion())) { - // TODO: this also fires on major version mismatch + } else if(dep.version.compareWithReason(installedDependency->getVersion()) == VersionCompareResult::TooOld) { this->addProblem({ LoadProblem::Type::OutdatedDependency, mod, @@ -568,12 +567,11 @@ void Loader::Impl::findProblems() { log::error("{} requires {} {}", id, dep.id, dep.version); break; } else { - // this should never happen i think? - // (major mismatch should eventually fall through here though once that's fixed) + // fires on major mismatch or too new version of dependency this->addProblem({ LoadProblem::Type::MissingDependency, mod, - fmt::format("{}", dep.id) + fmt::format("{} {}", dep.id, dep.version) }); log::error("{} requires {} {}", id, dep.id, dep.version); break; @@ -597,9 +595,9 @@ void Loader::Impl::findProblems() { case ModMetadata::Incompatibility::Importance::Breaking: { this->addProblem({ - LoadProblem::Type::PresentIncompatibility, + dep.version.toString()[0] == '<' ? LoadProblem::Type::OutdatedIncompatibility : LoadProblem::Type::PresentIncompatibility, mod, - fmt::format("{} {}", dep.id, dep.version.toString()) + fmt::format("{}", dep.id) }); log::error("{} breaks {} {}", id, dep.id, dep.version); } break; @@ -608,7 +606,7 @@ void Loader::Impl::findProblems() { this->addProblem({ LoadProblem::Type::PresentIncompatibility, mod, - fmt::format("{} {}", dep.id, dep.version.toString()) + fmt::format("{}", dep.id) }); log::error("{} supersedes {} {}", id, dep.id, dep.version); } break; diff --git a/loader/src/ui/internal/list/ProblemsListCell.cpp b/loader/src/ui/internal/list/ProblemsListCell.cpp index b494d99a..e5d3140a 100644 --- a/loader/src/ui/internal/list/ProblemsListCell.cpp +++ b/loader/src/ui/internal/list/ProblemsListCell.cpp @@ -96,7 +96,11 @@ bool ProblemsListCell::init(LoadProblem problem, ProblemsListPopup* list, CCSize break; case LoadProblem::Type::PresentIncompatibility: icon = "info-alert.png"_spr; - message = fmt::format("{} is incompatible with {}", cause, problem.message); + message = fmt::format("Uninstall {} to use {}", problem.message, cause); + break; + case LoadProblem::Type::OutdatedIncompatibility: + icon = "info-alert.png"_spr; + message = fmt::format("Update {} to use {}", problem.message, cause); break; case LoadProblem::Type::UnzipFailed: icon = "info-alert.png"_spr; From 4c354ed4606030e3ca16ecad95ac5a080fde7705 Mon Sep 17 00:00:00 2001 From: Cvolton Date: Thu, 15 Feb 2024 21:40:54 +0100 Subject: [PATCH 3/4] show incompatibilities on top of problems list --- loader/src/ui/internal/list/ProblemsListPopup.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/loader/src/ui/internal/list/ProblemsListPopup.cpp b/loader/src/ui/internal/list/ProblemsListPopup.cpp index 336128af..3b2332b8 100644 --- a/loader/src/ui/internal/list/ProblemsListPopup.cpp +++ b/loader/src/ui/internal/list/ProblemsListPopup.cpp @@ -38,6 +38,7 @@ void ProblemsListPopup::createList(Mod* scrollTo) { } CCArray* ProblemsListPopup::createCells(Mod* scrollTo, float& scrollValue) { + std::vector veryTop; std::vector top; std::vector middle; std::vector bottom; @@ -50,6 +51,9 @@ CCArray* ProblemsListPopup::createCells(Mod* scrollTo, float& scrollValue) { case geode::LoadProblem::Type::Recommendation: middle.push_back(ProblemsListCell::create(problem, this, this->getCellSize())); break; + case geode::LoadProblem::Type::OutdatedIncompatibility: + case geode::LoadProblem::Type::PresentIncompatibility: + veryTop.push_back(ProblemsListCell::create(problem, this, this->getCellSize())); default: top.push_back(ProblemsListCell::create(problem, this, this->getCellSize())); break; @@ -69,6 +73,10 @@ CCArray* ProblemsListPopup::createCells(Mod* scrollTo, float& scrollValue) { scrollFound = true; }; + for (auto const& item : veryTop) { + tryFindScroll(item); + final->addObject(item); + } for (auto const& item : top) { tryFindScroll(item); final->addObject(item); From 4b667cc82c91e0297a064decfc43dd0f4bd9c468 Mon Sep 17 00:00:00 2001 From: Cvolton Date: Thu, 15 Feb 2024 21:58:30 +0100 Subject: [PATCH 4/4] change message for conflicting mods as well --- loader/include/Geode/loader/Loader.hpp | 1 + loader/src/loader/LoaderImpl.cpp | 4 ++-- loader/src/ui/internal/list/ProblemsListCell.cpp | 10 +++++++++- loader/src/ui/internal/list/ProblemsListPopup.cpp | 1 + 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/loader/include/Geode/loader/Loader.hpp b/loader/include/Geode/loader/Loader.hpp index 44de7429..c310d8b3 100644 --- a/loader/include/Geode/loader/Loader.hpp +++ b/loader/include/Geode/loader/Loader.hpp @@ -28,6 +28,7 @@ namespace geode { Suggestion, Recommendation, Conflict, + OutdatedConflict, InvalidFile, Duplicate, SetupFailed, diff --git a/loader/src/loader/LoaderImpl.cpp b/loader/src/loader/LoaderImpl.cpp index 85a0c308..6b20c219 100644 --- a/loader/src/loader/LoaderImpl.cpp +++ b/loader/src/loader/LoaderImpl.cpp @@ -586,9 +586,9 @@ void Loader::Impl::findProblems() { switch(dep.importance) { case ModMetadata::Incompatibility::Importance::Conflicting: { this->addProblem({ - LoadProblem::Type::Conflict, + dep.version.toString()[0] == '<' ? LoadProblem::Type::OutdatedConflict : LoadProblem::Type::Conflict, mod, - fmt::format("{} {}", dep.id, dep.version.toString()) + fmt::format("{}", dep.id) }); log::warn("{} conflicts with {} {}", id, dep.id, dep.version); } break; diff --git a/loader/src/ui/internal/list/ProblemsListCell.cpp b/loader/src/ui/internal/list/ProblemsListCell.cpp index e5d3140a..ec231a82 100644 --- a/loader/src/ui/internal/list/ProblemsListCell.cpp +++ b/loader/src/ui/internal/list/ProblemsListCell.cpp @@ -54,8 +54,16 @@ bool ProblemsListCell::init(LoadProblem problem, ProblemsListPopup* list, CCSize message = fmt::format("{} recommends {}", cause, problem.message); break; case LoadProblem::Type::Conflict: + // i copy pasted the message from incompatibility + // because as far as i can tell there's no behavorial + // difference, so it makes no sense to show the difference + // to the user icon = "info-warning.png"_spr; - message = fmt::format("{} conflicts with {}", cause, problem.message); + message = fmt::format("Uninstall {} to use {}", problem.message, cause); + break; + case LoadProblem::Type::OutdatedConflict: + icon = "info-alert.png"_spr; + message = fmt::format("Update {} to use {}", problem.message, cause); break; case LoadProblem::Type::InvalidFile: icon = "info-alert.png"_spr; diff --git a/loader/src/ui/internal/list/ProblemsListPopup.cpp b/loader/src/ui/internal/list/ProblemsListPopup.cpp index 3b2332b8..00daecbb 100644 --- a/loader/src/ui/internal/list/ProblemsListPopup.cpp +++ b/loader/src/ui/internal/list/ProblemsListPopup.cpp @@ -54,6 +54,7 @@ CCArray* ProblemsListPopup::createCells(Mod* scrollTo, float& scrollValue) { case geode::LoadProblem::Type::OutdatedIncompatibility: case geode::LoadProblem::Type::PresentIncompatibility: veryTop.push_back(ProblemsListCell::create(problem, this, this->getCellSize())); + break; default: top.push_back(ProblemsListCell::create(problem, this, this->getCellSize())); break;