stride3d
create unique IDs for from prefab instantiated entities and components
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
- [ ] Docs change / refactoring / dependency upgrade
- [x] 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.
- [x] I have added tests to cover my changes.
- [x] All new and existing tests passed.
- [x] I have built and run the editor to try this change out.
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 Instantiate() method. This would otherwise impacts other parts of the engine, which have not been tested for regression here.
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 AssetCloner class (see sources/assets/Stride.Core.Assets/AssetCloner.cs and sources/assets/Stride.Core.Assets/AssetClonerFlags.cs).
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 AssetCloner for instantiation, as the prefabs are held here as a
PrefabAsset.
When instantiating a prefab or cloning entities at runtime, however, the EntityCloner is used. The EntityCloner is only used within the engine for the public functions for instantiating prefabs and cloning entities. These themselves are not used within the engine.
Cloning is implemented through serialization. In contrast to the EntityCloner, the AssetCloner generates new IDs during deserialization. It is therefore independent of the EntityProcessor. The problem with the ambiguous IDs always occurs when a prefab is instantiated or an entity is cloned at runtime. The only question is whether or when subsequent errors occur.
From my standpoint, it would also make sense if the EntityCloner, like the AssetCloner, generates a new ID when deserializing an entity or component. I would then change the implementation analogous to the AssetCloner.
Yep, sounds alright to me as well, tried it out on my end just to validate and instantiation does copy the Id from the source entity instead of replacing it with another one.
