Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the problem that there is no cleanup script when direct gc. #1716

Conversation

cptbtptpbcptdtptp
Copy link
Collaborator

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

What is the current behavior? (You can also link to an open issue here)

What is the new behavior (if this is a feature change)?

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

Other information:

@cptbtptpbcptdtptp cptbtptpbcptdtptp added the bug Something isn't working label Aug 29, 2023
@cptbtptpbcptdtptp cptbtptpbcptdtptp self-assigned this Aug 29, 2023
@cptbtptpbcptdtptp
Copy link
Collaborator Author

Demo:

/**
 * @title Script Basic
 * @category Basic
 */
import {
  Camera,
  DirectLight,
  Entity,
  Script,
  WebGLEngine,
  Light,
  Scene
} from "@galacean/engine";

WebGLEngine.create({ canvas: "canvas" }).then((engine) => {
  engine.canvas.resizeByClientSize();
  const scene = engine.sceneManager.activeScene;
  const root = scene.createRootEntity("root");

  const cameraEntity = root.createChild("camera");
  cameraEntity.addComponent(Camera);
  cameraEntity.transform.setPosition(0, 0, 5);

  const lightEntity = root.createChild("light");
  lightEntity.addComponent(DirectLight);

  root.addComponent(TestScript);
  scene.destroy();
  engine.resourceManager.gc();
  engine.run();
});

class TestScript extends Script {
  onStart(): void {
    console.log("on start");
  }

  onUpdate(deltaTime: number): void {
    console.log("on update");
  }

  onLateUpdate(deltaTime: number): void {
    console.log("on late update");
  }

  onPhysicsUpdate(): void {
    console.log("on physics update");
  }
}

@GuoLei1990
Copy link
Member

Entity destroy shouldn't dependent gc()

@@ -132,7 +132,6 @@ export class ComponentsManager {
script.onStart();
}
}
onStartScripts.length = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete this will cause performance problem

this._onPhysicsUpdateScripts.garbageCollection();
this._onUpdateAnimations.garbageCollection();
this._onUpdateRenderers.garbageCollection();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this._elements.length = this.length; can save memory

_gc(): void {
this._colliders.garbageCollection();
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same before

this._spotLights.garbageCollection();
this._pointLights.garbageCollection();
this._directLights.garbageCollection();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same before

@GuoLei1990 GuoLei1990 added the resource Resource-related functions label Aug 30, 2023
@GuoLei1990 GuoLei1990 closed this Sep 1, 2023
@GuoLei1990
Copy link
Member

Fixed in: #1724

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working resource Resource-related functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants