Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Serialization] Fix diverging rules for editor and runtime serialization of fields and properties #1875

Merged
merged 11 commits into from Oct 16, 2023
Merged

[Serialization] Fix diverging rules for editor and runtime serialization of fields and properties #1875

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
99788dd
[Serialization] Fix diverging rules for editor and runtime serializat…
Eideren Oct 1, 2023
2f43d44
Address review
Eideren Oct 1, 2023
e2dce13
Throw instead of ignore and clarify exception
Eideren Oct 1, 2023
6d54e1a
Rollback previous commit, misread the conclusion in PR #1868
Eideren Oct 2, 2023
0cac54c
Fix tests and anonymous type serialization
Eideren Oct 2, 2023
0238489
Finish API coverage from serialization changes
Eideren Oct 2, 2023
dd8cbb9
Change some of the accessor to conform with binary/runtime serializer
Eideren Oct 4, 2023
63185eb
Ignore setter for readonly fields
Eideren Oct 4, 2023
3f025a1
Fix the last few access modifiers
Eideren Oct 5, 2023
794cf27
Address review
Eideren Oct 6, 2023
c364136
Fix outdated comment
Eideren Oct 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions sources/core/Stride.Core.AssemblyProcessor/ComplexSerializerRegistry.cs
View file
Open in desktop
Expand Up @@ -397,6 +397,8 @@ static bool IsAccessibleThroughAccessModifiers(PropertyDefinition property)
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");
Eideren marked this conversation as resolved.
Show resolved Hide resolved

Expand Down
8 changes: 6 additions & 2 deletions sources/core/Stride.Core.Reflection/TypeDescriptors/ObjectDescriptor.cs
View file
Open in desktop
Expand Up @@ -276,6 +276,8 @@ static bool IsAccessibleThroughAccessModifiers(PropertyInfo property)
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 && TryGetBackingField(property, out _);
Eideren marked this conversation as resolved.
Show resolved Hide resolved

Expand Down Expand Up @@ -318,8 +320,10 @@ protected virtual bool PrepareMember(MemberDescriptorBase member, MemberInfo met
member.Mode = memberAttribute.Mode;
if (!member.HasSet)
{
if (memberAttribute.Mode == DataMemberMode.Assign || memberType.IsValueType || memberType == typeof(string))
member.Mode = DataMemberMode.Never;
if (memberAttribute.Mode == DataMemberMode.Assign)
manio143 marked this conversation as resolved.
Show resolved Hide resolved
throw new ArgumentException($"{memberType.FullName} {member.OriginalName} is not writeable by {memberAttribute.Mode.ToString()}, its {nameof(DataMemberMode)} must not be {memberAttribute.Mode}.");
manio143 marked this conversation as resolved.
Show resolved Hide resolved
if (memberType.IsValueType || memberType == typeof(string))
throw new ArgumentException($"{memberType.FullName} {member.OriginalName} is not writeable by {memberAttribute.Mode.ToString()}, {member.OriginalName} must have a setter.");
}
member.Order = memberAttribute.Order;
}
Expand Down
2 changes: 0 additions & 2 deletions sources/engine/Stride.Graphics/Sprite.cs
View file
Open in desktop
Expand Up @@ -149,7 +149,6 @@ public Vector4 Borders
/// <summary>
/// Gets the value indicating if the image has unstretchable borders.
/// </summary>
[DataMember]
public bool HasBorders { get; private set; }
Eideren marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
Expand All @@ -165,7 +164,6 @@ public Vector2 Size
/// Gets the size of the sprite in pixels.
/// Note that the orientation of the image is taken into account in this calculation.
/// </summary>
[DataMember]
public Vector2 SizeInPixels
{
get { return sizeInPixels; }
Expand Down