NumericEditor attribute fix for better Min/Max support in the DPE Inspector (#16471)

* fix for components like Fly Camera Input, that are their own reference

Signed-off-by: Alex Montgomery <[email protected]>

* change NumericEditor's name based on template type so they can each be
registered separately

Signed-off-by: Alex Montgomery <[email protected]>

* fix non-fallback cases

Signed-off-by: Alex Montgomery <[email protected]>

* remove unnecessary fallback for converting to DOM value

Signed-off-by: Alex Montgomery <[email protected]>

---------

Signed-off-by: Alex Montgomery <[email protected]>

Code/Framework/AzFramework/AzFramework/DocumentPropertyEditor/DocumentSchema.cpp

@@ -36,7 +36,7 @@ namespace AZ::DocumentPropertyEditor
         return {};
     }
 
-    AZStd::shared_ptr<AZ::Attribute> TypeIdAttributeDefinition::DomValueToLegacyAttribute(const AZ::Dom::Value& value) const
+    AZStd::shared_ptr<AZ::Attribute> TypeIdAttributeDefinition::DomValueToLegacyAttribute(const AZ::Dom::Value& value, bool) const
     {
         AZ::Uuid uuidValue = DomToValue(value).value_or(AZ::Uuid::CreateNull());
         return AZStd::make_shared<AZ::AttributeData<AZ::Uuid>>(AZStd::move(uuidValue));
@@ -67,7 +67,7 @@ namespace AZ::DocumentPropertyEditor
         return {};
     }
 
-    AZStd::shared_ptr<AZ::Attribute> NamedCrcAttributeDefinition::DomValueToLegacyAttribute(const AZ::Dom::Value& value) const
+    AZStd::shared_ptr<AZ::Attribute> NamedCrcAttributeDefinition::DomValueToLegacyAttribute(const AZ::Dom::Value& value, bool) const
     {
         AZ::Crc32 crc = 0;
         if (value.IsString())

Code/Framework/AzFramework/AzFramework/DocumentPropertyEditor/DocumentSchema.h

@@ -10,11 +10,11 @@
 
 #include <AzCore/DOM/DomUtils.h>
 #include <AzCore/DOM/DomValue.h>
-#include <AzCore/Serialization/EditContext.h>
 #include <AzCore/Name/Name.h>
 #include <AzCore/Name/NameDictionary.h>
 #include <AzCore/Outcome/Outcome.h>
 #include <AzCore/RTTI/AttributeReader.h>
+#include <AzCore/Serialization/EditContext.h>
 #include <AzCore/std/smart_ptr/make_shared.h>
 #include <AzCore/std/string/fixed_string.h>
 #include <AzFramework/DocumentPropertyEditor/DocumentAdapter.h>
@@ -114,9 +114,12 @@ namespace AZ::DocumentPropertyEditor
         virtual Name GetName() const = 0;
         //! Gets this attribute's type ID.
         virtual AZ::TypeId GetTypeId() const = 0;
-        //! Converts this attribute to an AZ::Attribute usable by the ReflectedPropertyEditor.
-        virtual AZStd::shared_ptr<AZ::Attribute> DomValueToLegacyAttribute(const AZ::Dom::Value& value) const = 0;
-        //! Converts this attribute from an AZ::Attribute to a Dom::Value usable in the DocumentPropertyEditor.
+        /*! Converts this attribute to an AZ::Attribute usable by the ReflectedPropertyEditor
+            @param fallback if false, the Attribute type must match AZ::Dom::Value; if true, it will attempt a fallback on failure */
+        virtual AZStd::shared_ptr<AZ::Attribute> DomValueToLegacyAttribute(const AZ::Dom::Value& value, bool fallback = true) const = 0;
+
+        /*! Converts this attribute from an AZ::Attribute to a Dom::Value usable in the DocumentPropertyEditor.
+            @param fallback if false, a Read<AttributeType> failure will return a null Value; if true, it will attempt a fallback on failure */
         virtual AZ::Dom::Value LegacyAttributeToDomValue(void* instance, AZ::Attribute* attribute) const = 0;
     };
 
@@ -174,7 +177,7 @@ namespace AZ::DocumentPropertyEditor
             return azrtti_typeid<AttributeType>();
         }
 
-        AZStd::shared_ptr<AZ::Attribute> DomValueToLegacyAttribute(const AZ::Dom::Value& value) const override
+        AZStd::shared_ptr<AZ::Attribute> DomValueToLegacyAttribute(const AZ::Dom::Value& value, bool fallback) const override
         {
             if constexpr (AZStd::is_same_v<AttributeType, AZ::Dom::Value>)
             {
@@ -182,10 +185,15 @@ namespace AZ::DocumentPropertyEditor
             }
             else
             {
-                AZStd::optional<AttributeType> attributeValue = DomToValue(value);
-                return attributeValue.has_value()
-                    ? AZStd::make_shared<AZ::AttributeData<AttributeType>>(AZStd::move(attributeValue.value()))
-                    : nullptr;
+                if (fallback)
+                {
+                    AZStd::optional<AttributeType> attributeValue = DomToValue(value);
+                    if (attributeValue.has_value())
+                    {
+                        return AZStd::make_shared<AZ::AttributeData<AttributeType>>(AZStd::move(attributeValue.value()));
+                    }
+                }
+                return nullptr;
             }
         }
 
