`engine.runRenderLoop()` still called after stopping & disposing it—resulting in an attempt to read a null property

I’m trying to change between scenes, which, in my code, are structured as follows:

  • A base BasicScene TypeScript class is responsible for the functionality shared by every scene, such as creating an Engine, a Babylon Scene, a UniversalCamera, a skybox, a ground for collisions, etc.
  • Derived scenes import their own models and make various specific changes, such as repositioning the camera or spawning a video screen.
  • index.ts, the entry point of the program, holds the function SetScene(), receiving a SceneType enum as the argument (e.g. SceneType.Mall) and switching to it. Upon doing so, it saves a reference to the new scene. That way, the next time the function is called, it first disposes the existing scene and then creates the new one.

Here’s what the code looks like.

The Base BasicScene

export class BasicScene {
  engine: Engine;
  scene: Scene;
  camera: UniversalCamera;

  constructor(canvas: HTMLCanvasElement) {
    const { engine, scene, camera } = this.CreateBasicScene(canvas);
    this.engine = engine;
    this.scene = scene;
    this.camera = camera;

    engine.runRenderLoop(() => scene.render());
  }

  protected async ImportEnvironmentModel(
    scene: Scene,
    model_directory: string,
    model_name: string
  ): Promise<void> {
    const root = new TransformNode("environment_model", scene);

    const container = await SceneLoader.LoadAssetContainerAsync(
      model_directory,
      model_name,
      scene
    );

    for (const mesh of container.meshes) {
      if (mesh.parent == null) mesh.setParent(root);
    }

    root.scaling = new Vector3(2, 2, 2);
    container.addAllToScene();
  }

  private CreateBasicScene(canvas: HTMLCanvasElement) {
    const engine = new Engine(canvas, true);
    const scene = new Scene(engine);
    const camera = this.CreateCamera(scene, canvas);
    const ground = this.CreateGround(scene);

    this.CreateHemisphericLight(scene);
    this.CreateSkybox(scene);
    this.HandleDevices(engine, camera);
    this.EnableXRSupport(ground, scene);
    this.RegisterInspectorOnInput(scene);

    scene.collisionsEnabled = true;
    scene.gravity = new Vector3(0, -0.15, 0);

    return { engine, scene, camera };
  }

  private CreateGround(scene: Scene): Mesh {
    const ground = MeshBuilder.CreateGround(
      "collision_ground",
      {
        width: 200,
        height: 200,
      },
      scene
    );
    ground.checkCollisions = true;
    ground.position = new Vector3(0, 0.074, 0);
    ground.isVisible = false;
    return ground;
  }

  private CreateCamera(
    scene: Scene,
    canvas: HTMLCanvasElement
  ): UniversalCamera {
    const camera = new UniversalCamera(
      "universal_camera",
      new Vector3(0, 0.74 * 2, 0.74 * 2),
      scene
    );
    camera.attachControl(canvas, true);
    const yRotation = Angle.FromDegrees(90);
    camera.rotation = new Vector3(0, yRotation.radians(), 0);
    camera.speed = 0.15;
    camera.minZ = 0;
    camera.ellipsoid = new Vector3(0.3, 0.7, 0.3);
    camera.checkCollisions = true;
    camera.applyGravity = true;

    // Add WASD movement.
    const ascii = (char: String) => char.charCodeAt(0);

    camera.keysUp.push(ascii("W"));
    camera.keysLeft.push(ascii("A"));
    camera.keysDown.push(ascii("S"));
    camera.keysRight.push(ascii("D"));

    return camera;
  }

  private EnableXRSupport(ground: Mesh, scene: Scene) {
    scene.createDefaultXRExperienceAsync({
      floorMeshes: [ground],
    });
  }

  private CreateHemisphericLight(scene: Scene): void {
    const light = new HemisphericLight(
      "hemispheric_light",
      Vector3.Up(),
      scene
    );
    light.intensity = 0.5;
  }

  private CreateSkybox(scene: Scene) {
    const skyboxTex = new CubeTexture(
      "./assets/environments/environmentSpecular.env",
      scene
    );
    const skybox = scene.createDefaultSkybox(skyboxTex, true);
    const skyboxMat = skybox?.material as PBRMaterial;
    if (skyboxMat) skyboxMat.microSurface = 0.7;
  }

  private HandleDevices(engine: Engine, camera: UniversalCamera) {
    const deviceSrcManager = new DeviceSourceManager(engine);

    deviceSrcManager.onDeviceConnectedObservable.add(device => {
      if (device.deviceType == DeviceType.Touch) {
        camera.inputs.removeByType("FreeCameraTouchInput");
        camera.inputs.addVirtualJoystick();
        camera.speed = 0.1;
      }
    });
  }

