Level: an async option on load/reload/next/previous, and a microtask deferral

[obiot]

Closes #1646.

The timer

level.load() deferred its work with a timer so the current frame could unwind before the world is reset. The deferral is still needed — it is routinely called from a trigger handler mid-loop, and safeLoadLevel resets and destroys the very container the loop may be iterating, while state.stop() only sets a flag.

The timer itself is a 2011 artefact: that line and its comment trace to v0.9.0, four years before promises existed. Browsers clamp a timer to ≥1 s in a background tab, so a load queued as the tab hides sat behind that clamp. A microtask drains when the JS stack empties — the end of the rAF callback holding update and draw — so it unwinds the frame identically and is not clamped.

The no-loop branch stays synchronous exactly as before.

The async option

load, reload, next and previous each keep one name and gain a flag:

level.load("map1");                          // boolean, exactly as before
await level.load("map1", { async: true });   // Promise<boolean>
if (await level.next({ async: true })) { … }

options.onLoaded still fires either way, so the forms mix.

No type break. The signatures are preserved as JSDoc @overload pairs, not a boolean | Promise<boolean> union — a union fails every existing const ok: boolean = level.load(id) with TS2322, which I verified before choosing. The overload form compiles both that and the awaited call against the real emitted build:

const ok: boolean = level.load("map1");                       // still compiles
const a: boolean = await level.load("map1", { async: true });  // Promise<boolean>

Running out of levels still reports false rather than rejecting — reaching the end of a game is an ordinary outcome. An unknown level id throws synchronously in both forms: that is a typo, not a load failure, and it should not need await to surface.

The bounds check next and previous each spelled out is now a shared levelIdAt(offset) helper, so the two cannot drift.

Trigger stops rewriting its caller's options

The fade/mask path sequenced hide → load → reveal by replacing settings.onLoaded with its own function and calling the user's from inside — mutating an option object the caller owns. Awaiting the load removes the interception.

The viewport is deliberately re-read after the load: game.reset() reassigns app.viewport, which is precisely why the callback this replaces read it late.

The cost of putting the switch in the options

await level.load(id) without the flag is silent, because await true is valid JavaScript. It happens to be harmless today — the deferral is a single microtask queued before the await's continuation, so the load still runs first — but that is incidental ordering, not a contract. Documented on the LevelLoadOptions typedef and pinned by a test that records exactly this.

Tests

No spec called level.load() at all before this, so both files are new — 24 tests across the legacy contract, the flag, the scheduling, the reload/next/previous paths, and the trigger.

Nine mutations of the changed behaviour fail as they should: the flag ignored, the microtask reverted to a timer, the unknown id rejecting instead of throwing, the loop not stopped, the synchronous branch throwing instead of rejecting, next/previous off-by-one, the onLoaded wrap reintroduced, and a stale viewport captured before the load.

A tenth (rewriting the bounds helper as levelIdx[index] ?? null) survives, and is an equivalent mutant rather than a gap: out-of-range array access is already undefined, so the explicit bounds check is belt-and-braces. The observable behaviour is covered by the two off-by-one mutations.

The viewport guard is a source check rather than a behavioural test — the reveal only runs when the hide tween completes, which needs a live game loop the suite does not have. It was vacuous on the first attempt (the explanatory comment in the inspected slice contained the string it asserted on), so comment lines are stripped before matching, and it was re-verified against the mutation.

Also

reload() was documented as returning object — "the current level" — but returns whatever load() returns. The 2011 original returned nothing at all, so the declaration was never right. Corrected to boolean; getCurrentLevel() is the call that hands back the level object.

Verification

276 files, 6716 tests, 0 failures. Lint 0 errors, build clean, emitted overloads and legacy compatibility both checked against the built types.

[Copilot]

🟡 Changes recommended

There are a few correctness/docs issues to address (notably loadAsync() error-surface consistency and a non-Markdown {@link ...} tag in the changelog).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR modernizes level-loading scheduling by replacing the legacy setTimeout deferral with a microtask-based deferral, and introduces an awaitable level.loadAsync() to let callers observe completion without changing the existing level.load() boolean-return contract. It also updates Trigger’s level-transition path to avoid mutating caller-owned settings.onLoaded, and adds new Vitest coverage for the behavior and scheduling.

