From bbae6490d974c8d7084a9c615be5d0334f2b19e6 Mon Sep 17 00:00:00 2001 From: srikappa-amzn Date: Thu, 10 Jun 2021 14:38:14 -0700 Subject: [PATCH 01/16] Enabled LyTestTools trait only for windows and mac --- cmake/Platform/Android/PAL_android.cmake | 1 + cmake/Platform/Linux/PAL_linux.cmake | 1 + cmake/Platform/Mac/PAL_mac.cmake | 1 + cmake/Platform/Windows/PAL_windows.cmake | 1 + cmake/Platform/iOS/PAL_ios.cmake | 1 + scripts/ctest/CMakeLists.txt | 24 +++++++++++++----------- 6 files changed, 18 insertions(+), 11 deletions(-) diff --git a/cmake/Platform/Android/PAL_android.cmake b/cmake/Platform/Android/PAL_android.cmake index 5f35767f98..dd61e35e53 100644 --- a/cmake/Platform/Android/PAL_android.cmake +++ b/cmake/Platform/Android/PAL_android.cmake @@ -24,6 +24,7 @@ ly_set(PAL_TRAIT_BUILD_CPACK_SUPPORTED FALSE) # Test library support ly_set(PAL_TRAIT_TEST_GOOGLE_TEST_SUPPORTED FALSE) ly_set(PAL_TRAIT_TEST_GOOGLE_BENCHMARK_SUPPORTED FALSE) +ly_set(PAL_TRAIT_TEST_LYTESTTOOLS_SUPPORTED FALSE) ly_set(PAL_TRAIT_TEST_PYTEST_SUPPORTED FALSE) ly_set(PAL_TRAIT_TEST_TARGET_TYPE MODULE) diff --git a/cmake/Platform/Linux/PAL_linux.cmake b/cmake/Platform/Linux/PAL_linux.cmake index 40b73adcec..c15f2bada9 100644 --- a/cmake/Platform/Linux/PAL_linux.cmake +++ b/cmake/Platform/Linux/PAL_linux.cmake @@ -24,6 +24,7 @@ ly_set(PAL_TRAIT_BUILD_CPACK_SUPPORTED FALSE) # Test library support ly_set(PAL_TRAIT_TEST_GOOGLE_TEST_SUPPORTED TRUE) ly_set(PAL_TRAIT_TEST_GOOGLE_BENCHMARK_SUPPORTED TRUE) +ly_set(PAL_TRAIT_TEST_LYTESTTOOLS_SUPPORTED FALSE) ly_set(PAL_TRAIT_TEST_PYTEST_SUPPORTED TRUE) ly_set(PAL_TRAIT_TEST_TARGET_TYPE MODULE) diff --git a/cmake/Platform/Mac/PAL_mac.cmake b/cmake/Platform/Mac/PAL_mac.cmake index 988d36af14..f49578a83a 100644 --- a/cmake/Platform/Mac/PAL_mac.cmake +++ b/cmake/Platform/Mac/PAL_mac.cmake @@ -24,6 +24,7 @@ ly_set(PAL_TRAIT_BUILD_CPACK_SUPPORTED FALSE) # Test library support ly_set(PAL_TRAIT_TEST_GOOGLE_TEST_SUPPORTED TRUE) ly_set(PAL_TRAIT_TEST_GOOGLE_BENCHMARK_SUPPORTED TRUE) +ly_set(PAL_TRAIT_TEST_LYTESTTOOLS_SUPPORTED TRUE) ly_set(PAL_TRAIT_TEST_PYTEST_SUPPORTED FALSE) ly_set(PAL_TRAIT_TEST_TARGET_TYPE MODULE) diff --git a/cmake/Platform/Windows/PAL_windows.cmake b/cmake/Platform/Windows/PAL_windows.cmake index ddcc3a27c7..fbf65db63f 100644 --- a/cmake/Platform/Windows/PAL_windows.cmake +++ b/cmake/Platform/Windows/PAL_windows.cmake @@ -24,6 +24,7 @@ ly_set(PAL_TRAIT_BUILD_CPACK_SUPPORTED TRUE) # Test library support ly_set(PAL_TRAIT_TEST_GOOGLE_TEST_SUPPORTED TRUE) ly_set(PAL_TRAIT_TEST_GOOGLE_BENCHMARK_SUPPORTED TRUE) +ly_set(PAL_TRAIT_TEST_LYTESTTOOLS_SUPPORTED TRUE) ly_set(PAL_TRAIT_TEST_PYTEST_SUPPORTED TRUE) ly_set(PAL_TRAIT_TEST_TARGET_TYPE MODULE) diff --git a/cmake/Platform/iOS/PAL_ios.cmake b/cmake/Platform/iOS/PAL_ios.cmake index a6828a1bc7..981bb9cab1 100644 --- a/cmake/Platform/iOS/PAL_ios.cmake +++ b/cmake/Platform/iOS/PAL_ios.cmake @@ -24,6 +24,7 @@ ly_set(PAL_TRAIT_BUILD_CPACK_SUPPORTED FALSE) # Test library support ly_set(PAL_TRAIT_TEST_GOOGLE_TEST_SUPPORTED FALSE) ly_set(PAL_TRAIT_TEST_GOOGLE_BENCHMARK_SUPPORTED FALSE) +ly_set(PAL_TRAIT_TEST_LYTESTTOOLS_SUPPORTED FALSE) ly_set(PAL_TRAIT_TEST_PYTEST_SUPPORTED FALSE) ly_set(PAL_TRAIT_TEST_TARGET_TYPE MODULE) diff --git a/scripts/ctest/CMakeLists.txt b/scripts/ctest/CMakeLists.txt index 575be53cc7..c4f86087b1 100644 --- a/scripts/ctest/CMakeLists.txt +++ b/scripts/ctest/CMakeLists.txt @@ -21,18 +21,20 @@ endif() ################################################################################ foreach(suite_name ${LY_TEST_GLOBAL_KNOWN_SUITE_NAMES}) - ly_add_pytest( - NAME pytest_sanity_${suite_name}_no_gpu - PATH ${CMAKE_CURRENT_LIST_DIR}/sanity_test.py - TEST_SUITE ${suite_name} - ) + if(PAL_TRAIT_TEST_LYTESTTOOLS_SUPPORTED) + ly_add_pytest( + NAME pytest_sanity_${suite_name}_no_gpu + PATH ${CMAKE_CURRENT_LIST_DIR}/sanity_test.py + TEST_SUITE ${suite_name} + ) - ly_add_pytest( - NAME pytest_sanity_${suite_name}_requires_gpu - PATH ${CMAKE_CURRENT_LIST_DIR}/sanity_test.py - TEST_SUITE ${suite_name} - TEST_REQUIRES gpu - ) + ly_add_pytest( + NAME pytest_sanity_${suite_name}_requires_gpu + PATH ${CMAKE_CURRENT_LIST_DIR}/sanity_test.py + TEST_SUITE ${suite_name} + TEST_REQUIRES gpu + ) + endif() endforeach() # EPB Sanity test is being registered here to validate that the ly_add_editor_python_test function works. From 7590ef04b26c9c68076dba8f5446fe8538a0caa6 Mon Sep 17 00:00:00 2001 From: jiaweig Date: Thu, 10 Jun 2021 16:48:48 -0700 Subject: [PATCH 02/16] Increase the platform limit to handle more objects. --- .../Assets/Config/Platform/Windows/DX12/PlatformLimits.azasset | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gems/Atom/Feature/Common/Assets/Config/Platform/Windows/DX12/PlatformLimits.azasset b/Gems/Atom/Feature/Common/Assets/Config/Platform/Windows/DX12/PlatformLimits.azasset index 7e71111ef1..3c88544e62 100644 --- a/Gems/Atom/Feature/Common/Assets/Config/Platform/Windows/DX12/PlatformLimits.azasset +++ b/Gems/Atom/Feature/Common/Assets/Config/Platform/Windows/DX12/PlatformLimits.azasset @@ -8,7 +8,7 @@ "$type": "DX12::PlatformLimitsDescriptor", "m_descriptorHeapLimits": { - "DESCRIPTOR_HEAP_TYPE_CBV_SRV_UAV": [16384, 262144], + "DESCRIPTOR_HEAP_TYPE_CBV_SRV_UAV": [1000000, 1000000], "DESCRIPTOR_HEAP_TYPE_SAMPLER": [2048, 2048], "DESCRIPTOR_HEAP_TYPE_RTV": [2048, 0], "DESCRIPTOR_HEAP_TYPE_DSV": [2048, 0] From bf1b8001361923548feb06ea077ce4fcf61bc8fc Mon Sep 17 00:00:00 2001 From: srikappa-amzn Date: Fri, 11 Jun 2021 10:25:28 -0700 Subject: [PATCH 03/16] minor change to order of if and foreach --- scripts/ctest/CMakeLists.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/ctest/CMakeLists.txt b/scripts/ctest/CMakeLists.txt index c4f86087b1..c07aaf4bff 100644 --- a/scripts/ctest/CMakeLists.txt +++ b/scripts/ctest/CMakeLists.txt @@ -20,8 +20,8 @@ endif() # Tests ################################################################################ -foreach(suite_name ${LY_TEST_GLOBAL_KNOWN_SUITE_NAMES}) - if(PAL_TRAIT_TEST_LYTESTTOOLS_SUPPORTED) +if(PAL_TRAIT_TEST_LYTESTTOOLS_SUPPORTED) + foreach(suite_name ${LY_TEST_GLOBAL_KNOWN_SUITE_NAMES}) ly_add_pytest( NAME pytest_sanity_${suite_name}_no_gpu PATH ${CMAKE_CURRENT_LIST_DIR}/sanity_test.py @@ -34,8 +34,8 @@ foreach(suite_name ${LY_TEST_GLOBAL_KNOWN_SUITE_NAMES}) TEST_SUITE ${suite_name} TEST_REQUIRES gpu ) - endif() -endforeach() + endforeach() +endif() # EPB Sanity test is being registered here to validate that the ly_add_editor_python_test function works. #if(PAL_TRAIT_BUILD_HOST_TOOLS AND PAL_TRAIT_BUILD_TESTS_SUPPORTED AND AutomatedTesting IN_LIST LY_PROJECTS_TARGET_NAME) From 3895c93e04a3fe33c458c0a5eb7ccb6768d956ce Mon Sep 17 00:00:00 2001 From: greerdv Date: Fri, 11 Jun 2021 22:05:37 +0100 Subject: [PATCH 04/16] avoid some divisions by zero in simd math types --- .../AzCore/Math/Internal/SimdMathCommon_simd.inl | 13 +++++++++++-- .../AzCore/Math/Internal/SimdMathVec2_sse.inl | 2 ++ .../AzCore/Math/Internal/SimdMathVec3_sse.inl | 2 ++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Math/Internal/SimdMathCommon_simd.inl b/Code/Framework/AzCore/AzCore/Math/Internal/SimdMathCommon_simd.inl index 770a8fa104..d06372256d 100644 --- a/Code/Framework/AzCore/AzCore/Math/Internal/SimdMathCommon_simd.inl +++ b/Code/Framework/AzCore/AzCore/Math/Internal/SimdMathCommon_simd.inl @@ -292,8 +292,13 @@ namespace AZ const typename VecType::FloatType cmp2 = VecType::AndNot(cmp0, cmp1); // -1/x + // this step is calculated for all values of x, but only used if x > Sqrt(2) + 1 + // in order to avoid a division by zero, detect if xabs is zero here and replace it with an arbitrary value + // if xabs does equal zero, the value here doesn't matter because the result will be thrown away + typename VecType::FloatType xabsSafe = + VecType::Add(xabs, VecType::And(VecType::CmpEq(xabs, VecType::ZeroFloat()), FastLoadConstant(Simd::g_vec1111))); const typename VecType::FloatType y0 = VecType::And(cmp0, FastLoadConstant(Simd::g_HalfPi)); - typename VecType::FloatType x0 = VecType::Div(FastLoadConstant(Simd::g_vec1111), xabs); + typename VecType::FloatType x0 = VecType::Div(FastLoadConstant(Simd::g_vec1111), xabsSafe); x0 = VecType::Xor(x0, VecType::CastToFloat(FastLoadConstant(Simd::g_negateMask))); const typename VecType::FloatType y1 = VecType::And(cmp2, FastLoadConstant(Simd::g_QuarterPi)); @@ -368,8 +373,12 @@ namespace AZ typename VecType::FloatType offset = VecType::And(x_lt_0, offset1); + // the result of this part of the computation is thrown away if x equals 0, + // but if x does equal 0, it will cause a division by zero + // so replace zero by an arbitrary value here in that case + typename VecType::FloatType xSafe = VecType::Add(x, VecType::And(x_eq_0, FastLoadConstant(Simd::g_vec1111))); const typename VecType::FloatType atan_mask = VecType::Not(VecType::Or(x_eq_0, y_eq_0)); - const typename VecType::FloatType atan_arg = VecType::Div(y, x); + const typename VecType::FloatType atan_arg = VecType::Div(y, xSafe); typename VecType::FloatType atan_result = VecType::Atan(atan_arg); atan_result = VecType::Add(atan_result, offset); atan_result = VecType::AndNot(pio2_mask, atan_result); diff --git a/Code/Framework/AzCore/AzCore/Math/Internal/SimdMathVec2_sse.inl b/Code/Framework/AzCore/AzCore/Math/Internal/SimdMathVec2_sse.inl index 8f35af258c..63e2dca8fd 100644 --- a/Code/Framework/AzCore/AzCore/Math/Internal/SimdMathVec2_sse.inl +++ b/Code/Framework/AzCore/AzCore/Math/Internal/SimdMathVec2_sse.inl @@ -471,6 +471,7 @@ namespace AZ AZ_MATH_INLINE Vec2::FloatType Vec2::Reciprocal(FloatArgType value) { + value = Sse::ReplaceFourth(Sse::ReplaceThird(value, 1.0f), 1.0f); return Sse::Reciprocal(value); } @@ -513,6 +514,7 @@ namespace AZ AZ_MATH_INLINE Vec2::FloatType Vec2::SqrtInv(FloatArgType value) { + value = Sse::ReplaceFourth(Sse::ReplaceThird(value, 1.0f), 1.0f); return Sse::SqrtInv(value); } diff --git a/Code/Framework/AzCore/AzCore/Math/Internal/SimdMathVec3_sse.inl b/Code/Framework/AzCore/AzCore/Math/Internal/SimdMathVec3_sse.inl index 78a0d5db66..75ee2ab7c5 100644 --- a/Code/Framework/AzCore/AzCore/Math/Internal/SimdMathVec3_sse.inl +++ b/Code/Framework/AzCore/AzCore/Math/Internal/SimdMathVec3_sse.inl @@ -507,6 +507,7 @@ namespace AZ AZ_MATH_INLINE Vec3::FloatType Vec3::Reciprocal(FloatArgType value) { + value = Sse::ReplaceFourth(value, 1.0f); return Sse::Reciprocal(value); } @@ -549,6 +550,7 @@ namespace AZ AZ_MATH_INLINE Vec3::FloatType Vec3::SqrtInv(FloatArgType value) { + value = Sse::ReplaceFourth(value, 1.0f); return Sse::SqrtInv(value); } From e6b8dc2ce1cb8571e9197cfec15b73ac795e7166 Mon Sep 17 00:00:00 2001 From: amzn-sj Date: Fri, 11 Jun 2021 14:37:12 -0700 Subject: [PATCH 05/16] Disable AzTest and AzTestRunner in monolithic builds --- Code/Framework/AzTest/CMakeLists.txt | 42 ++++++++++++++------------ Code/Tools/AzTestRunner/CMakeLists.txt | 2 +- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/Code/Framework/AzTest/CMakeLists.txt b/Code/Framework/AzTest/CMakeLists.txt index fe5ec2d0ff..d7b8813639 100644 --- a/Code/Framework/AzTest/CMakeLists.txt +++ b/Code/Framework/AzTest/CMakeLists.txt @@ -8,25 +8,27 @@ # remove or modify any license notices. This file is distributed on an "AS IS" BASIS, # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # - -ly_get_list_relative_pal_filename(pal_dir ${CMAKE_CURRENT_LIST_DIR}/AzTest/Platform/${PAL_PLATFORM_NAME}) -ly_add_target( - NAME AzTest STATIC - NAMESPACE AZ - FILES_CMAKE - AzTest/aztest_files.cmake - ${pal_dir}/platform_${PAL_PLATFORM_NAME_LOWERCASE}_files.cmake - INCLUDE_DIRECTORIES - PUBLIC - . - ${pal_dir} - BUILD_DEPENDENCIES - PUBLIC - 3rdParty::googletest::GMock - 3rdParty::googletest::GTest - 3rdParty::GoogleBenchmark - AZ::AzCore - PLATFORM_INCLUDE_FILES +if(NOT LY_MONOLITHIC_GAME) + ly_get_list_relative_pal_filename(pal_dir ${CMAKE_CURRENT_LIST_DIR}/AzTest/Platform/${PAL_PLATFORM_NAME}) + + ly_add_target( + NAME AzTest STATIC + NAMESPACE AZ + FILES_CMAKE + AzTest/aztest_files.cmake + ${pal_dir}/platform_${PAL_PLATFORM_NAME_LOWERCASE}_files.cmake + INCLUDE_DIRECTORIES + PUBLIC + . + ${pal_dir} + BUILD_DEPENDENCIES + PUBLIC + 3rdParty::googletest::GMock + 3rdParty::googletest::GTest + 3rdParty::GoogleBenchmark + AZ::AzCore + PLATFORM_INCLUDE_FILES ${pal_dir}/platform_${PAL_PLATFORM_NAME_LOWERCASE}.cmake -) + ) +endif() diff --git a/Code/Tools/AzTestRunner/CMakeLists.txt b/Code/Tools/AzTestRunner/CMakeLists.txt index e6dd09e15b..fcff173b01 100644 --- a/Code/Tools/AzTestRunner/CMakeLists.txt +++ b/Code/Tools/AzTestRunner/CMakeLists.txt @@ -13,7 +13,7 @@ ly_get_list_relative_pal_filename(pal_dir ${CMAKE_CURRENT_LIST_DIR}/Platform/${P include(${pal_dir}/platform_traits_${PAL_PLATFORM_NAME_LOWERCASE}.cmake) -if(PAL_TRAIT_AZTESTRUNNER_SUPPORTED) +if(PAL_TRAIT_AZTESTRUNNER_SUPPORTED AND NOT LY_MONOLITHIC_GAME) ly_add_target( NAME AzTestRunner ${PAL_TRAIT_AZTESTRUNNER_LAUNCHER_TYPE} From 21f209311cd50ec2e4cb989bac4333d390cd1b3c Mon Sep 17 00:00:00 2001 From: amzn-sj Date: Fri, 11 Jun 2021 14:54:24 -0700 Subject: [PATCH 06/16] Parameter may be unused --- Code/LauncherUnified/StaticModules.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Code/LauncherUnified/StaticModules.in b/Code/LauncherUnified/StaticModules.in index 03b22b076a..5f2c9d7526 100644 --- a/Code/LauncherUnified/StaticModules.in +++ b/Code/LauncherUnified/StaticModules.in @@ -27,7 +27,7 @@ namespace AZ ${extern_module_declarations} -extern "C" void CreateStaticModules(AZStd::vector& modulesOut) +extern "C" void CreateStaticModules([[maybe_unused]] AZStd::vector& modulesOut) { ${module_invocations} } From ac5196dbf531c66c076f7ffda243a235013d3fda Mon Sep 17 00:00:00 2001 From: amzn-sj Date: Fri, 11 Jun 2021 15:19:28 -0700 Subject: [PATCH 07/16] Revert previous change --- Code/LauncherUnified/StaticModules.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Code/LauncherUnified/StaticModules.in b/Code/LauncherUnified/StaticModules.in index 5f2c9d7526..03b22b076a 100644 --- a/Code/LauncherUnified/StaticModules.in +++ b/Code/LauncherUnified/StaticModules.in @@ -27,7 +27,7 @@ namespace AZ ${extern_module_declarations} -extern "C" void CreateStaticModules([[maybe_unused]] AZStd::vector& modulesOut) +extern "C" void CreateStaticModules(AZStd::vector& modulesOut) { ${module_invocations} } From f6a6614e4b68d3a5dfcdd979e61a619a95d60931 Mon Sep 17 00:00:00 2001 From: amzn-sj Date: Fri, 11 Jun 2021 15:29:46 -0700 Subject: [PATCH 08/16] StaticModules.in must be generated after the gems are enabled --- CMakeLists.txt | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0585089325..4a12e0c50a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -105,24 +105,25 @@ endforeach() # Post-processing ################################################################################ # The following steps have to be done after all targets are registered: -# Defer generation of the StaticModules.inl file which is needed to create the AZ::Module derived class in monolithic -# builds until after all the targets are known -ly_delayed_generate_static_modules_inl() # 1. Add any dependencies registered via ly_enable_gems ly_enable_gems_delayed() -# 2. generate a settings registry .setreg file for all ly_add_project_dependencies() and ly_add_target_dependencies() calls +# 2. Defer generation of the StaticModules.inl file which is needed to create the AZ::Module derived class in monolithic +# builds until after all the targets are known and all the gems are enabled +ly_delayed_generate_static_modules_inl() + +# 3. generate a settings registry .setreg file for all ly_add_project_dependencies() and ly_add_target_dependencies() calls # to provide applications with the filenames of gem modules to load # This must be done before ly_delayed_target_link_libraries() as that inserts BUILD_DEPENDENCIES as MANUALLY_ADDED_DEPENDENCIES # if the build dependency is a MODULE_LIBRARY. That would cause a false load dependency to be generated ly_delayed_generate_settings_registry() -# 3. link targets where the dependency was yet not declared, we need to have the declaration so we do different +# 4. link targets where the dependency was yet not declared, we need to have the declaration so we do different # linking logic depending on the type of target ly_delayed_target_link_libraries() -# 4. generate a registry file for unit testing for platforms that support unit testing +# 5. generate a registry file for unit testing for platforms that support unit testing if(PAL_TRAIT_BUILD_TESTS_SUPPORTED) ly_delayed_generate_unit_test_module_registry() endif() From 469b1a0858ac8bf4827d93f2473cc41067a2282c Mon Sep 17 00:00:00 2001 From: amzn-sj Date: Fri, 11 Jun 2021 16:30:46 -0700 Subject: [PATCH 09/16] Fix monolithic link error involving gem variants which are "aliased" as interface libraries with multiple gem target dependencies --- Code/LauncherUnified/launcher_generator.cmake | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Code/LauncherUnified/launcher_generator.cmake b/Code/LauncherUnified/launcher_generator.cmake index 36cd3c5899..15bbc0fe89 100644 --- a/Code/LauncherUnified/launcher_generator.cmake +++ b/Code/LauncherUnified/launcher_generator.cmake @@ -196,6 +196,16 @@ function(ly_delayed_generate_static_modules_inl) ly_get_gem_load_dependencies(all_game_gem_dependencies ${project_name}.GameLauncher) foreach(game_gem_dependency ${all_game_gem_dependencies}) + # Sometimes, a gem's Client variant may be an interface library + # which dependes on multiple gem targets. The interface libraries + # should be skipped; the real dependencies of the interface will be processed + if(TARGET ${game_gem_dependency}) + get_target_property(target_type ${game_gem_dependency} TYPE) + if(${target_type} STREQUAL "INTERFACE_LIBRARY") + continue() + endif() + endif() + # To match the convention on how gems targets vs gem modules are named, # we remove the ".Static" from the suffix # Replace "." with "_" From a86062bd91eb3184726319cec221304a8976b32a Mon Sep 17 00:00:00 2001 From: amzn-sj Date: Fri, 11 Jun 2021 16:42:09 -0700 Subject: [PATCH 10/16] Skip interface libraries when genrating StaticModules for game server as well --- Code/LauncherUnified/launcher_generator.cmake | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Code/LauncherUnified/launcher_generator.cmake b/Code/LauncherUnified/launcher_generator.cmake index 15bbc0fe89..5fa0f7a0e3 100644 --- a/Code/LauncherUnified/launcher_generator.cmake +++ b/Code/LauncherUnified/launcher_generator.cmake @@ -234,6 +234,14 @@ function(ly_delayed_generate_static_modules_inl) list(APPEND all_server_gem_dependencies ${server_gem_load_dependencies} ${server_gem_dependency}) endforeach() foreach(server_gem_dependency ${all_server_gem_dependencies}) + # Skip interface libraries + if(TARGET ${server_gem_dependency}) + get_target_property(target_type ${server_gem_dependency} TYPE) + if(${target_type} STREQUAL "INTERFACE_LIBRARY") + continue() + endif() + endif() + # Replace "." with "_" string(REPLACE "." "_" server_gem_dependency ${server_gem_dependency}) From f32590d241c12b01c1ff0ed0c2f2c248b0ca60b1 Mon Sep 17 00:00:00 2001 From: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Date: Fri, 11 Jun 2021 16:43:53 -0700 Subject: [PATCH 11/16] LYN-4042 | No UI Indication that Level is Modified (#1277) * Restore the code that display the dirty marker on the Level container. Ensure the dirty marker is cleared on level save. * Moving call one level up to allow usage of the SaveTemplateToString function without clearing the dirty flag. --- .../PrefabEditorEntityOwnershipService.cpp | 17 ++++++++++++----- .../UI/Prefab/LevelRootUiHandler.cpp | 13 +++++++++++-- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp index fa7d5f6e9b..6e96511507 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp @@ -252,13 +252,20 @@ namespace AzToolsFramework } AZStd::string out; - if (m_loaderInterface->SaveTemplateToString(m_rootInstance->GetTemplateId(), out)) + + if (!m_loaderInterface->SaveTemplateToString(m_rootInstance->GetTemplateId(), out)) { - const size_t bytesToWrite = out.size(); - const size_t bytesWritten = stream.Write(bytesToWrite, out.data()); - return bytesWritten == bytesToWrite; + return false; } - return false; + + const size_t bytesToWrite = out.size(); + const size_t bytesWritten = stream.Write(bytesToWrite, out.data()); + if(bytesWritten != bytesToWrite) + { + return false; + } + m_prefabSystemComponent->SetTemplateDirtyFlag(templateId, false); + return true; } void PrefabEditorEntityOwnershipService::CreateNewLevelPrefab(AZStd::string_view filename, const AZStd::string& templateFilename) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Prefab/LevelRootUiHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Prefab/LevelRootUiHandler.cpp index 7915403c0a..d6b7aebddc 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Prefab/LevelRootUiHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Prefab/LevelRootUiHandler.cpp @@ -58,8 +58,17 @@ namespace AzToolsFramework if (!path.empty()) { - infoString = - QObject::tr("(%1)").arg(path.Filename().Native().data()); + QString saveFlag = ""; + auto dirtyOutcome = m_prefabPublicInterface->HasUnsavedChanges(path); + + if (dirtyOutcome.IsSuccess() && dirtyOutcome.GetValue() == true) + { + saveFlag = "*"; + } + + infoString = QObject::tr("(%1%2)") + .arg(path.Filename().Native().data()) + .arg(saveFlag); } return infoString; From 633de3d28bfa8582eebe0d41aff086cfc8995207 Mon Sep 17 00:00:00 2001 From: scottr Date: Fri, 11 Jun 2021 17:58:14 -0700 Subject: [PATCH 12/16] [cpack/stabilization/2106] bootstrap installer theme support (required for changes requested by legal) --- .../Windows/Packaging/Bootstrapper.wxs | 4 + .../Packaging/BootstrapperTheme.wxl.in | 70 +++++++++++++++ .../Packaging/BootstrapperTheme.xml.in | 87 +++++++++++++++++++ .../Platform/Windows/PackagingPostBuild.cmake | 1 + .../Platform/Windows/Packaging_windows.cmake | 22 +++++ .../Windows/platform_windows_files.cmake | 2 + 6 files changed, 186 insertions(+) create mode 100644 cmake/Platform/Windows/Packaging/BootstrapperTheme.wxl.in create mode 100644 cmake/Platform/Windows/Packaging/BootstrapperTheme.xml.in diff --git a/cmake/Platform/Windows/Packaging/Bootstrapper.wxs b/cmake/Platform/Windows/Packaging/Bootstrapper.wxs index 55e8a8cd95..fd8aa68b77 100644 --- a/cmake/Platform/Windows/Packaging/Bootstrapper.wxs +++ b/cmake/Platform/Windows/Packaging/Bootstrapper.wxs @@ -22,6 +22,8 @@ @@ -29,6 +31,8 @@ diff --git a/cmake/Platform/Windows/Packaging/BootstrapperTheme.wxl.in b/cmake/Platform/Windows/Packaging/BootstrapperTheme.wxl.in new file mode 100644 index 0000000000..fe125cfdfe --- /dev/null +++ b/cmake/Platform/Windows/Packaging/BootstrapperTheme.wxl.in @@ -0,0 +1,70 @@ + + + + + [WixBundleName] Setup + [WixBundleName] + Version [WixBundleVersion] + Are you sure you want to cancel? + + + Welcome + +Setup will install [WixBundleName] on your computer. Click install to continue, options to set the install directory or Close to exit. + + [WixBundleName] <a href="#">license terms</a>. + I &agree to the license terms and conditions + &Options + &Install + &Close + + + Setup Options + Install location: + &Browse + &OK + &Cancel + + + Modify Setup + &Repair + &Uninstall + &Close + + + Setup Progress + Processing: + Initializing... + &Cancel + + + Setup Successful + Installation Successfully Completed + Repair Successfully Completed + Uninstall Successfully Completed + &Launch + &Close + + + Setup Failed + Setup Failed + Uninstall Failed + Repair Failed + +One or more issues caused the setup to fail. Please fix the issues and then retry setup. For more information see the <a href="#">log file</a>. + + &Close + + + Setup Help + +/install | /repair | /uninstall | /layout [directory] - installs, repairs, uninstalls or creates a complete local copy of the bundle in directory. Install is the default. + +/passive | /quiet - displays minimal UI with no prompts or displays no UI and no prompts. By default UI and all prompts are displayed. + +/log log.txt - logs to a specific file. By default a log file is created in %TEMP%. + + &Close + diff --git a/cmake/Platform/Windows/Packaging/BootstrapperTheme.xml.in b/cmake/Platform/Windows/Packaging/BootstrapperTheme.xml.in new file mode 100644 index 0000000000..61b338cc13 --- /dev/null +++ b/cmake/Platform/Windows/Packaging/BootstrapperTheme.xml.in @@ -0,0 +1,87 @@ + + + + #(loc.WindowTitle) + + Segoe UI + Segoe UI + Segoe UI + Segoe UI + + + + #(loc.Title) + + + + @WIX_THEME_INSTALL_LICENSE_ELEMENT@ + + #(loc.InstallAcceptCheckbox) + + + + + + + + #(loc.OptionsHeader) + + #(loc.OptionsLocationLabel) + + + + + + + + + + #(loc.ModifyHeader) + + + + + + + + + #(loc.ProgressHeader) + + #(loc.ProgressLabel) + #(loc.OverallProgressPackageText) + + + + + + + + #(loc.SuccessHeader) + #(loc.SuccessInstallHeader) + #(loc.SuccessRepairHeader) + #(loc.SuccessUninstallHeader) + + + + + + + + #(loc.FailureHeader) + #(loc.FailureInstallHeader) + #(loc.FailureUninstallHeader) + #(loc.FailureRepairHeader) + + #(loc.FailureHyperlinkLogText) + + + + + + + + #(loc.HelpHeader) + #(loc.HelpText) + + + diff --git a/cmake/Platform/Windows/PackagingPostBuild.cmake b/cmake/Platform/Windows/PackagingPostBuild.cmake index 89b3efb44b..1dcbedcba7 100644 --- a/cmake/Platform/Windows/PackagingPostBuild.cmake +++ b/cmake/Platform/Windows/PackagingPostBuild.cmake @@ -25,6 +25,7 @@ set(_ext_flags ) set(_addtional_defines + -dCPACK_BOOTSTRAP_THEME_FILE=${CPACK_BINARY_DIR}/BootstrapperTheme -dCPACK_BOOTSTRAP_UPGRADE_GUID=${CPACK_WIX_BOOTSTRAP_UPGRADE_GUID} -dCPACK_DOWNLOAD_SITE=${CPACK_DOWNLOAD_SITE} -dCPACK_LOCAL_INSTALLER_DIR=${_cpack_wix_out_dir} diff --git a/cmake/Platform/Windows/Packaging_windows.cmake b/cmake/Platform/Windows/Packaging_windows.cmake index 2fd281ad51..aec0edeee2 100644 --- a/cmake/Platform/Windows/Packaging_windows.cmake +++ b/cmake/Platform/Windows/Packaging_windows.cmake @@ -92,6 +92,28 @@ set(CPACK_WIX_EXTENSIONS set(_embed_artifacts "yes") if(LY_INSTALLER_DOWNLOAD_URL) + + if(LY_INSTALLER_LICENSE_URL) + set(WIX_THEME_INSTALL_LICENSE_ELEMENT + "#(loc.InstallLicenseLinkText)" + ) + else() + set(WIX_THEME_INSTALL_LICENSE_ELEMENT + "" + ) + endif() + + configure_file( + "${CPACK_SOURCE_DIR}/Platform/Windows/Packaging/BootstrapperTheme.xml.in" + "${CPACK_BINARY_DIR}/BootstrapperTheme.xml" + @ONLY + ) + configure_file( + "${CPACK_SOURCE_DIR}/Platform/Windows/Packaging/BootstrapperTheme.wxl.in" + "${CPACK_BINARY_DIR}/BootstrapperTheme.wxl" + @ONLY + ) + set(_embed_artifacts "no") # the bootstrapper will at the very least need a different upgrade guid diff --git a/cmake/Platform/Windows/platform_windows_files.cmake b/cmake/Platform/Windows/platform_windows_files.cmake index 3ce53fbcea..b3cdc8a4ff 100644 --- a/cmake/Platform/Windows/platform_windows_files.cmake +++ b/cmake/Platform/Windows/platform_windows_files.cmake @@ -26,6 +26,8 @@ set(FILES Packaging_windows.cmake PackagingPostBuild.cmake Packaging/Bootstrapper.wxs + Packaging/BootstrapperTheme.wxl.in + Packaging/BootstrapperTheme.xml.in Packaging/Shortcuts.wxs Packaging/Template.wxs.in ) From 8459cbe5c2e1c75f47720594ffa8976ebf81b465 Mon Sep 17 00:00:00 2001 From: Doug McDiarmid Date: Sat, 12 Jun 2021 01:18:24 -0700 Subject: [PATCH 13/16] Removed the DiffuseGlobalIllumination component from the editor Entity Component list, it was intended to be a Level Component only. Checked for a valid quality level in DiffuseGlobalIlluminationFeatureProcessor::SetQualityLevel. Initialized the quality level to Low in DiffuseGlobalIlluminationComponentConfig. --- .../DiffuseGlobalIlluminationFeatureProcessorInterface.h | 4 +++- .../DiffuseGlobalIlluminationFeatureProcessor.cpp | 6 ++++++ .../DiffuseGlobalIlluminationComponentConfig.h | 2 +- .../EditorDiffuseGlobalIlluminationComponent.cpp | 2 +- 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Include/Atom/Feature/DiffuseGlobalIllumination/DiffuseGlobalIlluminationFeatureProcessorInterface.h b/Gems/Atom/Feature/Common/Code/Include/Atom/Feature/DiffuseGlobalIllumination/DiffuseGlobalIlluminationFeatureProcessorInterface.h index 88faac1728..ce50cea17f 100644 --- a/Gems/Atom/Feature/Common/Code/Include/Atom/Feature/DiffuseGlobalIllumination/DiffuseGlobalIlluminationFeatureProcessorInterface.h +++ b/Gems/Atom/Feature/Common/Code/Include/Atom/Feature/DiffuseGlobalIllumination/DiffuseGlobalIlluminationFeatureProcessorInterface.h @@ -23,7 +23,9 @@ namespace AZ { Low, Medium, - High + High, + + Count }; //! This class provides general features and configuration for the diffuse global illumination environment, diff --git a/Gems/Atom/Feature/Common/Code/Source/DiffuseGlobalIllumination/DiffuseGlobalIlluminationFeatureProcessor.cpp b/Gems/Atom/Feature/Common/Code/Source/DiffuseGlobalIllumination/DiffuseGlobalIlluminationFeatureProcessor.cpp index 5040665456..1c28e18e0e 100644 --- a/Gems/Atom/Feature/Common/Code/Source/DiffuseGlobalIllumination/DiffuseGlobalIlluminationFeatureProcessor.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/DiffuseGlobalIllumination/DiffuseGlobalIlluminationFeatureProcessor.cpp @@ -43,6 +43,12 @@ namespace AZ void DiffuseGlobalIlluminationFeatureProcessor::SetQualityLevel(DiffuseGlobalIlluminationQualityLevel qualityLevel) { + if (qualityLevel >= DiffuseGlobalIlluminationQualityLevel::Count) + { + AZ_Assert(false, "SetQualityLevel called with invalid quality level [%d]", qualityLevel); + return; + } + m_qualityLevel = qualityLevel; UpdatePasses(); diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/DiffuseGlobalIllumination/DiffuseGlobalIlluminationComponentConfig.h b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/DiffuseGlobalIllumination/DiffuseGlobalIlluminationComponentConfig.h index fb99a99e1a..8f1b216af5 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/DiffuseGlobalIllumination/DiffuseGlobalIlluminationComponentConfig.h +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/DiffuseGlobalIllumination/DiffuseGlobalIlluminationComponentConfig.h @@ -29,7 +29,7 @@ namespace AZ static void Reflect(ReflectContext* context); - DiffuseGlobalIlluminationQualityLevel m_qualityLevel; + DiffuseGlobalIlluminationQualityLevel m_qualityLevel = DiffuseGlobalIlluminationQualityLevel::Low; }; } } diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/DiffuseGlobalIllumination/EditorDiffuseGlobalIlluminationComponent.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/DiffuseGlobalIllumination/EditorDiffuseGlobalIlluminationComponent.cpp index bdb5686e89..7df965d831 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/DiffuseGlobalIllumination/EditorDiffuseGlobalIlluminationComponent.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/DiffuseGlobalIllumination/EditorDiffuseGlobalIlluminationComponent.cpp @@ -36,7 +36,7 @@ namespace AZ ->Attribute(Edit::Attributes::Category, "Atom") ->Attribute(AZ::Edit::Attributes::Icon, "Icons/Components/Component_Placeholder.svg") ->Attribute(AZ::Edit::Attributes::ViewportIcon, "Icons/Components/Viewport/Component_Placeholder.png") - ->Attribute(Edit::Attributes::AppearsInAddComponentMenu, AZStd::vector({ AZ_CRC("Level", 0x9aeacc13), AZ_CRC("Game", 0x232b318c) })) + ->Attribute(Edit::Attributes::AppearsInAddComponentMenu, AZStd::vector({ AZ_CRC("Level", 0x9aeacc13) })) ->Attribute(Edit::Attributes::AutoExpand, true) ->Attribute(Edit::Attributes::HelpPageURL, "https://") ; From 59f3813b104f582b51872356d37460f2e6bab274 Mon Sep 17 00:00:00 2001 From: Benjamin Jillich <43751992+amzn-jillich@users.noreply.github.com> Date: Mon, 14 Jun 2021 08:45:00 +0200 Subject: [PATCH 14/16] [LYN-3481] EMotionFX crashes when reloading an Actor (#1184) (#1217) * [LYN-3481] Added new actor instance request and notification buses * [LYN-3481] Actor instance notifies bus about it being created or destroyed * [LYN-3481] Selection lists are now automatically removing destroyed actor instances * [LYN-3481] Morph targets window plugin reinitializing when used actor instance got destroyed * [LYN-3481] Removing the OnDeleteActorInstance() from the emfx event handler/manager and porting the recorder to the new actor instance bus * [LYN-3481] Removed the create actor instance calls from the event handler/manager * [LYN-3481] Fixing automated tests --- .../CommandSystem/Source/SelectionList.cpp | 7 +++ .../CommandSystem/Source/SelectionList.h | 7 ++- .../Code/EMotionFX/Source/ActorInstance.cpp | 13 ++--- .../Code/EMotionFX/Source/ActorInstanceBus.h | 54 +++++++++++++++++++ .../Code/EMotionFX/Source/EventHandler.h | 12 ----- .../Code/EMotionFX/Source/EventManager.cpp | 21 -------- .../Code/EMotionFX/Source/EventManager.h | 10 ---- .../Code/EMotionFX/Source/Recorder.cpp | 10 ++-- .../Code/EMotionFX/Source/Recorder.h | 8 +-- .../MorphTargetsWindowPlugin.cpp | 37 +++++++------ .../MorphTargetsWindowPlugin.h | 6 +++ .../Code/EMotionFX/emotionfx_files.cmake | 1 + 12 files changed, 107 insertions(+), 79 deletions(-) create mode 100644 Gems/EMotionFX/Code/EMotionFX/Source/ActorInstanceBus.h diff --git a/Gems/EMotionFX/Code/EMotionFX/CommandSystem/Source/SelectionList.cpp b/Gems/EMotionFX/Code/EMotionFX/CommandSystem/Source/SelectionList.cpp index a9f8cc063c..9beeebef86 100644 --- a/Gems/EMotionFX/Code/EMotionFX/CommandSystem/Source/SelectionList.cpp +++ b/Gems/EMotionFX/Code/EMotionFX/CommandSystem/Source/SelectionList.cpp @@ -20,10 +20,12 @@ namespace CommandSystem SelectionList::SelectionList() { EMotionFX::ActorNotificationBus::Handler::BusConnect(); + EMotionFX::ActorInstanceNotificationBus::Handler::BusConnect(); } SelectionList::~SelectionList() { + EMotionFX::ActorInstanceNotificationBus::Handler::BusDisconnect(); EMotionFX::ActorNotificationBus::Handler::BusDisconnect(); } @@ -378,4 +380,9 @@ namespace CommandSystem RemoveActor(actor); } + + void SelectionList::OnActorInstanceDestroyed(EMotionFX::ActorInstance* actorInstance) + { + RemoveActorInstance(actorInstance); + } } // namespace CommandSystem diff --git a/Gems/EMotionFX/Code/EMotionFX/CommandSystem/Source/SelectionList.h b/Gems/EMotionFX/Code/EMotionFX/CommandSystem/Source/SelectionList.h index cd2eb3af5d..c85f8e601b 100644 --- a/Gems/EMotionFX/Code/EMotionFX/CommandSystem/Source/SelectionList.h +++ b/Gems/EMotionFX/Code/EMotionFX/CommandSystem/Source/SelectionList.h @@ -15,6 +15,7 @@ #include "CommandSystemConfig.h" #include #include +#include #include #include #include @@ -27,7 +28,8 @@ namespace CommandSystem * specific time stamp in a scene. */ class COMMANDSYSTEM_API SelectionList - : EMotionFX::ActorNotificationBus::Handler + : private EMotionFX::ActorNotificationBus::Handler + , private EMotionFX::ActorInstanceNotificationBus::Handler { MCORE_MEMORYOBJECTCATEGORY(SelectionList, MCore::MCORE_DEFAULT_ALIGNMENT, MEMCATEGORY_COMMANDSYSTEM); @@ -400,6 +402,9 @@ namespace CommandSystem // ActorNotificationBus overrides void OnActorDestroyed(EMotionFX::Actor* actor) override; + // ActorInstanceNotificationBus overrides + void OnActorInstanceDestroyed(EMotionFX::ActorInstance* actorInstance) override; + AZStd::vector mSelectedNodes; /**< Array of selected nodes. */ AZStd::vector mSelectedActors; /**< The selected actors. */ AZStd::vector mSelectedActorInstances; /**< Array of selected actor instances. */ diff --git a/Gems/EMotionFX/Code/EMotionFX/Source/ActorInstance.cpp b/Gems/EMotionFX/Code/EMotionFX/Source/ActorInstance.cpp index ec1b00bda4..8175d15496 100644 --- a/Gems/EMotionFX/Code/EMotionFX/Source/ActorInstance.cpp +++ b/Gems/EMotionFX/Code/EMotionFX/Source/ActorInstance.cpp @@ -34,6 +34,7 @@ #include "NodeGroup.h" #include "Recorder.h" #include "TransformData.h" +#include #include #include @@ -153,20 +154,14 @@ namespace EMotionFX // register it GetActorManager().RegisterActorInstance(this); - // automatically register the actor instance - GetEventManager().OnCreateActorInstance(this); - GetActorManager().GetScheduler()->RecursiveInsertActorInstance(this); + + ActorInstanceNotificationBus::Broadcast(&ActorInstanceNotificationBus::Events::OnActorInstanceCreated, this); } - // the destructor ActorInstance::~ActorInstance() { - // trigger the OnDeleteActorInstance event - GetEventManager().OnDeleteActorInstance(this); - - // remove it from the recording - GetRecorder().RemoveActorInstanceFromRecording(this); + ActorInstanceNotificationBus::Broadcast(&ActorInstanceNotificationBus::Events::OnActorInstanceDestroyed, this); // get rid of the motion system if (mMotionSystem) diff --git a/Gems/EMotionFX/Code/EMotionFX/Source/ActorInstanceBus.h b/Gems/EMotionFX/Code/EMotionFX/Source/ActorInstanceBus.h new file mode 100644 index 0000000000..15b25ce0d3 --- /dev/null +++ b/Gems/EMotionFX/Code/EMotionFX/Source/ActorInstanceBus.h @@ -0,0 +1,54 @@ +/* +* All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or +* its licensors. +* +* For complete copyright and license terms please see the LICENSE at the root of this +* distribution (the "License"). All use of this software is governed by the License, +* or, if provided, by the license below or the license accompanying this file. Do not +* remove or modify any license notices. This file is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* +*/ + +#pragma once + +#include + +namespace EMotionFX +{ + class ActorInstance; + + /** + * EMotion FX Actor Instance Request Bus + * Used for making requests to actor instances. + */ + class ActorInstanceRequests + : public AZ::EBusTraits + { + public: + }; + + using ActorInstanceRequestBus = AZ::EBus; + + /** + * EMotion FX Actor Instance Notification Bus + * Used for monitoring events from actor instances. + */ + class ActorInstanceNotifications + : public AZ::EBusTraits + { + public: + // Enable multi-threaded access by locking primitive using a mutex when connecting handlers to the EBus or executing events. + using MutexType = AZStd::recursive_mutex; + + virtual void OnActorInstanceCreated([[maybe_unused]] ActorInstance* actorInstance) {} + + /** + * Called when any of the actor instances gets destructed. + * @param actorInstance The actorInstance that gets destructed. + */ + virtual void OnActorInstanceDestroyed([[maybe_unused]] ActorInstance* actorInstance) {} + }; + + using ActorInstanceNotificationBus = AZ::EBus; +} // namespace EMotionFX diff --git a/Gems/EMotionFX/Code/EMotionFX/Source/EventHandler.h b/Gems/EMotionFX/Code/EMotionFX/Source/EventHandler.h index 1e6fb86156..2c8ae9d868 100644 --- a/Gems/EMotionFX/Code/EMotionFX/Source/EventHandler.h +++ b/Gems/EMotionFX/Code/EMotionFX/Source/EventHandler.h @@ -51,7 +51,6 @@ namespace EMotionFX EVENT_TYPE_MOTION_INSTANCE_LAST_EVENT = EVENT_TYPE_ON_QUEUE_MOTION_INSTANCE, EVENT_TYPE_ON_DELETE_ACTOR, - EVENT_TYPE_ON_DELETE_ACTOR_INSTANCE, EVENT_TYPE_ON_SIMULATE_PHYSICS, EVENT_TYPE_ON_CUSTOM_EVENT, EVENT_TYPE_ON_DRAW_LINE, @@ -64,7 +63,6 @@ namespace EMotionFX EVENT_TYPE_ON_CREATE_MOTION_INSTANCE, EVENT_TYPE_ON_CREATE_MOTION_SYSTEM, EVENT_TYPE_ON_CREATE_ACTOR, - EVENT_TYPE_ON_CREATE_ACTOR_INSTANCE, EVENT_TYPE_ON_POST_CREATE_ACTOR, EVENT_TYPE_ON_DELETE_ANIM_GRAPH, EVENT_TYPE_ON_DELETE_ANIM_GRAPH_INSTANCE, @@ -298,15 +296,6 @@ namespace EMotionFX */ virtual void OnDeleteActor(Actor* actor) { MCORE_UNUSED(actor); } - /** - * The event that gets triggered once an ActorInstance object is being deleted. - * You could for example use this event to delete any allocations you have done inside the - * custom user data object linked with the ActorInstance object. - * You can get and set this data object with the ActorInstance::GetCustomData() and ActorInstance::SetCustomData(...) methods. - * @param actorInstance The actorInstance that is being deleted. - */ - virtual void OnDeleteActorInstance(ActorInstance* actorInstance) { MCORE_UNUSED(actorInstance); } - virtual void OnSimulatePhysics(float timeDelta) { MCORE_UNUSED(timeDelta); } virtual void OnCustomEvent(uint32 eventType, void* data) { MCORE_UNUSED(eventType); MCORE_UNUSED(data); } @@ -321,7 +310,6 @@ namespace EMotionFX virtual void OnCreateMotionInstance(MotionInstance* motionInstance) { MCORE_UNUSED(motionInstance); } virtual void OnCreateMotionSystem(MotionSystem* motionSystem) { MCORE_UNUSED(motionSystem); } virtual void OnCreateActor(Actor* actor) { MCORE_UNUSED(actor); } - virtual void OnCreateActorInstance(ActorInstance* actorInstance) { MCORE_UNUSED(actorInstance); } virtual void OnPostCreateActor(Actor* actor) { MCORE_UNUSED(actor); } // delete callbacks diff --git a/Gems/EMotionFX/Code/EMotionFX/Source/EventManager.cpp b/Gems/EMotionFX/Code/EMotionFX/Source/EventManager.cpp index bc9843c787..10c64b9433 100644 --- a/Gems/EMotionFX/Code/EMotionFX/Source/EventManager.cpp +++ b/Gems/EMotionFX/Code/EMotionFX/Source/EventManager.cpp @@ -305,16 +305,6 @@ namespace EMotionFX } - void EventManager::OnDeleteActorInstance(ActorInstance* actorInstance) - { - const EventHandlerVector& eventHandlers = m_eventHandlersByEventType[EVENT_TYPE_ON_DELETE_ACTOR_INSTANCE]; - for (EventHandler* eventHandler : eventHandlers) - { - eventHandler->OnDeleteActorInstance(actorInstance); - } - } - - // draw a debug triangle void EventManager::OnDrawTriangle(const AZ::Vector3& posA, const AZ::Vector3& posB, const AZ::Vector3& posC, const AZ::Vector3& normalA, const AZ::Vector3& normalB, const AZ::Vector3& normalC, uint32 color) { @@ -670,17 +660,6 @@ namespace EMotionFX } - // create an actor instance - void EventManager::OnCreateActorInstance(ActorInstance* actorInstance) - { - const EventHandlerVector& eventHandlers = m_eventHandlersByEventType[EVENT_TYPE_ON_CREATE_ACTOR_INSTANCE]; - for (EventHandler* eventHandler : eventHandlers) - { - eventHandler->OnCreateActorInstance(actorInstance); - } - } - - // on post create actor void EventManager::OnPostCreateActor(Actor* actor) { diff --git a/Gems/EMotionFX/Code/EMotionFX/Source/EventManager.h b/Gems/EMotionFX/Code/EMotionFX/Source/EventManager.h index 81767d5a66..f12d1de8cb 100644 --- a/Gems/EMotionFX/Code/EMotionFX/Source/EventManager.h +++ b/Gems/EMotionFX/Code/EMotionFX/Source/EventManager.h @@ -286,15 +286,6 @@ namespace EMotionFX */ void OnDeleteActor(Actor* actor); - /** - * The event that gets triggered once an ActorInstance object is being deleted. - * You could for example use this event to delete any allocations you have done inside the - * custom user data object linked with the ActorInstance object. - * You can get and set this data object with the ActorInstance::GetCustomData() and ActorInstance::SetCustomData(...) methods. - * @param actorInstance The actorInstance that is being deleted. - */ - void OnDeleteActorInstance(ActorInstance* actorInstance); - void OnSimulatePhysics(float timeDelta); void OnCustomEvent(uint32 eventType, void* data); void OnDrawTriangle(const AZ::Vector3& posA, const AZ::Vector3& posB, const AZ::Vector3& posC, const AZ::Vector3& normalA, const AZ::Vector3& normalB, const AZ::Vector3& normalC, uint32 color); @@ -343,7 +334,6 @@ namespace EMotionFX void OnCreateMotionInstance(MotionInstance* motionInstance); void OnCreateMotionSystem(MotionSystem* motionSystem); void OnCreateActor(Actor* actor); - void OnCreateActorInstance(ActorInstance* actorInstance); void OnPostCreateActor(Actor* actor); // delete callbacks diff --git a/Gems/EMotionFX/Code/EMotionFX/Source/Recorder.cpp b/Gems/EMotionFX/Code/EMotionFX/Source/Recorder.cpp index d12970bc04..8670851334 100644 --- a/Gems/EMotionFX/Code/EMotionFX/Source/Recorder.cpp +++ b/Gems/EMotionFX/Code/EMotionFX/Source/Recorder.cpp @@ -106,16 +106,12 @@ namespace EMotionFX mCurrentPlayTime = 0.0f; mObjects.SetMemoryCategory(EMFX_MEMCATEGORY_RECORDER); - - GetEMotionFX().GetEventManager()->AddEventHandler(this); + EMotionFX::ActorInstanceNotificationBus::Handler::BusConnect(); } Recorder::~Recorder() { - if (EventManager* eventManager = GetEMotionFX().GetEventManager()) - { - eventManager->RemoveEventHandler(this); - } + EMotionFX::ActorInstanceNotificationBus::Handler::BusDisconnect(); Clear(); } @@ -1448,7 +1444,7 @@ namespace EMotionFX Unlock(); } - void Recorder::OnDeleteActorInstance(ActorInstance* actorInstance) + void Recorder::OnActorInstanceDestroyed(EMotionFX::ActorInstance* actorInstance) { // Actor instances created by actor components do not use the command system and don't call a ClearRecorder command. // Thus, these actor instances will have to be removed from the recorder to avoid dangling data. diff --git a/Gems/EMotionFX/Code/EMotionFX/Source/Recorder.h b/Gems/EMotionFX/Code/EMotionFX/Source/Recorder.h index 157ef2c88b..33ab3ce35d 100644 --- a/Gems/EMotionFX/Code/EMotionFX/Source/Recorder.h +++ b/Gems/EMotionFX/Code/EMotionFX/Source/Recorder.h @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -47,7 +48,7 @@ namespace EMotionFX class EMFX_API Recorder : public BaseObject - , public EventHandler + , private EMotionFX::ActorInstanceNotificationBus::Handler { public: AZ_CLASS_ALLOCATOR_DECL @@ -319,9 +320,8 @@ namespace EMotionFX void RemoveActorInstanceFromRecording(ActorInstance* actorInstance); void RemoveAnimGraphFromRecording(AnimGraph* animGraph); - // EventHandler overrides - const AZStd::vector GetHandledEventTypes() const override { return {EMotionFX::EVENT_TYPE_ON_DELETE_ACTOR_INSTANCE}; } - void OnDeleteActorInstance(ActorInstance* actorInstance) override; + // ActorInstanceNotificationBus overrides + void OnActorInstanceDestroyed(EMotionFX::ActorInstance* actorInstance) override; void SampleAndApplyTransforms(float timeInSeconds, ActorInstance* actorInstance) const; void SampleAndApplyMainTransform(float timeInSeconds, ActorInstance* actorInstance) const; diff --git a/Gems/EMotionFX/Code/EMotionFX/Tools/EMotionStudio/Plugins/StandardPlugins/Source/MorphTargetsWindow/MorphTargetsWindowPlugin.cpp b/Gems/EMotionFX/Code/EMotionFX/Tools/EMotionStudio/Plugins/StandardPlugins/Source/MorphTargetsWindow/MorphTargetsWindowPlugin.cpp index 80bbb24928..a0b018d999 100644 --- a/Gems/EMotionFX/Code/EMotionFX/Tools/EMotionStudio/Plugins/StandardPlugins/Source/MorphTargetsWindow/MorphTargetsWindowPlugin.cpp +++ b/Gems/EMotionFX/Code/EMotionFX/Tools/EMotionStudio/Plugins/StandardPlugins/Source/MorphTargetsWindow/MorphTargetsWindowPlugin.cpp @@ -17,25 +17,26 @@ #include "../../../../EMStudioSDK/Source/EMStudioCore.h" #include #include +#include #include #include "../../../../EMStudioSDK/Source/EMStudioManager.h" - namespace EMStudio { - // constructor MorphTargetsWindowPlugin::MorphTargetsWindowPlugin() : EMStudio::DockWidgetPlugin() { - mDialogStack = nullptr; - mCurrentActorInstance = nullptr; + mDialogStack = nullptr; + mCurrentActorInstance = nullptr; mStaticTextWidget = nullptr; + + EMotionFX::ActorInstanceNotificationBus::Handler::BusConnect(); } - - // destructor MorphTargetsWindowPlugin::~MorphTargetsWindowPlugin() { + EMotionFX::ActorInstanceNotificationBus::Handler::BusDisconnect(); + // unregister the command callbacks and get rid of the memory for (auto callback : m_callbacks) { @@ -110,14 +111,16 @@ namespace EMStudio mMorphTargetGroups.clear(); } - // reinit the morph target dialog, e.g. if selection changes void MorphTargetsWindowPlugin::ReInit(bool forceReInit) { - // get the selected actorinstance - const CommandSystem::SelectionList& selection = GetCommandManager()->GetCurrentSelection(); - EMotionFX::ActorInstance* actorInstance = selection.GetSingleActorInstance(); + const CommandSystem::SelectionList& selection = GetCommandManager()->GetCurrentSelection(); + EMotionFX::ActorInstance* actorInstance = selection.GetSingleActorInstance(); + ReInit(actorInstance, forceReInit); + } + void MorphTargetsWindowPlugin::ReInit(EMotionFX::ActorInstance* actorInstance, bool forceReInit) + { // show hint if no/multiple actor instances is/are selected if (actorInstance == nullptr) { @@ -135,10 +138,7 @@ namespace EMStudio return; } - // get our selected actor instance and the corresponding actor - EMotionFX::Actor* actor = actorInstance->GetActor(); - - // only reinit the morph targets if actorinstance changed + // only reinit the morph targets if actor instance changed if (mCurrentActorInstance != actorInstance || forceReInit) { // set the current actor instance in any case @@ -150,7 +150,7 @@ namespace EMStudio AZStd::vector phonemeInstances; AZStd::vector defaultMorphTargetInstances; - // get the morph target setup + EMotionFX::Actor* actor = actorInstance->GetActor(); EMotionFX::MorphSetup* morphSetup = actor->GetMorphSetup(actorInstance->GetLODLevel()); if (morphSetup == nullptr) { @@ -278,6 +278,13 @@ namespace EMStudio } } + void MorphTargetsWindowPlugin::OnActorInstanceDestroyed(EMotionFX::ActorInstance* actorInstance) + { + if (mCurrentActorInstance == actorInstance) + { + ReInit(/*actorInstance=*/nullptr); + } + } //----------------------------------------------------------------------------------------- // Command callbacks diff --git a/Gems/EMotionFX/Code/EMotionFX/Tools/EMotionStudio/Plugins/StandardPlugins/Source/MorphTargetsWindow/MorphTargetsWindowPlugin.h b/Gems/EMotionFX/Code/EMotionFX/Tools/EMotionStudio/Plugins/StandardPlugins/Source/MorphTargetsWindow/MorphTargetsWindowPlugin.h index ec64af2789..1f5abba310 100644 --- a/Gems/EMotionFX/Code/EMotionFX/Tools/EMotionStudio/Plugins/StandardPlugins/Source/MorphTargetsWindow/MorphTargetsWindowPlugin.h +++ b/Gems/EMotionFX/Code/EMotionFX/Tools/EMotionStudio/Plugins/StandardPlugins/Source/MorphTargetsWindow/MorphTargetsWindowPlugin.h @@ -16,6 +16,7 @@ #include #include "../../../../EMStudioSDK/Source/DockWidgetPlugin.h" #include +#include #include "MorphTargetGroupWidget.h" #include #include @@ -26,6 +27,7 @@ namespace EMStudio { class MorphTargetsWindowPlugin : public EMStudio::DockWidgetPlugin + , private EMotionFX::ActorInstanceNotificationBus::Handler { Q_OBJECT MCORE_MEMORYOBJECTCATEGORY(MorphTargetsWindowPlugin, MCore::MCORE_DEFAULT_ALIGNMENT, MEMCATEGORY_STANDARDPLUGINS); @@ -54,6 +56,7 @@ namespace EMStudio EMStudioPlugin* Clone() override; // update the morph targets window based on the current selection + void ReInit(EMotionFX::ActorInstance* actorInstance, bool forceReInit = false); void ReInit(bool forceReInit = false); // clear all widgets from the window @@ -70,6 +73,9 @@ namespace EMStudio void WindowReInit(bool visible); private: + // ActorInstanceNotificationBus overrides + void OnActorInstanceDestroyed(EMotionFX::ActorInstance* actorInstance) override; + // declare the callbacks MCORE_DEFINECOMMANDCALLBACK(CommandSelectCallback); MCORE_DEFINECOMMANDCALLBACK(CommandUnselectCallback); diff --git a/Gems/EMotionFX/Code/EMotionFX/emotionfx_files.cmake b/Gems/EMotionFX/Code/EMotionFX/emotionfx_files.cmake index 9f5fe617d4..9743ab5f15 100644 --- a/Gems/EMotionFX/Code/EMotionFX/emotionfx_files.cmake +++ b/Gems/EMotionFX/Code/EMotionFX/emotionfx_files.cmake @@ -15,6 +15,7 @@ set(FILES Source/ActorBus.h Source/ActorInstance.cpp Source/ActorInstance.h + Source/ActorInstanceBus.h Source/ActorManager.cpp Source/ActorManager.h Source/ActorUpdateScheduler.h From f8cf4ba08767ecb02b82b49b48786cf803fd1a0e Mon Sep 17 00:00:00 2001 From: Benjamin Jillich <43751992+amzn-jillich@users.noreply.github.com> Date: Mon, 14 Jun 2021 17:34:29 +0200 Subject: [PATCH 15/16] [LYN-3416] Animation: Attachment component does not update it's location automatically (#1305) --- .../Code/Source/Animation/EditorAttachmentComponent.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Animation/EditorAttachmentComponent.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Animation/EditorAttachmentComponent.cpp index f14340b4c9..e7f2a98a71 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Animation/EditorAttachmentComponent.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Animation/EditorAttachmentComponent.cpp @@ -118,8 +118,7 @@ namespace AZ void EditorAttachmentComponent::Activate() { Base::Activate(); - m_boneFollower.Activate(GetEntity(), CreateAttachmentConfiguration(), - false); // Entity's don't animate in Editor + m_boneFollower.Activate(GetEntity(), CreateAttachmentConfiguration(), /*targetCanAnimate=*/true); } void EditorAttachmentComponent::Deactivate() From 6f61454be4fbe3bd9b32c16773ec50b41c9e9911 Mon Sep 17 00:00:00 2001 From: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com> Date: Mon, 14 Jun 2021 14:20:39 -0500 Subject: [PATCH 16/16] Added support for remapping entity refs across slice instances (#1284) * Fixed memory assert on shutdown. Cleaned up the entity pointers on serialization, so that they no longer leak themselves, their asset references, or anything else within them. * Added support for remapping entity refs across slice instances. Slice instances can have components with entity references that have been modified to reference entities in other slice instances, or even in the parent. This change detects and remaps those references so that they work correctly. --- .../SerializeContextTools/SliceConverter.cpp | 192 ++++++++++++++---- .../SerializeContextTools/SliceConverter.h | 46 +++-- 2 files changed, 185 insertions(+), 53 deletions(-) diff --git a/Code/Tools/SerializeContextTools/SliceConverter.cpp b/Code/Tools/SerializeContextTools/SliceConverter.cpp index 9fba060960..e7dc49d5b3 100644 --- a/Code/Tools/SerializeContextTools/SliceConverter.cpp +++ b/Code/Tools/SerializeContextTools/SliceConverter.cpp @@ -177,9 +177,10 @@ namespace AZ AZ::Entity* rootEntity = reinterpret_cast(classPtr); bool convertResult = ConvertSliceToPrefab(context, outputPath, isDryRun, rootEntity); - // Clear out the references to any nested slices so that the nested assets get unloaded correctly at the end of - // the conversion. - ClearSliceAssetReferences(rootEntity); + + // Delete the root entity pointer. Otherwise, it will leak itself along with all of the slice asset references held + // within it. + delete rootEntity; return convertResult; }; @@ -229,8 +230,12 @@ namespace AZ return false; } - // Get all of the entities from the slice. + // Get all of the entities from the slice. We're taking ownership of them, so we also remove them from the slice component + // without deleting them. + constexpr bool deleteEntities = false; + constexpr bool removeEmptyInstances = true; SliceComponent::EntityList sliceEntities = sliceComponent->GetNewEntities(); + sliceComponent->RemoveAllEntities(deleteEntities, removeEmptyInstances); AZ_Printf("Convert-Slice", " Slice contains %zu entities.\n", sliceEntities.size()); // Create the Prefab with the entities from the slice. @@ -273,6 +278,12 @@ namespace AZ } } + // Save off a mapping of the slice's metadata entity ID as well, even though we never converted the entity itself. + // This will help us better detect entity ID mapping errors for nested slice instances. + AZ::Entity* metadataEntity = sliceComponent->GetMetadataEntity(); + constexpr bool isMetadataEntity = true; + m_aliasIdMapper.emplace(metadataEntity->GetId(), SliceEntityMappingInfo(templateId, "MetadataEntity", isMetadataEntity)); + // Update the prefab template with the fixed-up data in our prefab instance. AzToolsFramework::Prefab::PrefabDom prefabDom; bool storeResult = AzToolsFramework::Prefab::PrefabDomUtils::StoreInstanceInPrefabDom(*sourceInstance, prefabDom); @@ -402,6 +413,21 @@ namespace AZ "Convert-Slice", " Attaching %zu instances of nested slice '%s'.\n", instances.size(), nestedPrefabPath.Native().c_str()); + // Before processing any further, save off all the known entity IDs from all the instances and how they map back to + // the base nested prefab that they've come from (i.e. this one). As we proceed up the chain of nesting, this will + // build out a hierarchical list of owning instances for each entity that we can trace upwards to know where to add + // the entity into our nested prefab instance. + // This step needs to occur *before* converting the instances themselves, because while converting instances, they + // might have entity ID references that point to other instances. By having the full instance entity ID map in place + // before conversion, we'll be able to fix them up appropriately. + + for (auto& instance : instances) + { + AZStd::string instanceAlias = GetInstanceAlias(instance); + UpdateSliceEntityInstanceMappings(instance.GetEntityIdToBaseMap(), instanceAlias); + } + + // Now that we have all the entity ID mappings, convert all the instances. for (auto& instance : instances) { bool instanceConvertResult = ConvertSliceInstance(instance, sliceAsset, nestedTemplate, sourceInstance); @@ -415,6 +441,28 @@ namespace AZ return true; } + AZStd::string SliceConverter::GetInstanceAlias(const AZ::SliceComponent::SliceInstance& instance) + { + // When creating the new instance, we would like to have deterministic instance aliases. Prefabs that depend on this one + // will have patches that reference the alias, so if we reconvert this slice a second time, we would like it to produce + // the same results. To get a deterministic and unique alias, we rely on the slice instance. The slice instance contains + // a map of slice entity IDs to unique instance entity IDs. We'll just consistently use the first entry in the map as the + // unique instance ID. + AZStd::string instanceAlias; + auto entityIdMap = instance.GetEntityIdMap(); + if (!entityIdMap.empty()) + { + instanceAlias = AZStd::string::format("Instance_%s", entityIdMap.begin()->second.ToString().c_str()); + } + else + { + AZ_Error("Convert-Slice", false, " Couldn't create deterministic instance alias."); + instanceAlias = AZStd::string::format("Instance_%s", AZ::Entity::MakeId().ToString().c_str()); + } + return instanceAlias; + } + + bool SliceConverter::ConvertSliceInstance( AZ::SliceComponent::SliceInstance& instance, AZ::Data::Asset& sliceAsset, @@ -438,27 +486,7 @@ namespace AZ auto instanceToTemplateInterface = AZ::Interface::Get(); auto prefabSystemComponentInterface = AZ::Interface::Get(); - // When creating the new instance, we would like to have deterministic instance aliases. Prefabs that depend on this one - // will have patches that reference the alias, so if we reconvert this slice a second time, we would like it to produce - // the same results. To get a deterministic and unique alias, we rely on the slice instance. The slice instance contains - // a map of slice entity IDs to unique instance entity IDs. We'll just consistently use the first entry in the map as the - // unique instance ID. - AZStd::string instanceAlias; - auto entityIdMap = instance.GetEntityIdMap(); - if (!entityIdMap.empty()) - { - instanceAlias = AZStd::string::format("Instance_%s", entityIdMap.begin()->second.ToString().c_str()); - } - else - { - instanceAlias = AZStd::string::format("Instance_%s", AZ::Entity::MakeId().ToString().c_str()); - } - - // Before processing any further, save off all the known entity IDs from this instance and how they map back to the base - // nested prefab that they've come from (i.e. this one). As we proceed up the chain of nesting, this will build out a - // hierarchical list of owning instances for each entity that we can trace upwards to know where to add the entity into - // our nested prefab instance. - UpdateSliceEntityInstanceMappings(instance.GetEntityIdToBaseMap(), instanceAlias); + AZStd::string instanceAlias = GetInstanceAlias(instance); // Create a new unmodified prefab Instance for the nested slice instance. auto nestedInstance = AZStd::make_unique(); @@ -619,6 +647,10 @@ namespace AZ SetParentEntity(containerEntity->get(), topLevelInstance->GetContainerEntityId(), onlySetIfInvalid); } + // After doing all of the above, run through entity references in any of the patched entities, and fix up the entity IDs to + // match the new ones in our prefabs. + RemapIdReferences(m_aliasIdMapper, topLevelInstance, nestedInstance.get(), instantiated, dependentSlice->GetSerializeContext()); + // Add the nested instance itself to the top-level prefab. To do this, we need to add it to our top-level instance, // create a patch out of it, and patch the top-level prefab template. @@ -750,17 +782,6 @@ namespace AZ AZ_Error("Convert-Slice", disconnected, "Asset Processor failed to disconnect successfully."); } - void SliceConverter::ClearSliceAssetReferences(AZ::Entity* rootEntity) - { - SliceComponent* sliceComponent = AZ::EntityUtils::FindFirstDerivedComponent(rootEntity); - // Make a copy of the slice list and remove all of them from the loaded component. - AZ::SliceComponent::SliceList slices = sliceComponent->GetSlices(); - for (auto& slice : slices) - { - sliceComponent->RemoveSlice(&slice); - } - } - void SliceConverter::UpdateSliceEntityInstanceMappings( const AZ::SliceComponent::EntityIdToEntityIdMap& sliceEntityIdMap, const AZStd::string& currentInstanceAlias) { @@ -789,9 +810,108 @@ namespace AZ AZ_Assert(oldId == newId, "The same entity instance ID has unexpectedly appeared twice in the same nested prefab."); } } + else + { + AZ_Warning("Convert-Slice", false, " Couldn't find an entity ID conversion for %s.", oldId.ToString().c_str()); + } } } + void SliceConverter::RemapIdReferences( + const AZStd::unordered_map& idMapper, + AzToolsFramework::Prefab::Instance* topLevelInstance, + AzToolsFramework::Prefab::Instance* nestedInstance, + SliceComponent::InstantiatedContainer* instantiatedEntities, + SerializeContext* context) + { + // Given a set of instantiated entities, run through all of them, look for entity references, and replace the entity IDs with + // new ones that match up with our prefabs. + + IdUtils::Remapper::ReplaceIdsAndIdRefs( + instantiatedEntities, + [idMapper, &topLevelInstance, &nestedInstance]( + const EntityId& sourceId, bool isEntityId, [[maybe_unused]] const AZStd::function& idGenerator) -> EntityId + { + EntityId newId = sourceId; + + // Only convert valid entity references. Actual entity IDs have already been taken care of elsewhere, so ignore them. + if (!isEntityId && sourceId.IsValid()) + { + auto entityEntry = idMapper.find(sourceId); + + // Since we've already remapped transform hierarchies to include container entities, it's possible that our entity + // reference is pointing to a container, which means it won't be in our slice mapping table. In that case, just + // return it as-is. + if (entityEntry == idMapper.end()) + { + return sourceId; + } + + // We've got a slice->prefab mapping entry, so now we need to use it. + auto& mappingStruct = entityEntry->second; + + if (mappingStruct.m_nestedInstanceAliases.empty()) + { + // If we don't have a chain of nested instance aliases, then this entity reference is either within the + // current nested instance or it's pointing to an entity in the top-level instance. We'll try them both + // to look for a match. + + EntityId prefabId = nestedInstance->GetEntityId(mappingStruct.m_entityAlias); + if (!prefabId.IsValid()) + { + prefabId = topLevelInstance->GetEntityId(mappingStruct.m_entityAlias); + } + + if (prefabId.IsValid()) + { + newId = prefabId; + } + else + { + AZ_Error("Convert-Slice", false, " Couldn't find source ID %s", sourceId.ToString().c_str()); + } + } + else + { + // We *do* have a chain of nested instance aliases. This chain could either be relative to the nested instance + // or the top-level instance. We can tell which one it is by which one can find the first nested instance + // alias. + + AzToolsFramework::Prefab::Instance* entityInstance = nestedInstance; + auto it = mappingStruct.m_nestedInstanceAliases.rbegin(); + if (!entityInstance->FindNestedInstance(*it).has_value()) + { + entityInstance = topLevelInstance; + } + + // Now that we've got a starting point, iterate through the chain of nested instance aliases to find the + // correct instance to get the entity ID for. We have to go from slice IDs -> entity aliases -> entity IDs + // because prefab instance creation can change some of our entity IDs along the way. + for (; it != mappingStruct.m_nestedInstanceAliases.rend(); it++) + { + auto foundInstance = entityInstance->FindNestedInstance(*it); + if (foundInstance.has_value()) + { + entityInstance = &(foundInstance->get()); + } + else + { + AZ_Assert(false, "Couldn't find nested instance %s", it->c_str()); + } + } + + EntityId prefabId = entityInstance->GetEntityId(mappingStruct.m_entityAlias); + if (prefabId.IsValid()) + { + newId = prefabId; + } + } + } + + return newId; + }, + context); + } } // namespace SerializeContextTools } // namespace AZ diff --git a/Code/Tools/SerializeContextTools/SliceConverter.h b/Code/Tools/SerializeContextTools/SliceConverter.h index ee0bb0a539..31c8306477 100644 --- a/Code/Tools/SerializeContextTools/SliceConverter.h +++ b/Code/Tools/SerializeContextTools/SliceConverter.h @@ -42,6 +42,28 @@ namespace AZ bool ConvertSliceFiles(Application& application); private: + // When converting slice entities, especially for nested slices, we need to keep track of the original + // entity ID, the entity alias it uses in the prefab, and which template and nested instance path it maps to. + // As we encounter each instanced entity ID, we can look it up in this structure and use this to determine how to properly + // add it to the correct place in the hierarchy. + struct SliceEntityMappingInfo + { + SliceEntityMappingInfo( + AzToolsFramework::Prefab::TemplateId templateId, + AzToolsFramework::Prefab::EntityAlias entityAlias, + bool isMetadataEntity = false) + : m_templateId(templateId) + , m_entityAlias(entityAlias) + , m_isMetadataEntity(isMetadataEntity) + { + } + + AzToolsFramework::Prefab::TemplateId m_templateId; + AzToolsFramework::Prefab::EntityAlias m_entityAlias; + AZStd::vector m_nestedInstanceAliases; + bool m_isMetadataEntity{ false }; + }; + bool ConnectToAssetProcessor(); void DisconnectFromAssetProcessor(); @@ -58,27 +80,17 @@ namespace AZ void SetParentEntity(const AZ::Entity& entity, const AZ::EntityId& parentId, bool onlySetIfInvalid); void PrintPrefab(AzToolsFramework::Prefab::TemplateId templateId); bool SavePrefab(AZ::IO::PathView outputPath, AzToolsFramework::Prefab::TemplateId templateId); - void ClearSliceAssetReferences(AZ::Entity* rootEntity); void UpdateSliceEntityInstanceMappings( const AZ::SliceComponent::EntityIdToEntityIdMap& sliceEntityIdMap, const AZStd::string& currentInstanceAlias); + AZStd::string GetInstanceAlias(const AZ::SliceComponent::SliceInstance& instance); - // When converting slice entities, especially for nested slices, we need to keep track of the original - // entity ID, the entity alias it uses in the prefab, and which template and nested instance path it maps to. - // As we encounter each instanced entity ID, we can look it up in this structure and use this to determine how to properly - // add it to the correct place in the hierarchy. - struct SliceEntityMappingInfo - { - SliceEntityMappingInfo(AzToolsFramework::Prefab::TemplateId templateId, AzToolsFramework::Prefab::EntityAlias entityAlias) - : m_templateId(templateId) - , m_entityAlias(entityAlias) - { - } - - AzToolsFramework::Prefab::TemplateId m_templateId; - AzToolsFramework::Prefab::EntityAlias m_entityAlias; - AZStd::vector m_nestedInstanceAliases; - }; + void RemapIdReferences( + const AZStd::unordered_map& idMapper, + AzToolsFramework::Prefab::Instance* topLevelInstance, + AzToolsFramework::Prefab::Instance* nestedInstance, + SliceComponent::InstantiatedContainer* instantiatedEntities, + SerializeContext* context); // Track all of the entity IDs created and associate them with enough conversion information to know how to place the // entities in the correct place in the prefab hierarchy and fix up parent entity ID mappings to work with the nested