Fixes issue #18620 - Icons not being correct And related other issues. (#18625)

* Fixes issue #18620 - Icons not being correct And related

See https://github.com/o3de/o3de/issues/18620 for a detailed issue report.

This PR fixes the issue by doing a few specific things
* Moves a bunch of entity utility functions from inside the Property Editor (As in, the GUI Control) into
  the Entity Utility class so that it can be used from more than one location.
* Makes it so that the OnWriteEnd / OnWriteBegin functions are actually called when reading from json,
  so that classes wanting to do a post-load fixup actually get invoked
* Adds comments to the serializer to explain the OnRead* and OnWrite* functions and caveats.
* Fixes several classes that were listening to OnReadEnd thinking it meant that reading into the C++ object had finished.
  They should have been listening for OnWriteEnd instead.
* Makes it so that the Inspector component only saves the component order if it differs from default
* Adds an automated test to verify the new code.
* Makes it so that the icon component caches the icon to use only when actually asked for the icon
* Makes it so that the icon component and inspector component can deal with the case where the components are in default order.
* Makes it so that icon files (SVG as well as PNG) are tagged as such in the AP so that they can actually be used.

Signed-off-by: Nicholas Lawson <[email protected]>

Code/Framework/AzCore/AzCore/Serialization/Json/JsonDeserializer.cpp

@@ -191,6 +191,12 @@ namespace AZ
 
         AZ_Assert(context.GetRegistrationContext() && context.GetSerializeContext(), "Expected valid registration context and serialize context.");
 
+        // Compatibility with Reflection Serialize - it expects this callback after before into a C++ class.
+        if (classData.m_eventHandler)
+        {
+            classData.m_eventHandler->OnWriteBegin(object);
+        }
+
         size_t numLoads = 0;
         ResultCode retVal(Tasks::ReadField);
         for (auto iter = value.MemberBegin(); iter != value.MemberEnd(); ++iter)
@@ -232,6 +238,12 @@ namespace AZ
             retVal.Combine(ResultCode(Tasks::ReadField, numLoads == 0 ? Outcomes::DefaultsUsed : Outcomes::PartialDefaults));
         }
 
+        // Compatibility with Reflection Serialize - it expects this callback after reading into a C++ class.
+        if (classData.m_eventHandler)
+        {
+            classData.m_eventHandler->OnWriteEnd(object);
+        }
+
         return retVal;
     }
 

Code/Framework/AzCore/AzCore/Serialization/SerializeContext.h