Changes:

• Add level.loadAsync(levelId, options): Promise<void> while keeping level.load()’s boolean return and “fire-and-forget” behavior.
• Replace timer-based deferral with a microtask deferral when the game loop is running; preserve synchronous load behavior when no loop is running.
• Refactor trigger level transitions to await the load (via loadAsync) instead of wrapping/overwriting settings.onLoaded, and add new tests.

File summaries

File	Description
packages/melonjs/src/level/level.js	Adds loadAsync(), refactors load() to delegate and rethrow failures, and replaces timer deferral with a microtask deferral when the loop is running.
packages/melonjs/src/renderable/trigger.js	Updates transition path to use level.loadAsync(...).then(...) for reveal instead of rewriting settings.onLoaded.
packages/melonjs/tests/level_load_async.spec.js	New tests for loadAsync, legacy load() contract, and microtask-vs-timer scheduling behavior.
packages/melonjs/tests/trigger_level_change.spec.js	New tests ensuring trigger transition behavior doesn’t overwrite caller callbacks and that load is deferred until hide completes.
packages/melonjs/CHANGELOG.md	Documents the new API and the background-tab timer clamp fix.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 4
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[obiot]

Added the three missing twins — reloadAsync(), nextAsync(), previousAsync(). loadAsync() alone left a game able to await its first level but not a reload or a level transition.

Each resolves with exactly what its synchronous counterpart returns, so a port is mechanical:

if (level.next())              →   if (await level.nextAsync())

That is why loadAsync() now resolves true rather than void — the rule is worth more than the marginally noisier type.

Running out of levels resolves false without loading anything rather than rejecting. next() returns false there, and reaching the end of a game is an ordinary outcome, not an error.

Emitted types, verified from build/level/level.d.ts — all four originals unchanged:

load	boolean	loadAsync → Promise<boolean>
reload	object	reloadAsync → Promise<boolean>
next	boolean	nextAsync → Promise<boolean>
previous	boolean	previousAsync → Promise<boolean>

Six more tests, and five mutations of the new behaviour all fail as they should: nextAsync resolving true at the end, loading past the end, a previousAsync off-by-one at the start, reloadAsync not reloading, and loadAsync resolving undefined.

Noticed but deliberately not fixed: reload() declares object above, from a stale @returns {object} the current level — it actually returns whatever load() returns. Correcting it would change an emitted type, which is precisely what this PR is careful not to do, so it is left for a separate call.

Suite now 276 files, 6713 tests, 0 failures.

[Copilot]

🟡 Changes recommended

It introduces unconditional queueMicrotask usage (risking runtime ReferenceError in unsupported environments) and the PR description’s stated scope conflicts with the included async twin APIs.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

packages/melonjs/src/level/level.js:198

• load() rethrows rejections using queueMicrotask, but this global is not guaranteed to exist in all runtimes. If it’s missing, a rejected loadAsync() will cause a ReferenceError here instead of surfacing the original failure. Consider a small fallback to setTimeout when queueMicrotask is unavailable.

	load(levelId, options) {
		// Fire-and-forget by contract: this returns `true`, not the promise, so
		// existing (including typed) callers are unaffected. Use `loadAsync()`
		// to await the load. The rejection is rethrown on a clean stack so a
		// failure still surfaces as an uncaught error the way it did when the
		// deferral was a timer, rather than as a silent unhandled rejection.
		this.loadAsync(levelId, options).catch((error) => {
			queueMicrotask(() => {
				throw error;
			});
		});

packages/melonjs/src/renderable/trigger.js:209

• This catch path rethrows via queueMicrotask, which may be undefined in some runtimes; in that case the code would throw a ReferenceError and potentially mask the real level-load failure. Using a simple fallback (e.g. setTimeout) keeps the intended “uncaught” loudness without requiring queueMicrotask support.

							})
							.catch((error) => {
								// same loudness as the fire-and-forget form
								queueMicrotask(() => {
									throw error;
								});
							});

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

It changes core level-loading scheduling and trigger transition sequencing in a way that can have subtle runtime/event-loop effects best validated by a human reviewer.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

level.load() now forces some previously synchronous failures (when the loop isn’t running) to become asynchronous throws via loadAsync().catch(...), which is a behavior/contract change that should be addressed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