  private RegisterInspectorOnInput(scene: Scene) {
    const toggleInspector = () => {
      if (scene.debugLayer.isVisible()) scene.debugLayer.hide();
      else scene.debugLayer.show();
    };

    scene.onKeyboardObservable.add(kbInfo => {
      if (kbInfo.type == KeyboardEventTypes.KEYDOWN && kbInfo.event.key == "i")
        toggleInspector();
    });
  }
}

Derived Scenes, e.g. ShoppingMall

export class ShoppingMall extends BasicScene {
  changeScene: Function;

  constructor(canvas: HTMLCanvasElement, changeScene: Function) {
    super(canvas);
    this.changeScene = changeScene;

    super
      .ImportEnvironmentModel(
        this.scene,
        "./assets/models/mall/",
        "scene.babylon"
      )
      .then(() => {
        const door = this.scene.getMeshByName("door store");
        if (door) door.checkCollisions = true;

        // ...And other changes.
      });

    this.camera.position = new Vector3(0.35, 1.48, 55.8);
    this.camera.rotation = new Vector3(0, Tools.ToRadians(180), 0);
    this.camera.onCollide = otherMesh => {
      if (otherMesh.name == "door store") this.changeScene(SceneType.Main);
    };
  }
}

The Entry Point, index.ts

const canvas = document.getElementById("renderCanvas") as HTMLCanvasElement;
const streamUrl = "https://test-streams.mux.dev/x36xhzz/x36xhzz.m3u8";
let currentScene: BasicScene;

export enum SceneType {
  Main,
  Mall,
}

function SetScene(newScene: SceneType): void {
  if (currentScene) {
    currentScene.engine.stopRenderLoop();
    currentScene.scene.dispose();
    ///currentScene.engine.dispose();
    // As you can see, I tried various methods & combinations
    // of disposing the existing scene. None worked.
  }

  switch (newScene) {
    case SceneType.Main:
      currentScene = new Main(canvas, streamUrl, SetScene);
      break;
    case SceneType.Mall:
      currentScene = new ShoppingMall(canvas, SetScene);
      break;
  }
}

// Set up the first scene.
SetScene(SceneType.Main);

The Problem

When I’m switching from one scene to another (i.e. when currentScene isn’t null), immediately after any of the dispose() functions are called, I get the following error:

Uncaught TypeError: Cannot read properties of null (reading 'cameraRigMode')
    at Scene.render (scene.js:3430:1)
    at basic-scene.ts:58:44
    at Engine._renderFrame (engine.js:807:1)
    at Engine._renderLoop (engine.js:822:1)

The 58th line in basic-scene.ts is the following:

engine.runRenderLoop(() => scene.render());

I put a time-out between disposing the current scene and creating a new one, and the error message pops up immediately after the first, so the problem is that, despite calling engine.stopRenderLoop(), it’s still going.

The Question

What’s the proper way of disposing of a scene, then?

I have only one (Babylon) scene at a time, but just in case, I also tried to get rid of all scenes in engine.scenes, and nothing changed.

engine.runRenderLoop will only stop running if you dispose the engine itself and not the scene it is rendering.
To change scenes you will need to change the reference to the scene that the engine is currently rendering. I would separate the engine creation and the scene creation and keep a reference of currentScene. The renderloop should render currentScene, which can be changed at any given moment. To stay safe, make sure the currentScene object is still available before you run render(), and you are good to go

1 Like

I implemented the changes you suggested, but sadly, that didn’t resolve the problem. I get the same error, except now the new scene doesn’t load at all.

The new code:

const engine = new Engine(canvas);
let currentScene: Scene;

function SetScene(
  newScene: SceneType,
  enteringFromMall: boolean = false
): void {
  const oldScene = currentScene;

  switch (newScene) {
    case SceneType.Main:
      const main = new main(engine, streamUrl, SetScene, enteringFromMall);
      currentScene = main.scene;
      break;
    case SceneType.Mall:
      const mall = new ShoppingMall(engine, SetScene);
      currentScene = mall.scene;
      break;
  }

  if (oldScene) oldScene.dispose();
}

// Set up the first scene.
SetScene(SceneType.Main);
engine.runRenderLoop(() => currentScene.render());

If I comment out the last line—if (oldScene) oldScene.dispose()—I get a different error:

Uncaught TypeError: Cannot read properties of undefined (reading '0')
    at deviceInputSystem.js:212:58

The scene loads in this case, but of course, that leaves the previous scene running. Not ideal. :tired_face:

