fix: Instantiate() behavior for Prefab and Entity references by Eideren · Pull Request #2914 · stride3d/stride

Eideren · 2025-09-30T15:50:50Z

PR Details

Prefab instantiation clones the prefab itself and returns only its content, changed it so that prefab clones its list of entities and returns that.
There doesn't seem to be that much of a difference initially, but let's dive deeper:

When cloning an object through EntityCloner, the engine, for a given property it will do one of two things

Either create a whole new object based on the source object of that property
or just pass the reference of the source to the clone.
It does so depending on the type of the object and some other misc logic.

For objects that are fully cloned, like our Prefab, the system keeps track of the newly created object to swap any other references it finds in component properties that point to the source object by our newly created clone of that object.

Meaning that if the prefab contains a component that has a prefab property pointing to that same prefab, after cloning, the property would now point to the newly created clone of the prefab instead of the original prefab.
This may seem like expected behavior, until you consider the fact that Instantiate() does not return a new prefab, but a list of entities instead.

Entity References

The other part of this PR is to ensure that references in components to Entity that lay outside the hierarchy are not cloned. Instantiation is about cloning a hierarchy under an entity, not every entity ever referenced in the components of said entity.

I would have liked to include EntityComponent references too, but I couldn't fit them in yet, it requires a larger amount of change I'm not as confident wouldn't break users game, but at least this time around it will throw an exception in that case instead of duplicating the reference.
The reason it's not as simple is because EntityComponent do not have a specific serializer, we would need one that derive from DataContentSerializerWithReuse, the serializer for TransformComponent derives from a simple DataSerializer for example.

Also refactored a test which used a goto for no good reason. Don't get me wrong, I like gotos, but using it here just doesn't make sense.

Related Issue

Couldn't find any.

Types of changes

Docs change / refactoring / dependency upgrade
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist

My change requires a change to the documentation.
I have added tests to cover my changes.
All new and existing tests passed.
I have built and run the editor to try this change out.

Eideren · 2025-10-14T16:44:51Z

Another nasty issue I just found which would be fixed by this PR, when your prefabs contain a circular reference like in the following case:

Prefab#0:
	Entity#0:
		Component#0:
			MyPrefab: ref to Prefab#0

Instantiating this prefab returns the following hierarchy

Entity#1:
	Component#1:
		MyPrefab: ref to Prefab#1

Prefab#1 is a new instance of the same prefab like outlined in the main post - and its content is ...

Prefab#1:
	Entity#1:
		...

Surprise ! The very same entity that was just instantiated and is now part of the scene !
Meaning that if you now instantiate Prefab#1 you are actually cloning the object that is part of the scene, with its component and properties altered.
Its state will differ compared to spawning Prefab#0 even though they both came from the same prefab.

VaclavElias · 2025-10-14T21:29:44Z

How do you even find these issues? 🙂

Eideren · 2025-10-15T06:02:49Z

That's my secret ;)

Kryptos-FR · 2025-10-15T19:29:14Z

I'll have to check manually for that one. Reading from my phone doesn't compute in my head.

Just to be sure, this doesn't break nested prefabs and archetypes (at design time)? Or copy/pasting a prefab in the asset explorer?

Eideren · 2025-10-15T19:38:24Z

@Kryptos-FR Have a look at the new test I setup, might make more sense to read through that than my nonsense :)

Eideren · 2025-10-19T15:53:25Z

this doesn't break nested prefabs and archetypes (at design time)? Or copy/pasting a prefab in the asset explorer?

Forgot to answer, nope, it does not :P

Do you have time to take a quick look at this one ?

Kryptos-FR · 2025-10-19T17:58:52Z

I'll try early next week, it's late today.

Kryptos-FR

Looks ok to me.

However I discovered a regression in the editor regarding copy/pasting nested prefabs. That regression doesn't seem related to this fix (I could reproduce it with earlier version), but it didn't occur in Stride 4.0. I'll investigate more to find where the regression was introduced and try to apply your fix to a version without the regression to figure out if there are other potential regressions hidden by that one.

Kryptos-FR · 2025-10-22T09:51:22Z

sources/engine/Stride.Engine/Engine/Design/EntityCloner.cs

        /// <param name="prefab">The prefab to clone.</param>
        /// <returns>A cloned prefab</returns>
-        public static Prefab Clone(Prefab prefab)
+        public static List<Entity> Instantiate(Prefab prefab)


Maybe update the documentation since the method name has changed.

Kryptos-FR · 2025-10-22T09:55:41Z

sources/engine/Stride.Engine/Engine/Design/EntityCloner.cs

                cloneSerializerSelector = new SerializerSelector(true, false, "Default", "Clone");
            }

+            if (entitySerializerSelector == null)


nit: simplify with the ??= operator.

Kryptos-FR · 2025-10-22T09:59:46Z

sources/engine/Stride.Engine.Tests/TestEntity.cs

+    {
+        public required Prefab Prefab { get; set; }
+        public required Entity ExternalEntityRef { get; set; }
+        /*public required TransformComponent ExternalComponentRef { get; set; }*/


Add a comment around the commented out code to explain why it is kept around (maybe reference the issue/PR id as well).

Eideren · 2025-10-22T13:12:09Z

Took care of the review - I think we can merge this fix regardless of the regression you mentioned ? If this specific change ends up compromising that regression we can always modify it through the PR for that regression ?

Kryptos-FR · 2025-10-22T13:46:09Z

Yes, it looks the bug is related to copy/paste processors and unlikely to be affected by this change.

Kryptos-FR · 2025-10-22T13:48:29Z

sources/engine/Stride.Engine/Engine/Design/EntityCloner.cs

        /// <summary>
-        /// Clones the specified prefab.
-        /// <see cref="Entity"/>, children <see cref="Entity"/> and their <see cref="EntityComponent"/> will be cloned.
+        /// Instantiate the content of the prefab provided.


nit: Instantiate**s** method comments are in third-person by convention

Kryptos-FR

Aside from the latest nit-picking, looks good to me.

Eideren · 2025-10-22T14:18:51Z

Done, thanks for the review @Kryptos-FR !

commit aaae940 Merge: ee81d20 325a7fa Author: xen2 <virgile@stride3d.net> Date: Thu Oct 23 18:22:23 2025 +0900 Merge pull request stride3d#1664 from vvvv/dev/joreg/add-touch-to-winforms Adds touch support to Winforms based GameForm commit ee81d20 Author: Eideren <contact@eideren.com> Date: Wed Oct 22 16:18:35 2025 +0200 fix: Instantiate() behavior for Prefab and Entity references (stride3d#2914) commit 325a7fa Merge: 77e2101 82f7fae Author: joreg <joreg@vvvv.org> Date: Wed Oct 22 11:13:44 2025 +0200 Merge branch 'dev/joreg/add-touch-to-winforms' of https://github.com/vvvv/stride into dev/joreg/add-touch-to-winforms commit 77e2101 Author: joreg <joreg@vvvv.org> Date: Wed Oct 22 11:13:35 2025 +0200 fixed potentially problematic cast commit 82f7fae Author: joreg <joreg@vvvv.org> Date: Wed Oct 22 11:12:19 2025 +0200 Update sources/engine/Stride.Games/Desktop/GameForm.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> commit 1956f70 Author: joreg <joreg@vvvv.org> Date: Tue Oct 21 14:37:15 2025 +0200 Update sources/engine/Stride.Input/Windows/PointerWinforms.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> commit 2c1bc62 Author: joreg <joreg@vvvv.org> Date: Tue Oct 14 15:22:12 2025 +0200 Touch position is now normalized 0..1 like in other implementations commit 13d11c8 Merge: c73271b b74a54d Author: joreg <joreg@vvvv.org> Date: Tue Oct 14 14:05:36 2025 +0200 Merge branch 'master' into dev/joreg/add-touch-to-winforms commit c73271b Author: joreg <joreg@vvvv.org> Date: Thu May 18 13:03:58 2023 +0200 PointerSDL now uses GetFingerId() from baseclass (to be in line with Winforms implementation) commit 4ab9f70 Author: joreg <joreg@vvvv.org> Date: Thu May 18 13:03:10 2023 +0200 add touch support for Winforms based GameForm

…d#2914) (cherry picked from commit ee81d20)

(cherry picked from commit ee81d20)

Kryptos-FR · 2025-10-26T14:21:06Z

I have created #2953 to track the regression I mentioned earlier (which is not caused by this PR).

fix: Instantiate() behavior for Prefab and Entity references

ddba122

Eideren added the area-Serialization label Sep 30, 2025

Kryptos-FR self-requested a review October 15, 2025 19:36

Kryptos-FR reviewed Oct 22, 2025

View reviewed changes

Eideren added 3 commits October 22, 2025 14:59

Update EntityCloner.Instantiate docs

8cf1b65

simplify null checkand assign with ??=

1d250a8

Document commented out test

ab42649

Kryptos-FR reviewed Oct 22, 2025

View reviewed changes

Kryptos-FR approved these changes Oct 22, 2025

View reviewed changes

Address latest review

000eb75

Eideren merged commit ee81d20 into stride3d:master Oct 22, 2025
2 checks passed

Eideren deleted the instantiate_fix branch October 22, 2025 14:18

tebjan pushed a commit to vvvv/stride that referenced this pull request Oct 24, 2025

fix: Instantiate() behavior for Prefab and Entity references (stride3…

1709bb7

…d#2914) (cherry picked from commit ee81d20)

tebjan pushed a commit that referenced this pull request Oct 24, 2025

fix: Instantiate() behavior for Prefab and Entity references (#2914)

7cfd7f2

(cherry picked from commit ee81d20)

Uh oh!

Conversation

Eideren commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Details

Entity References

Related Issue

Types of changes

Checklist

Uh oh!

Eideren commented Oct 14, 2025

Uh oh!

VaclavElias commented Oct 14, 2025

Uh oh!

Eideren commented Oct 15, 2025

Uh oh!

Kryptos-FR commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Eideren commented Oct 15, 2025

Uh oh!

Eideren commented Oct 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kryptos-FR commented Oct 19, 2025

Uh oh!

Kryptos-FR left a comment

Choose a reason for hiding this comment

Uh oh!

Kryptos-FR Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

Kryptos-FR Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

Kryptos-FR Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

Eideren commented Oct 22, 2025

Uh oh!

Kryptos-FR commented Oct 22, 2025

Uh oh!

Kryptos-FR Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kryptos-FR left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Eideren commented Oct 22, 2025

Uh oh!

Kryptos-FR commented Oct 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Eideren commented Sep 30, 2025 •

edited

Loading

Kryptos-FR commented Oct 15, 2025 •

edited

Loading

Eideren commented Oct 19, 2025 •

edited

Loading

Kryptos-FR Oct 22, 2025 •

edited

Loading