From 55f44a6257f72abb233481aba105e12e76c36017 Mon Sep 17 00:00:00 2001 From: GriffinR Date: Thu, 10 Apr 2025 22:37:32 -0400 Subject: [PATCH 1/3] Add project version check via git --- include/config.h | 2 + include/mainwindow.h | 2 +- include/project.h | 1 + src/config.cpp | 3 ++ src/mainwindow.cpp | 58 +++++++++++++++++------ src/project.cpp | 108 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 159 insertions(+), 15 deletions(-) diff --git a/include/config.h b/include/config.h index eb9c9ac3..4bfc2bc5 100644 --- a/include/config.h +++ b/include/config.h @@ -336,6 +336,7 @@ public: this->unusedTileCovered = 0x0000; this->unusedTileSplit = 0x0000; this->maxEventsPerGroup = 255; + this->forcedMajorVersion = 0; this->identifiers.clear(); this->readKeys.clear(); } @@ -406,6 +407,7 @@ public: int collisionSheetHeight; QList warpBehaviors; int maxEventsPerGroup; + int forcedMajorVersion; protected: virtual QString getConfigFilepath() override; diff --git a/include/mainwindow.h b/include/mainwindow.h index cdc641c2..b3df2e9b 100644 --- a/include/mainwindow.h +++ b/include/mainwindow.h @@ -355,7 +355,7 @@ private: void refreshCollisionSelector(); void setLayoutOnlyMode(bool layoutOnly); - bool checkProjectSanity(); + bool isInvalidProject(Project *project); bool loadProjectData(); bool setProjectUI(); void clearProjectUI(); diff --git a/include/project.h b/include/project.h index 9a031c0d..554fb3c7 100644 --- a/include/project.h +++ b/include/project.h @@ -87,6 +87,7 @@ public: void clearHealLocations(); bool sanityCheck(); + int getSupportedMajorVersion(QString *errorOut = nullptr); bool load(); Map* loadMap(const QString &mapName); diff --git a/src/config.cpp b/src/config.cpp index 42a59149..8a17a45c 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -835,6 +835,8 @@ void ProjectConfig::parseConfigKeyValue(QString key, QString value) { this->warpBehaviors.append(getConfigUint32(key, s)); } else if (key == "max_events_per_group") { this->maxEventsPerGroup = getConfigInteger(key, value, 1, INT_MAX, 255); + } else if (key == "forced_major_version") { + this->forcedMajorVersion = getConfigInteger(key, value); } else { logWarn(QString("Invalid config key found in config file %1: '%2'").arg(this->getConfigFilepath()).arg(key)); } @@ -931,6 +933,7 @@ QMap ProjectConfig::getKeyValueMap() { warpBehaviorStrs.append("0x" + QString("%1").arg(value, 2, 16, QChar('0')).toUpper()); map.insert("warp_behaviors", warpBehaviorStrs.join(",")); map.insert("max_events_per_group", QString::number(this->maxEventsPerGroup)); + map.insert("forced_major_version", QString::number(this->forcedMajorVersion)); return map; } diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 650acdaf..6a3bdb56 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -668,7 +668,7 @@ bool MainWindow::openProject(QString dir, bool initial) { this->editor->setProject(project); // Make sure project looks reasonable before attempting to load it - if (!checkProjectSanity()) { + if (isInvalidProject(this->editor->project)) { delete this->editor->project; return false; } @@ -708,22 +708,52 @@ bool MainWindow::loadProjectData() { return success; } -bool MainWindow::checkProjectSanity() { - if (editor->project->sanityCheck()) - return true; +bool MainWindow::isInvalidProject(Project *project) { + if (!project->sanityCheck()) { + logWarn(QString("The directory '%1' failed the project sanity check.").arg(project->root)); - logWarn(QString("The directory '%1' failed the project sanity check.").arg(editor->project->root)); + ErrorMessage msgBox(QStringLiteral("The selected directory appears to be invalid."), this); + msgBox.setInformativeText(QString("The directory '%1' is missing key files.\n\n" + "Make sure you selected the correct project directory " + "(the one used to make your .gba file, e.g. 'pokeemerald').").arg(project->root)); + auto tryAnyway = msgBox.addButton("Try Anyway", QMessageBox::ActionRole); + msgBox.exec(); - ErrorMessage msgBox(QStringLiteral("The selected directory appears to be invalid."), this); - msgBox.setInformativeText(QString("The directory '%1' is missing key files.\n\n" - "Make sure you selected the correct project directory " - "(the one used to make your .gba file, e.g. 'pokeemerald').").arg(editor->project->root)); - auto tryAnyway = msgBox.addButton("Try Anyway", QMessageBox::ActionRole); - msgBox.exec(); - if (msgBox.clickedButton() == tryAnyway) { - // The user has chosen to try to load this project anyway. + // The user may choose to try to load this project anyway. // This will almost certainly fail, but they'll get a more specific error message. - return true; + if (msgBox.clickedButton() != tryAnyway){ + return true; + } + } + QString error; + int projectVersion = project->getSupportedMajorVersion(&error); + if (projectVersion <= 0) { + // Failed to identify a supported major version. + // We can't draw any conclusions from this, so we don't consider the project to be invalid. + logWarn(error.isEmpty() ? QStringLiteral("Unable to identify project's Porymap version.") : error); + } else { + logInfo(QString("Successful project version check. Supports at least Porymap v%1.").arg(projectVersion)); + + if (projectVersion < porymapVersion.majorVersion() && projectConfig.forcedMajorVersion < porymapVersion.majorVersion()) { + // We were unable to find the necessary changes for Porymap's current major version. + // Warn the user that this might mean their project is missing breaking changes. + // Note: Do not report 'projectVersion' to the user in this message. We've already logged it for troubleshooting. + // It is very plausible that the user may have reproduced the required changes in an + // unknown commit, rather than merging the required changes directly from the base repo. + // In this case the 'projectVersion' may actually be too old to use for their repo. + ErrorMessage msgBox(QStringLiteral("Your project may be incompatible!"), this); + msgBox.setInformativeText(QString("Make sure '%1' has all the required changes for Porymap version %2." + "") // TODO: Once we have a wiki or manual page describing breaking changes, link that here. + .arg(project->getProjectTitle()) + .arg(porymapVersion.majorVersion())); + auto tryAnyway = msgBox.addButton("Try Anyway", QMessageBox::ActionRole); + msgBox.exec(); + if (msgBox.clickedButton() != tryAnyway){ + return true; + } + // User opted to try with this version anyway. Don't warn them about this version again. + projectConfig.forcedMajorVersion = porymapVersion.majorVersion(); + } } return false; } diff --git a/src/project.cpp b/src/project.cpp index bbbc052c..1b165df2 100644 --- a/src/project.cpp +++ b/src/project.cpp @@ -77,6 +77,114 @@ bool Project::sanityCheck() { return false; } +// Porymap projects have no standardized way for Porymap to determine whether they're compatible as of the latest breaking changes. +// We can use the project's git history (if it has one, and we're able to get it) to make a reasonable guess. +// We know the hashes of the commits in the base repos that contain breaking changes, so if we find one of these then the project +// should support at least up to that Porymap major version. If this fails for any reason it returns a version of -1. +// This has relatively tight timeout windows (500ms for each process, compared to the default 30,000ms). This version check +// is not important enough to significantly slow down project launch, we'd rather just timeout. +int Project::getSupportedMajorVersion(QString *errorOut) { + const int failureVersion = -1; + QString gitPath = QStandardPaths::findExecutable("git"); + if (gitPath.isEmpty()) { + if (errorOut) *errorOut = QStringLiteral("Failed to identify project history: Unable to locate git."); + return failureVersion; + } + + QProcess process; + process.setWorkingDirectory(this->root); + process.setProgram(gitPath); + process.setReadChannel(QProcess::StandardOutput); + process.setStandardInputFile(QProcess::nullDevice()); // We won't have any writing to do. + + // First we need to know which (if any) known git history this project belongs to. + // We'll get the root commit, then compare it to the known root commits for the base project repos. + static const QStringList args_getRootCommit = { "rev-list", "--max-parents=0", "HEAD" }; + process.setArguments(args_getRootCommit); + process.start(); + if (!process.waitForFinished(500) || process.exitStatus() != QProcess::ExitStatus::NormalExit || process.exitCode() != 0) { + if (errorOut) { + *errorOut = QStringLiteral("Failed to identify project history"); + if (process.error() != QProcess::UnknownError && !process.errorString().isEmpty()) { + errorOut->append(QString(": %1").arg(process.errorString())); + } else { + process.setReadChannel(QProcess::StandardError); + QString error = QString(process.readLine()).remove('\n'); + if (!error.isEmpty()) errorOut->append(QString(": %1").arg(error)); + } + } + return failureVersion; + } + const QString rootCommit = QString(process.readLine()).remove('\n'); + + // The keys in this map are the hashes of the root commits for each of the 3 base repos. + // The values are a list of pairs, where the first element is a major version number, and the + // second element is the hash of the earliest commit that supports that major version. + static const QMap>> historyMap = { + // pokeemerald + {"33b799c967fd63d04afe82eecc4892f3e45781b3", { + {6, "07c897ad48c36b178093bde8ca360823127d812b"}, // TODO: Update to merge commit for pokeemerald's porymap-6 branch + {5, "c76beed98990a57c84d3930190fd194abfedf7e8"}, + {4, "cb5b8da77b9ba6837fcc8c5163bedc5008b12c2c"}, + {3, "204c431993dad29661a9ff47326787cd0cf381e6"}, + {2, "cdae0c1444bed98e652c87dc3e3edcecacfef8be"}, + {1, ""} + }}, + // pokefirered + {"670fef77ac4d9116d5fdc28c0da40622919a062b", { + {6, "7722e7a92ca5fa69925dcef82f6c89c35ec48171"}, // TODO: Update to merge commit for pokefirered's porymap-6 branch + {5, "52591dcee42933d64f60c59276fc13c3bb89c47b"}, + {4, "200c82e01a94dbe535e6ed8768d8afad4444d4d2"}, + }}, + // pokeruby + {"1362b60f3467f0894d55e82f3294980b6373021d", { + {6, "bc5aeaa64ecad03aa4ab9e1000ba94916276c936"}, // TODO: Update to merge commit for pokeruby's porymap-6 branch + {5, "d99cb43736dd1d4ee4820f838cb259d773d8bf25"}, + {4, "f302fcc134bf354c3655e3423be68fd7a99cb396"}, + {3, "b4f4d2c0f03462dcdf3492aad27890294600eb2e"}, + {2, "0e8ccfc4fd3544001f4c25fafd401f7558bdefba"}, + {1, ""} + }}, + }; + if (!historyMap.contains(rootCommit)) { + // Either this repo does not share history with one of the base repos, + // (that's ok, don't report an error) or we got some unexpected result. + return failureVersion; + } + + // We now know which base repo that the user's repo shares history with. + // Next we check to see if it contains the changes required to support particular major versions of Porymap. + // We'll start with the most recent latest version and work backwards. + for (const auto &pair : historyMap.value(rootCommit)) { + int versionNum = pair.first; + QString commitHash = pair.second; + if (commitHash.isEmpty()) { + // An empty commit hash means 'consider any point in the history a supported version' + return versionNum; + } + process.setArguments({ "merge-base", "--is-ancestor", commitHash, "HEAD" }); + process.start(); + if (!process.waitForFinished(500) || process.exitStatus() != QProcess::ExitStatus::NormalExit) { + if (errorOut) { + *errorOut = QStringLiteral("Failed to identify project's supported Porymap version"); + if (process.error() != QProcess::UnknownError && !process.errorString().isEmpty()) { + errorOut->append(QString(": %1").arg(process.errorString())); + } else { + process.setReadChannel(QProcess::StandardError); + QString error = QString(process.readLine()).remove('\n'); + if (!error.isEmpty()) errorOut->append(QString(": %1").arg(error)); + } + } + return failureVersion; + } + if (process.exitCode() == 0) { + // Identified a supported major version + return versionNum; + } + } + return failureVersion; +} + bool Project::load() { resetFileCache(); this->disabledSettingsNames.clear(); From 168792b9c6d7da724967151e55cf8f5d7a5be8f3 Mon Sep 17 00:00:00 2001 From: GriffinR Date: Sun, 13 Apr 2025 23:10:51 -0400 Subject: [PATCH 2/3] Fix comment typo --- src/project.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/project.cpp b/src/project.cpp index 1b165df2..9ce9f6ed 100644 --- a/src/project.cpp +++ b/src/project.cpp @@ -154,7 +154,7 @@ int Project::getSupportedMajorVersion(QString *errorOut) { // We now know which base repo that the user's repo shares history with. // Next we check to see if it contains the changes required to support particular major versions of Porymap. - // We'll start with the most recent latest version and work backwards. + // We'll start with the most recent major version and work backwards. for (const auto &pair : historyMap.value(rootCommit)) { int versionNum = pair.first; QString commitHash = pair.second; From 31cf00de8b5b2e29bc4e1e9e36228374d5fdb483 Mon Sep 17 00:00:00 2001 From: GriffinR Date: Wed, 30 Apr 2025 13:03:30 -0400 Subject: [PATCH 3/3] Update git search error messages --- include/mainwindow.h | 2 ++ src/mainwindow.cpp | 49 +++++++++++++++++++++++++++----------------- src/project.cpp | 27 +++++++++++++----------- 3 files changed, 47 insertions(+), 31 deletions(-) diff --git a/include/mainwindow.h b/include/mainwindow.h index b2bba6ca..5de2a50a 100644 --- a/include/mainwindow.h +++ b/include/mainwindow.h @@ -352,6 +352,8 @@ private: void setLayoutOnlyMode(bool layoutOnly); bool isInvalidProject(Project *project); + bool checkProjectSanity(Project *project); + bool checkProjectVersion(Project *project); bool loadProjectData(); bool setProjectUI(); void clearProjectUI(); diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index e15828ae..4e82dce4 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -725,39 +725,50 @@ bool MainWindow::loadProjectData() { } bool MainWindow::isInvalidProject(Project *project) { - if (!project->sanityCheck()) { - logWarn(QString("The directory '%1' failed the project sanity check.").arg(project->root)); + return !(checkProjectSanity(project) && checkProjectVersion(project)); +} - ErrorMessage msgBox(QStringLiteral("The selected directory appears to be invalid."), porysplash); - msgBox.setInformativeText(QString("The directory '%1' is missing key files.\n\n" - "Make sure you selected the correct project directory " - "(the one used to make your .gba file, e.g. 'pokeemerald').").arg(project->root)); - auto tryAnyway = msgBox.addButton("Try Anyway", QMessageBox::ActionRole); - msgBox.exec(); +bool MainWindow::checkProjectSanity(Project *project) { + if (project->sanityCheck()) + return true; - // The user may choose to try to load this project anyway. + logWarn(QString("The directory '%1' failed the project sanity check.").arg(project->root)); + + ErrorMessage msgBox(QStringLiteral("The selected directory appears to be invalid."), porysplash); + msgBox.setInformativeText(QString("The directory '%1' is missing key files.\n\n" + "Make sure you selected the correct project directory " + "(the one used to make your .gba file, e.g. 'pokeemerald').").arg(project->root)); + auto tryAnyway = msgBox.addButton("Try Anyway", QMessageBox::ActionRole); + msgBox.exec(); + if (msgBox.clickedButton() == tryAnyway) { + // The user has chosen to try to load this project anyway. // This will almost certainly fail, but they'll get a more specific error message. - if (msgBox.clickedButton() != tryAnyway){ - return true; - } + return true; } + return false; +} + +bool MainWindow::checkProjectVersion(Project *project) { QString error; int projectVersion = project->getSupportedMajorVersion(&error); - if (projectVersion <= 0) { + if (projectVersion < 0) { // Failed to identify a supported major version. // We can't draw any conclusions from this, so we don't consider the project to be invalid. - logWarn(error.isEmpty() ? QStringLiteral("Unable to identify project's Porymap version.") : error); + QString msg = QStringLiteral("Failed to check project version"); + logWarn(error.isEmpty() ? msg : QString("%1: '%2'").arg(msg).arg(error)); } else { - logInfo(QString("Successful project version check. Supports at least Porymap v%1.").arg(projectVersion)); + QString msg = QStringLiteral("Successfully checked project version. "); + logInfo(msg + ((projectVersion != 0) ? QString("Supports at least Porymap v%1").arg(projectVersion) + : QStringLiteral("Too old for any Porymap version"))); if (projectVersion < porymapVersion.majorVersion() && projectConfig.forcedMajorVersion < porymapVersion.majorVersion()) { // We were unable to find the necessary changes for Porymap's current major version. - // Warn the user that this might mean their project is missing breaking changes. + // Unless they have explicitly suppressed this message, warn the user that this might mean their project is missing breaking changes. // Note: Do not report 'projectVersion' to the user in this message. We've already logged it for troubleshooting. // It is very plausible that the user may have reproduced the required changes in an // unknown commit, rather than merging the required changes directly from the base repo. // In this case the 'projectVersion' may actually be too old to use for their repo. - ErrorMessage msgBox(QStringLiteral("Your project may be incompatible!"), this); + ErrorMessage msgBox(QStringLiteral("Your project may be incompatible!"), porysplash); msgBox.setInformativeText(QString("Make sure '%1' has all the required changes for Porymap version %2." "") // TODO: Once we have a wiki or manual page describing breaking changes, link that here. .arg(project->getProjectTitle()) @@ -765,13 +776,13 @@ bool MainWindow::isInvalidProject(Project *project) { auto tryAnyway = msgBox.addButton("Try Anyway", QMessageBox::ActionRole); msgBox.exec(); if (msgBox.clickedButton() != tryAnyway){ - return true; + return false; } // User opted to try with this version anyway. Don't warn them about this version again. projectConfig.forcedMajorVersion = porymapVersion.majorVersion(); } } - return false; + return true; } void MainWindow::showProjectOpenFailure() { diff --git a/src/project.cpp b/src/project.cpp index b8183553..b340ed27 100644 --- a/src/project.cpp +++ b/src/project.cpp @@ -79,13 +79,15 @@ bool Project::sanityCheck() { // We can use the project's git history (if it has one, and we're able to get it) to make a reasonable guess. // We know the hashes of the commits in the base repos that contain breaking changes, so if we find one of these then the project // should support at least up to that Porymap major version. If this fails for any reason it returns a version of -1. -// This has relatively tight timeout windows (500ms for each process, compared to the default 30,000ms). This version check -// is not important enough to significantly slow down project launch, we'd rather just timeout. int Project::getSupportedMajorVersion(QString *errorOut) { + // This has relatively tight timeout windows (500ms for each process, compared to the default 30,000ms). This version check + // is not important enough to significantly slow down project launch, we'd rather just timeout. + const int timeoutLimit = 500; const int failureVersion = -1; - QString gitPath = QStandardPaths::findExecutable("git"); + QString gitName = "git"; + QString gitPath = QStandardPaths::findExecutable(gitName); if (gitPath.isEmpty()) { - if (errorOut) *errorOut = QStringLiteral("Failed to identify project history: Unable to locate git."); + if (errorOut) *errorOut = QString("Unable to locate %1.").arg(gitName); return failureVersion; } @@ -95,14 +97,14 @@ int Project::getSupportedMajorVersion(QString *errorOut) { process.setReadChannel(QProcess::StandardOutput); process.setStandardInputFile(QProcess::nullDevice()); // We won't have any writing to do. - // First we need to know which (if any) known git history this project belongs to. + // First we need to know which (if any) known history this project belongs to. // We'll get the root commit, then compare it to the known root commits for the base project repos. static const QStringList args_getRootCommit = { "rev-list", "--max-parents=0", "HEAD" }; process.setArguments(args_getRootCommit); process.start(); - if (!process.waitForFinished(500) || process.exitStatus() != QProcess::ExitStatus::NormalExit || process.exitCode() != 0) { + if (!process.waitForFinished(timeoutLimit) || process.exitStatus() != QProcess::ExitStatus::NormalExit || process.exitCode() != 0) { if (errorOut) { - *errorOut = QStringLiteral("Failed to identify project history"); + *errorOut = QStringLiteral("Failed to identify commit history"); if (process.error() != QProcess::UnknownError && !process.errorString().isEmpty()) { errorOut->append(QString(": %1").arg(process.errorString())); } else { @@ -145,8 +147,8 @@ int Project::getSupportedMajorVersion(QString *errorOut) { }}, }; if (!historyMap.contains(rootCommit)) { - // Either this repo does not share history with one of the base repos, - // (that's ok, don't report an error) or we got some unexpected result. + // Either this repo does not share history with one of the base repos, or we got some unexpected result. + if (errorOut) *errorOut = QStringLiteral("Unrecognized commit history"); return failureVersion; } @@ -162,9 +164,9 @@ int Project::getSupportedMajorVersion(QString *errorOut) { } process.setArguments({ "merge-base", "--is-ancestor", commitHash, "HEAD" }); process.start(); - if (!process.waitForFinished(500) || process.exitStatus() != QProcess::ExitStatus::NormalExit) { + if (!process.waitForFinished(timeoutLimit) || process.exitStatus() != QProcess::ExitStatus::NormalExit) { if (errorOut) { - *errorOut = QStringLiteral("Failed to identify project's supported Porymap version"); + *errorOut = QStringLiteral("Failed to search commit history"); if (process.error() != QProcess::UnknownError && !process.errorString().isEmpty()) { errorOut->append(QString(": %1").arg(process.errorString())); } else { @@ -180,7 +182,8 @@ int Project::getSupportedMajorVersion(QString *errorOut) { return versionNum; } } - return failureVersion; + // We recognized the commit history, but it's too old for any version of Porymap to support. + return 0; } bool Project::load() {