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 server panics with "asset paths must have extensions" #10703

Closed
juh9870 opened this issue Nov 22, 2023 · 2 comments · Fixed by #10153
Closed

Asset server panics with "asset paths must have extensions" #10703

juh9870 opened this issue Nov 22, 2023 · 2 comments · Fixed by #10153
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior

Comments

@juh9870
Copy link

juh9870 commented Nov 22, 2023

Bevy version

0.12.0

[Optional] Relevant system information

name version
cargo 1.73.0
OS NixOS 23.05.1906.8163a64662b (Stoat) x86_64

What you did

Attempted to load a folder that contains .directory files generated by Dolphin file explorer

What went wrong

Method get_meta_path causes asset server to panic with a failed assertion

Additional information

Debugger screenshot from the moment of panic
image

Offending code:

/// Appends `.meta` to the given path.
pub(crate) fn get_meta_path(path: &Path) -> PathBuf {
let mut meta_path = path.to_path_buf();
let mut extension = path
.extension()
.expect("asset paths must have extensions")
.to_os_string();
extension.push(".meta");
meta_path.set_extension(extension);
meta_path
}

@juh9870 juh9870 added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Nov 22, 2023
@alice-i-cecile alice-i-cecile added A-Assets Load files from disk to use for things like images, models, and sounds and removed S-Needs-Triage This issue needs to be labelled labels Nov 23, 2023
@alice-i-cecile
Copy link
Member

Is this fixed with #10153?

@bushrat011899
Copy link
Contributor

Is this fixed with #10153?

Yes I believe this issue will also be resolved. I replaced that expect with an unwrap_or_default (representing no extension with an empty string)

https:/bushrat011899/bevy/blob/85e39151941e3888df054ba1fd712dfe31a8b092/crates/bevy_asset/src/io/mod.rs#L248-L255

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

- Addresses **Support processing and loading files without extensions**
from #9714
- Addresses **More runtime loading configuration** from #9714
- Fixes #367
- Fixes #10703

## Solution

`AssetServer::load::<A>` and `AssetServer::load_with_settings::<A>` can
now use the `Asset` type parameter `A` to select a registered
`AssetLoader` without inspecting the provided `AssetPath`. This change
cascades onto `LoadContext::load` and `LoadContext::load_with_settings`.
This allows the loading of assets which have incorrect or ambiguous file
extensions.

```rust
// Allow the type to be inferred by context
let handle = asset_server.load("data/asset_no_extension");

// Hint the type through the handle
let handle: Handle<CustomAsset> = asset_server.load("data/asset_no_extension");

// Explicit through turbofish
let handle = asset_server.load::<CustomAsset>("data/asset_no_extension");
```

Since a single `AssetPath` no longer maps 1:1 with an `Asset`, I've also
modified how assets are loaded to permit multiple asset types to be
loaded from a single path. This allows for two different `AssetLoaders`
(which return different types of assets) to both load a single path (if
requested).

```rust
// Uses GltfLoader
let model = asset_server.load::<Gltf>("cube.gltf");

// Hypothetical Blob loader for data transmission (for example)
let blob = asset_server.load::<Blob>("cube.gltf");
```

As these changes are reflected in the `LoadContext` as well as the
`AssetServer`, custom `AssetLoaders` can also take advantage of this
behaviour to create more complex assets.

---

## Change Log

- Updated `custom_asset` example to demonstrate extension-less assets.
- Added `AssetServer::get_handles_untyped` and Added
`AssetServer::get_path_ids`

## Notes

As a part of that refactor, I chose to store `AssetLoader`s (within
`AssetLoaders`) using a `HashMap<TypeId, ...>` instead of a `Vec<...>`.
My reasoning for this was I needed to add a relationship between `Asset`
`TypeId`s and the `AssetLoader`, so instead of having a `Vec` and a
`HashMap`, I combined the two, removing the `usize` index from the
adjacent maps.

---------

Co-authored-by: Alice Cecile <[email protected]>
tjamaan pushed a commit to tjamaan/bevy that referenced this issue Feb 6, 2024
# Objective

- Addresses **Support processing and loading files without extensions**
from bevyengine#9714
- Addresses **More runtime loading configuration** from bevyengine#9714
- Fixes bevyengine#367
- Fixes bevyengine#10703

## Solution

`AssetServer::load::<A>` and `AssetServer::load_with_settings::<A>` can
now use the `Asset` type parameter `A` to select a registered
`AssetLoader` without inspecting the provided `AssetPath`. This change
cascades onto `LoadContext::load` and `LoadContext::load_with_settings`.
This allows the loading of assets which have incorrect or ambiguous file
extensions.

```rust
// Allow the type to be inferred by context
let handle = asset_server.load("data/asset_no_extension");

// Hint the type through the handle
let handle: Handle<CustomAsset> = asset_server.load("data/asset_no_extension");

// Explicit through turbofish
let handle = asset_server.load::<CustomAsset>("data/asset_no_extension");
```

Since a single `AssetPath` no longer maps 1:1 with an `Asset`, I've also
modified how assets are loaded to permit multiple asset types to be
loaded from a single path. This allows for two different `AssetLoaders`
(which return different types of assets) to both load a single path (if
requested).

```rust
// Uses GltfLoader
let model = asset_server.load::<Gltf>("cube.gltf");

// Hypothetical Blob loader for data transmission (for example)
let blob = asset_server.load::<Blob>("cube.gltf");
```

As these changes are reflected in the `LoadContext` as well as the
`AssetServer`, custom `AssetLoaders` can also take advantage of this
behaviour to create more complex assets.

---

## Change Log

- Updated `custom_asset` example to demonstrate extension-less assets.
- Added `AssetServer::get_handles_untyped` and Added
`AssetServer::get_path_ids`

## Notes

As a part of that refactor, I chose to store `AssetLoader`s (within
`AssetLoaders`) using a `HashMap<TypeId, ...>` instead of a `Vec<...>`.
My reasoning for this was I needed to add a relationship between `Asset`
`TypeId`s and the `AssetLoader`, so instead of having a `Vec` and a
`HashMap`, I combined the two, removing the `usize` index from the
adjacent maps.

---------

Co-authored-by: Alice Cecile <[email protected]>
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants