thelinuxlich / artemis_CSharp

Artemis Entity System Framework ported to C#
Other
341 stars 78 forks source link

When calling Entity.Delete(), the entity passed to EntitySystem.OnRemoved does not contain any components. #99

Open raycrasher opened 8 years ago

raycrasher commented 8 years ago

Sample code:

entity.AddComponent(new TestHealthComponent());
world.Update();
entity.Delete();

// in system, using Aspect.All(typeof(TestHealthComponent))

public override void OnRemoved(Entity entity) 
{
    Debug.Assert(entity.GetComponent<TestHealthComponent>()!=null);  <- fails
}

The components should be accessible during the call to OnRemoved, so that the system can have a chance to clean up.

I think this is related to Issue #49.

In EntityWorld.Update, the following code is called:

                    this.TagManager.Unregister(entity);
                    this.GroupManager.Remove(entity);
                    this.EntityManager.Remove(entity);

before this

this.EntityManager.Refresh(entity);

Thus, OnRemove is called with an empty Entity.

I have tried coming up with a solution, but my knowledge of Artemis' internals is still too limited.

Thank you!

Maximusya commented 8 years ago

Try the following: in EntityManager.Remove(Entity entity) move the code

if (this.RemovedEntityEvent != null)
{
    this.RemovedEntityEvent(entity);
}

up. Place it before this.RemoveComponentsOfEntity(entity); or even this.ActiveEntities.Set(entity.Id, null);

Thus you could subscribe your systems to EntityManager.RemovedEntityEvent (not the OnRemoved override) and in the handler you could Get any component you like. Ignore subsequent OnRemoved method call.

Yes, there still would be an issue if you try to access Tag or Group of the entity - but the problem with Components seems to be solved that way.

raycrasher commented 8 years ago

Thank you for the prompt reply!

As to your suggestion, wouldn't this be a bit inefficient? Take a bullet-hell shooter, for example. With hundreds of bullets deleted every frame, subscribing a non-bullet system to EntityManager.RemovedEntityEvent would be unwise.

I am currently investigating whether I can call EntitySystem.OnRemoved() inside EntityManager.Remove itself.

As for now, I have held off on doing cleanup code inside the OnRemoved() override, instead I am doing cleanup before Entity.Delete() is called using extension methods.

Maximusya commented 8 years ago

Even without subscription to EntityManager.RemovedEntityEvent every EntitySystem gets internal OnChange(Entity entity) method call with every entity that has been changed or deleted. There is a quick check whether any action should be performed: EntitySystem.OnChange:

...
bool contains = (this.SystemBit & entity.SystemBits) == this.SystemBit;
bool interest = this.Aspect.Interests(entity);
...

So if you do subscribe to EntityManager.RemovedEntityEvent, you could filter out "bullets" in non-bullet systems (though some work on access modifiers would be required to use same quick check).

BTW could you post here your cleanup extension method please? Just curios.

raycrasher commented 8 years ago

Sure! It's for a scene graph system I use for rendering hierarchical sprites.

public static void DeleteWithChildren(this Entity entity)
{
    var component = entity.GetComponent<SceneGraphComponent>();
    if (component != null)
    {
        if (component.Parent != null)
        {
            component.Parent.GetChildren().Remove(entity);
            component.Parent = null;
        }
        for(int i=component.Children.Count - 1; i>=0; i--)
        {
            component.Children[i].DeleteWithChildren();
        }
    }
    entity.Delete();
}

I was originally going to put it in an OnRemoved override for the system that handles SceneGraphComponents.