Any ideas?

It’s hard for me to follow the app’s architecture, so it’s very hard to say what went wrong there.

Having said that - make sure you attach the new scene, and make sure you don’t keep any references to the scene’s callbacks, inputs etc’ in your code. i.e. - make sure that the scene is completely decoupled from any other components.
The error is in deviceInputSystem. you can debuig and see what it is trying to read there, will probably provide you a few clues to the error.

What you are doing should (technically) work. I don’t know how the main and ShoppingMall classes look like, but if they return a new scene and are reusing the old engine, there shouldn’t be any issue.

1 Like

If you derive your scenes from the base scene, you’re creating a separate engine object for each scene. This can mess the things a lot. Isn’t this the case?

I changed that based on RaanaW’s suggestion. My new code is in the post above. Here’s a breakdown of how it works:

  1. In index.ts, the entry point of the program, I create a new Engine.
  2. Later, in SetScene(), when creating one of my BasicScene-derived classes, I pass in that engine. Everything else remains the same: they use that engine to create their (Babylon) Scenes, etc.
  3. I store the current scene’s Babylon Scene inside currentScene, which engine renders in its runRenderLoop() callback.
  4. Whenever SetScene() is called again, it stores a local reference to currentScene in oldScene, changes the currentScene pointer to a new one, and calls dispose() on the oldScene afterward. The rationale was to get engine pointing to the new Scene before destroying the old one.

Unfortunately, that didn’t work.

Hi!

Disposing a scene is working as you can see in this PG so there is a bug in your code somewhere :bug: :slight_smile: It’s pretty hard to help you without seeing your full source code. :frowning:

Or you can go further and set the scene2 to null to allow the js engine garbage collect the disposed scene object.

3 Likes

Or this one is even closer to your scenario:

One other thing, don’t use the Function type, it leads to errors:

public myMethod(callback: Function): void

but do this (example)

public myMethod(callback: (param1: string, param2: number) => number): void

This will make your IDE and you happier!

r.

4 Likes

Thank you, roland. I took a look at the PGs you provided, and honestly, I have no idea. I tested putting a delay between stopping the engine’s render loops and disposing of the scene, and indeed, that did work—on my computer.

I adjusted my code accordingly, and it looks like this now:

const engine = new Engine(canvas);

async function SetScene(
  sceneType: SceneType,
  enteringFromMall: boolean = false
): Promise<void> {
  engine.stopRenderLoop();
  const oldScene = engine.scenes[0];
  let newScene: Scene;

  switch (sceneType) {
    case SceneType.Main:
      const main = new Main(engine, streamUrl, SetScene, enteringFromMall);
      newScene = main.scene;
      break;
    case SceneType.Mall:
      const mall = new ShoppingMall(engine, SetScene);
      newScene = mall.scene;
      break;
  }

  engine.runRenderLoop(() => newScene.render());

  if (oldScene) {
    await Tools.DelayAsync(1000);
    oldScene.dispose();
  }
}

I decided to dispose of the previous scene after loading the new one so as not to add an unnecessary delay to loading times, but even so, this only a stopgap, not a solution: what happens if the program hangs unexpectedly and takes longer than a second to stop its render loops?

I could increase the delay duration, but that comes with its own problems. The Engine class unfortunately doesn’t come with an “on rendering stopped” observable, or at least none that I could find, so I can’t dispose of the scene in response to that.


At least the problem is clear now: the engine isn’t stopping immediately after stopRenderLoop() is called. For some reason, it’s still trying to render the last scene; I checked by logging the old and the new scene’s UIDs, and added a breakpoint where the error happens, and the scene whose activeCamera the code is trying to access is the old one.


A snippet from Scene.prototype.render in scene.js.

What’s curious is that the code checks that this.activeCamera is defined and non-null, and then once more calls update() on it. Only on the line after, when trying to access cameraRigMode, does it complain that activeCamera is null.

There is no need to create a new render loop. there is no need to stop the render loop. Keep the same render loop, rendering a reference outside of the scope of the update function, and update this variable with the new scene.
What you are doing is adding a new render loop while not removing the old render loop that references the old scene.

1 Like

Huh. I tried that already, and it didn’t work. Was about to type out a reply in that vein, but just in case, I decided to give it another try, and it does work now. Somehow. I did convert the callback mechanism to an observable, but IDK if that’s it. Either way, thanks, man.

Wait, no, it doesn’t work. I just forgot to dispose of the last scene. God, I’m tired. Same issue as before: I get the error message. Moreover, the program hangs and doesn’t do anything.

