-
-
Notifications
You must be signed in to change notification settings - Fork 911
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
create unique IDs for from prefab instantiated entities and components #2068
base: master
Are you sure you want to change the base?
create unique IDs for from prefab instantiated entities and components #2068
Conversation
@@ -30,7 +31,33 @@ public sealed class Prefab | |||
public List<Entity> Instantiate() | |||
{ | |||
var newPrefab = EntityCloner.Clone(this); | |||
CreateNewIds(newPrefab.Entities); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be something for EntityCloner
to do? Seems more like a responsability for it, as its goal is to clone a hierarchy of Entity
s and EntityComponent
s so they can be used afterwards.
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, you don't want to duplicate everything. Otherwise, you are never going to be able to reuse stuff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't mean to duplicate it, but to move it to EntityCloner
, so it can clone an entity tree with or without unique ids.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't talking about code but about cloning itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After a few debugging sessions through the engine, I think it should be fixed in the EntityCloner
. See my comment below.
I'm surprised by this PR because I recall we should already have code that "fixup" IDs in case it is needed. Did you check for any regression elsewhere? For instance, does the same issue happen outside of an Entity processor? If not, then it means it is only an issue related to entity processors and shouldn't be fixed in the On the other hand, if it is a general issue with instantiating prefabs, then we should implement something similar to what is done in the |
There are at least two ways in which prefabs are instantiated, depending on whether it happens in the Game Studio when the hierarchy is changed or at runtime. The Game Studio uses the |
Yep, sounds alright to me as well, tried it out on my end just to validate and instantiation does copy the |
PR Details
After instantiation of a prefab, all entities and components get a new ID.
Closes: #2067
Description
Fixes the crash of the Game Studio described in #2067 caused by ambiguous IDs. This could also fix some other consequential errors happening through the ambiguous IDs.
Related Issue
#2067
Motivation and Context
The Game Studio crashed each time I changed the hierarchy in my scene.
Types of changes
Checklist