From ddba122b3017aa2cadb7c493f0ce083bf2d13930 Mon Sep 17 00:00:00 2001 From: Eideren Date: Tue, 30 Sep 2025 17:23:30 +0200 Subject: [PATCH] fix: Instantiate() behavior for Prefab and Entity references --- .../engine/Stride.Engine.Tests/TestEntity.cs | 71 +++++++++++++------ .../Engine/Design/CloneSerializer.cs | 10 ++- .../Engine/Design/EntityCloner.cs | 15 +++- sources/engine/Stride.Engine/Engine/Prefab.cs | 3 +- 4 files changed, 70 insertions(+), 29 deletions(-) diff --git a/sources/engine/Stride.Engine.Tests/TestEntity.cs b/sources/engine/Stride.Engine.Tests/TestEntity.cs index cb6df52180..2e36ab795f 100644 --- a/sources/engine/Stride.Engine.Tests/TestEntity.cs +++ b/sources/engine/Stride.Engine.Tests/TestEntity.cs @@ -6,6 +6,7 @@ using System.Linq; using Xunit; using Stride.Core; +using Stride.Core.Annotations; using Stride.Engine.Design; using Stride.Rendering; @@ -129,8 +130,6 @@ public void TestMultipleComponents() [Fact] public void TestEntityAndPrefabClone() { - Prefab prefab = null; - var entity = new Entity("Parent"); var childEntity = new Entity("Child"); entity.AddChild(childEntity); @@ -141,17 +140,25 @@ public void TestEntityAndPrefabClone() var newEntity = entity.Clone(); - // NOTE: THE CODE AFTER THIS IS EXECUTED TWO TIMES - // 1st time: newEntity = entity.Clone(); - // 2nd time: newEntity = prefab.Instantiate()[0]; - check_new_Entity: + CheckEntity(newEntity); + + // Check prefab cloning + var prefab = new Prefab(); + prefab.Entities.Add(entity); + var newEntities = prefab.Instantiate(); + Assert.Single(newEntities); + + CheckEntity(newEntities[0]); + return; + + void CheckEntity([NotNull] Entity e) { - Assert.Single(newEntity.Transform.Children); - var newChildEntity = newEntity.Transform.Children[0].Entity; + Assert.Single(e.Transform.Children); + var newChildEntity = e.Transform.Children[0].Entity; Assert.Equal("Child", newChildEntity.Name); - Assert.NotNull(newEntity.Get()); - var newCustom = newEntity.Get(); + Assert.NotNull(e.Get()); + var newCustom = e.Get(); // Make sure that the old component and the new component are different Assert.NotEqual(custom, newCustom); @@ -162,19 +169,31 @@ public void TestEntityAndPrefabClone() // Verify that objects references outside the Entity/Component hierarchy are not cloned (shared) Assert.Equal(custom.CustomObject, newCustom.CustomObject); } + } - // Woot, ugly test using a goto, avoid factorizing code in a delegate method, ugly but effective, goto FTW - if (prefab == null) - { - // Check prefab cloning - prefab = new Prefab(); - prefab.Entities.Add(entity); - var newEntities = prefab.Instantiate(); - Assert.Single(newEntities); - - newEntity = newEntities[0]; - goto check_new_Entity; - } + [Fact] + public void TestCloningBehavior() + { + var externalEntity = new Entity(); + var sourceEntity = new Entity(); + var sourceComponent = new EntityComponentWithPrefab { Prefab = new Prefab(), ExternalEntityRef = externalEntity/*, ExternalComponentRef = externalEntity.Transform*/ }; + sourceComponent.Prefab.Entities.Add(sourceEntity); + sourceEntity.Add(sourceComponent); + + var clonedComponent = sourceComponent.Prefab.Instantiate()[0].Get(); + + // Validate that cloning did clone the entity + Assert.NotEqual(clonedComponent.Entity, sourceComponent.Entity); + + // References to prefabs should not be deep cloned as they are content types + Assert.Equal(clonedComponent.Prefab, sourceComponent.Prefab); + + // References to entities outside this one's hierarchy should not clone the entity referenced, it should point to the same reference + Assert.Equal(clonedComponent.ExternalEntityRef, sourceComponent.ExternalEntityRef); + + // References to entity component outside this one's hierarchy should not clone the component referenced, it should point to the same reference + // Not yet supported + /*Assert.Equal(clonedComponent.ExternalComponentRef, sourceComponent.ExternalComponentRef);*/ } private class DelegateEntityComponentNotify : EntityManager @@ -270,4 +289,12 @@ public abstract class CustomEntityComponentBase : EntityComponent { public Action Changed; } + + [DataContract] + public class EntityComponentWithPrefab : EntityComponent + { + public required Prefab Prefab { get; set; } + public required Entity ExternalEntityRef { get; set; } + /*public required TransformComponent ExternalComponentRef { get; set; }*/ + } } diff --git a/sources/engine/Stride.Engine/Engine/Design/CloneSerializer.cs b/sources/engine/Stride.Engine/Engine/Design/CloneSerializer.cs index b296160f4a..ded97498e1 100644 --- a/sources/engine/Stride.Engine/Engine/Design/CloneSerializer.cs +++ b/sources/engine/Stride.Engine/Engine/Design/CloneSerializer.cs @@ -37,6 +37,9 @@ public override void PreSerialize(ref T obj, ArchiveMode mode, SerializationStre } else { + var dataSerializer = cloneContext.EntitySerializerSelector.GetSerializer(); + if (dataSerializer != this) + dataSerializer.PreSerialize(ref obj, mode, stream); cloneContext.SerializedObjects.Add(obj); } } @@ -58,7 +61,9 @@ public override void PreSerialize(ref T obj, ArchiveMode mode, SerializationStre } else { - base.PreSerialize(ref obj, mode, stream); + var dataSerializer = cloneContext.EntitySerializerSelector.GetSerializer(); + if (dataSerializer != this) + dataSerializer.PreSerialize(ref obj, mode, stream); cloneContext.SerializedObjects.Add(obj); } } @@ -75,7 +80,8 @@ public override void Serialize(ref T obj, ArchiveMode mode, SerializationStream // Serialize object //stream.Context.Set(EntitySerializer.InsideEntityComponentProperty, false); - dataSerializer.Serialize(ref obj, mode, stream); + if (dataSerializer != this) + dataSerializer.Serialize(ref obj, mode, stream); if (obj is EntityComponent) { diff --git a/sources/engine/Stride.Engine/Engine/Design/EntityCloner.cs b/sources/engine/Stride.Engine/Engine/Design/EntityCloner.cs index 80029e241b..c3064e7eba 100644 --- a/sources/engine/Stride.Engine/Engine/Design/EntityCloner.cs +++ b/sources/engine/Stride.Engine/Engine/Design/EntityCloner.cs @@ -29,10 +29,13 @@ namespace Stride.Engine.Design [DataSerializerGlobal(typeof(CloneSerializer), Profile = "Clone")] [DataSerializerGlobal(typeof(CloneSerializer), Profile = "Clone")] [DataSerializerGlobal(typeof(CloneSerializer), Profile = "Clone")] + [DataSerializerGlobal(typeof(CloneSerializer), Profile = "Clone")] + [DataSerializerGlobal(typeof(CloneSerializer), Profile = "Clone")] public class EntityCloner { private static readonly CloneContext cloneContext = new CloneContext(); private static SerializerSelector cloneSerializerSelector = null; + private static SerializerSelector entitySerializerSelector = null; public static readonly PropertyKey CloneContextProperty = new PropertyKey("CloneContext", typeof(EntityCloner)); // CloneObject TLS used to clone entities, so that we don't create one everytime we clone @@ -51,7 +54,7 @@ private static HashSet ClonedObjects() /// /// The prefab to clone. /// A cloned prefab - public static Prefab Clone(Prefab prefab) + public static List Instantiate(Prefab prefab) { if (prefab == null) throw new ArgumentNullException(nameof(prefab)); var clonedObjects = ClonedObjects(); @@ -61,7 +64,8 @@ public static Prefab Clone(Prefab prefab) { CollectEntityTreeHelper(entity, clonedObjects); } - return Clone(clonedObjects, null, prefab); + + return Clone(clonedObjects, null, prefab.Entities); } finally { @@ -132,12 +136,17 @@ private static T Clone(HashSet clonedObjects, TryGetValueFunctionA collection of entities extracted from the prefab public List Instantiate() { - var newPrefab = EntityCloner.Clone(this); - return newPrefab.Entities; + return EntityCloner.Instantiate(this); } } }