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.

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

fix: UrlReference triggering quantum change propagation, leading to exceptions with cyclic scene references #2528

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions sources/assets/Stride.Core.Assets/Analysis/BuildAssetNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,10 @@ public override void VisitObject(object obj, ObjectDescriptor descriptor, bool v
{
references.Add(reference);
}
else if (obj is UrlReferenceBase urlRef)
{
references.Add(urlRef);
}
else if (AssetRegistry.IsContentType(obj.GetType()))
{
reference = AttachedReferenceManager.GetAttachedReference(obj);
Expand Down
8 changes: 2 additions & 6 deletions sources/assets/Stride.Core.Assets/AssetRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -290,13 +290,9 @@ public static IReadOnlyList<Type> GetAssetTypes(Type contentType)
lock (RegistryLock)
{
var currentType = contentType;
if (UrlReferenceHelper.IsGenericUrlReferenceType(currentType))
if (UrlReferenceBase.TryGetAssetType(contentType, out var assetType))
{
currentType = UrlReferenceHelper.GetTargetContentType(currentType);
}
else if (UrlReferenceHelper.IsUrlReferenceType(contentType))
{
return GetPublicTypes().Where(t => IsAssetType(t)).ToList();
currentType = assetType;
}
List<Type> assetTypes;
return ContentToAssetTypes.TryGetValue(currentType, out assetTypes) ? new List<Type>(assetTypes) : new List<Type>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ internal class UrlReferenceSerializer : AssetScalarSerializerBase
{
public override bool CanVisit(Type type)
{
return UrlReferenceHelper.IsUrlReferenceType(type);
return UrlReferenceBase.IsUrlReferenceType(type);
}

public override object ConvertFrom(ref ObjectContext context, Scalar fromScalar)
Expand All @@ -26,21 +26,14 @@ public override object ConvertFrom(ref ObjectContext context, Scalar fromScalar)
throw new YamlException(fromScalar.Start, fromScalar.End, "Unable to decode url reference [{0}]. Expecting format GUID:LOCATION".ToFormat(fromScalar.Value));
}

var urlReference = UrlReferenceHelper.CreateReference(context.Descriptor.Type, guid, location.FullPath);

return urlReference;
return UrlReferenceBase.New(context.Descriptor.Type, guid, location.FullPath);
}

public override string ConvertTo(ref ObjectContext objectContext)
{
if (objectContext.Instance is UrlReference urlReference)
if (objectContext.Instance is UrlReferenceBase urlReference)
{
var attachedReference = AttachedReferenceManager.GetAttachedReference(urlReference);

if (attachedReference != null)
{
return $"{attachedReference.Id}:{urlReference.Url}";
}
return $"{urlReference.Id}:{urlReference.Url}";
}

throw new YamlException($"Unable to extract url reference from object [{objectContext.Instance}]");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ public UrlReferenceConverter()
/// <inheritdoc/>
public override bool CanConvertFrom(ITypeDescriptorContext context, Type sourceType)
{
return UrlReferenceHelper.IsUrlReferenceType(TypeConverterHelper.GetDestinationType(context));
return UrlReferenceBase.IsUrlReferenceType(TypeConverterHelper.GetDestinationType(context));
}

/// <inheritdoc/>
public override object ConvertFrom(ITypeDescriptorContext context, CultureInfo culture, object value)
{
var attachedReference = AttachedReferenceManager.GetAttachedReference(value);
var destinationType = TypeConverterHelper.GetDestinationType(context);
return UrlReferenceHelper.CreateReference(destinationType, attachedReference.Id, attachedReference.Url);
return UrlReferenceBase.New(destinationType, attachedReference.Id, attachedReference.Url);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,31 +7,25 @@
namespace Stride.Core.Serialization.Serializers
{
/// <summary>
/// Serializer base class for for <see cref="UrlReference"/>.
/// Serializer base class for <see cref="UrlReference"/>.
/// </summary>
public abstract class UrlReferenceDataSerializerBase<T> : DataSerializer<T>
where T: IUrlReference
where T: UrlReferenceBase, new()
{
/// <inheritdoc/>
public override void Serialize(ref T urlReference, ArchiveMode mode, [NotNull] SerializationStream stream)
{
if (mode == ArchiveMode.Serialize)
{
var attachedReference = AttachedReferenceManager.GetAttachedReference(urlReference);
if(attachedReference == null)
{
throw new InvalidOperationException("UrlReference does not have an AttachedReference.");
}

stream.Write(attachedReference.Id);
stream.Write(attachedReference.Url);
stream.Write(urlReference.Id);
stream.Write(urlReference.Url);
}
else
{
var id = stream.Read<AssetId>();
var url = stream.ReadString();

urlReference = (T)UrlReferenceHelper.CreateReference(typeof(T), id, url);
urlReference = new T { Url = url, Id = id };
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) .NET Foundation and Contributors (https://dotnetfoundation.org/ & https://stride3d.net)
// Distributed under the MIT license. See the LICENSE.md file in the project root for more information.
using System;
using Stride.Core.Assets;
using Stride.Core.Serialization.Serializers;

namespace Stride.Core.Serialization
Expand All @@ -27,6 +28,14 @@ public UrlReference()
public UrlReference(string url) : base(url)
{
}

/// <summary>
/// Create a new <see cref="UrlReference"/> instance.
/// </summary>
/// <exception cref="ArgumentNullException">If <paramref name="url"/> is <c>null</c> or empty.</exception>
public UrlReference(AssetId id, string url) : base(id, url)
{
}
}

/// <summary>
Expand All @@ -53,5 +62,13 @@ public UrlReference()
public UrlReference(string url) : base(url)
{
}

/// <summary>
/// Create a new <see cref="UrlReference"/> instance.
/// </summary>
/// <exception cref="ArgumentNullException">If <paramref name="url"/> is <c>null</c> or empty.</exception>
public UrlReference(AssetId id, string url) : base(id, url)
{
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright (c) .NET Foundation and Contributors (https://dotnetfoundation.org/ & https://stride3d.net)
// Distributed under the MIT license. See the LICENSE.md file in the project root for more information.
using System;
using System.Diagnostics.CodeAnalysis;
using Stride.Core.Assets;
using Stride.Core.Serialization.Contents;

namespace Stride.Core.Serialization
Expand All @@ -10,11 +12,8 @@ namespace Stride.Core.Serialization
/// </summary>
[DataContract("urlref", Inherited = true)]
[DataStyle(DataStyle.Compact)]
[ReferenceSerializer]
public abstract class UrlReferenceBase : IUrlReference
public abstract class UrlReferenceBase : IReference, IUrlReference
{
private string url;

/// <summary>
/// Create a new <see cref="UrlReferenceBase"/> instance.
/// </summary>
Expand All @@ -35,36 +34,70 @@ protected UrlReferenceBase(string url)
throw new ArgumentNullException(nameof(url), $"{nameof(url)} cannot be null or empty.");
}

this.url = url;
this.Url = url;
}

/// <summary>
/// Gets the Url of the referenced asset.
/// Create a new <see cref="UrlReferenceBase"/> instance.
/// </summary>
[DataMember(10)]
public string Url
/// <exception cref="ArgumentNullException">If <paramref name="url"/> is <c>null</c> or empty.</exception>
protected UrlReferenceBase(AssetId id, string url)
{
get => url;
internal set
if (string.IsNullOrWhiteSpace(url))
{
if (string.IsNullOrEmpty(value))
{
throw new ArgumentNullException(nameof(value), $"{nameof(Url)} cannot be null or empty.");
}
url = value;
throw new ArgumentNullException(nameof(url), $"{nameof(url)} cannot be null or empty.");
}

this.Url = url;
this.Id = id;
}

/// <summary>
/// Gets the Url of the referenced asset.
/// </summary>
[DataMember(10)]
public string Url { get; init; }

/// <summary>
/// Gets the Id of the referenced asset.
/// </summary>
[DataMember(20)]
public AssetId Id { get; init; }

/// <summary>
/// Gets whether the url is <c>null</c> or empty.
/// </summary>
[DataMemberIgnore]
public bool IsEmpty => string.IsNullOrEmpty(url);
public bool IsEmpty => string.IsNullOrEmpty(Url);

/// <inheritdoc/>
public override string ToString()
{
return $"{Url}";
}

string IReference.Location => Url;

public static UrlReferenceBase New(Type urlReferenceType, AssetId id, string url)
{
return (UrlReferenceBase)Activator.CreateInstance(urlReferenceType, id, url);
}

public static bool IsUrlReferenceType(Type type)
{
return typeof(UrlReferenceBase).IsAssignableFrom(type);
}

public static bool TryGetAssetType(Type type, [MaybeNullWhen(false)] out Type assetType)
{
if (type.IsAssignableTo(typeof(UrlReferenceBase)) && type.IsGenericType)
{
assetType = type.GetGenericArguments()[0];
return true;
}

assetType = null;
return false;
}
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
using Stride.Engine;
using Stride.Core.Assets.Templates;
using Stride.Core.Packages;
using Stride.Core.Serialization;
using Stride.Engine.Gizmos;
using Stride.Editor.Annotations;
using Stride.Editor.Preview.View;
Expand Down Expand Up @@ -233,6 +234,7 @@ public override void InitializeSession(SessionViewModel session)
public override void RegisterPrimitiveTypes(ICollection<Type> primitiveTypes)
{
primitiveTypes.Add(typeof(AssetReference));
primitiveTypes.Add(typeof(UrlReferenceBase));
}

/// <inheritdoc />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Stride.Core.Reflection;
using Stride.Core.Presentation.Quantum;
using Stride.Core.Presentation.Quantum.Presenters;
using Stride.Core.Serialization;

namespace Stride.Core.Assets.Editor.Quantum.NodePresenters.Commands
{
Expand Down Expand Up @@ -79,6 +80,6 @@ protected override void ExecuteSync(INodePresenter nodePresenter, object paramet

public static bool CanConstruct(Type elementType) => !elementType.IsClass || elementType.GetConstructor(Type.EmptyTypes) != null || elementType == typeof(string);

public static bool IsReferenceType(Type elementType) => AssetRegistry.IsContentType(elementType) || typeof(AssetReference).IsAssignableFrom(elementType);
public static bool IsReferenceType(Type elementType) => AssetRegistry.IsContentType(elementType) || typeof(AssetReference).IsAssignableFrom(elementType) || UrlReferenceBase.IsUrlReferenceType(elementType);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Stride.Core.Extensions;
using Stride.Core.Reflection;
using Stride.Core.Presentation.Quantum.Presenters;
using Stride.Core.Serialization;

namespace Stride.Core.Assets.Editor.Quantum.NodePresenters.Updaters
{
Expand Down Expand Up @@ -69,6 +70,6 @@ protected override void UpdateNode(IAssetNodePresenter node)

private static bool IsInstantiable(Type type) => TypeDescriptorFactory.Default.AttributeRegistry.GetAttribute<NonInstantiableAttribute>(type) == null;

private static bool IsReferenceType(Type type) => AssetRegistry.IsContentType(type) || typeof(AssetReference).IsAssignableFrom(type);
private static bool IsReferenceType(Type type) => AssetRegistry.IsContentType(type) || typeof(AssetReference).IsAssignableFrom(type) || UrlReferenceBase.IsUrlReferenceType(type);
}
}
Loading