Skip to content

Commit 3d89327

Browse files
Revert asset lifetime changes and preserve staticness of asset paths in AssetServer::load() (#20562)
# Objective - Fixes #19844 ## Solution - Revert #10823 - Also reverts #19094, as the ignored test added there does not compile with the new lifetime rules. ## TODO This PR was made > 10k issues ago, and git can do funky things. Opening as a draft to investigate the diff before proceeding. - [x] merge main into the old branch - [x] revert the PR - [x] make sure the diff looks okay - [x] add comments explaining why we did this weird thing --------- Co-authored-by: Zac Harrold <zac@harrold.com.au>
1 parent d2d6289 commit 3d89327

File tree

3 files changed

+21
-149
lines changed

3 files changed

+21
-149
lines changed

crates/bevy_asset/src/io/source.rs

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -72,26 +72,12 @@ impl<'a> AssetSourceId<'a> {
7272
}
7373
}
7474

75-
impl AssetSourceId<'static> {
76-
/// Indicates this [`AssetSourceId`] should have a static lifetime.
77-
#[inline]
78-
pub fn as_static(self) -> Self {
79-
match self {
80-
Self::Default => Self::Default,
81-
Self::Name(value) => Self::Name(value.as_static()),
82-
}
83-
}
84-
85-
/// Constructs an [`AssetSourceId`] with a static lifetime.
86-
#[inline]
87-
pub fn from_static(value: impl Into<Self>) -> Self {
88-
value.into().as_static()
89-
}
90-
}
91-
92-
impl<'a> From<&'a str> for AssetSourceId<'a> {
93-
fn from(value: &'a str) -> Self {
94-
AssetSourceId::Name(CowArc::Borrowed(value))
75+
// This is only implemented for static lifetimes to ensure `Path::clone` does not allocate
76+
// by ensuring that this is stored as a `CowArc::Static`.
77+
// Please read https://github.com/bevyengine/bevy/issues/19844 before changing this!
78+
impl From<&'static str> for AssetSourceId<'static> {
79+
fn from(value: &'static str) -> Self {
80+
AssetSourceId::Name(value.into())
9581
}
9682
}
9783

@@ -101,10 +87,10 @@ impl<'a, 'b> From<&'a AssetSourceId<'b>> for AssetSourceId<'b> {
10187
}
10288
}
10389