Hey @RaananW!
It was a quick example I made while my whole family was already skiing and waiting for me :pray: and I just wanted to show the disposal of the scene stuff :vulcan_salute:

@VerifiedTinker any chance sharing your code?

@VerifiedTinker
Another thing is bothering me in your code. You’re not using curly braces in your simple if statements. This might lead to errors and are very easily overlooked. Maybe the error came from something really this trivial. I mean don’t write if statements like this

if (cond) doSomething()

but always use curly braces

if (cond) { doSomething() }

1 Like

@roland, @RaananW, after a long session of commenting out parts of the code and testing if it still broke, I believe I’ve found the problem:

this.camera.onCollide = otherMesh => {
  if (otherMesh.name == "door store") {
    this.onEnterAnotherScene.notifyObservers(SceneType.Main);
    // This fires SetScene().
    // The same code sits in Main; it just directs to ShoppingMall instead.
  }
};

This is my makeshift solution for trigger colliders. There’s nothing wrong with it in and of itself—even if I leave it be, I can still transition to another scene by pressing the spacebar, which I bound to SetScene() for debugging purposes, just fine.

But if I do it by walking into a door, which is what this snippet checks, we get our error.

I could maybe try to debug why, but stepping through the code yesterday netted me nothing but headaches. Is there a different method of implementing trigger colliders? I could manually check every frame if the user is inside a trigger area, but perhaps there’s a better, or built-in, way.

The OnIntersectionEnterTrigger for Actions unfortunately only works on meshes. I could try attaching an invisible capsule mesh to the camera, but that sounds… dubious.

When dealing with multiple scenes I always use my SceneManager class which is responsible for all operations on scenes. I never create/dispose/enable/disable a scene outside of this class so I can keep track of the scenes. If your scene management is distributed in many classes you can easily loose control. It’s basically a good idea to follow this pattern (something like the Factory pattern) not just for the scenes, but for other things as well, mainly if you’re passing pointers through multiple classes. This centralized control allows you to debug your code much more easily and basically you should not encounter null pointers when correctly implemented.

This is my last guess/recommendation here, I am out of ideas. Sorry, it’s damn hard to help you without looking at your code. Good luck dude and don’t loose hope, you will crack this one, I am sure! :vulcan_salute:

2 Likes

Just a story: my thin instances stopped to render and it took me two days to get them back. The reason was, I somehow removed by mistake a line which was setting the layerMask on the base mesh :rofl::rofl::rofl::rofl: damn, I always promise myself that I am not going to refactor working code but I always break this rule and then this kind of shit happens :joy:

Seems that your function may be triggered onCollide several times while moving with a camera.
The space bar works and it is good. Probably you need an additional flag (false/true) to change after the first camera collision and after that don’t notify observers anymore.

It is completely normal - to use invisible meshes in case where you need them for the better illusion :slight_smile:

@roland Sorry I couldn’t put up more code here, but there really isn’t much to add besides what’s already there. It’s a new project. I really only have the two concrete BasicScenes, and they both have a door mesh that, if walked into, is supposed to respawn the user into another BasicScene.

My architecture’s similar to yours, in regards to SceneManager—it’s just sitting messily inside the entry-point index.ts until I fix the issues and refactor it. The only thing a BasicScene does is fire an observer, telling the scene manager to change to a new one.

@labris Good idea. Sadly, no. I checked, and SetScene() is called only once. I added the flags you suggested anyway, but that didn’t change anything.


Ultimately, I think I may have found a working solution: I load the new scene first, and only once its ready do I dispose of the old one. I’m not entirely sure this fixes the root cause and won’t create bugs in the future, but it’s the best I’ve got for now. I’d like to move on to other features. Scene management is something I can always come back to later.

For the final iteration, the code looks like this:

const engine = new Engine(canvas);
let currentScene: BasicScene;

function SetScene(
  sceneType: SceneType,
  enteringFromMall: boolean = false
): void {
  const oldScene = currentScene;

  switch (sceneType) {
    case SceneType.Main:
      const main = new Main(engine, streamUrl, enteringFromMall);
      currentScene = main;
      break;
    case SceneType.Mall:
      const mall = new ShoppingMall(engine);
      currentScene = mall;
      break;
    default:
      throw "The argument is invalid.";
  }

  currentScene.onEnterAnotherScene.add(type => SetScene(type));
  currentScene.scene.onReadyObservable.add(() => {
    if (oldScene && oldScene.scene) oldScene.scene.dispose();
  });
}

// Set up the first scene.
SetScene(SceneType.Main);
engine.runRenderLoop(() => currentScene.scene.render());

Thanks for your help, everyone. I appreciate it.

3 Likes