From 91f027a05cb2c3df3bbe1b6881c75509334be9cb Mon Sep 17 00:00:00 2001 From: mnaumov Date: Mon, 26 Apr 2021 17:42:49 -0700 Subject: [PATCH] PR feedback --- .../Serialization/EditContextConstants.inl | 1 + .../UI/PropertyEditor/PropertyAssetCtrl.cpp | 33 +++++++++---------- .../UI/PropertyEditor/PropertyAssetCtrl.hxx | 21 ++---------- .../ReflectedPropertyEditor.cpp | 10 ------ .../ReflectedPropertyEditor.hxx | 4 --- .../DynamicProperty/DynamicProperty.h | 3 ++ .../Inspector/InspectorPropertyGroupWidget.h | 3 +- .../DynamicProperty/DynamicProperty.cpp | 11 +++++++ .../InspectorPropertyGroupWidget.cpp | 4 +-- .../Code/Source/Document/MaterialDocument.cpp | 4 +++ .../MaterialInspector/MaterialInspector.cpp | 9 ++--- 11 files changed, 44 insertions(+), 59 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Serialization/EditContextConstants.inl b/Code/Framework/AzCore/AzCore/Serialization/EditContextConstants.inl index eac6e5760e..90b9ba5afd 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/EditContextConstants.inl +++ b/Code/Framework/AzCore/AzCore/Serialization/EditContextConstants.inl @@ -118,6 +118,7 @@ namespace AZ const static AZ::Crc32 StringLineEditingCompleteNotify = AZ_CRC("StringLineEditingCompleteNotify", 0x139e5fa9); const static AZ::Crc32 NameLabelOverride = AZ_CRC("NameLabelOverride", 0x9ff79cab); + const static AZ::Crc32 AssetPickerTitle = AZ_CRC_CE("AssetPickerTitle"); const static AZ::Crc32 ChildNameLabelOverride = AZ_CRC("ChildNameLabelOverride", 0x73dd2909); //! Container attribute that is used to override labels for its elements given the index of the element const static AZ::Crc32 IndexedChildNameLabelOverride = AZ_CRC("IndexedChildNameLabelOverride", 0x5f313ac2); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyAssetCtrl.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyAssetCtrl.cpp index 0f52fda06a..11afd97197 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyAssetCtrl.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyAssetCtrl.cpp @@ -679,22 +679,7 @@ namespace AzToolsFramework AssetSelectionModel PropertyAssetCtrl::GetAssetSelectionModel() { auto selectionModel = AssetSelectionModel::AssetTypeSelection(GetCurrentAssetType()); - - QString title; - auto propertyRowWidget = FindFirstParent(parent()); - if (propertyRowWidget) - { - if (!propertyRowWidget->label().isEmpty()) - { - title = propertyRowWidget->label(); - } - auto reflectedPropertyEditor = FindFirstParent(propertyRowWidget->parent()); - if (reflectedPropertyEditor && !reflectedPropertyEditor->GetTitle().isEmpty()) - { - title = QString("%1 %2").arg(reflectedPropertyEditor->GetTitle()).arg(title); - } - } - selectionModel.SetTitle(title); + selectionModel.SetTitle(m_title); return selectionModel; } @@ -1076,6 +1061,11 @@ namespace AzToolsFramework m_editButton->setIcon(icon); } + void PropertyAssetCtrl::SetTitle(const QString& title) + { + m_title = title; + } + void PropertyAssetCtrl::SetEditNotifyTarget(void* editNotifyTarget) { m_editNotifyTarget = editNotifyTarget; @@ -1211,7 +1201,16 @@ namespace AzToolsFramework { (void)debugName; - if (attrib == AZ_CRC("EditCallback", 0xb74f2ee1)) + if (attrib == AZ_CRC_CE("AssetPickerTitle")) + { + AZStd::string title; + attrValue->Read(title); + if (!title.empty()) + { + GUI->SetTitle(title.c_str()); + } + } + else if (attrib == AZ_CRC("EditCallback", 0xb74f2ee1)) { PropertyAssetCtrl::EditCallbackType* func = azdynamic_cast(attrValue->GetAttribute()); if (func) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyAssetCtrl.hxx b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyAssetCtrl.hxx index 90b0f16947..6812805d02 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyAssetCtrl.hxx +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyAssetCtrl.hxx @@ -95,6 +95,7 @@ namespace AzToolsFramework void OnAssetIDChanged(AZ::Data::AssetId newAssetID); protected: + QString m_title; ThumbnailDropDown* m_thumbnailDropDown = nullptr; Thumbnailer::ThumbnailWidget* m_thumbnail = nullptr; QPushButton* m_errorButton = nullptr; @@ -178,9 +179,6 @@ namespace AzToolsFramework void HandleFieldClear(); AZStd::string AddDefaultSuffix(const AZStd::string& filename); - template - Widget_Type* FindFirstParent(QObject* pParent) const; - ////////////////////////////////////////////////////////////////////////// // AssetSystemBus void SourceFileChanged(AZStd::string relativePath, AZStd::string scanFolder, AZ::Uuid sourceUUID) override; @@ -195,6 +193,7 @@ namespace AzToolsFramework ////////////////////////////////////////////////////////////////////////// public slots: + void SetTitle(const QString& title); void SetEditNotifyTarget(void* editNotifyTarget); void SetEditNotifyCallback(EditCallbackType* editNotifyCallback); // This is meant to be used with the "EditCallback" Attribute void SetClearNotifyCallback(ClearCallbackType* clearNotifyCallback); // This is meant to be used with the "ClearNotify" Attribute @@ -236,22 +235,6 @@ namespace AzToolsFramework void UpdateThumbnail(); }; - template - Widget_Type* PropertyAssetCtrl::FindFirstParent(QObject* pParent) const - { - Widget_Type* widget = nullptr; - while (pParent) - { - widget = qobject_cast(pParent); - if (widget) - { - break; - } - pParent = pParent->parent(); - } - return widget; - } - class AssetPropertyHandlerDefault : QObject , public PropertyHandler, PropertyAssetCtrl> diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/ReflectedPropertyEditor.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/ReflectedPropertyEditor.cpp index 41cff1364a..04957ed5e1 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/ReflectedPropertyEditor.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/ReflectedPropertyEditor.cpp @@ -2248,16 +2248,6 @@ namespace AzToolsFramework m_impl->m_visibilityCallback = callback; } - void ReflectedPropertyEditor::SetTitle(const QString& title) - { - m_title = title; - } - - const QString& ReflectedPropertyEditor::GetTitle() const - { - return m_title; - } - QWidget* ReflectedPropertyEditor::GetContainerWidget() { return m_impl->m_containerWidget; diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/ReflectedPropertyEditor.hxx b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/ReflectedPropertyEditor.hxx index 26d189ce03..ef542074a8 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/ReflectedPropertyEditor.hxx +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/ReflectedPropertyEditor.hxx @@ -156,9 +156,6 @@ namespace AzToolsFramework using VisibilityCallback = AZStd::function; void SetVisibilityCallback(VisibilityCallback callback); - void SetTitle(const QString& title); - const QString& GetTitle() const; - signals: void OnExpansionContractionDone(); private: @@ -166,7 +163,6 @@ namespace AzToolsFramework std::unique_ptr m_impl; AZStd::string m_currentFilterString; - QString m_title; virtual void paintEvent(QPaintEvent* event) override; int m_updateDepth = 0; diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/DynamicProperty/DynamicProperty.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/DynamicProperty/DynamicProperty.h index f481bbc397..5fd2862c65 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/DynamicProperty/DynamicProperty.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/DynamicProperty/DynamicProperty.h @@ -49,6 +49,7 @@ namespace AtomToolsFramework AZ::Name m_id; AZStd::string m_nameId; AZStd::string m_displayName; + AZStd::string m_groupName; AZStd::string m_description; AZStd::any m_defaultValue; AZStd::any m_parentValue; @@ -108,6 +109,8 @@ namespace AtomToolsFramework private: // Functions used to configure edit data attributes. AZStd::string GetDisplayName() const; + AZStd::string GetGroupName() const; + AZStd::string GetAssetPickerTitle() const; AZStd::string GetDescription() const; AZStd::vector> GetEnumValues() const; diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Inspector/InspectorPropertyGroupWidget.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Inspector/InspectorPropertyGroupWidget.h index b002f81081..71ca975f58 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Inspector/InspectorPropertyGroupWidget.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Inspector/InspectorPropertyGroupWidget.h @@ -44,8 +44,7 @@ namespace AtomToolsFramework const AZ::Uuid& instanceClassId, AzToolsFramework::IPropertyEditorNotify* instanceNotificationHandler = {}, QWidget* parent = {}, - const AzToolsFramework::InstanceDataHierarchy::ValueComparisonFunction& valueComparisonFunction = {}, - QString title = QString()); + const AzToolsFramework::InstanceDataHierarchy::ValueComparisonFunction& valueComparisonFunction = {}); void Refresh() override; void Rebuild() override; diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/DynamicProperty/DynamicProperty.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/DynamicProperty/DynamicProperty.cpp index 1ff475b2ef..754c35f0c2 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/DynamicProperty/DynamicProperty.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/DynamicProperty/DynamicProperty.cpp @@ -135,6 +135,7 @@ namespace AtomToolsFramework m_editData.m_elementId = AZ::Edit::UIHandlers::Default; AddEditDataAttributeMemberFunction(AZ::Edit::Attributes::NameLabelOverride, &DynamicProperty::GetDisplayName); + AddEditDataAttributeMemberFunction(AZ::Edit::Attributes::AssetPickerTitle, &DynamicProperty::GetAssetPickerTitle); AddEditDataAttributeMemberFunction(AZ::Edit::Attributes::DescriptionTextOverride, &DynamicProperty::GetDescription); AddEditDataAttributeMemberFunction(AZ::Edit::Attributes::ReadOnly, &DynamicProperty::IsReadOnly); AddEditDataAttributeMemberFunction(AZ::Edit::Attributes::EnumValues, &DynamicProperty::GetEnumValues); @@ -197,6 +198,16 @@ namespace AtomToolsFramework return !m_config.m_displayName.empty() ? m_config.m_displayName : m_config.m_nameId; } + AZStd::string DynamicProperty::GetGroupName() const + { + return m_config.m_groupName; + } + + AZStd::string DynamicProperty::GetAssetPickerTitle() const + { + return GetGroupName().empty() ? GetDisplayName() : GetGroupName() + " " + GetDisplayName(); + } + AZStd::string DynamicProperty::GetDescription() const { return AZStd::string::format("%s%s(Script Name = '%s')", diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Inspector/InspectorPropertyGroupWidget.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Inspector/InspectorPropertyGroupWidget.cpp index ffddb0cd2c..c4d78d1acb 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Inspector/InspectorPropertyGroupWidget.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Inspector/InspectorPropertyGroupWidget.cpp @@ -22,8 +22,7 @@ namespace AtomToolsFramework const AZ::Uuid& instanceClassId, AzToolsFramework::IPropertyEditorNotify* instanceNotificationHandler, QWidget* parent, - const AzToolsFramework::InstanceDataHierarchy::ValueComparisonFunction& valueComparisonFunction, - QString title) + const AzToolsFramework::InstanceDataHierarchy::ValueComparisonFunction& valueComparisonFunction) : InspectorGroupWidget(parent) { AZ::SerializeContext* context = nullptr; @@ -35,7 +34,6 @@ namespace AtomToolsFramework m_layout->setSpacing(0); m_propertyEditor = new AzToolsFramework::ReflectedPropertyEditor(this); - m_propertyEditor->SetTitle(title); m_propertyEditor->SetHideRootProperties(true); m_propertyEditor->SetAutoResizeLabels(true); m_propertyEditor->SetValueComparisonFunction(valueComparisonFunction); diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp index 2e59cb16f9..6bdee9a319 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp @@ -773,6 +773,7 @@ namespace MaterialEditor AtomToolsFramework::ConvertToPropertyConfig(propertyConfig, propertyDefinition); propertyConfig.m_originalValue = AtomToolsFramework::ConvertToEditableType(m_materialAsset->GetPropertyValues()[propertyIndex.GetIndex()]); propertyConfig.m_parentValue = AtomToolsFramework::ConvertToEditableType(parentPropertyValues[propertyIndex.GetIndex()]); + propertyConfig.m_groupName = m_materialTypeSourceData.FindGroup(groupNameId)->m_displayName; m_properties[propertyConfig.m_id] = AtomToolsFramework::DynamicProperty(propertyConfig); } return true; @@ -789,6 +790,7 @@ namespace MaterialEditor propertyConfig.m_id = "details.materialType"; propertyConfig.m_nameId = "materialType"; propertyConfig.m_displayName = "Material Type"; + propertyConfig.m_groupName = "Details"; propertyConfig.m_description = propertyConfig.m_displayName; propertyConfig.m_defaultValue = AZStd::any(materialTypeAsset); propertyConfig.m_originalValue = propertyConfig.m_defaultValue; @@ -802,6 +804,7 @@ namespace MaterialEditor propertyConfig.m_id = "details.parentMaterial"; propertyConfig.m_nameId = "parentMaterial"; propertyConfig.m_displayName = "Parent Material"; + propertyConfig.m_groupName = "Details"; propertyConfig.m_description = propertyConfig.m_displayName; propertyConfig.m_defaultValue = AZStd::any(parentMaterialAsset); propertyConfig.m_originalValue = propertyConfig.m_defaultValue; @@ -822,6 +825,7 @@ namespace MaterialEditor propertyConfig.m_id = MaterialPropertyId(UvGroupName, shaderInput).GetCStr(); propertyConfig.m_nameId = shaderInput; propertyConfig.m_displayName = shaderInput; + propertyConfig.m_groupName = "UV Names"; propertyConfig.m_description = shaderInput; propertyConfig.m_defaultValue = uvName; propertyConfig.m_originalValue = uvName; diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.cpp index 2975ba8c23..776af3147b 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.cpp @@ -93,8 +93,10 @@ namespace MaterialEditor [this](const AzToolsFramework::InstanceDataNode* source, const AzToolsFramework::InstanceDataNode* target) { AZ_UNUSED(source); const AtomToolsFramework::DynamicProperty* property = AtomToolsFramework::FindDynamicPropertyForInstanceDataNode(target); + + //property->AddEditDataAttributeMemberFunction(AZ::Edit::Attributes::NameLabelOverride, &DynamicProperty::GetDisplayName); return property && AtomToolsFramework::ArePropertyValuesEqual(property->GetValue(), property->GetConfig().m_parentValue); - }, groupDisplayName.c_str()); + }); AddGroup(groupNameId, groupDisplayName, groupDescription, propertyGroupWidget); } @@ -126,7 +128,7 @@ namespace MaterialEditor AZ_UNUSED(source); const AtomToolsFramework::DynamicProperty* property = AtomToolsFramework::FindDynamicPropertyForInstanceDataNode(target); return property && AtomToolsFramework::ArePropertyValuesEqual(property->GetValue(), property->GetConfig().m_parentValue); - }, groupDisplayName.c_str()); + }); AddGroup(groupNameId, groupDisplayName, groupDescription, propertyGroupWidget); } @@ -161,8 +163,7 @@ namespace MaterialEditor AZ_UNUSED(source); const AtomToolsFramework::DynamicProperty* property = AtomToolsFramework::FindDynamicPropertyForInstanceDataNode(target); return property && AtomToolsFramework::ArePropertyValuesEqual(property->GetValue(), property->GetConfig().m_parentValue); - }, - groupDisplayName.c_str()); + }); AddGroup(groupNameId, groupDisplayName, groupDescription, propertyGroupWidget); } }