Skip to content

Commit 7dd5686

Browse files
authored
Fix duplicate asset loader registration warning (#17105)
# Objective The original fix (#11870) did not actually implement the described logic. It checked if there were independently multiple loaders for a given asset type and multiple loaders for a given extension. However, this did not handle the case where those loaders were not the same. For example, there could be a loader for type `Foo` and extension `.foo`. Anther loader could exist for type `Bar` but extension `.bar`. If a third loader was added for type `Foo` but extension `.bar`, the warning would have been incorrectly logged. ## Solution Instead of independently checking to see if there are preexisting loaders for both the extension and type, look up the indices of the loaders for the type in question. Then check to see if the loaders registered for the extensions has any overlap. Only log if there are loaders that fit this criteria. ## Testing Ran CI tests. Locally tested the situation describe in the objective section for the normal `App::init_asset_loader` flow. I think testing could be done on the pre-registration flow for loaders still. I tested on Windows, but the changes should not be affected by platform.
1 parent aa09b66 commit 7dd5686

File tree

1 file changed

+31
-22
lines changed

1 file changed

+31
-22
lines changed

crates/bevy_asset/src/server/loaders.rs

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ impl AssetLoaders {
4747
};
4848

4949
if is_new {
50+
let existing_loaders_for_type_id = self.type_id_to_loaders.get(&loader_asset_type);
5051
let mut duplicate_extensions = Vec::new();
5152
for extension in AssetLoader::extensions(&*loader) {
5253
let list = self
@@ -55,26 +56,29 @@ impl AssetLoaders {
5556
.or_default();
5657

5758
if !list.is_empty() {
58-
duplicate_extensions.push(extension);
59+
if let Some(existing_loaders_for_type_id) = existing_loaders_for_type_id {
60+
if list
61+
.iter()
62+
.any(|index| existing_loaders_for_type_id.contains(index))
63+
{
64+
duplicate_extensions.push(extension);
65+
}
66+
}
5967
}
6068

6169
list.push(loader_index);
6270
}
63-
64-
self.type_name_to_loader.insert(type_name, loader_index);
65-
66-
let list = self
67-
.type_id_to_loaders
68-
.entry(loader_asset_type)
69-
.or_default();
70-
71-
let duplicate_asset_registration = !list.is_empty();
72-
if !duplicate_extensions.is_empty() && duplicate_asset_registration {
71+
if !duplicate_extensions.is_empty() {
7372
warn!("Duplicate AssetLoader registered for Asset type `{loader_asset_type_name}` with extensions `{duplicate_extensions:?}`. \
7473
Loader must be specified in a .meta file in order to load assets of this type with these extensions.");
7574
}
7675

77-
list.push(loader_index);
76+
self.type_name_to_loader.insert(type_name, loader_index);
77+
78+
self.type_id_to_loaders
79+
.entry(loader_asset_type)
80+
.or_default()
81+
.push(loader_index);
7882

7983
self.loaders.push(MaybeAssetLoader::Ready(loader));
8084
} else {
@@ -108,6 +112,8 @@ impl AssetLoaders {
108112

109113
self.preregistered_loaders.insert(type_name, loader_index);
110114
self.type_name_to_loader.insert(type_name, loader_index);
115+
116+
let existing_loaders_for_type_id = self.type_id_to_loaders.get(&loader_asset_type);
111117
let mut duplicate_extensions = Vec::new();
112118
for extension in extensions {
113119
let list = self
@@ -116,24 +122,27 @@ impl AssetLoaders {
116122
.or_default();
117123

118124
if !list.is_empty() {
119-
duplicate_extensions.push(extension);
125+
if let Some(existing_loaders_for_type_id) = existing_loaders_for_type_id {
126+
if list
127+
.iter()
128+
.any(|index| existing_loaders_for_type_id.contains(index))
129+
{
130+
duplicate_extensions.push(extension);
131+
}
132+
}
120133
}
121134

122135
list.push(loader_index);
123136
}
124-
125-
let list = self
126-
.type_id_to_loaders
127-
.entry(loader_asset_type)
128-
.or_default();
129-
130-
let duplicate_asset_registration = !list.is_empty();
131-
if !duplicate_extensions.is_empty() && duplicate_asset_registration {
137+
if !duplicate_extensions.is_empty() {
132138
warn!("Duplicate AssetLoader preregistered for Asset type `{loader_asset_type_name}` with extensions `{duplicate_extensions:?}`. \
133139
Loader must be specified in a .meta file in order to load assets of this type with these extensions.");
134140
}
135141

136-
list.push(loader_index);
142+
self.type_id_to_loaders
143+
.entry(loader_asset_type)
144+
.or_default()
145+
.push(loader_index);
137146

138147
let (mut sender, receiver) = async_broadcast::broadcast(1);
139148
sender.set_overflow(true);

0 commit comments

Comments
 (0)