Fix UI overridden margin in derived controls being reset (#2310)

* [Quantum] Fix member values of struct not persisting when the struct itself is flagged as overridden * [Quantum] Fix method name typo in AssetPropertyGraph, fix minor typo in IGraphNode property documentation --------- Co-authored-by: Basewq <[email protected]>
stride3d · Jun 7, 2024 · c1b36e1 · c1b36e1
1 parent b67eab4
commit c1b36e1
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 11 deletions.
diff --git a/sources/assets/Stride.Core.Assets.Quantum.Tests/Helpers/Types.cs b/sources/assets/Stride.Core.Assets.Quantum.Tests/Helpers/Types.cs
@@ -137,13 +137,36 @@ public class MyAssetWithRef2 : MyAssetBase
             public static int MemberCount => 4 + 3; // 4 (number of members in Asset) + 3 (number of members in Types.MyAssetWithRef2)
         }
 
+        [DataContract]
+        [AssetDescription(FileExtension)]
+        public class MyAssetWithStructWithPrimitives : MyAssetBase
+        {
+            public StructWithPrimitives StructValue { get; set; }
+        }
 
         [DataContract]
         public struct StructWithList
         {
             public List<string> MyStrings { get; set; }
         }
 
+        [DataContract]
+        [DataStyle(DataStyle.Compact)]
+        public struct StructWithPrimitives : IEquatable<StructWithPrimitives>
+        {
+            [DefaultValue(0)]
+            public int Value1 { get; set; }
+            [DefaultValue(0)]
+            public int Value2 { get; set; }
+
+            public override readonly bool Equals(object obj) => obj is StructWithPrimitives value && Equals(value);
+            public readonly bool Equals(StructWithPrimitives other) => Value1 == other.Value1 && Value2 == other.Value2;
+
+            public override readonly int GetHashCode() => HashCode.Combine(Value1, Value2);
+
+            public override string ToString() => $"(Value1: {Value1}, Value2: {Value2})";
+        }
+
         public interface IMyInterface
         {
             string Value { get; set; }

diff --git a/sources/assets/Stride.Core.Assets.Quantum.Tests/TestReconcileWithBase.cs b/sources/assets/Stride.Core.Assets.Quantum.Tests/TestReconcileWithBase.cs
@@ -45,6 +45,29 @@ public void TestPrimitiveMember()
             Assert.Equal("MyBaseString", context.DerivedAsset.MyString);
         }
 
+        [Fact]
+        public void TestStructWithPrimitivesOverrideMember()
+        {
+            const string primitiveMemberBaseYaml = @"!Stride.Core.Assets.Quantum.Tests.Helpers.Types+MyAssetWithStructWithPrimitives,Stride.Core.Assets.Quantum.Tests
+Id: 10000000-0000-0000-0000-000000000000
+Tags: []
+StructValue: {}
+";
+            const string primitiveMemberOverriddenYaml = @"!Stride.Core.Assets.Quantum.Tests.Helpers.Types+MyAssetWithStructWithPrimitives,Stride.Core.Assets.Quantum.Tests
+Id: 30000000-0000-0000-0000-000000000000
+Archetype: 10000000-0000-0000-0000-000000000000:MyAsset
+Tags: []
+StructValue*: {Value1: 10, Value2: 20}
+";
+            var expectedOverriddenValue = new Types.StructWithPrimitives { Value1 = 10, Value2 = 20 };
+            var context = DeriveAssetTest<Types.MyAssetWithStructWithPrimitives, Types.MyAssetBasePropertyGraph>.LoadFromYaml(primitiveMemberBaseYaml, primitiveMemberOverriddenYaml);
+            Assert.Equal(default, context.BaseAsset.StructValue);
+            Assert.Equal(expectedOverriddenValue, context.DerivedAsset.StructValue);
+            context.DerivedGraph.ReconcileWithBase();
+            Assert.Equal(default, context.BaseAsset.StructValue);
+            Assert.Equal(expectedOverriddenValue, context.DerivedAsset.StructValue);
+        }
+
         [Fact]
         public void TestCollectionMismatchItem()
         {

diff --git a/sources/assets/Stride.Core.Assets.Quantum/AssetPropertyGraph.cs b/sources/assets/Stride.Core.Assets.Quantum/AssetPropertyGraph.cs
@@ -217,14 +217,14 @@ public void ReconcileWithBase()
         private void ReconcileWithBase([NotNull] IAssetNode rootNode, Dictionary<IGraphNode, NodeIndex> nodesToReset = null)
         {
             // Two passes: first pass will reconcile almost everything, but skip object reference.
-            // The reason is that the target of the reference might not exist yet (might need to be reconcilied)
-            var visitor = CreateReconcilierVisitor();
-            visitor.Visiting += (node, path) => ReconcileWithBaseNode((IAssetNode)node, false, nodesToReset);
+            // The reason is that the target of the reference might not exist yet (might need to be reconciled)
+            var visitor = CreateReconcilerVisitor();
+            visitor.Visiting += (node, path) => ReconcileWithBaseNode((IAssetNode)node, path, reconcileObjectReference: false, nodesToReset);
             visitor.Visit(rootNode);
             // Second pass: this one should only reconcile remaining object reference.
             // TODO: these two passes could be improved!
-            visitor = CreateReconcilierVisitor();
-            visitor.Visiting += (node, path) => ReconcileWithBaseNode((IAssetNode)node, true, nodesToReset);
+            visitor = CreateReconcilerVisitor();
+            visitor.Visiting += (node, path) => ReconcileWithBaseNode((IAssetNode)node, path, reconcileObjectReference: true, nodesToReset);
             visitor.Visit(rootNode);
         }
 
@@ -291,7 +291,11 @@ public virtual void ClearReferencesToObjects([NotNull] IEnumerable<Guid> objectI
         /// </summary>
         /// <returns>A new instance of <see cref="GraphVisitorBase"/> for reconciliation.</returns>
         [NotNull]
-        public virtual GraphVisitorBase CreateReconcilierVisitor()
+        [Obsolete($"To be removed in future releases. Use {nameof(CreateReconcilerVisitor)} instead.")]
+        public virtual GraphVisitorBase CreateReconcilierVisitor() => CreateReconcilerVisitor();
+
+        [NotNull]
+        public virtual GraphVisitorBase CreateReconcilerVisitor()
         {
             return new AssetGraphVisitorBase(Definition);
         }
@@ -744,11 +748,11 @@ private void OnBaseContentChanged(INodeChangeEventArgs e, IGraphNode node)
             BaseContentChanged?.Invoke(e, node);
         }
 
-        private void ReconcileWithBaseNode(IAssetNode assetNode, bool reconcileObjectReference, Dictionary<IGraphNode, NodeIndex> nodesToReset)
+        private void ReconcileWithBaseNode(IAssetNode assetNode, GraphNodePath currentPath, bool reconcileObjectReference, Dictionary<IGraphNode, NodeIndex> nodesToReset)
         {
             var memberNode = assetNode as AssetMemberNode;
             var objectNode = assetNode as IAssetObjectNodeInternal;
-            // Non-overridable members should not be reconcilied.
+            // Non-overridable members should not be reconciled.
             if (assetNode?.BaseNode == null || !memberNode?.CanOverride == true)
                 return;
 
@@ -758,7 +762,7 @@ private void ReconcileWithBaseNode(IAssetNode assetNode, bool reconcileObjectRef
             // Reconcile occurs only when the node is not overridden.
             if (memberNode != null)
             {
-                if (ShouldReconcileMember(memberNode, reconcileObjectReference, nodesToReset))
+                if (ShouldReconcileMember(memberNode, currentPath, reconcileObjectReference, nodesToReset))
                 {
                     // If we have no setter, we cannot reconcile this property. Usually it means that the value is already correct (eg. it's an instance of the correct type,
                     // or it's a value that cannot change), so we'll just keep going and try to reconcile the children of this member.
@@ -946,7 +950,7 @@ private void ReconcileWithBaseNode(IAssetNode assetNode, bool reconcileObjectRef
             }
         }
 
-        private bool ShouldReconcileMember([NotNull] IAssetMemberNode memberNode, bool reconcileObjectReference, Dictionary<IGraphNode, NodeIndex> nodesToReset)
+        private bool ShouldReconcileMember([NotNull] IAssetMemberNode memberNode, GraphNodePath currentPath, bool reconcileObjectReference, Dictionary<IGraphNode, NodeIndex> nodesToReset)
         {
             var localValue = memberNode.Retrieve();
             var baseValue = memberNode.BaseNode.Retrieve();
@@ -986,6 +990,16 @@ private bool ShouldReconcileMember([NotNull] IAssetMemberNode memberNode, bool r
                 return localRef?.Id != baseRef?.Id || localRef?.Url != baseRef?.Url;
             }
 
+            // Value type, check if it is a child of a value type that has been overridden
+            foreach (var pathNode in currentPath)
+            {
+                if (pathNode is IAssetMemberNode assetMemberNode && assetMemberNode.IsContentOverridden())
+                {
+                    // We are inside a struct that was overridden, so we are actually considered overridden
+                    return false;
+                }
+            }
+
             // Value type, we check for equality
             return !Equals(localValue, baseValue);
         }

diff --git a/sources/presentation/Stride.Core.Quantum/IGraphNode.cs b/sources/presentation/Stride.Core.Quantum/IGraphNode.cs
@@ -31,7 +31,7 @@ public interface IGraphNode
         ITypeDescriptor Descriptor { get; }
 
         /// <summary>
-        /// Gets wheither this node holds a reference or is a direct value.
+        /// Gets whether this node holds a reference or is a direct value.
         /// </summary>
         bool IsReference { get; }