Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply waiting and timeout on all assets, not just img, audio and video #5491

Merged
merged 1 commit into from
Mar 7, 2024

Conversation

mrxz
Copy link
Contributor

@mrxz mrxz commented Mar 7, 2024

Description:
The asset loading system differentiated between <img>, <audio>, <video> elements and the <a-asset-item> elements. Only the former would be awaited and subject to the timeout, the <a-asset-item> elements would use the normal 'wait till loaded' mechanism of a-node.js.

Besides the timeout not applying to <a-asset-items> it opened the door for race conditions where the timeout could cause a concurrent super.load resulting in two loaded events being emitted. This is the cause behind #5130

This PR moves the <a-asset-item> handling into a-assets ensuring it waits on all assets in the same way. The timeout will now correctly cause the loading event being emitted without awaiting any (pending) assets.

Fixes #5130

Changes proposed:

  • Wait on <a-asset-item>s in a-assets
  • Avoid race condition where a-assets would emit two loaded events

@dmarcos
Copy link
Member

dmarcos commented Mar 7, 2024

Thanks!

@dmarcos dmarcos merged commit 264da50 into aframevr:master Mar 7, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assets timeout produces an error with stacktrace
2 participants