Skip to content

Commit

Permalink
[Serialization] Fix diverging rules for editor and runtime serializat…
Browse files Browse the repository at this point in the history
…ion of fields and properties (#1875)
  • Loading branch information
Eideren committed Oct 16, 2023
1 parent 0bc67fc commit b9f19bb
Show file tree
Hide file tree
Showing 45 changed files with 237 additions and 292 deletions.
Git LFS file not shown
@@ -1 +1 @@
52F819550E98D126A685C556595CB25A66BFC7CD97352FA5D127B4A759B52FDB
F5307F2607FE9E761088E06BACCDCB5ECE8739B25418A6B19421004D872C343D
Git LFS file not shown
Git LFS file not shown
Git LFS file not shown
Git LFS file not shown
@@ -1 +1 @@
E17AB0CB8BC5F55CA985C21612E1DA0AF77EC6EA6CDD6FAF9F76252B0217D226
F47741092523EC3BF4C9C1B87BEC8E1FAECC919E1F8319CE64A696F876E186CB
Git LFS file not shown
Git LFS file not shown
Git LFS file not shown
Expand Up @@ -28,7 +28,7 @@ public MyAsset()
// TODO: Re-enable non-pure collections here once we support them for serialization!
//MapItems2 = new MyDictionary();
MapItems3 = new MyDictionaryPure();
CustomObjectWithProtectedSet = new CustomObject { Name = "customObject" };
CustomObjectWithInternalSet = new CustomObject { Name = "customObject" };
}

[DataMember(0)]
Expand Down Expand Up @@ -65,7 +65,8 @@ public MyAsset()

public MyDictionaryPure MapItems3 { get; set; }

public object CustomObjectWithProtectedSet { get; protected set; }
[DataMember]
public CustomObject CustomObjectWithInternalSet { get; internal set; }
}

