stride icon indicating copy to clipboard operation
stride copied to clipboard

Multiple instantiation of prefabs causes ambiguous IDs

Open laske185 opened this issue 2 years ago • 3 comments

Release Type: Official Release

Version: 4.2.0.2042

Platform(s): Windows

Describe the bug When a scene contains two or more entity components with an entity processor that instantiates the same prefab and attaches the instantiated entities to the entity of the component, then the editors crashes when changing the hierarchy.

This is caused because the instantiated entities and components got the same IDs as the corresponding elements in the prefab. When this instantiation happens multiple times, the hierarchy has different elements with the same ID that bring forth errors.

To Reproduce Steps to reproduce the behavior:

  1. Create an entity component
  2. Create an entity processor for that component that instantiates the prefab and attaches the entities to the entity of the component
  3. Assign the component to at least two entities in the scene in the editor
  4. Assign a prefab to the components
  5. Change the hierarchy, f.e. by adding a new empty entity.

Example implementation: Component:

[DefaultEntityComponentProcessor(typeof(MyProcessor))]
[DataContract]
public class MyComponent : EntityComponent
{
    public Prefab Prefab { get;  set; }
}

Processor:

internal class MyProcessor : EntityProcessor<MyComponent, MyProcessor.MyData>
{
    private class MyData
    {
        public required Entity Entity { get; init; }
        public List<Entity> Entities { get; } = [];
    }

    public override void Update(GameTime time)
    {
        base.Update(time);

        foreach (var (component, data) in ComponentDatas)
        {
            if (data.Entities.Count == 0 && component.Prefab != null)
            {
                var entities = component.Prefab.Instantiate();
                foreach (var entity in entities)
                {
                    data.Entity.AddChild(entity);
                }
                data.Entities.AddRange(entities);
            }
        }
    }

    protected override MyData GenerateComponentData([NotNull] Entity entity, [NotNull] MyComponent component) => new()
    {
        Entity = entity
    };
}

Expected behavior All entities and components instantiated by a prefab should get a unique ID.

Log and callstacks

The EntityHierarchyEditorController throws an ArgumentException here:

protected override Dictionary<Guid, IIdentifiable> CollectIdentifiableObjects()
{
    var allEntities = Game.ContentScene.Yield().BreadthFirst(x => x.Children).SelectMany(x => x.Entities).BreadthFirst(x => x.Transform.Children.Select(y => y.Entity));
    var definition = AssetQuantumRegistry.GetDefinition(Asset.Asset.GetType());
    var identifiableObjects = new Dictionary<Guid, IIdentifiable>();
    foreach (var entityNode in allEntities.Select(x => GameSideNodeContainer.GetOrCreateNode(x)))
    {
        foreach (var identifiable in IdentifiableObjectCollector.Collect(definition, entityNode))
        {
            // Throws here: 'An item with the same key has already been added. Key: xxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxx'
            identifiableObjects.Add(identifiable.Key, identifiable.Value);
        }
    }
    return identifiableObjects;
}

laske185 avatar Nov 26 '23 09:11 laske185

Easy to fix: do the fixup in your entity processor.

What is the actual use case of instantiating prefabs from within an entity processor? I don't think it should be done this way. It's not the purpose of an entity processor in my opinion. That should be better done with a script. The purpose of an entity processor is usually to gather associated data (such as sound instance, animation system, etc.), not to instantiate more entities or prefabs. What will happen if the prefab also contains the same custom component: it might create a recursive loop, with new entities being instantiated forever.

Note: I'm not saying there is no bug in this area. But I think more analysis should be done to understand the underlying issue and the consequence of any "fix". For instance, in the editor we have some flags to fixup ids when using the AssetCloner (see sources/assets/Stride.Core.Assets/AssetCloner.cs and sources/assets/Stride.Core.Assets/AssetClonerFlags.cs). The fact that it crashes in the editor indicates it tried to do some instantiation in the editor game which is not a supported behavior.

Kryptos-FR avatar Nov 26 '23 14:11 Kryptos-FR

The use case is that I want to create a generic environment, based on the settings in the EntityComponent, from combined prefabs at runtime.

My only known way to see this generic environment in game studio is to use an EntityProcessor. This is because, unlike ScriptComponents, it is also executed in the game studio.

Or are there other options?

laske185 avatar Nov 29 '23 11:11 laske185

Right now there aren't, no. What you're trying to do is not yet supported, the editor builds a layer on top of the scene hierarchy that abstracts away a couple of things. It doesn't really expect new entities being spawned through user-code. This is something that we should support in the future, likely after the editor rewrite, but right now your example is kind of outside supported behavior. The underlying issue may not be though, if you can replicate this issue at runtime then we should definitely look into that.

Eideren avatar Nov 29 '23 12:11 Eideren