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.

Sign up for GitHub

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

Post effect sketches without root properly reload #473

Open
wants to merge 2 commits into
base: alpha
Choose a base branch
from
Open

Post effect sketches without root properly reload #473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7134b22
Add try catch when initializing user code
cale-bradbury Dec 6, 2024
3f0a13b
Search for post effects and handle their removal as rootless sketches
cale-bradbury Dec 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/engine/src/HedronEngine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ export class HedronEngine {

public async initiateSketchModules(moduleIds: string[]) {
for (const moduleId of moduleIds) {
await this.addSketchModule(moduleId)
try {
await this.addSketchModule(moduleId)
} catch (error) {
console.error('Init error in: ' + moduleId, error)
}
}

this.store.setState({ isSketchModulesReady: true })
Expand Down Expand Up @@ -108,6 +112,7 @@ export class HedronEngine {

if (instance.getPasses) {
instance.getPasses(debugScene).forEach((pass) => {
pass.name = sketch.id
debugScene.addPass(pass)
})
}
Expand Down
8 changes: 8 additions & 0 deletions packages/engine/src/world/EngineScene.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,15 @@ export class EngineScene {
this.passes.push(pass)
}

removePass(pass: Pass): void {
this.passes = this.passes.filter((p) => p !== pass)
}

clearPasses(): void {
this.passes = [this.renderPass]
}

getPassesByName(name: string): Pass[] {
return this.passes.filter((pass) => pass.name === name)
}
}
13 changes: 10 additions & 3 deletions packages/engine/src/world/SketchManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,21 @@ export class SketchManager {
}

public removeSketchFromScene = (instanceId: string): void => {
const scene = getDebugScene().scene
const oldSketch = scene.getObjectByName(instanceId)
const engineScene = getDebugScene()
const threeScene = engineScene.scene
const oldSketch = threeScene.getObjectByName(instanceId)

if (!oldSketch) {
const pass = engineScene.getPassesByName(instanceId)
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this method? Can we not just do this.sketchInstances[instanceId].getPasses? Maybe you have a good reason for doing it this way though, curious to know.

if (pass?.length) {
pass.forEach((p) => engineScene.removePass(p))
Copy link
Member

Choose a reason for hiding this comment

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

I think this would also work 😉

Suggested change
pass.forEach((p) => engineScene.removePass(p))
pass.forEach(engineScene.removePass)

delete this.sketchInstances[instanceId]
return
}
throw new Error(`couldn't find sketch to remove: ${instanceId}`)
}

scene.remove(oldSketch)
threeScene.remove(oldSketch)
Comment on lines +46 to +52
Copy link
Member

@funwithtriangles funwithtriangles Dec 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Wondering if we may need to still call threeScene.remove sometimes even when there are passes on the sketch?

Is it not quite likely that some sketches may have both postprocessing and 3D objects? Rather than immediately returning at the end of the passes stuff, I'm wondering if it makes sense to shuffle the logic a bit so the code keeps keeps executing to the end of the method. You could optionally call threeScene.remove(oldSketch). Also wouldn't need to repeat the delete this.sketchInstances[instanceId] as it will just naturally get there anyway.

delete this.sketchInstances[instanceId]
}

Expand Down
Loading