Doraku / DefaultEcs

Entity Component System framework aiming for syntax and usage simplicity with maximum performance for game development.
MIT No Attribution
658 stars 62 forks source link

Issue when disposing an entity from Update(TState state, in Entity entity) #176

Closed dkominek closed 1 year ago

dkominek commented 1 year ago

When I call Dispose within an override of Update(TState state, in Entity entity) on an AEntitySetSystem, the next entity in the ReadOnlySpan of Update(TSatate state, in Entity entity) is being disposed instead of the current entity. I narrowed the issue down to the EntityDisposingMessage updating the Entity Id to the next entity before the EntityDisposedMessage is published, causing the wrong entity to be disposed. This only occurs if they system has more than 1 entity being acted upon and any entity except the last entity is disposed.

Here is an example system:

using System;
using DefaultEcs;
using DefaultEcs.System;

namespace Lifecycle;

public struct Lifetime
{
    public float Duration { get; private set; }

    public float Created { get; set; } = MinValue;
    public float AliveUntil => Created + Duration;

    public Lifetime(float duration)
    {
        Duration = duration;
    }
}

[With(typeof(Lifetime))]
class LifecycleManager : AEntitySetSystem<float>
{
    public SLifecycleManager(World world) : base(world)
    {
    }

    protected override void Update(float state, in Entity entity)
    {
          ref var lifetime = ref entity.Get<Lifetime>();
          if (lifetime.Created == float.MinValue)
          {
              lifetime.Created = state.TotalTime;
          }

          if (lifetime.AliveUntil < state.TotalTime)
          {
              // An exception occurs when processing the next entity in the set (first line of this function)
              entity.Dispose();

              // Stepping through the source, it looks like something in the Publish(EntityDisposingMessage) 
              // is updating the entity id of the entity struct before publishing EntityDisposedMessage, 
              // probably happening when updating the ReadOnlySpan of the set?

              // It seems that using "in Entity" for the parameter of this function may be the cause, is there 
              // some design decision as to why the entity is passed by reference to 
              // Update(float state, in Entity entity) function?

              // The exception does not occur when creating a new variable from the "in" parameter and 
              // disposing from that as shown below
              var newEntity = entity;
              newEntity.Dispose() // this does not fail
          }
    }
}
dkominek commented 1 year ago

Reading through the latest on Gitter, it seems that maybe useBuffer would resolve the issue? LMK your thoughts.

Doraku commented 1 year ago

hello, sorry for not answering earlier but that's probably it yeah. by default systems iterate entities directly on the underlying set to reduce memory copy. When you dispose it it is swapped with the last entity of the set at the same time. useBuffer parameter of the system constructor is specifically here for that case, so that the system iterate on a copy of the set instead of the set directly.