From 808c78310965503d83be6303bdcd2edb0800567a Mon Sep 17 00:00:00 2001 From: AMZN-Phil Date: Tue, 16 Nov 2021 14:24:02 -0800 Subject: [PATCH 1/4] Show python errors in Project Manager for adding repos and downloading gems Signed-off-by: AMZN-Phil --- .../ProjectManager/Source/DownloadWorker.cpp | 2 +- .../Source/GemRepo/GemRepoScreen.cpp | 8 ++-- .../ProjectManager/Source/PythonBindings.cpp | 24 ++++++++++-- .../ProjectManager/Source/PythonBindings.h | 2 +- .../Source/PythonBindingsInterface.h | 4 +- scripts/o3de/o3de/register.py | 2 +- scripts/o3de/o3de/repo.py | 4 +- scripts/o3de/o3de/utils.py | 38 +++++++++++-------- 8 files changed, 55 insertions(+), 29 deletions(-) diff --git a/Code/Tools/ProjectManager/Source/DownloadWorker.cpp b/Code/Tools/ProjectManager/Source/DownloadWorker.cpp index 560bfe05de..163e2c5869 100644 --- a/Code/Tools/ProjectManager/Source/DownloadWorker.cpp +++ b/Code/Tools/ProjectManager/Source/DownloadWorker.cpp @@ -33,7 +33,7 @@ namespace O3DE::ProjectManager } else { - emit Done(tr("Gem download failed")); + emit Done(gemInfoResult.GetError().c_str()); } } diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp index 91432c2346..057976921e 100644 --- a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp @@ -92,8 +92,8 @@ namespace O3DE::ProjectManager return; } - bool addGemRepoResult = PythonBindingsInterface::Get()->AddGemRepo(repoUri); - if (addGemRepoResult) + AZ::Outcome addGemRepoResult = PythonBindingsInterface::Get()->AddGemRepo(repoUri); + if (addGemRepoResult.IsSuccess()) { Reinit(); emit OnRefresh(); @@ -101,8 +101,8 @@ namespace O3DE::ProjectManager else { QString failureMessage = tr("Failed to add gem repo: %1.").arg(repoUri); - QMessageBox::critical(this, tr("Operation failed"), failureMessage); - AZ_Error("Project Manger", false, failureMessage.toUtf8()); + QMessageBox::critical(this, failureMessage, addGemRepoResult.GetError().c_str()); + AZ_Error("Project Manager", false, failureMessage.toUtf8()); } } } diff --git a/Code/Tools/ProjectManager/Source/PythonBindings.cpp b/Code/Tools/ProjectManager/Source/PythonBindings.cpp index d1dedaaec4..c30e9ebd28 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindings.cpp +++ b/Code/Tools/ProjectManager/Source/PythonBindings.cpp @@ -61,6 +61,7 @@ namespace Platform namespace RedirectOutput { using RedirectOutputFunc = AZStd::function; + AZStd::string lastPythonError; struct RedirectOutput { @@ -210,6 +211,16 @@ namespace RedirectOutput }); SetRedirection("stderr", g_redirect_stderr_saved, g_redirect_stderr, []([[maybe_unused]] const char* msg) { + if (lastPythonError.empty()) + { + lastPythonError = msg; + const int lengthOfErrorPrefix = 11; + auto errorPrefix = lastPythonError.find("ERROR:root:"); + if (errorPrefix != AZStd::string::npos) + { + lastPythonError.erase(errorPrefix, lengthOfErrorPrefix); + } + } AZ_TracePrintf("Python", msg); }); @@ -1051,12 +1062,13 @@ namespace O3DE::ProjectManager return result && refreshResult; } - bool PythonBindings::AddGemRepo(const QString& repoUri) + AZ::Outcome PythonBindings::AddGemRepo(const QString& repoUri) { bool registrationResult = false; bool result = ExecuteWithLock( [&] { + RedirectOutput::lastPythonError.clear(); auto pyUri = QString_To_Py_String(repoUri); auto pythonRegistrationResult = m_register.attr("register")( pybind11::none(), pybind11::none(), pybind11::none(), pybind11::none(), pybind11::none(), pybind11::none(), pyUri); @@ -1065,7 +1077,12 @@ namespace O3DE::ProjectManager registrationResult = !pythonRegistrationResult.cast(); }); - return result && registrationResult; + if (!result || !registrationResult) + { + return AZ::Failure(AZStd::move(RedirectOutput::lastPythonError)); + } + + return AZ::Success(); } bool PythonBindings::RemoveGemRepo(const QString& repoUri) @@ -1225,6 +1242,7 @@ namespace O3DE::ProjectManager auto result = ExecuteWithLockErrorHandling( [&] { + RedirectOutput::lastPythonError.clear(); auto downloadResult = m_download.attr("download_gem")( QString_To_Py_String(gemName), // gem name pybind11::none(), // destination path @@ -1248,7 +1266,7 @@ namespace O3DE::ProjectManager } else if (!downloadSucceeded) { - return AZ::Failure("Failed to download gem."); + return AZ::Failure(AZStd::move(RedirectOutput::lastPythonError)); } return AZ::Success(); diff --git a/Code/Tools/ProjectManager/Source/PythonBindings.h b/Code/Tools/ProjectManager/Source/PythonBindings.h index ecc6f65dc3..9f5f06110e 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindings.h +++ b/Code/Tools/ProjectManager/Source/PythonBindings.h @@ -62,7 +62,7 @@ namespace O3DE::ProjectManager // Gem Repos AZ::Outcome RefreshGemRepo(const QString& repoUri) override; bool RefreshAllGemRepos() override; - bool AddGemRepo(const QString& repoUri) override; + AZ::Outcome AddGemRepo(const QString& repoUri) override; bool RemoveGemRepo(const QString& repoUri) override; AZ::Outcome, AZStd::string> GetAllGemRepoInfos() override; AZ::Outcome, AZStd::string> GetAllGemRepoGemsInfos() override; diff --git a/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h b/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h index 65337869fd..26a487f307 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h +++ b/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h @@ -200,9 +200,9 @@ namespace O3DE::ProjectManager /** * Registers this gem repo with the current engine. * @param repoUri the absolute filesystem path or url to the gem repo. - * @return true on success, false on failure. + * @return an outcome with a string error message on failure. */ - virtual bool AddGemRepo(const QString& repoUri) = 0; + virtual AZ::Outcome AddGemRepo(const QString& repoUri) = 0; /** * Unregisters this gem repo with the current engine. diff --git a/scripts/o3de/o3de/register.py b/scripts/o3de/o3de/register.py index 8a2bb788aa..8420e202dc 100644 --- a/scripts/o3de/o3de/register.py +++ b/scripts/o3de/o3de/register.py @@ -490,7 +490,7 @@ def register_repo(json_data: dict, repo_sha256 = hashlib.sha256(url.encode()) cache_file = manifest.get_o3de_cache_folder() / str(repo_sha256.hexdigest() + '.json') - result = utils.download_file(parsed_uri, cache_file) + result = utils.download_file(parsed_uri, cache_file, True) if result == 0: json_data.setdefault('repos', []).insert(0, repo_uri) diff --git a/scripts/o3de/o3de/repo.py b/scripts/o3de/o3de/repo.py index c8fac38605..3ebb79fb27 100644 --- a/scripts/o3de/o3de/repo.py +++ b/scripts/o3de/o3de/repo.py @@ -24,6 +24,7 @@ def process_add_o3de_repo(file_name: str or pathlib.Path, repo_set: set) -> int: file_name = pathlib.Path(file_name).resolve() if not validation.valid_o3de_repo_json(file_name): + logger.error(f'Repository JSON {file_name} could not be loaded or is missing required values') return 1 cache_folder = manifest.get_o3de_cache_folder() @@ -114,7 +115,7 @@ def get_gem_json_paths_from_cached_repo(repo_uri: str) -> set: file_name = pathlib.Path(cache_filename).resolve() if not file_name.is_file(): - logger.error(f'Could not find cached repo json file for {repo_uri}') + logger.error(f'Could not find cached repository json file for {repo_uri}. Try refreshing the repository.') return gem_set with file_name.open('r') as f: @@ -167,6 +168,7 @@ def refresh_repo(repo_uri: str, download_file_result = utils.download_file(parsed_uri, cache_file, True) if download_file_result != 0: + logger.error(f'Repo json {repo_uri} could not download.') return download_file_result if not validation.valid_o3de_repo_json(cache_file): diff --git a/scripts/o3de/o3de/utils.py b/scripts/o3de/o3de/utils.py index 88f84ae75e..7b7d0e3e27 100644 --- a/scripts/o3de/o3de/utils.py +++ b/scripts/o3de/o3de/utils.py @@ -125,7 +125,8 @@ def download_file(parsed_uri, download_path: pathlib.Path, force_overwrite: bool """ if download_path.is_file(): if not force_overwrite: - logger.warn(f'File already downloaded to {download_path}.') + logger.error(f'File already downloaded to {download_path} and force_overwrite is not set.') + return 1 else: try: os.unlink(download_path) @@ -134,20 +135,25 @@ def download_file(parsed_uri, download_path: pathlib.Path, force_overwrite: bool return 1 if parsed_uri.scheme in ['http', 'https', 'ftp', 'ftps']: - with urllib.request.urlopen(parsed_uri.geturl()) as s: - download_file_size = 0 - try: - download_file_size = s.headers['content-length'] - except KeyError: - pass - def download_progress(downloaded_bytes): - if download_progress_callback: - return download_progress_callback(int(downloaded_bytes), int(download_file_size)) - return False - with download_path.open('wb') as f: - download_cancelled = copyfileobj(s, f, download_progress) - if download_cancelled: - return 1 + try: + with urllib.request.urlopen(parsed_uri.geturl()) as s: + download_file_size = 0 + try: + download_file_size = s.headers['content-length'] + except KeyError: + pass + def download_progress(downloaded_bytes): + if download_progress_callback: + return download_progress_callback(int(downloaded_bytes), int(download_file_size)) + return False + with download_path.open('wb') as f: + download_cancelled = copyfileobj(s, f, download_progress) + if download_cancelled: + logger.warn(f'Download of file to {download_path} cancelled.') + return 1 + except urllib.error.HTTPError as e: + logger.error(f'HTTP Error {e.code} opening {parsed_uri.geturl()}') + return 1 else: origin_file = pathlib.Path(parsed_uri.geturl()).resolve() if not origin_file.is_file(): @@ -167,7 +173,7 @@ def download_zip_file(parsed_uri, download_zip_path: pathlib.Path, force_overwri return download_file_result if not zipfile.is_zipfile(download_zip_path): - logger.error(f"File zip {download_zip_path} is invalid.") + logger.error(f"File zip {download_zip_path} is invalid. Try re-downloading the file.") download_zip_path.unlink() return 1 From ba5102b37d7835e531a4b9198a7d2447ef244718 Mon Sep 17 00:00:00 2001 From: AMZN-Phil Date: Tue, 16 Nov 2021 17:40:12 -0800 Subject: [PATCH 2/4] Change to optionally show all python error strings Signed-off-by: AMZN-Phil --- .../Source/DownloadController.cpp | 16 ++++++- .../Source/DownloadController.h | 2 +- .../ProjectManager/Source/DownloadWorker.cpp | 6 +-- .../ProjectManager/Source/DownloadWorker.h | 2 +- .../Source/GemRepo/GemRepoScreen.cpp | 18 +++++++- .../ProjectManager/Source/PythonBindings.cpp | 42 ++++++++++++------- .../ProjectManager/Source/PythonBindings.h | 6 ++- .../Source/PythonBindingsInterface.h | 8 ++-- 8 files changed, 69 insertions(+), 31 deletions(-) diff --git a/Code/Tools/ProjectManager/Source/DownloadController.cpp b/Code/Tools/ProjectManager/Source/DownloadController.cpp index 06e51b5116..9b64c5e276 100644 --- a/Code/Tools/ProjectManager/Source/DownloadController.cpp +++ b/Code/Tools/ProjectManager/Source/DownloadController.cpp @@ -73,13 +73,25 @@ namespace O3DE::ProjectManager emit GemDownloadProgress(m_gemNames.front(), bytesDownloaded, totalBytes); } - void DownloadController::HandleResults(const QString& result) + void DownloadController::HandleResults(const QString& result, const QString& detailedError) { bool succeeded = true; if (!result.isEmpty()) { - QMessageBox::critical(nullptr, tr("Gem download"), result); + if (!detailedError.isEmpty()) + { + QMessageBox gemDownloadError; + gemDownloadError.setIcon(QMessageBox::Critical); + gemDownloadError.setWindowTitle(tr("Gem download")); + gemDownloadError.setText(result); + gemDownloadError.setDetailedText(detailedError); + gemDownloadError.exec(); + } + else + { + QMessageBox::critical(nullptr, tr("Gem download"), result); + } succeeded = false; } diff --git a/Code/Tools/ProjectManager/Source/DownloadController.h b/Code/Tools/ProjectManager/Source/DownloadController.h index 211d9b48bc..5e637971e9 100644 --- a/Code/Tools/ProjectManager/Source/DownloadController.h +++ b/Code/Tools/ProjectManager/Source/DownloadController.h @@ -54,7 +54,7 @@ namespace O3DE::ProjectManager } public slots: void UpdateUIProgress(int bytesDownloaded, int totalBytes); - void HandleResults(const QString& result); + void HandleResults(const QString& result, const QString& detailedError); signals: void StartGemDownload(const QString& gemName); diff --git a/Code/Tools/ProjectManager/Source/DownloadWorker.cpp b/Code/Tools/ProjectManager/Source/DownloadWorker.cpp index 163e2c5869..e58c41c89e 100644 --- a/Code/Tools/ProjectManager/Source/DownloadWorker.cpp +++ b/Code/Tools/ProjectManager/Source/DownloadWorker.cpp @@ -24,16 +24,16 @@ namespace O3DE::ProjectManager { emit UpdateProgress(bytesDownloaded, totalBytes); }; - AZ::Outcome gemInfoResult = + AZ::Outcome> gemInfoResult = PythonBindingsInterface::Get()->DownloadGem(m_gemName, gemDownloadProgress, /*force*/true); if (gemInfoResult.IsSuccess()) { - emit Done(""); + emit Done("", ""); } else { - emit Done(gemInfoResult.GetError().c_str()); + emit Done(gemInfoResult.GetError().first.c_str(), gemInfoResult.GetError().second.c_str()); } } diff --git a/Code/Tools/ProjectManager/Source/DownloadWorker.h b/Code/Tools/ProjectManager/Source/DownloadWorker.h index 4084080ff7..d33de7bacc 100644 --- a/Code/Tools/ProjectManager/Source/DownloadWorker.h +++ b/Code/Tools/ProjectManager/Source/DownloadWorker.h @@ -32,7 +32,7 @@ namespace O3DE::ProjectManager signals: void UpdateProgress(int bytesDownloaded, int totalBytes); - void Done(QString result = ""); + void Done(QString result = "", QString detailedResult = ""); private: diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp index 057976921e..67fdfaa790 100644 --- a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp @@ -92,7 +92,8 @@ namespace O3DE::ProjectManager return; } - AZ::Outcome addGemRepoResult = PythonBindingsInterface::Get()->AddGemRepo(repoUri); + AZ::Outcome < void, + AZStd::pair> addGemRepoResult = PythonBindingsInterface::Get()->AddGemRepo(repoUri); if (addGemRepoResult.IsSuccess()) { Reinit(); @@ -101,7 +102,20 @@ namespace O3DE::ProjectManager else { QString failureMessage = tr("Failed to add gem repo: %1.").arg(repoUri); - QMessageBox::critical(this, failureMessage, addGemRepoResult.GetError().c_str()); + if (!addGemRepoResult.GetError().second.empty()) + { + QMessageBox gemDownloadError; + gemDownloadError.setIcon(QMessageBox::Critical); + gemDownloadError.setWindowTitle(failureMessage); + gemDownloadError.setText(addGemRepoResult.GetError().first.c_str()); + gemDownloadError.setDetailedText(addGemRepoResult.GetError().second.c_str()); + gemDownloadError.exec(); + } + else + { + QMessageBox::critical(this, failureMessage, addGemRepoResult.GetError().first.c_str()); + } + AZ_Error("Project Manager", false, failureMessage.toUtf8()); } } diff --git a/Code/Tools/ProjectManager/Source/PythonBindings.cpp b/Code/Tools/ProjectManager/Source/PythonBindings.cpp index c30e9ebd28..5f80705e97 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindings.cpp +++ b/Code/Tools/ProjectManager/Source/PythonBindings.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -61,7 +62,7 @@ namespace Platform namespace RedirectOutput { using RedirectOutputFunc = AZStd::function; - AZStd::string lastPythonError; + AZStd::vector pythonErrorStrings; struct RedirectOutput { @@ -211,16 +212,16 @@ namespace RedirectOutput }); SetRedirection("stderr", g_redirect_stderr_saved, g_redirect_stderr, []([[maybe_unused]] const char* msg) { - if (lastPythonError.empty()) + AZStd::string lastPythonError = msg; + constexpr const char* pythonErrorPrefix = "ERROR:root:"; + constexpr size_t lengthOfErrorPrefix = AZStd::char_traits::length(pythonErrorPrefix); + auto errorPrefix = lastPythonError.find(pythonErrorPrefix); + if (errorPrefix != AZStd::string::npos) { - lastPythonError = msg; - const int lengthOfErrorPrefix = 11; - auto errorPrefix = lastPythonError.find("ERROR:root:"); - if (errorPrefix != AZStd::string::npos) - { - lastPythonError.erase(errorPrefix, lengthOfErrorPrefix); - } + lastPythonError.erase(errorPrefix, lengthOfErrorPrefix); } + pythonErrorStrings.push_back(lastPythonError); + AZ_TracePrintf("Python", msg); }); @@ -387,6 +388,8 @@ namespace O3DE::ProjectManager pybind11::gil_scoped_release release; pybind11::gil_scoped_acquire acquire; + RedirectOutput::pythonErrorStrings.clear(); + try { executionCallback(); @@ -1062,13 +1065,12 @@ namespace O3DE::ProjectManager return result && refreshResult; } - AZ::Outcome PythonBindings::AddGemRepo(const QString& repoUri) + AZ::Outcome> PythonBindings::AddGemRepo(const QString& repoUri) { bool registrationResult = false; bool result = ExecuteWithLock( [&] { - RedirectOutput::lastPythonError.clear(); auto pyUri = QString_To_Py_String(repoUri); auto pythonRegistrationResult = m_register.attr("register")( pybind11::none(), pybind11::none(), pybind11::none(), pybind11::none(), pybind11::none(), pybind11::none(), pyUri); @@ -1079,7 +1081,7 @@ namespace O3DE::ProjectManager if (!result || !registrationResult) { - return AZ::Failure(AZStd::move(RedirectOutput::lastPythonError)); + return AZ::Failure>(GetSimpleDetailedErrorPair()); } return AZ::Success(); @@ -1232,7 +1234,7 @@ namespace O3DE::ProjectManager return AZ::Success(AZStd::move(gemInfos)); } - AZ::Outcome PythonBindings::DownloadGem( + AZ::Outcome> PythonBindings::DownloadGem( const QString& gemName, std::function gemProgressCallback, bool force) { // This process is currently limited to download a single gem at a time. @@ -1242,7 +1244,6 @@ namespace O3DE::ProjectManager auto result = ExecuteWithLockErrorHandling( [&] { - RedirectOutput::lastPythonError.clear(); auto downloadResult = m_download.attr("download_gem")( QString_To_Py_String(gemName), // gem name pybind11::none(), // destination path @@ -1262,11 +1263,12 @@ namespace O3DE::ProjectManager if (!result.IsSuccess()) { - return result; + AZStd::pair pythonRunError(result.GetError(), result.GetError()); + return AZ::Failure>(AZStd::move(pythonRunError)); } else if (!downloadSucceeded) { - return AZ::Failure(AZStd::move(RedirectOutput::lastPythonError)); + return AZ::Failure>(GetSimpleDetailedErrorPair()); } return AZ::Success(); @@ -1292,4 +1294,12 @@ namespace O3DE::ProjectManager return result && updateAvaliableResult; } + + AZStd::pair PythonBindings::GetSimpleDetailedErrorPair() + { + AZStd::string detailedString = RedirectOutput::pythonErrorStrings.size() == 1 ? "" : AZStd::accumulate( + RedirectOutput::pythonErrorStrings.begin(), RedirectOutput::pythonErrorStrings.end(), AZStd::string("")); + + return AZStd::pair(RedirectOutput::pythonErrorStrings.front(), detailedString); + } } diff --git a/Code/Tools/ProjectManager/Source/PythonBindings.h b/Code/Tools/ProjectManager/Source/PythonBindings.h index 9f5f06110e..fd36d6d13e 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindings.h +++ b/Code/Tools/ProjectManager/Source/PythonBindings.h @@ -62,11 +62,12 @@ namespace O3DE::ProjectManager // Gem Repos AZ::Outcome RefreshGemRepo(const QString& repoUri) override; bool RefreshAllGemRepos() override; - AZ::Outcome AddGemRepo(const QString& repoUri) override; + AZ::Outcome> AddGemRepo(const QString& repoUri) override; bool RemoveGemRepo(const QString& repoUri) override; AZ::Outcome, AZStd::string> GetAllGemRepoInfos() override; AZ::Outcome, AZStd::string> GetAllGemRepoGemsInfos() override; - AZ::Outcome DownloadGem(const QString& gemName, std::function gemProgressCallback, bool force = false) override; + AZ::Outcome> DownloadGem( + const QString& gemName, std::function gemProgressCallback, bool force = false) override; void CancelDownload() override; bool IsGemUpdateAvaliable(const QString& gemName, const QString& lastUpdated) override; @@ -82,6 +83,7 @@ namespace O3DE::ProjectManager AZ::Outcome GemRegistration(const QString& gemPath, const QString& projectPath, bool remove = false); bool RegisterThisEngine(); bool StopPython(); + AZStd::pair GetSimpleDetailedErrorPair(); bool m_pythonStarted = false; diff --git a/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h b/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h index 26a487f307..45fb8928f9 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h +++ b/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h @@ -200,9 +200,9 @@ namespace O3DE::ProjectManager /** * Registers this gem repo with the current engine. * @param repoUri the absolute filesystem path or url to the gem repo. - * @return an outcome with a string error message on failure. + * @return an outcome with a pair of string error and detailed messages on failure. */ - virtual AZ::Outcome AddGemRepo(const QString& repoUri) = 0; + virtual AZ::Outcome> AddGemRepo(const QString& repoUri) = 0; /** * Unregisters this gem repo with the current engine. @@ -228,9 +228,9 @@ namespace O3DE::ProjectManager * @param gemName the name of the Gem to download. * @param gemProgressCallback a callback function that is called with an int percentage download value. * @param force should we forcibly overwrite the old version of the gem. - * @return an outcome with a string error message on failure. + * @return an outcome with a pair of string error and detailed messages on failure. */ - virtual AZ::Outcome DownloadGem( + virtual AZ::Outcome> DownloadGem( const QString& gemName, std::function gemProgressCallback, bool force = false) = 0; /** From a7e90fd6c85a5a3423dbb7b7b194209a519ab3e2 Mon Sep 17 00:00:00 2001 From: AMZN-Phil Date: Wed, 17 Nov 2021 08:15:41 -0800 Subject: [PATCH 3/4] Fix copy paste variable name Signed-off-by: AMZN-Phil --- .../ProjectManager/Source/GemRepo/GemRepoScreen.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp index 67fdfaa790..f62c30c280 100644 --- a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp @@ -104,12 +104,12 @@ namespace O3DE::ProjectManager QString failureMessage = tr("Failed to add gem repo: %1.").arg(repoUri); if (!addGemRepoResult.GetError().second.empty()) { - QMessageBox gemDownloadError; - gemDownloadError.setIcon(QMessageBox::Critical); - gemDownloadError.setWindowTitle(failureMessage); - gemDownloadError.setText(addGemRepoResult.GetError().first.c_str()); - gemDownloadError.setDetailedText(addGemRepoResult.GetError().second.c_str()); - gemDownloadError.exec(); + QMessageBox addRepoError; + addRepoError.setIcon(QMessageBox::Critical); + addRepoError.setWindowTitle(failureMessage); + addRepoError.setText(addGemRepoResult.GetError().first.c_str()); + addRepoError.setDetailedText(addGemRepoResult.GetError().second.c_str()); + addRepoError.exec(); } else { From e9ec37f20bd43c2fdc6c6b654e99d77fa9c7b3d9 Mon Sep 17 00:00:00 2001 From: AMZN-Phil Date: Wed, 17 Nov 2021 14:03:24 -0800 Subject: [PATCH 4/4] Moved global vector inside PythonBindings Signed-off-by: AMZN-Phil --- .../ProjectManager/Source/PythonBindings.cpp | 22 ++++++++++++++----- .../ProjectManager/Source/PythonBindings.h | 4 ++++ .../Source/PythonBindingsInterface.h | 11 ++++++++++ 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/Code/Tools/ProjectManager/Source/PythonBindings.cpp b/Code/Tools/ProjectManager/Source/PythonBindings.cpp index 9023d04331..0477cb726c 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindings.cpp +++ b/Code/Tools/ProjectManager/Source/PythonBindings.cpp @@ -62,7 +62,6 @@ namespace Platform namespace RedirectOutput { using RedirectOutputFunc = AZStd::function; - AZStd::vector pythonErrorStrings; struct RedirectOutput { @@ -220,7 +219,7 @@ namespace RedirectOutput { lastPythonError.erase(errorPrefix, lengthOfErrorPrefix); } - pythonErrorStrings.push_back(lastPythonError); + O3DE::ProjectManager::PythonBindingsInterface::Get()->AddErrorString(lastPythonError); AZ_TracePrintf("Python", msg); }); @@ -388,7 +387,7 @@ namespace O3DE::ProjectManager pybind11::gil_scoped_release release; pybind11::gil_scoped_acquire acquire; - RedirectOutput::pythonErrorStrings.clear(); + ClearErrorStrings(); try { @@ -1325,9 +1324,20 @@ namespace O3DE::ProjectManager AZStd::pair PythonBindings::GetSimpleDetailedErrorPair() { - AZStd::string detailedString = RedirectOutput::pythonErrorStrings.size() == 1 ? "" : AZStd::accumulate( - RedirectOutput::pythonErrorStrings.begin(), RedirectOutput::pythonErrorStrings.end(), AZStd::string("")); + AZStd::string detailedString = m_pythonErrorStrings.size() == 1 + ? "" + : AZStd::accumulate(m_pythonErrorStrings.begin(), m_pythonErrorStrings.end(), AZStd::string("")); - return AZStd::pair(RedirectOutput::pythonErrorStrings.front(), detailedString); + return AZStd::pair(m_pythonErrorStrings.front(), detailedString); + } + + void PythonBindings::AddErrorString(AZStd::string errorString) + { + m_pythonErrorStrings.push_back(errorString); + } + + void PythonBindings::ClearErrorStrings() + { + m_pythonErrorStrings.clear(); } } diff --git a/Code/Tools/ProjectManager/Source/PythonBindings.h b/Code/Tools/ProjectManager/Source/PythonBindings.h index 9ac040a428..6bef459acf 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindings.h +++ b/Code/Tools/ProjectManager/Source/PythonBindings.h @@ -72,6 +72,9 @@ namespace O3DE::ProjectManager void CancelDownload() override; bool IsGemUpdateAvaliable(const QString& gemName, const QString& lastUpdated) override; + void AddErrorString(AZStd::string errorString) override; + void ClearErrorStrings() override; + private: AZ_DISABLE_COPY_MOVE(PythonBindings); @@ -104,5 +107,6 @@ namespace O3DE::ProjectManager pybind11::handle m_pathlib; bool m_requestCancelDownload = false; + AZStd::vector m_pythonErrorStrings; }; } diff --git a/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h b/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h index 9dc1078d6c..25eadd3f69 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h +++ b/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h @@ -252,6 +252,17 @@ namespace O3DE::ProjectManager * @return true if update is avaliable, false if not. */ virtual bool IsGemUpdateAvaliable(const QString& gemName, const QString& lastUpdated) = 0; + + /** + * Add an error string to be returned when the current python call is complete. + * @param The error string to be displayed. + */ + virtual void AddErrorString(AZStd::string errorString) = 0; + + /** + * Clears the current list of error strings. + */ + virtual void ClearErrorStrings() = 0; }; using PythonBindingsInterface = AZ::Interface;