Cleaning up destroyed entities

[krispya]

• Sometimes entities are created with effects and those effects need to be cleaned up when the entity is destroyed.
• An entity's data is not safe to access once it is destroyed even if it is possible. It can be deleted or overwritten at any time.
• A removed tracker currently picks up destroyed entities but again the data is not safe to access.

What is the preferred workflow and behavior here? Should the removed trackers remove entities that are destroyed? Should there be a way to flag entities for cleanup so destroying them keeps their data until another command is given? Should data that needs to persist for cleanup be stored outside of the entity? Should side effects be done in callbacks like onAdd and onRemove?

[Ctrlmonster]

Thought I'd share the approach I went with. This involves @pmndrs/Directed – whose connection we should figure out anyway (i.e. do we want to a built in a notion of systems into Koota directly).

// build the gameplay schedule (basically all our systems are part of it)
gameplaySchedule.build();

// create a new schedule and make executing the gameplay schedule part of it
export const schedule = new Schedule();
const RunGameplay = ({world}) => {
  gameplaySchedule.run({world});
}

// add the entity destruction system after it
schedule.add(RunGameplay);
schedule.add(DestroyEntities, {after: RunGameplay});
schedule.build(); // <- this is the schedule we'll run in rAF

// new trait that the `DestroyEntities` system will query for
export const MarkedForDestruction = trait({
  destructionCb: {
    cb: (entity: Entity, world: World) => {}
  }
});

// helper function that can be called instead of `entity.destroy()` whenever we want 
// to schedule destruction. Optionally accepts a callback
export function destroyAtEndOfFrame(entity: Entity, onDestroy?) {
  entity.add(MarkedForDestruction({
    destructionCb: {
      cb: onDestroy
    }
  }));
}

// Omitting the destruction system – all it does is query for entities with the 
// MarkedForDestruction trait, execute the callback and destroy the entity.

This then allows you to write your own cleanup systems, i.e. world.query(IsPlayer, MarkedForDestruction) or pass a callback with the necessary work directly to the destruction system destroyAtEndOfFrame(playerEntity, () => someMap.delete(playerEntity)).

I feel like at the current point in time it's better not to build anything in, unless we decide we want to make systems part of the package (I'm a proponent of that, the ecosystem is currently lacking a systems scheduler) and offer a default schedule out of the box, then sth like this could be an approach.

[krispya]

This is the approach where instead of destroying an entity, it gets tagged for cleanup and then a system down the line destroys it. The problem I see with this is that it makes destroying an entity an unsafe operation. You now need to have perfect knowledge over which entities need cleanup, so should not be destroyed, and which entities don't need cleanup, so can be destroyed. Which means you need knowledge of what traits are on it and the systems operating on those traits. I foresee this being a problem with a growing ecosystem where each package installed is a black box.

Ideally, we implement a solution where entities can be safely destroyed and cleaned up without special knowledge about how all the internals are working.

[krispya]

IMO, a system abstraction is something that would work on top of whatever we implement, so we would need to be able to express the cleanup and destroy life cycles without it anyway.

[Ctrlmonster]

What if we build in a destruction tag like above with a cleanup system at the end of the schedule and then have world.query() automatically query for Not(MarkedForDestruction).

Then an extra method like queryDestroyed() could automatically add the MarkedForDestruction tag to queries, so you're able to fish for "about to destroyed entities" in a cleanup system, while all other queries will pretend that the entity already doesn't exist.