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

Asset loading on the Web produces many 404 errors for .meta files #10157

Closed
coreh opened this issue Oct 17, 2023 · 19 comments · Fixed by #10623
Closed

Asset loading on the Web produces many 404 errors for .meta files #10157

coreh opened this issue Oct 17, 2023 · 19 comments · Fixed by #10623
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times O-Web Specific to web (WASM) builds
Milestone

Comments

@coreh
Copy link
Contributor

coreh commented Oct 17, 2023

Bevy version

5733d24

What you did

Ran any example that loads assets under WASM / WebGL with:

cargo run -p build-wasm-example -- $EXAMPLE_NAME_HERE && python3 -m http.server --directory examples
/wasm

What went wrong

Got barraged with 404 error messages in the web inspector with the page trying to load .meta files for every asset loaded.

Additional information

image
  • Looks like with Bevy Asset V2 #8624 we now look for .meta files for every asset that we load
  • On native this is a non issue, however on the web we do a bunch of requests, which both spam the console with 404 errors and also potentially slow down loading of the files that actually exist
@coreh coreh added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Oct 17, 2023
@alice-i-cecile alice-i-cecile added A-Assets Load files from disk to use for things like images, models, and sounds C-Performance A change motivated by improving speed, memory usage or compile times O-Web Specific to web (WASM) builds and removed S-Needs-Triage This issue needs to be labelled labels Oct 17, 2023
@hymm
Copy link
Contributor

hymm commented Oct 17, 2023

A solution might be for the web asset source to write out a manifest file of the available asset files. This could also be used to solve load_folder not working on the web.

@mockersf mockersf added this to the 0.12 milestone Oct 17, 2023
@mockersf
Copy link
Member

Adding the 0.12 milestone. I think this should be looked over before the release, it will raise a lot of questions on wasm

@hymm
Copy link
Contributor

hymm commented Oct 26, 2023

Should probably just suppress the error message on wasm for now, until we come up with something better.

@alice-i-cecile alice-i-cecile modified the milestones: 0.12, 0.13 Oct 27, 2023
@alice-i-cecile
Copy link
Member

No PR exists, and the release is imminent. Bumping to 0.13, but I'd nominate this for 0.12.1 as well.

@rparrett
Copy link
Contributor

rparrett commented Nov 7, 2023

Adding this info for searchability.

This is also tripping up users who are using trunk or other dev web servers that act in an "SPA" mode where the server doesn't actually serve 404s.

They will see error messages like

/home/redacted/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_asset-0.12.0/src/server/mod.rs:274 Failed to deserialize meta for asset image.png: Failed to deserialize minimal asset meta: SpannedError { code: ExpectedNamedStructLike("AssetMetaMinimal"), position: Position { line: 1, col: 1 } }

Because the web server is just serving the contents of index.html with a status of 200 when a file is missing.

@forbjok
Copy link
Contributor

forbjok commented Nov 8, 2023

When uploaded to itch.io, the server the assets are hosted on return 403 instead of 404, and this actually causes the game to completely fail to run.

@alice-i-cecile alice-i-cecile modified the milestones: 0.13, 0.12.1 Nov 9, 2023
@alice-i-cecile
Copy link
Member

If we can get a fix in for this, we should include it in 0.12.1

@rparrett
Copy link
Contributor

rparrett commented Nov 11, 2023

Is there a procedure users can follow right now to generate default meta files to work around the issue on itch?