104-
impl<'a> From<Option<&'a str>> for AssetSourceId<'a> {
105-
fn from(value: Option<&'a str>) -> Self {
90+
impl From<Option<&'static str>> for AssetSourceId<'static> {
91+
fn from(value: Option<&'static str>) -> Self {
10692
match value {
107-
Some(value) => AssetSourceId::Name(CowArc::Borrowed(value)),
93+
Some(value) => AssetSourceId::Name(value.into()),
10894
None => AssetSourceId::Default,
10995
}
11096
}
@@ -329,7 +315,7 @@ pub struct AssetSourceBuilders {
329315
impl AssetSourceBuilders {
330316
/// Inserts a new builder with the given `id`
331317
pub fn insert(&mut self, id: impl Into<AssetSourceId<'static>>, source: AssetSourceBuilder) {
332-
match AssetSourceId::from_static(id) {
318+
match id.into() {
333319
AssetSourceId::Default => {
334320
self.default = Some(source);
335321
}

crates/bevy_asset/src/lib.rs

Lines changed: 1 addition & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ impl AssetApp for App {
560560
id: impl Into<AssetSourceId<'static>>,
561561
source: AssetSourceBuilder,
562562
) -> &mut Self {
563-
let id = AssetSourceId::from_static(id);
563+
let id = id.into();
564564
if self.world().get_resource::<AssetServer>().is_some() {
565565
error!("{} must be registered before `AssetPlugin` (typically added as part of `DefaultPlugins`)", id);
566566
}
@@ -1980,94 +1980,6 @@ mod tests {
19801980
app.world_mut().run_schedule(Update);
19811981
}
19821982

1983-
#[test]
1984-
#[ignore = "blocked on https://github.com/bevyengine/bevy/issues/11111"]
1985-
fn same_asset_different_settings() {
1986-
// Test loading the same asset twice with different settings. This should
1987-
// produce two distinct assets.
1988-
1989-
// First, implement an asset that's a single u8, whose value is copied from
1990-
// the loader settings.
1991-
1992-
#[derive(Asset, TypePath)]
1993-
struct U8Asset(u8);
1994-
1995-
#[derive(Serialize, Deserialize, Default)]
1996-
struct U8LoaderSettings(u8);
1997-
1998-
struct U8Loader;
1999-
2000-
impl AssetLoader for U8Loader {
2001-
type Asset = U8Asset;
2002-
type Settings = U8LoaderSettings;
2003-
type Error = crate::loader::LoadDirectError;
2004-
2005-
async fn load(
2006-
&self,
2007-
_: &mut dyn Reader,
2008-
settings: &Self::Settings,
2009-
_: &mut LoadContext<'_>,
2010-
) -> Result<Self::Asset, Self::Error> {
2011-
Ok(U8Asset(settings.0))
2012-
}
2013-
2014-
fn extensions(&self) -> &[&str] {
2015-
&["u8"]
2016-
}
2017-
}
2018-
2019-
// Create a test asset.
2020-
2021-
let dir = Dir::default();
2022-
dir.insert_asset(Path::new("test.u8"), &[]);
2023-
2024-
let asset_source = AssetSource::build()
2025-
.with_reader(move || Box::new(MemoryAssetReader { root: dir.clone() }));
2026-
2027-
// Set up the app.
2028-
2029-
let mut app = App::new();
2030-
2031-
app.register_asset_source(AssetSourceId::Default, asset_source)
2032-
.add_plugins((TaskPoolPlugin::default(), AssetPlugin::default()))
2033-
.init_asset::<U8Asset>()
2034-
.register_asset_loader(U8Loader);
2035-
2036-
let asset_server = app.world().resource::<AssetServer>();
2037-
2038-
// Load the test asset twice but with different settings.
2039-
2040-
fn load(asset_server: &AssetServer, path: &str, value: u8) -> Handle<U8Asset> {
2041-
asset_server.load_with_settings::<U8Asset, U8LoaderSettings>(
2042-
path,
2043-
move |s: &mut U8LoaderSettings| s.0 = value,
2044-
)
2045-
}
2046-
2047-
let handle_1 = load(asset_server, "test.u8", 1);
2048-
let handle_2 = load(asset_server, "test.u8", 2);
2049-
2050-
// Handles should be different.
2051-
2052-
assert_ne!(handle_1, handle_2);
2053-
2054-
run_app_until(&mut app, |world| {
2055-
let (Some(asset_1), Some(asset_2)) = (
2056-
world.resource::<Assets<U8Asset>>().get(&handle_1),
2057-
world.resource::<Assets<U8Asset>>().get(&handle_2),
2058-
) else {
2059-
return None;
2060-
};
2061-
2062-
// Values should match the settings.
2063-
2064-
assert_eq!(asset_1.0, 1);
2065-
assert_eq!(asset_2.0, 2);
2066-
2067-
Some(())
2068-
});
2069-
}
2070-
20711983
#[test]
20721984
fn insert_dropped_handle_returns_error() {
20731985
let mut app = App::new();

crates/bevy_asset/src/path.rs

Lines changed: 10 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -535,43 +535,17 @@ impl<'a> AssetPath<'a> {
535535
}
536536
}
537537

538-
impl AssetPath<'static> {
539-
/// Indicates this [`AssetPath`] should have a static lifetime.
538+
// This is only implemented for static lifetimes to ensure `Path::clone` does not allocate
539+
// by ensuring that this is stored as a `CowArc::Static`.
540+
// Please read https://github.com/bevyengine/bevy/issues/19844 before changing this!
541+
impl From<&'static str> for AssetPath<'static> {
540542
#[inline]
541-
pub fn as_static(self) -> Self {
542-
let Self {
543-
source,
544-
path,
545-
label,
546-
} = self;
547-
548-
let source = source.as_static();
549-
let path = path.as_static();
550-
let label = label.map(CowArc::as_static);
551-
552-
Self {
553-
source,
554-
path,
555-
label,
556-
}
557-
}
558-
559-
/// Constructs an [`AssetPath`] with a static lifetime.
560-
#[inline]
561-
pub fn from_static(value: impl Into<Self>) -> Self {
562-
value.into().as_static()
563-
}
564-
}
565-
566-
impl<'a> From<&'a str> for AssetPath<'a> {
567-
#[inline]
568-
fn from(asset_path: &'a str) -> Self {
543+
fn from(asset_path: &'static str) -> Self {
569544
let (source, path, label) = Self::parse_internal(asset_path).unwrap();
570-
571545
AssetPath {
572546
source: source.into(),
573-
path: CowArc::Borrowed(path),
574-
label: label.map(CowArc::Borrowed),
547+
path: CowArc::Static(path),
548+
label: label.map(CowArc::Static),
575549
}
576550
}
577551
}
@@ -590,12 +564,12 @@ impl From<String> for AssetPath<'static> {
590564
}
591565
}
592566

593-
impl<'a> From<&'a Path> for AssetPath<'a> {
567+
impl From<&'static Path> for AssetPath<'static> {
594568
#[inline]
595-
fn from(path: &'a Path) -> Self {
569+
fn from(path: &'static Path) -> Self {
596570
Self {
597571
source: AssetSourceId::Default,
598-
path: CowArc::Borrowed(path),
572+
path: CowArc::Static(path),
599573
label: None,
600574
}
601575
}

0 commit comments

Comments
 (0)