From dc336ffabaf6173e0659a8ddca879d548b12ba48 Mon Sep 17 00:00:00 2001 From: Sergey Pereslavtsev Date: Tue, 26 Oct 2021 16:10:35 +0100 Subject: [PATCH 1/4] Added validation to net prefab processing to error if there are hierarchical entities without Net hierarchy components Signed-off-by: Sergey Pereslavtsev --- .../Pipeline/NetworkPrefabProcessor.cpp | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp index 27e59c0bf4..e9b7ace7c9 100644 --- a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp +++ b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp @@ -17,6 +17,8 @@ #include #include #include +#include +#include namespace Multiplayer { @@ -93,6 +95,41 @@ namespace Multiplayer }); } + static bool HasNetHierarchyComponents(const AZ::Entity* entity) + { + return (entity->FindComponent() != nullptr) + || (entity->FindComponent() != nullptr); + } + + static void ValidateNetHierarchies(const AZStd::vector& prefabNetEntities) + { + AZStd::unordered_map prefabEntitesMap; + + for (auto* entity : prefabNetEntities) + { + auto* entityTransform = entity->FindComponent(); + AZ_Assert(entityTransform, "Net entities have to have Transform Component"); + + AZ::EntityId parentId = entityTransform->GetParentId(); + + // The input entities array is sorted in the parent to children order, + // so we only need to check against already iterated entities + if (parentId.IsValid() && prefabEntitesMap.contains(parentId)) + { + AZ::Entity* parentEntity = prefabEntitesMap[parentId]; + bool parentValid = HasNetHierarchyComponents(parentEntity); + AZ_Error("NetworkPrefabProcessor", parentValid, "Parent Entity %s must have a Network Hierarchy component assigned", + parentEntity->GetName().c_str()); + + bool childValid = HasNetHierarchyComponents(entity); + AZ_Error("NetworkPrefabProcessor", childValid, "Child Entity %s must have a Network Hierarchy component assigned", + entity->GetName().c_str()); + } + + prefabEntitesMap[entity->GetId()] = entity; + } + } + void NetworkPrefabProcessor::ProcessPrefab(PrefabProcessorContext& context, AZStd::string_view prefabName, PrefabDom& prefab) { using namespace AzToolsFramework::Prefab; @@ -131,6 +168,9 @@ namespace Multiplayer // Sort the entities prior to processing. The entities will end up in the net spawnable in this order. SpawnableUtils::SortEntitiesByTransformHierarchy(prefabNetEntities); + // Here we validate the hierarchical net entities have one of Network Hierarchy components. + ValidateNetHierarchies(prefabNetEntities); + // Create an asset for our future network spawnable: this allows us to put references to the asset in the components AZ::Data::Asset networkSpawnableAsset; networkSpawnableAsset.Create(networkSpawnable->GetId()); From 348bb7d37e5872cdfe36c310c4591551d512c357 Mon Sep 17 00:00:00 2001 From: Sergey Pereslavtsev Date: Thu, 28 Oct 2021 13:49:08 +0100 Subject: [PATCH 2/4] Added setreg option for network spawnables serialization format Signed-off-by: Sergey Pereslavtsev --- .../Pipeline/NetworkPrefabProcessor.cpp | 32 +++++++++++++++---- .../Source/Pipeline/NetworkPrefabProcessor.h | 19 ++++++++++- Gems/Multiplayer/Registry/prefab.tools.setreg | 10 ++++-- 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp index e9b7ace7c9..7e0e29030a 100644 --- a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp +++ b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp @@ -33,8 +33,10 @@ namespace Multiplayer mpTools->SetDidProcessNetworkPrefabs(false); } - context.ListPrefabs([&context](AZStd::string_view prefabName, PrefabDom& prefab) { - ProcessPrefab(context, prefabName, prefab); + AZ::DataStream::StreamType serializationFormat = GetAzSerializationFormat(); + + context.ListPrefabs([&context, serializationFormat](AZStd::string_view prefabName, PrefabDom& prefab) { + ProcessPrefab(context, prefabName, prefab, serializationFormat); }); if (mpTools && !context.GetProcessedObjects().empty()) @@ -47,7 +49,15 @@ namespace Multiplayer { if (auto* serializeContext = azrtti_cast(context); serializeContext != nullptr) { - serializeContext->Class()->Version(2); + serializeContext->Enum() + ->Value("Binary", SerializationFormats::Binary) + ->Value("Text", SerializationFormats::Text) + ; + + serializeContext->Class() + ->Version(3) + ->Field("SerializationFormat", &NetworkPrefabProcessor::m_serializationFormat) + ; } } @@ -130,7 +140,7 @@ namespace Multiplayer } } - void NetworkPrefabProcessor::ProcessPrefab(PrefabProcessorContext& context, AZStd::string_view prefabName, PrefabDom& prefab) + void NetworkPrefabProcessor::ProcessPrefab(PrefabProcessorContext& context, AZStd::string_view prefabName, PrefabDom& prefab, AZ::DataStream::StreamType serializationFormat) { using namespace AzToolsFramework::Prefab; @@ -144,10 +154,10 @@ namespace Multiplayer AZStd::string uniqueName = prefabName; uniqueName += ".network.spawnable"; - auto serializer = [](AZStd::vector& output, const ProcessedObjectStore& object) -> bool { + auto serializer = [serializationFormat](AZStd::vector& output, const ProcessedObjectStore& object) -> bool { AZ::IO::ByteContainerStream stream(&output); auto& asset = object.GetAsset(); - return AZ::Utils::SaveObjectToStream(stream, AZ::DataStream::ST_BINARY, &asset, asset.GetType()); + return AZ::Utils::SaveObjectToStream(stream, serializationFormat, &asset, asset.GetType()); }; auto&& [object, networkSpawnable] = @@ -218,4 +228,14 @@ namespace Multiplayer context.GetProcessedObjects().push_back(AZStd::move(object)); } + + AZ::DataStream::StreamType NetworkPrefabProcessor::GetAzSerializationFormat() const + { + if (m_serializationFormat == SerializationFormats::Text) + { + return AZ::DataStream::StreamType::ST_JSON; + } + + return AZ::DataStream::StreamType::ST_BINARY; + } } diff --git a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.h b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.h index 6eb0c2b4de..0fd3529db7 100644 --- a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.h +++ b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.h @@ -9,6 +9,7 @@ #pragma once #include +#include namespace AzToolsFramework::Prefab::PrefabConversionUtils { @@ -33,7 +34,23 @@ namespace Multiplayer static void Reflect(AZ::ReflectContext* context); + //! The format the network spawnables are going to be stored in. + enum class SerializationFormats + { + Binary, //!< Binary is generally preferable for performance. + Text //!< Store in text format which is usually slower but helps with debugging. + }; + + AZ::DataStream::StreamType GetAzSerializationFormat() const; + protected: - static void ProcessPrefab(PrefabProcessorContext& context, AZStd::string_view prefabName, PrefabDom& prefab); + static void ProcessPrefab(PrefabProcessorContext& context, AZStd::string_view prefabName, PrefabDom& prefab, AZ::DataStream::StreamType serializationFormat); + + SerializationFormats m_serializationFormat = SerializationFormats::Binary; }; } + +namespace AZ +{ + AZ_TYPE_INFO_SPECIALIZE(Multiplayer::NetworkPrefabProcessor::SerializationFormats, "{F69B49EB-9D67-4D9C-99E7-DFA35D4ACCD2}"); +} diff --git a/Gems/Multiplayer/Registry/prefab.tools.setreg b/Gems/Multiplayer/Registry/prefab.tools.setreg index 7f25cf9a43..256f2d189a 100644 --- a/Gems/Multiplayer/Registry/prefab.tools.setreg +++ b/Gems/Multiplayer/Registry/prefab.tools.setreg @@ -18,8 +18,14 @@ "GameObjectCreation": [ { "$type": "AzToolsFramework::Prefab::PrefabConversionUtils::EditorInfoRemover" }, - { "$type": "Multiplayer::NetworkPrefabProcessor" }, - { "$type": "AzToolsFramework::Prefab::PrefabConversionUtils::PrefabCatchmentProcessor" } + { + "$type": "Multiplayer::NetworkPrefabProcessor", + "SerializationFormat": "Binary" // Options are "Binary" (default) or "Text". Prefer "Binary" for performance. + }, + { + "$type": "AzToolsFramework::Prefab::PrefabConversionUtils::PrefabCatchmentProcessor", + "SerializationFormat": "Binary" // Options are "Binary" (default) or "Text". Prefer "Binary" for performance. + } ] } } From 43e6731714c6d93919d81d1a5088456a519a4ffc Mon Sep 17 00:00:00 2001 From: Sergey Pereslavtsev Date: Thu, 28 Oct 2021 15:17:11 +0100 Subject: [PATCH 3/4] Removed hierarchy validation since it is not always applicable Signed-off-by: Sergey Pereslavtsev --- .../Pipeline/NetworkPrefabProcessor.cpp | 38 ------------------- 1 file changed, 38 deletions(-) diff --git a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp index 7e0e29030a..19d997b8d1 100644 --- a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp +++ b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp @@ -105,41 +105,6 @@ namespace Multiplayer }); } - static bool HasNetHierarchyComponents(const AZ::Entity* entity) - { - return (entity->FindComponent() != nullptr) - || (entity->FindComponent() != nullptr); - } - - static void ValidateNetHierarchies(const AZStd::vector& prefabNetEntities) - { - AZStd::unordered_map prefabEntitesMap; - - for (auto* entity : prefabNetEntities) - { - auto* entityTransform = entity->FindComponent(); - AZ_Assert(entityTransform, "Net entities have to have Transform Component"); - - AZ::EntityId parentId = entityTransform->GetParentId(); - - // The input entities array is sorted in the parent to children order, - // so we only need to check against already iterated entities - if (parentId.IsValid() && prefabEntitesMap.contains(parentId)) - { - AZ::Entity* parentEntity = prefabEntitesMap[parentId]; - bool parentValid = HasNetHierarchyComponents(parentEntity); - AZ_Error("NetworkPrefabProcessor", parentValid, "Parent Entity %s must have a Network Hierarchy component assigned", - parentEntity->GetName().c_str()); - - bool childValid = HasNetHierarchyComponents(entity); - AZ_Error("NetworkPrefabProcessor", childValid, "Child Entity %s must have a Network Hierarchy component assigned", - entity->GetName().c_str()); - } - - prefabEntitesMap[entity->GetId()] = entity; - } - } - void NetworkPrefabProcessor::ProcessPrefab(PrefabProcessorContext& context, AZStd::string_view prefabName, PrefabDom& prefab, AZ::DataStream::StreamType serializationFormat) { using namespace AzToolsFramework::Prefab; @@ -178,9 +143,6 @@ namespace Multiplayer // Sort the entities prior to processing. The entities will end up in the net spawnable in this order. SpawnableUtils::SortEntitiesByTransformHierarchy(prefabNetEntities); - // Here we validate the hierarchical net entities have one of Network Hierarchy components. - ValidateNetHierarchies(prefabNetEntities); - // Create an asset for our future network spawnable: this allows us to put references to the asset in the components AZ::Data::Asset networkSpawnableAsset; networkSpawnableAsset.Create(networkSpawnable->GetId()); From 9109b9de8dc2a72c06db4746b95019e78e0e92ca Mon Sep 17 00:00:00 2001 From: Sergey Pereslavtsev Date: Thu, 28 Oct 2021 15:19:11 +0100 Subject: [PATCH 4/4] Removed includes Signed-off-by: Sergey Pereslavtsev --- .../Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp index 19d997b8d1..bec659180c 100644 --- a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp +++ b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp @@ -17,8 +17,6 @@ #include #include #include -#include -#include namespace Multiplayer {