From f4c76ed2791eab2bf4cc15ee780dea98c47b3e9f Mon Sep 17 00:00:00 2001 From: Georgina Date: Tue, 12 Aug 2025 08:12:26 -0400 Subject: [PATCH 1/5] pr comments --- .../src/components/properties/particles/attractor.tsx | 9 ++++++--- .../components/properties/particles/attractorList.tsx | 3 +-- .../properties/particles/particleSystemProperties.tsx | 4 +--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/dev/inspector-v2/src/components/properties/particles/attractor.tsx b/packages/dev/inspector-v2/src/components/properties/particles/attractor.tsx index 547398dd25b..3a287c01bca 100644 --- a/packages/dev/inspector-v2/src/components/properties/particles/attractor.tsx +++ b/packages/dev/inspector-v2/src/components/properties/particles/attractor.tsx @@ -69,9 +69,8 @@ export const AttractorComponent: FunctionComponent = (props) => const classes = useAttractorStyles(); const [shown, setShown] = useState(true); - // Create observer and cleanup on unmount (we can't use useResource since Observer is not an IDisposable) + // Create observer and cleanup on unmount via useEffect below (we can't use useResource since Observer is not an IDisposable) const sceneOnAfterRenderObserverRef = useRef>(); - useEffect(() => () => sceneOnAfterRenderObserverRef.current?.remove(), []); // We only want to recreate the impostor mesh and associated if id, scene, or attractor/impostor changes const impostor = useResource(useCallback(() => CreateImpostor(id, scene, attractor, impostorScale, impostorMaterial), [id, scene, attractor])); @@ -87,11 +86,15 @@ export const AttractorComponent: FunctionComponent = (props) => label.render(scene.getViewMatrix(), scene.getProjectionMatrix()); } }); + return () => { + sceneOnAfterRenderObserverRef.current?.remove(); + }; }, [impostor, label, props.impostorColor]); + // If impostor or impostorScale change, update impostor scaling useEffect(() => { impostor.scaling.setAll(impostorScale); - }, [impostorScale]); + }, [impostor, impostorScale]); return (
diff --git a/packages/dev/inspector-v2/src/components/properties/particles/attractorList.tsx b/packages/dev/inspector-v2/src/components/properties/particles/attractorList.tsx index ebc0e489eda..fabcabe9b55 100644 --- a/packages/dev/inspector-v2/src/components/properties/particles/attractorList.tsx +++ b/packages/dev/inspector-v2/src/components/properties/particles/attractorList.tsx @@ -15,7 +15,6 @@ import { useResource } from "../../../hooks/resourceHooks"; import { AttractorComponent } from "./attractor"; type AttractorListProps = { scene: Scene; - gizmoManager: GizmoManager; attractors: Array; system: ParticleSystem; }; @@ -52,7 +51,7 @@ export const AttractorList: FunctionComponent = (props) => { // All impostors share a scale and material/color (for now!) const [impostorScale, setImpostorScale] = useState(1); - const [impostorColor, setImpostorColor] = useState(() => Color3.White()); + const [impostorColor, setImpostorColor] = useState(() => Color3.White()); const impostorMaterial = useResource(useCallback(() => CreateSharedMaterial(scene, impostorColor), [scene])); // All impostors share a gizmoManager. controlledImpostor state ensures re-render of children so that their gizmoEnabled toggle is accurate diff --git a/packages/dev/inspector-v2/src/components/properties/particles/particleSystemProperties.tsx b/packages/dev/inspector-v2/src/components/properties/particles/particleSystemProperties.tsx index 27cd4239fe7..3ca591c85f7 100644 --- a/packages/dev/inspector-v2/src/components/properties/particles/particleSystemProperties.tsx +++ b/packages/dev/inspector-v2/src/components/properties/particles/particleSystemProperties.tsx @@ -1,5 +1,4 @@ import type { FactorGradient, ColorGradient as Color4Gradient, IValueGradient, ParticleSystem } from "core/index"; -import { GizmoManager } from "core/Gizmos"; import { Color3, Color4 } from "core/Maths/math.color"; import { useCallback } from "react"; @@ -72,13 +71,12 @@ export const ParticleSystemColorProperties: FunctionComponent<{ particleSystem: export const ParticleSystemAttractorProperties: FunctionComponent<{ particleSystem: ParticleSystem }> = (props) => { const { particleSystem: system } = props; - const gizmoManager = new GizmoManager(system.getScene()!); const attractors = useParticleSystemProperty(system, "attractors", "property", "addAttractor", "removeAttractor"); return ( <> - + ); }; From 4d9f66ea1eba8b62259868576fe91cd9de798f55 Mon Sep 17 00:00:00 2001 From: Georgina Date: Tue, 12 Aug 2025 16:09:53 -0400 Subject: [PATCH 2/5] fix ref not capturing async val, address comment --- .../properties/particles/attractor.tsx | 12 ++----- .../particles/particleSystemProperties.tsx | 9 +++++- .../inspector-v2/src/hooks/resourceHooks.ts | 31 +++++++++---------- 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/packages/dev/inspector-v2/src/components/properties/particles/attractor.tsx b/packages/dev/inspector-v2/src/components/properties/particles/attractor.tsx index 3a287c01bca..7c2ae54c4e0 100644 --- a/packages/dev/inspector-v2/src/components/properties/particles/attractor.tsx +++ b/packages/dev/inspector-v2/src/components/properties/particles/attractor.tsx @@ -7,10 +7,9 @@ import type { Color3 } from "core/Maths"; import { Color4, Matrix } from "core/Maths"; import type { AbstractMesh } from "core/Meshes"; import { CreateSphere } from "core/Meshes"; -import type { Observer } from "core/Misc"; import type { Attractor } from "core/Particles"; import type { Scene } from "core/scene"; -import { useCallback, useEffect, useRef, useState, type FunctionComponent } from "react"; +import { useCallback, useEffect, useState, type FunctionComponent } from "react"; import { SyncedSliderInput } from "shared-ui-components/fluent/primitives/syncedSlider"; import { ToggleButton } from "shared-ui-components/fluent/primitives/toggleButton"; import { useAsyncResource, useResource } from "../../../hooks/resourceHooks"; @@ -54,7 +53,6 @@ async function CreateTextRendererAsync(scene: Scene, impostor: AbstractMesh, ind textRenderer.addParagraph("#" + index, {}, Matrix.Scaling(0.5, 0.5, 0.5).multiply(Matrix.Translation(0, 1, 0))); textRenderer.isBillboard = true; textRenderer.color = Color4.FromColor3(color, 1.0); - textRenderer.render(scene.getViewMatrix(), scene.getProjectionMatrix()); textRenderer.parent = impostor; return textRenderer; } @@ -69,17 +67,13 @@ export const AttractorComponent: FunctionComponent = (props) => const classes = useAttractorStyles(); const [shown, setShown] = useState(true); - // Create observer and cleanup on unmount via useEffect below (we can't use useResource since Observer is not an IDisposable) - const sceneOnAfterRenderObserverRef = useRef>(); - // We only want to recreate the impostor mesh and associated if id, scene, or attractor/impostor changes const impostor = useResource(useCallback(() => CreateImpostor(id, scene, attractor, impostorScale, impostorMaterial), [id, scene, attractor])); const label = useAsyncResource(useCallback(async () => await CreateTextRendererAsync(scene, impostor, id, props.impostorColor), [scene, impostor, id])); // If impostor, color, or label change, recreate the observer function so that it isnt hooked to old state useEffect(() => { - sceneOnAfterRenderObserverRef.current?.remove(); - sceneOnAfterRenderObserverRef.current = scene.onAfterRenderObservable.add(() => { + const onAfterRender = scene.onAfterRenderObservable.add(() => { attractor.position.copyFrom(impostor.position); if (label) { label.color = Color4.FromColor3(props.impostorColor); @@ -87,7 +81,7 @@ export const AttractorComponent: FunctionComponent = (props) => } }); return () => { - sceneOnAfterRenderObserverRef.current?.remove(); + scene.onAfterRenderObservable.remove(onAfterRender); }; }, [impostor, label, props.impostorColor]); diff --git a/packages/dev/inspector-v2/src/components/properties/particles/particleSystemProperties.tsx b/packages/dev/inspector-v2/src/components/properties/particles/particleSystemProperties.tsx index 3ca591c85f7..af0bbfbbdee 100644 --- a/packages/dev/inspector-v2/src/components/properties/particles/particleSystemProperties.tsx +++ b/packages/dev/inspector-v2/src/components/properties/particles/particleSystemProperties.tsx @@ -8,6 +8,7 @@ import { useInterceptObservable } from "../../../hooks/instrumentationHooks"; import { useObservableState } from "../../../hooks/observableHooks"; import { Color4GradientList, FactorGradientList } from "shared-ui-components/fluent/hoc/gradientList"; import { AttractorList } from "./attractorList"; +import { MessageBar } from "shared-ui-components/fluent/primitives/messageBar"; export const ParticleSystemEmissionProperties: FunctionComponent<{ particleSystem: ParticleSystem }> = (props) => { const { particleSystem: system } = props; @@ -73,10 +74,16 @@ export const ParticleSystemAttractorProperties: FunctionComponent<{ particleSyst const { particleSystem: system } = props; const attractors = useParticleSystemProperty(system, "attractors", "property", "addAttractor", "removeAttractor"); + const scene = system.getScene(); return ( <> - + {scene ? ( + + ) : ( + // Should never get here since sceneExplorer only displays if there is a scene, but adding UX in case that assumption changes in future + + )} ); }; diff --git a/packages/dev/inspector-v2/src/hooks/resourceHooks.ts b/packages/dev/inspector-v2/src/hooks/resourceHooks.ts index 3b566466cc1..e422d09694c 100644 --- a/packages/dev/inspector-v2/src/hooks/resourceHooks.ts +++ b/packages/dev/inspector-v2/src/hooks/resourceHooks.ts @@ -1,6 +1,6 @@ import type { IDisposable } from "core/index"; -import { useRef, useEffect } from "react"; +import { useRef, useEffect, useState } from "react"; /** * Custom hook to manage a resource with automatic disposal. The resource is created once initially, and recreated @@ -44,46 +44,43 @@ export function useResource(factory: () => T): T { * @returns The created resource. */ export function useAsyncResource(factory: (abortSignal: AbortSignal) => Promise): T | undefined { - const resourceRef = useRef(); + const [resource, setResource] = useState(); const factoryRef = useRef(factory); // Update refs to capture latest values factoryRef.current = factory; useEffect(() => { - const abortController = new AbortController(); // Create AbortController - const currentResource: T | undefined = resourceRef.current; + const abortController = new AbortController(); + let isMounted = true; // Dispose old resource if it exists - currentResource?.dispose(); - resourceRef.current = undefined; + resource?.dispose(); + setResource(undefined); // Create new resource void (async () => { try { - const newVal = await factory(abortController.signal); // Pass the signal - if (!abortController.signal.aborted) { - resourceRef.current = newVal; + const newVal = await factory(abortController.signal); + if (isMounted && !abortController.signal.aborted) { + setResource(newVal); // This will trigger a re-render so the new resource is returned to caller } else { newVal.dispose(); } } catch (error) { - // Handle abortion gracefully if (error instanceof Error && error.name === "AbortError") { - // Request was aborted, this is expected return; } - // Optionally handle other errors here - global.console.error("Failed to create async resource:", error); } })(); return () => { - abortController.abort(); // Abort the operation - resourceRef.current?.dispose(); - resourceRef.current = undefined; + isMounted = false; + abortController.abort(); + resource?.dispose(); + setResource(undefined); }; }, [factory]); - return resourceRef.current; + return resource; } From bbc170f747bb66fb91ee7dd11915ba975f0cc395 Mon Sep 17 00:00:00 2001 From: Georgina Date: Tue, 12 Aug 2025 16:41:20 -0400 Subject: [PATCH 3/5] comments/cleanup --- .../properties/particles/attractor.tsx | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/dev/inspector-v2/src/components/properties/particles/attractor.tsx b/packages/dev/inspector-v2/src/components/properties/particles/attractor.tsx index 7c2ae54c4e0..3a50d87de05 100644 --- a/packages/dev/inspector-v2/src/components/properties/particles/attractor.tsx +++ b/packages/dev/inspector-v2/src/components/properties/particles/attractor.tsx @@ -45,12 +45,12 @@ const CreateImpostor = (id: number, scene: Scene, attractor: Attractor, initialS return impostor; }; -async function CreateTextRendererAsync(scene: Scene, impostor: AbstractMesh, index: number, color: Color3) { +async function CreateTextRendererAsync(id: number, scene: Scene, impostor: AbstractMesh, color: Color3) { const sdfFontDefinition = await (await fetch("https://assets.babylonjs.com/fonts/roboto-regular.json")).text(); const fontAsset = new FontAsset(sdfFontDefinition, "https://assets.babylonjs.com/fonts/roboto-regular.png"); const textRenderer = await TextRenderer.CreateTextRendererAsync(fontAsset, scene.getEngine()); - textRenderer.addParagraph("#" + index, {}, Matrix.Scaling(0.5, 0.5, 0.5).multiply(Matrix.Translation(0, 1, 0))); + textRenderer.addParagraph("#" + id, {}, Matrix.Scaling(0.5, 0.5, 0.5).multiply(Matrix.Translation(0, 1, 0))); textRenderer.isBillboard = true; textRenderer.color = Color4.FromColor3(color, 1.0); textRenderer.parent = impostor; @@ -63,27 +63,27 @@ async function CreateTextRendererAsync(scene: Scene, impostor: AbstractMesh, ind * @returns */ export const AttractorComponent: FunctionComponent = (props) => { - const { attractor, id, impostorScale, impostorMaterial, scene } = props; + const { attractor, id, impostorScale, impostorMaterial, impostorColor, scene, onControl, isControlled } = props; const classes = useAttractorStyles(); const [shown, setShown] = useState(true); // We only want to recreate the impostor mesh and associated if id, scene, or attractor/impostor changes const impostor = useResource(useCallback(() => CreateImpostor(id, scene, attractor, impostorScale, impostorMaterial), [id, scene, attractor])); - const label = useAsyncResource(useCallback(async () => await CreateTextRendererAsync(scene, impostor, id, props.impostorColor), [scene, impostor, id])); + const label = useAsyncResource(useCallback(async () => await CreateTextRendererAsync(id, scene, impostor, impostorColor), [id, scene, impostor])); // If impostor, color, or label change, recreate the observer function so that it isnt hooked to old state useEffect(() => { const onAfterRender = scene.onAfterRenderObservable.add(() => { attractor.position.copyFrom(impostor.position); if (label) { - label.color = Color4.FromColor3(props.impostorColor); + label.color = Color4.FromColor3(impostorColor); label.render(scene.getViewMatrix(), scene.getProjectionMatrix()); } }); return () => { - scene.onAfterRenderObservable.remove(onAfterRender); + onAfterRender.remove(); }; - }, [impostor, label, props.impostorColor]); + }, [impostor, scene, label, impostorColor]); // If impostor or impostorScale change, update impostor scaling useEffect(() => { @@ -106,8 +106,8 @@ export const AttractorComponent: FunctionComponent = (props) => props.onControl(control ? impostor : undefined)} + value={isControlled(impostor)} + onChange={(control: boolean) => onControl(control ? impostor : undefined)} />
); From 90b22aa2c3f8a6ef135f0c3a7d2244b1818bf8af Mon Sep 17 00:00:00 2001 From: Georgina Date: Tue, 12 Aug 2025 16:42:52 -0400 Subject: [PATCH 4/5] remove ismounted check --- packages/dev/inspector-v2/src/hooks/resourceHooks.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/dev/inspector-v2/src/hooks/resourceHooks.ts b/packages/dev/inspector-v2/src/hooks/resourceHooks.ts index e422d09694c..7a6b404315c 100644 --- a/packages/dev/inspector-v2/src/hooks/resourceHooks.ts +++ b/packages/dev/inspector-v2/src/hooks/resourceHooks.ts @@ -52,7 +52,6 @@ export function useAsyncResource(factory: (abortSignal: A useEffect(() => { const abortController = new AbortController(); - let isMounted = true; // Dispose old resource if it exists resource?.dispose(); @@ -62,7 +61,7 @@ export function useAsyncResource(factory: (abortSignal: A void (async () => { try { const newVal = await factory(abortController.signal); - if (isMounted && !abortController.signal.aborted) { + if (!abortController.signal.aborted) { setResource(newVal); // This will trigger a re-render so the new resource is returned to caller } else { newVal.dispose(); @@ -75,7 +74,6 @@ export function useAsyncResource(factory: (abortSignal: A })(); return () => { - isMounted = false; abortController.abort(); resource?.dispose(); setResource(undefined); From 7d9e539f4ccd00659fb3542943f99b3801d33a54 Mon Sep 17 00:00:00 2001 From: Georgina Date: Tue, 12 Aug 2025 19:42:42 -0400 Subject: [PATCH 5/5] fix type import --- .../src/components/properties/particles/attractor.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/dev/inspector-v2/src/components/properties/particles/attractor.tsx b/packages/dev/inspector-v2/src/components/properties/particles/attractor.tsx index 3a50d87de05..006b76b026f 100644 --- a/packages/dev/inspector-v2/src/components/properties/particles/attractor.tsx +++ b/packages/dev/inspector-v2/src/components/properties/particles/attractor.tsx @@ -9,7 +9,8 @@ import type { AbstractMesh } from "core/Meshes"; import { CreateSphere } from "core/Meshes"; import type { Attractor } from "core/Particles"; import type { Scene } from "core/scene"; -import { useCallback, useEffect, useState, type FunctionComponent } from "react"; +import { useCallback, useEffect, useState } from "react"; +import type { FunctionComponent } from "react"; import { SyncedSliderInput } from "shared-ui-components/fluent/primitives/syncedSlider"; import { ToggleButton } from "shared-ui-components/fluent/primitives/toggleButton"; import { useAsyncResource, useResource } from "../../../hooks/resourceHooks";