Use AssetListLoader to load assets in AppBase.preload

[lucaheft]

Description

Uses AssetListLoader in AppBase.preload to load assets marked as preload.

I also changed the fetching of assets to filter by loaded as well. By the time preload is called some assets might already be loaded. (Wasm modules, as well as some assets which are already loading because they were added to the asset registry. See #3107)

Also added tests for preload

Fixes #4185

Checklist

• I have read the contributing guidelines
• My code follows the project's coding standards
• This PR focuses on a single change

[Copilot]

Pull request overview

This PR refactors the AppBase.preload method to use the AssetListLoader utility for loading preload-marked assets, replacing the previous manual asset loading and event handling implementation. It also adds filtering to exclude already-loaded assets before starting the preload process, and includes comprehensive tests for the preload functionality.

Key changes:

• Replaced manual asset loading loop with AssetListLoader for cleaner code
• Added filter to exclude already-loaded assets from the preload process
• Added test coverage for preload functionality with both positive and negative test cases

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/framework/app-base.js	Refactored preload method to use AssetListLoader instead of manual asset loading; added filtering for already-loaded assets
test/framework/application.test.mjs	Added comprehensive tests for preload functionality covering both assets with preload=true and preload=false scenarios

Comments suppressed due to low confidence (1)

src/framework/app-base.js:711

• Avoid automated semicolon insertion (90% of all statements in the enclosing function have an explicit semicolon).

        const assets = this.assets.filter(asset => asset.preload === true && asset.loaded === false)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The AssetListLoader only fires 'progress' events on successful asset loads, not on errors. This changes the behavior from the previous implementation where preload:progress was fired for both successful loads and errors. This means if some assets fail to load, the progress will not reach 1.0 (100%). Consider also handling the 'error' event from AssetListLoader, or document this behavior change.

Suggested change

	assetListLoader.on('progress', onAssetLoad);
	assetListLoader.on('progress', onAssetLoad);
	assetListLoader.on('error', onAssetLoad);

[lucaheft]

There exists no error event for AssetListLoader. We would need to make adjustments to the AssetListLoader class to make this work.

[willeastcott]

Thanks for taking this on, @lucaheft, and apologies for the long wait on a review.

After looking at it closely, we've decided not to take this change, and we're going to close #4185 along with it.

The blocker is the one Copilot spotted: AssetListLoader only fires progress for successful loads, so a preload asset that fails no longer produces a preload:progress event. I verified this with three preload assets (one already loaded, one good, one 404):

	preload:progress values	preload:end
main	0.333, 0.667, 1.0	fires
this PR	0.5	fires

Every Editor-built loading screen drives its bar from preload:progress, so one missing asset would leave the bar stuck short of full even though the app goes on to start. Fixing that properly means changing AssetListLoader's public progress semantics, at which point the dedup stops being a simplification.

Two smaller points that pushed the same way: AssetListLoader defers completion through setTimeout, so preload:end and the callback move a tick later on every app start, and it routes legacy .json model URLs through loadFromUrl, a path preload never took before. Neither is a bug on its own, but they're behavior changes on the hottest startup path in exchange for saving roughly ten lines.

The existing loop is small and already counts errors toward progress, so we'll keep it as is. The tests you added were a good idea though, and we'd happily take a PR that adds #preload coverage on its own (note the fixtures server now listens on port 3210 rather than 3000).