diff --git a/packages/melonjs/CHANGELOG.md b/packages/melonjs/CHANGELOG.md index 5712fead3..45c703f30 100644 --- a/packages/melonjs/CHANGELOG.md +++ b/packages/melonjs/CHANGELOG.md @@ -25,6 +25,7 @@ - UI: `UITextButton#bitmapText`, for `label` ### Fixed +- Camera: `fadeIn()` / `fadeOut()` (and a `FadeEffect`, so the fade state transition and a fading level trigger too) always call `onComplete`. The fade counted as done once its colour's alpha read as the target, which a 32-bit float can do one step before the tween ends; the camera then removed the effect, stopping the tween before it called `onComplete`, so code waiting on it stayed stuck. The fade now ends when its tween does ([#1810](https://github.com/melonjs/melonJS/issues/1810)) - Rendering: on WebGL and WebGPU, a clip inside a clip is the overlap of the two, as it already was on Canvas. A clipping container nested in another, or a scrolling `UIPanel` in a clipping parent, drew past the outer clip wherever it stuck out of it, because the inner `clipRect` replaced the outer scissor ([#1816](https://github.com/melonjs/melonJS/issues/1816)). The clipping example shows the case - UI: a radio group holds one choice from when its checkboxes are made, not only once they are in the scene: before, several could be checked at once until they were added, and `UICheckbox.checkedIn()` answered `undefined`. A checkbox leaves its group when destroyed; groups hold their members weakly, so one dropped without being destroyed can still be collected - UI: `UICheckbox#toggle()` on a checked radio button leaves it checked, as a click does, instead of leaving its group with nothing checked diff --git a/packages/melonjs/src/camera/effects/fade_effect.ts b/packages/melonjs/src/camera/effects/fade_effect.ts index ccd114cd5..c3f7734cc 100644 --- a/packages/melonjs/src/camera/effects/fade_effect.ts +++ b/packages/melonjs/src/camera/effects/fade_effect.ts @@ -41,7 +41,7 @@ export default class FadeEffect extends CameraEffect { direction: "in" | "out"; /** - * target alpha value for completion check + * the alpha the fade ends on * @ignore * @internal */ @@ -103,10 +103,12 @@ export default class FadeEffect extends CameraEffect { } override update(): void { - // check if the tween has reached its target - if (this.direction === "in" && this.color.alpha >= this._targetAlpha) { - this.isComplete = true; - } else if (this.direction === "out" && this.color.alpha <= 0.0) { + // Over when the tween is: it has run its course and called + // `onComplete`. Not when the alpha reads as the target: the colour + // stores it as a 32-bit float, so a value one step short of the end + // can already round to it, and the camera then destroyed the effect, + // stopping the tween before it ever called `onComplete` (#1810). + if (this.tween !== undefined && !this.tween._isRunning) { this.isComplete = true; } } diff --git a/packages/melonjs/tests/camera.spec.js b/packages/melonjs/tests/camera.spec.js index f63dd8846..d3f955eb1 100644 --- a/packages/melonjs/tests/camera.spec.js +++ b/packages/melonjs/tests/camera.spec.js @@ -1,4 +1,4 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { Application, boot, @@ -1473,6 +1473,57 @@ describe("Camera2d", () => { expect(called).toEqual(true); }); + it("a fade calls onComplete before the camera removes it, whatever the float rounding (#1810)", () => { + // frame by frame as the game loop runs it: the tween steps, then the + // camera updates its effects. The alpha is stored as a 32-bit float, + // so on some durations it reads as the target one step before the + // tween ends; the effect must still wait for the tween + const frame = 1000 / 60; + const missed = []; + for (const direction of ["in", "out"]) { + for (let duration = 100; duration <= 1000; duration += 25) { + const { camera } = setup(); + const onComplete = vi.fn(); + const fade = new FadeEffect(camera, { + color: "#ffffff", + duration, + direction, + onComplete, + }); + camera.addCameraEffect(fade); + const tween = fade.tween; + for (let i = 0; i < 200 && camera.getCameraEffect(FadeEffect); i++) { + tween.update(frame); + camera.update(frame); + } + expect(camera.getCameraEffect(FadeEffect)).toBeUndefined(); + if (onComplete.mock.calls.length !== 1) { + missed.push(`${direction} ${duration}ms`); + } + } + } + expect(missed).toEqual([]); + }); + + it("a fade stays until its tween ends, even once its alpha reads as the target", () => { + const { camera } = setup(); + const onComplete = vi.fn(); + const fade = camera.addCameraEffect( + new FadeEffect(camera, { color: "#000", duration: 300, onComplete }), + ); + // the colour already at the target, the tween not yet over + fade.color.alpha = 1; + camera.update(16); + expect(camera.getCameraEffect(FadeEffect)).toBe(fade); + expect(onComplete).not.toHaveBeenCalled(); + // to its end exactly: a finished tween leaves the game loop, so + // it is not stepped again + advanceTween(fade.tween, 300); + camera.update(16); + expect(onComplete).toHaveBeenCalledTimes(1); + expect(camera.getCameraEffect(FadeEffect)).toBeUndefined(); + }); + it("fadeIn() convenience method should add a FadeEffect", () => { const { camera } = setup(); camera.fadeIn("#000", 500); diff --git a/packages/melonjs/tests/fade-oncomplete.spec.js b/packages/melonjs/tests/fade-oncomplete.spec.js new file mode 100644 index 000000000..eecfe8712 --- /dev/null +++ b/packages/melonjs/tests/fade-oncomplete.spec.js @@ -0,0 +1,117 @@ +/** + * `viewport.fadeIn()` / `fadeOut()` always call `onComplete` (#1810). + * + * Driven the way a game runs: each frame is a `TICK`, as the browser loop + * emits it. The application updates first (its stage updates the camera, + * which removes the effects that are complete), then the fade's tween steps. + * + * A fade used to count as complete once its colour's alpha read as the + * target. The alpha is a 32-bit float, so on some durations it rounds to + * the target one step before the tween ends; the camera then removed the + * fade and stopped the tween before it called `onComplete`. Code waiting on + * the callback, a scene change say, never ran. + */ +import { beforeAll, describe, expect, it, vi } from "vitest"; +import { Application, boot, event, FadeEffect, video } from "../src/index.js"; + +const FRAME = 1000 / 60; + +let app; +// the clock the frames are stamped with, moved on by every frame +let now; + +/** run one frame of the game loop at 60 fps */ +const tick = () => { + now += FRAME; + event.emit(event.TICK, now); +}; + +/** + * run frames until the camera has no fade left, or a limit + * @returns the number of frames run + */ +const runFade = () => { + let frames = 0; + while (app.viewport.getCameraEffect(FadeEffect) && frames < 400) { + tick(); + frames++; + } + return frames; +}; + +describe("viewport.fadeIn() / fadeOut() call onComplete (#1810)", () => { + beforeAll(async () => { + boot(); + app = new Application(320, 240, { + parent: "screen", + renderer: video.CANVAS, + consoleHeader: false, + }); + await app.init(); + now = globalThis.performance.now(); + }); + + it("fadeIn calls onComplete once, then the fade is gone, at every duration", () => { + const missed = []; + for (let duration = 100; duration <= 1000; duration += 25) { + const onComplete = vi.fn(); + app.viewport.fadeIn("#000000", duration, onComplete); + runFade(); + expect(app.viewport.getCameraEffect(FadeEffect)).toBeUndefined(); + if (onComplete.mock.calls.length !== 1) { + missed.push(duration); + } + } + expect(missed).toEqual([]); + }); + + it("fadeOut calls onComplete once, then the fade is gone, at every duration", () => { + const missed = []; + for (let duration = 100; duration <= 1000; duration += 25) { + const onComplete = vi.fn(); + app.viewport.fadeOut("#000000", duration, onComplete); + runFade(); + expect(app.viewport.getCameraEffect(FadeEffect)).toBeUndefined(); + if (onComplete.mock.calls.length !== 1) { + missed.push(duration); + } + } + expect(missed).toEqual([]); + }); + + it("the issue's case: a scene change chained on fadeIn callbacks reaches its end", () => { + // as the Alex 4 outro moved from scene to scene in a fadeIn callback + const scenes = []; + const next = (n) => { + scenes.push(n); + if (n < 4) { + app.viewport.fadeIn("#000000", 300, () => { + next(n + 1); + }); + } + }; + next(1); + for (let i = 0; i < 4 && app.viewport.getCameraEffect(FadeEffect); i++) { + runFade(); + } + expect(scenes).toEqual([1, 2, 3, 4]); + }); + + it("the fade ends on the frame after its tween does, not before", () => { + const onComplete = vi.fn(); + app.viewport.fadeIn("#000000", 300, onComplete); + const fade = app.viewport.getCameraEffect(FadeEffect); + let frames = 0; + while (!onComplete.mock.calls.length && frames < 400) { + // still there for every frame before the callback + expect(app.viewport.getCameraEffect(FadeEffect)).toBe(fade); + tick(); + frames++; + } + expect(onComplete).toHaveBeenCalledTimes(1); + // 300 ms is 18 frames at 60 fps + expect(frames).toBeGreaterThanOrEqual(18); + tick(); + expect(app.viewport.getCameraEffect(FadeEffect)).toBeUndefined(); + }); +});