From 134f07eb5a4de3c43ab3060e5c6c17b9d698e04b Mon Sep 17 00:00:00 2001 From: chcurran <82187351+carlitosan@users.noreply.github.com> Date: Wed, 21 Apr 2021 21:36:31 -0700 Subject: [PATCH] Fixes for node palette exclusion, restrict asset variable type to fix LYN-3090 --- .../Widgets/NodePalette/NodePaletteModel.cpp | 60 +++----- .../VariablePaletteTableView.cpp | 4 +- .../ScriptCanvas/Core/SlotConfigurations.cpp | 6 +- .../Code/Include/ScriptCanvas/Data/Data.cpp | 23 --- .../Code/Include/ScriptCanvas/Data/Data.h | 2 - .../ScriptCanvas/Data/DataRegistry.cpp | 29 +++- .../Include/ScriptCanvas/Data/DataRegistry.h | 12 +- .../Include/ScriptCanvas/SystemComponent.h | 3 + .../Code/Source/SystemComponent.cpp | 134 ++++++++++-------- 9 files changed, 140 insertions(+), 133 deletions(-) diff --git a/Gems/ScriptCanvas/Code/Editor/View/Widgets/NodePalette/NodePaletteModel.cpp b/Gems/ScriptCanvas/Code/Editor/View/Widgets/NodePalette/NodePaletteModel.cpp index 4154c3982e..2edc950e6f 100644 --- a/Gems/ScriptCanvas/Code/Editor/View/Widgets/NodePalette/NodePaletteModel.cpp +++ b/Gems/ScriptCanvas/Code/Editor/View/Widgets/NodePalette/NodePaletteModel.cpp @@ -57,7 +57,7 @@ namespace { if (excludeAttributeData) { - AZ::u64 exclusionFlags = AZ::Script::Attributes::ExcludeFlags::List | AZ::Script::Attributes::ExcludeFlags::ListOnly | AZ::ScriptCanvasAttributes::VariableCreationForbidden; + AZ::u64 exclusionFlags = AZ::Script::Attributes::ExcludeFlags::List | AZ::Script::Attributes::ExcludeFlags::ListOnly; if (typeId == AzToolsFramework::Components::EditorComponentBase::TYPEINFO_Uuid()) { @@ -143,11 +143,6 @@ namespace { return; } - - if (!ScriptCanvas::Data::IsAllowedBehaviorClassVariableType(behaviorClass->m_typeId)) - { - return; - } } const auto isExposableOutcome = ScriptCanvas::IsExposable(method); @@ -479,38 +474,24 @@ namespace continue; } - // Only bind Behavior Classes marked with the Scope type of Launcher + if (auto excludeFromPointer = AZ::FindAttribute(AZ::Script::Attributes::ExcludeFrom, behaviorClass->m_attributes)) + { + AZ::Script::Attributes::ExcludeFlags excludeFlags{}; + AZ::AttributeReader(nullptr, excludeFromPointer).Read(excludeFlags); + + if ((excludeFlags & (AZ::Script::Attributes::ExcludeFlags::List | AZ::Script::Attributes::ExcludeFlags::ListOnly)) != 0) + { + continue; + } + } + if (!AZ::Internal::IsInScope(behaviorClass->m_attributes, AZ::Script::Attributes::ScopeFlags::Launcher)) { - continue; // skip this class + continue; } // Objects and Object methods { - bool canCreate = serializeContext->FindClassData(behaviorClass->m_typeId) != nullptr && - !HasAttribute(behaviorClass, AZ::ScriptCanvasAttributes::VariableCreationForbidden); - - // In order to create variables, the class must have full memory support - canCreate = canCreate && - (behaviorClass->m_allocate - && behaviorClass->m_cloner - && behaviorClass->m_mover - && behaviorClass->m_destructor - && behaviorClass->m_deallocate); - - if (canCreate) - { - // Do not allow variable creation for data that derives from AZ::Component - for (auto base : behaviorClass->m_baseClasses) - { - if (AZ::Component::TYPEINFO_Uuid() == base) - { - canCreate = false; - break; - } - } - } - AZStd::string categoryPath; AZStd::string translationContext = ScriptCanvasEditor::TranslationHelper::GetContextName(ScriptCanvasEditor::TranslationContextGroup::ClassMethod, behaviorClass->m_name); @@ -530,17 +511,14 @@ namespace } } - if (canCreate) - { - auto dataRegistry = ScriptCanvas::GetDataRegistry(); - ScriptCanvas::Data::Type type = dataRegistry->m_typeIdTraitMap[ScriptCanvas::Data::eType::BehaviorContextObject].m_dataTraits.GetSCType(behaviorClass->m_typeId); + auto dataRegistry = ScriptCanvas::GetDataRegistry(); + ScriptCanvas::Data::Type type = dataRegistry->m_typeIdTraitMap[ScriptCanvas::Data::eType::BehaviorContextObject].m_dataTraits.GetSCType(behaviorClass->m_typeId); - if (type.IsValid()) + if (type.IsValid()) + { + if (dataRegistry->m_creatableTypes.contains(type)) { - if (!AZ::FindAttribute(AZ::ScriptCanvasAttributes::AllowInternalCreation, behaviorClass->m_attributes)) - { - ScriptCanvasEditor::VariablePaletteRequestBus::Broadcast(&ScriptCanvasEditor::VariablePaletteRequests::RegisterVariableType, type); - } + ScriptCanvasEditor::VariablePaletteRequestBus::Broadcast(&ScriptCanvasEditor::VariablePaletteRequests::RegisterVariableType, type); } } diff --git a/Gems/ScriptCanvas/Code/Editor/View/Widgets/VariablePanel/VariablePaletteTableView.cpp b/Gems/ScriptCanvas/Code/Editor/View/Widgets/VariablePanel/VariablePaletteTableView.cpp index a05468d565..9bec796f53 100644 --- a/Gems/ScriptCanvas/Code/Editor/View/Widgets/VariablePanel/VariablePaletteTableView.cpp +++ b/Gems/ScriptCanvas/Code/Editor/View/Widgets/VariablePanel/VariablePaletteTableView.cpp @@ -122,8 +122,8 @@ namespace ScriptCanvasEditor for (const AZ::Uuid& objectId : objectTypes) { - // Verify whether this is an allowed BC variable type - if (!ScriptCanvas::Data::IsAllowedBehaviorClassVariableType(objectId)) + ScriptCanvas::Data::Type type = dataRegistry->m_typeIdTraitMap[ScriptCanvas::Data::eType::BehaviorContextObject].m_dataTraits.GetSCType(objectId); + if (!type.IsValid() || !dataRegistry->m_creatableTypes.contains(type)) { continue; } diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Core/SlotConfigurations.cpp b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Core/SlotConfigurations.cpp index bc6445e6e3..17dd68c084 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Core/SlotConfigurations.cpp +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Core/SlotConfigurations.cpp @@ -105,11 +105,9 @@ namespace ScriptCanvas void DataSlotConfiguration::SetType(const AZ::BehaviorParameter& typeDesc) { - auto dataRegistry = GetDataRegistry(); Data::Type scType = !AZ::BehaviorContextHelper::IsStringParameter(typeDesc) ? Data::FromAZType(typeDesc.m_typeId) : Data::Type::String(); - auto typeIter = dataRegistry->m_creatableTypes.find(scType); - - if (typeIter != dataRegistry->m_creatableTypes.end()) + auto dataRegistry = GetDataRegistry(); + if (dataRegistry->IsUseableInSlot(scType)) { m_datum.SetType(scType); } diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Data/Data.cpp b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Data/Data.cpp index e71c97e89a..2653b1495f 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Data/Data.cpp +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Data/Data.cpp @@ -389,29 +389,6 @@ namespace ScriptCanvas return AZ::Utils::IsVectorContainerType(ToAZType(type)); } - bool IsAllowedBehaviorClassVariableType(const AZ::Uuid& id) - { - AZ::BehaviorContext* behaviorContext = nullptr; - AZ::ComponentApplicationBus::BroadcastResult(behaviorContext, &AZ::ComponentApplicationRequests::GetBehaviorContext); - AZ_Assert(behaviorContext, "Unable to retrieve behavior context."); - - const auto& classIterator = behaviorContext->m_typeToClassMap.find(id); - if (classIterator != behaviorContext->m_typeToClassMap.end()) - { - AZ::BehaviorClass* behaviorClass = classIterator->second; - if (behaviorClass->FindAttribute(AZ::ScriptCanvasAttributes::VariableCreationForbidden)) - { - return false; - } - } - else - { - return false; - } - - return true; - } - bool IsSetContainerType(const AZ::Uuid& type) { return AZ::Utils::IsSetContainerType(type); diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Data/Data.h b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Data/Data.h index 64dc8999f7..59530d934d 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Data/Data.h +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Data/Data.h @@ -197,8 +197,6 @@ namespace ScriptCanvas bool IsVectorContainerType(const AZ::Uuid& type); bool IsVectorContainerType(const Type& type); - bool IsAllowedBehaviorClassVariableType(const AZ::Uuid& id); - AZStd::vector GetContainedTypes(const AZ::Uuid& type); AZStd::vector GetContainedTypes(const Type& type); AZStd::pair GetOutcomeTypes(const AZ::Uuid& type); diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Data/DataRegistry.cpp b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Data/DataRegistry.cpp index 85cdf56458..9068ba0172 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Data/DataRegistry.cpp +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Data/DataRegistry.cpp @@ -105,14 +105,24 @@ namespace ScriptCanvas AZ_Error("Script Canvas", it.second, "Cannot register a second Trait struct with the same ScriptCanvas type(%u)", it.first->first); } - void DataRegistry::RegisterType(const AZ::TypeId& typeId, TypeProperties typeProperties) + void DataRegistry::RegisterType(const AZ::TypeId& typeId, TypeProperties typeProperties, Createability registration) { Data::Type behaviorContextType = Data::FromAZType(typeId); if (behaviorContextType.GetType() == Data::eType::BehaviorContextObject && !behaviorContextType.GetAZType().IsNull()) { - if (m_creatableTypes.find(behaviorContextType) == m_creatableTypes.end()) + if (registration == Createability::SlotAndVariable) { - m_creatableTypes[behaviorContextType] = typeProperties; + if (m_creatableTypes.find(behaviorContextType) == m_creatableTypes.end()) + { + m_creatableTypes[behaviorContextType] = typeProperties; + } + } + else if (registration == Createability::SlotOnly) + { + if (m_slottableTypes.find(behaviorContextType) == m_slottableTypes.end()) + { + m_slottableTypes[behaviorContextType] = typeProperties; + } } } } @@ -125,4 +135,15 @@ namespace ScriptCanvas m_creatableTypes.erase(behaviorContextType); } } -} \ No newline at end of file + + bool DataRegistry::IsUseableInSlot(const Data::Type& scType) const + { + return m_creatableTypes.contains(scType) || m_slottableTypes.contains(scType); + } + + bool DataRegistry::IsUseableInSlot(const AZ::TypeId& typeId) const + { + Data::Type scType = Data::FromAZType(typeId); + return IsUseableInSlot(scType); + } +} diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Data/DataRegistry.h b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Data/DataRegistry.h index 57a67c79d9..c4837ebd27 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Data/DataRegistry.h +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Data/DataRegistry.h @@ -34,11 +34,21 @@ namespace ScriptCanvas AZ_TYPE_INFO(DataRegistry, "{41049FA8-EA56-401F-9720-6FE9028A1C01}"); AZ_CLASS_ALLOCATOR(DataRegistry, AZ::SystemAllocator, 0); - void RegisterType(const AZ::TypeId& typeId, TypeProperties typeProperties); + enum class Createability + { + None, + SlotAndVariable, + SlotOnly, + }; + void RegisterType(const AZ::TypeId& typeId, TypeProperties typeProperties, Createability registration); void UnregisterType(const AZ::TypeId& typeId); + bool IsUseableInSlot(const AZ::TypeId& typeId) const; + bool IsUseableInSlot(const Data::Type& type) const; + AZStd::unordered_map m_typeIdTraitMap; // Creates a mapping of the Data::eType TypeId to the trait structure AZStd::unordered_map m_creatableTypes; + AZStd::unordered_map m_slottableTypes; }; void InitDataRegistry(); diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/SystemComponent.h b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/SystemComponent.h index 3d5acfbee1..3c00562505 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/SystemComponent.h +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/SystemComponent.h @@ -22,6 +22,7 @@ #include #include #include +#include namespace AZ { @@ -65,6 +66,8 @@ namespace ScriptCanvas inline bool IsAnyScriptInterpreted() const { return true; } + AZStd::pair GetCreatibility(AZ::SerializeContext* serializeContext, AZ::BehaviorClass* behaviorClass); + // SystemRequestBus::Handler... bool IsScriptUnitTestingInProgress() override; void MarkScriptUnitTestBegin() override; diff --git a/Gems/ScriptCanvas/Code/Source/SystemComponent.cpp b/Gems/ScriptCanvas/Code/Source/SystemComponent.cpp index bbcbde5c6c..dd0c340357 100644 --- a/Gems/ScriptCanvas/Code/Source/SystemComponent.cpp +++ b/Gems/ScriptCanvas/Code/Source/SystemComponent.cpp @@ -23,7 +23,6 @@ #include #include #include -#include #include #include #include @@ -285,6 +284,72 @@ namespace ScriptCanvas m_ownedObjectsByAddress.erase(object); } + AZStd::pair SystemComponent::GetCreatibility(AZ::SerializeContext* serializeContext, AZ::BehaviorClass* behaviorClass) + { + TypeProperties typeProperties; + + bool canCreate{}; + // BehaviorContext classes with the ExcludeFrom attribute with a value of the ExcludeFlags::List is not creatable + const AZ::u64 exclusionFlags = AZ::Script::Attributes::ExcludeFlags::List; + auto excludeClassAttributeData = azrtti_cast*>(AZ::FindAttribute(AZ::Script::Attributes::ExcludeFrom, behaviorClass->m_attributes)); + + const AZ::u64 flags = excludeClassAttributeData ? excludeClassAttributeData->Get(nullptr) : 0; + bool listOnly = ((flags & AZ::Script::Attributes::ExcludeFlags::ListOnly) == AZ::Script::Attributes::ExcludeFlags::ListOnly); // ListOnly exclusions may create variables + canCreate = listOnly || (!excludeClassAttributeData || (!(flags & exclusionFlags))); + canCreate = canCreate && (serializeContext->FindClassData(behaviorClass->m_typeId)); + canCreate = canCreate && !ScriptCanvasSystemComponentCpp::IsDeprecated(behaviorClass->m_attributes); + + if (canCreate) + { + for (auto base : behaviorClass->m_baseClasses) + { + if (AZ::Component::TYPEINFO_Uuid() == base) + { + canCreate = false; + break; // only out of the for : base classes loop. DO NOT break out of the parent loop. + } + } + } + + // Assets are not safe enough for variable creation, yet. They can be created with one Az type (Data::Asset), but set to nothing. + // When read back in, they will (if lucky) just be Data::Asset, which breaks type safety at best, and requires a lot of sanity checking. + // This is NOT blacked at the createable types or BehaviorContext level, since they could be used to at least pass information through, + // and may be used other scripting contexts. + AZ::IRttiHelper* rttiHelper = behaviorClass->m_azRtti; + if (rttiHelper && rttiHelper->GetGenericTypeId() == azrtti_typeid()) + { + canCreate = false; + } + + if (AZ::FindAttribute(AZ::ScriptCanvasAttributes::AllowInternalCreation, behaviorClass->m_attributes)) + { + canCreate = true; + typeProperties.m_isTransient = true; + } + + // create able variables must have full memory support + canCreate = canCreate && + (behaviorClass->m_allocate + && behaviorClass->m_cloner + && behaviorClass->m_mover + && behaviorClass->m_destructor + && behaviorClass->m_deallocate) && + AZStd::none_of(behaviorClass->m_baseClasses.begin(), behaviorClass->m_baseClasses.end(), [](const AZ::TypeId& base) { return azrtti_typeid() == base; }); + + if (!canCreate) + { + return { DataRegistry::Createability::None , TypeProperties{} }; + } + else if (!AZ::FindAttribute(AZ::ScriptCanvasAttributes::VariableCreationForbidden, behaviorClass->m_attributes)) + { + return { DataRegistry::Createability::SlotAndVariable, typeProperties }; + } + else + { + return { DataRegistry::Createability::SlotOnly, typeProperties }; + } + } + void SystemComponent::RegisterCreatableTypes() { AZ::SerializeContext* serializeContext{}; @@ -297,40 +362,11 @@ namespace ScriptCanvas auto dataRegistry = ScriptCanvas::GetDataRegistry(); for (const auto& classIter : behaviorContext->m_classes) { - TypeProperties typeProperties; - - bool canCreate{}; - const AZ::BehaviorClass* behaviorClass = classIter.second; - // BehaviorContext classes with the ExcludeFrom attribute with a value of the ExcludeFlags::List is not creatable - const AZ::u64 exclusionFlags = AZ::Script::Attributes::ExcludeFlags::List; - auto excludeClassAttributeData = azrtti_cast*>(AZ::FindAttribute(AZ::Script::Attributes::ExcludeFrom, behaviorClass->m_attributes)); - - const AZ::u64 flags = excludeClassAttributeData ? excludeClassAttributeData->Get(nullptr) : 0; - bool listOnly = ((flags & AZ::Script::Attributes::ExcludeFlags::ListOnly) == AZ::Script::Attributes::ExcludeFlags::ListOnly); // ListOnly exclusions may create variables - - canCreate = listOnly || (!excludeClassAttributeData || (!(flags & exclusionFlags))); - canCreate = canCreate && (serializeContext->FindClassData(behaviorClass->m_typeId)); - canCreate = canCreate && !ScriptCanvasSystemComponentCpp::IsDeprecated(behaviorClass->m_attributes); - - if (AZ::FindAttribute(AZ::ScriptCanvasAttributes::AllowInternalCreation, behaviorClass->m_attributes)) - { - canCreate = true; - typeProperties.m_isTransient = true; - } - - // create able variables must have full memory support - canCreate = canCreate && - ( behaviorClass->m_allocate - && behaviorClass->m_cloner - && behaviorClass->m_mover - && behaviorClass->m_destructor - && behaviorClass->m_deallocate) && - AZStd::none_of(behaviorClass->m_baseClasses.begin(), behaviorClass->m_baseClasses.end(), [](const AZ::TypeId& base) { return azrtti_typeid() == base; }); - - if (canCreate) - { - dataRegistry->RegisterType(behaviorClass->m_typeId, typeProperties); - } + auto createability = GetCreatibility(serializeContext, classIter.second); + if (createability.first != DataRegistry::Createability::None) + { + dataRegistry->RegisterType(classIter.second->m_typeId, createability.second, createability.first); + } } } @@ -339,33 +375,19 @@ namespace ScriptCanvas auto dataRegistry = ScriptCanvas::GetDataRegistry(); if (!dataRegistry) { + AZ_Warning("ScriptCanvas", false, "Data registry not available. Can't register new class."); + return; } + AZ::SerializeContext* serializeContext{}; AZ::ComponentApplicationBus::BroadcastResult(serializeContext, &AZ::ComponentApplicationRequests::GetSerializeContext); - AZ_Assert(serializeContext, "Serialize Context should not be missing at this point"); + AZ_Assert(serializeContext, "Serialize Context missing. Can't register new class."); - TypeProperties typeProperties; - - // BehaviorContext classes with the ExcludeFrom attribute with a value of the ExcludeFlags::List is not creatable - const AZ::u64 exclusionFlags = AZ::Script::Attributes::ExcludeFlags::List; - auto excludeClassAttributeData = azrtti_cast*>(AZ::FindAttribute(AZ::Script::Attributes::ExcludeFrom, behaviorClass->m_attributes)); - bool canCreate = !excludeClassAttributeData || !(excludeClassAttributeData->Get(nullptr) & exclusionFlags); - canCreate = canCreate && (serializeContext->FindClassData(behaviorClass->m_typeId) || AZ::FindAttribute(AZ::ScriptCanvasAttributes::AllowInternalCreation, behaviorClass->m_attributes)); - canCreate = canCreate && !ScriptCanvasSystemComponentCpp::IsDeprecated(behaviorClass->m_attributes); - - // create able variables must have full memory support - canCreate = canCreate && - (behaviorClass->m_allocate - && behaviorClass->m_cloner - && behaviorClass->m_mover - && behaviorClass->m_destructor - && behaviorClass->m_deallocate) && - AZStd::none_of(behaviorClass->m_baseClasses.begin(), behaviorClass->m_baseClasses.end(), [](const AZ::TypeId& base) { return azrtti_typeid() == base; }); - - if (canCreate) + auto createability = GetCreatibility(serializeContext, behaviorClass); + if (createability.first != DataRegistry::Createability::None) { - dataRegistry->RegisterType(behaviorClass->m_typeId, typeProperties); + dataRegistry->RegisterType(behaviorClass->m_typeId, createability.second, createability.first); } }