Remove load_expect_dir as it's misused

- load_expect_dir while expected to fail on erros, fails only on
filesystem errors and only on root directory.
This commit replaces this function with `read_expect_dir` which returns iterator
which loads all files and panics if can't load them.

common/assets/src/lib.rs

@@ -1,3 +1,4 @@
+//#![warn(clippy::pedantic)]
 //! Load assets (images or voxel data) from files
 
 use dot_vox::DotVoxData;
@@ -86,6 +87,18 @@ pub trait AssetExt: Sized + Send + Sync + 'static {
     fn get_or_insert(specifier: &str, default: Self) -> AssetHandle<Self>;
 }
 
+/// Loads directory and all files in it
+///
+/// NOTE: If you call `.iter()` on it, all failed files will be ignored
+/// If you want to handle errors, call `.ids()` which will return
+/// iterator over assets specifiers
+///
+/// # Errors
+/// An error is returned if the given id does not match a valid readable
+/// directory.
+///
+/// When loading a directory recursively, directories that can't be read are
+/// ignored.
 pub fn load_dir<T: DirLoadable>(
     specifier: &str,
     recursive: bool,
@@ -94,10 +107,20 @@ pub fn load_dir<T: DirLoadable>(
     ASSETS.load_dir(specifier, recursive)
 }
 
+/// Loads directory and all files in it
+///
+/// # Panics
+/// 1) If can't load directory (filesystem errors)
+/// 2) If file can't be loaded (parsing problem)
 #[track_caller]
-pub fn load_expect_dir<T: DirLoadable>(specifier: &str, recursive: bool) -> AssetDirHandle<T> {
+pub fn read_expect_dir<T: DirLoadable>(
-    load_dir(specifier, recursive)
+    specifier: &str,
-        .unwrap_or_else(|e| panic!("Failed loading directory. error={:?}", e))
+    recursive: bool,
+) -> impl Iterator<Item = AssetGuard<T>> {
+    load_dir::<T>(specifier, recursive)
+        .unwrap_or_else(|e| panic!("Failed loading directory {}. error={:?}", e, specifier))
+        .ids()
+        .map(|entry| T::load_expect(entry).read())
 }
 
 impl<T: Compound> AssetExt for T {
@@ -266,8 +289,7 @@ mod tests {
             .filter(|path| path.is_file())
             .filter(|path| {
                 path.extension()
-                    .map(|e| ext == e.to_ascii_lowercase())
+                    .map_or(false, |e| ext == e.to_ascii_lowercase())
-                    .unwrap_or(false)
             })
             .for_each(|path| {
                 let file = File::open(&path).expect("Failed to open the file");
@@ -280,7 +302,6 @@ mod tests {
     }
 }
 
-#[warn(clippy::pedantic)]
 #[cfg(feature = "asset_tweak")]
 pub mod asset_tweak {
     use super::{find_root, Asset, AssetExt, RonLoader};
@@ -300,7 +321,6 @@ pub mod asset_tweak {
         const EXTENSION: &'static str = "ron";
     }
 
-    #[must_use]
     /// # Usage
     /// Create file with content which represent tweaked value
     ///
@@ -329,7 +349,6 @@ pub mod asset_tweak {
         value
     }
 
-    #[must_use]
     /// # Usage
     /// Will create file "assets/tweak/{specifier}.ron" if not exists
     /// and return passed `value`.

common/src/comp/inventory/item/mod.rs

@@ -625,6 +625,7 @@ impl Item {
     pub fn new_from_asset_glob(asset_glob: &str) -> Result<Vec<Self>, Error> {
         let specifier = asset_glob.strip_suffix(".*").unwrap_or(asset_glob);
         let defs = assets::load_dir::<RawItemDef>(specifier, true)?;
+        // use ids() instead of iter() because we don't want to ignore errors
         defs.ids().map(Item::new_from_asset).collect()
     }
 
@@ -893,7 +894,8 @@ mod tests {
 
     #[test]
     fn test_assets_items() {
-        let defs = assets::load_expect_dir::<RawItemDef>("common.items", true);
+        let defs = assets::load_dir::<RawItemDef>("common.items", true)
+            .expect("Failed to access items directory");
         for item in defs.ids().map(Item::new_from_asset_expect) {
             std::mem::drop(item)
         }

common/src/comp/inventory/loadout_builder.rs

@@ -736,10 +736,9 @@ mod tests {
         // It just load everything that could
         // TODO: add some checks, e.g. that Armor(Head) key correspond
         // to Item with ItemKind Head(_)
-        let loadouts = assets::load_expect_dir::<LoadoutSpec>("common.loadout", true);
+        let loadouts = assets::read_expect_dir::<LoadoutSpec>("common.loadout", true);
-        for loadout in loadouts.iter() {
+        for loadout in loadouts {
-            let spec = loadout.read();
+            for (&key, entry) in &loadout.0 {
-            for (&key, entry) in &spec.0 {
                 entry.validate(key);
             }
         }

common/src/generation.rs

@@ -312,8 +312,8 @@ mod tests {
     #[test]
     fn test_all_entity_assets() {
         // It just load everything that could
-        let entity_configs = assets::load_expect_dir::<EntityConfig>("common.entity", true);
+        let entity_configs = assets::read_expect_dir::<EntityConfig>("common.entity", true);
-        for config in entity_configs.iter() {
+        for config in entity_configs {
             let EntityConfig {
                 main_tool,
                 second_tool,
@@ -322,7 +322,7 @@ mod tests {
                 name: _name,
                 body,
                 loot,
-            } = config.cloned();
+            } = config.clone();
 
             if let Some(main_tool) = main_tool {
                 main_tool.validate(EquipSlot::ActiveMainhand);

common/src/lottery.rs

@@ -146,9 +146,9 @@ mod tests {
         }
 
         let loot_tables =
-            assets::load_expect_dir::<Lottery<LootSpec<String>>>("common.loot_tables", true);
+            assets::read_expect_dir::<Lottery<LootSpec<String>>>("common.loot_tables", true);
-        for loot_table in loot_tables.iter() {
+        for loot_table in loot_tables {
-            validate_table_contents(loot_table.cloned());
+            validate_table_contents(loot_table.clone());
         }
     }
 }

common/src/skillset_builder.rs

@@ -40,7 +40,7 @@ fn skills_from_nodes(nodes: &[SkillNode]) -> Vec<(Skill, Option<u16>)> {
     for node in nodes {
         match node {
             SkillNode::Tree(asset) => {
-                skills.append(&mut skills_from_asset_expect(&asset));
+                skills.append(&mut skills_from_asset_expect(asset));
             },
             SkillNode::Skill(req) => {
                 skills.push(*req);
@@ -151,11 +151,11 @@ mod tests {
 
     #[test]
     fn test_all_skillset_assets() {
-        let skillsets = assets::load_expect_dir::<SkillSetTree>("common.skillset", true);
+        let skillsets = assets::read_expect_dir::<SkillSetTree>("common.skillset", true);
-        for skillset in skillsets.iter() {
+        for skillset in skillsets {
             std::mem::drop({
                 let mut skillset_builder = SkillSetBuilder::default();
-                let nodes = &*skillset.read().0;
+                let nodes = &*skillset.0;
                 let tree = skills_from_nodes(nodes);
                 for (skill, level) in tree {
                     skillset_builder = skillset_builder.with_skill(skill, level);

voxygen/i18n/src/data.rs

@@ -372,7 +372,8 @@ impl assets::Compound for LocalizationList {
         specifier: &str,
     ) -> Result<Self, assets::Error> {
         // List language directories
-        let languages = assets::load_expect_dir::<FindManifests>(specifier, false)
+        let languages = assets::load_dir::<FindManifests>(specifier, false)
+            .unwrap_or_else(|e| panic!("Failed to get manifests from {}: {:?}", specifier, e))
             .ids()
             .filter_map(|spec| cache.load::<RawLocalization>(spec).ok())
             .map(|localization| localization.read().metadata.clone())