[obiot]

Responses to the review, plus a design change since it was written.

queueMicrotask → Promise.resolve().then(() => { throw error }) (two comments): declining. The two are not equivalent for this purpose, measured in the browser:

queueMicrotask(() => { throw e })          → ["uncaught-error"]
Promise.resolve().then(() => { throw e })  → ["unhandled-rejection"]

That rethrow exists specifically to preserve the uncaught error the timer-based version produced. The suggested form would turn it into an unhandled rejection, which is the behaviour change the code is there to avoid. On availability: queueMicrotask is baseline in every browser since 2018 and Node 11, comfortably older than the ES2022 target this package already requires.

loadAsync() error-surface inconsistency: valid, fixed. The deferred branch produced a rejection while the synchronous one let the exception escape the call, so the surface depended on state.isRunning() — and .catch() could never see the synchronous case, since the throw beat the handler being attached. The synchronous branch now returns a rejected promise, with a test asserting both branches reject identically. Good catch.

{@link ...} in the changelog: already gone — the entry had been rewritten before the review ran.

PR description out of step with scope: it was, and it is now further out of step, because the API changed after this review. The description has been rewritten.

Design change: the four *Async siblings are gone. Eight loading methods for four operations was too much surface, so the switch moved into the options object: level.load(id, { async: true }), and the same on reload/next/previous.

The signatures are preserved as JSDoc @overload pairs rather than a boolean | Promise<boolean> union — the union form fails every existing const ok: boolean = level.load(id) with TS2322, verified with tsc before choosing. The overload form compiles both the legacy call and the awaited one against the real emitted build.

[Copilot]

🟡 Changes recommended

There are a few correctness/robustness issues in the newly added docs/tests and a missing queueMicrotask fallback that could cause runtime failures in unsupported environments.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

packages/melonjs/src/renderable/trigger.js:208

• queueMicrotask is used here to rethrow load failures, but it isn’t feature-detected or polyfilled in this repo. In runtimes without queueMicrotask, this catch handler will throw a ReferenceError and may mask the original error. Consider falling back to setTimeout for the rethrow path.

								// same loudness as the fire-and-forget form
								queueMicrotask(() => {
									throw error;
								});

• Files reviewed: 5/5 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The changes are cohesive, preserve legacy behavior, and are backed by targeted tests covering the new async contract, scheduling, and Trigger sequencing.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The implementation and inline API docs describe timer/macrotask deferral, which conflicts with the PR description’s “microtask deferral” claim and includes multiple “microtask” doc strings that don’t match the actual scheduling.

Review details

Suppressed comments (5)

packages/melonjs/src/level/level.js:215

• The docs here say the load is deferred to a microtask while the loop is running, but the implementation below schedules via defer, which is a setTimeout(..., 0) macrotask. This should say timer/macrotask to match real behavior.

	 * While the game loop is running the load is DEFERRED to a microtask, so
	 * this returns before anything is in the world. Sequence follow-up work from
	 * `options.onLoaded`, from an `event.LEVEL_LOADED` listener, or by passing
	 * `async: true` and awaiting the promise that overload returns.

packages/melonjs/src/level/level.js:345

• This comment says reload is deferred to a microtask, but level.load() defers via defer (setTimeout), i.e. a timer/macrotask. Updating this avoids misleading API docs.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:384

• This comment says next() is deferred to a microtask, but the deferral is a timer/macrotask via defer (setTimeout). Please align wording with the actual scheduling semantics.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:270

• The PR description/linked issue text calls out replacing the old setTimeout deferral with a microtask to avoid background-tab timer clamping, but this implementation explicitly keeps using a timer/macrotask via defer (which is setTimeout(..., 0)). That means background clamping behavior remains; either the PR description/title needs updating, or the scheduling should be changed to an actual microtask (e.g. queueMicrotask/Promise.resolve().then(...)) and the associated tests updated accordingly.

		// A TIMER, through the engine's own `defer`, and deliberately not a
		// microtask. A macrotask cannot run inside another, so the load lands
		// after the current frame whatever the frame does. A microtask only
		// lands there while the whole update-and-draw path stays synchronous —
		// true today, but it would start running mid-frame the day anything in
		// that path awaits, silently and with nothing to catch it.