@@ -235,7 +243,7 @@ namespace AZ::DocumentPropertyEditor
 
         Dom::Value ValueToDom(const AZ::TypeId& attribute) const override;
         AZStd::optional<AZ::TypeId> DomToValue(const Dom::Value& value) const override;
-        AZStd::shared_ptr<AZ::Attribute> DomValueToLegacyAttribute(const AZ::Dom::Value& value) const override;
+        AZStd::shared_ptr<AZ::Attribute> DomValueToLegacyAttribute(const AZ::Dom::Value& value, bool fallback = true) const override;
         AZ::Dom::Value LegacyAttributeToDomValue(void* instance, AZ::Attribute* attribute) const override;
     };
 
@@ -251,11 +259,11 @@ namespace AZ::DocumentPropertyEditor
 
         Dom::Value ValueToDom(const AZ::Name& attribute) const override;
         AZStd::optional<AZ::Name> DomToValue(const Dom::Value& value) const override;
-        AZStd::shared_ptr<AZ::Attribute> DomValueToLegacyAttribute(const AZ::Dom::Value& value) const override;
+        AZStd::shared_ptr<AZ::Attribute> DomValueToLegacyAttribute(const AZ::Dom::Value& value, bool fallback = true) const override;
         AZ::Dom::Value LegacyAttributeToDomValue(void* instance, AZ::Attribute* attribute) const override;
     };
 
-    template <typename GenericValueType>
+    template<typename GenericValueType>
     class GenericValueAttributeDefinition final : public AttributeDefinition<AZStd::pair<GenericValueType, AZStd::string>>
     {
     public:
@@ -291,8 +299,7 @@ namespace AZ::DocumentPropertyEditor
                 if (auto data = azdynamic_cast<AttributeData<AZStd::unique_ptr<EnumConstantBaseType>>*>(attribute); data != nullptr)
                 {
                     EnumConstantBaseType* value = static_cast<EnumConstantBaseType*>(data->Get(instance).get());
-                    return ValueToDom(
-                        AZStd::make_pair(value->GetEnumValueAsUInt(), value->GetEnumValueName()));
+                    return ValueToDom(AZStd::make_pair(value->GetEnumValueAsUInt(), value->GetEnumValueName()));
                 }
             }
 
@@ -394,8 +401,7 @@ namespace AZ::DocumentPropertyEditor
                 auto genericValue = AZStd::any_cast<GenericValueType>(opaqueValue);
                 if (opaqueValue->is<GenericValueType>() && genericValue)
                 {
-                    result.emplace_back(
-                        AZStd::make_pair(*genericValue, entryDom[EntryDescriptionKey].GetString()));
+                    result.emplace_back(AZStd::make_pair(*genericValue, entryDom[EntryDescriptionKey].GetString()));
                 }
             }
 
@@ -617,7 +623,7 @@ namespace AZ::DocumentPropertyEditor
             return false;
         }
 
-        AZStd::shared_ptr<AZ::Attribute> DomValueToLegacyAttribute(const AZ::Dom::Value& value) const override
+        AZStd::shared_ptr<AZ::Attribute> DomValueToLegacyAttribute(const AZ::Dom::Value& value, bool) const override
         {
             // If we're already an attribute, return a non-owning shared_ptr
             if (value.IsOpaqueValue() && value.GetOpaqueValue().is<AZ::Attribute*>())

Code/Framework/AzFramework/AzFramework/DocumentPropertyEditor/PropertyEditorNodes.h

@@ -174,6 +174,11 @@ namespace AZ::DocumentPropertyEditor::Nodes
     };
 
     template<typename T = Dom::Value>
