From bf8acca375e64632cd2e8970e72a2f5fd1379628 Mon Sep 17 00:00:00 2001 From: juliancoffee Date: Mon, 2 Aug 2021 14:29:45 +0300 Subject: [PATCH 1/6] Introduce helper functions and deduplicate docs --- common/assets/src/lib.rs | 80 +++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 38 deletions(-) diff --git a/common/assets/src/lib.rs b/common/assets/src/lib.rs index b23f27bb2f..f504c017f8 100644 --- a/common/assets/src/lib.rs +++ b/common/assets/src/lib.rs @@ -322,32 +322,52 @@ pub mod asset_tweak { const EXTENSION: &'static str = "ron"; } + // helper function to load asset and return value + // asset_specifier is full "path" to asset relative to ASSETS_PATH. + fn read_expect(asset_specifier: &str) -> T + where + T: Clone + Sized + Send + Sync + 'static + DeserializeOwned, + { + let handle = as AssetExt>::load_expect(asset_specifier); + let AssetTweakWrapper(value) = handle.read().clone(); + value + } + + // helper function to create new file to tweak + // the file will be filled with passed value + // returns passed value + fn create_new(tweak_dir: &Path, filename: &str, value: T) -> T + where + T: Clone + Sized + Send + Sync + 'static + DeserializeOwned + Serialize, + { + fs::create_dir_all(tweak_dir).expect("failed to create directory for tweak files"); + let f = fs::File::create(tweak_dir.join(filename)).unwrap_or_else(|error| { + panic!("failed to create file {:?}. Error: {:?}", filename, error) + }); + // TODO: can we not require clone here? + let tweaker = AssetTweakWrapper(value.clone()); + if let Err(e) = to_writer_pretty(f, &tweaker, PrettyConfig::new()) { + panic!("failed to write to file {:?}. Error: {:?}", filename, e); + } + + value + } + /// # Usage - /// Create file with content which represent tweaked value + /// Read value from file using our asset cache machinery. /// - /// Example if you want to tweak integer value - /// ```no_run - /// use veloren_common_assets::asset_tweak; - /// let x: i32 = asset_tweak::tweak_expect("x"); - /// ``` - /// File needs to look like that - /// ```text - /// assets/tweak/x.ron - /// (5) - /// ``` - /// Note the parentheses. + /// Will hot-reload (if corresponded feature is enabled). /// - /// # Panics - /// 1) If given `asset_specifier` does not exists - /// 2) If asseet is broken + /// If you don't have a file or its content is invalid, + /// this function will panic. + /// + /// Read documentation for `tweak_expect_or_create` for more. pub fn tweak_expect(specifier: &str) -> T where T: Clone + Sized + Send + Sync + 'static + DeserializeOwned, { - let asset_specifier: &str = &format!("tweak.{}", specifier); - let handle = as AssetExt>::load_expect(asset_specifier); - let AssetTweakWrapper(value) = handle.read().clone(); - value + let asset_specifier = format!("tweak.{}", specifier); + read_expect(&asset_specifier) } /// # Usage @@ -369,10 +389,6 @@ pub mod asset_tweak { /// (5) /// ``` /// Note the parentheses. - /// - /// # Panics - /// 1) If asset is broken - /// 2) filesystem errors pub fn tweak_expect_or_create(specifier: &str, value: T) -> T where T: Clone + Sized + Send + Sync + 'static + DeserializeOwned + Serialize, @@ -386,22 +402,10 @@ pub mod asset_tweak { let filename = format!("{}.ron", specifier); if Path::new(&tweak_dir.join(&filename)).is_file() { - let asset_specifier: &str = &format!("tweak.{}", specifier); - let handle = as AssetExt>::load_expect(asset_specifier); - let AssetTweakWrapper(new_value) = handle.read().clone(); - - new_value + let asset_specifier = format!("tweak.{}", specifier); + read_expect(&asset_specifier) } else { - fs::create_dir_all(&tweak_dir).expect("failed to create directory for tweak files"); - let f = fs::File::create(tweak_dir.join(&filename)).unwrap_or_else(|err| { - panic!("failed to create file {:?}. Error: {:?}", &filename, err) - }); - to_writer_pretty(f, &AssetTweakWrapper(value.clone()), PrettyConfig::new()) - .unwrap_or_else(|err| { - panic!("failed to write to file {:?}. Error: {:?}", &filename, err) - }); - - value + create_new(&tweak_dir, &filename, value) } } From 4630237739c67127fdaf1ecdbca1b83ceeb3fdbe Mon Sep 17 00:00:00 2001 From: juliancoffee Date: Mon, 2 Aug 2021 16:27:44 +0300 Subject: [PATCH 2/6] Remove clone() for creating new tweak file --- common/assets/src/lib.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/common/assets/src/lib.rs b/common/assets/src/lib.rs index f504c017f8..a0316c46b0 100644 --- a/common/assets/src/lib.rs +++ b/common/assets/src/lib.rs @@ -322,7 +322,7 @@ pub mod asset_tweak { const EXTENSION: &'static str = "ron"; } - // helper function to load asset and return value + // helper function to load asset and return contained value // asset_specifier is full "path" to asset relative to ASSETS_PATH. fn read_expect(asset_specifier: &str) -> T where @@ -338,14 +338,13 @@ pub mod asset_tweak { // returns passed value fn create_new(tweak_dir: &Path, filename: &str, value: T) -> T where - T: Clone + Sized + Send + Sync + 'static + DeserializeOwned + Serialize, + T: Sized + Send + Sync + 'static + DeserializeOwned + Serialize, { fs::create_dir_all(tweak_dir).expect("failed to create directory for tweak files"); let f = fs::File::create(tweak_dir.join(filename)).unwrap_or_else(|error| { panic!("failed to create file {:?}. Error: {:?}", filename, error) }); - // TODO: can we not require clone here? - let tweaker = AssetTweakWrapper(value.clone()); + let tweaker = AssetTweakWrapper(&value); if let Err(e) = to_writer_pretty(f, &tweaker, PrettyConfig::new()) { panic!("failed to write to file {:?}. Error: {:?}", filename, e); } From b6cfd067248f24a5b773f259ba82759fffcbf196 Mon Sep 17 00:00:00 2001 From: juliancoffee Date: Mon, 2 Aug 2021 17:42:28 +0300 Subject: [PATCH 3/6] Add ability to specify directory in asset_tweak --- common/assets/src/lib.rs | 361 ++++++++++++++++++++++++++------------- 1 file changed, 239 insertions(+), 122 deletions(-) diff --git a/common/assets/src/lib.rs b/common/assets/src/lib.rs index a0316c46b0..a03483ab2a 100644 --- a/common/assets/src/lib.rs +++ b/common/assets/src/lib.rs @@ -303,13 +303,31 @@ mod tests { } } +/// Set of functions for easy tweaking values using our asset cache machinery. +/// +/// Will hot-reload (if corresponded feature is enabled). #[cfg(feature = "asset_tweak")] pub mod asset_tweak { - use super::{find_root, Asset, AssetExt, RonLoader}; + use super::{Asset, AssetExt, RonLoader, ASSETS_PATH}; use ron::ser::{to_writer_pretty, PrettyConfig}; use serde::{de::DeserializeOwned, Deserialize, Serialize}; use std::{fs, path::Path}; + /// Specifier to use with tweak functions in this module + /// + /// `Tweak("test")` will be interpreted as `/tweak/test.ron`. + /// + /// `Asset(&["path", "to", "file"])` will be interpreted as + /// `/path/to/file.ron` + // TODO: should we care about situation where + // lifetime of slice and lifetime of strings are different? + // + // Should we use references at all? + pub enum Specifier<'a> { + Tweak(&'a str), + Asset(&'a [&'a str]), + } + #[derive(Clone, Deserialize, Serialize)] struct AssetTweakWrapper(T); @@ -322,20 +340,65 @@ pub mod asset_tweak { const EXTENSION: &'static str = "ron"; } - // helper function to load asset and return contained value - // asset_specifier is full "path" to asset relative to ASSETS_PATH. - fn read_expect(asset_specifier: &str) -> T + /// Read value from file, will panic if file doesn't exist. + /// + /// If you don't have a file or its content is invalid, + /// this function will panic. + /// If you want to have some default content, + /// read documentation for [tweak_expect_or_create] for more. + /// + /// # Examples: + /// How not to use. + /// ```should_panic + /// use veloren_common_assets::asset_tweak::{tweak_expect, Specifier}; + /// + /// // will panic if you don't have a file + /// let specifier = Specifier::Asset(&["no_way_we_have_this_directory", "x"]); + /// let x: i32 = tweak_expect(specifier); + /// ``` + /// + /// How to use. + /// ``` + /// use std::fs; + /// use veloren_common_assets::{ + /// asset_tweak::{tweak_expect, Specifier}, + /// ASSETS_PATH, + /// }; + /// + /// // you need to create file first + /// let tweak_path = ASSETS_PATH.join("tweak/y.ron"); + /// // note parentheses + /// fs::write(&tweak_path, b"(10)"); + /// + /// let y: i32 = tweak_expect(Specifier::Tweak("y")); + /// assert_eq!(y, 10); + /// + /// // Specifier::Tweak is just a shorthand + /// // for Specifier::Asset(&["tweak", ..]) + /// let z: i32 = tweak_expect(Specifier::Asset(&["tweak", "y"])); + /// assert_eq!(y, 10); + /// + /// // you may want to remove this file later + /// std::fs::remove_file(tweak_path); + /// ``` + pub fn tweak_expect(specifier: Specifier) -> T where T: Clone + Sized + Send + Sync + 'static + DeserializeOwned, { - let handle = as AssetExt>::load_expect(asset_specifier); + let asset_specifier = match specifier { + Specifier::Tweak(specifier) => format!("tweak.{}", specifier), + Specifier::Asset(path) => path.join("."), + }; + let handle = as AssetExt>::load_expect(&asset_specifier); let AssetTweakWrapper(value) = handle.read().clone(); + value } - // helper function to create new file to tweak - // the file will be filled with passed value - // returns passed value + // Helper function to create new file to tweak. + // + // The file will be filled with passed value + // returns passed value. fn create_new(tweak_dir: &Path, filename: &str, value: T) -> T where T: Sized + Send + Sync + 'static + DeserializeOwned + Serialize, @@ -352,66 +415,71 @@ pub mod asset_tweak { value } - /// # Usage - /// Read value from file using our asset cache machinery. - /// - /// Will hot-reload (if corresponded feature is enabled). - /// - /// If you don't have a file or its content is invalid, - /// this function will panic. - /// - /// Read documentation for `tweak_expect_or_create` for more. - pub fn tweak_expect(specifier: &str) -> T - where - T: Clone + Sized + Send + Sync + 'static + DeserializeOwned, - { - let asset_specifier = format!("tweak.{}", specifier); - read_expect(&asset_specifier) + // Helper function to get directory and file from asset list. + // + // Converts ["path", "to", "file"] to (String("path/to"), "file") + fn directory_and_name<'a>(path: &'a [&'a str]) -> (String, &'a str) { + let (file, path) = path.split_last().expect("empty asset list"); + let directory = path.join("/"); + + (directory, file) } - /// # Usage - /// Will create file "assets/tweak/{specifier}.ron" if not exists - /// and return passed `value`. - /// If file exists will read a value from such file. + /// Read a value from asset, creating file if not exists. /// - /// In release builds (if `debug_assertions` == false) just returns passed - /// `value` + /// If file exists will read a value from such file + /// using [tweak_expect]. /// - /// Example if you want to tweak integer value - /// ```no_run - /// use veloren_common_assets::asset_tweak; - /// let x: i32 = asset_tweak::tweak_expect_or_create("x", 5); - /// ``` - /// File needs to look like that + /// File should look like that (note the parentheses). /// ```text /// assets/tweak/x.ron /// (5) /// ``` - /// Note the parentheses. - pub fn tweak_expect_or_create(specifier: &str, value: T) -> T + /// + /// # Example: + /// Tweaking integer value + /// ``` + /// use veloren_common_assets::{ + /// asset_tweak::{tweak_expect_or_create, Specifier}, + /// ASSETS_PATH, + /// }; + /// + /// // first time it will create the file + /// let x: i32 = tweak_expect_or_create(Specifier::Tweak("x"), 5); + /// let file_path = ASSETS_PATH.join("tweak/x.ron"); + /// assert!(file_path.is_file()); + /// assert_eq!(x, 5); + /// + /// // next time it will read value from file + /// // whatever you will pass as default + /// let x: i32 = tweak_expect_or_create(Specifier::Tweak("x"), 42); + /// assert_eq!(x, 5); + /// + /// // you may want to remove this file later + /// std::fs::remove_file(file_path); + /// ``` + pub fn tweak_expect_or_create(specifier: Specifier, value: T) -> T where T: Clone + Sized + Send + Sync + 'static + DeserializeOwned + Serialize, { - if cfg!(not(debug_assertions)) { - return value; - } + let (dir, filename) = match specifier { + Specifier::Tweak(name) => (ASSETS_PATH.join("tweak"), format!("{}.ron", name)), + Specifier::Asset(list) => { + let (directory, name) = directory_and_name(list); + (ASSETS_PATH.join(directory), format!("{}.ron", name)) + }, + }; - let root = find_root().expect("failed to discover repository_root"); - let tweak_dir = root.join("assets/tweak/"); - let filename = format!("{}.ron", specifier); - - if Path::new(&tweak_dir.join(&filename)).is_file() { - let asset_specifier = format!("tweak.{}", specifier); - read_expect(&asset_specifier) + if Path::new(&dir.join(&filename)).is_file() { + tweak_expect(specifier) } else { - create_new(&tweak_dir, &filename, value) + create_new(&dir, &filename, value) } } #[cfg(test)] mod tests { - use super::{find_root, tweak_expect, tweak_expect_or_create}; - use serial_test::serial; + use super::*; use std::{ convert::AsRef, fmt::Debug, @@ -469,90 +537,139 @@ pub mod asset_tweak { P: AsRef + Debug, { fn drop(&mut self) { - fs::remove_file(&self.file) - .unwrap_or_else(|_| panic!("failed to create file {:?}", &self.file)); + fs::remove_file(&self.file).unwrap_or_else(|e| { + panic!("failed to remove file {:?}. Error: {:?}", &self.file, e) + }); } } - #[test] - #[serial] - fn test_tweaked_string() { - let root = find_root().expect("failed to discover repository_root"); - let tweak_dir = root.join("assets/tweak/"); - let _dir_guard = DirectoryGuard::create(tweak_dir.clone()); + // helper function to create environment with needed directory and file + // and responsible for cleaning + fn run_with_file(tweak_path: &[&str], test: impl Fn(&mut File)) { + let (tweak_dir, tweak_name) = directory_and_name(tweak_path); + let tweak_folder = ASSETS_PATH.join(tweak_dir); + let tweak_file = tweak_folder.join(format!("{}.ron", tweak_name)); - // define test files - let from_int = tweak_dir.join("__test_int_tweak.ron"); - let from_string = tweak_dir.join("__test_string_tweak.ron"); - let from_map = tweak_dir.join("__test_map_tweak.ron"); + let _dir_guard = DirectoryGuard::create(tweak_folder); + let (_file_guard, mut file) = FileGuard::create(tweak_file); - // setup fs guards - let (_file_guard1, mut file1) = FileGuard::create(from_int); - let (_file_guard2, mut file2) = FileGuard::create(from_string); - let (_file_guard3, mut file3) = FileGuard::create(from_map); - - // write to file and check result - file1 - .write_all(b"(5)") - .expect("failed to write to the file"); - let x = tweak_expect::("__test_int_tweak"); - assert_eq!(x, 5); - - // write to file and check result - file2 - .write_all(br#"("Hello Zest")"#) - .expect("failed to write to the file"); - let x = tweak_expect::("__test_string_tweak"); - assert_eq!(x, "Hello Zest".to_owned()); - - // write to file and check result - file3 - .write_all( - br#" - ({ - "wow": 4, - "such": 5, - }) - "#, - ) - .expect("failed to write to the file"); - let x: std::collections::HashMap = tweak_expect("__test_map_tweak"); - let mut map = std::collections::HashMap::new(); - map.insert("wow".to_owned(), 4); - map.insert("such".to_owned(), 5); - assert_eq!(x, map); + test(&mut file); } #[test] - #[serial] - fn test_tweaked_create() { - let root = find_root().expect("failed to discover repository_root"); - let tweak_dir = root.join("assets/tweak/"); + fn test_tweaked_int() { + use Specifier::Asset; - let test_path1 = tweak_dir.join("__test_int_create.ron"); - let _file_guard1 = FileGuard::hold(&test_path1); - let x = tweak_expect_or_create("__test_int_create", 5); - assert_eq!(x, 5); - assert!(test_path1.is_file()); - // Recheck it loads back correctly - let x = tweak_expect_or_create("__test_int_create", 5); - assert_eq!(x, 5); + let tweak_path = &["tweak_test_int", "tweak"]; - let test_path2 = tweak_dir.join("__test_tuple_create.ron"); - let _file_guard2 = FileGuard::hold(&test_path2); - let (x, y, z) = tweak_expect_or_create("__test_tuple_create", (5.0, 6.0, 7.0)); - assert_eq!((x, y, z), (5.0, 6.0, 7.0)); - // Recheck it loads back correctly - let (x, y, z) = tweak_expect_or_create("__test_tuple_create", (5.0, 6.0, 7.0)); - assert_eq!((x, y, z), (5.0, 6.0, 7.0)); + run_with_file(tweak_path, |file| { + file.write_all(b"(5)").expect("failed to write to the file"); + let x: i32 = tweak_expect(Asset(tweak_path)); + assert_eq!(x, 5); + }); + } - // Test that file has stronger priority - let test_path3 = tweak_dir.join("__test_priority.ron"); - let (_file_guard3, mut file) = FileGuard::create(&test_path3); - file.write_all(b"(10)") + #[test] + fn test_tweaked_string() { + use Specifier::Asset; + let tweak_path = &["tweak_test_string", "tweak"]; + + run_with_file(tweak_path, |file| { + file.write_all(br#"("Hello Zest")"#) + .expect("failed to write to the file"); + + let x: String = tweak_expect(Asset(tweak_path)); + assert_eq!(x, "Hello Zest".to_owned()); + }); + } + + #[test] + fn test_tweaked_hashmap() { + use Specifier::Asset; + type Map = std::collections::HashMap; + + let tweak_path = &["tweak_test_map", "tweak"]; + + run_with_file(tweak_path, |file| { + file.write_all( + br#" + ({ + "wow": 4, + "such": 5, + }) + "#, + ) .expect("failed to write to the file"); - let x = tweak_expect_or_create("__test_priority", 6); - assert_eq!(x, 10); + + let x: Map = tweak_expect(Asset(tweak_path)); + + let mut map = Map::new(); + map.insert("wow".to_owned(), 4); + map.insert("such".to_owned(), 5); + assert_eq!(x, map); + }); + } + + fn run_with_path(tweak_path: &[&str], test: impl Fn(&Path)) { + let (tweak_dir, tweak_name) = directory_and_name(tweak_path); + + let tweak_folder = ASSETS_PATH.join(tweak_dir); + let test_path = tweak_folder.join(format!("{}.ron", tweak_name)); + + let _file_guard = FileGuard::hold(&test_path); + + test(&test_path); + } + + #[test] + fn test_create_tweak() { + use Specifier::Asset; + + let tweak_path = &["tweak_create_test", "tweak"]; + + run_with_path(tweak_path, |test_path| { + let x = tweak_expect_or_create(Asset(tweak_path), 5); + assert_eq!(x, 5); + assert!(test_path.is_file()); + // Recheck it loads back correctly + let x = tweak_expect_or_create(Asset(tweak_path), 5); + assert_eq!(x, 5); + }); + } + + #[test] + fn test_create_tweak_deep() { + use Specifier::Asset; + + let tweak_path = &["so_much", "deep_test", "tweak_create_test", "tweak"]; + + run_with_path(tweak_path, |test_path| { + let x = tweak_expect_or_create(Asset(tweak_path), 5); + assert_eq!(x, 5); + assert!(test_path.is_file()); + // Recheck it loads back correctly + let x = tweak_expect_or_create(Asset(tweak_path), 5); + assert_eq!(x, 5); + }); + } + + #[test] + fn test_create_but_prioritize_loaded() { + use Specifier::Asset; + + let tweak_path = &["tweak_create_and_prioritize_test", "tweak"]; + + run_with_path(tweak_path, |test_path| { + let x = tweak_expect_or_create(Asset(tweak_path), 5); + assert_eq!(x, 5); + assert!(test_path.is_file()); + + // Recheck it loads back + // with content as priority + fs::write(test_path, b"(10)").expect("failed to write to the file"); + let x = tweak_expect_or_create(Asset(tweak_path), 5); + assert_eq!(x, 10); + }); } } } From f9733b58cb4b2752756975c4b46766ff0e5e1c3a Mon Sep 17 00:00:00 2001 From: juliancoffee Date: Tue, 3 Aug 2021 01:44:19 +0300 Subject: [PATCH 4/6] Remove redundant serial_test crate --- Cargo.lock | 23 ----------------------- common/assets/Cargo.toml | 3 +-- 2 files changed, 1 insertion(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3132f6e3fc..283b9489c4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4831,28 +4831,6 @@ dependencies = [ "syn 1.0.73", ] -[[package]] -name = "serial_test" -version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e0bccbcf40c8938196944a3da0e133e031a33f4d6b72db3bda3cc556e361905d" -dependencies = [ - "lazy_static", - "parking_lot 0.11.1", - "serial_test_derive", -] - -[[package]] -name = "serial_test_derive" -version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b2acd6defeddb41eb60bb468f8825d0cfd0c2a76bc03bfd235b6a1dc4f6a1ad5" -dependencies = [ - "proc-macro2 1.0.27", - "quote 1.0.9", - "syn 1.0.73", -] - [[package]] name = "sha1" version = "0.6.0" @@ -5920,7 +5898,6 @@ dependencies = [ "lazy_static", "ron", "serde", - "serial_test", "tracing", "walkdir 2.3.2", ] diff --git a/common/assets/Cargo.toml b/common/assets/Cargo.toml index 4b46aecac6..ae7fb642c8 100644 --- a/common/assets/Cargo.toml +++ b/common/assets/Cargo.toml @@ -15,11 +15,10 @@ tracing = "0.1" # asset tweak serde = {version = "1.0", features = ["derive"], optional = true} -serial_test = {version = "0.5", optional = true} [dev-dependencies] walkdir = "2.3.2" [features] hot-reloading = ["assets_manager/hot-reloading"] -asset_tweak = ["serial_test", "serde", "hot-reloading"] +asset_tweak = ["serde", "hot-reloading"] From d4d6072c035b130303d895c7428649d02a537907 Mon Sep 17 00:00:00 2001 From: juliancoffee Date: Tue, 3 Aug 2021 16:13:35 +0300 Subject: [PATCH 5/6] Add asset_tweak macros --- common/assets/src/lib.rs | 122 +++++++++++++++++++++++++++++++++++---- 1 file changed, 111 insertions(+), 11 deletions(-) diff --git a/common/assets/src/lib.rs b/common/assets/src/lib.rs index a03483ab2a..80122a52af 100644 --- a/common/assets/src/lib.rs +++ b/common/assets/src/lib.rs @@ -303,11 +303,17 @@ mod tests { } } -/// Set of functions for easy tweaking values using our asset cache machinery. -/// -/// Will hot-reload (if corresponded feature is enabled). #[cfg(feature = "asset_tweak")] pub mod asset_tweak { + //! Set of functions and macros for easy tweaking values + //! using our asset cache machinery. + //! + //! Because of how macros works, you will not find + //! [tweak] and [tweak_from] macros in this module, + //! import it from [assets](super) crate directly. + //! + //! Will hot-reload (if corresponded feature is enabled). + // TODO: don't use the same ASSETS_PATH as game uses? use super::{Asset, AssetExt, RonLoader, ASSETS_PATH}; use ron::ser::{to_writer_pretty, PrettyConfig}; use serde::{de::DeserializeOwned, Deserialize, Serialize}; @@ -366,17 +372,17 @@ pub mod asset_tweak { /// }; /// /// // you need to create file first - /// let tweak_path = ASSETS_PATH.join("tweak/y.ron"); + /// let tweak_path = ASSETS_PATH.join("tweak/year.ron"); /// // note parentheses /// fs::write(&tweak_path, b"(10)"); /// - /// let y: i32 = tweak_expect(Specifier::Tweak("y")); + /// let y: i32 = tweak_expect(Specifier::Tweak("year")); /// assert_eq!(y, 10); /// /// // Specifier::Tweak is just a shorthand /// // for Specifier::Asset(&["tweak", ..]) - /// let z: i32 = tweak_expect(Specifier::Asset(&["tweak", "y"])); - /// assert_eq!(y, 10); + /// let y1: i32 = tweak_expect(Specifier::Asset(&["tweak", "year"])); + /// assert_eq!(y1, 10); /// /// // you may want to remove this file later /// std::fs::remove_file(tweak_path); @@ -445,15 +451,15 @@ pub mod asset_tweak { /// }; /// /// // first time it will create the file - /// let x: i32 = tweak_expect_or_create(Specifier::Tweak("x"), 5); - /// let file_path = ASSETS_PATH.join("tweak/x.ron"); + /// let x: i32 = tweak_expect_or_create(Specifier::Tweak("stars"), 5); + /// let file_path = ASSETS_PATH.join("tweak/stars.ron"); /// assert!(file_path.is_file()); /// assert_eq!(x, 5); /// /// // next time it will read value from file /// // whatever you will pass as default - /// let x: i32 = tweak_expect_or_create(Specifier::Tweak("x"), 42); - /// assert_eq!(x, 5); + /// let x1: i32 = tweak_expect_or_create(Specifier::Tweak("stars"), 42); + /// assert_eq!(x1, 5); /// /// // you may want to remove this file later /// std::fs::remove_file(file_path); @@ -477,6 +483,100 @@ pub mod asset_tweak { } } + /// Convinient macro to quickly tweak value. + /// + /// Will use [Specifier]`::Tweak` specifier and call + /// [tweak_expect] if passed only name + /// or [tweak_expect_or_create] if default is passed. + /// + /// # Examples: + /// ``` + /// // note that you need to export it from `assets` crate, + /// // not from `assets::asset_tweak` + /// use veloren_common_assets::{tweak, ASSETS_PATH}; + /// + /// // you need to create file first + /// let own_path = ASSETS_PATH.join("tweak/grizelda.ron"); + /// // note parentheses + /// std::fs::write(&own_path, b"(10)"); + /// + /// let z: i32 = tweak!("grizelda"); + /// assert_eq!(z, 10); + /// + /// // voila, you don't need to care about creating file first + /// let p: i32 = tweak!("peter", 8); + /// + /// let created_path = ASSETS_PATH.join("tweak/peter.ron"); + /// assert!(created_path.is_file()); + /// assert_eq!(p, 8); + /// + /// // will use default value only first time + /// // if file exists, will load from this file + /// let p: i32 = tweak!("peter", 50); + /// assert_eq!(p, 8); + /// + /// // you may want to remove this file later + /// std::fs::remove_file(own_path); + /// std::fs::remove_file(created_path); + /// ``` + #[macro_export] + macro_rules! tweak { + ($name:literal) => {{ + use $crate::asset_tweak::{tweak_expect, Specifier::Tweak}; + + tweak_expect(Tweak($name)) + }}; + + ($name:literal, $default:expr) => {{ + use $crate::asset_tweak::{tweak_expect_or_create, Specifier::Tweak}; + + tweak_expect_or_create(Tweak($name), $default) + }}; + } + + /// Convinient macro to quickly tweak value from some existing path. + /// + /// Will use [Specifier]`::Asset` specifier and call + /// [tweak_expect] if passed only name + /// or [tweak_expect_or_create] if default is passed. + /// + /// The main use case is when you have some object + /// which needs constant tuning of values, but you can't afford + /// loading a file. + /// So you can use tweak_from! and then just copy values from asset + /// to your object. + /// + /// # Examples: + /// ```no_run + /// // note that you need to export it from `assets` crate, + /// // not from `assets::asset_tweak` + /// use serde::{Deserialize, Serialize}; + /// use veloren_common_assets::{tweak_from, ASSETS_PATH}; + /// + /// #[derive(Clone, PartialEq, Deserialize, Serialize)] + /// struct Data { + /// x: i32, + /// y: i32, + /// } + /// + /// let default = Data { x: 5, y: 7 }; + /// let data: Data = tweak_from!(&["common", "body", "dimensions"], default); + /// ``` + #[macro_export] + macro_rules! tweak_from { + ($path:expr) => {{ + use $crate::asset_tweak::{tweak_expect, Specifier::Asset}; + + tweak_expect(Asset($path)) + }}; + + ($path:expr, $default:expr) => {{ + use $crate::asset_tweak::{tweak_expect_or_create, Specifier::Asset}; + + tweak_expect_or_create(Asset($path), $default) + }}; + } + #[cfg(test)] mod tests { use super::*; From ce04dd5c69a141f773ca051f6026f82cfe053899 Mon Sep 17 00:00:00 2001 From: juliancoffee Date: Fri, 6 Aug 2021 12:57:29 +0300 Subject: [PATCH 6/6] Adress review, add test for structs --- common/assets/src/lib.rs | 63 +++++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 23 deletions(-) diff --git a/common/assets/src/lib.rs b/common/assets/src/lib.rs index 80122a52af..fb21436cfa 100644 --- a/common/assets/src/lib.rs +++ b/common/assets/src/lib.rs @@ -325,10 +325,6 @@ pub mod asset_tweak { /// /// `Asset(&["path", "to", "file"])` will be interpreted as /// `/path/to/file.ron` - // TODO: should we care about situation where - // lifetime of slice and lifetime of strings are different? - // - // Should we use references at all? pub enum Specifier<'a> { Tweak(&'a str), Asset(&'a [&'a str]), @@ -658,34 +654,30 @@ pub mod asset_tweak { #[test] fn test_tweaked_int() { - use Specifier::Asset; - let tweak_path = &["tweak_test_int", "tweak"]; run_with_file(tweak_path, |file| { file.write_all(b"(5)").expect("failed to write to the file"); - let x: i32 = tweak_expect(Asset(tweak_path)); + let x: i32 = tweak_expect(Specifier::Asset(tweak_path)); assert_eq!(x, 5); }); } #[test] fn test_tweaked_string() { - use Specifier::Asset; let tweak_path = &["tweak_test_string", "tweak"]; run_with_file(tweak_path, |file| { file.write_all(br#"("Hello Zest")"#) .expect("failed to write to the file"); - let x: String = tweak_expect(Asset(tweak_path)); + let x: String = tweak_expect(Specifier::Asset(tweak_path)); assert_eq!(x, "Hello Zest".to_owned()); }); } #[test] fn test_tweaked_hashmap() { - use Specifier::Asset; type Map = std::collections::HashMap; let tweak_path = &["tweak_test_map", "tweak"]; @@ -701,7 +693,7 @@ pub mod asset_tweak { ) .expect("failed to write to the file"); - let x: Map = tweak_expect(Asset(tweak_path)); + let x: Map = tweak_expect(Specifier::Asset(tweak_path)); let mut map = Map::new(); map.insert("wow".to_owned(), 4); @@ -710,6 +702,37 @@ pub mod asset_tweak { }); } + #[test] + fn test_tweaked_with_macro_struct() { + // partial eq and debug because of assert_eq in this test + #[derive(Debug, PartialEq, Clone, Deserialize, Serialize)] + struct Wow { + such: i32, + field: f32, + } + + let tweak_path = &["tweak_test_struct", "tweak"]; + + run_with_file(tweak_path, |file| { + file.write_all( + br#" + (( + such: 5, + field: 35.752346, + )) + "#, + ) + .expect("failed to write to the file"); + + let x: Wow = crate::tweak_from!(tweak_path); + let expected = Wow { + such: 5, + field: 35.752_346, + }; + assert_eq!(x, expected); + }); + } + fn run_with_path(tweak_path: &[&str], test: impl Fn(&Path)) { let (tweak_dir, tweak_name) = directory_and_name(tweak_path); @@ -723,51 +746,45 @@ pub mod asset_tweak { #[test] fn test_create_tweak() { - use Specifier::Asset; - let tweak_path = &["tweak_create_test", "tweak"]; run_with_path(tweak_path, |test_path| { - let x = tweak_expect_or_create(Asset(tweak_path), 5); + let x = tweak_expect_or_create(Specifier::Asset(tweak_path), 5); assert_eq!(x, 5); assert!(test_path.is_file()); // Recheck it loads back correctly - let x = tweak_expect_or_create(Asset(tweak_path), 5); + let x = tweak_expect_or_create(Specifier::Asset(tweak_path), 5); assert_eq!(x, 5); }); } #[test] fn test_create_tweak_deep() { - use Specifier::Asset; - let tweak_path = &["so_much", "deep_test", "tweak_create_test", "tweak"]; run_with_path(tweak_path, |test_path| { - let x = tweak_expect_or_create(Asset(tweak_path), 5); + let x = tweak_expect_or_create(Specifier::Asset(tweak_path), 5); assert_eq!(x, 5); assert!(test_path.is_file()); // Recheck it loads back correctly - let x = tweak_expect_or_create(Asset(tweak_path), 5); + let x = tweak_expect_or_create(Specifier::Asset(tweak_path), 5); assert_eq!(x, 5); }); } #[test] fn test_create_but_prioritize_loaded() { - use Specifier::Asset; - let tweak_path = &["tweak_create_and_prioritize_test", "tweak"]; run_with_path(tweak_path, |test_path| { - let x = tweak_expect_or_create(Asset(tweak_path), 5); + let x = tweak_expect_or_create(Specifier::Asset(tweak_path), 5); assert_eq!(x, 5); assert!(test_path.is_file()); // Recheck it loads back // with content as priority fs::write(test_path, b"(10)").expect("failed to write to the file"); - let x = tweak_expect_or_create(Asset(tweak_path), 5); + let x = tweak_expect_or_create(Specifier::Asset(tweak_path), 5); assert_eq!(x, 10); }); }