packages/melonjs/src/level/level.js:425

• This comment says previous() is deferred to a microtask, but the deferral is a timer/macrotask via defer (setTimeout). Please align wording with the actual scheduling semantics.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

• Files reviewed: 9/9 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

There are documented semantics claiming “microtask” deferral (and PR text referencing a microtask switch) that conflict with the actual timer/macrotask scheduling used in the implementation and tests.

Review details

Suppressed comments (6)

packages/melonjs/src/level/level.js:202

• This JSDoc says the deferred branch is a “microtask”, but the implementation explicitly schedules via utils.function.defer (setTimeout), i.e. a timer/macrotask. The docs should match the actual scheduling semantics.

	 * While the game loop is running the load is DEFERRED to a microtask, so
	 * this returns before anything is in the world. Sequence follow-up work from
	 * `options.onLoaded`, from an `event.LEVEL_LOADED` listener, or by passing
	 * `async: true` and awaiting the promise that overload returns.

packages/melonjs/src/level/level.js:336

• This overload doc claims the load is deferred to a microtask, but level.load() actually defers via utils.function.defer (setTimeout) when the loop is running. Update wording to avoid misleading callers about scheduling and background-tab timer clamping.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:375

• This doc mentions “microtask” deferral, but next()/previous() just delegate to level.load(), which defers via utils.function.defer (setTimeout) while running. Align the wording with the actual macrotask/timer behavior.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:416

• This doc mentions “microtask” deferral, but previous() delegates to level.load(), which defers via utils.function.defer (setTimeout) while running. Update wording to reflect the macrotask/timer scheduling.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/tests/level_load_async.spec.js:21

• The file header comment says level.load() deferral is “now a microtask”, but the code under test (and several assertions below) treats it as timer/macrotask deferral via utils.function.defer (setTimeout). This comment should be corrected to avoid contradicting the tests’ intent.

 * The deferral in `level.load()` dates to 2011 and used a timer because that
 * was the only way to defer at the time. It is still needed — `level.load()` is
 * routinely called from inside the loop, and `safeLoadLevel` resets and
 * destroys the very container the loop may be iterating — but it is now a
 * microtask, and `async: true` hands that completion back instead of a boolean.

packages/melonjs/src/level/level.js:256

• The PR description/title claims level loading switched from a timer to a microtask to avoid background-tab timer clamping, but the implementation here explicitly keeps a timer/macrotask (via utils.function.defer/setTimeout) and even calls out “deliberately not a microtask”. Either update the PR description to match the shipped behavior, or change the scheduling to a true microtask and adjust tests/docs accordingly.

		// A TIMER, through the engine's own `defer`, and deliberately not a
		// microtask. A macrotask cannot run inside another, so the load lands
		// after the current frame whatever the frame does. A microtask only
		// lands there while the whole update-and-draw path stays synchronous —
		// true today, but it would start running mid-frame the day anything in

• Files reviewed: 9/9 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Several updated JSDoc comments and the PR narrative describe microtask-based deferral, but the implementation still defers via defer/setTimeout (timer/macrotask), creating user-facing documentation and expectation mismatches that should be resolved.

Review details

Suppressed comments (5)

packages/melonjs/src/level/level.js:202

• The JSDoc says the load is deferred to a microtask while the loop is running, but the implementation explicitly defers via utils.function.defer (setTimeout), i.e. a timer/macrotask. This mismatch is likely to mislead users reading the API docs about when the world is actually populated.

	 * While the game loop is running the load is DEFERRED to a microtask, so
	 * this returns before anything is in the world. Sequence follow-up work from
	 * `options.onLoaded`, from an `event.LEVEL_LOADED` listener, or by passing
	 * `async: true` and awaiting the promise that overload returns.

packages/melonjs/src/level/level.js:336

• This docstring says reload is deferred to a microtask when the loop is running, but level.load/reload defer via utils.function.defer (setTimeout), i.e. a timer/macrotask. Keeping the docs consistent with the actual scheduling avoids confusion around ordering.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:375

• This docstring says next() is deferred to a microtask when the loop is running, but the actual scheduling is via utils.function.defer (setTimeout), i.e. a timer/macrotask. The wording should match the real behavior so callers know what they can safely sequence after the call.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:416

