MobManager should remove items from equipment or inventory on removeMob()

[nelsonsbrian]

Since core supports Npc with inventory and equipment, it should also support removing those items when the Npc is removed. Currently we have MobManager.removeMob(mob) that handles most of clean up. Right now, all the Items that are in possession of the Npc, still remain in the ItemManager. This will just build up over time as the uptime of the server increases.

For the time being, I've added some code (in my repository) to the removeMob() method in the MobManager class. I can see this functionality being added here or in the Npc class.

  if (mob.equipment && mob.equipment.size) {
    mob.equipment.forEach((item, slot) => mob.unequip(slot));
  }

  if (mob.inventory && mob.inventory.size) {
    mob.inventory.forEach(item => this.state.ItemManager.remove(item));
  }

I unequip the items first as the current stack will eventually call Character.removeItem() on Items being worn in equipment and that removeItem method points toward inventory.

Would like to see this be officially supported.

[azigler]

I like this idea. However, you should also account for the same thing on PlayerManager, since this same thing happens with items held and equipped by a Player.

Also, you can't refer to state with this.state here, it will throw an error. Instead, you could attach the ItemManager from state on the Item (for example, as item.__manager) during the Item#hydrate function. You could do that like this:

// src/Item.js
...
hydrate(state, serialized = {}) {
    if (this.__hydrated) {
      Logger.warn('Attempted to hydrate already hydrated item.');
      return false;
    }
    this.__manager = state.ItemManager;

    // rest of hydrate function...
...

Once you had that, then you could do something like this to remove an Item from ItemManager from within the Item itself:

if (mob.inventory && mob.inventory.size) {
    mob.inventory.forEach(item => item.__manager.remove(item));
}