+    struct NumericEditor;
+
+    AZ_TYPE_INFO_TEMPLATE_WITH_NAME(NumericEditor, "NumericEditor", "{C891BF19-B60C-45E2-BFD0-027D15DDC939}", AZ_TYPE_INFO_CLASS);
+
+    template<typename T>
     struct NumericEditor : PropertyEditorDefinition
     {
         static_assert(
@@ -182,9 +187,9 @@ namespace AZ::DocumentPropertyEditor::Nodes
         using StorageType = AZStd::conditional_t<
             AZStd::is_same_v<T, Dom::Value>,
             Dom::Value,
-            AZStd::conditional_t<AZStd::is_floating_point_v<T>, double, AZStd::conditional_t<AZStd::is_signed_v<T>, int64_t, uint64_t>>>;
+            AZStd::conditional_t<AZStd::is_floating_point_v<T>, double, AZStd::conditional_t<AZStd::is_signed_v<T>, AZ::s64, AZ::u64>>>;
 
-        static constexpr AZStd::string_view Name = "NumericEditor";
+        inline static const AZStd::string_view Name = AzTypeInfo<NumericEditor>::Name();
         static constexpr auto Min = AttributeDefinition<StorageType>("Min");
         static constexpr auto Max = AttributeDefinition<StorageType>("Max");
         static constexpr auto Step = AttributeDefinition<StorageType>("Step");

Code/Framework/AzFramework/AzFramework/DocumentPropertyEditor/Reflection/LegacyReflectionBridge.cpp

@@ -1005,15 +1005,15 @@ namespace AZ::Reflection
 
                         // See if any registered attribute definitions can read this attribute
                         Dom::Value attributeValue;
-                        propertyEditorSystem->EnumerateRegisteredAttributes(
-                            name,
-                            [&](const DocumentPropertyEditor::AttributeDefinitionInterface& attributeReader)
+
+                        auto readValue = [&](const DocumentPropertyEditor::AttributeDefinitionInterface& attributeReader)
+                        {
+                            if (attributeValue.IsNull())
                             {
-                                if (attributeValue.IsNull())
-                                {
-                                    attributeValue = attributeReader.LegacyAttributeToDomValue(instance, it->second);
-                                }
-                            });
+                                attributeValue = attributeReader.LegacyAttributeToDomValue(instance, it->second);
+                            }
+                        };
+                        propertyEditorSystem->EnumerateRegisteredAttributes(name, readValue);
 
                         // Fall back on a generic read that handles primitives.
                         if (attributeValue.IsNull())

Code/Framework/AzQtComponents/AzQtComponents/DPEDebugViewStandalone/main.cpp

@@ -202,6 +202,26 @@ namespace DPEDebugView
             return m_toggle ? AZ::Edit::PropertyVisibility::Show : AZ::Edit::PropertyVisibility::Hide;
         }
 
+        double DoubleMin() const
+        {
+            return -9.0;
+        }
+
+        double DoubleMax() const
+        {
+            return 9.0;
+        }
+
+        int IntMin() const
+        {
+            return -8;
+        }
+
+        int IntMax() const
+        {
+            return 8;
+        }
+
         static void Reflect(AZ::ReflectContext* context)
         {
             if (auto serializeContext = azrtti_cast<AZ::SerializeContext*>(context))
@@ -248,11 +268,13 @@ namespace DPEDebugView
                         ->DataElement(AZ::Edit::UIHandlers::Default, &TestContainer::m_toggle, "toggle switch", "")
                             ->Attribute(AZ::Edit::Attributes::ChangeNotify, AZ::Edit::PropertyRefreshLevels::AttributesAndValues)
                         ->DataElement(AZ::Edit::UIHandlers::Default, &TestContainer::m_simpleInt, "simple int", "")
+                        ->Attribute(AZ::Edit::Attributes::Min, &TestContainer::IntMin)
+                        ->Attribute(AZ::Edit::Attributes::Max, &TestContainer::IntMax)
                         ->DataElement(AZ::Edit::UIHandlers::Slider, &TestContainer::m_doubleSlider, "double slider", "")
+                        ->Attribute(AZ::Edit::Attributes::Min, &TestContainer::DoubleMin)
+                        ->Attribute(AZ::Edit::Attributes::Max, &TestContainer::DoubleMax)
                         ->DataElement(AZ::Edit::UIHandlers::Default, &TestContainer::m_simpleString, "simple string", "")
                             ->Attribute(AZ::Edit::Attributes::Visibility, &TestContainer::IsToggleEnabled)
-                        ->Attribute(AZ::Edit::Attributes::Min, -10.0)
-                        ->Attribute(AZ::Edit::Attributes::Max, 10.0)
                         ->ClassElement(AZ::Edit::ClassElements::Group, "Containers")
                         ->Attribute(AZ::Edit::Attributes::AutoExpand, false)
                         ->DataElement(AZ::Edit::UIHandlers::Default, &TestContainer::m_vector, "vector<string>", "")

Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyEditorAPI_Internals.h

@@ -282,7 +282,14 @@ namespace AzToolsFramework
                     {
                         if (marshalledAttribute == nullptr)
                         {
-                            marshalledAttribute = attributeReader.DomValueToLegacyAttribute(attributeIt->second);
+                            // try the conversion once without type fallback
+                            marshalledAttribute = attributeReader.DomValueToLegacyAttribute(attributeIt->second, false);
+
+                            if (marshalledAttribute == nullptr)
+                            {
+                                // still null, try it again allowing type fallback
+                                attributeReader.DomValueToLegacyAttribute(attributeIt->second, true);
+                            }
                         }
                     });