System removal

[pavelvasev]

Hi Mike!

As I told in another discussion, there is a need to remove systems dynamically. If the implementation of such need does not break ecs.js plans, I suggest 2 variants of API and would be happy to hear your opinion.

variant 1

var remover = ECS.addSystem(w, boringSystem)
....
remover();

variant 2

var s = ECS.addSystem(w, boringSystem)
....
ECS.removeSystem( w, s );

My own opinion is that variant 2 looks like interfaces of other calls in ecs.js. Moreover it have additional flexibility for future for the needs of other system management (priorities, pause..).

What do you think of it?

p.s. it seems additionally it will be useful to have something like onRemove option for systems, in addition to onUpdate/etc.

[mreinstein]

My own opinion is that variant 2 looks like interfaces of other calls in ecs.js

I agree, I think variant 2 is what I'd probably go with.

I suspect that this would probably require some refactoring, because currently the addSystem function internally instantiates the system. Open for ideas though!

[pavelvasev]

I suspect that this would probably require some refactoring, because currently the addSystem function internally instantiates the system. Open for ideas though!

I couldn't catch a problem you are talking about... Could you please clarify? In my thougths, in overall it is something like this:

function addSystem (world, fn) {
    const system = fn(world)

    world.stats.systems.push({
        name: fn.name || 'anonymousSystem',
        timeElapsed: 0, // milliseconds spent in this system this frame
        // key is filterId, value is number of entities that matched the filter
        filters: { }
    })

    if (!system.onFixedUpdate)
        system.onFixedUpdate = function () { }

    if (!system.onPreUpdate)
        system.onPreUpdate = function () { }

    if (!system.onUpdate)
        system.onUpdate = function () { }

    if (!system.onPostUpdate)
        system.onPostUpdate = function () { }

    world.systems.push(system)

    return system; // this plays a role of system_handle
}

function removeSystem( world, system_handle ) {
  let system = system_handle;
  let i = world.systems.indexOf(system);
  if (i >= 0) {
    // for future: if (system.onRemove) system.onRemove();
    world.systems.splice( i,1 );
    // sinse stats array is in sync with systems array, use same index for stats removal
    world.stats.systems.splice( i,1 );
  }
}

[mreinstein]

Ah I see, so you'd return system to keep a handle which could then be passed to removeSystem(...). Yes, that could work. The only change I'd make is using removeArrayItems rather than splice. And it would need unit tests.

[mreinstein]

@pavelvasev still open to a PR for this if you're keen! :)

[mreinstein]

System removal isn't something I need, but again if anyone comes along and decides they want this, I'm happy to re-open it.