Fix memory growth when a model is replaced or loses a load race

src/controllers/actors/playerObject.ts

@@ -9,7 +9,7 @@ import { ModelObject } from '../render/modelObject';
 import { CharacterClassNumber, PlayerClass, getBaseClass } from '../../core/types';
 import { Entity, World, type Item } from '../../ecs/world';
 import { PlayerAction } from '@versions/shared/objects/enum';
-import { loadGLTF } from '../assets/modelLoader';
+import { disposeLoadedModel, loadGLTF } from '../assets/modelLoader';
 import { Ecs } from '../../ecs/current';
 import {
   BACK_BONE,
@@ -343,7 +343,8 @@ export class PlayerObject extends ModelObject {
 
     const seq = ++this.Pet.loadSeq;
     const gltf = await loadGLTF(spec.model, Ecs.world!);
-    if (seq !== this.Pet.loadSeq) return;
+    // A load overtaken by a newer one is freed, not left in the scene.
+    if (seq !== this.Pet.loadSeq) return disposeLoadedModel(gltf);
     this.Pet.load(gltf);
 
     for (const mesh of this.Pet.getMeshes(true)) {
@@ -365,7 +366,7 @@ export class PlayerObject extends ModelObject {
     if (!world) return;
     const seq = ++this.GmMark.loadSeq;
     const gltf = await loadGLTF(GM_MARK_MODEL, world);
-    if (seq !== this.GmMark.loadSeq) return;
+    if (seq !== this.GmMark.loadSeq) return disposeLoadedModel(gltf);
     this.GmMark.load(gltf);
     for (const mesh of this.GmMark.getMeshes()) mesh.isPickable = false;
   }
@@ -534,7 +535,9 @@ export class PlayerObject extends ModelObject {
     this.PartsPending++;
     try {
       const gltf = await loadGLTF(dir + modelPath, Ecs.world!);
-      if (seq !== part.loadSeq) return;
+      // Overtaken by a newer dress (a second inventory change before this
+      // one landed): its clone - skeleton, bone texture, clips - is freed here.
+      if (seq !== part.loadSeq) return disposeLoadedModel(gltf);
       part.load(gltf);
 
       gltf.mesh.isPickable = this.IsInteractable;

src/controllers/render/modelObject.ts

@@ -1166,8 +1166,11 @@ export class ModelObject {
     this.gltf = gltf;
     this._node.name = this.NodeNamePrefix + gltf.mesh.name;
 
+    // The whole old clone, not only its meshes. `instantiateModelsToScene`
+    // cloned a skeleton (with its bone texture) and clips for it that live in
+    // the scene: a re-dress left them behind on every inventory change.
     if (oldGltf && oldGltf !== gltf) {
-      oldGltf.mesh.dispose();
+      disposeGltf(oldGltf);
     }
 
     gltf.mesh.setParent(this._node);

src/controllers/render/modelSwapLeak.test.ts

@@ -0,0 +1,101 @@
+import { afterAll, describe, expect, it, vi } from 'vitest';
+import { NullEngine } from '@babylonjs/core/Engines/nullEngine';
+import { Scene } from '@babylonjs/core/scene';
+import { Mesh } from '@babylonjs/core/Meshes/mesh';
+import { CreateBox } from '@babylonjs/core/Meshes/Builders/boxBuilder';
+import { Skeleton } from '@babylonjs/core/Bones/skeleton';
+import { Bone } from '@babylonjs/core/Bones/bone';
+import { AnimationGroup } from '@babylonjs/core/Animations/animationGroup';
+import { runInAction } from 'mobx';
+import { ModelObject } from '@/controllers/render/modelObject';
+import { Ecs } from '@/ecs/current';
+import type { World } from '@/ecs/world';
+import { InventoryConstants } from '@/core/inventoryConstants';
+import { Hero } from '@/modules/hero/state';
+
+/**
+ * The memory that grew 200-700 MB an hour in long sessions. A model swapped
+ * for another left the old clone's skeleton (and its bone texture) and clips in the scene, and
+ * every inventory change swapped every worn part for an identical one.
+ */
+
+vi.hoisted(() => {
+  const g = globalThis as Record<string, unknown>;
+  g.location ??= { href: 'http://localhost/', search: '', hostname: 'localhost', protocol: 'http:' };
+});
+
+const engine = new NullEngine();
+const scene = new Scene(engine);
+afterAll(() => {
+  Ecs.world = null;
+  scene.dispose();
+  engine.dispose();
+});
+
+/** One `loadGLTF` clone: meshes, its own skeleton and a clip, all in the scene. */
+function clone(name: string) {
+  const root = new Mesh(`${name}.glb`, scene);
+  const body = CreateBox('node_0', { size: 1 }, scene);
+  body.parent = root;
+  const skeleton = new Skeleton(`skeleton_${name}`, `skeleton_${name}`, scene);
+  new Bone('bone_0', skeleton);
+  body.skeleton = skeleton;
+  return { mesh: root, skeleton, animationGroups: [new AnimationGroup(`${name}_action_0`, scene)] };
+}
+
+describe('a model loaded over another', () => {
+  it('frees the old clone whole: meshes, skeleton and clips', () => {
+    const object = new ModelObject(scene);
+    const first = clone('Sword01');
+    object.load(first);
+    const second = clone('Sword02');
+    object.load(second);
+
+    expect(first.mesh.isDisposed()).toBe(true);
+    expect(scene.skeletons).not.toContain(first.skeleton);
+    expect(scene.animationGroups).not.toContain(first.animationGroups[0]);
+    // The new one is untouched.
+    expect(scene.skeletons).toContain(second.skeleton);
+    expect(scene.animationGroups).toContain(second.animationGroups[0]);
+    object.dispose();
+  });
+
+  it('leaves nothing behind after many swaps', () => {
+    const object = new ModelObject(scene);
+    const skeletons = scene.skeletons.length;
+    const clips = scene.animationGroups.length;
+    for (let i = 0; i < 50; i++) object.load(clone(`Wing${i}`));
+    object.dispose();
+    expect(scene.skeletons.length).toBe(skeletons);
+    expect(scene.animationGroups.length).toBe(clips);
+  });
+});
+
+describe('the hero dressed from the inventory', () => {
+  // `Ecs.world` is deeply observable: it keeps a copy, so the appearance is read back from it.
+  Ecs.world = { scene, playerEntity: { charAppearance: { changed: false } } } as unknown as World;
+  const appearance = (Ecs.world.playerEntity as unknown as { charAppearance: { changed: boolean } }).charAppearance;
+
+  const wear = (slot: number, item: object | null) =>
+    runInAction(() => {
+      Hero.items[slot] = item as never;
+    });
+
+  it('is dressed again only when what is worn changes', () => {
+    wear(InventoryConstants.ArmorSlot, { group: 8, num: 1, lvl: 7, durability: 40 });
+    Hero.syncPlayerAppearance();
+    expect(appearance.changed).toBe(true);
+
+    // A potion drunk, loot picked up, durability lost: nothing worn looks different.
+    appearance.changed = false;
+    wear(InventoryConstants.ArmorSlot, { group: 8, num: 1, lvl: 7, durability: 39 });
+    Hero.syncPlayerAppearance();
+    Hero.syncPlayerAppearance();
+    expect(appearance.changed).toBe(false);
+
+    // A higher level does.
+    wear(InventoryConstants.ArmorSlot, { group: 8, num: 1, lvl: 8, durability: 39 });
+    Hero.syncPlayerAppearance();
+    expect(appearance.changed).toBe(true);
+  });
+});

src/ecs/systems/bandSystem.ts

@@ -3,7 +3,7 @@ import { angleLinkMatrix } from '../../controllers/render/boneLink';
 import { devQuery, devQueryNumbers } from '../../core/devSeams';
 import type { BmdLink } from '../../controllers/render/boneLink';
 import { instrumentById, instrumentParts, isInstrumentId, type InstrumentPart } from '../../modules/band/instruments';
-import { loadGLTF } from '../../controllers/assets/modelLoader';
+import { disposeLoadedModel, loadGLTF } from '../../controllers/assets/modelLoader';
 import { reportLoadFailure } from '../../controllers/assets';
 import { ModelObject } from '../../controllers/render/modelObject';
 import type { PlayerAction } from '@versions/shared/objects/enum';
@@ -96,7 +96,7 @@ export const BandSystem: ISystemFactory = world => {
     const seq = ++model.loadSeq;
     void loadGLTF(part.model, world).then(gltf => {
       const p = e.performing;
-      if (seq !== model.loadSeq || !p || (p.model !== model && !p.parts.includes(model))) return;
+      if (seq !== model.loadSeq || !p || (p.model !== model && !p.parts.includes(model))) return disposeLoadedModel(gltf);
       model.load(gltf);
       for (const mesh of model.getMeshes(true)) {
         mesh.isPickable = false;

src/ecs/systems/modelLoaderSystem.ts

@@ -1,6 +1,6 @@
 import { With } from 'miniplex';
 import { MapPlayerNetClassToModelClass } from '../../controllers/actors/mapPlayerNetClassToModelClass';
-import { getModel, loadGLTF } from '../../controllers/assets/modelLoader';
+import { disposeLoadedModel, getModel, loadGLTF } from '../../controllers/assets/modelLoader';
 import { ModelObject } from '../../controllers/render/modelObject';
 import { PlayerObject } from '../../controllers/actors/playerObject';
 import { applyNpcShadow } from '../../controllers/actors/modelFactoryPerId';
@@ -179,6 +179,9 @@ function createModelObject(
         return loadGLTF(modelFilePath, world).then(gltf => {
           if (entity.modelObject) {
             entity.modelObject.load(gltf);
+          } else {
+            // The entity went while it loaded; nothing will own this clone.
+            disposeLoadedModel(gltf);
           }
         });
       }

src/modules/hero/state.ts

@@ -26,6 +26,18 @@ function walkDirection(rotYRadians: number): number {
   return ((Math.round((rotYRadians * 4) / Math.PI) + 1) % 8 + 8) % 8;
 }
 
+/**
+ * what of a worn item the body shows (model, level glow, excellent/ancient, sockets).
+ * Durability, stack count and options are left out: they never change the look.
+ */
+function lookOf(item: Item | null | undefined): string {
+  if (!item) return '-';
+  return `${item.group}:${item.num}:${item.lvl ?? 0}:${item.isExcellent ? 1 : 0}:${item.isAncient ? 1 : 0}:${item.socketCount ?? 0}`;
+}
+
+/** the look last handed to each hero entity, so an unchanged one is not dressed again. */
+const dressedLook = new WeakMap<object, string>();
+
 /**
  * The hero: name, class, stats, vitals, level and experience, money, the inventory and worn items,
  * and the requests that act on them (stat points, walking, emotes). Packets write it (`net.ts`).
@@ -251,7 +263,16 @@ class HeroState {
     // horns. PetSystem spawns the world objects, PlayerObject.Pet carries the
     // body-linked Imp, and AnimationSystem reads it for the ride clips.
     playerEntity.charAppearance.pet = this.petSlot || null;
-    playerEntity.charAppearance.changed = true;
+    // every inventory change (a potion drunk, loot picked up, an item moved
+    // in the vault) comes through here. Re-dressing an unchanged body rebuilt
+    // all its parts each time; only a change in what is worn does it now.
+    const a = playerEntity.charAppearance;
+    const look = [a.helm, a.armor, a.pants, a.gloves, a.boots, a.leftHand, a.rightHand, a.wings, a.pet]
+      .map(lookOf)
+      .join('|');
+    if (dressedLook.get(playerEntity) === look) return;
+    dressedLook.set(playerEntity, look);
+    a.changed = true;
   }
 
   increaseStatRequest(stat: number): void {