edit: Here is something that seems to work:

  • Enable processing temporarily.

  • Run your app once. It will generate meta files in the assets directory.

  • (bug?) Bevy attempts to load the meta files without the assets subdirectory in the path. This is true when in either AssetMode. So you need to move the meta files one directory up in the hierarchy.
    (edit: yes, bug, will be fixed later by #10527)

    That is, if your assets folder looks like this:

    assets/fonts/whatever.ttf
    assets/images/sprite.png
    assets/level.ron
    

    Then the output directory that you zip up and send off to itch should look something like

    index.html
    assets/images/sprite.png
    assets/level.ron
    images/sprite.png.meta
    level.ron.meta
    
  • You can disable asset processing now

edit: unconfirmed, but I had some feedback on Discord that Itch still serves http 403 for the .meta files even if they are present. bevy_embedded_assets might be another workaround.

@P13L0v3r
Copy link

Since it wasn't said,

.set(AssetPlugin {
    mode: AssetMode::Unprocessed,
    ..default()
})

does not fix the issue.

@cart
Copy link
Member

cart commented Nov 14, 2023

Yeah this needs fixing for 0.12.1, especially given that the next Bevy Jam is coming up / WASM builds are a big part of that.

I think long term the path forward is "asset packing/compression with manifests". That way we can record if a meta file exists in the manifest and avoid making the request. And WASM deployments should probably ultimately be using processing + packing anyway to cut down on bandwidth usage / loading times.

Short term, I think we should probably just add a knob that controls meta file lookup behavior. Ex:

enum AssetMetaMode {
    // Always look for meta files
    Always,
    // Only look up meta files for assets in the given list 
    AllowList(Vec<AssetPath<'static>),
    // Never look for meta files. The default loader meta values will be used
    Never,
}

Given that most people probably aren't using meta files at this point, this is probably the right "patch fix" approach. Users can just set this to AssetMetaMode::Never. Ideally we have a fix that requires no manual intervention, but I haven't thought of one yet.

@cart cart mentioned this issue Nov 18, 2023
github-merge-queue bot pushed a commit that referenced this issue Nov 27, 2023
# Objective

Fixes #10157

## Solution

Add `AssetMetaCheck` resource which can configure when/if asset meta
files will be read:

```rust
app
  // Never attempts to look up meta files. The default meta configuration will be used for each asset.
  .insert_resource(AssetMetaCheck::Never)
  .add_plugins(DefaultPlugins)
```


This serves as a band-aid fix for the issue with wasm's
`HttpWasmAssetReader` creating a bunch of requests for non-existent
meta, which can slow down asset loading (by waiting for the 404
response) and creates a bunch of noise in the logs. This also provides a
band-aid fix for the more serious issue of itch.io deployments returning
403 responses, which results in full failure of asset loads.

If users don't want to include meta files for all deployed assets for
web builds, and they aren't using meta files at all, they should set
this to `AssetMetaCheck::Never`.

If users do want to include meta files for specific assets, they can use
`AssetMetaCheck::Paths`, which will only look up meta for those paths.

Currently, this defaults to `AssetMetaCheck::Always`, which makes this
fully non-breaking for the `0.12.1` release. _**However it _is_ worth
discussing making this `AssetMetaCheck::Never` by default**_, given that
I doubt most people are using meta files without the Asset Processor
enabled. This would be a breaking change, but it would make WASM / Itch
deployments work by default, which is a pretty big win imo. The downside
is that people using meta files _without_ processing would need to
manually enable `AssetMetaCheck::Always`, which is also suboptimal.

When in `AssetMetaCheck::Processed`, the meta check resource is ignored,
as processing requires asset meta files to function.

In general, I don't love adding this knob as things should ideally "just
work" in all cases. But this is the reality of the current situation.

---

## Changelog

- Added `AssetMetaCheck` resource, which can configure when/if asset
meta files will be read
cart added a commit that referenced this issue Nov 30, 2023
# Objective

Fixes #10157

## Solution

Add `AssetMetaCheck` resource which can configure when/if asset meta
files will be read:

```rust
app
  // Never attempts to look up meta files. The default meta configuration will be used for each asset.
  .insert_resource(AssetMetaCheck::Never)
  .add_plugins(DefaultPlugins)
```


This serves as a band-aid fix for the issue with wasm's
`HttpWasmAssetReader` creating a bunch of requests for non-existent
meta, which can slow down asset loading (by waiting for the 404
response) and creates a bunch of noise in the logs. This also provides a
band-aid fix for the more serious issue of itch.io deployments returning
403 responses, which results in full failure of asset loads.

If users don't want to include meta files for all deployed assets for
web builds, and they aren't using meta files at all, they should set
this to `AssetMetaCheck::Never`.

If users do want to include meta files for specific assets, they can use
`AssetMetaCheck::Paths`, which will only look up meta for those paths.

Currently, this defaults to `AssetMetaCheck::Always`, which makes this
fully non-breaking for the `0.12.1` release. _**However it _is_ worth
discussing making this `AssetMetaCheck::Never` by default**_, given that
I doubt most people are using meta files without the Asset Processor
enabled. This would be a breaking change, but it would make WASM / Itch
deployments work by default, which is a pretty big win imo. The downside
is that people using meta files _without_ processing would need to
manually enable `AssetMetaCheck::Always`, which is also suboptimal.

When in `AssetMetaCheck::Processed`, the meta check resource is ignored,
as processing requires asset meta files to function.

In general, I don't love adding this knob as things should ideally "just
work" in all cases. But this is the reality of the current situation.

---

## Changelog

- Added `AssetMetaCheck` resource, which can configure when/if asset
meta files will be read
@valyagolev
Copy link

valyagolev commented Dec 10, 2023

Since nobody posted very specifically what to do here, answer is:

add .insert_resource(AssetMetaCheck::Never)

to your app before adding default plugins

@rparrett
Copy link
Contributor

Since nobody posted very specifically what to do here, answer is:

There's some code in the linked PR that adds the workaround. Make sure you add this resource before DefaultPlugins.

@awwsmm
Copy link
Contributor

awwsmm commented Feb 28, 2024

Can confirm that this is still happening in 0.13.0 and that @valyagolev 's single-line change prevents 404s from spamming the browser console

@SK83RJOSH
Copy link

SK83RJOSH commented May 5, 2024

Just wanted to follow up and say that this should definitely be kept in/after 0.14.0 - my app almost entirely processes non-bevy game assets that will never have meta files. Including loading/mounting archives which gates all other types of file I/O from happening - which ironically means if that if this feature is enabled, the archives themselves can get gated/hung by the asset waiting for it's associated meta file to load.

Definitely something I can workaround by just letting those requests bypass the gate, but if someone else were to do something similar, and the archives themselves contain meta files... well it begins to get rather complicated quickly. 🙂

That said, if this is kept in 14.0.0, I think it would be worthwhile to investigate an approach that lets particular resources/loaders/processors opt-out of meta files, as plugins would likely not want to force users to disable meta files across the board/manually register all exceptions.

@mrchantey
Copy link
Contributor

Single Page Applications

When serving assets from a typical SPA setup they will not load at all without AssetMetaCheck::Never. This is because example.com/assets/foo.meta redirects back to index.html which bevy will cheerfully attempt to read as a meta file.

btw heres the ^0.14 one-liner

app.world_mut().insert_resource(AssetMetaCheck::Never);

@benfrankel
Copy link
Contributor

In bevy 0.14 the workaround is now:

app.add_plugins(DefaultPlugins.set(AssetPlugin {
    meta_check: AssetMetaCheck::Never,
}));

@SK83RJOSH
Copy link

Just wanted to follow up and say the move to making this a non-resource makes the story for plugins even more complicated than before. Now I have to offer guidance on disabling meta files before my plugin is loaded, rather than just opt-out if my plugin is loaded, which makes the user story more complex.

As mentioned before, it would be nice to make this something a given source opts into/out of, instead of having it be global setting.

@AgustinRamiroDiaz
Copy link

AgustinRamiroDiaz commented Aug 24, 2024

In bevy 0.14 the workaround is now:

app.add_plugins(DefaultPlugins.set(AssetPlugin {
    meta_check: AssetMetaCheck::Never,
}));

I think defaults are also needed

    app.add_plugins(DefaultPlugins.set(AssetPlugin {
        meta_check: AssetMetaCheck::Never,
        ..default()
    }));

github-merge-queue bot pushed a commit that referenced this issue Sep 9, 2024
# Objective

We should attempt to document the entirety of bevy_assets. `AssetMode`
is missing docs explaining what it is, how it's used and why it exists.

## Solution

Add docs, focusing on the context in
#10157.
@katopz
Copy link

katopz commented Oct 6, 2024

This should be add to all example or AssetMetaCheck::Never by default. I lost a day and crying because of this! And everything should work by default by all mean. T-T

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times O-Web Specific to web (WASM) builds
Projects
None yet
Development

Successfully merging a pull request may close this issue.