Add Smart Assets module for managing scene asset references#18434
Open
georginahalpern wants to merge 53 commits intoBabylonJS:masterfrom
Open
Add Smart Assets module for managing scene asset references#18434georginahalpern wants to merge 53 commits intoBabylonJS:masterfrom
georginahalpern wants to merge 53 commits intoBabylonJS:masterfrom
Conversation
… fileFormatPlanning
… fileFormatPlanning
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nion-babylon # Conflicts: # packages/tools/devHost/webpack.config.js
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
WebGL2 visualization test reporter: |
Collaborator
|
Visualization tests for WebGPU |
Collaborator
⚡ Performance Test ResultsBaseline: Latest · Candidate: Dev
🔘 Not Significant — p ≥ 0.05 (1)
|
Drop two register-then-load chains in the Inspector that the load
functions already cover via their (managerOrScene, key, url, options)
overload:
- Add Asset and both swap paths now go straight to LoadSmartAssetAsync /
LoadSmartAssetTextureAsync with the URL inline.
- Drop GetOrCreateSmartAssetManager calls that only existed to forward
the manager into a function that accepts the scene directly
(RemoveSmartAssetAsync / ReloadSmartAssetAsync, and every save/load
button in SmartAssetProjectTools).
Drop two unused public exports from @dev/core's SmartAssets surface:
- CreateSmartAssetManager — superseded by GetOrCreateSmartAssetManager.
- ResolveSmartAsset — no production caller; tests covered by the
GetAllSmartAssets path.
End-user call pattern is now just:
await LoadSmartAssetAsync(scene, "chair", "models/chair.glb");
no manager handle required.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Smart Assets API surface had four overlapping ways to get a manager
(GetSmartAssetManagerFromScene, GetOrCreateSmartAssetManager,
CreateSmartAssetManager, and the SmartAssetManagerOrScene union absorbed
by ResolveSmartAssetManager). All converged on the same answer.
Replace them with a single GetSmartAssetManager(scene) that always
returns — auto-creating and attaching one if the scene doesn't already
have one. Every public function now takes Scene only; ResolveSmartAssetManager
and SmartAssetManagerOrScene are gone, CreateSmartAssetManager is now
file-private (used internally by GetSmartAssetManager).
Inspector callers updated: the SmartAsset pane registers eagerly (always
visible, empty when no assets — matches the existing empty-state copy);
the prompt service eagerly resolves the current scene's manager so its
onAssetNotFound hook is bound even if a SAM existed before the inspector
opened. Internal ResolveNotFoundAsync forwards manager.scene when calling
back into the public API.
End-user call pattern is now uniformly:
await LoadSmartAssetAsync(scene, "chair", "chair.glb");
GetSmartAssetManager(scene).onAssetNotFound = async (...) => ...;
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drop 5 exports that no production consumer uses today: - RegisterSmartAsset (LoadSmartAssetAsync auto-registers; LoadSmartAssetMapAsync uses it internally) - LoadAllSmartAssetsAsync (only LoadSmartAssetMapAsync calls it) - DisposeSmartAssetManager (auto-disposed with the scene) - SmartAssetRegistrationOptions type (no external typing usage) - ISerializedSmartAssetEntry type (no external typing usage) The functions and types remain exported from smartAssetManager.ts itself, so internal callers and tests are unaffected — they're just no longer part of the @dev/core public surface. Public surface shrinks from 18 exports to 13. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drop the standalone SetSmartAssetRefreshCallback API and accept the callback as an optional reloadSource field on a new SmartAssetLoadOptions type that LoadSmartAssetAsync / LoadSmartAssetTextureAsync take. The callback was only ever used right after a load to wire up Reload-from-disk for blob-backed assets, so collapsing the two calls is both fewer lines at the call site and removes the misleading reload-vs-refresh terminology mismatch (Reload is the action; reloadSource is the configuration that feeds it). Internal refreshCallbacks map renamed to reloadSources for consistency. Public API surface drops one export (SetSmartAssetRefreshCallback); gains one type export (SmartAssetLoadOptions). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
LoadSmartAssetAsync / LoadSmartAssetTextureAsync now detect when the
caller passes a different URL for an already-loaded key and automatically
discard the stale state before fetching the new URL. Previously the
load functions silently returned the cached AssetContainer (or created a
duplicate texture) and only the URL was updated in the registry — so
LoadSmartAssetAsync(scene, key, newUrl) was effectively a no-op for any
already-loaded key. Now it does the right thing and end users have a
clean update-the-URL path:
await LoadSmartAssetAsync(scene, "chair", "models/chair-v2.glb");
Inspector smart-asset pane now derives its TextureFileAccept array from
GetSmartAssetTextureExtensions() instead of duplicating the same list of
extensions inline.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Smart Assets pane service used a setTimeout-based retry (up to 10 attempts at 16ms each, with version tracking) to handle the case where pane.select() was called before the SidePaneStrip had mounted the new pane. That whole dance is unnecessary because shellService's onSelectSidePane observable is created with notifyIfTriggered = true — it caches the most recent select request and replays it to the strip's useEffect observer when it subscribes (or re-subscribes after a dock change). Drop schedulePaneSelection, the version counter, the selectionRequested flag, and the AddSmartAssetManagerCreatedObserver path that only existed to re-trigger the retry. Pane registration stays eager (already done that way for any active scene); selection is just a synchronous call. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drop the smartAssetsPaneSelection.ts cross-module observable bridge and have the React prompt host call shellService.sidePanes.find().select() directly when the user picks a replacement file. The bridge existed because smartAssetHandler.tsx is a module-level helper exported to the Playground (inspectorV2Module.inspectorAssetNotFoundHandler) and can't take a service-container reference. But the React host inside SmartAssetPromptServiceDefinition's factory already has shellService as a consumed dependency — wrap the host with an onAssetSelected closure that focuses the pane. Same behavior, smaller architecture: - Delete smartAssetsPaneSelection.ts (-37 lines). - inspectorAssetNotFoundHandler is now just RunQueuedMissingAssetPromptAsync. - SmartAssetPromptServiceDefinition drops AddSmartAssetManagerCreatedObserver (eager GetSmartAssetManager already covers the bind-on-create case after the recent core refactor) and the per-manager disposer map. - SmartAssetsServiceDefinition drops Add/Clear/EnableSmartAssetsPaneSelection* imports and the scene-change re-register hook. - Test rewritten to verify just the handler-delegation contract. Net: ~100 LOC, one whole file gone. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
georginahalpern
pushed a commit
to georginahalpern/Babylon.js
that referenced
this pull request
May 8, 2026
Capture the design rules that the SmartAssets review pass had to enforce the hard way (~7 follow-up commits stripping speculative API surface) so future modules don't repeat them. New file: - api-design.instructions.md — public API surface restraint, single entry points per concept, no Register+Load splits, no parallel Refresh/Reload verbs, auto-cleanup on owning-object disposal, closure props over cross-module observable bridges, no setTimeout retry loops around timing races. Cites the SmartAssets module as the worked example. Targeted additions to existing files: - inspector.instructions.md — closure-prop pattern for service factories that need to drive UI from React components, plus a note that IShellService.onSelectSidePane is a sticky observable so a single synchronous pane.select() is race-free. - entities.instructions.md — when caching on scene.metadata, register cleanup on scene.onDisposeObservable at creation time; document the metadata-nulled-before-onDispose-fires ordering gotcha. - performance.instructions.md — Observable<void> with notifyIfTriggered does not replay because the cached value is always undefined; use a non-undefined sentinel type like Observable<true>. (Real bug caught by Copilot review on PR BabylonJS#18434.) - index.md — link to the new api-design file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
georginahalpern
force-pushed
the
smartAssets
branch
from
e2cd0c1 to
3daceb6
Compare
Collaborator
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
The .files copy in GenerateV2Manifest defended against a mutation that
nothing in the save path performs - both BuildPlaygroundManifestPayload
and PackSnippetData treat the manifest as read-only (JSON.stringify).
The vite alias for `inspector: ../../dev/inspector-v2/src` exists
to satisfy the three `import { type X } from \inspector/...\` lines
in rendererComponent.tsx. Those are type-only imports - esbuild
elides them and TypeScript already resolves the specifier via the
root tsconfig's `inspector/*` path mapping. Vite never has to
resolve the module.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
Reverts incidental eslint cleanup that was unrelated to the SmartAssets integration so the diff shows just the feature changes: - Restore the file-level @typescript-eslint/naming-convention and @typescript-eslint/no-floating-promises disables. - Drop newly-added 'void' prefixes on _downloadManager.downloadAsync, _showInspectorAsync, and _compileAndRunAsync (now covered by the file-level disable). - Restore the line-level disable above _compileAndRunAsync. The scene-parameter additions on _showInspectorAsync and _ensureInspectable stay - those are required so the missing-asset prompt can open the Inspector during user createScene execution, before this._scene is assigned at the end of _compileAndRunAsync. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
Collaborator
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
Collaborator
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18434/merge/index.html#WGZLGJ#4600 Links to test your changes to core in the published versions of the Babylon tools (does not contain changes you made to the tools themselves): https://playground.babylonjs.com/?snapshot=refs/pull/18434/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/18434/merge#BCU1XR#0 If you made changes to the sandbox or playground in this PR, additional comments will be generated soon containing links to the dev versions of those tools. |
Collaborator
|
You have changed file(s) that made possible changes to the sandbox. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/SANDBOX/refs/pull/18434/merge/ |
Collaborator
Collaborator
|
You have made possible changes to the playground. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/18434/merge/ The snapshot playground with the CDN snapshot (only when available): Note that neither Babylon scenes nor textures are uploaded to the snapshot directory, so some playgrounds won't work correctly. |
Collaborator
Collaborator
🟢 Memory Leak Test Results13 passed, 0 leaked out of 13 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (13)
|
Collaborator
|
Visualization tests for WebGPU |
Collaborator
|
WebGL2 visualization test reporter: |
Collaborator
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduces Smart Assets, a new
@babylonjs/coremodule for tracking, swapping, and reloading the asset files that back a scene (models, textures, asset maps), plus an Inspector pane and Playground integration that build on it.How this fits into the larger
.babylonprojeffortThis PR is a standalone slice of the broader
.babylonprojwork — a planned zip-based project format that lets a scene round-trip with all its mutable side-channel state (assets, property overrides, edits made in the Inspector) instead of treating each asset file as immutable.The full
.babylonprojwork breaks into two cooperating pieces:Together, Smart Assets + Overrides become the load/save backbone of the
.babylonprojformat. Landing Smart Assets first keeps the change reviewable in isolation and gives the Inspector / Playground a useful asset-management feature today, even before the project-file format ships.What's new
packages/dev/core/src/SmartAssets/— module-level API for aSmartAssetManagerattached to eachScene:AssetContainerand texture objects so they can be located by key lateronAssetNotFoundcallback for missing-file fallbacks (URL orFile)AddSmartAssetManagerCreatedObserverfor cross-cutting subscribers (Playground, Inspector)SerializeSmartAssetManagerMap/LoadSmartAssetMapAsync) for persisting the asset tablepackages/dev/inspector-v2/— Smart Assets pane:packages/tools/playground/— Playground delegates missing-asset prompts to the Inspector, and shows the wait-ring while replacement assets load.packages/dev/sharedUiComponents/src/fluent/primitives/dialog.tsx— new shared Fluent Dialog primitive used by the prompt service.Tests