Examples: the 3D examples await their level load - #1651
Merged
Conversation
The three 3D examples that go through the level director — forest, glTF scene and glTF character — now use the `async` option from #1647 instead of the `onLoaded` callback, so the setup that follows the load reads as ordinary sequential code. `loader.preload` deliberately keeps its callback form. Awaiting it would delay returning the teardown function, and the forest example needs that cleanup to exist while its 3 MB glb is still loading. Also corrects a JSDoc default that shipped in the published types: `LevelLoadOptions.castGroundShadow` was documented as defaulting to `false`, but the option is tri-state — omitting it means "inherit the application setting", which is on by default. The three sibling declarations (Mesh, GLTFScene, GLTFModel) already said so. The emitted type is unchanged; only the prose was wrong, in the direction that makes someone set the flag to `true` believing it is off. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The examples now pass async callbacks to loader.preload, but the loader does not await them, so errors after the first await can surface as unhandled promise rejections unless explicitly handled.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates the 3D level-director examples to use the async: true level.load form (so post-load setup can be written sequentially), and corrects a misleading JSDoc default for LevelLoadOptions.castGroundShadow.
Changes:
- Converted the forest + glTF scene + glTF character examples from
onLoadedtoawait level.load(..., { async: true })followed by explicit setup calls. - Corrected
LevelLoadOptions.castGroundShadowdocumentation to reflect tri-state “omit to inherit app setting” behavior.
File summaries
| File | Description |
|---|---|
| packages/melonjs/src/level/level.js | Fixes castGroundShadow option prose to match tri-state/inheritance behavior. |
| packages/examples/src/examples/gltf/ExampleGltfCharacter.tsx | Switches level load flow to async: true + sequential setup. |
| packages/examples/src/examples/gltf/ExampleGltf.tsx | Switches level load flow to async: true + sequential setup. |
| packages/examples/src/examples/forest/ExampleForest.tsx | Switches level load flow to async: true + sequential setup. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+249
to
+253
| await level.load("forest", { | ||
| scale: SCALE, | ||
| castGroundShadow: true, | ||
| shadowGroundY: GROUND_Y, | ||
| onLoaded: setupScene, | ||
| async: true, |
Comment on lines
+335
to
339
| await level.load("diorama", { | ||
| scale: SCALE, | ||
| castGroundShadow: true, | ||
| onLoaded: setupScene, | ||
| async: true, | ||
| }); |
Comment on lines
+271
to
275
| // actually in the world, so the setup below reads as ordinary | ||
| // sequential code rather than a callback | ||
| await level.load("character", { scale: SCALE, async: true }); | ||
| setupScene(); | ||
| }, |
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.
The three 3D examples that go through the level director — forest, glTF scene and glTF character — now use the
asyncoption from #1647 instead of theonLoadedcallback, so the setup that follows the load reads as ordinary sequential code:The other
Camera3dexamples (afterBurner,billboard,camera3d,materialTextures,nightcity) build their scenes in code and never calllevel.load, so there was nothing to convert. The remaining non-async call sites are all 2D and were left alone.loader.preloaddeliberately keeps its callback form. Awaiting it would delay returning the teardown function, and the forest example needs that cleanup to exist while its 3 MB glb is still loading.Also: a JSDoc default that shipped in the published types
LevelLoadOptions.castGroundShadowwas documented as defaulting tofalse, but the option is tri-state — omitting it means "inherit the application setting", which istrueindefaultApplicationSettings. The three sibling declarations (Mesh,GLTFScene,GLTFModel) already said "omit to inherit"; this was the odd one out.The emitted type is unchanged (
castGroundShadow?: booleaneither way) — only the prose was wrong, in the direction that makes someone set the flag totruebelieving it is off.Verification
tscon the examples package: exit 0No CHANGELOG entry: examples do not go in the engine changelog, and the JSDoc correction is below the bar for something a reader would act on.
🤖 Generated with Claude Code
https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N