bevy icon indicating copy to clipboard operation
bevy copied to clipboard

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

Open laundmo opened this issue 1 year ago • 6 comments

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 avatar Jun 30 '24 17:06 laundmo

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

bugsweeper avatar Jul 03 '24 09:07 bugsweeper

@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.

laundmo avatar Jul 03 '24 09:07 laundmo

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?

bugsweeper avatar Jul 03 '24 11:07 bugsweeper

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.

laundmo avatar Jul 03 '24 13:07 laundmo

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?

bugsweeper avatar Jul 04 '24 06:07 bugsweeper

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 avatar Jul 04 '24 11:07 alice-i-cecile