Is there a way Archetype onEntityRemoved listeners can still access the removed component?

[mikecann]

There are scenarios where you would like to access the removed component in the onEntityRemoved handler of an archetype so you can perform some sort of cleanup when an entity is removed.

For example:

const archetype = this.world.archetype("mesh", "pick");

archetype.onEntityAdded.add((e) => {
  const action = new ExecuteCodeAction(ActionManager.OnPickTrigger, (event) => {
    // do something with this
  });
  
  // We save the action so we can unregister it when removed
  e.pick.action = action;
  e.mesh.actionManager.registerAction(action);
});

archetype.onEntityRemoved.add((e) => {  
    // oops, "pick" has been removed from the entity so we cannot access the action to "unlisten" it
  e.mesh.actionManager.unregisterAction(e.pick.action)
});

As the above example shows however the "pick" component no longer exists if it was removed from the entity (which caused it to be removed from this archetype) which is correct but it means that we are now no longer able to "unregister" the action and on actionManager on mesh.

Perhaps the removed components could be supplied to onEntityRemoved too? Or perhaps another signal onBeforeEntityRemoved?

[mikecann]

For now I work around the solution with the following:

archetype.onEntityRemoved.add((e) => {
  const actions = e.mesh.actions.filter((a) => a.trigger == ActionManager.OnPickTrigger);
  for (const action of [...actions]) e.mesh.actionManager.unregisterAction(action);
});

Which bypasses the need for the removed component.

[hmans]

Thanks for the issue. This happens because entities are immediately mutated, so once the component is gone, it's gone. In Miniplex 2.0, I could add some extra events to World that are invoked synchronously when the user adds or removes components:

• onComponentAdded(entity, componentName, componentValue)
• onComponentRemoved(entity, componentName, previousComponentValue)

What do you think about these?

Other than that, here are some other things you could do (even in 1.0):

• Create an archetype specifically for the pick component and subscribe to its onEntityRemoved event
• In your existing callback, you could just test entity.pick for undefined, because that is guaranteed to mean that pick once was on the entity (otherwise it would never have been added to the archetype), but has since (probably now) been removed

[mikecann]

What do you think about these?

Ye those could work.. I assume the cost of executing onComponentAdded onComponentRemoved for every component added or removed even if there isnt a listener wouldnt be too large in the grand scheme of things?

Another way it could be done I suppose is if you make the type of onEntityTouched a bit more complex:

type EntityTouchedEvent =
  | {
      kind: "component-added";
      component: any;
      entity: E;
    }
  | {
      kind: "component-removed";
      component: any;
      entity: E;
    }
  | {
      kind: "init";
      entity: E;
    };

onEntityTouched = new Event<EntityTouchedEvent>();

But probably this would hurt performance with lots of extra allocations per entity change..

Hmmm 🤔

You could add more events to Bucket onComponentAdded and onComponentRemoved but I guess you are trying to keep Bucket generic and not leak your abstraction.

[hmans]

I've been playing around with a few approaches and probably need to change the way events are implemented for this, but it's not a big deal. There definitely needs a way to have an event trickle down the cascade of buckets starting with the world bucket that notifies interested parties that an entity is about to change (and do it before it has actually changed.)

So, good catch! I'll put this in the 2.0 milestone and figure something out.

[hmans]

A quick update, I'm having a hard time finding a good pattern here that will both work and not go overboard on created objects.

I'm toying with the idea of providing a different sort of pattern where, for example, your onEntityAdded callback may return a "teardown" function that will automatically be executed once the entity leaves the bucket again. This still doesn't give you access to the removed component, but it would allow you to store the data you need inside the callback's scope, similar to how you'd do it inside a React useEffect callback.

With this, your original example could look like this:

archetype.onEntityAdded.add((e) => {
  const action = new ExecuteCodeAction(ActionManager.OnPickTrigger, (event) => {
    // do something with this
  });
  
  e.mesh.actionManager.registerAction(action);

  return () => {
    e.mesh.actionManager.unregisterAction(action)
  }
});

Does this make sense? Would this help you?

[hmans]

Addendum: while I think the above idea is neat, it doesn't fully solve the issue. The real problem is that between the on-added code and the return-value on-removed-code, the entity will be mutated. In the example code I gave, this isn't a problem, because we're only accessing a variable declared within the callback's scope (action), but it's still too tempting to source data from the removed entity, especially considering it'll still have its type (where the needed component is available.)

tl;dr I still need to find a way to process entities that leave buckets before they're actually mutated. Damn! :-)

[mikecann]

Does this make sense? Would this help you?

Yes I like it but as you mention in your above addendum it could potentially cause some unexpected user errors. You could alternatively pass in the new "potentially removed components" entity into the callback and just hope the user will use that instead:

archetype.onEntityAdded.add((e) => {
  const action = new ExecuteCodeAction(ActionManager.OnPickTrigger, (event) => {
    // do something with this
  });

  e.mesh.actionManager.registerAction(action);

  // e is now Partial<Entity> becuase any of the components could have been removed
  return (e) => {
    e.mesh?.actionManager.unregisterAction(action)
  }
});

IMO I would be perfectly fine with something like this. As with React Hooks, it could be a footgun to users but I think thats the price you pay. On the plus side I think the use of onEntityAdded and onEntityRemoved would be minimal unlike hooks.

I'm having a hard time finding a good pattern here that will both work and not go overboard on created objects.

Im assuming you are referring to the fact that the API would look semething like this:

archetype.onEntityRemoved.add((entity, removedComponents) => {})