@@ -1191,27 +1191,53 @@ namespace AZ::Serialize
 
     /**
      * Serialize class events.
+     * These can be used to hook certain events that occur when reading/writing objects using the Serialize Reflection Context
+     * Note that these events will NOT be called if you have installed a custom serializer for your custom class, since
+     * in that case, your class will be entirely handled by that custom serializer installed.  This is true for both the ObjectStream (XML, binary)
+     * and JSON serializers - using a custom serializer skips this part, so if you do need to do anything special, do it in the custom serializer.
      * IMPORTANT: Serialize events can be called from serialization thread(s). So all functions should be thread safe.
+     *
+     *  Important note:
+     * OnReadBegin and OnReadEnd are not called when using the Json serializer, because the Json serializer's API assumes that
+     * reading from C++ objects into JSON has no side effects on the objects being serialized, and thus will not call a non-const
+     * callback such as these.
+     *
+     * The ObjectStream serializer has no problem with this and will in fact const-cast the const ptr fed into it, just so that it can
+     * invoke OnReadBegin and OnReadEnd.
+     * 
+     * OnReadBegin and OnReadEnd are called during serialization - that is, when reading FROM a C++ object INTO an ObjectStream stream,
+     * and the purpose of which is to fixup data in the c++ object before / after saving it to a data stream.
+     * If you want to fix up data after LOADING it into a C++ object, use OnWriteEnd, not OnReadEnd.
+     *
+     * Additional Caveat:  It is possible to tell the serialize context to walk a tree of reflected C++ objects and invoke a callback
+     * for each element on them - this is, in fact, how serializing works (it walks the tree of reflected objects and serializes them).
+     * If you are using this feature, you should be aware that OnReadBegin and OnReadEnd will be called for each element in the tree,
+     * since it is "reading from" the objects.  However, if you pass the ENUM_ACCESS_FOR_WRITE flag, it will INSTEAD call OnWriteBegin
+     * and OnWriteEnd for the c++ objects it is visiting, despite the fact that you are technically enumerating them.
      */
     class IEventHandler
     {
     public:
         virtual ~IEventHandler() {}
 
+        /// the Read**** functions are called when SERIALIZING.  Do not use them to do post-load fixups.
         /// Called right before we start reading from the instance pointed by classPtr.
         virtual void OnReadBegin(void* classPtr) { (void)classPtr; }
         /// Called after we are done reading from the instance pointed by classPtr.
         virtual void OnReadEnd(void* classPtr) { (void)classPtr; }
 
+        /// The Write**** functions are called when DESERIALIZING.  You can use these to do post-load fixups.
         /// Called right before we start writing to the instance pointed by classPtr.
         virtual void OnWriteBegin(void* classPtr) { (void)classPtr; }
         /// Called after we are done writing to the instance pointed by classPtr.
         /// NOTE: Care must be taken when using this callback. It is called when ID remapping occurs,
-        /// an instance is clone or an instance is loaded from an objectstream.
+        /// an instance is cloned or an instance is loaded from an ObjectStream.
         /// This means that this function can be invoked multiple times in the course of serializing a new instance from an ObjectStream
         /// or cloning an object.
         virtual void OnWriteEnd(void* classPtr) { (void)classPtr; }
 
+        /// PATCHING
+        /// These functions will not be called when using the Json serializer, as patching operates entirely differently.
         /// Called right before we start data patching the instance pointed by classPtr.
         virtual void OnPatchBegin(void* classPtr, const DataPatchNodeInfo& patchInfo) { (void)classPtr; (void)patchInfo; }
         /// Called after we are done data patching the instance pointed by classPtr.

Code/Framework/AzFramework/AzFramework/Spawnable/Script/SpawnableScriptAssetRef.h

@@ -47,7 +47,8 @@ namespace AzFramework::Scripts
     private:
         class SerializationEvents : public AZ::SerializeContext::IEventHandler
         {
-            void OnReadEnd(void* classPtr) override
+            // Called when the Serializer has completed writing TO the c++ object in memory.
+            void OnWriteEnd(void* classPtr) override
             {
                 SpawnableScriptAssetRef* spawnableScriptAssetRef = reinterpret_cast<SpawnableScriptAssetRef*>(classPtr);
                 // Call SetAsset to connect AssetBus handler as soon as m_asset field is set

Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntityHelpers.cpp

@@ -662,6 +662,163 @@ namespace AzToolsFramework
             componentsOnEntity.end());
     }
 
+    
+    AZStd::optional<int> GetFixedComponentListIndex(const AZ::Component* component)
+    {
+        if (component)
+        {
+            auto componentClassData = GetComponentClassData(component);
+            if (componentClassData && componentClassData->m_editData)
+            {
+                if (auto editorDataElement = componentClassData->m_editData->FindElementData(AZ::Edit::ClassElements::EditorData))
+                {
+                    if (auto attribute = editorDataElement->FindAttribute(AZ::Edit::Attributes::FixedComponentListIndex))
+                    {
+                        if (auto attributeData = azdynamic_cast<AZ::Edit::AttributeData<int>*>(attribute))
+                        {
+                            return { attributeData->Get(nullptr) };
+                        }
+                    }
+                }
+            }
+        }
+        return {};
+    }
+    
+    bool IsComponentDraggable(const AZ::Component* component)
+    {
+        return !GetFixedComponentListIndex(component).has_value();
+    }
+
+    bool IsComponentRemovable(const AZ::Component* component)
+    {
+        // Determine if this component can be removed.
+        auto componentClassData = component ? GetComponentClassData(component) : nullptr;
+        if (componentClassData && componentClassData->m_editData)
+        {
+            if (auto editorDataElement = componentClassData->m_editData->FindElementData(AZ::Edit::ClassElements::EditorData))
+            {
+                if (auto attribute = editorDataElement->FindAttribute(AZ::Edit::Attributes::RemoveableByUser))
+                {
+                    if (auto attributeData = azdynamic_cast<AZ::Edit::AttributeData<bool>*>(attribute))
+                    {
+                        return attributeData->Get(nullptr);
+                    }
+                }
+            }
+        }
+
+        if (componentClassData && AppearsInAnyComponentMenu(*componentClassData))
+        {
+            return true;
+        }
+
+        // If this is a GenericComponentWrapper which wraps a nullptr, let the user remove it
+        if (auto genericComponentWrapper = azrtti_cast<const Components::GenericComponentWrapper*>(component))
+        {
+            if (!genericComponentWrapper->GetTemplate())
+            {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    void SortComponentsByOrder(const AZ::EntityId entityId, AZ::Entity::ComponentArrayType& componentsOnEntity)
+    {
+        // sort by component order, shuffling anything not found in component order to the end
+        ComponentOrderArray componentOrder;
+        EditorInspectorComponentRequestBus::EventResult(
+            componentOrder, entityId, &EditorInspectorComponentRequests::GetComponentOrderArray);
+
+        if (componentOrder.empty())
+        {
+            return;
+        }
+
+        AZStd::sort(
+            componentsOnEntity.begin(),
+            componentsOnEntity.end(),
+            [&componentOrder](const AZ::Component* component1, const AZ::Component* component2)
+            {
+                return
+                    AZStd::find(componentOrder.begin(), componentOrder.end(), component1->GetId()) <
+                    AZStd::find(componentOrder.begin(), componentOrder.end(), component2->GetId());
+            });
+    }
+
+    void SortComponentsByPriority(AZ::Entity::ComponentArrayType& componentsOnEntity)
+    {
+        // The default order for components is sorted by priorit only
+        struct OrderedSortComponentEntry
+        {
+            AZ::Component* m_component;
+            int m_originalOrder;
+
+            OrderedSortComponentEntry(AZ::Component* component, int originalOrder)
+            {
+                m_component = component;
+                m_originalOrder = originalOrder;
+            }
+        };
+
+        AZStd::vector< OrderedSortComponentEntry> sortedComponents;
+        int index = 0;
+        for (AZ::Component* component : componentsOnEntity)
+        {
+            sortedComponents.push_back(OrderedSortComponentEntry(component, index++));
+        }
+
+        // shuffle immovable components back to the front
+        AZStd::sort(
+            sortedComponents.begin(),
+            sortedComponents.end(),
+            [](const OrderedSortComponentEntry& component1, const OrderedSortComponentEntry& component2)
+            {
+                AZStd::optional<int> fixedComponentListIndex1 = GetFixedComponentListIndex(component1.m_component);
+                AZStd::optional<int> fixedComponentListIndex2 = GetFixedComponentListIndex(component2.m_component);
+
+                // If both components have fixed list indices, sort based on those indices
+                if (fixedComponentListIndex1.has_value() && fixedComponentListIndex2.has_value())
+                {
+                    return fixedComponentListIndex1.value() < fixedComponentListIndex2.value();
+                }
+
+                // If component 1 has a fixed list index, sort it first
+                if (fixedComponentListIndex1.has_value())
+                {
+                    return true;
+                }
+
+                // If component 2 has a fixed list index, component 1 should not be sorted before it
+                if (fixedComponentListIndex2.has_value())
+                {
+                    return false;
+                }
+
+                if (!IsComponentRemovable(component1.m_component) && IsComponentRemovable(component2.m_component))
+                {
+                    return true;
+                }
+
+                if (IsComponentRemovable(component1.m_component) && !IsComponentRemovable(component2.m_component))
+                {
+                    return false;
+                }
+
+                //maintain original order if they don't need swapping
+                return component1.m_originalOrder < component2.m_originalOrder;
+            });
+
+        //create new order array from sorted structure
+        componentsOnEntity.clear();
+        for (OrderedSortComponentEntry& component : sortedComponents)
+        {
+            componentsOnEntity.push_back(component.m_component);
+        }
+    }
+
     bool IsSelected(const AZ::EntityId entityId)
     {
         bool selected = false;

Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntityHelpers.h

@@ -8,13 +8,15 @@
 #pragma once
 
 #include <AzCore/Component/Component.h>
+#include <AzCore/Component/ComponentApplication.h>
 #include <AzCore/Component/Entity.h>
 #include <AzCore/Serialization/SerializeContext.h>
+#include <AzCore/std/optional.h>
+
 #include <AzToolsFramework/ToolsComponents/EditorComponentBase.h>
 #include <AzToolsFramework/ToolsComponents/EditorDisabledCompositionBus.h>
 #include <AzToolsFramework/ToolsComponents/EditorPendingCompositionBus.h>
 #include <AzToolsFramework/API/EntityCompositionRequestBus.h>
-#include <AzCore/Component/ComponentApplication.h>
 
 namespace AzToolsFramework
 {
@@ -110,6 +112,30 @@ namespace AzToolsFramework
     /// false if the component should be hidden from users.
     bool ShouldInspectorShowComponent(const AZ::Component* component);
 
+    //! Components can set an attribute (@ref AZ::Edit::Attributes::FixedComponentListIndex) to specify
+    //! that they should appear at a fixed position in the property editor and should not be draggable.
+    //! This helper function will return that index if it is set, or an empty optional if it is not.
+    AZStd::optional<int> GetFixedComponentListIndex(const AZ::Component* component);
+
+    //! Returns true if the component can be removed by the entity inspector.
+    bool IsComponentRemovable(const AZ::Component* component);
+
+    //! Returns true if the given component is draggable in the entity inspector.
+    bool IsComponentDraggable(const AZ::Component* component);
+
+    //! Given a ComponentArrayType, sort them into the order they would by default be sorted
+    //! based on various attributes such as whether they can be dragged, deleted, whether they are
+    //! visible, etc, but does not take into account the user modified order from dragging and dropping
+    //! Note that this sort will try its best to keep things in the order they were in the original array
+    //! and will attempt to only move elements around if necessary, so they can be sorted in another way first,
+    //! then sorted by this function to ensure any additional constraints are met.
+    void SortComponentsByPriority(AZ::Entity::ComponentArrayType& componentsOnEntity);
+
+    //! Sorts the components on the entity based on the order specified in the componentOrder array on the entity.
+    //! The order will shuffle around since this is not a stable sort, but it can be sorted by priority afterwards to stabilize.
+    //! Helper function, same as above, but sorts components by order by getting the order from the given entityId.
+    void SortComponentsByOrder(const AZ::EntityId entityId, AZ::Entity::ComponentArrayType& componentsOnEntity);
+
     AZ::EntityId GetEntityIdForSortInfo(const AZ::EntityId parentId);
 
     void AddEntityIdToSortInfo(const AZ::EntityId parentId, const AZ::EntityId childId, bool forceAddToBack = false);

Code/Framework/AzToolsFramework/AzToolsFramework/ToolsComponents/EditorEntityIconComponent.cpp

@@ -14,6 +14,7 @@
 #include <AzToolsFramework/API/EditorAssetSystemAPI.h>
 #include <AzToolsFramework/API/EditorViewportIconDisplayInterface.h>
 #include <AzToolsFramework/API/ToolsApplicationAPI.h>
+#include <AzToolsFramework/Entity/EditorEntityHelpers.h>
 #include <AzToolsFramework/ToolsComponents/EditorVisibilityBus.h>
 #include <AzToolsFramework/ToolsComponents/GenericComponentWrapper.h>
 #include <AzToolsFramework/ToolsComponents/TransformComponent.h>
@@ -65,8 +66,8 @@ namespace AzToolsFramework
 
         void EditorEntityIconComponent::Activate()
         {
+            m_needsInitialUpdate = true;
             EditorComponentBase::Activate();
-            AZ::EntityBus::Handler::BusConnect(GetEntityId());
             EditorEntityIconComponentRequestBus::Handler::BusConnect(GetEntityId());
             EditorInspectorComponentNotificationBus::Handler::BusConnect(GetEntityId());
         }
@@ -75,7 +76,6 @@ namespace AzToolsFramework
         {
             EditorInspectorComponentNotificationBus::Handler::BusDisconnect();
             EditorEntityIconComponentRequestBus::Handler::BusDisconnect();
-            AZ::EntityBus::Handler::BusDisconnect();
             EditorComponentBase::Deactivate();
         }
 
@@ -83,8 +83,14 @@ namespace AzToolsFramework
         {
             if (m_entityIconAssetId != assetId)
             {
+                // Note that we could be going from a situation where we had an icon, to one where we don't so the cache needs refreshing.
+                if (!assetId.IsValid())
+                {
+                    m_needsInitialUpdate = true;
+                }
                 m_entityIconAssetId = assetId;
-                m_entityIconCache.SetEntityIconPath(CalculateEntityIconPath(m_firstComponentIdCache));
+                m_entityIconCache.SetEntityIconPath(CalculateEntityIconPath());
+                                
                 EditorEntityIconComponentNotificationBus::Event(GetEntityId(), &EditorEntityIconComponentNotificationBus::Events::OnEntityIconChanged, m_entityIconAssetId);
                 SetDirty();
             }
@@ -97,36 +103,38 @@ namespace AzToolsFramework
 
         AZStd::string EditorEntityIconComponent::GetEntityIconPath()
         {
-            if (m_entityIconCache.Empty())
-            {
-                UpdateFirstComponentIdCache();
-                m_entityIconCache.SetEntityIconPath(CalculateEntityIconPath(m_firstComponentIdCache));
-            }
-
+            RefreshCachesIfNecessary();
             return m_entityIconCache.GetEntityIconPath();
         }
 
         int EditorEntityIconComponent::GetEntityIconTextureId()
         {
+            RefreshCachesIfNecessary();
             return m_entityIconCache.GetEntityIconTextureId();
         }
 
-        bool EditorEntityIconComponent::IsEntityIconHiddenInViewport()
+        void EditorEntityIconComponent::RefreshCachesIfNecessary()
         {
-            return (!m_entityIconAssetId.IsValid() && m_preferNoViewportIcon);
-        }
-
-        void EditorEntityIconComponent::OnEntityActivated(const AZ::EntityId&)
-        {
-            if (m_entityIconCache.Empty())
+            if (!m_needsInitialUpdate)
             {
-                /* The case where the entity is activated the first time. */
+                return;
+            }
+
+            m_needsInitialUpdate = false;
 
+            if (m_firstComponentIdCache == AZ::InvalidComponentId)
+            {
                 UpdatePreferNoViewportIconFlag();
                 UpdateFirstComponentIdCache();
-                m_entityIconCache.SetEntityIconPath(CalculateEntityIconPath(m_firstComponentIdCache));
-                EditorEntityIconComponentNotificationBus::Event(GetEntityId(), &EditorEntityIconComponentNotificationBus::Events::OnEntityIconChanged, m_entityIconAssetId);
             }
+
+            m_entityIconCache.SetEntityIconPath(CalculateEntityIconPath());
+        }
+
+        bool EditorEntityIconComponent::IsEntityIconHiddenInViewport()
+        {
+            RefreshCachesIfNecessary();
+            return (!m_entityIconAssetId.IsValid() && m_preferNoViewportIcon);
         }
 
         void EditorEntityIconComponent::OnComponentOrderChanged()
@@ -138,22 +146,23 @@ namespace AzToolsFramework
 
                 if (firstComponentIdChanged)
                 {
-                    m_entityIconCache.SetEntityIconPath(GetDefaultEntityIconPath(m_firstComponentIdCache));
+                    m_entityIconCache.SetEntityIconPath(GetDefaultEntityIconPath());
                     EditorEntityIconComponentNotificationBus::Event(GetEntityId(), &EditorEntityIconComponentNotificationBus::Events::OnEntityIconChanged, m_entityIconAssetId);
                 }
                 else if (preferNoViewportIconFlagChanged)
                 {
                     EditorEntityIconComponentNotificationBus::Event(GetEntityId(), &EditorEntityIconComponentNotificationBus::Events::OnEntityIconChanged, m_entityIconAssetId);
                 }
+                m_needsInitialUpdate = false; // avoid doing the work twice.
             }
         }
 
-        AZStd::string EditorEntityIconComponent::CalculateEntityIconPath(AZ::ComponentId firstComponentId)
+        AZStd::string EditorEntityIconComponent::CalculateEntityIconPath()
         {
             AZStd::string entityIconPath = GetEntityIconAssetPath();
             if (entityIconPath.empty())
             {
-                entityIconPath = GetDefaultEntityIconPath(firstComponentId);
+                entityIconPath = GetDefaultEntityIconPath();
             }
             return entityIconPath;
         }
@@ -182,10 +191,13 @@ namespace AzToolsFramework
             }
         }
 
-        AZStd::string EditorEntityIconComponent::GetDefaultEntityIconPath(AZ::ComponentId firstComponentId)
+        AZStd::string EditorEntityIconComponent::GetDefaultEntityIconPath()
         {
             AZStd::string entityIconPath;
 
+            RefreshCachesIfNecessary();
+            AZ::ComponentId firstComponentId = m_firstComponentIdCache;
+
             if (firstComponentId != AZ::InvalidComponentId)
             {
                 AZ::Entity* entity = GetEntity();
@@ -210,32 +222,48 @@ namespace AzToolsFramework
 
         bool EditorEntityIconComponent::UpdateFirstComponentIdCache()
         {
+            // The idea here is to find the first component that is not a fixed component (like TransformComponent, UniformScaleComponent,
+            // etc.) on the entity and use that as the default icon.
+            // The entity only stores its component order if the component order is different from the default order, which is only the case
+            // if the user has personally modified the order of the components, which is not the case for the vast majority of entities in real levels.
+            // So to figure out the icon, we need to essentially do the same work the inspector would do, which is to take the components on the entity
+            // and filter out the invisible ones, then sort them by order if order is present, and then by priority.
             bool firstComponentIdChanged = false;
             ComponentOrderArray componentOrderArray;
             EditorInspectorComponentRequestBus::EventResult(componentOrderArray, GetEntityId(), &EditorInspectorComponentRequests::GetComponentOrderArray);
-            if (componentOrderArray.empty())
-            {
-                if (m_firstComponentIdCache != AZ::InvalidComponentId)
-                {
-                    m_firstComponentIdCache = AZ::InvalidComponentId;
-                    firstComponentIdChanged = true;
-                }
-            }
-            else
+            AZ::Entity::ComponentArrayType components;
+            components = GetEntity()->GetComponents();
+            RemoveHiddenComponents(components);
+            SortComponentsByOrder(GetEntity()->GetId(), components);
+            SortComponentsByPriority(components);
+
+            AZ::ComponentId componentIdToSet = AZ::InvalidComponentId;
+            // we want to use the first "non-fixed" component on the entity (ie, not the uniform scale, or transform) but we'll settle for
+            // the first visible component if we can't find one.
+            if (!components.empty())
             {
-                if (componentOrderArray.size() > 1)
+                for (const AZ::Component* component : components)
                 {
-                    if (m_firstComponentIdCache != componentOrderArray[1])
+                    if (componentIdToSet == AZ::InvalidComponentId)
                     {
-                        m_firstComponentIdCache = componentOrderArray[1];
-                        firstComponentIdChanged = true;
+                        // initialize it to the first one as a fallback
+                        componentIdToSet = component->GetId();
+                    }
+                    else
+                    {
+                        if (IsComponentDraggable(component))
+                        {
+                            componentIdToSet = component->GetId();
+                            break;
+                        }
                     }
                 }
-                else if(m_firstComponentIdCache != componentOrderArray.front())
-                {
-                    m_firstComponentIdCache = componentOrderArray.front();
-                    firstComponentIdChanged = true;
-                }
+            }
+
+            if (m_firstComponentIdCache != componentIdToSet)
+            {
+                m_firstComponentIdCache = componentIdToSet;
+                firstComponentIdChanged = true;
             }
 
             return firstComponentIdChanged;

Code/Framework/AzToolsFramework/AzToolsFramework/ToolsComponents/EditorEntityIconComponent.h

@@ -77,22 +77,20 @@ namespace AzToolsFramework
             int GetEntityIconTextureId() override;
             bool IsEntityIconHiddenInViewport() override;
 
-            // EntityBus
-            void OnEntityActivated(const AZ::EntityId&) override;
-
             // EditorInspectorComponentNotificationBus
             void OnComponentOrderChanged() override;
 
             /// Return the path of the entity icon asset identified by \ref m_entityIconAssetId if it's valid,
             /// else return the path of the icon of the first component in this entity's EditorInspector list,
             /// otherwise return the path of the default entity icon.
-            AZStd::string CalculateEntityIconPath(AZ::ComponentId firstComponentId);
+            AZStd::string CalculateEntityIconPath();
             AZStd::string GetEntityIconAssetPath();
-            AZStd::string GetDefaultEntityIconPath(AZ::ComponentId firstComponentId);
+            AZStd::string GetDefaultEntityIconPath();
 
             /// Return a boolean indicating if \ref m_firstComponentIdCache has been changed.
             bool UpdateFirstComponentIdCache();
             bool UpdatePreferNoViewportIconFlag();
+            void RefreshCachesIfNecessary();
 
             AZ::Data::AssetId m_entityIconAssetId = AZ::Data::AssetId();
 
@@ -103,6 +101,8 @@ namespace AzToolsFramework
 
             bool m_preferNoViewportIcon = false; ///< Indicates if any component of this entity
                                                  ///< has the PreferNoViewportIcon Edit Attribute.
+
+            bool m_needsInitialUpdate = true;
         };
     }
 }

Code/Framework/AzToolsFramework/AzToolsFramework/ToolsComponents/EditorInspectorComponent.cpp

@@ -8,6 +8,7 @@
 #include "EditorInspectorComponent.h"
 #include <AzCore/Serialization/EditContext.h>
 #include <AzCore/std/sort.h>
+#include <AzToolsFramework/Entity/EditorEntityHelpers.h>
 
 namespace AzToolsFramework
 {
@@ -124,12 +125,6 @@ namespace AzToolsFramework
 
         void EditorInspectorComponent::PrepareSave()
         {
-            // If we didn't dirty the component order, we do not need to regenerate the serialized entry array
-            if (!m_componentOrderIsDirty)
-            {
-                return;
-            }
-
             // Clear the actual persistent id storage to get rebuilt from the order entry array
             m_componentOrderEntryArray.clear();
             m_componentOrderEntryArray.reserve(m_componentOrderArray.size());
@@ -139,8 +134,6 @@ namespace AzToolsFramework
             {
                 m_componentOrderEntryArray.push_back({ m_componentOrderArray[componentIndex], componentIndex });
             }
-
-            m_componentOrderIsDirty = false;
         }
 
         void EditorInspectorComponent::PostLoad()
@@ -163,8 +156,6 @@ namespace AzToolsFramework
             {
                 m_componentOrderArray.push_back(componentOrderEntry.m_componentId);
             }
-
-            m_componentOrderIsDirty = false;
         }
 
         ComponentOrderArray EditorInspectorComponent::GetComponentOrderArray()
@@ -176,16 +167,49 @@ namespace AzToolsFramework
         {
             if (m_componentOrderArray == componentOrderArray)
             {
+                // the order is unchanged, not necessary to set or invoke callbacks.
                 return;
             }
 
-            m_componentOrderArray = componentOrderArray;
+            // If we get here, the caller is requesting a new order, but, we only want to persist this if the order is actually
+            // different from what default would be.
+            AZ::Entity::ComponentArrayType components;
+            components = GetEntity()->GetComponents();
+            RemoveHiddenComponents(components);
+            SortComponentsByPriority(components);
+
+            ComponentOrderArray defaultOrderArray;
+            defaultOrderArray.reserve(components.size());
+
+            for (const auto& component : components)
+            {
+                defaultOrderArray.push_back(component->GetId());
+            }
+
+            if (defaultOrderArray == componentOrderArray)
+            {
+                // the new order is the same as the default order
+                if (m_componentOrderArray.empty())
+                {
+                    // the new order is default and the previous order is default, nothing to do.
+                    return;
+                }
+
+                m_componentOrderArray.clear();
+            }
+            else
+            {
+                // the new order is non-default and the previous order is different from it, we need to persist it.
+                m_componentOrderArray = componentOrderArray;
+            }
 
+            // if we get here, we either went from a default order to a user-specified order
+            // or vice versa.  Either way, the order has changed and needs to persist the change.
+            // We should only get in here from user interaction - dragging things around, cut and paste, etc
+            // so we are in an undo state.
             SetDirty();
+            PrepareSave();
 
-            // mark the order as dirty before sending the OnComponentOrderChanged event in order for PrepareSave to be properly handled in the case 
-            // one of the event listeners needs to build the InstanceDataHierarchy
-            m_componentOrderIsDirty = true;
             EditorInspectorComponentNotificationBus::Event(GetEntityId(), &EditorInspectorComponentNotificationBus::Events::OnComponentOrderChanged);
         }
 

Code/Framework/AzToolsFramework/AzToolsFramework/ToolsComponents/EditorInspectorComponent.h

@@ -58,17 +58,8 @@ namespace AzToolsFramework
             class ComponentOrderSerializationEvents
                 : public AZ::SerializeContext::IEventHandler
             {
-                /** 
-                 * Called right before we start reading from the instance pointed by classPtr.
-                 */
-                void OnReadBegin(void* classPtr) override
-                {
-                    EditorInspectorComponent* component = reinterpret_cast<EditorInspectorComponent*>(classPtr);
-                    component->PrepareSave();
-                }
-
                 /**
-                 * Called right after we finish writing data to the instance pointed at by classPtr.
+                 * Called after reading from a serialized file into a EditorInspectorComponent object.
                  */
                 void OnWriteEnd(void* classPtr) override
                 {
@@ -95,8 +86,6 @@ namespace AzToolsFramework
             ComponentOrderEntryArray m_componentOrderEntryArray; ///< The serialized order array which uses the persistent id mechanism as described above*/
 
             ComponentOrderArray m_componentOrderArray; ///< The simple vector of component id is what is used by the component order ebus and is generated from the serialized data
-            
-            bool m_componentOrderIsDirty = true; ///< This flag indicates our stored serialization order data is out of date and must be rebuilt before serialization occurs
         };
     } // namespace Components
 } // namespace AzToolsFramework

Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/EntityPropertyEditor.cpp

@@ -1415,88 +1415,9 @@ namespace AzToolsFramework
             RemoveHiddenComponents(componentsOnEntity);
             SortComponentsByOrder(entityId, componentsOnEntity);
             SortComponentsByPriority(componentsOnEntity);
-            SaveComponentOrder(entityId, componentsOnEntity);
-        }
-    }
-
-    void EntityPropertyEditor::SortComponentsByPriority(AZ::Entity::ComponentArrayType& componentsOnEntity)
-    {
-        // Create list of components with their current order. AZStd::sort isn't guaranteed to maintain order for equivalent entities.
-
-        AZStd::vector< OrderedSortComponentEntry> sortedComponents;
-        int index = 0;
-        for (AZ::Component* component : componentsOnEntity)
-        {
-            sortedComponents.push_back(OrderedSortComponentEntry(component, index++));
-        }
-
-        // shuffle immovable components back to the front
-        AZStd::sort(
-            sortedComponents.begin(),
-            sortedComponents.end(),
-            [](const OrderedSortComponentEntry& component1, const OrderedSortComponentEntry& component2)
-            {
-                AZStd::optional<int> fixedComponentListIndex1 = GetFixedComponentListIndex(component1.m_component);
-                AZStd::optional<int> fixedComponentListIndex2 = GetFixedComponentListIndex(component2.m_component);
-
-                // If both components have fixed list indices, sort based on those indices
-                if (fixedComponentListIndex1.has_value() && fixedComponentListIndex2.has_value())
-                {
-                    return fixedComponentListIndex1.value() < fixedComponentListIndex2.value();
-                }
-
-                // If component 1 has a fixed list index, sort it first
-                if (fixedComponentListIndex1.has_value())
-                {
-                    return true;
-                }
-
-                // If component 2 has a fixed list index, component 1 should not be sorted before it
-                if (fixedComponentListIndex2.has_value())
-                {
-                    return false;
-                }
-
-                if (!IsComponentRemovable(component1.m_component) && IsComponentRemovable(component2.m_component))
-                {
-                    return true;
-                }
-
-                if (IsComponentRemovable(component1.m_component) && !IsComponentRemovable(component2.m_component))
-                {
-                    return false;
-                }
-
-                //maintain original order if they don't need swapping
-                return component1.m_originalOrder < component2.m_originalOrder;
-            });
-
-        //create new order array from sorted structure
-        componentsOnEntity.clear();
-        for (OrderedSortComponentEntry& component : sortedComponents)
-        {
-            componentsOnEntity.push_back(component.m_component);
         }
     }
 
