Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Multiple instantiation of prefabs causes ambiguous IDs #2067

Open
laske185 opened this issue Nov 26, 2023 · 3 comments · May be fixed by #2068
Open

Multiple instantiation of prefabs causes ambiguous IDs #2067

laske185 opened this issue Nov 26, 2023 · 3 comments · May be fixed by #2068
Labels
bug Something isn't working

Comments

@laske185
Copy link

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 laske185 added the bug Something isn't working label Nov 26, 2023
laske185 added a commit to laske185/stride that referenced this issue Nov 26, 2023
@laske185
fix: create unique IDs for from prefab instantiated entities and comp…
267e462 
…onents

Closes: stride3d#2067
laske185 added a commit to laske185/stride that referenced this issue Nov 26, 2023
@laske185
fix: create unique IDs for from prefab instantiated entities and comp…
04a40b4 
…onents

Closes: stride3d#2067
@laske185 laske185 linked a pull request Nov 26, 2023 that will close this issue
Open
8 tasks
@Kryptos-FR
Copy link
Member

Kryptos-FR commented Nov 26, 2023 •
edited

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.

@laske185
Copy link
Author

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?

@Eideren
Copy link
Collaborator

Eideren commented Nov 29, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

create unique IDs for from prefab instantiated entities and components laske185/stride
3 participants
@Kryptos-FR @Eideren @laske185