Where removedComponents is an object / map that contains key / value of components removed, e.g. { pick: PickComponent }

And your concern is that if you have to create a removedComponents object every time you invoke onEntityRemoved it could be expensive because its going to create a lot of objects?

As mentioned above IMO onEntityRemoved listening would be a rare case, most systems wouldnt need to listen to it, so before you call it you could do a check to see if there are any listeners then dont execute it it.. e.g.:

if (this.onEntityRemoved.listeners.length > 0) {
  // build the removedComponents object
  // call the listeners
}
else // dont do the extra work 

[hmans]

archetype.onEntityRemoved.add((entity, removedComponents) => {})

I think a problem with this approach is that the archetype might be indexing multiple components. In that case, the entity would be removed from the archetype as soon as any of the indexed components is removed, meaning that the code consuming the removedComponents object here would first need to check which component was removed (and is thus made available in the object), and react accordingly.

No, I need to find a way to trigger onEntityRemoved while the entity is still unmodified, or find a different solution. Eep!

[hmans]

Picking up on the "effect-like" API I mentioned previously, I think that what could actually make it work is the fact that if component values are objects (instead of scalar values), getting a local reference to it will allow the teardown function to access the component's current value even though it has been removed from the entity.

This would actually work:

type Entity = {
  health?: { current: number, max: number }
}

const world = new World<Entity>()

world.archetype("health").onEntityAdded.add((entity) => {
  /* Get a local reference to the health component */
  const { health } = entity

  console.log(`A new entity appeared with ${health.current}/${health.max} HP!`)

  return () => {
    /* Even though `health` no longer exists on `entity`, our local variable is still pointing
    at the same object, so we can in fact inspect the "last active value": */
   
    if (health.current < 25) {
      terribleDyingSound.play()
    }
  }
})

The one caveat remaining would be that this would not work with components that just hold scalar values, ie. if health here was just typed number and not { current: number, max: number }.

[hmans]

Alright. My desperation knows no bounds, so I sat down this morning and rewrote the entire core library from scratch, making it a little more aware of what archetypes and queries are. And now this works beautifully, and I have the test case to prove it:

This works now because this new version is able to check if the removal of a component will cause the entity to be removed from an archetype before this actually happens. Yay!

[mikecann]

LOL! You mad man!

This works now because this new version is able to check if the removal of a component will cause the entity to be removed from an archetype before this actually happens. Yay!

Yey :)

Did you do the rewrite as an experiment / spike or are you actually going to go with the above?

Whats up with this new syntax?

world.archetype({ all: ["age"] });

is onEntityRemoved the correct name for it? Has it been removed from the archetype yet or is it still there? If I was to iterate entities in the archetype in the callback would I sill find it there? If so should it be onBeforeEntityRemoved ?

[hmans]

Did you do the rewrite as an experiment / spike or are you actually going to go with the above?

It's a quick rewrite of the core library (not a huge deal considering its extremely lean nature) that I will very likely merge into main (it currently lives in this PR here.) The previous iteration was a little too focused on simply using predicate functions to derive archetype buckets from the main world object, and was going so far with this generalized approach that it made certain ECS-ish things (like the one we're discussing in this issue) difficult. This new implementation gives the world a little more knowledge about what the archetype buckets actually represent, while still retaining all of the nice changes introduced for 2.0.

Whats up with this new syntax?

world.archetype({ all: ["age"] });

One of the things I want to start supporting in 2.0 is being able to query for entities that do not have specific components (or have any of a list of components). This new version expresses these queries as objects with any, all and none keys. Consider these objects mostly an implementation detail; I'm working on a small DSL that makes composing these as simple as something like this:

world.archetype(all("foo", "bar").none("baz"))

(Subject to change, as with most other things :b)

I also support the API we've had so far:

world.archetype("foo", "bar")

is onEntityRemoved the correct name for it? Has it been removed from the archetype yet or is it still there? If I was to iterate entities in the archetype in the callback would I sill find it there? If so should it be onBeforeEntityRemoved ?

The callback is invoked after the entity has been removed from the archetype (but before the entity object actually loses the component), so if you were to iterate over the entities in the archetype within that callback, that entity would already be gone.

Is there value in providing a callback that gets invoked before the entity is removed from the archetype? It would be easy to add, but I have no idea if it would be useful.

[mikecann]

I also support the API we've had so far:

Okay cool. I personally have never had the need to do more complex queries such as any, none etc. But it seems like a lot of ECS frameworks do implement this so I guess some people use them?

If I wanted to define an archetype that does have a component I would just not include it..

const archWithPositionAndVelocity = world.archetype("position", "velocity");
const archWithoutVelocity = world.archetype("position");

I will very likely merge into main

Excellent :) Sorry this has blown out your 2.0 release so much but its something that I personally will be using in several places in our codebase.

[hmans]

If I wanted to define an archetype that does have a component I would just not include it..

The typical use case is that you may have entities that are "paused" in some manner. For example, a physics library might put physics bodies to sleep until they collide with something, and model this using a sleeping component.

Having said that, I've always argued that it's in many to most cases probably better for performance to just do a runtime if (entity.sleeping) continue within the loop instead of moving entities into and out of an archetype, but on the other hand this requires systems to have extra knowledge about the entity structure, which could become a (design) issue in more complex setups.

Either way, a lot of users have been asking for this sort of functionality, so here it is. :P

Excellent :) Sorry this has blown out your 2.0 release so much but its something that I personally will be using in several places in our codebase.

Absolutely no worries! These are important improvements.