From 3bca63bb7187d640928505d1662c395afb9c4894 Mon Sep 17 00:00:00 2001 From: Guthrie Adams Date: Sat, 11 Dec 2021 15:50:37 -0600 Subject: [PATCH 1/8] Temporary fix for material component losing image overrides with prefabs The bug reported that overridden texture properties would be lost whenever an entity was created, destroyed, or a prefab was created. Initially, it seemed like there was a problem with the custom JSON serializer for material properties. Debugging proved this to be incorrect because all of the data was converted to JSON values in the serializer on multiple passes. At some point during prefab patching, the data for the asset properties is lost while other values like colors and floats serialize correctly. Converting the asset data values into asset IDs resolves the immediate problem for the material component but the underlying issue is still under investigation by the prefab team. This change is being posted for review in case the underlying issue cannot be resolved in time for the next release. Signed-off-by: Guthrie Adams Fixing unittests and moving texture conversion into material component controller Signed-off-by: Guthrie Adams --- .../Material/MaterialAssignmentSerializer.cpp | 24 ++++++++++---- .../Material/MaterialAssignmentSerializer.h | 18 +++++++--- .../Code/Source/Util/MaterialPropertyUtil.cpp | 12 ++++--- .../Material/MaterialComponentController.cpp | 33 +++++++++++++++++++ .../Material/MaterialComponentController.h | 5 +++ 5 files changed, 76 insertions(+), 16 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignmentSerializer.cpp b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignmentSerializer.cpp index b757e4bd7e..85dc26089f 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignmentSerializer.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignmentSerializer.cpp @@ -16,7 +16,9 @@ namespace AZ AZ_CLASS_ALLOCATOR_IMPL(JsonMaterialAssignmentSerializer, AZ::SystemAllocator, 0); JsonSerializationResult::Result JsonMaterialAssignmentSerializer::Load( - void* outputValue, [[maybe_unused]] const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, + void* outputValue, + [[maybe_unused]] const Uuid& outputValueTypeId, + const rapidjson::Value& inputValue, JsonDeserializerContext& context) { namespace JSR = JsonSerializationResult; @@ -62,6 +64,7 @@ namespace AZ LoadAny(propertyValue, inputPropertyPair.value, context, result) || LoadAny(propertyValue, inputPropertyPair.value, context, result) || LoadAny(propertyValue, inputPropertyPair.value, context, result) || + LoadAny>(propertyValue, inputPropertyPair.value, context, result) || LoadAny>(propertyValue, inputPropertyPair.value, context, result) || LoadAny>(propertyValue, inputPropertyPair.value, context, result)) { @@ -78,7 +81,10 @@ namespace AZ } JsonSerializationResult::Result JsonMaterialAssignmentSerializer::Store( - rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue, [[maybe_unused]] const Uuid& valueTypeId, + rapidjson::Value& outputValue, + const void* inputValue, + const void* defaultValue, + [[maybe_unused]] const Uuid& valueTypeId, JsonSerializerContext& context) { namespace JSR = AZ::JsonSerializationResult; @@ -138,9 +144,9 @@ namespace AZ StoreAny(propertyValue, outputPropertyValue, context, result) || StoreAny(propertyValue, outputPropertyValue, context, result) || StoreAny(propertyValue, outputPropertyValue, context, result) || + StoreAny>(propertyValue, outputPropertyValue, context, result) || StoreAny>(propertyValue, outputPropertyValue, context, result) || - StoreAny>( - propertyValue, outputPropertyValue, context, result)) + StoreAny>(propertyValue, outputPropertyValue, context, result)) { outputPropertyValueContainer.AddMember( rapidjson::Value::StringRefType(propertyName.GetCStr()), outputPropertyValue, @@ -164,7 +170,9 @@ namespace AZ template bool JsonMaterialAssignmentSerializer::LoadAny( - AZStd::any& propertyValue, const rapidjson::Value& inputPropertyValue, AZ::JsonDeserializerContext& context, + AZStd::any& propertyValue, + const rapidjson::Value& inputPropertyValue, + AZ::JsonDeserializerContext& context, AZ::JsonSerializationResult::ResultCode& result) { if (inputPropertyValue.IsObject() && inputPropertyValue.HasMember("Value") && inputPropertyValue.HasMember("$type")) @@ -187,7 +195,9 @@ namespace AZ template bool JsonMaterialAssignmentSerializer::StoreAny( - const AZStd::any& propertyValue, rapidjson::Value& outputPropertyValue, AZ::JsonSerializerContext& context, + const AZStd::any& propertyValue, + rapidjson::Value& outputPropertyValue, + AZ::JsonSerializerContext& context, AZ::JsonSerializationResult::ResultCode& result) { if (propertyValue.is()) @@ -199,7 +209,7 @@ namespace AZ result.Combine(StoreTypeId(typeValue, azrtti_typeid(), context)); outputPropertyValue.AddMember("$type", typeValue, context.GetJsonAllocator()); - T value = AZStd::any_cast(propertyValue); + const T& value = AZStd::any_cast(propertyValue); result.Combine( ContinueStoringToJsonObjectField(outputPropertyValue, "Value", &value, nullptr, azrtti_typeid(), context)); return true; diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignmentSerializer.h b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignmentSerializer.h index e92d756639..069b4d4cdb 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignmentSerializer.h +++ b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignmentSerializer.h @@ -25,21 +25,31 @@ namespace AZ AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load( - void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, + void* outputValue, + const Uuid& outputValueTypeId, + const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; JsonSerializationResult::Result Store( - rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue, const Uuid& valueTypeId, + rapidjson::Value& outputValue, + const void* inputValue, + const void* defaultValue, + const Uuid& valueTypeId, JsonSerializerContext& context) override; private: template bool LoadAny( - AZStd::any& propertyValue, const rapidjson::Value& inputPropertyValue, AZ::JsonDeserializerContext& context, + AZStd::any& propertyValue, + const rapidjson::Value& inputPropertyValue, + AZ::JsonDeserializerContext& context, AZ::JsonSerializationResult::ResultCode& result); + template bool StoreAny( - const AZStd::any& propertyValue, rapidjson::Value& outputPropertyValue, AZ::JsonSerializerContext& context, + const AZStd::any& propertyValue, + rapidjson::Value& outputPropertyValue, + AZ::JsonSerializerContext& context, AZ::JsonSerializationResult::ResultCode& result); }; } // namespace Render diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Util/MaterialPropertyUtil.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Util/MaterialPropertyUtil.cpp index 3ffd8efa6e..2ad4522094 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Util/MaterialPropertyUtil.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Util/MaterialPropertyUtil.cpp @@ -33,11 +33,13 @@ namespace AtomToolsFramework { if (value.Is>()) { - const AZ::Data::Asset& imageAsset = value.GetValue>(); - return AZStd::any(AZ::Data::Asset( - imageAsset.GetId(), - azrtti_typeid(), - imageAsset.GetHint())); + const auto& imageAsset = value.GetValue>(); + return AZStd::any(AZ::Data::Asset(imageAsset.GetId(), azrtti_typeid(), imageAsset.GetHint())); + } + else if (value.Is>()) + { + const auto& image = value.GetValue>(); + return AZStd::any(AZ::Data::Asset(image->GetAssetId(), azrtti_typeid())); } return AZ::RPI::MaterialPropertyValue::ToAny(value); diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp index 0ccdae28de..996a575c69 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp @@ -104,6 +104,7 @@ namespace AZ MaterialComponentController::MaterialComponentController(const MaterialComponentConfig& config) : m_configuration(config) { + ConvertAssetsForSerialization(); } void MaterialComponentController::Activate(EntityId entityId) @@ -135,6 +136,7 @@ namespace AZ void MaterialComponentController::SetConfiguration(const MaterialComponentConfig& config) { m_configuration = config; + ConvertAssetsForSerialization(); } const MaterialComponentConfig& MaterialComponentController::GetConfiguration() const @@ -338,6 +340,7 @@ namespace AZ // before LoadMaterials() is called [LYN-2249] auto temp = m_configuration.m_materials; m_configuration.m_materials = materials; + ConvertAssetsForSerialization(); LoadMaterials(); } @@ -489,6 +492,7 @@ namespace AZ auto& materialAssignment = m_configuration.m_materials[materialAssignmentId]; const bool wasEmpty = materialAssignment.m_propertyOverrides.empty(); materialAssignment.m_propertyOverrides[AZ::Name(propertyName)] = value; + ConvertAssetsForSerialization(); if (materialAssignment.RequiresLoading()) { @@ -586,6 +590,7 @@ namespace AZ auto& materialAssignment = m_configuration.m_materials[materialAssignmentId]; const bool wasEmpty = materialAssignment.m_propertyOverrides.empty(); materialAssignment.m_propertyOverrides = propertyOverrides; + ConvertAssetsForSerialization(); if (materialAssignment.RequiresLoading()) { @@ -667,5 +672,33 @@ namespace AZ TickBus::Handler::BusConnect(); } } + + void MaterialComponentController::ConvertAssetsForSerialization() + { + for (auto& materialAssignmentPair : m_configuration.m_materials) + { + MaterialAssignment& materialAssignment = materialAssignmentPair.second; + for (auto& propertyPair : materialAssignment.m_propertyOverrides) + { + auto& value = propertyPair.second; + if (value.is>()) + { + value = AZStd::any_cast>(value).GetId(); + } + else if (value.is>()) + { + value = AZStd::any_cast>(value).GetId(); + } + else if (value.is>()) + { + value = AZStd::any_cast>(value).GetId(); + } + else if (value.is>()) + { + value = AZStd::any_cast>(value)->GetAssetId(); + } + } + } + } } // namespace Render } // namespace AZ diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h index 74b1cfda4d..d3eaa4433d 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h @@ -99,6 +99,11 @@ namespace AZ //! Queue material instance recreation notifiucations until tick void QueueMaterialUpdateNotification(); + //! Converts property overrides storing image asset references into asset IDs. This addresses a problem where image property + //! overrides are lost during prefab serialization and patching. This suboptimal function will be removed once the underlying + //! problem is resolved. + void ConvertAssetsForSerialization(); + EntityId m_entityId; MaterialComponentConfig m_configuration; AZStd::unordered_set m_materialsWithDirtyProperties; From c019fe8946269e581683c383966651a81c5f9d38 Mon Sep 17 00:00:00 2001 From: Mike Chang Date: Thu, 2 Dec 2021 17:05:42 -0800 Subject: [PATCH 2/8] Add conditional to pull O3DE_BUILD_VERSION through environment var (#6096) Signed-off-by: Mike Chang --- cmake/Version.cmake | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmake/Version.cmake b/cmake/Version.cmake index de93ebefef..c5504ec62a 100644 --- a/cmake/Version.cmake +++ b/cmake/Version.cmake @@ -16,3 +16,8 @@ if("$ENV{O3DE_VERSION}") # Overriding through environment set(LY_VERSION_STRING "$ENV{O3DE_VERSION}") endif() + +if("$ENV{O3DE_BUILD_VERSION}") + # Overriding through environment + set(LY_VERSION_BUILD_NUMBER "$ENV{O3DE_BUILD_VERSION}") +endif() From f44697fbf45b6288d9f2995cd43b2cef4927b9dc Mon Sep 17 00:00:00 2001 From: AMZN-Phil Date: Tue, 7 Dec 2021 14:05:05 -0800 Subject: [PATCH 3/8] Check whether env variables are defined Signed-off-by: AMZN-Phil --- cmake/Version.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/Version.cmake b/cmake/Version.cmake index c5504ec62a..6fa32e9c73 100644 --- a/cmake/Version.cmake +++ b/cmake/Version.cmake @@ -12,12 +12,12 @@ set(LY_VERSION_STRING "0.0.0.0" CACHE STRING "Open 3D Engine's version") set(LY_VERSION_BUILD_NUMBER 0 CACHE STRING "Open 3D Engine's build number") set(LY_VERSION_ENGINE_NAME "o3de" CACHE STRING "Open 3D Engine's engine name") -if("$ENV{O3DE_VERSION}") +if(DEFINED ENV{O3DE_VERSION}) # Overriding through environment set(LY_VERSION_STRING "$ENV{O3DE_VERSION}") endif() -if("$ENV{O3DE_BUILD_VERSION}") +if(DEFINED ENV{O3DE_BUILD_VERSION}) # Overriding through environment set(LY_VERSION_BUILD_NUMBER "$ENV{O3DE_BUILD_VERSION}") endif() From 38c8d941564ae709a7839a25883807b723fa4657 Mon Sep 17 00:00:00 2001 From: AMZN-Phil Date: Tue, 7 Dec 2021 15:12:11 -0800 Subject: [PATCH 4/8] Use version check to allow for a defined empty string to fail and greater to check build number is a number greater than 0 Signed-off-by: AMZN-Phil --- cmake/Version.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/Version.cmake b/cmake/Version.cmake index 6fa32e9c73..d15d5b9f86 100644 --- a/cmake/Version.cmake +++ b/cmake/Version.cmake @@ -12,12 +12,12 @@ set(LY_VERSION_STRING "0.0.0.0" CACHE STRING "Open 3D Engine's version") set(LY_VERSION_BUILD_NUMBER 0 CACHE STRING "Open 3D Engine's build number") set(LY_VERSION_ENGINE_NAME "o3de" CACHE STRING "Open 3D Engine's engine name") -if(DEFINED ENV{O3DE_VERSION}) +if("$ENV{O3DE_VERSION}" VERSION_GREATER "0.0.0.0") # Overriding through environment set(LY_VERSION_STRING "$ENV{O3DE_VERSION}") endif() -if(DEFINED ENV{O3DE_BUILD_VERSION}) +if("$ENV{O3DE_BUILD_VERSION}" GREATER 0) # Overriding through environment set(LY_VERSION_BUILD_NUMBER "$ENV{O3DE_BUILD_VERSION}") endif() From 82af1e6870ba7f8e705d5f0559591ce42fa57384 Mon Sep 17 00:00:00 2001 From: AMZN-Phil Date: Tue, 7 Dec 2021 17:10:26 -0800 Subject: [PATCH 5/8] Force a string check on the version numbers Signed-off-by: AMZN-Phil --- cmake/Version.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/Version.cmake b/cmake/Version.cmake index d15d5b9f86..876c34f8c4 100644 --- a/cmake/Version.cmake +++ b/cmake/Version.cmake @@ -12,12 +12,12 @@ set(LY_VERSION_STRING "0.0.0.0" CACHE STRING "Open 3D Engine's version") set(LY_VERSION_BUILD_NUMBER 0 CACHE STRING "Open 3D Engine's build number") set(LY_VERSION_ENGINE_NAME "o3de" CACHE STRING "Open 3D Engine's engine name") -if("$ENV{O3DE_VERSION}" VERSION_GREATER "0.0.0.0") +if(NOT "$ENV{O3DE_VERSION}" STREQUAL "") # Overriding through environment set(LY_VERSION_STRING "$ENV{O3DE_VERSION}") endif() -if("$ENV{O3DE_BUILD_VERSION}" GREATER 0) +if(NOT "$ENV{O3DE_BUILD_VERSION}" STREQUAL "") # Overriding through environment set(LY_VERSION_BUILD_NUMBER "$ENV{O3DE_BUILD_VERSION}") endif() From 66f0f1cf5a03c1196ef029f95cac99f58970d471 Mon Sep 17 00:00:00 2001 From: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com> Date: Wed, 19 Jan 2022 17:55:05 -0800 Subject: [PATCH 6/8] Duplicate engine detection and help in Project Manager (#6984) Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com> --- .../ProjectManager/Source/Application.cpp | 86 ++++++++ .../Tools/ProjectManager/Source/Application.h | 1 + Code/Tools/ProjectManager/Source/EngineInfo.h | 5 +- .../Source/EngineSettingsScreen.cpp | 5 +- .../Source/GemRepo/GemRepoScreen.cpp | 19 +- .../ProjectManager/Source/ProjectUtils.cpp | 19 ++ .../ProjectManager/Source/ProjectUtils.h | 9 + .../ProjectManager/Source/PythonBindings.cpp | 183 ++++++++++-------- .../ProjectManager/Source/PythonBindings.h | 12 +- .../Source/PythonBindingsInterface.h | 22 ++- scripts/o3de/o3de/engine_properties.py | 5 + scripts/o3de/o3de/manifest.py | 108 ++++++----- 12 files changed, 322 insertions(+), 152 deletions(-) diff --git a/Code/Tools/ProjectManager/Source/Application.cpp b/Code/Tools/ProjectManager/Source/Application.cpp index 29e0df3c3a..08a812999f 100644 --- a/Code/Tools/ProjectManager/Source/Application.cpp +++ b/Code/Tools/ProjectManager/Source/Application.cpp @@ -21,6 +21,7 @@ #include #include #include +#include namespace O3DE::ProjectManager { @@ -111,6 +112,11 @@ namespace O3DE::ProjectManager } } + if (!RegisterEngine(interactive)) + { + return false; + } + const AZ::CommandLine* commandLine = GetCommandLine(); AZ_Assert(commandLine, "Failed to get command line"); @@ -165,6 +171,86 @@ namespace O3DE::ProjectManager return m_entity != nullptr; } + bool Application::RegisterEngine(bool interactive) + { + // get this engine's info + auto engineInfoOutcome = m_pythonBindings->GetEngineInfo(); + if (!engineInfoOutcome) + { + if (interactive) + { + QMessageBox::critical(nullptr, + QObject::tr("Failed to get engine info"), + QObject::tr("A valid engine.json could not be found or loaded. " + "Please verify a valid engine.json file exists in %1") + .arg(GetEngineRoot())); + } + + AZ_Error("Project Manager", false, "Failed to get engine info"); + return false; + } + + EngineInfo engineInfo = engineInfoOutcome.GetValue(); + if (engineInfo.m_registered) + { + return true; + } + + bool forceRegistration = false; + + // check if an engine with this name is already registered + auto existingEngineResult = m_pythonBindings->GetEngineInfo(engineInfo.m_name); + if (existingEngineResult) + { + if (!interactive) + { + AZ_Error("Project Manager", false, "An engine with the name %s is already registered with the path %s", + engineInfo.m_name.toUtf8().constData(), engineInfo.m_path.toUtf8().constData()); + return false; + } + + // get the updated engine name unless the user wants to cancel + bool okPressed = false; + const EngineInfo& otherEngineInfo = existingEngineResult.GetValue(); + + engineInfo.m_name = QInputDialog::getText(nullptr, + QObject::tr("Engine '%1' already registered").arg(engineInfo.m_name), + QObject::tr("An engine named '%1' is already registered.

" + "Current path
%2

" + "New path
%3

" + "Press 'OK' to force registration, or provide a new engine name below.
" + "Alternatively, press `Cancel` to close the Project Manager and resolve the issue manually.") + .arg(engineInfo.m_name, otherEngineInfo.m_path, engineInfo.m_path), + QLineEdit::Normal, + engineInfo.m_name, + &okPressed); + + if (!okPressed) + { + // user elected not to change the name or force registration + return false; + } + + forceRegistration = true; + } + + auto registerOutcome = m_pythonBindings->SetEngineInfo(engineInfo, forceRegistration); + if (!registerOutcome) + { + if (interactive) + { + ProjectUtils::DisplayDetailedError(QObject::tr("Failed to register engine"), registerOutcome); + } + + AZ_Error("Project Manager", false, "Failed to register engine %s : %s", + engineInfo.m_path.toUtf8().constData(), registerOutcome.GetError().first.c_str()); + + return false; + } + + return true; + } + void Application::TearDown() { if (m_entity) diff --git a/Code/Tools/ProjectManager/Source/Application.h b/Code/Tools/ProjectManager/Source/Application.h index ad55694b18..8f633b28c4 100644 --- a/Code/Tools/ProjectManager/Source/Application.h +++ b/Code/Tools/ProjectManager/Source/Application.h @@ -34,6 +34,7 @@ namespace O3DE::ProjectManager private: bool InitLog(const char* logName); + bool RegisterEngine(bool interactive); AZStd::unique_ptr m_pythonBindings; QSharedPointer m_app; diff --git a/Code/Tools/ProjectManager/Source/EngineInfo.h b/Code/Tools/ProjectManager/Source/EngineInfo.h index 5fd3faf2ea..c28aede030 100644 --- a/Code/Tools/ProjectManager/Source/EngineInfo.h +++ b/Code/Tools/ProjectManager/Source/EngineInfo.h @@ -25,13 +25,16 @@ namespace O3DE::ProjectManager QString m_name; QString m_thirdPartyPath; - // from o3de_manifest.json QString m_path; + + // from o3de_manifest.json QString m_defaultProjectsFolder; QString m_defaultGemsFolder; QString m_defaultTemplatesFolder; QString m_defaultRestrictedFolder; + bool m_registered = false; + bool IsValid() const; }; } // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/EngineSettingsScreen.cpp b/Code/Tools/ProjectManager/Source/EngineSettingsScreen.cpp index c7df00f423..26f5b8ae11 100644 --- a/Code/Tools/ProjectManager/Source/EngineSettingsScreen.cpp +++ b/Code/Tools/ProjectManager/Source/EngineSettingsScreen.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -114,10 +115,10 @@ namespace O3DE::ProjectManager engineInfo.m_defaultGemsFolder = m_defaultGems->lineEdit()->text(); engineInfo.m_defaultTemplatesFolder = m_defaultProjectTemplates->lineEdit()->text(); - bool result = PythonBindingsInterface::Get()->SetEngineInfo(engineInfo); + auto result = PythonBindingsInterface::Get()->SetEngineInfo(engineInfo); if (!result) { - QMessageBox::critical(this, tr("Engine Settings"), tr("Failed to save engine settings.")); + ProjectUtils::DisplayDetailedError(tr("Failed to save engine settings"), result, this); } } else diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp index f62c30c280..843538d9da 100644 --- a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -92,8 +93,7 @@ namespace O3DE::ProjectManager return; } - AZ::Outcome < void, - AZStd::pair> addGemRepoResult = PythonBindingsInterface::Get()->AddGemRepo(repoUri); + auto addGemRepoResult = PythonBindingsInterface::Get()->AddGemRepo(repoUri); if (addGemRepoResult.IsSuccess()) { Reinit(); @@ -102,20 +102,7 @@ namespace O3DE::ProjectManager else { QString failureMessage = tr("Failed to add gem repo: %1.").arg(repoUri); - if (!addGemRepoResult.GetError().second.empty()) - { - 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 - { - QMessageBox::critical(this, failureMessage, addGemRepoResult.GetError().first.c_str()); - } - + ProjectUtils::DisplayDetailedError(failureMessage, addGemRepoResult, this); AZ_Error("Project Manager", false, failureMessage.toUtf8()); } } diff --git a/Code/Tools/ProjectManager/Source/ProjectUtils.cpp b/Code/Tools/ProjectManager/Source/ProjectUtils.cpp index b7748d8aa2..209140a004 100644 --- a/Code/Tools/ProjectManager/Source/ProjectUtils.cpp +++ b/Code/Tools/ProjectManager/Source/ProjectUtils.cpp @@ -659,5 +659,24 @@ namespace O3DE::ProjectManager return AZ::Success(QString(projectBuildPath.c_str())); } + void DisplayDetailedError(const QString& title, const AZ::Outcome>& outcome, QWidget* parent) + { + const AZStd::string& generalError = outcome.GetError().first; + const AZStd::string& detailedError = outcome.GetError().second; + + if (!detailedError.empty()) + { + QMessageBox errorDialog(parent); + errorDialog.setIcon(QMessageBox::Critical); + errorDialog.setWindowTitle(title); + errorDialog.setText(generalError.c_str()); + errorDialog.setDetailedText(detailedError.c_str()); + errorDialog.exec(); + } + else + { + QMessageBox::critical(parent, title, generalError.c_str()); + } + } } // namespace ProjectUtils } // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/ProjectUtils.h b/Code/Tools/ProjectManager/Source/ProjectUtils.h index 713803c20b..8602ffa692 100644 --- a/Code/Tools/ProjectManager/Source/ProjectUtils.h +++ b/Code/Tools/ProjectManager/Source/ProjectUtils.h @@ -98,5 +98,14 @@ namespace O3DE::ProjectManager */ AZ::IO::FixedMaxPath GetEditorExecutablePath(const AZ::IO::PathView& projectPath); + + /** + * Display a dialog with general and detailed sections for the given AZ::Outcome + * @param title Dialog title + * @param outcome The AZ::Outcome with general and detailed error messages + * @param parent Optional QWidget parent + */ + void DisplayDetailedError(const QString& title, const AZ::Outcome>& outcome, QWidget* parent = nullptr); + } // namespace ProjectUtils } // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/PythonBindings.cpp b/Code/Tools/ProjectManager/Source/PythonBindings.cpp index 12f97e6d2e..00ece7396d 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindings.cpp +++ b/Code/Tools/ProjectManager/Source/PythonBindings.cpp @@ -312,6 +312,7 @@ namespace O3DE::ProjectManager m_register = pybind11::module::import("o3de.register"); m_manifest = pybind11::module::import("o3de.manifest"); m_engineTemplate = pybind11::module::import("o3de.engine_template"); + m_engineProperties = pybind11::module::import("o3de.engine_properties"); m_enableGemProject = pybind11::module::import("o3de.enable_gem"); m_disableGemProject = pybind11::module::import("o3de.disable_gem"); m_editProjectProperties = pybind11::module::import("o3de.project_properties"); @@ -319,9 +320,6 @@ namespace O3DE::ProjectManager m_repo = pybind11::module::import("o3de.repo"); m_pathlib = pybind11::module::import("pathlib"); - // make sure the engine is registered - RegisterThisEngine(); - m_pythonStarted = !PyErr_Occurred(); return m_pythonStarted; } @@ -346,36 +344,6 @@ namespace O3DE::ProjectManager return !PyErr_Occurred(); } - bool PythonBindings::RegisterThisEngine() - { - bool registrationResult = true; // already registered is considered successful - bool pythonResult = ExecuteWithLock( - [&] - { - // check current engine path against all other registered engines - // to see if we are already registered - auto allEngines = m_manifest.attr("get_engines")(); - if (pybind11::isinstance(allEngines)) - { - for (auto engine : allEngines) - { - AZ::IO::FixedMaxPath enginePath(Py_To_String(engine)); - if (enginePath.Compare(m_enginePath) == 0) - { - return; - } - } - } - - auto result = m_register.attr("register")(QString_To_Py_Path(QString(m_enginePath.c_str()))); - registrationResult = (result.cast() == 0); - }); - - bool finalResult = (registrationResult && pythonResult); - AZ_Assert(finalResult, "Registration of this engine failed!"); - return finalResult; - } - AZ::Outcome PythonBindings::ExecuteWithLockErrorHandling(AZStd::function executionCallback) { if (!Py_IsInitialized()) @@ -407,16 +375,22 @@ namespace O3DE::ProjectManager return ExecuteWithLockErrorHandling(executionCallback).IsSuccess(); } - AZ::Outcome PythonBindings::GetEngineInfo() + EngineInfo PythonBindings::EngineInfoFromPath(pybind11::handle enginePath) { EngineInfo engineInfo; - bool result = ExecuteWithLock([&] { - auto enginePath = m_manifest.attr("get_this_engine_path")(); + try + { + auto engineData = m_manifest.attr("get_engine_json_data")(pybind11::none(), enginePath); + if (pybind11::isinstance(engineData)) + { + engineInfo.m_version = Py_To_String_Optional(engineData, "O3DEVersion", "0.0.0.0"); + engineInfo.m_name = Py_To_String_Optional(engineData, "engine_name", "O3DE"); + engineInfo.m_path = Py_To_String(enginePath); + } auto o3deData = m_manifest.attr("load_o3de_manifest")(); if (pybind11::isinstance(o3deData)) { - engineInfo.m_path = Py_To_String(enginePath); auto defaultGemsFolder = m_manifest.attr("get_o3de_gems_folder")(); engineInfo.m_defaultGemsFolder = Py_To_String_Optional(o3deData, "default_gems_folder", Py_To_String(defaultGemsFolder)); @@ -433,19 +407,36 @@ namespace O3DE::ProjectManager engineInfo.m_thirdPartyPath = Py_To_String_Optional(o3deData, "default_third_party_folder", Py_To_String(defaultThirdPartyFolder)); } - auto engineData = m_manifest.attr("get_engine_json_data")(pybind11::none(), enginePath); - if (pybind11::isinstance(engineData)) + // check if engine path is registered + auto allEngines = m_manifest.attr("get_engines")(); + if (pybind11::isinstance(allEngines)) { - try - { - engineInfo.m_version = Py_To_String_Optional(engineData, "O3DEVersion", "0.0.0.0"); - engineInfo.m_name = Py_To_String_Optional(engineData, "engine_name", "O3DE"); - } - catch ([[maybe_unused]] const std::exception& e) + const AZ::IO::FixedMaxPath enginePathFixed(Py_To_String(enginePath)); + for (auto engine : allEngines) { - AZ_Warning("PythonBindings", false, "Failed to get EngineInfo from %s", Py_To_String(enginePath)); + AZ::IO::FixedMaxPath otherEnginePath(Py_To_String(engine)); + if (otherEnginePath.Compare(enginePathFixed) == 0) + { + engineInfo.m_registered = true; + break; + } } } + } + catch ([[maybe_unused]] const std::exception& e) + { + AZ_Warning("PythonBindings", false, "Failed to get EngineInfo from %s", Py_To_String(enginePath)); + } + return engineInfo; + } + + AZ::Outcome PythonBindings::GetEngineInfo() + { + EngineInfo engineInfo; + + bool result = ExecuteWithLock([&] { + auto enginePath = m_manifest.attr("get_this_engine_path")(); + engineInfo = EngineInfoFromPath(enginePath); }); if (!result || !engineInfo.IsValid()) @@ -458,10 +449,55 @@ namespace O3DE::ProjectManager } } - bool PythonBindings::SetEngineInfo(const EngineInfo& engineInfo) + AZ::Outcome PythonBindings::GetEngineInfo(const QString& engineName) { + EngineInfo engineInfo; bool result = ExecuteWithLock([&] { - auto registrationResult = m_register.attr("register")( + auto enginePathResult = m_manifest.attr("get_registered")(QString_To_Py_String(engineName)); + + // if a valid registered object is not found None is returned + if (!pybind11::isinstance(enginePathResult)) + { + engineInfo = EngineInfoFromPath(enginePathResult); + } + }); + + if (!result || !engineInfo.IsValid()) + { + return AZ::Failure(); + } + else + { + return AZ::Success(AZStd::move(engineInfo)); + } + } + + IPythonBindings::DetailedOutcome PythonBindings::SetEngineInfo(const EngineInfo& engineInfo, bool force) + { + bool registrationSuccess = false; + bool pythonSuccess = ExecuteWithLock([&] { + + EngineInfo currentEngine = EngineInfoFromPath(QString_To_Py_Path(engineInfo.m_path)); + + // be kind to source control and avoid needlessly updating engine.json + if (currentEngine.IsValid() && + (currentEngine.m_name.compare(engineInfo.m_name) != 0 || currentEngine.m_version.compare(engineInfo.m_version) != 0)) + { + auto enginePropsResult = m_engineProperties.attr("edit_engine_props")( + QString_To_Py_Path(engineInfo.m_path), + pybind11::none(), // existing engine_name + QString_To_Py_String(engineInfo.m_name), + QString_To_Py_String(engineInfo.m_version) + ); + + if (enginePropsResult.cast() != 0) + { + // do not proceed with registration + return; + } + } + + auto result = m_register.attr("register")( QString_To_Py_Path(engineInfo.m_path), pybind11::none(), // project_path pybind11::none(), // gem_path @@ -474,16 +510,22 @@ namespace O3DE::ProjectManager QString_To_Py_Path(engineInfo.m_defaultGemsFolder), QString_To_Py_Path(engineInfo.m_defaultTemplatesFolder), pybind11::none(), // default_restricted_folder - QString_To_Py_Path(engineInfo.m_thirdPartyPath) - ); + QString_To_Py_Path(engineInfo.m_thirdPartyPath), + pybind11::none(), // external_subdir_engine_path + pybind11::none(), // external_subdir_project_path + false, // remove + force + ); - if (registrationResult.cast() != 0) - { - result = false; - } + registrationSuccess = result.cast() == 0; }); - return result; + if (pythonSuccess && registrationSuccess) + { + return AZ::Success(); + } + + return AZ::Failure(GetErrorPair()); } AZ::Outcome PythonBindings::GetGemInfo(const QString& path, const QString& projectPath) @@ -1064,7 +1106,7 @@ namespace O3DE::ProjectManager return result && refreshResult; } - AZ::Outcome> PythonBindings::AddGemRepo(const QString& repoUri) + IPythonBindings::DetailedOutcome PythonBindings::AddGemRepo(const QString& repoUri) { bool registrationResult = false; bool result = ExecuteWithLock( @@ -1080,7 +1122,7 @@ namespace O3DE::ProjectManager if (!result || !registrationResult) { - return AZ::Failure>(GetSimpleDetailedErrorPair()); + return AZ::Failure(GetErrorPair()); } return AZ::Success(); @@ -1170,13 +1212,10 @@ namespace O3DE::ProjectManager return gemRepoInfo; } -//#define MOCK_GEM_REPO_INFO true - AZ::Outcome, AZStd::string> PythonBindings::GetAllGemRepoInfos() { QVector gemRepos; -#ifndef MOCK_GEM_REPO_INFO auto result = ExecuteWithLockErrorHandling( [&] { @@ -1189,18 +1228,6 @@ namespace O3DE::ProjectManager { return AZ::Failure(result.GetError().c_str()); } -#else - GemRepoInfo mockJohnRepo("JohnCreates", "John Smith", QDateTime(QDate(2021, 8, 31), QTime(11, 57)), true); - mockJohnRepo.m_summary = "John's Summary. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Fusce sollicitudin dapibus urna"; - mockJohnRepo.m_repoUri = "https://github.com/o3de/o3de"; - mockJohnRepo.m_additionalInfo = "John's additional info. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Fusce sollicitu."; - gemRepos.push_back(mockJohnRepo); - - GemRepoInfo mockJaneRepo("JanesGems", "Jane Doe", QDateTime(QDate(2021, 9, 10), QTime(18, 23)), false); - mockJaneRepo.m_summary = "Jane's Summary."; - mockJaneRepo.m_repoUri = "https://github.com/o3de/o3de.org"; - gemRepos.push_back(mockJaneRepo); -#endif // MOCK_GEM_REPO_INFO std::sort(gemRepos.begin(), gemRepos.end()); return AZ::Success(AZStd::move(gemRepos)); @@ -1261,7 +1288,7 @@ namespace O3DE::ProjectManager return AZ::Success(AZStd::move(gemInfos)); } - AZ::Outcome> PythonBindings::DownloadGem( + IPythonBindings::DetailedOutcome PythonBindings::DownloadGem( const QString& gemName, std::function gemProgressCallback, bool force) { // This process is currently limited to download a single gem at a time. @@ -1290,12 +1317,12 @@ namespace O3DE::ProjectManager if (!result.IsSuccess()) { - AZStd::pair pythonRunError(result.GetError(), result.GetError()); - return AZ::Failure>(AZStd::move(pythonRunError)); + IPythonBindings::ErrorPair pythonRunError(result.GetError(), result.GetError()); + return AZ::Failure(AZStd::move(pythonRunError)); } else if (!downloadSucceeded) { - return AZ::Failure>(GetSimpleDetailedErrorPair()); + return AZ::Failure(GetErrorPair()); } return AZ::Success(); @@ -1322,13 +1349,13 @@ namespace O3DE::ProjectManager return result && updateAvaliableResult; } - AZStd::pair PythonBindings::GetSimpleDetailedErrorPair() + IPythonBindings::ErrorPair PythonBindings::GetErrorPair() { AZStd::string detailedString = m_pythonErrorStrings.size() == 1 ? "" : AZStd::accumulate(m_pythonErrorStrings.begin(), m_pythonErrorStrings.end(), AZStd::string("")); - return AZStd::pair(m_pythonErrorStrings.front(), detailedString); + return IPythonBindings::ErrorPair(m_pythonErrorStrings.front(), detailedString); } void PythonBindings::AddErrorString(AZStd::string errorString) diff --git a/Code/Tools/ProjectManager/Source/PythonBindings.h b/Code/Tools/ProjectManager/Source/PythonBindings.h index 48841b6565..e2a8109128 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindings.h +++ b/Code/Tools/ProjectManager/Source/PythonBindings.h @@ -35,7 +35,8 @@ namespace O3DE::ProjectManager // Engine AZ::Outcome GetEngineInfo() override; - bool SetEngineInfo(const EngineInfo& engineInfo) override; + AZ::Outcome GetEngineInfo(const QString& engineName) override; + DetailedOutcome SetEngineInfo(const EngineInfo& engineInfo, bool force = false) override; // Gem AZ::Outcome GetGemInfo(const QString& path, const QString& projectPath = {}) override; @@ -62,12 +63,12 @@ namespace O3DE::ProjectManager // Gem Repos AZ::Outcome RefreshGemRepo(const QString& repoUri) override; bool RefreshAllGemRepos() override; - AZ::Outcome> AddGemRepo(const QString& repoUri) override; + DetailedOutcome AddGemRepo(const QString& repoUri) override; bool RemoveGemRepo(const QString& repoUri) override; AZ::Outcome, AZStd::string> GetAllGemRepoInfos() override; AZ::Outcome, AZStd::string> GetGemInfosForRepo(const QString& repoUri) override; AZ::Outcome, AZStd::string> GetGemInfosForAllRepos() override; - AZ::Outcome> DownloadGem( + DetailedOutcome DownloadGem( const QString& gemName, std::function gemProgressCallback, bool force = false) override; void CancelDownload() override; bool IsGemUpdateAvaliable(const QString& gemName, const QString& lastUpdated) override; @@ -80,14 +81,14 @@ namespace O3DE::ProjectManager AZ::Outcome ExecuteWithLockErrorHandling(AZStd::function executionCallback); bool ExecuteWithLock(AZStd::function executionCallback); + EngineInfo EngineInfoFromPath(pybind11::handle enginePath); GemInfo GemInfoFromPath(pybind11::handle path, pybind11::handle pyProjectPath); GemRepoInfo GetGemRepoInfo(pybind11::handle repoUri); ProjectInfo ProjectInfoFromPath(pybind11::handle path); ProjectTemplateInfo ProjectTemplateInfoFromPath(pybind11::handle path, pybind11::handle pyProjectPath); AZ::Outcome GemRegistration(const QString& gemPath, const QString& projectPath, bool remove = false); - bool RegisterThisEngine(); bool StopPython(); - AZStd::pair GetSimpleDetailedErrorPair(); + IPythonBindings::ErrorPair GetErrorPair(); bool m_pythonStarted = false; @@ -96,6 +97,7 @@ namespace O3DE::ProjectManager AZStd::recursive_mutex m_lock; pybind11::handle m_engineTemplate; + pybind11::handle m_engineProperties; pybind11::handle m_cmake; pybind11::handle m_register; pybind11::handle m_manifest; diff --git a/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h b/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h index c7c8af2ce1..a42ff310c3 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h +++ b/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h @@ -31,6 +31,10 @@ namespace O3DE::ProjectManager IPythonBindings() = default; virtual ~IPythonBindings() = default; + //! First string in pair is general error, second is detailed + using ErrorPair = AZStd::pair; + using DetailedOutcome = AZ::Outcome; + /** * Get whether Python was started or not. All Python functionality will fail if Python * failed to start. @@ -49,17 +53,25 @@ namespace O3DE::ProjectManager // Engine /** - * Get info about the engine + * Get info about the current engine * @return an outcome with EngineInfo on success */ virtual AZ::Outcome GetEngineInfo() = 0; + /** + * Get info about an engine by name + * @param engineName The name of the engine to get info about + * @return an outcome with EngineInfo on success + */ + virtual AZ::Outcome GetEngineInfo(const QString& engineName) = 0; + /** * Set info about the engine + * @param force True to force registration even if an engine with the same name is already registered * @param engineInfo an EngineInfo object + * @return a detailed error outcome on failure. */ - virtual bool SetEngineInfo(const EngineInfo& engineInfo) = 0; - + virtual DetailedOutcome SetEngineInfo(const EngineInfo& engineInfo, bool force = false) = 0; // Gems @@ -202,7 +214,7 @@ namespace O3DE::ProjectManager * @param repoUri the absolute filesystem path or url to the gem repo. * @return an outcome with a pair of string error and detailed messages on failure. */ - virtual AZ::Outcome> AddGemRepo(const QString& repoUri) = 0; + virtual DetailedOutcome AddGemRepo(const QString& repoUri) = 0; /** * Unregisters this gem repo with the current engine. @@ -237,7 +249,7 @@ namespace O3DE::ProjectManager * @param force should we forcibly overwrite the old version of the gem. * @return an outcome with a pair of string error and detailed messages on failure. */ - virtual AZ::Outcome> DownloadGem( + virtual DetailedOutcome DownloadGem( const QString& gemName, std::function gemProgressCallback, bool force = false) = 0; /** diff --git a/scripts/o3de/o3de/engine_properties.py b/scripts/o3de/o3de/engine_properties.py index 92930dbb1c..56cb71f258 100644 --- a/scripts/o3de/o3de/engine_properties.py +++ b/scripts/o3de/o3de/engine_properties.py @@ -25,6 +25,11 @@ def edit_engine_props(engine_path: pathlib.Path = None, if not engine_path and not engine_name: logger.error(f'Either a engine path or a engine name must be supplied to lookup engine.json') return 1 + + if not new_name and not new_version: + logger.error('A new engine name or new version, or both must be supplied.') + return 1 + if not engine_path: engine_path = manifest.get_registered(engine_name=engine_name) diff --git a/scripts/o3de/o3de/manifest.py b/scripts/o3de/o3de/manifest.py index e46b819a7c..c06a4d12fc 100644 --- a/scripts/o3de/o3de/manifest.py +++ b/scripts/o3de/o3de/manifest.py @@ -599,75 +599,93 @@ def get_registered(engine_name: str = None, engine_path = pathlib.Path(engine).resolve() engine_json = engine_path / 'engine.json' - with engine_json.open('r') as f: - try: - engine_json_data = json.load(f) - except json.JSONDecodeError as e: - logger.warning(f'{engine_json} failed to load: {str(e)}') - else: - this_engines_name = engine_json_data['engine_name'] - if this_engines_name == engine_name: - return engine_path + if not pathlib.Path(engine_json).is_file(): + logger.warning(f'{engine_json} does not exist') + else: + with engine_json.open('r') as f: + try: + engine_json_data = json.load(f) + except json.JSONDecodeError as e: + logger.warning(f'{engine_json} failed to load: {str(e)}') + else: + this_engines_name = engine_json_data['engine_name'] + if this_engines_name == engine_name: + return engine_path + engines_path = json_data.get('engines_path', {}) + if engine_name in engines_path: + return pathlib.Path(engines_path[engine_name]).resolve() elif isinstance(project_name, str): projects = get_all_projects() for project_path in projects: project_path = pathlib.Path(project_path).resolve() project_json = project_path / 'project.json' - with project_json.open('r') as f: - try: - project_json_data = json.load(f) - except json.JSONDecodeError as e: - logger.warning(f'{project_json} failed to load: {str(e)}') - else: - this_projects_name = project_json_data['project_name'] - if this_projects_name == project_name: - return project_path + if not pathlib.Path(project_json).is_file(): + logger.warning(f'{project_json} does not exist') + else: + with project_json.open('r') as f: + try: + project_json_data = json.load(f) + except json.JSONDecodeError as e: + logger.warning(f'{project_json} failed to load: {str(e)}') + else: + this_projects_name = project_json_data['project_name'] + if this_projects_name == project_name: + return project_path elif isinstance(gem_name, str): gems = get_all_gems(project_path) for gem_path in gems: gem_path = pathlib.Path(gem_path).resolve() gem_json = gem_path / 'gem.json' - with gem_json.open('r') as f: - try: - gem_json_data = json.load(f) - except json.JSONDecodeError as e: - logger.warning(f'{gem_json} failed to load: {str(e)}') - else: - this_gems_name = gem_json_data['gem_name'] - if this_gems_name == gem_name: - return gem_path + if not pathlib.Path(gem_json).is_file(): + logger.warning(f'{gem_json} does not exist') + else: + with gem_json.open('r') as f: + try: + gem_json_data = json.load(f) + except json.JSONDecodeError as e: + logger.warning(f'{gem_json} failed to load: {str(e)}') + else: + this_gems_name = gem_json_data['gem_name'] + if this_gems_name == gem_name: + return gem_path elif isinstance(template_name, str): templates = get_all_templates(project_path) for template_path in templates: template_path = pathlib.Path(template_path).resolve() template_json = template_path / 'template.json' - with template_json.open('r') as f: - try: - template_json_data = json.load(f) - except json.JSONDecodeError as e: - logger.warning(f'{template_path} failed to load: {str(e)}') - else: - this_templates_name = template_json_data['template_name'] - if this_templates_name == template_name: - return template_path + if not pathlib.Path(template_json).is_file(): + logger.warning(f'{template_json} does not exist') + else: + with template_json.open('r') as f: + try: + template_json_data = json.load(f) + except json.JSONDecodeError as e: + logger.warning(f'{template_path} failed to load: {str(e)}') + else: + this_templates_name = template_json_data['template_name'] + if this_templates_name == template_name: + return template_path elif isinstance(restricted_name, str): restricted = get_all_restricted(project_path) for restricted_path in restricted: restricted_path = pathlib.Path(restricted_path).resolve() restricted_json = restricted_path / 'restricted.json' - with restricted_json.open('r') as f: - try: - restricted_json_data = json.load(f) - except json.JSONDecodeError as e: - logger.warning(f'{restricted_json} failed to load: {str(e)}') - else: - this_restricted_name = restricted_json_data['restricted_name'] - if this_restricted_name == restricted_name: - return restricted_path + if not pathlib.Path(restricted_json).is_file(): + logger.warning(f'{restricted_json} does not exist') + else: + with restricted_json.open('r') as f: + try: + restricted_json_data = json.load(f) + except json.JSONDecodeError as e: + logger.warning(f'{restricted_json} failed to load: {str(e)}') + else: + this_restricted_name = restricted_json_data['restricted_name'] + if this_restricted_name == restricted_name: + return restricted_path elif isinstance(default_folder, str): if default_folder == 'engines': From 93358dcbeb5148ea76bbb8fc10f0e325dcfe3a83 Mon Sep 17 00:00:00 2001 From: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> Date: Fri, 21 Jan 2022 09:19:53 +0000 Subject: [PATCH 7/8] Cherry-pick of PR-6700 - Updates to ViewportTitleDlg to better expose grid snapping visualization (#6997) * cherry-pick of PR 6700 Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> * revert title dialog look to old style Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> --- Code/Editor/ViewportTitleDlg.cpp | 65 ++++++++++++++++++++------------ Code/Editor/ViewportTitleDlg.h | 3 +- 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/Code/Editor/ViewportTitleDlg.cpp b/Code/Editor/ViewportTitleDlg.cpp index 75d16e9a40..12f47f70eb 100644 --- a/Code/Editor/ViewportTitleDlg.cpp +++ b/Code/Editor/ViewportTitleDlg.cpp @@ -229,25 +229,35 @@ void CViewportTitleDlg::SetupHelpersButton() void CViewportTitleDlg::SetupOverflowMenu() { - // Setup the overflow menu - QMenu* overFlowMenu = new QMenu(this); + // setup the overflow menu + auto overflowMenu = new QMenu(this); - m_audioMuteAction = new QAction("Mute Audio", overFlowMenu); + m_audioMuteAction = new QAction("Mute Audio", overflowMenu); connect(m_audioMuteAction, &QAction::triggered, this, &CViewportTitleDlg::OnBnClickedMuteAudio); - overFlowMenu->addAction(m_audioMuteAction); + overflowMenu->addAction(m_audioMuteAction); - m_enableVRAction = new QAction("Enable VR Preview", overFlowMenu); + m_enableVRAction = new QAction("Enable VR Preview", overflowMenu); connect(m_enableVRAction, &QAction::triggered, this, &CViewportTitleDlg::OnBnClickedEnableVR); - overFlowMenu->addAction(m_enableVRAction); + overflowMenu->addAction(m_enableVRAction); - overFlowMenu->addSeparator(); + overflowMenu->addSeparator(); - m_enableGridSnappingAction = new QAction("Enable Grid Snapping", overFlowMenu); + m_enableGridSnappingAction = new QAction("Enable Grid Snapping", overflowMenu); connect(m_enableGridSnappingAction, &QAction::triggered, this, &CViewportTitleDlg::OnGridSnappingToggled); m_enableGridSnappingAction->setCheckable(true); - overFlowMenu->addAction(m_enableGridSnappingAction); + overflowMenu->addAction(m_enableGridSnappingAction); - m_gridSizeActionWidget = new QWidgetAction(overFlowMenu); + m_enableGridVisualizationAction = new QAction("Show Grid", overflowMenu); + connect( + m_enableGridVisualizationAction, &QAction::triggered, + [] + { + SandboxEditor::SetShowingGrid(!SandboxEditor::ShowingGrid()); + }); + m_enableGridVisualizationAction->setCheckable(true); + overflowMenu->addAction(m_enableGridVisualizationAction); + + m_gridSizeActionWidget = new QWidgetAction(overflowMenu); m_gridSpinBox = new AzQtComponents::DoubleSpinBox(); m_gridSpinBox->setValue(SandboxEditor::GridSnappingSize()); m_gridSpinBox->setMinimum(1e-2f); @@ -257,31 +267,31 @@ void CViewportTitleDlg::SetupOverflowMenu() m_gridSpinBox, QOverload::of(&AzQtComponents::DoubleSpinBox::valueChanged), this, &CViewportTitleDlg::OnGridSpinBoxChanged); m_gridSizeActionWidget->setDefaultWidget(m_gridSpinBox); - overFlowMenu->addAction(m_gridSizeActionWidget); + overflowMenu->addAction(m_gridSizeActionWidget); - overFlowMenu->addSeparator(); + overflowMenu->addSeparator(); - m_enableAngleSnappingAction = new QAction("Enable Angle Snapping", overFlowMenu); + m_enableAngleSnappingAction = new QAction("Enable Angle Snapping", overflowMenu); connect(m_enableAngleSnappingAction, &QAction::triggered, this, &CViewportTitleDlg::OnAngleSnappingToggled); m_enableAngleSnappingAction->setCheckable(true); - overFlowMenu->addAction(m_enableAngleSnappingAction); + overflowMenu->addAction(m_enableAngleSnappingAction); - m_angleSizeActionWidget = new QWidgetAction(overFlowMenu); + m_angleSizeActionWidget = new QWidgetAction(overflowMenu); m_angleSpinBox = new AzQtComponents::DoubleSpinBox(); m_angleSpinBox->setValue(SandboxEditor::AngleSnappingSize()); m_angleSpinBox->setMinimum(1e-2f); - m_angleSpinBox->setToolTip(tr("Angle Snapping")); + m_angleSpinBox->setToolTip(tr("Angle size")); QObject::connect( m_angleSpinBox, QOverload::of(&AzQtComponents::DoubleSpinBox::valueChanged), this, &CViewportTitleDlg::OnAngleSpinBoxChanged); m_angleSizeActionWidget->setDefaultWidget(m_angleSpinBox); - overFlowMenu->addAction(m_angleSizeActionWidget); + overflowMenu->addAction(m_angleSizeActionWidget); - m_ui->m_overflowBtn->setMenu(overFlowMenu); + m_ui->m_overflowBtn->setMenu(overflowMenu); m_ui->m_overflowBtn->setPopupMode(QToolButton::InstantPopup); - connect(overFlowMenu, &QMenu::aboutToShow, this, &CViewportTitleDlg::UpdateOverFlowMenuState); + connect(overflowMenu, &QMenu::aboutToShow, this, &CViewportTitleDlg::UpdateOverFlowMenuState); UpdateMuteActionText(); } @@ -1004,31 +1014,36 @@ void CViewportTitleDlg::OnAngleSnappingToggled() MainWindow::instance()->GetActionManager()->GetAction(AzToolsFramework::SnapAngle)->trigger(); } -void CViewportTitleDlg::OnGridSpinBoxChanged(double value) +void CViewportTitleDlg::OnGridSpinBoxChanged(const double value) { - SandboxEditor::SetGridSnappingSize(static_cast(value)); + SandboxEditor::SetGridSnappingSize(aznumeric_cast(value)); } -void CViewportTitleDlg::OnAngleSpinBoxChanged(double value) +void CViewportTitleDlg::OnAngleSpinBoxChanged(const double value) { - SandboxEditor::SetAngleSnappingSize(static_cast(value)); + SandboxEditor::SetAngleSnappingSize(aznumeric_cast(value)); } void CViewportTitleDlg::UpdateOverFlowMenuState() { - bool gridSnappingActive = MainWindow::instance()->GetActionManager()->GetAction(AzToolsFramework::SnapToGrid)->isChecked(); + const bool gridSnappingActive = MainWindow::instance()->GetActionManager()->GetAction(AzToolsFramework::SnapToGrid)->isChecked(); { QSignalBlocker signalBlocker(m_enableGridSnappingAction); m_enableGridSnappingAction->setChecked(gridSnappingActive); } m_gridSizeActionWidget->setEnabled(gridSnappingActive); - bool angleSnappingActive = MainWindow::instance()->GetActionManager()->GetAction(AzToolsFramework::SnapAngle)->isChecked(); + const bool angleSnappingActive = MainWindow::instance()->GetActionManager()->GetAction(AzToolsFramework::SnapAngle)->isChecked(); { QSignalBlocker signalBlocker(m_enableAngleSnappingAction); m_enableAngleSnappingAction->setChecked(angleSnappingActive); } m_angleSizeActionWidget->setEnabled(angleSnappingActive); + + { + QSignalBlocker signalBlocker(m_enableGridVisualizationAction); + m_enableGridVisualizationAction->setChecked(SandboxEditor::ShowingGrid()); + } } namespace diff --git a/Code/Editor/ViewportTitleDlg.h b/Code/Editor/ViewportTitleDlg.h index 6996fe7750..f95828b428 100644 --- a/Code/Editor/ViewportTitleDlg.h +++ b/Code/Editor/ViewportTitleDlg.h @@ -171,6 +171,7 @@ protected: QAction* m_enableVRAction = nullptr; QAction* m_enableGridSnappingAction = nullptr; QAction* m_enableAngleSnappingAction = nullptr; + QAction* m_enableGridVisualizationAction = nullptr; QComboBox* m_cameraSpeed = nullptr; AzQtComponents::DoubleSpinBox* m_gridSpinBox = nullptr; AzQtComponents::DoubleSpinBox* m_angleSpinBox = nullptr; @@ -184,7 +185,7 @@ protected: namespace AzToolsFramework { - //! A component to reflect scriptable commands for the Editor + //! A component to reflect scriptable commands for the Editor. class ViewportTitleDlgPythonFuncsHandler : public AZ::Component { From 1909e5fa540e11d9fa010dc0b578e45f095dbd15 Mon Sep 17 00:00:00 2001 From: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com> Date: Fri, 21 Jan 2022 13:09:09 -0800 Subject: [PATCH 8/8] Handle case where engine.json missing or corrupt (#7049) Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com> --- Code/Tools/ProjectManager/Source/Application.cpp | 9 ++++----- Code/Tools/ProjectManager/Source/PythonBindings.cpp | 3 +++ scripts/o3de/o3de/manifest.py | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Code/Tools/ProjectManager/Source/Application.cpp b/Code/Tools/ProjectManager/Source/Application.cpp index 08a812999f..7ccd855b7e 100644 --- a/Code/Tools/ProjectManager/Source/Application.cpp +++ b/Code/Tools/ProjectManager/Source/Application.cpp @@ -196,9 +196,7 @@ namespace O3DE::ProjectManager return true; } - bool forceRegistration = false; - - // check if an engine with this name is already registered + // check if an engine with this name is already registered and has a valid engine.json auto existingEngineResult = m_pythonBindings->GetEngineInfo(engineInfo.m_name); if (existingEngineResult) { @@ -230,10 +228,11 @@ namespace O3DE::ProjectManager // user elected not to change the name or force registration return false; } - - forceRegistration = true; } + // always force register in case there is an engine registered in o3de_manifest.json, but + // the engine.json is missing or corrupt in which case GetEngineInfo() fails + constexpr bool forceRegistration = true; auto registerOutcome = m_pythonBindings->SetEngineInfo(engineInfo, forceRegistration); if (!registerOutcome) { diff --git a/Code/Tools/ProjectManager/Source/PythonBindings.cpp b/Code/Tools/ProjectManager/Source/PythonBindings.cpp index 00ece7396d..13fa73625c 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindings.cpp +++ b/Code/Tools/ProjectManager/Source/PythonBindings.cpp @@ -459,6 +459,9 @@ namespace O3DE::ProjectManager if (!pybind11::isinstance(enginePathResult)) { engineInfo = EngineInfoFromPath(enginePathResult); + + // it is possible an engine is registered in o3de_manifest.json but the engine.json is + // missing or corrupt in which case we do not consider it a registered engine } }); diff --git a/scripts/o3de/o3de/manifest.py b/scripts/o3de/o3de/manifest.py index c06a4d12fc..51b6eaf226 100644 --- a/scripts/o3de/o3de/manifest.py +++ b/scripts/o3de/o3de/manifest.py @@ -608,7 +608,7 @@ def get_registered(engine_name: str = None, except json.JSONDecodeError as e: logger.warning(f'{engine_json} failed to load: {str(e)}') else: - this_engines_name = engine_json_data['engine_name'] + this_engines_name = engine_json_data.get('engine_name','') if this_engines_name == engine_name: return engine_path engines_path = json_data.get('engines_path', {})