Removing a component seems to modify Entity parameter

[SyndaKitty]

Whenever I Remove a component from an Entity in the Update method that takes the state and an individual Entity, this seems to modify the Entity struct in some way and cause some strange behavior. I have no idea how this is even possible :)

Based on my understanding, the below should execute and pass the asserts. Instead what I find is that e1 has only ComponentA removed, at which point the Entity parameter somehow gets modified to point at e2, so ComponentB gets removed from e2. Then the update method gets called again for e2 and ComponentA is then removed from it. After the system returns, the execute fails for Debug.Assert(!e1.Has<ComponentB>());

using DefaultEcs;
using DefaultEcs.System;
using System.Diagnostics;

World world = new World();
var e1 = world.CreateEntity();
e1.Set(new ComponentA());
e1.Set(new ComponentB());
var e2 = world.CreateEntity();
e2.Set(new ComponentA());
e2.Set(new ComponentB());

RemoveComponentSystem system = new RemoveComponentSystem(world);
system.Update(0);

Debug.Assert(!e1.Has<ComponentA>());
Debug.Assert(!e1.Has<ComponentB>());
Debug.Assert(!e2.Has<ComponentA>());
Debug.Assert(!e2.Has<ComponentB>());

struct ComponentA { }
struct ComponentB { }

[With(typeof(ComponentA))]
[With(typeof(ComponentB))]
class RemoveComponentSystem : AEntitySetSystem<int> {
    public RemoveComponentSystem(World world) : base(world) { }

    protected override void Update(int state, in Entity entity) {
        entity.Remove<ComponentA>();
        entity.Remove<ComponentB>();
    }
}

[Doraku]

To remove copy operations, entities are directly enumerated from the inner EntitySet cache by default when using the AEntitySetSystem base type. So when you remove the first ComponentA, internally the entity is actually removed from the EntitySet of the system (because it no longer has a ComponentA) and since the entity was a reference to that memory, it no longer point to the correct entity. To remedy this when you make structural changes, you can use the base constructor taking a bool useBuffer and set it to true, the system will make a copy of the entities before updating them so it always refer to the correct entities. I should definitely update the documentation here to reflect that some of those rules applies to non multithreading code too.

[SyndaKitty]

I was afraid you were going to say this was intentional :) What you said makes sense though, and yes please do update the documentation, I was looking carefully to see if this was mentioned about Remove for single threading and did not see anything so assumed it was unintentional.

Due to the way I am doing things now, the command buffer doesn't really suit my needs, as I want to serialize all the entities a system processed so I can get a before/after snapshot for debugging. I found that making a copy of the Entity and using that in the update works as intended:

protected override void Update(int state, in Entity entity) {
    Entity entityCopy = entity;
    entityCopy.Remove<ComponentA>();
    entityCopy.Remove<ComponentB>();
}

Is there any problem with this approach?

[Doraku]

Your code works unintentionally :D Internally this is what happen: EntitySet > [e1, e2], Count = 2

• you remove ComponentA from e1
• e1 should no longer be part of the set so internally e2 is copied to the position of e1

EntitySet > [e2, e2], Count = 1

you use a copy of the entity passed by ref e1 and the system was already iterating the set with the previous count so it still works as intended but you are at the mercy of an internal change, if for example you are creating new entities which should end up in the set, it would no longer works because the new entity e3 would overwrite the second e2 (EntitySet > [e2, e3], Count = 2) and you would never update the entity e2 in your system. It's much safer to use useBuffer.

[SyndaKitty]

Got it! Thanks for the information :)