From c3462c4080c1626071bc7135672ba05afaef5f14 Mon Sep 17 00:00:00 2001 From: ICOM725 <113233781+ICOM725@users.noreply.github.com> Date: Mon, 7 Sep 2026 20:10:55 +0800 Subject: [PATCH] fix: forward glTF load options from triggers --- packages/melonjs/CHANGELOG.md | 1 + packages/melonjs/src/renderable/trigger.js | 12 ++++ .../tests/trigger_level_change.spec.js | 58 ++++++++++++++++++- 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/packages/melonjs/CHANGELOG.md b/packages/melonjs/CHANGELOG.md index 28d38d813..dbacd8a15 100644 --- a/packages/melonjs/CHANGELOG.md +++ b/packages/melonjs/CHANGELOG.md @@ -12,6 +12,7 @@ - Docs: the API reference carries the engine's own identity — logo, brand palette and favicon — and the header links out to the site, the wiki, the repository and Discord. A **Copy page** control hands the page you are reading to an assistant: it copies the page as Markdown with its canonical URL attached, or opens it directly in a chat. The landing page also gained a short section on using the reference with an AI assistant ### Fixed +- Trigger: forward glTF scene options to `level.load()` so scale, coordinate conversion, lights and ground-shadow settings are not silently ignored ([#1649](https://github.com/melonjs/melonJS/issues/1649)). - Level: `level.reload()` was documented as returning `object` — "the current level" — but it returns whatever `level.load()` returns, which is `true`. The declared type has been wrong for the method's whole life: the 2011 original returned nothing at all. `getCurrentLevel()` is the call that hands back the level object. This corrects the emitted type from `object` to `boolean`, so a `const lvl: object = level.reload()` that compiled while receiving `true` now fails to compile, at the site that was already wrong - Lit meshes: specular highlights sat in the wrong place under a scaled ancestor ([#1636](https://github.com/melonjs/melonJS/issues/1636)). The camera position was derived from the view as `-Rᵀ·t`, which is only the right point when the upper 3×3 is orthonormal — and `Container.draw` folds every ancestor into that matrix. It is now the translation column of the view's inverse - Lit meshes: specular lighting, and a mesh's alpha-map cutout, were wrong on whichever tier drew second in a frame. The instanced and non-instanced tiers are two programs sharing one batcher, and its skip-the-redundant-upload cache was not dropped when the program changed under it — so an instanced set behind a lit prop at the same shininess lost its specular outright, and instanced foliage rendered as opaque rectangles. Present since 20.0.0 diff --git a/packages/melonjs/src/renderable/trigger.js b/packages/melonjs/src/renderable/trigger.js index e4afaf520..726c288df 100644 --- a/packages/melonjs/src/renderable/trigger.js +++ b/packages/melonjs/src/renderable/trigger.js @@ -36,6 +36,12 @@ export default class Trigger extends Renderable { * @param {Function} [settings.onLoaded] - Level loaded callback. See {@link level.load} * @param {boolean} [settings.flatten] - Flatten all objects into the target container. See {@link level.load} * @param {boolean} [settings.setViewportBounds] - Resize the viewport to match the level. See {@link level.load} + * @param {number} [settings.scale] - Pixels per glTF unit. See {@link level.load} + * @param {boolean} [settings.rightHanded] - Convert the glTF scene to the engine's Y-down coordinates. See {@link level.load} + * @param {boolean} [settings.lights] - Load the glTF scene's lights. See {@link level.load} + * @param {number} [settings.lightIntensityScale] - Scale the glTF lights' authored intensity. See {@link level.load} + * @param {boolean} [settings.castGroundShadow] - Enable ground shadows for glTF meshes. See {@link level.load} + * @param {number} [settings.shadowGroundY] - World Y for glTF ground shadows. See {@link level.load} * @example * // fade transition (default) * world.addChild(new Trigger(x, y, { @@ -96,6 +102,12 @@ export default class Trigger extends Renderable { "onLoaded", "flatten", "setViewportBounds", + "scale", + "rightHanded", + "lights", + "lightIntensityScale", + "castGroundShadow", + "shadowGroundY", "to", ]) { if (typeof settings[property] !== "undefined") { diff --git a/packages/melonjs/tests/trigger_level_change.spec.js b/packages/melonjs/tests/trigger_level_change.spec.js index 1833b24b2..87d657d98 100644 --- a/packages/melonjs/tests/trigger_level_change.spec.js +++ b/packages/melonjs/tests/trigger_level_change.spec.js @@ -30,6 +30,7 @@ import state from "../src/state/state.ts"; describe("Trigger level change (#1646)", () => { let app; let loaded; + let loadOptions; let originalAddTo; beforeAll(async () => { @@ -41,8 +42,9 @@ describe("Trigger level change (#1646)", () => { }); await app.init(); originalAddTo = GLTFScene.prototype.addTo; - GLTFScene.prototype.addTo = function (container) { + GLTFScene.prototype.addTo = function (container, options) { loaded.push(container); + loadOptions.push(options); }; level.add("gltf", "trigger-target"); }); @@ -54,6 +56,7 @@ describe("Trigger level change (#1646)", () => { beforeEach(() => { loaded = []; + loadOptions = []; state.stop(); }); @@ -88,6 +91,40 @@ describe("Trigger level change (#1646)", () => { app.world.removeChildNow(t); }); + it("forwards glTF load options, including false and zero values", () => { + const options = { + scale: 50, + rightHanded: false, + lights: false, + lightIntensityScale: 0, + castGroundShadow: false, + shadowGroundY: 0, + }; + const t = trigger(options); + t.triggerEvent(); + expect(loadOptions).toHaveLength(1); + expect(loadOptions[0]).toMatchObject(options); + app.world.removeChildNow(t); + }); + + it("leaves omitted glTF options unset and ignores an authored async flag", () => { + const t = trigger({ async: true, scale: undefined }); + t.triggerEvent(); + expect(loadOptions).toHaveLength(1); + for (const key of [ + "scale", + "rightHanded", + "lights", + "lightIntensityScale", + "castGroundShadow", + "shadowGroundY", + "async", + ]) { + expect(loadOptions[0]).not.toHaveProperty(key); + } + app.world.removeChildNow(t); + }); + it("does NOT overwrite the caller's onLoaded on the transition path", () => { // The regression this refactor exists to remove. The old code did // `settings.onLoaded = function (…) { …reveal…; userOnLoaded.call(…) }`, @@ -136,7 +173,20 @@ describe("Trigger level change (#1646)", () => { // with the loop RUNNING, so the load genuinely defers — with it stopped // the load is synchronous and the ordering below proves nothing state.restart(); - const t = trigger({ color: "#000000", duration: 10 }); + const options = { + scale: 50, + rightHanded: false, + lights: false, + lightIntensityScale: 0.001, + castGroundShadow: true, + shadowGroundY: -10, + }; + const t = trigger({ + color: "#000000", + duration: 10, + async: false, + ...options, + }); t.triggerEvent(); // the hide effect, captured rather than added @@ -145,9 +195,10 @@ describe("Trigger level change (#1646)", () => { // swap the viewport while the load runs, as `game.reset()` would const previousAddTo = GLTFScene.prototype.addTo; - GLTFScene.prototype.addTo = function (container) { + GLTFScene.prototype.addTo = function (container, settings) { app.viewport = swapped; loaded.push(container); + loadOptions.push(settings); }; // Drive the hide tween to completion -> onComplete -> the load. Stop @@ -177,6 +228,7 @@ describe("Trigger level change (#1646)", () => { // the load happened, then the reveal — and on the viewport that existed // AFTER the load, not the one captured before it expect(loaded).toHaveLength(1); + expect(loadOptions[0]).toMatchObject({ ...options, async: true }); expect(seen).toHaveLength(2); expect(seen[1].loadedSoFar).toBe(1); expect(seen[1].who).toBe("swapped");