-    void SortComponentsByOrder(const AZ::EntityId entityId, AZ::Entity::ComponentArrayType& componentsOnEntity)
-    {
-        // sort by component order, shuffling anything not found in component order to the end
-        ComponentOrderArray componentOrder;
-        EditorInspectorComponentRequestBus::EventResult(
-            componentOrder, entityId, &EditorInspectorComponentRequests::GetComponentOrderArray);
-
-        AZStd::sort(
-            componentsOnEntity.begin(),
-            componentsOnEntity.end(),
-            [&componentOrder](const AZ::Component* component1, const AZ::Component* component2)
-            {
-                return
-                    AZStd::find(componentOrder.begin(), componentOrder.end(), component1->GetId()) <
-                    AZStd::find(componentOrder.begin(), componentOrder.end(), component2->GetId());
-            });
-    }
-
     void SaveComponentOrder(const AZ::EntityId entityId, AZStd::span<AZ::Component* const> componentsInOrder)
     {
         ComponentOrderArray componentOrder;
@@ -1521,42 +1442,7 @@ namespace AzToolsFramework
         return componentClassData && filter(*componentClassData);
     }
 
-    bool EntityPropertyEditor::IsComponentRemovable(const AZ::Component* component)
-    {
-        // Determine if this component can be removed.
-        auto componentClassData = component ? GetComponentClassData(component) : nullptr;
-        if (componentClassData && componentClassData->m_editData)
-        {
-            if (auto editorDataElement = componentClassData->m_editData->FindElementData(AZ::Edit::ClassElements::EditorData))
-            {
-                if (auto attribute = editorDataElement->FindAttribute(AZ::Edit::Attributes::RemoveableByUser))
-                {
-                    if (auto attributeData = azdynamic_cast<AZ::Edit::AttributeData<bool>*>(attribute))
-                    {
-                        return attributeData->Get(nullptr);
-                    }
-                }
-            }
-        }
-
-        if (componentClassData && AppearsInAnyComponentMenu(*componentClassData))
-        {
-            return true;
-        }
-
-        // If this is a GenericComponentWrapper which wraps a nullptr, let the user remove it
-        if (auto genericComponentWrapper = azrtti_cast<const Components::GenericComponentWrapper*>(component))
-        {
-            if (!genericComponentWrapper->GetTemplate())
-            {
-                return true;
-            }
-        }
-
-        return false;
-    }
-
-    bool EntityPropertyEditor::AreComponentsRemovable(AZStd::span<AZ::Component* const> components) const
+   bool EntityPropertyEditor::AreComponentsRemovable(AZStd::span<AZ::Component* const> components) const
     {
         for (auto component : components)
         {
@@ -1568,24 +1454,6 @@ namespace AzToolsFramework
         return true;
     }
 
-    AZStd::optional<int> EntityPropertyEditor::GetFixedComponentListIndex(const AZ::Component* component)
-    {
-        auto componentClassData = component ? GetComponentClassData(component) : nullptr;
-        if (componentClassData && componentClassData->m_editData)
-        {
-            if (auto editorDataElement = componentClassData->m_editData->FindElementData(AZ::Edit::ClassElements::EditorData))
-            {
-                if (auto attribute = editorDataElement->FindAttribute(AZ::Edit::Attributes::FixedComponentListIndex))
-                {
-                    if (auto attributeData = azdynamic_cast<AZ::Edit::AttributeData<int>*>(attribute))
-                    {
-                        return { attributeData->Get(nullptr) };
-                    }
-                }
-            }
-        }
-        return {};
-    }
 
     bool EntityPropertyEditor::AllowAnyComponentModification() const
     {
@@ -1594,11 +1462,6 @@ namespace AzToolsFramework
                currentLayout != InspectorLayout::ContainerEntity;
     }
 
-    bool EntityPropertyEditor::IsComponentDraggable(const AZ::Component* component)
-    {
-        return !GetFixedComponentListIndex(component).has_value();
-    }
-
     bool EntityPropertyEditor::AreComponentsDraggable(AZStd::span<AZ::Component* const> components) const
     {
         if (!AllowAnyComponentModification())
@@ -5442,6 +5305,11 @@ namespace AzToolsFramework
 
     void EntityPropertyEditor::OnComponentOrderChanged()
     {
+        if (m_isBuildingProperties)
+        {
+            return;
+        }
+
         QueuePropertyRefresh();
         m_shouldScrollToNewComponents = true;
     }

Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/EntityPropertyEditor.hxx

@@ -94,18 +94,6 @@ namespace AzToolsFramework
 
     using ComponentEditorVector = AZStd::vector<ComponentEditor*>;
 
-    struct OrderedSortComponentEntry
-    {
-        AZ::Component* m_component;
-        int m_originalOrder;
-
-        OrderedSortComponentEntry(AZ::Component* component, int originalOrder)
-        {
-            m_component = component;
-            m_originalOrder = originalOrder;
-        }
-    };
-
     /**
      * the entity property editor shows all components for a given entity or set of entities.
      * it displays their values and lets you edit them.  The editing actually happens through the sub editor parts, though.
@@ -186,8 +174,6 @@ namespace AzToolsFramework
 
         void SetSystemEntityEditor(bool isSystemEntityEditor);
 
-        static void SortComponentsByPriority(AZ::Entity::ComponentArrayType& componentsOnEntity);
-
         bool IsLockedToSpecificEntities() const { return !m_overrideSelectedEntityIds.empty(); }
 
         static bool AreComponentsCopyable(AZStd::span<AZ::Component* const> components, const ComponentFilter& filter);
@@ -306,10 +292,7 @@ namespace AzToolsFramework
         void UpdateEntityIcon();
         void UpdateEntityDisplay();
         static bool DoesComponentPassFilter(const AZ::Component* component, const ComponentFilter& filter);
-        static bool IsComponentRemovable(const AZ::Component* component);
         bool AreComponentsRemovable(AZStd::span<AZ::Component* const> components) const;
-        static AZStd::optional<int> GetFixedComponentListIndex(const AZ::Component* component);
-        static bool IsComponentDraggable(const AZ::Component* component);
         bool AllowAnyComponentModification() const;
         bool AreComponentsDraggable(AZStd::span<AZ::Component* const> components) const;
         bool AreComponentsCopyable(AZStd::span<AZ::Component* const> components) const;

Code/Framework/AzToolsFramework/Tests/EntityInspectorTests.cpp

@@ -23,11 +23,16 @@
 #include <AzToolsFramework/API/ToolsApplicationAPI.h>
 #include <AzToolsFramework/Entity/EditorEntityContextBus.h>
 #include <AzToolsFramework/Entity/EditorEntityHelpers.h>
+#include <AzToolsFramework/ToolsComponents/EditorInspectorComponent.h>
 #include <AzToolsFramework/UnitTest/ToolsTestApplication.h>
+#include <AzToolsFramework/ToolsComponents/EditorInspectorComponentBus.h>
 
 // Inspector Test Includes
 #include <AzToolsFramework/UI/ComponentPalette/ComponentPaletteUtil.hxx>
 
+#include <gmock/gmock.h>
+
+
 namespace UnitTest
 {
     // Test component that is NOT available for a user to interact with
@@ -269,6 +274,7 @@ namespace UnitTest
             m_application = aznew ToolsTestApplication("ComponentPaletteTests");
             AZ::ComponentApplication::StartupParameters startupParameters;
             startupParameters.m_loadSettingsRegistry = false;
+            startupParameters.m_loadAssetCatalog = false;
             m_application->Start(componentApplicationDesc, startupParameters);
             // Without this, the user settings component would attempt to save on finalize/shutdown. Since the file is
             // shared across the whole engine, if multiple tests are run in parallel, the saving could cause a crash 
@@ -370,4 +376,210 @@ namespace UnitTest
         // Verify that true is returned here when a system component is editable
         EXPECT_TRUE(AzToolsFramework::ComponentPaletteUtil::ContainsEditableComponents(context, &Filter_IsTestComponent3, AZ::ComponentDescriptor::DependencyArrayType()));
     }
+
+    // helperfunction to reflect serialize for all these components to keep code short
+    template<typename ComponentType>
+    void RegisterSerialize(AZ::ReflectContext* context, bool visible, const char* iconPath, int fixedIndex = -1)
+    {
+        if (auto serializeContext = azrtti_cast<AZ::SerializeContext*>(context))
+        {
+            serializeContext->Class<ComponentType, AZ::Component>();
+
+            if (AZ::EditContext* editContext = serializeContext->GetEditContext())
+            {
+                auto reflectionBuilder = editContext->Class<ComponentType>(AZ_STRINGIZE(ComponentType), AZ_STRINGIZE(ComponentType));
+                auto classElement = reflectionBuilder->ClassElement(AZ::Edit::ClassElements::EditorData, "");
+                classElement->Attribute(AZ::Edit::Attributes::AddableByUser, true)
+                    ->Attribute(AZ::Edit::Attributes::Visibility, visible ? AZ::Edit::PropertyVisibility::Show :  AZ::Edit::PropertyVisibility::Hide)
+                    ->Attribute(AZ::Edit::Attributes::AppearsInAddComponentMenu, AZ_CRC_CE("Game"))
+                    ->Attribute(AZ::Edit::Attributes::Category, "Inspector Test Components")
+                    ->Attribute(AZ::Edit::Attributes::Icon, iconPath)
+                    ->Attribute(AZ::Edit::Attributes::ViewportIcon, iconPath);
+
+                if (fixedIndex != -1)
+                {
+                    classElement->Attribute(AZ::Edit::Attributes::FixedComponentListIndex, fixedIndex);
+                }
+            }
+        }
+    }
+    
+    class EditorInspectorTestComponentBase : public AZ::Component
+    {
+    public:
+        // These functions are mandatory to provide but are of no use in this case.
+        void Activate() override {}
+        void Deactivate() override {}
+    };
+    
+    class EditorInspectorTestComponent1 : public EditorInspectorTestComponentBase
+    {
+        public:
+            AZ_COMPONENT(EditorInspectorTestComponent1, "{EF3D8047-4FAA-4615-93E1-C2B5B6EB3C08}", AZ::Component);
+            static void Reflect(AZ::ReflectContext* context)
+            {
+                // A component that is user movable and is visible
+                RegisterSerialize<EditorInspectorTestComponent1>(context, true, "Component1.png");
+            }
+    };
+
+    class EditorInspectorTestComponent2 : public EditorInspectorTestComponentBase
+    {
+        public:
+            AZ_COMPONENT(EditorInspectorTestComponent2, "{42BE5BEE-A7B9-4D8D-8F61-C0E0FDAA1450}", AZ::Component);
+            static void Reflect(AZ::ReflectContext* context)
+            {
+                // A component that is not movable, but is visible
+                RegisterSerialize<EditorInspectorTestComponent2>(context, true, "Component2.png", 0);
+            }
+    };
+
+    class EditorInspectorTestComponent3 : public EditorInspectorTestComponentBase
+    {
+        public:
+            AZ_COMPONENT(EditorInspectorTestComponent3, "{71329B94-76B3-4C8B-AF4B-159D51BDE820}", AZ::Component);
+            static void Reflect(AZ::ReflectContext* context)
+            {
+                // A component that is not visible
+                RegisterSerialize<EditorInspectorTestComponent3>(context, false, "Component3.png");
+            }
+    };
+
+    class EditorInspectorTestComponent4 : public EditorInspectorTestComponentBase
+    {
+        public:
+            AZ_COMPONENT(EditorInspectorTestComponent4, "{10385AEF-88AA-4682-AF1E-3EBE21E4632B}", AZ::Component);
+            static void Reflect(AZ::ReflectContext* context)
+            {
+                // Another component that is visible and movable
+                RegisterSerialize<EditorInspectorTestComponent4>(context, true, "Component4.png");
+            }
+    };
+    
+    class MockEditorInspectorNotificationBusHandler : public AzToolsFramework::EditorInspectorComponentNotificationBus::Handler
+    {
+    public:
+        MOCK_METHOD0(OnComponentOrderChanged, void());
+    };
+
+    class InspectorComponentOrderingTest
+        : public ComponentPaletteTests
+    {
+        void SetUp() override
+        {
+            ComponentPaletteTests::SetUp();
+            m_application->RegisterComponentDescriptor(EditorInspectorTestComponent1::CreateDescriptor());
+            m_application->RegisterComponentDescriptor(EditorInspectorTestComponent2::CreateDescriptor());
+            m_application->RegisterComponentDescriptor(EditorInspectorTestComponent3::CreateDescriptor());
+            m_application->RegisterComponentDescriptor(EditorInspectorTestComponent4::CreateDescriptor());
+            m_mockedInspectorBusHandler = AZStd::make_unique<::testing::NiceMock<MockEditorInspectorNotificationBusHandler>>();
+        }
+
+        void TearDown() override
+        {
+            m_mockedInspectorBusHandler->BusDisconnect();
+            m_mockedInspectorBusHandler.reset();
+            ComponentPaletteTests::TearDown();
+        }
+
+        protected:
+        AZStd::unique_ptr<::testing::NiceMock<MockEditorInspectorNotificationBusHandler>> m_mockedInspectorBusHandler;
+    };
+
+    // Makes sure that the inspector component (responsible for keeping track of any order overrides of components on it)
+    // only stores data and only emits events when the components are in a non default order.
+    // Also makes sure (since it invokes them) that the actual ordering utility functions, such as RemoveHiddenComponents,
+    // SortComponentsByPriority, and the functions they call, all work as expected.
+    TEST_F(InspectorComponentOrderingTest, AddingComponents_InspectorComponent_PersistsDataOnlyIfDifferentFromDefault)
+    {
+        using namespace AzToolsFramework;
+
+        AZ::EntityId entityId(123);
+
+        AZ::Entity testEntity(entityId);
+        testEntity.AddComponent(aznew EditorInspectorTestComponent1);
+        testEntity.AddComponent(aznew EditorInspectorTestComponent2);
+        testEntity.AddComponent(aznew EditorInspectorTestComponent3);
+        testEntity.AddComponent(aznew EditorInspectorTestComponent4);
+        testEntity.AddComponent(aznew Components::EditorInspectorComponent);
+
+        m_mockedInspectorBusHandler->BusConnect(entityId);
+
+        // activating the entity should not invoke the component order change bus at all, anything that cares about activation should listen for activation, not reorder.
+        EXPECT_CALL(*m_mockedInspectorBusHandler, OnComponentOrderChanged()).Times(0);
+        
+        // Activating an entity does reorder the actual components on the entity itself.
+        // They will not be in the order added.
+        // The actual order on the entity is not relevant to this test, but the stable sort function itself will place
+        // the components that provide services (EditorInspectorComponent) in this case, ahead of ones which don't, and
+        // if there is a tie, it will sort them by their typeid (their GUID).  In this case, it means the order will be:
+        // * EditorInspectorComponent (because it has services provided)
+        // * EditorInspectorTestComponent4 // TypeID starts with 10385AEF
+        // * EditorInspectorTestComponent2 // TypeID starts with 42BE5BEE
+        // * EditorInspectorTestComponent3 // TypeID starts with 71329B94
+        // * EditorInspectorTestComponent1 // TypeID starts with EF3D8047
+
+        testEntity.Init();
+        testEntity.Activate();
+
+        AZ::Entity::ComponentArrayType componentsOnEntity = testEntity.GetComponents();
+
+        EXPECT_EQ(componentsOnEntity.size(), 5);
+
+        // An empty component order array sent to an already empty entity should result in no callbacks.
+        ComponentOrderArray componentOrderArray;
+        EditorInspectorComponentRequestBus::Event(entityId, &EditorInspectorComponentRequests::SetComponentOrderArray, componentOrderArray);
+        EditorInspectorComponentRequestBus::EventResult(componentOrderArray, entityId, &EditorInspectorComponentRequests::GetComponentOrderArray);
+        EXPECT_TRUE(componentOrderArray.empty());
+
+        // Setting an empty component order when its already empty should result in no calls to the "Component order changed!" event.
+        EXPECT_CALL(*m_mockedInspectorBusHandler, OnComponentOrderChanged()).Times(0);
+
+        // Setting the component order array to what is already the default order should result in no callbacks:
+        AZ::Entity::ComponentArrayType components;
+        components = testEntity.GetComponents();
+        EXPECT_EQ(components.size(), 5);
+        RemoveHiddenComponents(components);
+        EXPECT_EQ(components.size(), 3); // the inspector component and the test Component 3 are hidden.
+        SortComponentsByPriority(components);
+        ASSERT_EQ(components.size(), 3); // sorting components should not change the number of components.
+
+        // After the sort, the first one in the array should be the fixed order one, that says it "must be in position 0"
+        EXPECT_EQ(components[0]->RTTI_GetType(), azrtti_typeid<EditorInspectorTestComponent2>());
+
+        // the others should remain in their original order, but be after it:
+        EXPECT_EQ(components[1]->RTTI_GetType(), azrtti_typeid<EditorInspectorTestComponent4>()); // Note the above, 4 comes before 1 due to the stable sort
+        EXPECT_EQ(components[2]->RTTI_GetType(), azrtti_typeid<EditorInspectorTestComponent1>());
+
+        // convert the vector of Component* to a vector of ComponentId
+        ComponentOrderArray defaultComponentOrder;
+        for (const AZ::Component* component : components)
+        {
+            defaultComponentOrder.push_back(component->GetId());
+        }
+        
+        // set the order.  Since its the default order, this should again not emit an event, nor update any data that would be persisted:
+        EditorInspectorComponentRequestBus::Event(entityId, &EditorInspectorComponentRequests::SetComponentOrderArray, defaultComponentOrder);
+        EditorInspectorComponentRequestBus::EventResult(componentOrderArray, entityId, &EditorInspectorComponentRequests::GetComponentOrderArray);
+        EXPECT_TRUE(componentOrderArray.empty());
+
+        // Setting the component order array to a different order than default should result in a callback and result in data to save.
+        // in this case, we swap the order of element [1] and [2], so that the final order should be [Component2, Component1, Component4]
+        ComponentOrderArray nonDefaultOrder = defaultComponentOrder;
+        AZStd::iter_swap(nonDefaultOrder.begin() + 1, nonDefaultOrder.begin() + 2);
+        EXPECT_CALL(*m_mockedInspectorBusHandler, OnComponentOrderChanged()).Times(1);
+        EditorInspectorComponentRequestBus::Event(entityId, &EditorInspectorComponentRequests::SetComponentOrderArray, nonDefaultOrder);
+        EditorInspectorComponentRequestBus::EventResult(componentOrderArray, entityId, &EditorInspectorComponentRequests::GetComponentOrderArray);
+        EXPECT_EQ(componentOrderArray.size(), 3);
+        EXPECT_EQ(componentOrderArray, nonDefaultOrder);
+
+        // Setting the component order array back to default, should result in it emptying it out and notifying since its changing (from non-default to default)
+        EXPECT_CALL(*m_mockedInspectorBusHandler, OnComponentOrderChanged()).Times(1);
+        EditorInspectorComponentRequestBus::Event(entityId, &EditorInspectorComponentRequests::SetComponentOrderArray, defaultComponentOrder);
+        EditorInspectorComponentRequestBus::EventResult(componentOrderArray, entityId, &EditorInspectorComponentRequests::GetComponentOrderArray);
+        EXPECT_TRUE(componentOrderArray.empty());
+
+        m_mockedInspectorBusHandler->BusDisconnect();
+        testEntity.Deactivate();
+    }
 }

Code/Framework/AzToolsFramework/Tests/UI/EntityPropertyEditorTests.cpp

@@ -6,28 +6,28 @@
  *
  */
 
-#include <AzCore/UnitTest/TestTypes.h>
+#include <AzCore/Asset/AssetManagerComponent.h>
 #include <AzCore/Serialization/SerializeContext.h>
+#include <AzCore/std/sort.h>
+#include <AzCore/UnitTest/TestTypes.h>
+
 #include <AzTest/AzTest.h>
-#include <AzToolsFramework/ComponentMode/ComponentModeCollection.h>
 
-#include <AzToolsFramework/Application/ToolsApplication.h>
-#include <AzToolsFramework/ViewportSelection/EditorInteractionSystemViewportSelectionRequestBus.h>
-#include <AzToolsFramework/ToolsComponents/TransformComponent.h>
-#include <AzToolsFramework/ToolsComponents/ScriptEditorComponent.h>
-#include <AzToolsFramework/UI/PropertyEditor/EntityPropertyEditor.hxx>
 #include <AzToolsFramework/API/EntityPropertyEditorRequestsBus.h>
+#include <AzToolsFramework/Application/ToolsApplication.h>
+#include <AzToolsFramework/ComponentMode/ComponentModeCollection.h>
+#include <AzToolsFramework/Entity/EditorEntityHelpers.h>
 #include <AzToolsFramework/ToolsComponents/EditorLockComponent.h>
 #include <AzToolsFramework/ToolsComponents/EditorVisibilityComponent.h>
+#include <AzToolsFramework/ToolsComponents/ScriptEditorComponent.h>
+#include <AzToolsFramework/ToolsComponents/TransformComponent.h>
+#include <AzToolsFramework/UI/PropertyEditor/EntityPropertyEditor.hxx>
+#include <AzToolsFramework/UnitTest/AzToolsFrameworkTestHelpers.h>
 #include <AzToolsFramework/ViewportSelection/EditorDefaultSelection.h>
-
-#include <AzCore/Asset/AssetManagerComponent.h>
-#include <AzCore/std/sort.h>
+#include <AzToolsFramework/ViewportSelection/EditorInteractionSystemViewportSelectionRequestBus.h>
 
 #include <QApplication>
 
-#include <AzToolsFramework/UnitTest/AzToolsFrameworkTestHelpers.h>
-
 namespace UnitTest
 {
     using namespace AZ;
@@ -78,7 +78,7 @@ namespace UnitTest
 
         // When this sort happens, the transformComponent should move to the top, the AssetDatabase should move to second, the order of the others should be unaltered, 
         // merely moved to after the AssetDatabase.
-        EntityPropertyEditor::SortComponentsByPriority(orderedComponents);
+        SortComponentsByPriority(orderedComponents);
 
         // Check the component arrays are intact.
         EXPECT_EQ(orderedComponents.size(), unorderedComponents.size());

Gems/MiniAudio/Code/Include/MiniAudio/SoundAssetRef.h

@@ -37,7 +37,9 @@ namespace MiniAudio
     private:
         class SerializationEvents : public AZ::SerializeContext::IEventHandler
         {
-            void OnReadEnd(void* classPtr) override
+            // OnWriteEnd happens after the object pointed at by classPtr
+            // has finished being deserialized and is fully loaded.
+            void OnWriteEnd(void* classPtr) override
             {
                 SoundAssetRef* SoundAssetRef = reinterpret_cast<class SoundAssetRef*>(classPtr);
                 // Call SetAsset to connect AssetBus handler as soon as m_asset field is set

Registry/AssetProcessorPlatformConfig.setreg

@@ -322,7 +322,7 @@
                     "productAssetType": "{B36FEB5C-41B6-4B58-A212-21EF5AEF523C}"
                 },
                 "RC png-entityicon": {
-                    "pattern": "(.*EntityIcons\\\\/).*\\\\.png",
+                    "pattern": "(.*(EntityIcons|Editor\\\\/Icons)\\\\/).*\\\\.(png|svg)",
                     "productAssetType": "{3436C30E-E2C5-4C3B-A7B9-66C94A28701B}",
                     "params": "skip",
                     "tools": "copy"