From 92ef82f9331dee683f3d4df7869c27d913321420 Mon Sep 17 00:00:00 2001 From: pereslav Date: Mon, 10 May 2021 19:52:23 +0100 Subject: [PATCH 01/21] Added handling parented net entities --- .../EntityReplicationManager.cpp | 6 ++--- .../NetworkEntity/NetworkEntityManager.cpp | 26 ++++++++++++++++++- .../Pipeline/NetworkPrefabProcessor.cpp | 4 +++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/Gems/Multiplayer/Code/Source/NetworkEntity/EntityReplication/EntityReplicationManager.cpp b/Gems/Multiplayer/Code/Source/NetworkEntity/EntityReplication/EntityReplicationManager.cpp index 74bdcd5cf0..64eb3fcc6a 100644 --- a/Gems/Multiplayer/Code/Source/NetworkEntity/EntityReplication/EntityReplicationManager.cpp +++ b/Gems/Multiplayer/Code/Source/NetworkEntity/EntityReplication/EntityReplicationManager.cpp @@ -550,10 +550,10 @@ namespace Multiplayer { replicatorEntity = entityList[0]; } - - AZ_Assert(replicatorEntity != nullptr, "Failed to create entity from prefab %s", prefabEntityId.m_prefabName.GetCStr()); - if (replicatorEntity == nullptr) + else { + AZ_Assert(false, "There should be exactly one created entity out of prefab %s, index %d. Got: %d", + prefabEntityId.m_prefabName.GetCStr(), prefabEntityId.m_entityOffset, entityList.size()); return false; } } diff --git a/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkEntityManager.cpp b/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkEntityManager.cpp index ce55f66caa..3bcd613d8b 100644 --- a/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkEntityManager.cpp +++ b/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkEntityManager.cpp @@ -334,15 +334,39 @@ namespace Multiplayer const AzFramework::Spawnable::EntityList& entities = spawnable.GetEntities(); size_t entitiesSize = entities.size(); + using EntityIdMap = AZStd::unordered_map; + EntityIdMap originalToCloneIdMap; + for (size_t i = 0; i < entitiesSize; ++i) { - AZ::Entity* clone = serializeContext->CloneObject(entities[i].get()); + AZ::Entity* originalEntity = entities[i].get(); + AZ::Entity* clone = serializeContext->CloneObject(originalEntity); AZ_Assert(clone != nullptr, "Failed to clone spawnable entity."); + clone->SetId(AZ::Entity::MakeId()); + originalToCloneIdMap[originalEntity->GetId()] = clone->GetId(); + NetBindComponent* netBindComponent = clone->FindComponent(); if (netBindComponent != nullptr) { + // Update TransformComponent parent Id. It is guaranteed for the entities array to be sorted from parent->child here. + auto* transformComponent = clone->FindComponent(); + AZ::EntityId parentId = transformComponent->GetParentId(); + if (parentId.IsValid()) + { + auto it = originalToCloneIdMap.find(parentId); + if (it != originalToCloneIdMap.end()) + { + transformComponent->SetParentRelative(it->second); + } + else + { + AZ_Warning("NetworkEntityManager", false, "Entity %s doesn't have the parent entity %s present in network.spawnable", + clone->GetName().c_str(), parentId.ToString().data()); + } + } + PrefabEntityId prefabEntityId; prefabEntityId.m_prefabName = m_networkPrefabLibrary.GetPrefabNameFromAssetId(spawnable.GetId()); prefabEntityId.m_entityOffset = aznumeric_cast(i); diff --git a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp index 4962d16fb4..56201bd5fd 100644 --- a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp +++ b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp @@ -59,6 +59,7 @@ namespace Multiplayer return result; } + void NetworkPrefabProcessor::ProcessPrefab(PrefabProcessorContext& context, AZStd::string_view prefabName, PrefabDom& prefab) { using namespace AzToolsFramework::Prefab; @@ -175,6 +176,9 @@ namespace Multiplayer (*it)->InvalidateDependencies(); (*it)->EvaluateDependencies(); } + + SpawnableUtils::SortEntitiesByTransformHierarchy(*networkSpawnable); + context.GetProcessedObjects().push_back(AZStd::move(object)); } else From 9775822778ec2cf8003ea86f455906f75bce1c14 Mon Sep 17 00:00:00 2001 From: scottr Date: Tue, 11 May 2021 16:09:16 -0700 Subject: [PATCH 02/21] [cpack_installer] remove wxs file ext from lfs filter --- .gitattributes | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitattributes b/.gitattributes index 1755def66a..55b43e4ba7 100644 --- a/.gitattributes +++ b/.gitattributes @@ -115,5 +115,4 @@ *.wav filter=lfs diff=lfs merge=lfs -text *.webm filter=lfs diff=lfs merge=lfs -text *.wem filter=lfs diff=lfs merge=lfs -text -*.wxs filter=lfs diff=lfs merge=lfs -text *.zip filter=lfs diff=lfs merge=lfs -text From c777e2e35301cd0054fe39e8fdccb5e632d48a21 Mon Sep 17 00:00:00 2001 From: scottr Date: Tue, 11 May 2021 18:07:19 -0700 Subject: [PATCH 03/21] [cpack_installer] some cpack cleanup and prep for online installer support (pre/post build steps) --- cmake/Packaging.cmake | 38 ++++++++++++------- .../Platform/Windows/PackagingPostBuild.cmake | 12 ++++++ .../Platform/Windows/Packaging_windows.cmake | 8 +++- .../Windows/platform_windows_files.cmake | 1 + 4 files changed, 43 insertions(+), 16 deletions(-) create mode 100644 cmake/Platform/Windows/PackagingPostBuild.cmake diff --git a/cmake/Packaging.cmake b/cmake/Packaging.cmake index 4f6565edc7..e398ea7509 100644 --- a/cmake/Packaging.cmake +++ b/cmake/Packaging.cmake @@ -13,6 +13,30 @@ if(NOT PAL_TRAIT_BUILD_CPACK_SUPPORTED) return() endif() +# set the common cpack variables first so they are accessible via configure_file +# when the platforms specific properties are applied below +set(LY_INSTALLER_DOWNLOAD_URL "" CACHE PATH "URL embded into the installer to download additional artifacts") + +set(CPACK_PACKAGE_VENDOR "${PROJECT_NAME}") +set(CPACK_PACKAGE_VERSION "${LY_VERSION_STRING}") +set(CPACK_PACKAGE_DESCRIPTION_SUMMARY "Installation Tool") + +string(TOLOWER ${PROJECT_NAME} _project_name_lower) +set(CPACK_PACKAGE_FILE_NAME "${_project_name_lower}_${LY_VERSION_STRING}_installer") + +set(DEFAULT_LICENSE_NAME "Apache-2.0") +set(DEFAULT_LICENSE_FILE "${CMAKE_SOURCE_DIR}/LICENSE.txt") + +set(CPACK_RESOURCE_FILE_LICENSE ${DEFAULT_LICENSE_FILE}) + +set(CPACK_PACKAGE_INSTALL_DIRECTORY "${CPACK_PACKAGE_VENDOR}/${CPACK_PACKAGE_VERSION}") + +# custom cpack cache variables for use in pre/post build scripts +set(CPACK_SOURCE_DIR ${CMAKE_SOURCE_DIR}/cmake) +set(CPACK_BINARY_DIR ${CMAKE_BINARY_DIR}/installer) +set(CPACK_DOWNLOAD_URL ${LY_INSTALLER_DOWNLOAD_URL}) + +# attempt to apply platform specific settings ly_get_absolute_pal_filename(pal_dir ${CMAKE_SOURCE_DIR}/cmake/Platform/${PAL_HOST_PLATFORM_NAME}) include(${pal_dir}/Packaging_${PAL_HOST_PLATFORM_NAME_LOWERCASE}.cmake) @@ -21,20 +45,6 @@ if(NOT CPACK_GENERATOR) return() endif() -set(CPACK_PACKAGE_VENDOR "${PROJECT_NAME}") -set(CPACK_PACKAGE_VERSION "${LY_VERSION_STRING}") -set(CPACK_PACKAGE_DESCRIPTION_SUMMARY "Installation Tool") - -string(TOLOWER ${PROJECT_NAME} _project_name_lower) -set(CPACK_PACKAGE_FILE_NAME "${_project_name_lower}_installer") - -set(DEFAULT_LICENSE_NAME "Apache-2.0") -set(DEFAULT_LICENSE_FILE "${CMAKE_CURRENT_SOURCE_DIR}/LICENSE.txt") - -set(CPACK_RESOURCE_FILE_LICENSE ${DEFAULT_LICENSE_FILE}) - -set(CPACK_PACKAGE_INSTALL_DIRECTORY "${CPACK_PACKAGE_VENDOR}/${CPACK_PACKAGE_VERSION}") - # IMPORTANT: required to be included AFTER setting all property overrides include(CPack REQUIRED) diff --git a/cmake/Platform/Windows/PackagingPostBuild.cmake b/cmake/Platform/Windows/PackagingPostBuild.cmake new file mode 100644 index 0000000000..fe57904003 --- /dev/null +++ b/cmake/Platform/Windows/PackagingPostBuild.cmake @@ -0,0 +1,12 @@ +# +# 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. +# + +message(STATUS "Hello from CPack post build!") diff --git a/cmake/Platform/Windows/Packaging_windows.cmake b/cmake/Platform/Windows/Packaging_windows.cmake index 8aa6f2386d..ba3ce011a4 100644 --- a/cmake/Platform/Windows/Packaging_windows.cmake +++ b/cmake/Platform/Windows/Packaging_windows.cmake @@ -32,7 +32,7 @@ set(CPACK_GENERATOR "WIX") # however, they are unique for each run. instead, let's do the auto generation here and add it to # the cache for run persistence. an additional cache file will be used to store the information on # the original generation so we still have the ability to detect if they are still being used. -set(_guid_cache_file "${CMAKE_BINARY_DIR}/installer/wix_guid_cache.cmake") +set(_guid_cache_file "${CPACK_BINARY_DIR}/wix_guid_cache.cmake") if(NOT EXISTS ${_guid_cache_file}) set(_wix_guid_namespace "6D43F57A-2917-4AD9-B758-1F13CDB08593") @@ -89,4 +89,8 @@ endif() set(CPACK_WIX_PRODUCT_GUID ${LY_WIX_PRODUCT_GUID}) set(CPACK_WIX_UPGRADE_GUID ${LY_WIX_UPGRADE_GUID}) -set(CPACK_WIX_TEMPLATE "${CMAKE_SOURCE_DIR}/cmake/Platform/Windows/PackagingTemplate.wxs.in") +set(CPACK_WIX_TEMPLATE "${CPACK_SOURCE_DIR}/Platform/Windows/PackagingTemplate.wxs.in") + +set(CPACK_POST_BUILD_SCRIPTS + ${CPACK_SOURCE_DIR}/Platform/Windows/PackagingPostBuild.cmake +) diff --git a/cmake/Platform/Windows/platform_windows_files.cmake b/cmake/Platform/Windows/platform_windows_files.cmake index 2fc869b43e..579621d5ea 100644 --- a/cmake/Platform/Windows/platform_windows_files.cmake +++ b/cmake/Platform/Windows/platform_windows_files.cmake @@ -24,5 +24,6 @@ set(FILES PALDetection_windows.cmake Install_windows.cmake Packaging_windows.cmake + PackagingPostBuild.cmake PackagingTemplate.wxs.in ) From 68711fce7599826e3b6feacb44adc6e00dd0ce53 Mon Sep 17 00:00:00 2001 From: pereslav Date: Thu, 13 May 2021 17:23:37 +0100 Subject: [PATCH 04/21] Added network prefab processor test --- Gems/Multiplayer/Code/CMakeLists.txt | 47 +++++++- Gems/Multiplayer/Code/Tests/MainTools.cpp | 55 +++++++++ .../Code/Tests/PrefabProcessingTests.cpp | 106 ++++++++++++++++++ .../Code/multiplayer_tools_tests_files.cmake | 15 +++ 4 files changed, 221 insertions(+), 2 deletions(-) create mode 100644 Gems/Multiplayer/Code/Tests/MainTools.cpp create mode 100644 Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp create mode 100644 Gems/Multiplayer/Code/multiplayer_tools_tests_files.cmake diff --git a/Gems/Multiplayer/Code/CMakeLists.txt b/Gems/Multiplayer/Code/CMakeLists.txt index 4eeee15c47..7a4eaeb014 100644 --- a/Gems/Multiplayer/Code/CMakeLists.txt +++ b/Gems/Multiplayer/Code/CMakeLists.txt @@ -59,6 +59,26 @@ ly_add_target( ) if (PAL_TRAIT_BUILD_HOST_TOOLS) + ly_add_target( + NAME Multiplayer.Tools.Static STATIC + NAMESPACE Gem + FILES_CMAKE + multiplayer_tools_files.cmake + COMPILE_DEFINITIONS + PUBLIC + MULTIPLAYER_TOOLS + INCLUDE_DIRECTORIES + PRIVATE + . + Source + ${pal_source_dir} + PUBLIC + Include + BUILD_DEPENDENCIES + PUBLIC + AZ::AzToolsFramework + Gem::Multiplayer.Static + ) ly_add_target( NAME Multiplayer.Tools MODULE @@ -74,8 +94,7 @@ if (PAL_TRAIT_BUILD_HOST_TOOLS) Include BUILD_DEPENDENCIES PRIVATE - AZ::AzToolsFramework - Gem::Multiplayer.Static + Gem::Multiplayer.Tools.Static ) ly_add_target( @@ -145,6 +164,30 @@ if (PAL_TRAIT_BUILD_TESTS_SUPPORTED) ly_add_googletest( NAME Gem::Multiplayer.Tests ) + + if (PAL_TRAIT_BUILD_HOST_TOOLS) + ly_add_target( + NAME Multiplayer.Tools.Tests ${PAL_TRAIT_TEST_TARGET_TYPE} + NAMESPACE Gem + FILES_CMAKE + multiplayer_tools_tests_files.cmake + INCLUDE_DIRECTORIES + PRIVATE + Tests + Source + . + BUILD_DEPENDENCIES + PRIVATE + AZ::AzTest + AZ::AzTestShared + AZ::AzToolsFrameworkTestCommon + Gem::Multiplayer.Tools.Static + ) + ly_add_googletest( + NAME Gem::Multiplayer.Tools.Tests + ) + endif() + endif() ly_add_target( diff --git a/Gems/Multiplayer/Code/Tests/MainTools.cpp b/Gems/Multiplayer/Code/Tests/MainTools.cpp new file mode 100644 index 0000000000..65a1d921a9 --- /dev/null +++ b/Gems/Multiplayer/Code/Tests/MainTools.cpp @@ -0,0 +1,55 @@ +/* +* 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. +* +*/ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace Multiplayer +{ + class MultiplayerToolsTestEnvironment : public AZ::Test::GemTestEnvironment + { + AZ::ComponentApplication* CreateApplicationInstance() override + { + return aznew UnitTest::ToolsTestApplication("MultiplayerToolsTest"); + } + + void AddGemsAndComponents() override + { + AZStd::vector descriptors({ + NetBindComponent::CreateDescriptor(), + NetBindMarkerComponent::CreateDescriptor(), + NetworkSpawnableHolderComponent::CreateDescriptor() + }); + + AddComponentDescriptors(descriptors); + } + }; +} // namespace UnitTest + +// Required to support running integration tests with Qt +AZTEST_EXPORT int AZ_UNIT_TEST_HOOK_NAME(int argc, char** argv) +{ + ::testing::InitGoogleMock(&argc, argv); + AzQtComponents::PrepareQtPaths(); + QApplication app(argc, argv); + AZ::Test::printUnusedParametersWarning(argc, argv); + AZ::Test::addTestEnvironments({new Multiplayer::MultiplayerToolsTestEnvironment}); + int result = RUN_ALL_TESTS(); + return result; +} diff --git a/Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp b/Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp new file mode 100644 index 0000000000..3d16d12f53 --- /dev/null +++ b/Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp @@ -0,0 +1,106 @@ +/* +* 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. +* +*/ + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace UnitTest +{ + class PrefabProcessingTestFixture : public ::testing::Test + { + public: + static void ConvertEntitiesToPrefab(const AZStd::vector& entities, AzToolsFramework::Prefab::PrefabDom& prefabDom) + { + auto* prefabSystem = AZ::Interface::Get(); + AZStd::unique_ptr sourceInstance(prefabSystem->CreatePrefab(entities, {}, "test/path")); + ASSERT_TRUE(sourceInstance); + + auto& prefabTemplateDom = prefabSystem->FindTemplateDom(sourceInstance->GetTemplateId()); + prefabDom.CopyFrom(prefabTemplateDom, prefabDom.GetAllocator()); + } + + static AZ::Entity* CreateSourceEntity(const char* name, bool networked, const AZ::Transform& tm, AZ::Entity* parent = nullptr) + { + AZ::Entity* entity = aznew AZ::Entity(name); + auto* transformComponent = entity->CreateComponent(); + + if (parent) + { + transformComponent->SetParent(parent->GetId()); + transformComponent->SetLocalTM(tm); + } + else + { + transformComponent->SetWorldTM(tm); + } + + if(networked) + { + entity->CreateComponent(); + } + + return entity; + } + }; + + TEST_F(PrefabProcessingTestFixture, NetworkPrefabProcessor_ProcessPrefabTwoEntities_NetEntityGoesToNetSpawnable) + { + using AzToolsFramework::Prefab::PrefabConversionUtils::PrefabProcessorContext; + + AZStd::vector entities; + + const AZStd::string staticEntityName = "static_floor"; + entities.emplace_back(CreateSourceEntity(staticEntityName.c_str(), false, AZ::Transform::CreateIdentity())); + + const AZStd::string netEntityName = "networked_entity"; + entities.emplace_back(CreateSourceEntity(netEntityName.c_str(), true, AZ::Transform::CreateIdentity())); + + AzToolsFramework::Prefab::PrefabDom prefabDom; + ConvertEntitiesToPrefab(entities, prefabDom); + + const AZStd::string prefabName = "testPrefab"; + PrefabProcessorContext prefabProcessorContext{AZ::Uuid::CreateRandom()}; + prefabProcessorContext.AddPrefab(prefabName, AZStd::move(prefabDom)); + + Multiplayer::NetworkPrefabProcessor processor; + processor.Process(prefabProcessorContext); + + EXPECT_TRUE(prefabProcessorContext.HasCompletedSuccessfully()); + + const auto& processedObjects = prefabProcessorContext.GetProcessedObjects(); + EXPECT_EQ(processedObjects.size(), 1); + + const AZ::Data::AssetData& spawnableAsset = processedObjects[0].GetAsset(); + EXPECT_EQ(prefabName + ".network.spawnable", processedObjects[0].GetId()); + EXPECT_EQ(spawnableAsset.GetType(), azrtti_typeid()); + + const AzFramework::Spawnable* netSpawnable = azrtti_cast(&spawnableAsset); + const AzFramework::Spawnable::EntityList& entityList = netSpawnable->GetEntities(); + auto countEntityCallback = [](const auto& name) + { + return [name](const auto& entity) + { + return entity->GetName() == name; + }; + }; + + EXPECT_EQ(0, AZStd::count_if(entityList.begin(), entityList.end(), countEntityCallback(staticEntityName))); + EXPECT_EQ(1, AZStd::count_if(entityList.begin(), entityList.end(), countEntityCallback(netEntityName))); + } + +} // namespace UnitTest diff --git a/Gems/Multiplayer/Code/multiplayer_tools_tests_files.cmake b/Gems/Multiplayer/Code/multiplayer_tools_tests_files.cmake new file mode 100644 index 0000000000..c308b3de52 --- /dev/null +++ b/Gems/Multiplayer/Code/multiplayer_tools_tests_files.cmake @@ -0,0 +1,15 @@ +# +# 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. +# + +set(FILES + Tests/MainTools.cpp + Tests/PrefabProcessingTests.cpp +) From fdc890b0fc693c636559a4c0b900658cbd7f55b5 Mon Sep 17 00:00:00 2001 From: pereslav Date: Thu, 13 May 2021 17:27:23 +0100 Subject: [PATCH 05/21] Added comments to the test --- Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp b/Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp index 3d16d12f53..df1da11725 100644 --- a/Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp +++ b/Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp @@ -64,31 +64,39 @@ namespace UnitTest AZStd::vector entities; + // Create test entities: 1 networked and 1 static const AZStd::string staticEntityName = "static_floor"; entities.emplace_back(CreateSourceEntity(staticEntityName.c_str(), false, AZ::Transform::CreateIdentity())); const AZStd::string netEntityName = "networked_entity"; entities.emplace_back(CreateSourceEntity(netEntityName.c_str(), true, AZ::Transform::CreateIdentity())); + // Convert the entities into prefab. Note: This will transfer the ownership of AZ::Entity* into Prefab AzToolsFramework::Prefab::PrefabDom prefabDom; ConvertEntitiesToPrefab(entities, prefabDom); + // Add the prefab into the Prefab Processor Context const AZStd::string prefabName = "testPrefab"; PrefabProcessorContext prefabProcessorContext{AZ::Uuid::CreateRandom()}; prefabProcessorContext.AddPrefab(prefabName, AZStd::move(prefabDom)); + // Request NetworkPrefabProcessor to process the prefab Multiplayer::NetworkPrefabProcessor processor; processor.Process(prefabProcessorContext); + // Validate results EXPECT_TRUE(prefabProcessorContext.HasCompletedSuccessfully()); + // Should be 1 networked spawnable const auto& processedObjects = prefabProcessorContext.GetProcessedObjects(); EXPECT_EQ(processedObjects.size(), 1); + // Verify the name and the type of the spawnable asset const AZ::Data::AssetData& spawnableAsset = processedObjects[0].GetAsset(); EXPECT_EQ(prefabName + ".network.spawnable", processedObjects[0].GetId()); EXPECT_EQ(spawnableAsset.GetType(), azrtti_typeid()); + // Verify we have only the networked entity in the network spawnable and not the static one const AzFramework::Spawnable* netSpawnable = azrtti_cast(&spawnableAsset); const AzFramework::Spawnable::EntityList& entityList = netSpawnable->GetEntities(); auto countEntityCallback = [](const auto& name) From 6d7f7547ec6a64c34258dfcb54db9abc6754602f Mon Sep 17 00:00:00 2001 From: scottr Date: Fri, 14 May 2021 08:20:59 -0700 Subject: [PATCH 06/21] [cpack_installer] fix incorrect caching type of installer download url and add configure of download info --- cmake/Packaging.cmake | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/cmake/Packaging.cmake b/cmake/Packaging.cmake index e398ea7509..188cfbf52d 100644 --- a/cmake/Packaging.cmake +++ b/cmake/Packaging.cmake @@ -15,7 +15,7 @@ endif() # set the common cpack variables first so they are accessible via configure_file # when the platforms specific properties are applied below -set(LY_INSTALLER_DOWNLOAD_URL "" CACHE PATH "URL embded into the installer to download additional artifacts") +set(LY_INSTALLER_DOWNLOAD_URL "" CACHE STRING "URL embded into the installer to download additional artifacts") set(CPACK_PACKAGE_VENDOR "${PROJECT_NAME}") set(CPACK_PACKAGE_VERSION "${LY_VERSION_STRING}") @@ -34,7 +34,6 @@ set(CPACK_PACKAGE_INSTALL_DIRECTORY "${CPACK_PACKAGE_VENDOR}/${CPACK_PACKAGE_VER # custom cpack cache variables for use in pre/post build scripts set(CPACK_SOURCE_DIR ${CMAKE_SOURCE_DIR}/cmake) set(CPACK_BINARY_DIR ${CMAKE_BINARY_DIR}/installer) -set(CPACK_DOWNLOAD_URL ${LY_INSTALLER_DOWNLOAD_URL}) # attempt to apply platform specific settings ly_get_absolute_pal_filename(pal_dir ${CMAKE_SOURCE_DIR}/cmake/Platform/${PAL_HOST_PLATFORM_NAME}) @@ -86,3 +85,11 @@ ly_configure_cpack_component( DISPLAY_NAME "${PROJECT_NAME} Core" DESCRIPTION "${PROJECT_NAME} Headers, Libraries and Tools" ) + +if(LY_INSTALLER_DOWNLOAD_URL) + cpack_configure_downloads( + ${LY_INSTALLER_DOWNLOAD_URL} + UPLOAD_DIRECTORY artifacts + ALL + ) +endif() From f6b1fac139f649062e6f9595e0476b5299439cad Mon Sep 17 00:00:00 2001 From: scottr Date: Fri, 14 May 2021 12:03:20 -0700 Subject: [PATCH 07/21] [cpack_installer] initial bootstrap installer generation, this bootstrapper is what downloads the artifacts --- .../Windows/PackagingBootstrapper.wxs | 36 ++++++++++++++ .../Platform/Windows/PackagingPostBuild.cmake | 47 ++++++++++++++++++- .../Platform/Windows/PackagingTemplate.wxs.in | 4 +- .../Platform/Windows/Packaging_windows.cmake | 14 +++++- .../Windows/platform_windows_files.cmake | 1 + 5 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 cmake/Platform/Windows/PackagingBootstrapper.wxs diff --git a/cmake/Platform/Windows/PackagingBootstrapper.wxs b/cmake/Platform/Windows/PackagingBootstrapper.wxs new file mode 100644 index 0000000000..711b60d854 --- /dev/null +++ b/cmake/Platform/Windows/PackagingBootstrapper.wxs @@ -0,0 +1,36 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/cmake/Platform/Windows/PackagingPostBuild.cmake b/cmake/Platform/Windows/PackagingPostBuild.cmake index fe57904003..064fb0d530 100644 --- a/cmake/Platform/Windows/PackagingPostBuild.cmake +++ b/cmake/Platform/Windows/PackagingPostBuild.cmake @@ -9,4 +9,49 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # -message(STATUS "Hello from CPack post build!") +# convert the path to a windows style path +string(REPLACE "/" "\\" _install_dir ${CPACK_PACKAGE_INSTALL_DIRECTORY}) + +# directory where the auto generated files live e.g /_CPack_Package/win64/WIX +set(_cpack_out_dir "${CPACK_TOPLEVEL_DIRECTORY}") +set(_out_dir "${CPACK_BINARY_DIR}/wixobj_bootstrap") + +set(_wix_ext_flags + -ext WixBalExtension +) + +set(_candle_command + ${CPACK_WIX_ROOT}/bin/candle.exe + -nologo + -arch x64 + "-I${_cpack_out_dir}" + ${_wix_ext_flags} + + -dCPACK_DOWNLOAD_SITE=${CPACK_DOWNLOAD_SITE} + -dCPACK_LOCAL_INSTALLER_DIR=${_cpack_out_dir} + -dCPACK_PACKAGE_FILE_NAME=${CPACK_PACKAGE_FILE_NAME} + -dCPACK_PACKAGE_INSTALL_DIRECTORY=${_install_dir} + + "${CPACK_SOURCE_DIR}/Platform/Windows/PackagingBootstrapper.wxs" + + -o "${_out_dir}" +) + +set(_light_command + ${CPACK_WIX_ROOT}/bin/light.exe + -nologo + ${_wix_ext_flags} + ${_out_dir}/*.wixobj + + -o "${CPACK_BINARY_DIR}/installer.exe" +) + +message(STATUS "Creating Installer Bootstrapper...") + +execute_process( + COMMAND + ${_candle_command} + + COMMAND + ${_light_command} +) diff --git a/cmake/Platform/Windows/PackagingTemplate.wxs.in b/cmake/Platform/Windows/PackagingTemplate.wxs.in index 3e5db03ec2..fd3610259a 100644 --- a/cmake/Platform/Windows/PackagingTemplate.wxs.in +++ b/cmake/Platform/Windows/PackagingTemplate.wxs.in @@ -14,8 +14,8 @@ - - + + Date: Mon, 17 May 2021 15:28:32 -0700 Subject: [PATCH 08/21] [cpack_installer] cpack variable usage cleanup --- cmake/Packaging.cmake | 8 +-- .../Platform/Windows/PackagingPostBuild.cmake | 58 ++++++++++--------- .../Platform/Windows/Packaging_windows.cmake | 2 +- 3 files changed, 36 insertions(+), 32 deletions(-) diff --git a/cmake/Packaging.cmake b/cmake/Packaging.cmake index 188cfbf52d..72216847f1 100644 --- a/cmake/Packaging.cmake +++ b/cmake/Packaging.cmake @@ -31,12 +31,12 @@ set(CPACK_RESOURCE_FILE_LICENSE ${DEFAULT_LICENSE_FILE}) set(CPACK_PACKAGE_INSTALL_DIRECTORY "${CPACK_PACKAGE_VENDOR}/${CPACK_PACKAGE_VERSION}") -# custom cpack cache variables for use in pre/post build scripts +# CMAKE_SOURCE_DIR doesn't equate to anything during execution of pre/post build scripts. +# to pass it down, we can utilize the auto-caching of any variable with prefix "CPACK_" set(CPACK_SOURCE_DIR ${CMAKE_SOURCE_DIR}/cmake) -set(CPACK_BINARY_DIR ${CMAKE_BINARY_DIR}/installer) # attempt to apply platform specific settings -ly_get_absolute_pal_filename(pal_dir ${CMAKE_SOURCE_DIR}/cmake/Platform/${PAL_HOST_PLATFORM_NAME}) +ly_get_absolute_pal_filename(pal_dir ${CPACK_SOURCE_DIR}/Platform/${PAL_HOST_PLATFORM_NAME}) include(${pal_dir}/Packaging_${PAL_HOST_PLATFORM_NAME_LOWERCASE}.cmake) # if we get here and the generator hasn't been set, then a non fatal error occurred disabling packaging support @@ -89,7 +89,7 @@ ly_configure_cpack_component( if(LY_INSTALLER_DOWNLOAD_URL) cpack_configure_downloads( ${LY_INSTALLER_DOWNLOAD_URL} - UPLOAD_DIRECTORY artifacts + UPLOAD_DIRECTORY ${CMAKE_BINARY_DIR}/_CPack_Uploads # to match the _CPack_Packages directory ALL ) endif() diff --git a/cmake/Platform/Windows/PackagingPostBuild.cmake b/cmake/Platform/Windows/PackagingPostBuild.cmake index 064fb0d530..5262b42beb 100644 --- a/cmake/Platform/Windows/PackagingPostBuild.cmake +++ b/cmake/Platform/Windows/PackagingPostBuild.cmake @@ -9,49 +9,53 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # -# convert the path to a windows style path -string(REPLACE "/" "\\" _install_dir ${CPACK_PACKAGE_INSTALL_DIRECTORY}) +# convert the path to a windows style path using string replace because TO_NATIVE_PATH +# only works on real paths +string(REPLACE "/" "\\" _fixed_package_install_dir ${CPACK_PACKAGE_INSTALL_DIRECTORY}) # directory where the auto generated files live e.g /_CPack_Package/win64/WIX -set(_cpack_out_dir "${CPACK_TOPLEVEL_DIRECTORY}") -set(_out_dir "${CPACK_BINARY_DIR}/wixobj_bootstrap") +set(_cpack_wix_out_dir ${CPACK_TOPLEVEL_DIRECTORY}) +set(_bootstrap_out_dir "${CPACK_TOPLEVEL_DIRECTORY}/bootstrap") -set(_wix_ext_flags +set(_bootstrap_filename "${CPACK_PACKAGE_FILE_NAME}.exe") +set(_bootstrap_output_file ${_cpack_wix_out_dir}/${_bootstrap_filename}) + +set(_ext_flags -ext WixBalExtension ) +set(_addtional_defines + -dCPACK_DOWNLOAD_SITE=${CPACK_DOWNLOAD_SITE} + -dCPACK_LOCAL_INSTALLER_DIR=${_cpack_wix_out_dir} + -dCPACK_PACKAGE_FILE_NAME=${CPACK_PACKAGE_FILE_NAME} + -dCPACK_PACKAGE_INSTALL_DIRECTORY=${_fixed_package_install_dir} +) + set(_candle_command - ${CPACK_WIX_ROOT}/bin/candle.exe + ${CPACK_WIX_CANDLE_EXECUTABLE} -nologo -arch x64 - "-I${_cpack_out_dir}" - ${_wix_ext_flags} - - -dCPACK_DOWNLOAD_SITE=${CPACK_DOWNLOAD_SITE} - -dCPACK_LOCAL_INSTALLER_DIR=${_cpack_out_dir} - -dCPACK_PACKAGE_FILE_NAME=${CPACK_PACKAGE_FILE_NAME} - -dCPACK_PACKAGE_INSTALL_DIRECTORY=${_install_dir} - + "-I${_cpack_wix_out_dir}" # to include cpack_variables.wxi + ${_addtional_defines} + ${_ext_flags} "${CPACK_SOURCE_DIR}/Platform/Windows/PackagingBootstrapper.wxs" - - -o "${_out_dir}" + -o "${_bootstrap_out_dir}/" ) set(_light_command - ${CPACK_WIX_ROOT}/bin/light.exe + ${CPACK_WIX_LIGHT_EXECUTABLE} -nologo - ${_wix_ext_flags} - ${_out_dir}/*.wixobj - - -o "${CPACK_BINARY_DIR}/installer.exe" + ${_ext_flags} + ${_bootstrap_out_dir}/*.wixobj + -o "${_bootstrap_output_file}" ) message(STATUS "Creating Installer Bootstrapper...") - execute_process( - COMMAND - ${_candle_command} - - COMMAND - ${_light_command} + COMMAND ${_candle_command} + COMMAND_ERROR_IS_FATAL ANY +) +execute_process( + COMMAND ${_light_command} + COMMAND_ERROR_IS_FATAL ANY ) diff --git a/cmake/Platform/Windows/Packaging_windows.cmake b/cmake/Platform/Windows/Packaging_windows.cmake index f8c79e11d5..4044450d4c 100644 --- a/cmake/Platform/Windows/Packaging_windows.cmake +++ b/cmake/Platform/Windows/Packaging_windows.cmake @@ -32,7 +32,7 @@ set(CPACK_GENERATOR "WIX") # however, they are unique for each run. instead, let's do the auto generation here and add it to # the cache for run persistence. an additional cache file will be used to store the information on # the original generation so we still have the ability to detect if they are still being used. -set(_guid_cache_file "${CPACK_BINARY_DIR}/wix_guid_cache.cmake") +set(_guid_cache_file "${CMAKE_BINARY_DIR}/CPackWiXConfig.cmake") if(NOT EXISTS ${_guid_cache_file}) set(_wix_guid_namespace "6D43F57A-2917-4AD9-B758-1F13CDB08593") From 970c87b487bcdc1d4b2260d18b865029381e952d Mon Sep 17 00:00:00 2001 From: pereslav Date: Mon, 17 May 2021 23:34:58 +0100 Subject: [PATCH 09/21] Refactored spawning network entities to use SpawnableEntityManager instead of duplicating the code in NetworkEntityManager --- .../Spawnable/SpawnableEntitiesInterface.h | 6 +- .../Spawnable/SpawnableEntitiesManager.cpp | 136 +++++++++++++++--- .../Spawnable/SpawnableEntitiesManager.h | 6 +- .../Multiplayer/INetworkEntityManager.h | 6 + .../Multiplayer/INetworkSpawnableLibrary.h | 34 +++++ .../NetworkEntity/NetworkEntityManager.cpp | 65 ++------- .../NetworkEntity/NetworkEntityManager.h | 8 +- .../NetworkEntity/NetworkSpawnableLibrary.cpp | 9 +- .../NetworkEntity/NetworkSpawnableLibrary.h | 19 +-- .../Pipeline/NetBindMarkerComponent.cpp | 83 ++++++++++- .../Source/Pipeline/NetBindMarkerComponent.h | 12 ++ .../Pipeline/NetworkPrefabProcessor.cpp | 18 +-- Gems/Multiplayer/Code/Tests/MainTools.cpp | 2 +- .../Code/Tests/PrefabProcessingTests.cpp | 2 +- Gems/Multiplayer/Code/multiplayer_files.cmake | 1 + 15 files changed, 301 insertions(+), 106 deletions(-) create mode 100644 Gems/Multiplayer/Code/Include/Multiplayer/INetworkSpawnableLibrary.h diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h index 95d0b9e3a7..ac66288ff2 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h @@ -84,6 +84,7 @@ namespace AzFramework }; using EntitySpawnCallback = AZStd::function; + using EntityPreInsertionCallback = AZStd::function; using EntityDespawnCallback = AZStd::function; using ReloadSpawnableCallback = AZStd::function; using ListEntitiesCallback = AZStd::function; @@ -110,7 +111,8 @@ namespace AzFramework //! @param completionCallback Optional callback that's called when spawning entities has completed. This can be called from //! a different thread than the one that made the function call. The returned list of entities contains all the newly //! created entities. - virtual void SpawnAllEntities(EntitySpawnTicket& ticket, EntitySpawnCallback completionCallback = {}) = 0; + virtual void SpawnAllEntities(EntitySpawnTicket& ticket, EntityPreInsertionCallback preInsertionCallback = {}, + EntitySpawnCallback completionCallback = {}) = 0; //! Spawn instances of some entities in the spawnable. //! @param ticket Stores the results of the call. Use this ticket to spawn additional entities or to despawn them. //! @param entityIndices The indices into the template entities stored in the spawnable that will be used to spawn entities from. @@ -118,7 +120,7 @@ namespace AzFramework //! a different thread than the one that made this function call. The returned list of entities contains all the newly //! created entities. virtual void SpawnEntities(EntitySpawnTicket& ticket, AZStd::vector entityIndices, - EntitySpawnCallback completionCallback = {}) = 0; + EntityPreInsertionCallback preInsertionCallback = {}, EntitySpawnCallback completionCallback = {}) = 0; //! Removes all entities in the provided list from the environment. //! @param ticket The ticket previously used to spawn entities with. //! @param completionCallback Optional callback that's called when despawning entities has completed. This can be called from diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp index 3ab004b516..b18307ff7f 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp @@ -14,17 +14,20 @@ #include #include #include +#include #include #include #include namespace AzFramework { - void SpawnableEntitiesManager::SpawnAllEntities(EntitySpawnTicket& ticket, EntitySpawnCallback completionCallback) + void SpawnableEntitiesManager::SpawnAllEntities(EntitySpawnTicket& ticket, EntityPreInsertionCallback preInsertionCallback, + EntitySpawnCallback completionCallback) { SpawnAllEntitiesCommand queueEntry; queueEntry.m_ticket = &ticket; queueEntry.m_completionCallback = AZStd::move(completionCallback); + queueEntry.m_preInsertionCallback = AZStd::move(preInsertionCallback); { AZStd::scoped_lock queueLock(m_pendingRequestQueueMutex); queueEntry.m_ticketId = GetTicketPayload(ticket).m_nextTicketId++; @@ -32,13 +35,15 @@ namespace AzFramework } } - void SpawnableEntitiesManager::SpawnEntities(EntitySpawnTicket& ticket, AZStd::vector entityIndices, - EntitySpawnCallback completionCallback) + void SpawnableEntitiesManager::SpawnEntities( + EntitySpawnTicket& ticket, AZStd::vector entityIndices, + EntityPreInsertionCallback preInsertionCallback, EntitySpawnCallback completionCallback) { SpawnEntitiesCommand queueEntry; queueEntry.m_ticket = &ticket; queueEntry.m_entityIndices = AZStd::move(entityIndices); queueEntry.m_completionCallback = AZStd::move(completionCallback); + queueEntry.m_preInsertionCallback = AZStd::move(preInsertionCallback); { AZStd::scoped_lock queueLock(m_pendingRequestQueueMutex); queueEntry.m_ticketId = GetTicketPayload(ticket).m_nextTicketId++; @@ -205,6 +210,9 @@ namespace AzFramework AZ::Entity* clone = serializeContext.CloneObject(&entityTemplate); AZ_Assert(clone != nullptr, "Failed to clone spawnable entity."); clone->SetId(AZ::Entity::MakeId()); + + // Need to inject a callback here + GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, clone); return clone; } @@ -214,23 +222,79 @@ namespace AzFramework Ticket& ticket = GetTicketPayload(*request.m_ticket); if (ticket.m_spawnable.IsReady() && request.m_ticketId == ticket.m_currentTicketId) { - size_t spawnedEntitiesCount = ticket.m_spawnedEntities.size(); + AZStd::vector& spawnedEntities = ticket.m_spawnedEntities; + AZStd::vector& spawnedEntityIndices = ticket.m_spawnedEntityIndices; - const Spawnable::EntityList& entities = ticket.m_spawnable->GetEntities(); - size_t entitiesSize = entities.size(); - ticket.m_spawnedEntities.reserve(ticket.m_spawnedEntities.size() + entitiesSize); - ticket.m_spawnedEntityIndices.reserve(ticket.m_spawnedEntityIndices.size() + entitiesSize); + // Keep track how many entities there were in the array initially + size_t spawnedEntitiesInitialCount = spawnedEntities.size(); - for(size_t i=0; iGetEntities(); + size_t entitiesToSpawnSize = entitiesToSpawn.size(); + + // Reserve buffers + spawnedEntities.reserve(spawnedEntities.size() + entitiesToSpawnSize); + ticket.m_spawnedEntityIndices.reserve(ticket.m_spawnedEntityIndices.size() + entitiesToSpawnSize); + + // TEMP: To be replaced by IdUtils::Remapper + using EntityIdMap = AZStd::unordered_map; + EntityIdMap templateToCloneIdMap; + // \TEMP + + // Clone the entities from Spawnable + for (size_t i = 0; i < entitiesToSpawnSize; ++i) { - ticket.m_spawnedEntities.push_back(SpawnSingleEntity(*entities[i], serializeContext)); - ticket.m_spawnedEntityIndices.push_back(i); + const AZ::Entity& entityTemplate = *entitiesToSpawn[i]; + + AZ::Entity* clone = serializeContext.CloneObject(&entityTemplate); + AZ_Assert(clone != nullptr, "Failed to clone spawnable entity."); + clone->SetId(AZ::Entity::MakeId()); + + spawnedEntities.push_back(clone); + spawnedEntityIndices.push_back(i); + + // TEMP: To be replaced by IdUtils::Remapper + templateToCloneIdMap[entityTemplate.GetId()] = clone->GetId(); + + // Update TransformComponent parent Id. It is guaranteed for the entities array to be sorted from parent->child here. + auto* transformComponent = clone->FindComponent(); + AZ::EntityId parentId = transformComponent->GetParentId(); + if (parentId.IsValid()) + { + auto it = templateToCloneIdMap.find(parentId); + if (it != templateToCloneIdMap.end()) + { + transformComponent->SetParentRelative(it->second); + } + else + { + AZ_Warning( + "SpawnableEntitiesManager", false, "Entity %s doesn't have the parent entity %s present in the spawnable", + clone->GetName().c_str(), parentId.ToString().data()); + } + } + // \TEMP } + // Let other systems know about newly spawned entities for any pre-processing before adding to the scene/game context. + if (request.m_preInsertionCallback) + { + request.m_preInsertionCallback(*request.m_ticket, SpawnableEntityContainerView( + ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount, ticket.m_spawnedEntities.end())); + } + + // Add to the game context, now the entities are active + AZStd::for_each(ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount, ticket.m_spawnedEntities.end(), + [](AZ::Entity* entity) + { + GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, entity); + }); + + // Let other systems know about newly spawned entities for any post-processing after adding to the scene/game context. if (request.m_completionCallback) { request.m_completionCallback(*request.m_ticket, SpawnableConstEntityContainerView( - ticket.m_spawnedEntities.begin() + spawnedEntitiesCount, ticket.m_spawnedEntities.end())); + ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount, ticket.m_spawnedEntities.end())); } m_onSpawnedEvent.Signal(ticket.m_spawnable); @@ -249,24 +313,56 @@ namespace AzFramework Ticket& ticket = GetTicketPayload(*request.m_ticket); if (ticket.m_spawnable.IsReady() && request.m_ticketId == ticket.m_currentTicketId) { - size_t spawnedEntitiesCount = ticket.m_spawnedEntities.size(); + AZStd::vector& spawnedEntities = ticket.m_spawnedEntities; + AZStd::vector& spawnedEntityIndices = ticket.m_spawnedEntityIndices; - const Spawnable::EntityList& entities = ticket.m_spawnable->GetEntities(); - size_t entitiesSize = entities.size(); - ticket.m_spawnedEntities.reserve(ticket.m_spawnedEntities.size() + entitiesSize); - ticket.m_spawnedEntityIndices.reserve(ticket.m_spawnedEntityIndices.size() + entitiesSize); + // Keep track how many entities there were in the array initially + size_t spawnedEntitiesInitialCount = spawnedEntities.size(); + + // These are 'template' entities we'll be cloning from + const Spawnable::EntityList& entitiesToSpawn = ticket.m_spawnable->GetEntities(); + size_t entitiesToSpawnSize = request.m_entityIndices.size(); + + spawnedEntities.reserve(spawnedEntities.size() + entitiesToSpawnSize); + spawnedEntityIndices.reserve(spawnedEntityIndices.size() + entitiesToSpawnSize); for (size_t index : request.m_entityIndices) { - ticket.m_spawnedEntities.push_back(SpawnSingleEntity(*entities[index], serializeContext)); - ticket.m_spawnedEntityIndices.push_back(index); + if (index < entitiesToSpawn.size()) + { + const AZ::Entity& entityTemplate = *entitiesToSpawn[index]; + + AZ::Entity* clone = serializeContext.CloneObject(&entityTemplate); + AZ_Assert(clone != nullptr, "Failed to clone spawnable entity."); + clone->SetId(AZ::Entity::MakeId()); + + spawnedEntities.push_back(clone); + spawnedEntityIndices.push_back(index); + + } } ticket.m_loadAll = false; + // Let other systems know about newly spawned entities for any pre-processing before adding to the scene/game context. + if (request.m_preInsertionCallback) + { + request.m_preInsertionCallback( + *request.m_ticket, + SpawnableEntityContainerView( + ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount, ticket.m_spawnedEntities.end())); + } + + // Add to the game context, now the entities are active + AZStd::for_each(ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount, ticket.m_spawnedEntities.end(), + [](AZ::Entity* entity) + { + GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, entity); + }); + if (request.m_completionCallback) { request.m_completionCallback(*request.m_ticket, SpawnableConstEntityContainerView( - ticket.m_spawnedEntities.begin() + spawnedEntitiesCount, ticket.m_spawnedEntities.end())); + ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount, ticket.m_spawnedEntities.end())); } m_onSpawnedEvent.Signal(ticket.m_spawnable); diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h index c70b9ccaa6..54f48055fd 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h @@ -47,8 +47,8 @@ namespace AzFramework // The following functions are thread safe // - void SpawnAllEntities(EntitySpawnTicket& ticket, EntitySpawnCallback completionCallback = {}) override; - void SpawnEntities(EntitySpawnTicket& ticket, AZStd::vector entityIndices, + void SpawnAllEntities(EntitySpawnTicket& ticket, EntityPreInsertionCallback preInsertionCallback = {}, EntitySpawnCallback completionCallback = {}) override; + void SpawnEntities(EntitySpawnTicket& ticket, AZStd::vector entityIndices, EntityPreInsertionCallback preInsertionCallback = {}, EntitySpawnCallback completionCallback = {}) override; void DespawnAllEntities(EntitySpawnTicket& ticket, EntityDespawnCallback completionCallback = {}) override; @@ -90,6 +90,7 @@ namespace AzFramework struct SpawnAllEntitiesCommand { EntitySpawnCallback m_completionCallback; + EntityPreInsertionCallback m_preInsertionCallback; EntitySpawnTicket* m_ticket; uint32_t m_ticketId; }; @@ -97,6 +98,7 @@ namespace AzFramework { AZStd::vector m_entityIndices; EntitySpawnCallback m_completionCallback; + EntityPreInsertionCallback m_preInsertionCallback; EntitySpawnTicket* m_ticket; uint32_t m_ticketId; }; diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/INetworkEntityManager.h b/Gems/Multiplayer/Code/Include/Multiplayer/INetworkEntityManager.h index 17224e64cb..028c0310e1 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/INetworkEntityManager.h +++ b/Gems/Multiplayer/Code/Include/Multiplayer/INetworkEntityManager.h @@ -78,6 +78,12 @@ namespace Multiplayer const AZ::Transform& transform ) = 0; + //! Configures new networked entity + //! @param netEntity the entity to setup + //! @param prefabEntryId the name of the spawnable the entity originated from + //! @param netEntityRole the net role the entity should be setup for + virtual void SetupNetEntity(AZ::Entity* netEntity, PrefabEntityId prefabEntityId, NetEntityRole netEntityRole) = 0; + //! Returns an ConstEntityPtr for the provided entityId. //! @param netEntityId the netEntityId to get an ConstEntityPtr for //! @return the requested ConstEntityPtr diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/INetworkSpawnableLibrary.h b/Gems/Multiplayer/Code/Include/Multiplayer/INetworkSpawnableLibrary.h new file mode 100644 index 0000000000..422f7f4be3 --- /dev/null +++ b/Gems/Multiplayer/Code/Include/Multiplayer/INetworkSpawnableLibrary.h @@ -0,0 +1,34 @@ +/* +* 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 +#include +#include + +namespace Multiplayer +{ + //! @class INetworkSpawnableLibrary + //! @brief The interface for managing network spawnables. + class INetworkSpawnableLibrary + { + public: + AZ_RTTI(INetworkSpawnableLibrary, "{A3CF809C-6C1D-4B43-B2C4-3901B5DE1ABE}"); + + virtual ~INetworkSpawnableLibrary() = default; + virtual void BuildSpawnablesList() = 0; + virtual void ProcessSpawnableAsset(const AZStd::string& relativePath, AZ::Data::AssetId id) = 0; + virtual AZ::Name GetSpawnableNameFromAssetId(AZ::Data::AssetId assetId) = 0; + virtual AZ::Data::AssetId GetAssetIdByName(AZ::Name name) = 0; + }; +} diff --git a/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkEntityManager.cpp b/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkEntityManager.cpp index 971353b0cb..fba5f477eb 100644 --- a/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkEntityManager.cpp +++ b/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkEntityManager.cpp @@ -34,18 +34,15 @@ namespace Multiplayer : m_networkEntityAuthorityTracker(*this) , m_removeEntitiesEvent([this] { RemoveEntities(); }, AZ::Name("NetworkEntityManager remove entities event")) , m_updateEntityDomainEvent([this] { UpdateEntityDomain(); }, AZ::Name("NetworkEntityManager update entity domain event")) - , m_onSpawnedHandler([this](AZ::Data::Asset spawnable) { this->OnSpawned(spawnable); }) - , m_onDespawnedHandler([this](AZ::Data::Asset spawnable) { this->OnDespawned(spawnable); }) { + AZ::Interface::Register(this); AzFramework::RootSpawnableNotificationBus::Handler::BusConnect(); - - AzFramework::SpawnableEntitiesInterface::Get()->AddOnSpawnedHandler(m_onSpawnedHandler); - AzFramework::SpawnableEntitiesInterface::Get()->AddOnDespawnedHandler(m_onDespawnedHandler); } NetworkEntityManager::~NetworkEntityManager() { AzFramework::RootSpawnableNotificationBus::Handler::BusDisconnect(); + AZ::Interface::Unregister(this); } void NetworkEntityManager::Initialize(HostId hostId, AZStd::unique_ptr entityDomain) @@ -365,7 +362,7 @@ namespace Multiplayer } PrefabEntityId prefabEntityId; - prefabEntityId.m_prefabName = m_networkPrefabLibrary.GetPrefabNameFromAssetId(spawnable.GetId()); + prefabEntityId.m_prefabName = m_networkPrefabLibrary.GetSpawnableNameFromAssetId(spawnable.GetId()); prefabEntityId.m_entityOffset = aznumeric_cast(i); const NetEntityId netEntityId = NextId(); @@ -493,57 +490,19 @@ namespace Multiplayer } } - void NetworkEntityManager::OnSpawned(AZ::Data::Asset spawnable) + void NetworkEntityManager::SetupNetEntity(AZ::Entity* netEntity, PrefabEntityId prefabEntityId, NetEntityRole netEntityRole) { - AzFramework::Spawnable* spawnableData = spawnable.GetAs(); - const auto& entityList = spawnableData->GetEntities(); - if (entityList.size() == 0) + auto* netBindComponent = netEntity->FindComponent(); + + if (netBindComponent) { - AZ_Error("NetworkEntityManager", false, "OnSpawned: Spawnable %s doesn't have any entities.", - spawnable.GetHint().c_str()); - return; + const NetEntityId netEntityId = NextId(); + netBindComponent->PreInit(netEntity, prefabEntityId, netEntityId, netEntityRole); } - - const auto& rootEntity = entityList[0]; - auto* spawnableHolder = rootEntity->FindComponent(); - if (!spawnableHolder) + else { - // Root entity doesn't have NetworkSpawnableHolderComponent. It means there's no corresponding network spawnable. - return; + AZ_Error("NetworkEntityManager", false, "SetupNetEntity called for an entity with no NetBindComponent. Entity: %s", + netEntity->GetName().c_str()); } - - AZ::Data::Asset netSpawnableAsset = spawnableHolder->GetNetworkSpawnableAsset(); - AzFramework::Spawnable* netSpawnable = netSpawnableAsset.GetAs(); - if (!netSpawnable) - { - // TODO: Temp sync load until JsonSerialization of loadBehavior is fixed. - netSpawnableAsset = AZ::Data::AssetManager::Instance().GetAsset( - netSpawnableAsset.GetId(), AZ::Data::AssetLoadBehavior::PreLoad); - AZ::Data::AssetManager::Instance().BlockUntilLoadComplete(netSpawnableAsset); - - netSpawnable = netSpawnableAsset.GetAs(); - } - - if (!netSpawnable) - { - AZ_Error("NetworkEntityManager", false, "OnRootSpawnableAssigned: Net spawnable doesn't have any data."); - return; - } - - auto* multiplayer = GetMultiplayer(); - - const auto agentType = multiplayer->GetAgentType(); - const bool spawnImmediately = - (agentType == MultiplayerAgentType::ClientServer || agentType == MultiplayerAgentType::DedicatedServer); - - if (spawnImmediately) - { - CreateEntitiesImmediate(*netSpawnable, NetEntityRole::Authority); - } - } - - void NetworkEntityManager::OnDespawned([[maybe_unused]]AZ::Data::Asset spawnable) - { - // TODO: Remove entities instantiated from the spawnable } } diff --git a/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkEntityManager.h b/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkEntityManager.h index e763e7ebca..482f2767e1 100644 --- a/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkEntityManager.h +++ b/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkEntityManager.h @@ -62,6 +62,8 @@ namespace Multiplayer const AZ::Transform& transform ) override; + void SetupNetEntity(AZ::Entity* netEntity, PrefabEntityId prefabEntityId, NetEntityRole netEntityRole) override; + uint32_t GetEntityCount() const override; NetworkEntityHandle AddEntityToEntityMap(NetEntityId netEntityId, AZ::Entity* entity) override; void MarkForRemoval(const ConstNetworkEntityHandle& entityHandle) override; @@ -93,9 +95,6 @@ namespace Multiplayer void RemoveEntities(); NetEntityId NextId(); - void OnSpawned(AZ::Data::Asset spawnable); - void OnDespawned(AZ::Data::Asset spawnable); - NetworkEntityTracker m_networkEntityTracker; NetworkEntityAuthorityTracker m_networkEntityAuthorityTracker; MultiplayerComponentRegistry m_multiplayerComponentRegistry; @@ -123,8 +122,5 @@ namespace Multiplayer DeferredRpcMessages m_localDeferredRpcMessages; NetworkSpawnableLibrary m_networkPrefabLibrary; - - AZ::Event>::Handler m_onSpawnedHandler; - AZ::Event>::Handler m_onDespawnedHandler; }; } diff --git a/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkSpawnableLibrary.cpp b/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkSpawnableLibrary.cpp index ebf5d2609b..935744f807 100644 --- a/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkSpawnableLibrary.cpp +++ b/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkSpawnableLibrary.cpp @@ -14,20 +14,23 @@ #include #include #include +#include namespace Multiplayer { NetworkSpawnableLibrary::NetworkSpawnableLibrary() { + AZ::Interface::Register(this); AzFramework::AssetCatalogEventBus::Handler::BusConnect(); } NetworkSpawnableLibrary::~NetworkSpawnableLibrary() { AzFramework::AssetCatalogEventBus::Handler::BusDisconnect(); + AZ::Interface::Unregister(this); } - void NetworkSpawnableLibrary::BuildPrefabsList() + void NetworkSpawnableLibrary::BuildSpawnablesList() { auto enumerateCallback = [this](const AZ::Data::AssetId id, const AZ::Data::AssetInfo& info) { @@ -50,10 +53,10 @@ namespace Multiplayer void NetworkSpawnableLibrary::OnCatalogLoaded([[maybe_unused]] const char* catalogFile) { - BuildPrefabsList(); + BuildSpawnablesList(); } - AZ::Name NetworkSpawnableLibrary::GetPrefabNameFromAssetId(AZ::Data::AssetId assetId) + AZ::Name NetworkSpawnableLibrary::GetSpawnableNameFromAssetId(AZ::Data::AssetId assetId) { if (assetId.IsValid()) { diff --git a/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkSpawnableLibrary.h b/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkSpawnableLibrary.h index a2c3d4ae56..6ebe2a7418 100644 --- a/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkSpawnableLibrary.h +++ b/Gems/Multiplayer/Code/Source/NetworkEntity/NetworkSpawnableLibrary.h @@ -12,30 +12,31 @@ #pragma once -#include -#include +#include #include -#include namespace Multiplayer { /// Implementation of the network prefab library interface. class NetworkSpawnableLibrary final - : private AzFramework::AssetCatalogEventBus::Handler + : public INetworkSpawnableLibrary + , private AzFramework::AssetCatalogEventBus::Handler { public: + AZ_RTTI(NetworkSpawnableLibrary, "{65E15F33-E893-49C2-A8E2-B6A8A6EF31E0}", INetworkSpawnableLibrary); + NetworkSpawnableLibrary(); ~NetworkSpawnableLibrary(); - void BuildPrefabsList(); - void ProcessSpawnableAsset(const AZStd::string& relativePath, AZ::Data::AssetId id); + /// INetworkSpawnableLibrary overrides. + void BuildSpawnablesList() override; + void ProcessSpawnableAsset(const AZStd::string& relativePath, AZ::Data::AssetId id) override; + AZ::Name GetSpawnableNameFromAssetId(AZ::Data::AssetId assetId) override; + AZ::Data::AssetId GetAssetIdByName(AZ::Name name) override; /// AssetCatalogEventBus overrides. void OnCatalogLoaded(const char* catalogFile) override; - AZ::Name GetPrefabNameFromAssetId(AZ::Data::AssetId assetId); - AZ::Data::AssetId GetAssetIdByName(AZ::Name name); - private: AZStd::unordered_map m_spawnables; AZStd::unordered_map m_spawnablesReverseLookup; diff --git a/Gems/Multiplayer/Code/Source/Pipeline/NetBindMarkerComponent.cpp b/Gems/Multiplayer/Code/Source/Pipeline/NetBindMarkerComponent.cpp index 84983b700e..1696e851a5 100644 --- a/Gems/Multiplayer/Code/Source/Pipeline/NetBindMarkerComponent.cpp +++ b/Gems/Multiplayer/Code/Source/Pipeline/NetBindMarkerComponent.cpp @@ -11,7 +11,12 @@ */ #include +#include #include +#include +#include +#include +#include namespace Multiplayer { @@ -21,15 +26,91 @@ namespace Multiplayer if (serializeContext) { serializeContext->Class() - ->Version(1); + ->Version(1) + ->Field("NetEntityIndex", &NetBindMarkerComponent::m_netEntityIndex) + ->Field("NetSpawnableAsset", &NetBindMarkerComponent::m_networkSpawnableAsset); } } + AzFramework::Spawnable* GetSpawnableFromAsset(AZ::Data::Asset& asset) + { + AzFramework::Spawnable* spawnable = asset.GetAs(); + if (!spawnable) + { + asset = + AZ::Data::AssetManager::Instance().GetAsset(asset.GetId(), AZ::Data::AssetLoadBehavior::PreLoad); + AZ::Data::AssetManager::Instance().BlockUntilLoadComplete(asset); + + spawnable = asset.GetAs(); + } + + return spawnable; + } + + void NetBindMarkerComponent::Activate() { + const auto agentType = AZ::Interface::Get()->GetAgentType(); + const bool spawnImmediately = + (agentType == MultiplayerAgentType::ClientServer || agentType == MultiplayerAgentType::DedicatedServer); + + if (spawnImmediately && m_networkSpawnableAsset.GetId().IsValid()) + { + AZ::Transform worldTm = GetEntity()->FindComponent()->GetWorldTM(); + auto preInsertionCallback = + [worldTm = AZStd::move(worldTm), netEntityIndex = m_netEntityIndex, spawnableAssetId = m_networkSpawnableAsset.GetId()] + (AzFramework::EntitySpawnTicket&, AzFramework::SpawnableEntityContainerView entities) + { + if (entities.size() == 1) + { + AZ::Entity* netEntity = *entities.begin(); + + auto* transformComponent = netEntity->FindComponent(); + transformComponent->SetWorldTM(worldTm); + + AZ::Name spawnableName = AZ::Interface::Get()->GetSpawnableNameFromAssetId(spawnableAssetId); + PrefabEntityId prefabEntityId; + prefabEntityId.m_prefabName = spawnableName; + prefabEntityId.m_entityOffset = netEntityIndex; + AZ::Interface::Get()->SetupNetEntity(netEntity, prefabEntityId, NetEntityRole::Authority); + } + else + { + AZ_Error("NetBindMarkerComponent", false, "Requested to spawn 1 entity, but received %d", entities.size()); + } + }; + + m_netSpawnTicket = AzFramework::EntitySpawnTicket(m_networkSpawnableAsset); + AzFramework::SpawnableEntitiesInterface::Get()->SpawnEntities(m_netSpawnTicket, {m_netEntityIndex}, preInsertionCallback); + } } void NetBindMarkerComponent::Deactivate() { + if(m_netSpawnTicket.IsValid()) + { + AzFramework::SpawnableEntitiesInterface::Get()->DespawnAllEntities(m_netSpawnTicket); + } } + + size_t NetBindMarkerComponent::GetNetEntityIndex() const + { + return m_netEntityIndex; + } + + void NetBindMarkerComponent::SetNetEntityIndex(size_t netEntityIndex) + { + m_netEntityIndex = netEntityIndex; + } + + void NetBindMarkerComponent::SetNetworkSpawnableAsset(AZ::Data::Asset networkSpawnableAsset) + { + m_networkSpawnableAsset = networkSpawnableAsset; + } + + AZ::Data::Asset NetBindMarkerComponent::GetNetworkSpawnableAsset() const + { + return m_networkSpawnableAsset; + } + } diff --git a/Gems/Multiplayer/Code/Source/Pipeline/NetBindMarkerComponent.h b/Gems/Multiplayer/Code/Source/Pipeline/NetBindMarkerComponent.h index ebafead73c..5ab42ab3aa 100644 --- a/Gems/Multiplayer/Code/Source/Pipeline/NetBindMarkerComponent.h +++ b/Gems/Multiplayer/Code/Source/Pipeline/NetBindMarkerComponent.h @@ -13,6 +13,9 @@ #pragma once #include +#include +#include +#include namespace Multiplayer { @@ -34,6 +37,15 @@ namespace Multiplayer void Deactivate() override; //! @} + size_t GetNetEntityIndex() const; + void SetNetEntityIndex(size_t val); + + void SetNetworkSpawnableAsset(AZ::Data::Asset networkSpawnableAsset); + AZ::Data::Asset GetNetworkSpawnableAsset() const; + private: + AZ::Data::Asset m_networkSpawnableAsset{AZ::Data::AssetLoadBehavior::PreLoad}; + size_t m_netEntityIndex = 0; + AzFramework::EntitySpawnTicket m_netSpawnTicket; }; } // namespace Multiplayer diff --git a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp index 0eed82b6ab..6c61e71ff7 100644 --- a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp +++ b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp @@ -114,29 +114,31 @@ namespace Multiplayer AZStd::unique_ptr networkInstance(aznew Instance()); - for (auto entityId : networkedEntityIds) + AZ::Data::Asset networkSpawnableAsset; + networkSpawnableAsset.Create(networkSpawnable->GetId()); + networkSpawnableAsset.SetAutoLoadBehavior(AZ::Data::AssetLoadBehavior::PreLoad); + + for (size_t entityIndex = 0; entityIndex < networkedEntityIds.size(); ++entityIndex) { + AZ::EntityId entityId = networkedEntityIds[entityIndex]; AZ::Entity* netEntity = sourceInstance->DetachEntity(entityId).release(); networkInstance->AddEntity(*netEntity); AZ::Entity* breadcrumbEntity = aznew AZ::Entity(netEntity->GetName()); breadcrumbEntity->SetRuntimeActiveByDefault(netEntity->IsRuntimeActiveByDefault()); - breadcrumbEntity->CreateComponent(); + NetBindMarkerComponent* netBindMarkerComponent = breadcrumbEntity->CreateComponent(); + // Each spawnable has a root meta-data entity at position 0, so starting net indices from 1 + netBindMarkerComponent->SetNetEntityIndex(entityIndex + 1); + netBindMarkerComponent->SetNetworkSpawnableAsset(networkSpawnableAsset); AzFramework::TransformComponent* transformComponent = netEntity->FindComponent(); breadcrumbEntity->CreateComponent(*transformComponent); - // TODO: Configure NetBindMarkerComponent to refer to the net entity sourceInstance->AddEntity(*breadcrumbEntity); } // Add net spawnable asset holder { - AZ::Data::AssetId assetId = networkSpawnable->GetId(); - AZ::Data::Asset networkSpawnableAsset; - networkSpawnableAsset.Create(assetId); - networkSpawnableAsset.SetAutoLoadBehavior(AZ::Data::AssetLoadBehavior::PreLoad); - EntityOptionalReference containerEntityRef = sourceInstance->GetContainerEntity(); if (containerEntityRef.has_value()) { diff --git a/Gems/Multiplayer/Code/Tests/MainTools.cpp b/Gems/Multiplayer/Code/Tests/MainTools.cpp index 65a1d921a9..56ad963dd9 100644 --- a/Gems/Multiplayer/Code/Tests/MainTools.cpp +++ b/Gems/Multiplayer/Code/Tests/MainTools.cpp @@ -15,7 +15,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp b/Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp index df1da11725..dfc74a230c 100644 --- a/Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp +++ b/Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp @@ -16,7 +16,7 @@ #include #include #include -#include +#include #include namespace UnitTest diff --git a/Gems/Multiplayer/Code/multiplayer_files.cmake b/Gems/Multiplayer/Code/multiplayer_files.cmake index 26909cbfd3..4bb1b769cc 100644 --- a/Gems/Multiplayer/Code/multiplayer_files.cmake +++ b/Gems/Multiplayer/Code/multiplayer_files.cmake @@ -15,6 +15,7 @@ set(FILES Include/Multiplayer/IMultiplayer.h Include/Multiplayer/IMultiplayerComponentInput.h Include/Multiplayer/INetworkEntityManager.h + Include/Multiplayer/INetworkSpawnableLibrary.h Include/Multiplayer/INetworkPlayerSpawner.h Include/Multiplayer/INetworkTime.h Include/Multiplayer/IReplicationWindow.h From b758a1920f8891e14a960683abc347316fffb885 Mon Sep 17 00:00:00 2001 From: pereslav Date: Mon, 17 May 2021 23:44:40 +0100 Subject: [PATCH 10/21] removed useless todo comment --- .../AzFramework/Spawnable/SpawnableEntitiesManager.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp index b18307ff7f..6799ea9353 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp @@ -211,8 +211,6 @@ namespace AzFramework AZ_Assert(clone != nullptr, "Failed to clone spawnable entity."); clone->SetId(AZ::Entity::MakeId()); - // Need to inject a callback here - GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, clone); return clone; } From 2ddbd36f9a7ff537bb98b05d8894326fc7636c81 Mon Sep 17 00:00:00 2001 From: scottr Date: Mon, 17 May 2021 16:34:24 -0700 Subject: [PATCH 11/21] [cpack_installer] add option to specify license url. replicate online artifacts copy --- cmake/Packaging.cmake | 12 ++++++++---- .../Windows/PackagingBootstrapper.wxs | 18 +++++++++++++----- .../Platform/Windows/PackagingPostBuild.cmake | 19 +++++++++++++++++++ 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/cmake/Packaging.cmake b/cmake/Packaging.cmake index 72216847f1..0fbd70e54c 100644 --- a/cmake/Packaging.cmake +++ b/cmake/Packaging.cmake @@ -13,10 +13,14 @@ if(NOT PAL_TRAIT_BUILD_CPACK_SUPPORTED) return() endif() -# set the common cpack variables first so they are accessible via configure_file -# when the platforms specific properties are applied below +# public facing options will eventually be converted into cpack specific ones below. +# all variables with the "CPACK_" prefix will automatically be cached for use in any +# of the build steps cpack runs e.g. pre-build, standard build, post-build. set(LY_INSTALLER_DOWNLOAD_URL "" CACHE STRING "URL embded into the installer to download additional artifacts") +set(LY_INSTALLER_LICENSE_URL "" CACHE STRING "Optionally embed a link to the license instead of raw text") +# set all common cpack variable overrides first so they can be accessible via configure_file +# when the platform specific settings are applied below set(CPACK_PACKAGE_VENDOR "${PROJECT_NAME}") set(CPACK_PACKAGE_VERSION "${LY_VERSION_STRING}") set(CPACK_PACKAGE_DESCRIPTION_SUMMARY "Installation Tool") @@ -28,11 +32,11 @@ set(DEFAULT_LICENSE_NAME "Apache-2.0") set(DEFAULT_LICENSE_FILE "${CMAKE_SOURCE_DIR}/LICENSE.txt") set(CPACK_RESOURCE_FILE_LICENSE ${DEFAULT_LICENSE_FILE}) +set(CPACK_LICENSE_URL ${LY_INSTALLER_LICENSE_URL}) set(CPACK_PACKAGE_INSTALL_DIRECTORY "${CPACK_PACKAGE_VENDOR}/${CPACK_PACKAGE_VERSION}") -# CMAKE_SOURCE_DIR doesn't equate to anything during execution of pre/post build scripts. -# to pass it down, we can utilize the auto-caching of any variable with prefix "CPACK_" +# CMAKE_SOURCE_DIR doesn't equate to anything during execution of pre/post build scripts set(CPACK_SOURCE_DIR ${CMAKE_SOURCE_DIR}/cmake) # attempt to apply platform specific settings diff --git a/cmake/Platform/Windows/PackagingBootstrapper.wxs b/cmake/Platform/Windows/PackagingBootstrapper.wxs index 711b60d854..f231f05413 100644 --- a/cmake/Platform/Windows/PackagingBootstrapper.wxs +++ b/cmake/Platform/Windows/PackagingBootstrapper.wxs @@ -16,11 +16,19 @@ Value="[ProgramFiles64Folder]$(var.CPACK_PACKAGE_INSTALL_DIRECTORY)" bal:Overridable="yes"/> - - - + + + + + + + + + Date: Mon, 17 May 2021 17:10:50 -0700 Subject: [PATCH 12/21] Fix bugs with creating prefabs with nested entities --- .../Instance/InstanceUpdateExecutor.cpp | 8 +++-- .../AzToolsFramework/Prefab/Link/Link.cpp | 10 +++--- .../Prefab/PrefabPublicHandler.cpp | 34 +++++++++++++++---- 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutor.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutor.cpp index 80ac86c974..6dfdfef39b 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutor.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutor.cpp @@ -120,9 +120,13 @@ namespace AzToolsFramework } } - auto findInstancesResult = m_templateInstanceMapperInterface->FindInstancesOwnedByTemplate(instanceTemplateId)->get(); + auto findInstancesResult = m_templateInstanceMapperInterface->FindInstancesOwnedByTemplate(instanceTemplateId); + AZ_Assert( + findInstancesResult.has_value(), "Prefab Instances corresponding to template with id %llu couldn't be found.", + instanceTemplateId); - if (findInstancesResult.find(instanceToUpdate) == findInstancesResult.end()) + if (findInstancesResult == AZStd::nullopt || + findInstancesResult->get().find(instanceToUpdate) == findInstancesResult->get().end()) { // Since nested instances get reconstructed during propagation, remove any nested instance that no longer // maps to a template. diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Link/Link.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Link/Link.cpp index e0834ed53b..308749ab28 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Link/Link.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Link/Link.cpp @@ -182,16 +182,16 @@ namespace AzToolsFramework else { AZ::JsonSerializationResult::ResultCode applyPatchResult = AZ::JsonSerialization::ApplyPatch( - linkedInstanceDom, + sourceTemplateDomCopy, targetTemplatePrefabDom.GetAllocator(), - sourceTemplatePrefabDom, patchesReference->get(), AZ::JsonMergeApproach::JsonPatch); + linkedInstanceDom.CopyFrom(sourceTemplateDomCopy, targetTemplatePrefabDom.GetAllocator()); if (applyPatchResult.GetProcessing() != AZ::JsonSerializationResult::Processing::Completed) { - AZ_Error("Prefab", false, - "Link::UpdateTarget - " - "ApplyPatches failed for Prefab DOM from source Template '%u' and target Template '%u'.", + AZ_Error( + "Prefab", false, + "Link::UpdateTarget - ApplyPatches failed for Prefab DOM from source Template '%u' and target Template '%u'.", m_sourceTemplateId, m_targetTemplateId); return false; } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp index 5ecff637a1..9bf101ccf5 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp @@ -63,7 +63,7 @@ namespace AzToolsFramework PrefabOperationResult PrefabPublicHandler::CreatePrefab(const AZStd::vector& entityIds, AZ::IO::PathView filePath) { - EntityList inputEntityList, topLevelEntities; + EntityList inputEntityList, topLevelEntities, topLevelNonContainerEntities; AZ::EntityId commonRootEntityId; InstanceOptionalReference commonRootEntityOwningInstance; PrefabOperationResult findCommonRootOutcome = FindCommonRootOwningInstance( @@ -73,6 +73,14 @@ namespace AzToolsFramework return findCommonRootOutcome; } + for (AZ::Entity* toplevelentity : topLevelEntities) + { + if (!IsInstanceContainerEntity(toplevelentity->GetId())) + { + topLevelNonContainerEntities.push_back(toplevelentity); + } + } + InstanceOptionalReference instanceToCreate; { // Initialize Undo Batch object @@ -122,11 +130,11 @@ namespace AzToolsFramework AZ::EntityId containerEntityId = instanceToCreate->get().GetContainerEntityId(); - // Parent the entities to the container entity. Parenting the container entities of the instances passed to createPrefab - // will be done during the creation of links below. - for (AZ::Entity* topLevelEntity : entities) + // Parent the non-container top level entities to the container entity. + // Parenting the top level container entities will be done during the creation of links. + for (AZ::Entity* entity : topLevelNonContainerEntities) { - AZ::TransformBus::Event(topLevelEntity->GetId(), &AZ::TransformBus::Events::SetParent, containerEntityId); + AZ::TransformBus::Event(entity->GetId(), &AZ::TransformBus::Events::SetParent, containerEntityId); } // Update the template of the instance since the entities are modified since the template creation. @@ -142,11 +150,25 @@ namespace AzToolsFramework AZ_Assert( nestedInstanceContainerEntity, "Invalid container entity found for the nested instance used in prefab creation."); + AZ::EntityId parentId; + AZ::TransformBus::EventResult( + parentId, nestedInstanceContainerEntity->get().GetId(), &AZ::TransformBus::Events::GetParentId); + + auto entityIterator = AZStd::find_if( + entities.begin(), entities.end(), [parentId](AZ::Entity* entity) { return entity->GetId() == parentId; }); + + // If the previous parent entity of the nested instance is not part of the entities of the newly created prefab, + // then set the parent of the nested prefab as the container entity of the newly created prefab. + if (entityIterator == entities.end()) + { + parentId = containerEntityId; + } + // These link creations shouldn't be undone because that would put the template in a non-usable state if a user // chooses to instantiate the template after undoing the creation. CreateLink( {&nestedInstanceContainerEntity->get()}, *nestedInstance, instanceToCreate->get().GetTemplateId(), - undoBatch.GetUndoBatch(), containerEntityId, false); + undoBatch.GetUndoBatch(), parentId, false); }); // Create a link between the templates of the newly created instance and the instance it's being parented under. From 761a77a4363deda44d79cf6e1f93bf80639c57bf Mon Sep 17 00:00:00 2001 From: srikappa Date: Mon, 17 May 2021 18:51:23 -0700 Subject: [PATCH 13/21] Avoid creating a new list for non-container top level entities --- .../Prefab/PrefabPublicHandler.cpp | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp index 9f466db520..e6bb8c7dee 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp @@ -63,7 +63,7 @@ namespace AzToolsFramework PrefabOperationResult PrefabPublicHandler::CreatePrefab(const AZStd::vector& entityIds, AZ::IO::PathView filePath) { - EntityList inputEntityList, topLevelEntities, topLevelNonContainerEntities; + EntityList inputEntityList, topLevelEntities; AZ::EntityId commonRootEntityId; InstanceOptionalReference commonRootEntityOwningInstance; PrefabOperationResult findCommonRootOutcome = FindCommonRootOwningInstance( @@ -73,14 +73,6 @@ namespace AzToolsFramework return findCommonRootOutcome; } - for (AZ::Entity* toplevelentity : topLevelEntities) - { - if (!IsInstanceContainerEntity(toplevelentity->GetId())) - { - topLevelNonContainerEntities.push_back(toplevelentity); - } - } - InstanceOptionalReference instanceToCreate; { // Initialize Undo Batch object @@ -132,9 +124,12 @@ namespace AzToolsFramework // Parent the non-container top level entities to the container entity. // Parenting the top level container entities will be done during the creation of links. - for (AZ::Entity* entity : topLevelNonContainerEntities) + for (AZ::Entity* topLevelEntity : topLevelEntities) { - AZ::TransformBus::Event(entity->GetId(), &AZ::TransformBus::Events::SetParent, containerEntityId); + if (!IsInstanceContainerEntity(topLevelEntity->GetId())) + { + AZ::TransformBus::Event(topLevelEntity->GetId(), &AZ::TransformBus::Events::SetParent, containerEntityId); + } } // Update the template of the instance since the entities are modified since the template creation. From 37b2ac797d9c78fb557051301ec52a146e5d3f57 Mon Sep 17 00:00:00 2001 From: scottr Date: Mon, 17 May 2021 20:52:06 -0700 Subject: [PATCH 14/21] [cpack_installer] bootstrap installer is copied to root of build directory. uploads directory is cleaned before copied to. --- cmake/Platform/Windows/PackagingPostBuild.cmake | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/cmake/Platform/Windows/PackagingPostBuild.cmake b/cmake/Platform/Windows/PackagingPostBuild.cmake index bfe725ab3b..cfed90e155 100644 --- a/cmake/Platform/Windows/PackagingPostBuild.cmake +++ b/cmake/Platform/Windows/PackagingPostBuild.cmake @@ -54,7 +54,7 @@ set(_light_command -o "${_bootstrap_output_file}" ) -message(STATUS "Creating Installer Bootstrapper...") +message(STATUS "Creating Bootstrap Installer...") execute_process( COMMAND ${_candle_command} COMMAND_ERROR_IS_FATAL ANY @@ -64,6 +64,12 @@ execute_process( COMMAND_ERROR_IS_FATAL ANY ) +file(COPY ${_bootstrap_output_file} + DESTINATION ${CPACK_PACKAGE_DIRECTORY} +) + +message(STATUS "Bootstrap installer generated to ${CPACK_PACKAGE_DIRECTORY}/${_bootstrap_filename}") + # use the internal default path if somehow not specified from cpack_configure_downloads if(NOT CPACK_UPLOAD_DIRECTORY) set(CPACK_UPLOAD_DIRECTORY ${CPACK_PACKAGE_DIRECTORY}/CPackUploads) @@ -73,6 +79,7 @@ endif() # through cpack_configure_downloads. this mimics the same process cpack does natively for # some other frameworks that have built-in online installer support. message(STATUS "Copying installer artifacts to upload directory...") +file(REMOVE_RECURSE ${CPACK_UPLOAD_DIRECTORY}) file(GLOB _artifacts "${_cpack_wix_out_dir}/*.msi" "${_cpack_wix_out_dir}/*.cab") file(COPY ${_artifacts} DESTINATION ${CPACK_UPLOAD_DIRECTORY} From efbb0077b5c8c79a73aa1a6a77a967e15ed81a19 Mon Sep 17 00:00:00 2001 From: scottr Date: Mon, 17 May 2021 21:43:46 -0700 Subject: [PATCH 15/21] [cpack_installer] configure install to be per machine --- cmake/Platform/Windows/PackagingTemplate.wxs.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/Platform/Windows/PackagingTemplate.wxs.in b/cmake/Platform/Windows/PackagingTemplate.wxs.in index fd3610259a..0b3c597ab6 100644 --- a/cmake/Platform/Windows/PackagingTemplate.wxs.in +++ b/cmake/Platform/Windows/PackagingTemplate.wxs.in @@ -12,7 +12,7 @@ Manufacturer="$(var.CPACK_PACKAGE_VENDOR)" UpgradeCode="$(var.CPACK_WIX_UPGRADE_GUID)"> - + From 66ad040102cf1b60bf54dba93826e6a48370c6f3 Mon Sep 17 00:00:00 2001 From: scottr Date: Mon, 17 May 2021 22:51:48 -0700 Subject: [PATCH 16/21] [cpack_installer] some minor comment cleanup --- cmake/Packaging.cmake | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/cmake/Packaging.cmake b/cmake/Packaging.cmake index 0fbd70e54c..8f4e1134b9 100644 --- a/cmake/Packaging.cmake +++ b/cmake/Packaging.cmake @@ -13,14 +13,14 @@ if(NOT PAL_TRAIT_BUILD_CPACK_SUPPORTED) return() endif() -# public facing options will eventually be converted into cpack specific ones below. -# all variables with the "CPACK_" prefix will automatically be cached for use in any -# of the build steps cpack runs e.g. pre-build, standard build, post-build. +# public facing options will be used for conversion into cpack specific ones below. set(LY_INSTALLER_DOWNLOAD_URL "" CACHE STRING "URL embded into the installer to download additional artifacts") set(LY_INSTALLER_LICENSE_URL "" CACHE STRING "Optionally embed a link to the license instead of raw text") # set all common cpack variable overrides first so they can be accessible via configure_file -# when the platform specific settings are applied below +# when the platform specific settings are applied below. additionally, any variable with +# the "CPACK_" prefix will automatically be cached for use in any phase of cpack namely +# pre/post build set(CPACK_PACKAGE_VENDOR "${PROJECT_NAME}") set(CPACK_PACKAGE_VERSION "${LY_VERSION_STRING}") set(CPACK_PACKAGE_DESCRIPTION_SUMMARY "Installation Tool") @@ -91,6 +91,7 @@ ly_configure_cpack_component( ) if(LY_INSTALLER_DOWNLOAD_URL) + # this will set the following variables: CPACK_DOWNLOAD_SITE, CPACK_DOWNLOAD_ALL, and CPACK_UPLOAD_DIRECTORY cpack_configure_downloads( ${LY_INSTALLER_DOWNLOAD_URL} UPLOAD_DIRECTORY ${CMAKE_BINARY_DIR}/_CPack_Uploads # to match the _CPack_Packages directory From b45d01919dac7e4ff096f58ab81c422e99d4faa6 Mon Sep 17 00:00:00 2001 From: scottr Date: Tue, 18 May 2021 00:22:17 -0700 Subject: [PATCH 17/21] [cpack_installer] simplify guid generation by using existing project props instead of timestamp in seed value. add bootstrapper specific guids. --- .../Windows/PackagingBootstrapper.wxs | 2 +- .../Platform/Windows/PackagingPostBuild.cmake | 1 + .../Platform/Windows/Packaging_windows.cmake | 42 ++++++++----------- 3 files changed, 19 insertions(+), 26 deletions(-) diff --git a/cmake/Platform/Windows/PackagingBootstrapper.wxs b/cmake/Platform/Windows/PackagingBootstrapper.wxs index f231f05413..c3d1dd7a7b 100644 --- a/cmake/Platform/Windows/PackagingBootstrapper.wxs +++ b/cmake/Platform/Windows/PackagingBootstrapper.wxs @@ -8,7 +8,7 @@ Date: Tue, 18 May 2021 14:00:41 +0100 Subject: [PATCH 18/21] Fixed build --- Gems/Multiplayer/Code/Tests/MainTools.cpp | 2 +- Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gems/Multiplayer/Code/Tests/MainTools.cpp b/Gems/Multiplayer/Code/Tests/MainTools.cpp index 56ad963dd9..ccb4d568f8 100644 --- a/Gems/Multiplayer/Code/Tests/MainTools.cpp +++ b/Gems/Multiplayer/Code/Tests/MainTools.cpp @@ -15,7 +15,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp b/Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp index dfc74a230c..c441d2ba5f 100644 --- a/Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp +++ b/Gems/Multiplayer/Code/Tests/PrefabProcessingTests.cpp @@ -16,7 +16,7 @@ #include #include #include -#include +#include #include namespace UnitTest From f721aa511fec7ece5bb0bc05e83949e667151bd7 Mon Sep 17 00:00:00 2001 From: pereslav Date: Tue, 18 May 2021 17:15:24 +0100 Subject: [PATCH 19/21] Fixed entity IDs of the original & networked spawnables --- .../Code/Source/Pipeline/NetworkPrefabProcessor.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp index 2aae0ee9b5..93136ab261 100644 --- a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp +++ b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp @@ -121,19 +121,23 @@ namespace Multiplayer for (size_t entityIndex = 0; entityIndex < networkedEntityIds.size(); ++entityIndex) { AZ::EntityId entityId = networkedEntityIds[entityIndex]; - AZ::Entity* netEntity = sourceInstance->DetachEntity(entityId).release(); + AZ::Entity* netEntity = sourceInstance->DetachEntity(entityId).release(); + // Net entity will need a new ID to avoid IDs collision + netEntity->SetId(AZ::Entity::MakeId()); networkInstance->AddEntity(*netEntity); - AZ::Entity* breadcrumbEntity = aznew AZ::Entity(netEntity->GetName()); + // Use the old ID for the breadcrumb entity to keep parent-child relationship in the original spawnable + AZ::Entity* breadcrumbEntity = aznew AZ::Entity(entityId, netEntity->GetName()); breadcrumbEntity->SetRuntimeActiveByDefault(netEntity->IsRuntimeActiveByDefault()); + NetBindMarkerComponent* netBindMarkerComponent = breadcrumbEntity->CreateComponent(); // Each spawnable has a root meta-data entity at position 0, so starting net indices from 1 netBindMarkerComponent->SetNetEntityIndex(entityIndex + 1); netBindMarkerComponent->SetNetworkSpawnableAsset(networkSpawnableAsset); AzFramework::TransformComponent* transformComponent = netEntity->FindComponent(); breadcrumbEntity->CreateComponent(*transformComponent); - + sourceInstance->AddEntity(*breadcrumbEntity); } From 54dc47eb91e55e6c70da8b624017c86b338aa50b Mon Sep 17 00:00:00 2001 From: scottr Date: Tue, 18 May 2021 11:50:01 -0700 Subject: [PATCH 20/21] [cpack_installer] fixed typo in help string --- cmake/Packaging.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/Packaging.cmake b/cmake/Packaging.cmake index 8f4e1134b9..ba610b1883 100644 --- a/cmake/Packaging.cmake +++ b/cmake/Packaging.cmake @@ -14,7 +14,7 @@ if(NOT PAL_TRAIT_BUILD_CPACK_SUPPORTED) endif() # public facing options will be used for conversion into cpack specific ones below. -set(LY_INSTALLER_DOWNLOAD_URL "" CACHE STRING "URL embded into the installer to download additional artifacts") +set(LY_INSTALLER_DOWNLOAD_URL "" CACHE STRING "URL embedded into the installer to download additional artifacts") set(LY_INSTALLER_LICENSE_URL "" CACHE STRING "Optionally embed a link to the license instead of raw text") # set all common cpack variable overrides first so they can be accessible via configure_file From 3182dc37c3d708ac37bf02b0bcd871877312c27d Mon Sep 17 00:00:00 2001 From: Tommy Walton <82672795+amzn-tommy@users.noreply.github.com> Date: Tue, 18 May 2021 13:52:01 -0700 Subject: [PATCH 21/21] Fix for ATOM-15488 : Rendering out an animation with shadows crashes the editor (#794) Previously, the SkinnedMeshFeatureProcessor assumed there would only be one skinning pass. However, that's not always the case. When rendering with track view, the feature processor was getting a pass that only updated once every three frames, which could lead to a condition where a skinned mesh was released, but the pass never submitted and cleared the previously added dispatch items, and one or two frames later it would go to submit after the skinned mesh and all of its resources had already been released. -Modified the skinning and morph target compute passes to pull dispatch items from the feature processor instead of the feature processor pushing them to the passes. -If more than one skinning (or morph target) pass is active in the frame, whichever one is first will submit all the dispatch items, and clear the feature processor's dispatch items before the next one tries to submit anything -Moved the logic for caching shader options from the SkinnedMeshComputePass to the SkinnedMeshFeatureProcessor, since there may be more than one pass but only one feature processor per scene --- .../MorphTargets/MorphTargetComputePass.cpp | 32 ++-- .../MorphTargets/MorphTargetComputePass.h | 8 +- .../MorphTargets/MorphTargetDispatchItem.cpp | 6 +- .../MorphTargets/MorphTargetDispatchItem.h | 4 +- .../SkinnedMesh/SkinnedMeshComputePass.cpp | 39 ++--- .../SkinnedMesh/SkinnedMeshComputePass.h | 11 +- .../SkinnedMesh/SkinnedMeshDispatchItem.cpp | 8 +- .../SkinnedMesh/SkinnedMeshDispatchItem.h | 4 +- .../SkinnedMeshFeatureProcessor.cpp | 144 ++++++++++-------- .../SkinnedMesh/SkinnedMeshFeatureProcessor.h | 25 ++- .../SkinnedMesh/SkinnedMeshRenderProxy.cpp | 14 +- 11 files changed, 149 insertions(+), 146 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Source/MorphTargets/MorphTargetComputePass.cpp b/Gems/Atom/Feature/Common/Code/Source/MorphTargets/MorphTargetComputePass.cpp index 35793b82a3..1bb537ecef 100644 --- a/Gems/Atom/Feature/Common/Code/Source/MorphTargets/MorphTargetComputePass.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/MorphTargets/MorphTargetComputePass.cpp @@ -12,6 +12,7 @@ #include +#include #include #include @@ -38,6 +39,11 @@ namespace AZ return m_shader; } + void MorphTargetComputePass::SetFeatureProcessor(SkinnedMeshFeatureProcessor* skinnedMeshFeatureProcessor) + { + m_skinnedMeshFeatureProcessor = skinnedMeshFeatureProcessor; + } + void MorphTargetComputePass::BuildAttachmentsInternal() { // The same buffer that skinning writes to is used to manage the computed vertex deltas that are passed from the @@ -45,30 +51,16 @@ namespace AZ AttachBufferToSlot(Name{ "MorphTargetDeltaOutput" }, SkinnedMeshOutputStreamManagerInterface::Get()->GetBuffer()); } - void MorphTargetComputePass::AddDispatchItem(const RHI::DispatchItem* dispatchItem) - { - AZ_Assert(dispatchItem != nullptr, "invalid dispatchItem"); - - AZStd::lock_guard lock(m_mutex); - //using an unordered_set here to prevent redundantly adding the same dispatchItem to the submission queue - //(i.e. if the same morph target exists in multiple views, it can call AddDispatchItem multiple times with the same item) - m_dispatches.insert(dispatchItem); - } - void MorphTargetComputePass::BuildCommandListInternal(const RHI::FrameGraphExecuteContext& context) { - RHI::CommandList* commandList = context.GetCommandList(); - - SetSrgsForDispatch(commandList); - - AZStd::lock_guard lock(m_mutex); - for (const RHI::DispatchItem* dispatchItem : m_dispatches) + if (m_skinnedMeshFeatureProcessor) { - commandList->Submit(*dispatchItem); - } + RHI::CommandList* commandList = context.GetCommandList(); - // Clear the dispatch items. They will need to be re-populated next frame - m_dispatches.clear(); + SetSrgsForDispatch(commandList); + + m_skinnedMeshFeatureProcessor->SubmitMorphTargetDispatchItems(commandList); + } } } // namespace Render } // namespace AZ diff --git a/Gems/Atom/Feature/Common/Code/Source/MorphTargets/MorphTargetComputePass.h b/Gems/Atom/Feature/Common/Code/Source/MorphTargets/MorphTargetComputePass.h index 3967d9190e..61fa485fbc 100644 --- a/Gems/Atom/Feature/Common/Code/Source/MorphTargets/MorphTargetComputePass.h +++ b/Gems/Atom/Feature/Common/Code/Source/MorphTargets/MorphTargetComputePass.h @@ -18,6 +18,8 @@ namespace AZ { namespace Render { + class SkinnedMeshFeatureProcessor; + //! The morph target compute pass submits dispatch items for morph targets. The dispatch items are cleared every frame, so it needs to be re-populated. class MorphTargetComputePass : public RPI::ComputePass @@ -31,16 +33,14 @@ namespace AZ static RPI::Ptr Create(const RPI::PassDescriptor& descriptor); - //! Thread-safe function for adding a dispatch item to the current frame. - void AddDispatchItem(const RHI::DispatchItem* dispatchItem); Data::Instance GetShader() const; + void SetFeatureProcessor(SkinnedMeshFeatureProcessor* m_skinnedMeshFeatureProcessor); private: void BuildAttachmentsInternal() override; void BuildCommandListInternal(const RHI::FrameGraphExecuteContext& context) override; - AZStd::mutex m_mutex; - AZStd::unordered_set m_dispatches; + SkinnedMeshFeatureProcessor* m_skinnedMeshFeatureProcessor = nullptr; }; } } diff --git a/Gems/Atom/Feature/Common/Code/Source/MorphTargets/MorphTargetDispatchItem.cpp b/Gems/Atom/Feature/Common/Code/Source/MorphTargets/MorphTargetDispatchItem.cpp index d7bbc315ed..7b3aedd64e 100644 --- a/Gems/Atom/Feature/Common/Code/Source/MorphTargets/MorphTargetDispatchItem.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/MorphTargets/MorphTargetDispatchItem.cpp @@ -11,7 +11,7 @@ */ #include -#include +#include #include #include @@ -30,7 +30,7 @@ namespace AZ MorphTargetDispatchItem::MorphTargetDispatchItem( const AZStd::intrusive_ptr inputBuffers, const MorphTargetMetaData& morphTargetMetaData, - RPI::Ptr morphTargetComputePass, + SkinnedMeshFeatureProcessor* skinnedMeshFeatureProcessor, MorphTargetInstanceMetaData morphInstanceMetaData, float morphDeltaIntegerEncoding) : m_inputBuffers(inputBuffers) @@ -38,7 +38,7 @@ namespace AZ , m_morphInstanceMetaData(morphInstanceMetaData) , m_accumulatedDeltaIntegerEncoding(morphDeltaIntegerEncoding) { - m_morphTargetShader = morphTargetComputePass->GetShader(); + m_morphTargetShader = skinnedMeshFeatureProcessor->GetMorphTargetShader(); RPI::ShaderReloadNotificationBus::Handler::BusConnect(m_morphTargetShader->GetAssetId()); } diff --git a/Gems/Atom/Feature/Common/Code/Source/MorphTargets/MorphTargetDispatchItem.h b/Gems/Atom/Feature/Common/Code/Source/MorphTargets/MorphTargetDispatchItem.h index 46680eb465..ad1fd969a5 100644 --- a/Gems/Atom/Feature/Common/Code/Source/MorphTargets/MorphTargetDispatchItem.h +++ b/Gems/Atom/Feature/Common/Code/Source/MorphTargets/MorphTargetDispatchItem.h @@ -37,7 +37,7 @@ namespace AZ namespace Render { - class MorphTargetComputePass; + class SkinnedMeshFeatureProcessor; //! Holds and manages an RHI DispatchItem for a specific morph target, and the resources that are needed to build and maintain it. class MorphTargetDispatchItem @@ -51,7 +51,7 @@ namespace AZ explicit MorphTargetDispatchItem( const AZStd::intrusive_ptr inputBuffers, const MorphTargetMetaData& morphTargetMetaData, - RPI::Ptr morphTargetComputePass, + SkinnedMeshFeatureProcessor* skinnedMeshFeatureProcessor, MorphTargetInstanceMetaData morphInstanceMetaData, float accumulatedDeltaRange ); diff --git a/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshComputePass.cpp b/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshComputePass.cpp index d7b2c605b4..a3feddb0b6 100644 --- a/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshComputePass.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshComputePass.cpp @@ -12,6 +12,7 @@ #include +#include #include #include @@ -22,11 +23,9 @@ namespace AZ { namespace Render { - SkinnedMeshComputePass::SkinnedMeshComputePass(const RPI::PassDescriptor& descriptor) : RPI::ComputePass(descriptor) { - m_cachedShaderOptions.SetShader(m_shader); } RPI::Ptr SkinnedMeshComputePass::Create(const RPI::PassDescriptor& descriptor) @@ -40,42 +39,30 @@ namespace AZ return m_shader; } - RPI::ShaderOptionGroup SkinnedMeshComputePass::CreateShaderOptionGroup(const SkinnedMeshShaderOptions shaderOptions, SkinnedMeshShaderOptionNotificationBus::Handler& shaderReinitializedHandler) + void SkinnedMeshComputePass::SetFeatureProcessor(SkinnedMeshFeatureProcessor* skinnedMeshFeatureProcessor) { - m_cachedShaderOptions.ConnectToShaderReinitializedEvent(shaderReinitializedHandler); - return m_cachedShaderOptions.CreateShaderOptionGroup(shaderOptions); - } - - void SkinnedMeshComputePass::AddDispatchItem(const RHI::DispatchItem* dispatchItem) - { - AZ_Assert(dispatchItem != nullptr, "invalid dispatchItem"); - - AZStd::lock_guard lock(m_mutex); - //using an unordered_set here to prevent redundantly adding the same dispatchItem to the submission queue - //(i.e. if the same skinnedMesh exists in multiple views, it can call AddDispatchItem multiple times with the same item) - m_dispatches.insert(dispatchItem); + m_skinnedMeshFeatureProcessor = skinnedMeshFeatureProcessor; } void SkinnedMeshComputePass::BuildCommandListInternal(const RHI::FrameGraphExecuteContext& context) { - RHI::CommandList* commandList = context.GetCommandList(); - - SetSrgsForDispatch(commandList); - - AZStd::lock_guard lock(m_mutex); - for (const RHI::DispatchItem* dispatchItem : m_dispatches) + if (m_skinnedMeshFeatureProcessor) { - commandList->Submit(*dispatchItem); - } + RHI::CommandList* commandList = context.GetCommandList(); - // Clear the dispatch items. They will need to be re-populated next frame - m_dispatches.clear(); + SetSrgsForDispatch(commandList); + + m_skinnedMeshFeatureProcessor->SubmitSkinningDispatchItems(commandList); + } } void SkinnedMeshComputePass::OnShaderReinitialized(const RPI::Shader& shader) { ComputePass::OnShaderReinitialized(shader); - m_cachedShaderOptions.SetShader(m_shader); + if (m_skinnedMeshFeatureProcessor) + { + m_skinnedMeshFeatureProcessor->OnSkinningShaderReinitialized(m_shader); + } } void SkinnedMeshComputePass::OnShaderVariantReinitialized(const RPI::Shader& shader, const RPI::ShaderVariantId&, RPI::ShaderVariantStableId) diff --git a/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshComputePass.h b/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshComputePass.h index d2e0fb77dc..5f7ff08e47 100644 --- a/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshComputePass.h +++ b/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshComputePass.h @@ -20,6 +20,8 @@ namespace AZ { namespace Render { + class SkinnedMeshFeatureProcessor; + //! The skinned mesh compute pass submits dispatch items for skinning. The dispatch items are cleared every frame, so it needs to be re-populated. class SkinnedMeshComputePass : public RPI::ComputePass @@ -33,10 +35,9 @@ namespace AZ static RPI::Ptr Create(const RPI::PassDescriptor& descriptor); - //! Thread-safe function for adding a dispatch item to the current frame. - void AddDispatchItem(const RHI::DispatchItem* dispatchItem); Data::Instance GetShader() const; - RPI::ShaderOptionGroup CreateShaderOptionGroup(const SkinnedMeshShaderOptions shaderOptions, SkinnedMeshShaderOptionNotificationBus::Handler& shaderReinitializedHandler); + + void SetFeatureProcessor(SkinnedMeshFeatureProcessor* m_skinnedMeshFeatureProcessor); private: void BuildCommandListInternal(const RHI::FrameGraphExecuteContext& context) override; @@ -45,9 +46,7 @@ namespace AZ void OnShaderReinitialized(const RPI::Shader& shader) override; void OnShaderVariantReinitialized(const RPI::Shader& shader, const RPI::ShaderVariantId& shaderVariantId, RPI::ShaderVariantStableId shaderVariantStableId) override; - AZStd::mutex m_mutex; - AZStd::unordered_set m_dispatches; - CachedSkinnedMeshShaderOptions m_cachedShaderOptions; + SkinnedMeshFeatureProcessor* m_skinnedMeshFeatureProcessor = nullptr; }; } } diff --git a/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshDispatchItem.cpp b/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshDispatchItem.cpp index c9054fc6c8..647a87a788 100644 --- a/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshDispatchItem.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshDispatchItem.cpp @@ -12,7 +12,7 @@ #include #include -#include +#include #include #include @@ -34,7 +34,7 @@ namespace AZ size_t lodIndex, Data::Instance boneTransforms, const SkinnedMeshShaderOptions& shaderOptions, - RPI::Ptr skinnedMeshComputePass, + SkinnedMeshFeatureProcessor* skinnedMeshFeatureProcessor, MorphTargetInstanceMetaData morphTargetInstanceMetaData, float morphTargetDeltaIntegerEncoding) : m_inputBuffers(inputBuffers) @@ -45,7 +45,7 @@ namespace AZ , m_morphTargetInstanceMetaData(morphTargetInstanceMetaData) , m_morphTargetDeltaIntegerEncoding(morphTargetDeltaIntegerEncoding) { - m_skinningShader = skinnedMeshComputePass->GetShader(); + m_skinningShader = skinnedMeshFeatureProcessor->GetSkinningShader(); // Shader options are generally set per-skinned mesh instance, but morph targets may only exist on some lods. Override the option for applying morph targets here if (m_morphTargetInstanceMetaData.m_accumulatedPositionDeltaOffsetInBytes != MorphTargetConstants::s_invalidDeltaOffset) @@ -58,7 +58,7 @@ namespace AZ } // CreateShaderOptionGroup will also connect to the SkinnedMeshShaderOptionNotificationBus - m_shaderOptionGroup = skinnedMeshComputePass->CreateShaderOptionGroup(m_shaderOptions, *this); + m_shaderOptionGroup = skinnedMeshFeatureProcessor->CreateSkinningShaderOptionGroup(m_shaderOptions, *this); } SkinnedMeshDispatchItem::~SkinnedMeshDispatchItem() diff --git a/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshDispatchItem.h b/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshDispatchItem.h index c80b002106..33bec89afd 100644 --- a/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshDispatchItem.h +++ b/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshDispatchItem.h @@ -38,7 +38,7 @@ namespace AZ namespace Render { - class SkinnedMeshComputePass; + class SkinnedMeshFeatureProcessor; //! Holds and manages an RHI DispatchItem for a specific skinned mesh, and the resources that are needed to build and maintain it. class SkinnedMeshDispatchItem @@ -55,7 +55,7 @@ namespace AZ size_t lodIndex, Data::Instance skinningMatrices, const SkinnedMeshShaderOptions& shaderOptions, - RPI::Ptr skinnedMeshComputePass, + SkinnedMeshFeatureProcessor* skinnedMeshFeatureProcessor, MorphTargetInstanceMetaData morphTargetInstanceMetaData, float morphTargetDeltaIntegerEncoding ); diff --git a/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshFeatureProcessor.cpp b/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshFeatureProcessor.cpp index 193a36e588..388f4112a0 100644 --- a/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshFeatureProcessor.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshFeatureProcessor.cpp @@ -24,8 +24,10 @@ #include #include #include +#include #include +#include #include #include @@ -84,11 +86,6 @@ namespace AZ AZ_PROFILE_FUNCTION(Debug::ProfileCategory::AzRender); AZ_ATOM_PROFILE_FUNCTION("SkinnedMesh", "SkinnedMeshFeatureProcessor: Render"); - if (!m_skinningPass) - { - return; - } - #if 0 //[GFX_TODO][ATOM-13564] Temporarily disable skinning culling until we figure out how to hook up visibility & lod selection with skinning: //Setup the culling workgroup (it will be re-used for each view) { @@ -132,7 +129,7 @@ namespace AZ //Dispatch the workgroup to each view for (const RPI::ViewPtr& viewPtr : packet.m_views) { - Job *processWorkgroupJob = AZ::CreateJobFunction( + Job* processWorkgroupJob = AZ::CreateJobFunction( [this, cullingSystem, viewPtr](AZ::Job& thisJob) { AZ_PROFILE_SCOPE_DYNAMIC(Debug::ProfileCategory::AzRender, "skinningMeshFP processWorkgroupJob - View: %s", viewPtr->GetName().GetCStr()); @@ -167,7 +164,16 @@ namespace AZ float maxScreenPercentage(lod.m_range.m_max); if (approxScreenPercentage >= minScreenPercentage && approxScreenPercentage <= maxScreenPercentage) { - m_skinningPass->AddDispatchItem(&renderProxy->m_dispatchItemsByLod[lodIndex]->GetRHIDispatchItem()); + AZStd::lock_guard lock(m_dispatchItemMutex); + m_skinningDispatches.insert(&renderProxy->m_dispatchItemsByLod[lodIndex]->GetRHIDispatchItem()); + for (size_t morphTargetIndex = 0; morphTargetIndex < renderProxy->m_morphTargetDispatchItemsByLod[lodIndex].size(); morphTargetIndex++) + { + const MorphTargetDispatchItem* dispatchItem = renderProxy->m_morphTargetDispatchItemsByLod[lodIndex][morphTargetIndex].get(); + if (dispatchItem && dispatchItem->GetWeight() > AZ::Constants::FloatEpsilon) + { + m_morphTargetDispatches.insert(&dispatchItem->GetRHIDispatchItem()); + } + } } } } @@ -232,13 +238,14 @@ namespace AZ //Note that this supports overlapping lod ranges (to support cross-fading lods, for example) if (approxScreenPercentage >= lod.m_screenCoverageMin && approxScreenPercentage <= lod.m_screenCoverageMax) { - m_skinningPass->AddDispatchItem(&renderProxy.m_dispatchItemsByLod[lodIndex]->GetRHIDispatchItem()); + AZStd::lock_guard lock(m_dispatchItemMutex); + m_skinningDispatches.insert(&renderProxy.m_dispatchItemsByLod[lodIndex]->GetRHIDispatchItem()); for (size_t morphTargetIndex = 0; morphTargetIndex < renderProxy.m_morphTargetDispatchItemsByLod[lodIndex].size(); morphTargetIndex++) { const MorphTargetDispatchItem* dispatchItem = renderProxy.m_morphTargetDispatchItemsByLod[lodIndex][morphTargetIndex].get(); if (dispatchItem && dispatchItem->GetWeight() > AZ::Constants::FloatEpsilon) { - m_morphTargetPass->AddDispatchItem(&dispatchItem->GetRHIDispatchItem()); + m_morphTargetDispatches.insert(&dispatchItem->GetRHIDispatchItem()); } } } @@ -248,19 +255,14 @@ namespace AZ #endif } - void SkinnedMeshFeatureProcessor::OnRenderPipelineAdded([[maybe_unused]] RPI::RenderPipelinePtr pipeline) + void SkinnedMeshFeatureProcessor::OnRenderPipelineAdded(RPI::RenderPipelinePtr pipeline) { - InitSkinningAndMorphPass(); + InitSkinningAndMorphPass(pipeline->GetRootPass()); } - void SkinnedMeshFeatureProcessor::OnRenderPipelineRemoved([[maybe_unused]] RPI::RenderPipeline* pipeline) + void SkinnedMeshFeatureProcessor::OnRenderPipelinePassesChanged(RPI::RenderPipeline* renderPipeline) { - InitSkinningAndMorphPass(); - } - - void SkinnedMeshFeatureProcessor::OnRenderPipelinePassesChanged([[maybe_unused]] RPI::RenderPipeline* renderPipeline) - { - InitSkinningAndMorphPass(); + InitSkinningAndMorphPass(renderPipeline->GetRootPass()); } void SkinnedMeshFeatureProcessor::OnBeginPrepareRender() @@ -268,9 +270,15 @@ namespace AZ m_renderProxiesChecker.soft_lock(); } - void SkinnedMeshFeatureProcessor::OnEndPrepareRender() + void SkinnedMeshFeatureProcessor::OnRenderEnd() { m_renderProxiesChecker.soft_unlock(); + + // Clear any dispatch items that were added but never submitted + // in case there were no passes that submitted this frame + // because they execute at a lower frequency + m_skinningDispatches.clear(); + m_morphTargetDispatches.clear(); } SkinnedMeshRenderProxyHandle SkinnedMeshFeatureProcessor::AcquireRenderProxy(const SkinnedMeshRenderProxyDesc& desc) @@ -295,61 +303,73 @@ namespace AZ return false; } - void SkinnedMeshFeatureProcessor::InitSkinningAndMorphPass() + void SkinnedMeshFeatureProcessor::InitSkinningAndMorphPass(const RPI::Ptr pipelineRootPass) { - m_skinningPass = nullptr; //reset it to null, just in case it fails to load the assets properly - m_morphTargetPass = nullptr; - - RPI::PassSystemInterface* passSystem = RPI::PassSystemInterface::Get(); - if (passSystem->HasPassesForTemplateName(AZ::Name{ "SkinningPassTemplate" })) + RPI::Ptr skinningPass = pipelineRootPass->FindPassByNameRecursive(AZ::Name{ "SkinningPass" }); + if (skinningPass) { - auto& skinningPasses = passSystem->GetPassesForTemplateName(AZ::Name{ "SkinningPassTemplate" }); + SkinnedMeshComputePass* skinnedMeshComputePass = azdynamic_cast(skinningPass.get()); + skinnedMeshComputePass->SetFeatureProcessor(this); - // For now, assume one skinning pass - if (!skinningPasses.empty() && skinningPasses[0]) + // There may be multiple skinning passes in the scene due to multiple pipelines, but there is only one skinning shader + m_skinningShader = skinnedMeshComputePass->GetShader(); + + if (!m_skinningShader) { - m_skinningPass = static_cast(skinningPasses[0]); - const Data::Instance shader = m_skinningPass->GetShader(); - - if (!shader) - { - AZ_Error(s_featureProcessorName, false, "Failed to get skinning pass shader. It may need to finish processing."); - } + AZ_Error(s_featureProcessorName, false, "Failed to get skinning pass shader. It may need to finish processing."); } else { - AZ_Error(s_featureProcessorName, false, "\"SkinningPassTemplate\" does not have any valid passes. Check your game project's .pass assets."); + m_cachedSkinningShaderOptions.SetShader(m_skinningShader); } } - else - { - AZ_Error(s_featureProcessorName, false, "Failed to find passes for \"SkinningPassTemplate\". Check your game project's .pass assets."); - } - if (passSystem->HasPassesForTemplateName(AZ::Name{ "MorphTargetPassTemplate" })) + RPI::Ptr morphTargetPass = pipelineRootPass->FindPassByNameRecursive(AZ::Name{ "MorphTargetPass" }); + if (morphTargetPass) { - auto& morphTargetPasses = passSystem->GetPassesForTemplateName(AZ::Name{ "MorphTargetPassTemplate" }); + MorphTargetComputePass* morphTargetComputePass = azdynamic_cast(morphTargetPass.get()); + morphTargetComputePass->SetFeatureProcessor(this); - // For now, assume one skinning pass - if (!morphTargetPasses.empty() && morphTargetPasses[0]) + // There may be multiple morph target passes in the scene due to multiple pipelines, but there is only one morph target shader + m_morphTargetShader = morphTargetComputePass->GetShader(); + + if (!m_morphTargetShader) { - m_morphTargetPass = static_cast(morphTargetPasses[0]); - const Data::Instance shader = m_morphTargetPass->GetShader(); + AZ_Error(s_featureProcessorName, false, "Failed to get morph target pass shader. It may need to finish processing."); + } + } + } - if (!shader) - { - AZ_Error(s_featureProcessorName, false, "Failed to get morph target pass shader. It may need to finish processing."); - } - } - else - { - AZ_Error(s_featureProcessorName, false, "\"MorphTargetPassTemplate\" does not have any valid passes. Check your game project's .pass assets."); - } - } - else + RPI::ShaderOptionGroup SkinnedMeshFeatureProcessor::CreateSkinningShaderOptionGroup(const SkinnedMeshShaderOptions shaderOptions, SkinnedMeshShaderOptionNotificationBus::Handler& shaderReinitializedHandler) + { + m_cachedSkinningShaderOptions.ConnectToShaderReinitializedEvent(shaderReinitializedHandler); + return m_cachedSkinningShaderOptions.CreateShaderOptionGroup(shaderOptions); + } + + void SkinnedMeshFeatureProcessor::OnSkinningShaderReinitialized(const Data::Instance skinningShader) + { + m_skinningShader = skinningShader; + m_cachedSkinningShaderOptions.SetShader(m_skinningShader); + } + + void SkinnedMeshFeatureProcessor::SubmitSkinningDispatchItems(RHI::CommandList* commandList) + { + AZStd::lock_guard lock(m_dispatchItemMutex); + for (const RHI::DispatchItem* dispatchItem : m_skinningDispatches) { - AZ_Error(s_featureProcessorName, false, "Failed to find passes for \"MorphTargetPassTemplate\". Check your game project's .pass assets."); + commandList->Submit(*dispatchItem); } + m_skinningDispatches.clear(); + } + + void SkinnedMeshFeatureProcessor::SubmitMorphTargetDispatchItems(RHI::CommandList* commandList) + { + AZStd::lock_guard lock(m_dispatchItemMutex); + for (const RHI::DispatchItem* dispatchItem : m_morphTargetDispatches) + { + commandList->Submit(*dispatchItem); + } + m_morphTargetDispatches.clear(); } SkinnedMeshRenderProxyInterfaceHandle SkinnedMeshFeatureProcessor::AcquireRenderProxyInterface(const SkinnedMeshRenderProxyDesc& desc) @@ -363,14 +383,14 @@ namespace AZ return ReleaseRenderProxy(handle); } - RPI::Ptr SkinnedMeshFeatureProcessor::GetSkinningPass() const + Data::Instance SkinnedMeshFeatureProcessor::GetSkinningShader() const { - return m_skinningPass; + return m_skinningShader; } - RPI::Ptr SkinnedMeshFeatureProcessor::GetMorphTargetPass() const + Data::Instance SkinnedMeshFeatureProcessor::GetMorphTargetShader() const { - return m_morphTargetPass; + return m_morphTargetShader; } } // namespace Render } // namespace AZ diff --git a/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshFeatureProcessor.h b/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshFeatureProcessor.h index 86dedfa161..bb3dd242a1 100644 --- a/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshFeatureProcessor.h +++ b/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshFeatureProcessor.h @@ -51,35 +51,46 @@ namespace AZ void Deactivate() override; void Simulate(const FeatureProcessor::SimulatePacket& packet) override; void Render(const FeatureProcessor::RenderPacket& packet) override; + void OnRenderEnd() override; // RPI::SceneNotificationBus overrides ... void OnRenderPipelineAdded(RPI::RenderPipelinePtr pipeline) override; - void OnRenderPipelineRemoved(RPI::RenderPipeline* pipeline) override; void OnRenderPipelinePassesChanged(RPI::RenderPipeline* renderPipeline) override; void OnBeginPrepareRender() override; - void OnEndPrepareRender() override; SkinnedMeshRenderProxyHandle AcquireRenderProxy(const SkinnedMeshRenderProxyDesc& desc); bool ReleaseRenderProxy(SkinnedMeshRenderProxyHandle& handle); - RPI::Ptr GetSkinningPass() const; - RPI::Ptr GetMorphTargetPass() const; + Data::Instance GetSkinningShader() const; + RPI::ShaderOptionGroup CreateSkinningShaderOptionGroup(const SkinnedMeshShaderOptions shaderOptions, SkinnedMeshShaderOptionNotificationBus::Handler& shaderReinitializedHandler); + void OnSkinningShaderReinitialized(const Data::Instance skinningShader); + void SubmitSkinningDispatchItems(RHI::CommandList* commandList); + + Data::Instance GetMorphTargetShader() const; + void SubmitMorphTargetDispatchItems(RHI::CommandList* commandList); private: AZ_DISABLE_COPY_MOVE(SkinnedMeshFeatureProcessor); - void InitSkinningAndMorphPass(); + void InitSkinningAndMorphPass(const RPI::Ptr pipelineRootPass); SkinnedMeshRenderProxyInterfaceHandle AcquireRenderProxyInterface(const SkinnedMeshRenderProxyDesc& desc) override; bool ReleaseRenderProxyInterface(SkinnedMeshRenderProxyInterfaceHandle& handle) override; static const char* s_featureProcessorName; - RPI::Ptr m_skinningPass; - RPI::Ptr m_morphTargetPass; + + Data::Instance m_skinningShader; + CachedSkinnedMeshShaderOptions m_cachedSkinningShaderOptions; + + Data::Instance m_morphTargetShader; + AZStd::concurrency_checker m_renderProxiesChecker; StableDynamicArray m_renderProxies; AZStd::unique_ptr m_statsCollector; MeshFeatureProcessor* m_meshFeatureProcessor = nullptr; + AZStd::unordered_set m_skinningDispatches; + AZStd::unordered_set m_morphTargetDispatches; + AZStd::mutex m_dispatchItemMutex; }; } // namespace Render diff --git a/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshRenderProxy.cpp b/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshRenderProxy.cpp index 90c54dfc10..090b720406 100644 --- a/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshRenderProxy.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/SkinnedMesh/SkinnedMeshRenderProxy.cpp @@ -60,13 +60,7 @@ namespace AZ bool SkinnedMeshRenderProxy::BuildDispatchItem([[maybe_unused]] const RPI::Scene& scene, size_t modelLodIndex, [[maybe_unused]] const SkinnedMeshShaderOptions& shaderOptions) { - if (!m_featureProcessor->GetSkinningPass()) - { - AZ_Error("Skinned Mesh Feature Processor", false, "Failed to get Skinning Pass. Make sure the project has a skinning pass."); - return false; - } - - Data::Instance skinningShader = m_featureProcessor->GetSkinningPass()->GetShader(); + Data::Instance skinningShader = m_featureProcessor->GetSkinningShader(); if (!skinningShader) { AZ_Error("Skinned Mesh Feature Processor", false, "Failed to get skinning shader from skinning pass"); @@ -89,7 +83,7 @@ namespace AZ m_instance->m_outputStreamOffsetsInBytes[modelLodIndex], modelLodIndex, m_boneTransforms, m_shaderOptions, - m_featureProcessor->GetSkinningPass(), + m_featureProcessor, m_instance->m_morphTargetInstanceMetaData[modelLodIndex], morphDeltaIntegerEncoding }); @@ -100,7 +94,7 @@ namespace AZ } // Get the data needed to create a morph target dispatch item - Data::Instance morphTargetShader = m_featureProcessor->GetMorphTargetPass()->GetShader(); + Data::Instance morphTargetShader = m_featureProcessor->GetMorphTargetShader(); const AZStd::vector>& morphTargetInputBuffersVector = m_inputBuffers->GetMorphTargetInputBuffers(modelLodIndex); AZ_Assert(morphTargetMetaDatas.size() == morphTargetInputBuffersVector.size(), "Skinned Mesh Feature Processor - Mismatch in morph target metadata count and morph target input buffer count"); @@ -118,7 +112,7 @@ namespace AZ aznew MorphTargetDispatchItem{ morphTargetInputBuffersVector[morphTargetIndex], morphTargetMetaDatas[morphTargetIndex], - m_featureProcessor->GetMorphTargetPass(), + m_featureProcessor, m_instance->m_morphTargetInstanceMetaData[modelLodIndex], morphDeltaIntegerEncoding });