[DataContract("CustomObject")]
Expand Down
Expand Up @@ -27,5 +27,5 @@ MapItems3:
11000000110000001100000011000000~key2: 2
12000000120000001200000012000000~key3: 3
13000000130000001300000013000000~key4: 3
CustomObjectWithProtectedSet:
CustomObjectWithInternalSet:
Name: customObject
5 changes: 5 additions & 0 deletions sources/assets/Stride.Core.Assets/AssetItem.cs
Expand Up @@ -59,6 +59,7 @@ internal AssetItem([NotNull] UFile location, [NotNull] Asset asset, Package pack
/// </summary>
/// <value>The location.</value>
[NotNull]
[DataMember]
public UFile Location { get => location; internal set => location = value ?? throw new ArgumentNullException(nameof(value)); }

/// <summary>
Expand All @@ -84,6 +85,7 @@ internal AssetItem([NotNull] UFile location, [NotNull] Asset asset, Package pack
/// Gets the package where this asset is stored.
/// </summary>
/// <value>The package.</value>
[DataMember]
public Package Package { get; internal set; }

/// <summary>
Expand Down Expand Up @@ -187,6 +189,7 @@ public UFile FullPath
/// </summary>
/// <value>The asset.</value>
[NotNull]
[DataMember]
public Asset Asset { get => asset; internal set => asset = value ?? throw new ArgumentNullException(nameof(value)); }

/// <summary>
Expand All @@ -197,13 +200,15 @@ public UFile FullPath
/// By default, contains the last modified time of the asset from the disk. If IsDirty is also updated from false to true
/// , this time will get current time of modification.
/// </remarks>
[DataMember]
public DateTime ModifiedTime { get; internal set; }

private long version;

/// <summary>
/// Gets the asset version incremental counter, increased everytime the asset is edited.
/// </summary>
[DataMember]
public long Version { get => Interlocked.Read(ref version); internal set => Interlocked.Exchange(ref version, value); }

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions sources/assets/Stride.Core.Assets/AssetReference.cs
Expand Up @@ -33,14 +33,14 @@ public AssetReference(AssetId id, UFile location)
/// </summary>
/// <value>The unique identifier of the reference asset..</value>
[DataMember(10)]
public AssetId Id { get; }
public AssetId Id { get; init; }

/// <summary>
/// Gets or sets the location of the asset.
/// </summary>
/// <value>The location.</value>
[DataMember(20)]
public string Location { get; }
public string Location { get; init; }

public bool Equals(AssetReference other)
{
Expand Down
4 changes: 2 additions & 2 deletions sources/assets/Stride.Core.Assets/BasePart.cs
Expand Up @@ -72,10 +72,10 @@ public BasePart([NotNull] AssetReference basePartAsset, Guid basePartId, Guid in
public AssetReference BasePartAsset { get; set; }

[DataMember(20)]
public Guid BasePartId { get; }
public Guid BasePartId { get; init; }

[DataMember(30)]
public Guid InstanceId { get; }
public Guid InstanceId { get; init; }

[CanBeNull]
public IIdentifiable ResolvePart(PackageSession session)
Expand Down
13 changes: 2 additions & 11 deletions sources/assets/Stride.Core.Assets/Bundle.cs
Expand Up @@ -14,15 +14,6 @@ namespace Stride.Core.Assets
[DebuggerDisplay("Bundle [{Name}] Selectors[{Selectors.Count}] Dependencies[{Dependencies.Count}]")]
public sealed class Bundle
{
/// <summary>
/// Initializes a new instance of the <see cref="Bundle"/> class.
/// </summary>
public Bundle()
{
Selectors = new List<AssetSelector>();
Dependencies = new List<string>();
}

/// <summary>
/// Gets or sets the name of this bundle.
/// </summary>
Expand All @@ -33,13 +24,13 @@ public Bundle()
/// Gets the selectors used by this bundle.
/// </summary>
/// <value>The selectors.</value>
public List<AssetSelector> Selectors { get; private set; }
public List<AssetSelector> Selectors { get; } = new List<AssetSelector>();

/// <summary>
/// Gets the bundle dependencies.
/// </summary>
/// <value>The dependencies.</value>
public List<string> Dependencies { get; private set; }
public List<string> Dependencies { get; } = new List<string>();

/// <summary>
/// Gets the output group (used in conjonction with <see cref="ProjectBuildProfile.OutputGroupDirectories"/> to control where file will be put).
Expand Down
10 changes: 1 addition & 9 deletions sources/assets/Stride.Core.Assets/Selectors/TagSelector.cs
Expand Up @@ -13,19 +13,11 @@ namespace Stride.Core.Assets.Selectors
[DataContract("TagSelector")]
public class TagSelector : AssetSelector
{
/// <summary>
/// Initializes a new instance of the <see cref="TagSelector"/> class.
/// </summary>
public TagSelector()
{
Tags = new TagCollection();
}

/// <summary>
/// Gets the tags that will be used to select an asset.
/// </summary>
/// <value>The tags.</value>
public TagCollection Tags { get; private set; }
public TagCollection Tags { get; } = new TagCollection();

public override IEnumerable<string> Select(PackageSession packageSession, IContentIndexMap contentIndexMap)
{
Expand Down
20 changes: 5 additions & 15 deletions sources/assets/Stride.Core.Assets/SolutionPlatform.cs
Expand Up @@ -16,21 +16,12 @@ namespace Stride.Core.Assets
[DataContract("SolutionPlatform")]
public class SolutionPlatform : SolutionPlatformPart
{
/// <summary>
/// Initializes a new instance of the <see cref="SolutionPlatform"/> class.
/// </summary>
public SolutionPlatform()
{
PlatformsPart = new SolutionPlatformPartCollection();
DefineConstants = new List<string>();
}

/// <summary>
/// Gets the alternative names that will appear in the .sln file equivalent to this platform.
/// </summary>
/// <value>The alternative names.</value>
[DataMember(20)]
public SolutionPlatformPartCollection PlatformsPart { get; private set; }
public SolutionPlatformPartCollection PlatformsPart { get; } = new SolutionPlatformPartCollection();

/// <summary>
/// Gets or sets the type of the platform.
Expand Down Expand Up @@ -58,7 +49,7 @@ public SolutionPlatform()
/// </summary>
/// <value>The define constants.</value>
[DataMember(40)]
public List<string> DefineConstants { get; private set; }
public List<string> DefineConstants { get; } = new List<string>();

/// <summary>
/// Gets or sets a value indicating whether this <see cref="SolutionPlatform"/> is available on this machine.
Expand Down Expand Up @@ -274,16 +265,15 @@ public class SolutionConfiguration
/// </summary>
public SolutionConfiguration(string name)
{
if (name == null) throw new ArgumentNullException("name");
if (name == null) throw new ArgumentNullException(nameof(name));
Name = name;
Properties = new List<string>();
}

/// <summary>
/// Gets or sets the configuration name (e.g. Debug, Release)
/// </summary>
/// <value>The name.</value>
public string Name { get; private set; }
public string Name { get; init; }

/// <summary>
/// Gets or sets a value indicating whether this instance is a debug configuration.
Expand All @@ -295,6 +285,6 @@ public SolutionConfiguration(string name)
/// Gets the additional msbuild properties for a specific configuration (Debug or Release)
/// </summary>
/// <value>The msbuild configuration properties.</value>
public List<string> Properties { get; private set; }
public List<string> Properties { get; } = new List<string>();
}
}
1 change: 1 addition & 0 deletions sources/assets/Stride.Core.Assets/Yaml/YamlAssetPath.cs
Expand Up @@ -127,6 +127,7 @@ public YamlAssetPath([NotNull] IEnumerable<Element> elements)
/// <summary>
/// The elements constituting this path.
/// </summary>
[DataMember]
public IReadOnlyList<Element> Elements => elements;

/// <summary>
Expand Down
Expand Up @@ -10,7 +10,6 @@
using System.Runtime.CompilerServices;
using System.Text;
using Mono.Cecil;
using Stride.Core;

namespace Stride.Core.AssemblyProcessor
{
Expand Down Expand Up @@ -282,25 +281,33 @@ public static IEnumerable<SerializableItem> GetSerializableItems(TypeDefinition
var fields = new List<FieldDefinition>();
var properties = new List<PropertyDefinition>();

var fieldEnum = type.Fields.Where(x => (x.IsPublic || (x.IsAssembly && x.CustomAttributes.Any(a => a.AttributeType.FullName == "Stride.Core.DataMemberAttribute"))) && !x.IsStatic && !ignoredMembers.Contains(x));
foreach (var field in type.Fields)
{
if (field.IsStatic)
continue;

// If there is a explicit or sequential layout, sort by offset
if (type.IsSequentialLayout || type.IsExplicitLayout)
fieldEnum = fieldEnum.OrderBy(x => x.Offset);
if (ignoredMembers.Contains(field))
continue;

foreach (var field in fieldEnum)
{
fields.Add(field);
if (field.IsPublic || (field.IsAssembly && field.CustomAttributes.Any(a => a.AttributeType.FullName == "Stride.Core.DataMemberAttribute")))
fields.Add(field);
}

// If there is a explicit or sequential layout, sort by offset
if (type.IsSequentialLayout || type.IsExplicitLayout)
fields.Sort((x,y) => x.Offset.CompareTo(y.Offset));

foreach (var property in type.Properties)
{
if (IsAccessibleThroughAccessModifiers(property) == false)
continue;

// Need a non-static public get method
if (property.GetMethod == null || !property.GetMethod.IsPublic || property.GetMethod.IsStatic)
if (property.GetMethod.IsStatic)
continue;

// If it's a struct (!IsValueType), we need a public set method as well
if (property.PropertyType.IsValueType && (property.SetMethod == null || !(property.SetMethod.IsAssembly || property.SetMethod.IsPublic)))
// If it's a struct (!IsValueType), we need a set method as well
if (property.PropertyType.IsValueType && property.SetMethod == null)
continue;

// Only take virtual properties (override ones will be handled by parent serializers)
Expand Down Expand Up @@ -382,6 +389,27 @@ public static IEnumerable<SerializableItem> GetSerializableItems(TypeDefinition
}
}

static bool IsAccessibleThroughAccessModifiers(PropertyDefinition property)
{
var get = property.GetMethod;
var set = property.SetMethod;

if (get == null)
return false;

bool forced = property.CustomAttributes.Any(a => a.AttributeType.FullName == "Stride.Core.DataMemberAttribute");

if (forced && (get.IsPublic || get.IsAssembly))
return true;

// By default, allow access for get-only auto-property, i.e.: { get; } but not { get => }
// as the later may create side effects, and without a setter, we can't 'set' as a fallback for those exceptional cases.
if (get.IsPublic)
return set?.IsPublic == true || set == null && property.DeclaringType.Fields.Any(x => x.Name == $"<{property.Name}>k__BackingField");

return false;
}

internal static bool IsMemberIgnored(ICollection<CustomAttribute> customAttributes, ComplexTypeSerializerFlags flags, DataMemberMode dataMemberMode)
{
// Check for DataMemberIgnore
Expand Down
2 changes: 1 addition & 1 deletion sources/core/Stride.Core.Design/Settings/SettingsFile.cs
Expand Up @@ -23,6 +23,6 @@ public SettingsFile(SettingsProfile profile)
/// Gets the settings profile to serialize.
/// </summary>
[DataMemberCustomSerializer]
public SettingsProfile Settings { get; private set; }
public SettingsProfile Settings { get; init; }
}
}
Expand Up @@ -30,7 +30,7 @@ public FieldDescriptor(ITypeDescriptor typeDescriptor, FieldInfo fieldInfo, Stri

public override bool IsPublic => FieldInfo.IsPublic;

public override bool HasSet => true;
public override bool HasSet => !FieldInfo.IsInitOnly;

public override object Get(object thisObject)
{
Expand Down
Expand Up @@ -21,11 +21,9 @@ public PropertyDescriptor(ITypeDescriptor typeDescriptor, PropertyInfo propertyI

PropertyInfo = propertyInfo;

getMethod = propertyInfo.GetGetMethod(false) ?? propertyInfo.GetGetMethod(true);
if (propertyInfo.CanWrite && propertyInfo.GetSetMethod(!IsPublic) != null)
{
setMethod = propertyInfo.GetSetMethod(!IsPublic);
}
getMethod = propertyInfo.GetMethod;
setMethod = propertyInfo.SetMethod;

TypeDescriptor = typeDescriptor;
}

Expand Down

0 comments on commit b9f19bb

Please sign in to comment.