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

Bad error when AssetServer::load is called on &str from event reader (possibly flawed lifetimes?) #14080

Open
laundmo opened this issue Jun 30, 2024 · 6 comments
Labels
A-ECS Entities, components, systems, and events C-Usability A simple quality-of-life change that makes Bevy easier to use S-Blocked This cannot move forward until something else changes S-Needs-Investigation This issue requires detective work to figure out what's going wrong

Comments

@laundmo
Copy link
Contributor

laundmo commented Jun 30, 2024

Bevy version

Bevy 0.13

What you did

Used a EventReader which includes a String, and passed a &str based on this to AssetServer::load

This is a minimal reproduction.

use bevy::prelude::*;
fn main() {}
#[derive(Event)]
struct LoadAsset(String);

fn load_asset(mut events: EventReader<LoadAsset>, assetserver: Res<AssetServer>) {
    for i in events.read() {
        let _sound: Handle<AudioSource> = assetserver.load(i.0.as_str());
    }
}

What went wrong

The resulting error message was very unclear, pointing at the events.read() call as "events escapes the function body here" which is highly confusing when it happens.

error[E0597]: `events` does not live long enough
  --> examples\t.rs:7:14
   |
6  | fn load_asset(mut events: EventReader<LoadAsset>, assetserver: Res<AssetServer>) {
   |               ---------- binding `events` declared here
7  |     for i in events.read() {
   |              ^^^^^^-------
   |              |
   |              borrowed value does not live long enough
   |              argument requires that `events` is borrowed for `'static`
...
10 | }
   |  - `events` dropped here while still borrowed

Additional information

This error was not encountered directly by me, but someone else I was helping with bevy. They're pretty new to Rust which is why cloning the String instead was not immediately obvious, causing some headaches.

I believe the fact that this error message is this wrong, could point towards some badly specified or unclear lifetimes in AssetServer::load or EvenrReader::read. I was unable to reproduce a similar error with only Rust stdlib, but I may be wrong here.

@laundmo laundmo added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Jun 30, 2024
@janhohenheim janhohenheim added A-ECS Entities, components, systems, and events C-Usability A simple quality-of-life change that makes Bevy easier to use S-Needs-Investigation This issue requires detective work to figure out what's going wrong P-Compile-Failure A failure to compile Bevy apps and removed S-Needs-Triage This issue needs to be labelled labels Jul 1, 2024
@bugsweeper
Copy link
Contributor

bugsweeper commented Jul 3, 2024

Since AssetServer::load takes Into<AssetPath> and AssetPath has both From<&'static str> and From<String> simple solution for you is to be more explicit with:

let _sound: Handle<AudioSource> = assetserver.load(i.0.clone());

since you already use owned String.
More over AssetServer converts to owned inside load

@laundmo
Copy link
Contributor Author

laundmo commented Jul 3, 2024

@bugsweeper i should clarify: i know this is the solution. i'm not asking for that. i'm reporting a error message which is quite confusing, as the error somehow points to the wrong line.

this may be a rustc issue, but i lack the knowledge of EventReade and AssetServer internals to reproduce it without bevy.

@bugsweeper
Copy link
Contributor

bugsweeper commented Jul 3, 2024

Oh, I see, the reason of this issue is not EventReander or even AssetServer. You will laugh, but AssetPath is to blame. Because there is impl From<&'static str> and impl From<String>, but no impl From<&'other_lifetime str> for AssetPath. We can't add such impl, because error[E0119]: conflicting implementations of trait 'From<&str>' for type 'path::AssetPath<'_>'. Best what you can use, as I see, is impl<'a> From<&'a String> like

let _sound: Handle<AudioSource> = assetserver.load(&i.0);

Is this acceptable to you?

@laundmo
Copy link
Contributor Author

laundmo commented Jul 3, 2024

I think you still misunderstand: The code is not the issue i am reporting. I'm not asking for a solution. I know it. The error message is what i'm reporting, as the error is confusing.

I have no idea if bevy can improve this error message in any way, but its definitely something worth reporting.

@bugsweeper
Copy link
Contributor

This error is rust lifetime error. In this circumstances bevy knows nothing about what happens. I think the best what we can do is to add to AssetServer::load (as more viewed than AssetPath::from) docs mention of such case. @laundmo Would someone else you was helping with bevy look at it? @alice-i-cecile do you agree with me?

@alice-i-cecile
Copy link
Member

Docs are unlikely to help much. A usage example doc test for this would be good though. IMO we should file an issue on the Rust repo linking this one.

@alice-i-cecile alice-i-cecile added S-Blocked This cannot move forward until something else changes A-ECS Entities, components, systems, and events S-Needs-Investigation This issue requires detective work to figure out what's going wrong and removed S-Needs-Investigation This issue requires detective work to figure out what's going wrong C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events P-Compile-Failure A failure to compile Bevy apps labels Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Usability A simple quality-of-life change that makes Bevy easier to use S-Blocked This cannot move forward until something else changes S-Needs-Investigation This issue requires detective work to figure out what's going wrong
Projects
None yet
Development

No branches or pull requests

4 participants