• This docstring says previous() is deferred to a microtask when the loop is running, but the actual scheduling is via utils.function.defer (setTimeout), i.e. a timer/macrotask. Aligning the docs with behavior matters for callers relying on precise ordering.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:257

• The PR description/issue text says the setTimeout deferral was replaced with a microtask to avoid background-tab timer clamping, but the implementation still defers through utils.function.defer, which is setTimeout(..., 0). If the goal is to eliminate timer clamping, this scheduling strategy won’t achieve that; if the goal changed to “centralize on defer”, the PR title/description (and issue closure) should be updated to match.

		// A TIMER, through the engine's own `defer`, and deliberately not a
		// microtask. A macrotask cannot run inside another, so the load lands
		// after the current frame whatever the frame does. A microtask only
		// lands there while the whole update-and-draw path stays synchronous —
		// true today, but it would start running mid-frame the day anything in
		// that path awaits, silently and with nothing to catch it.

• Files reviewed: 9/9 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

There are correctness/documentation inconsistencies around timer-vs-microtask deferral and at least one Trigger transition sequencing edge case (reveal ordering/behavior) plus a test that awaits only microtasks despite macrotask scheduling.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (7)

Previously missed (1) — in code that hasn't changed since the last review.

packages/melonjs/src/level/level.js:180

• The summary line for level.load says it "return[s] a promise" unconditionally, but the legacy (non-async) overload still returns a boolean. This wording is likely to confuse readers skimming docs/API reference.

packages/melonjs/src/level/level.js:202

• Docs say the deferred load is "a microtask", but the implementation explicitly schedules via defer()/setTimeout(..., 0) (a macrotask). Please align the JSDoc wording with the actual scheduling semantics.

	 * While the game loop is running the load is DEFERRED to a microtask, so
	 * this returns before anything is in the world. Sequence follow-up work from
	 * `options.onLoaded`, from an `event.LEVEL_LOADED` listener, or by passing
	 * `async: true` and awaiting the promise that overload returns.

packages/melonjs/tests/trigger_level_change.spec.js:164

• This test only awaits microtasks, but level.load(..., { async: true }) defers the actual load via utils.function.defer, which uses setTimeout(..., 0) (a macrotask). Without awaiting a timer tick, loaded/seen may still be unchanged when assertions run.

		// let the load's microtask and the reveal chained after it settle
		for (let i = 0; i < 4; i++) {
			await Promise.resolve();
		}

packages/melonjs/src/level/level.js:336

• This JSDoc mentions deferring to "a microtask", but level.load currently defers via defer()/setTimeout when the loop is running. Keeping these docs consistent avoids incorrect assumptions about ordering.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:375

• This JSDoc mentions deferring to "a microtask", but level.load currently defers via defer()/setTimeout when the loop is running. Keeping these docs consistent avoids incorrect assumptions about ordering.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:416

• This JSDoc mentions deferring to "a microtask", but level.load currently defers via defer()/setTimeout when the loop is running. Keeping these docs consistent avoids incorrect assumptions about ordering.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:257

• The PR description/title emphasize switching the deferral to a microtask, but the implementation here explicitly chooses a timer/macrotask via defer() (setTimeout) and calls out "deliberately not a microtask". If the intended behavior is macrotask deferral, the PR description/title should be updated; if microtask deferral is still desired, the implementation/tests/docs should be revisited for consistency.

		// A TIMER, through the engine's own `defer`, and deliberately not a
		// microtask. A macrotask cannot run inside another, so the load lands
		// after the current frame whatever the frame does. A microtask only
		// lands there while the whole update-and-draw path stays synchronous —
		// true today, but it would start running mid-frame the day anything in
		// that path awaits, silently and with nothing to catch it.

• Files reviewed: 9/9 changed files
• Comments generated: 1
• Review effort level: Lite

[obiot]

Latest round.

Stale microtask wording (level.js:199, :335, :374, :415, and the spec header): correct, fixed. The deferral went back to a timer via defer and the docs did not follow. Good catch — that is exactly the kind of drift that outlives a revert.

The reveal test only yielding to microtasks: correct, and worse than it looked. The test passed, but loaded reached 1 during a microtask-only loop, which nothing in the test guarantees — it was measuring whatever the scheduler happened to do. It yields to a macrotask now. Both mutations it exists for still fail: a stale viewport captured before the load, and a reveal that does not wait for the load.

