From c926e0bffab4a5c52d76610f08aae6de4049f2cf Mon Sep 17 00:00:00 2001 From: Domingos Martins Date: Sun, 8 Nov 2026 09:35:35 -0300 Subject: [PATCH] Fix memory growth when a model is replaced or loses a load race Long sessions grew by hundreds of MB per hour. Orphaned skeletons (with their bone textures) and animation groups piled up in the scene because: - ModelObject.load only disposed the old clone's mesh; the cloned skeleton and its clips stayed in the scene. It now uses disposeGltf. - A load overtaken by a newer one (Pet, GM mark, equipment parts, band instruments) or whose entity had already gone returned without freeing the freshly cloned model. It now calls disposeLoadedModel. - Hero.syncPlayerAppearance set charAppearance.changed on every inventory change (potion drunk, loot picked up, item moved), so each one re-dressed ~10 parts. It now only does so when what is worn (model, level, excellent/ancient, sockets) actually changes. Reproduced offline: 120 quick swaps left 128 orphaned skeletons and animation groups went 114 -> 1054; with the fix, 0 orphans. Tests: src/controllers/render/modelSwapLeak.test.ts (fail without the fix). --- src/controllers/actors/playerObject.ts | 11 +- src/controllers/render/modelObject.ts | 5 +- src/controllers/render/modelSwapLeak.test.ts | 101 +++++++++++++++++++ src/ecs/systems/bandSystem.ts | 4 +- src/ecs/systems/modelLoaderSystem.ts | 5 +- src/modules/hero/state.ts | 23 ++++- 6 files changed, 140 insertions(+), 9 deletions(-) create mode 100644 src/controllers/render/modelSwapLeak.test.ts diff --git a/src/controllers/actors/playerObject.ts b/src/controllers/actors/playerObject.ts index df685a48e..c4bfc4a5e 100644 --- a/src/controllers/actors/playerObject.ts +++ b/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; diff --git a/src/controllers/render/modelObject.ts b/src/controllers/render/modelObject.ts index 4fa08e83b..cc392affe 100644 --- a/src/controllers/render/modelObject.ts +++ b/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); diff --git a/src/controllers/render/modelSwapLeak.test.ts b/src/controllers/render/modelSwapLeak.test.ts new file mode 100644 index 000000000..14ddfddd9 --- /dev/null +++ b/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; + 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); + }); +}); diff --git a/src/ecs/systems/bandSystem.ts b/src/ecs/systems/bandSystem.ts index f597365c4..c37174b73 100644 --- a/src/ecs/systems/bandSystem.ts +++ b/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; diff --git a/src/ecs/systems/modelLoaderSystem.ts b/src/ecs/systems/modelLoaderSystem.ts index 140fca285..55b929ea9 100644 --- a/src/ecs/systems/modelLoaderSystem.ts +++ b/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); } }); } diff --git a/src/modules/hero/state.ts b/src/modules/hero/state.ts index 8f60dc7f5..ae4d30317 100644 --- a/src/modules/hero/state.ts +++ b/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(); + /** * 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 {