Reveal ordering vs state.restart(): the observation is right, the consequence does not follow, so no change.

The reveal did move from before state.restart() to after it. But it cannot produce a frame of unfaded level, because state.restart() schedules a requestAnimationFrame while the reveal is a promise continuation — a microtask in the same task the load ran in. Microtasks drain before the next animation frame, so the reveal is always in place before anything renders.

I wrote a test to pin that and then deleted it: asserting "no frame intervened" from outside the scheduler is racy, and the property is a language guarantee rather than engine behaviour. The existing test already pins the part that is ours — the reveal happens after the load, on the viewport that exists after it.

The other half of your point is real and intended: the caller's onLoaded now runs before the reveal rather than after. That is the interception being removed. The trigger no longer rewrites an options object it was handed, which is the defect this part of the PR exists to fix.

[Copilot]

🟡 Changes recommended

The implemented scheduling remains timer/macrotask-based (via defer/setTimeout), which materially conflicts with the PR title/description and issue scope claiming a microtask deferral and its background-tab unclamping benefits.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 9/9 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The PR title/description and linked issue state a microtask-based deferral change, but the implementation explicitly preserves timer/macrotask deferral, so the stated scope/outcome needs to be reconciled before merge.

Review details

Suppressed comments (5)

Previously missed (1) — in code that hasn't changed since the last review.

packages/melonjs/src/level/level.js:180

• The overload doc summary reads as if level.load() always returns a promise, but the boolean overload is the default behavior. Reword the summary to make it explicit that the promise return only applies when options.async is true, so generated docs don’t contradict the non-async signature.

This issue also appears in the following locations of the same file:

• line 323
• line 354
• line 395

packages/melonjs/src/level/level.js:354

• This summary sentence implies next() always returns a promise, but it only does so when async: true is provided (otherwise it returns a boolean). Consider rewording so the async behavior is clearly conditional.

	 * load the next level, and return a promise that settles once the level is in the world.

packages/melonjs/src/level/level.js:395

• This summary sentence implies previous() always returns a promise, but it only does so when async: true is provided (otherwise it returns a boolean). Rewording keeps the doc summary consistent with the overloads.

	 * load the previous level, and return a promise that settles once the level is in the world.

packages/melonjs/src/level/level.js:257

• The PR title/description and linked issue describe replacing the existing timer deferral with a microtask (to avoid background-tab timer clamping), but the implementation explicitly keeps a timer-based macrotask via defer/setTimeout. Please reconcile this (update the PR/issue scope if the timer is intentional, or adjust the scheduling strategy if the microtask behavior is still the goal).

		// A TIMER, through the engine's own `defer`, and deliberately not a
		// microtask. A macrotask cannot run inside another, so the load lands
		// after the current frame whatever the frame does. A microtask only
		// lands there while the whole update-and-draw path stays synchronous —
		// true today, but it would start running mid-frame the day anything in
		// that path awaits, silently and with nothing to catch it.

packages/melonjs/src/level/level.js:323

• This summary sentence implies reload() always returns a promise, but it only does so when async: true is provided (otherwise it returns a boolean). Making that explicit avoids misleading API docs.

	 * reload the current level, and return a promise that settles once the level is in the world.

• Files reviewed: 9/9 changed files
• Comments generated: 0 new
• Review effort level: Lite

[obiot]

Note for anyone arriving from the squash commit: its subject and body are stale. They were written against intermediate states that this PR later reverted, and the squash concatenated every commit message including the superseded ones.

What actually landed:

• The deferral is a timer, through the engine's own utils.function.defer. The microtask version was reverted: a macrotask cannot run inside another, so the load lands after the current frame structurally, rather than only while the update-and-draw path happens to stay synchronous.
• There is no loadAsync(). The switch is an async option on the existing load / reload / next / previous, preserved as JSDoc @overload pairs so const ok: boolean = level.load(id) still compiles.
• queueMicrotask is not used anywhere.
• The background-tab clamp is not fixed — that was the trade for going back to a timer, and the CHANGELOG entry claiming it was removed.

The code comments and the CHANGELOG in the merged tree are accurate; only the commit message is not. My miss for not retitling before the merge.