From 85cee14643d1489b40ec3f0ce4f7a5eca0b4bab3 Mon Sep 17 00:00:00 2001 From: juliancoffee Date: Sun, 26 Jun 2022 21:46:15 +0300 Subject: [PATCH 01/11] Refactoring, extract FileOpts dependent methods --- world/src/sim/mod.rs | 328 +++++++++++++++++++++++-------------------- 1 file changed, 173 insertions(+), 155 deletions(-) diff --git a/world/src/sim/mod.rs b/world/src/sim/mod.rs index 0dd84858b7..be5befc8aa 100644 --- a/world/src/sim/mod.rs +++ b/world/src/sim/mod.rs @@ -183,6 +183,176 @@ impl Default for FileOpts { fn default() -> Self { Self::Generate(SizeOpts::default()) } } +impl FileOpts { + fn content(&self) -> (Option, MapSizeLg, f64) { + let parsed_world_file = self.try_to_map(); + + let map_size_lg = parsed_world_file + .as_ref() + .and_then(|map| match MapSizeLg::new(map.map_size_lg) { + Ok(map_size_lg) => Some(map_size_lg), + Err(e) => { + warn!("World size of map does not satisfy invariants: {:?}", e); + None + }, + }) + .unwrap_or_else(|| self.map_size()); + + // NOTE: Change 1.0 to 4.0 for a 4x + // improvement in world detail. We also use this to automatically adjust + // grid_scale (multiplying by 4.0) and multiply mins_per_sec by + // 1.0 / (4.0 * 4.0) in ./erosion.rs, in order to get a similar rate of river + // formation. + // + // FIXME: This is a hack! At some point we will have a more principled way of + // dealing with this. + let default_continent_scale_hack = 2.0/*4.0*/; + let continent_scale_hack = if let Some(map) = &parsed_world_file { + map.continent_scale_hack + } else { + self.continent_scale_hack(default_continent_scale_hack) + }; + + (parsed_world_file, map_size_lg, continent_scale_hack) + } + + fn map_size(&self) -> MapSizeLg { + match self { + Self::Generate(opts) | Self::Save(opts) => MapSizeLg::new(Vec2 { + x: opts.x_lg, + y: opts.y_lg, + }) + .unwrap_or_else(|e| { + warn!("World size does not satisfy invariants: {:?}", e); + DEFAULT_WORLD_CHUNKS_LG + }), + _ => DEFAULT_WORLD_CHUNKS_LG, + } + } + + fn continent_scale_hack(&self, default: f64) -> f64 { + match self { + Self::Generate(opts) | Self::Save(opts) => opts.scale, + _ => default, + } + } + + fn try_to_map(&self) -> Option { + let map = match self { + Self::LoadLegacy(ref path) => { + let file = match File::open(path) { + Ok(file) => file, + Err(e) => { + warn!(?e, ?path, "Couldn't read path for maps"); + return None; + }, + }; + + let reader = BufReader::new(file); + let map: WorldFileLegacy = match bincode::deserialize_from(reader) { + Ok(map) => map, + Err(e) => { + warn!( + ?e, + "Couldn't parse legacy map. Maybe you meant to try a regular load?" + ); + return None; + }, + }; + + map.into_modern() + }, + Self::Load(ref path) => { + let file = match File::open(path) { + Ok(file) => file, + Err(e) => { + warn!(?e, ?path, "Couldn't read path for maps"); + return None; + }, + }; + + let reader = BufReader::new(file); + let map: WorldFile = match bincode::deserialize_from(reader) { + Ok(map) => map, + Err(e) => { + warn!( + ?e, + "Couldn't parse modern map. Maybe you meant to try a legacy load?" + ); + return None; + }, + }; + + map.into_modern() + }, + Self::LoadAsset(ref specifier) => match WorldFile::load_owned(specifier) { + Ok(map) => map.into_modern(), + Err(err) => { + match err.reason().downcast_ref::() { + Some(e) => { + warn!(?e, ?specifier, "Couldn't read asset specifier for maps"); + }, + None => { + warn!( + ?err, + "Couldn't parse modern map. Maybe you meant to try a legacy load?" + ); + }, + } + return None; + }, + }, + Self::Generate { .. } | Self::Save { .. } => return None, + }; + + match map { + Ok(map) => Some(map), + Err(e) => { + match e { + WorldFileError::WorldSizeInvalid => { + warn!("World size of map is invalid."); + }, + } + None + }, + } + } + + fn save(&self, map: &WorldFile) { + if let FileOpts::Save { .. } = self { + use std::time::SystemTime; + // Check if folder exists and create it if it does not + let mut path = PathBuf::from("./maps"); + if !path.exists() { + if let Err(e) = std::fs::create_dir(&path) { + warn!(?e, ?path, "Couldn't create folder for map"); + return; + } + } + path.push(format!( + // TODO: Work out a nice bincode file extension. + "map_{}.bin", + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .map(|d| d.as_millis()) + .unwrap_or(0) + )); + let file = match File::create(path.clone()) { + Ok(file) => file, + Err(e) => { + warn!(?e, ?path, "Couldn't create file for maps"); + return; + }, + }; + + let writer = BufWriter::new(file); + if let Err(e) = bincode::serialize_into(writer, map) { + warn!(?e, "Couldn't write map"); + } + } + } +} + pub struct WorldOpts { /// Set to false to disable seeding elements during worldgen. pub seed_elements: bool, @@ -399,129 +569,9 @@ impl WorldSim { pub fn generate(seed: u32, opts: WorldOpts, threadpool: &rayon::ThreadPool) -> Self { let calendar = opts.calendar; // separate lifetime of elements let world_file = opts.world_file; + // Parse out the contents of various map formats into the values we need. - let parsed_world_file = (|| { - let map = match world_file { - FileOpts::LoadLegacy(ref path) => { - let file = match File::open(path) { - Ok(file) => file, - Err(e) => { - warn!(?e, ?path, "Couldn't read path for maps"); - return None; - }, - }; - - let reader = BufReader::new(file); - let map: WorldFileLegacy = match bincode::deserialize_from(reader) { - Ok(map) => map, - Err(e) => { - warn!( - ?e, - "Couldn't parse legacy map. Maybe you meant to try a regular \ - load?" - ); - return None; - }, - }; - - map.into_modern() - }, - FileOpts::Load(ref path) => { - let file = match File::open(path) { - Ok(file) => file, - Err(e) => { - warn!(?e, ?path, "Couldn't read path for maps"); - return None; - }, - }; - - let reader = BufReader::new(file); - let map: WorldFile = match bincode::deserialize_from(reader) { - Ok(map) => map, - Err(e) => { - warn!( - ?e, - "Couldn't parse modern map. Maybe you meant to try a legacy load?" - ); - return None; - }, - }; - - map.into_modern() - }, - FileOpts::LoadAsset(ref specifier) => match WorldFile::load_owned(specifier) { - Ok(map) => map.into_modern(), - Err(err) => { - match err.reason().downcast_ref::() { - Some(e) => { - warn!(?e, ?specifier, "Couldn't read asset specifier for maps"); - }, - None => { - warn!( - ?err, - "Couldn't parse modern map. Maybe you meant to try a legacy \ - load?" - ); - }, - } - return None; - }, - }, - FileOpts::Generate { .. } | FileOpts::Save { .. } => return None, - }; - - match map { - Ok(map) => Some(map), - Err(e) => { - match e { - WorldFileError::WorldSizeInvalid => { - warn!("World size of map is invalid."); - }, - } - None - }, - } - })(); - - // NOTE: Change 1.0 to 4.0 for a 4x - // improvement in world detail. We also use this to automatically adjust - // grid_scale (multiplying by 4.0) and multiply mins_per_sec by - // 1.0 / (4.0 * 4.0) in ./erosion.rs, in order to get a similar rate of river - // formation. - // - // FIXME: This is a hack! At some point we will hae a more principled way of - // dealing with this. - let continent_scale_hack = 2.0/*4.0*/; - let (parsed_world_file, map_size_lg) = parsed_world_file - .and_then(|map| match MapSizeLg::new(map.map_size_lg) { - Ok(map_size_lg) => Some((Some(map), map_size_lg)), - Err(e) => { - warn!("World size of map does not satisfy invariants: {:?}", e); - None - }, - }) - .unwrap_or_else(|| { - let size_lg = match world_file { - FileOpts::Generate(SizeOpts { x_lg, y_lg, .. }) - | FileOpts::Save(SizeOpts { x_lg, y_lg, .. }) => { - MapSizeLg::new(Vec2 { x: x_lg, y: y_lg }).unwrap_or_else(|e| { - warn!("World size does not satisfy invariants: {:?}", e); - DEFAULT_WORLD_CHUNKS_LG - }) - }, - _ => DEFAULT_WORLD_CHUNKS_LG, - }; - (None, size_lg) - }); - let continent_scale_hack = if let Some(map) = &parsed_world_file { - map.continent_scale_hack - } else if let FileOpts::Generate(SizeOpts { scale, .. }) - | FileOpts::Save(SizeOpts { scale, .. }) = world_file - { - scale - } else { - continent_scale_hack - }; + let (parsed_world_file, map_size_lg, continent_scale_hack) = world_file.content(); let mut rng = ChaChaRng::from_seed(seed_expan::rng_state(seed)); let continent_scale = continent_scale_hack @@ -1147,39 +1197,7 @@ impl WorldSim { alt, basement, }); - (|| { - if let FileOpts::Save { .. } = world_file { - use std::time::SystemTime; - // Check if folder exists and create it if it does not - let mut path = PathBuf::from("./maps"); - if !path.exists() { - if let Err(e) = std::fs::create_dir(&path) { - warn!(?e, ?path, "Couldn't create folder for map"); - return; - } - } - path.push(format!( - // TODO: Work out a nice bincode file extension. - "map_{}.bin", - SystemTime::now() - .duration_since(SystemTime::UNIX_EPOCH) - .map(|d| d.as_millis()) - .unwrap_or(0) - )); - let file = match File::create(path.clone()) { - Ok(file) => file, - Err(e) => { - warn!(?e, ?path, "Couldn't create file for maps"); - return; - }, - }; - - let writer = BufWriter::new(file); - if let Err(e) = bincode::serialize_into(writer, &map) { - warn!(?e, "Couldn't write map"); - } - } - })(); + world_file.save(&map); // Skip validation--we just performed a no-op conversion for this map, so it had // better be valid! From 1a21dfbc5b438a663c99ee38abf42a4dbf825a30 Mon Sep 17 00:00:00 2001 From: juliancoffee Date: Wed, 13 Jul 2022 16:45:56 +0300 Subject: [PATCH 02/11] Add FileOpts::CacheLoad Add map setting that allows both generating (if not exists) and loading map --- world/src/sim/mod.rs | 136 +++++++++++++++++++++++++++++++------------ 1 file changed, 98 insertions(+), 38 deletions(-) diff --git a/world/src/sim/mod.rs b/world/src/sim/mod.rs index be5befc8aa..68ed0c6bb7 100644 --- a/world/src/sim/mod.rs +++ b/world/src/sim/mod.rs @@ -164,6 +164,10 @@ pub enum FileOpts { /// If set, generate the world map and save the world file (path is created /// the same way screenshot paths are). Save(SizeOpts), + /// Combination of Save and Load. + /// Load map if exists or generate the world map and save the + /// world file as + CacheLoad(String, SizeOpts), /// If set, load the world file from this path in legacy format (errors if /// path not found). This option may be removed at some point, since it /// only applies to maps generated before map saving was merged into @@ -218,21 +222,23 @@ impl FileOpts { fn map_size(&self) -> MapSizeLg { match self { - Self::Generate(opts) | Self::Save(opts) => MapSizeLg::new(Vec2 { - x: opts.x_lg, - y: opts.y_lg, - }) - .unwrap_or_else(|e| { - warn!("World size does not satisfy invariants: {:?}", e); - DEFAULT_WORLD_CHUNKS_LG - }), + Self::Generate(opts) | Self::Save(opts) | Self::CacheLoad(_, opts) => { + MapSizeLg::new(Vec2 { + x: opts.x_lg, + y: opts.y_lg, + }) + .unwrap_or_else(|e| { + warn!("World size does not satisfy invariants: {:?}", e); + DEFAULT_WORLD_CHUNKS_LG + }) + }, _ => DEFAULT_WORLD_CHUNKS_LG, } } fn continent_scale_hack(&self, default: f64) -> f64 { match self { - Self::Generate(opts) | Self::Save(opts) => opts.scale, + Self::Generate(opts) | Self::Save(opts) | Self::CacheLoad(_, opts) => opts.scale, _ => default, } } @@ -302,6 +308,46 @@ impl FileOpts { return None; }, }, + Self::CacheLoad { .. } => { + // FIXME: + // We check if map exists by comparing name and gen opts. + // But we also have another generation paramater that currently + // passed outside and used for both worldsim and worldgen. + // + // Ideally, we need to figure out how we want to use seed, i. e. + // moving worldgen seed to gen opts and use different sim seed from + // server config or grab sim seed from world file. + // + // `unwrap` is safe here, because CacheLoad has its path always defined + let path = self.map_path().unwrap(); + let path = std::path::Path::new("./maps/").join(path); + + let file = match File::open(&path) { + Ok(file) => file, + Err(e) => { + warn!( + ?e, + ?path, + "Couldn't find needed map. It will be regenerated." + ); + return None; + }, + }; + + let reader = BufReader::new(file); + let map: WorldFile = match bincode::deserialize_from(reader) { + Ok(map) => map, + Err(e) => { + warn!( + ?e, + "Couldn't parse modern map. Maybe you meant to try a legacy load?" + ); + return None; + }, + }; + + map.into_modern() + }, Self::Generate { .. } | Self::Save { .. } => return None, }; @@ -318,38 +364,52 @@ impl FileOpts { } } - fn save(&self, map: &WorldFile) { - if let FileOpts::Save { .. } = self { - use std::time::SystemTime; - // Check if folder exists and create it if it does not - let mut path = PathBuf::from("./maps"); - if !path.exists() { - if let Err(e) = std::fs::create_dir(&path) { - warn!(?e, ?path, "Couldn't create folder for map"); - return; - } - } - path.push(format!( - // TODO: Work out a nice bincode file extension. - "map_{}.bin", - SystemTime::now() - .duration_since(SystemTime::UNIX_EPOCH) - .map(|d| d.as_millis()) - .unwrap_or(0) - )); - let file = match File::create(path.clone()) { - Ok(file) => file, - Err(e) => { - warn!(?e, ?path, "Couldn't create file for maps"); - return; - }, - }; + fn map_path(&self) -> Option { + // TODO: Work out a nice bincode file extension. + match self { + Self::Save { .. } => { + use std::time::SystemTime; + Some(format!( + "map_{}.bin", + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .map(|d| d.as_millis()) + .unwrap_or(0) + )) + }, + Self::CacheLoad(name, opts) => Some(format!("map_{}{:?}.bin", name, opts)), + _ => None, + } + } - let writer = BufWriter::new(file); - if let Err(e) = bincode::serialize_into(writer, map) { - warn!(?e, "Couldn't write map"); + fn save(&self, map: &WorldFile) { + let name = if let Some(name) = self.map_path() { + name + } else { + return; + }; + + // Check if folder exists and create it if it does not + let mut path = PathBuf::from("./maps"); + if !path.exists() { + if let Err(e) = std::fs::create_dir(&path) { + warn!(?e, ?path, "Couldn't create folder for map"); + return; } } + path.push(name); + let file = match File::create(path.clone()) { + Ok(file) => file, + Err(e) => { + warn!(?e, ?path, "Couldn't create file for maps"); + return; + }, + }; + + let writer = BufWriter::new(file); + if let Err(e) = bincode::serialize_into(writer, map) { + warn!(?e, "Couldn't write map"); + } } } From ddf2c43c18ebd224ce6fa4c853f046293a444f19 Mon Sep 17 00:00:00 2001 From: juliancoffee Date: Wed, 13 Jul 2022 17:55:50 +0300 Subject: [PATCH 03/11] Make hacky hash for gen opts --- world/src/sim/mod.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/world/src/sim/mod.rs b/world/src/sim/mod.rs index 68ed0c6bb7..7d0ea59377 100644 --- a/world/src/sim/mod.rs +++ b/world/src/sim/mod.rs @@ -369,6 +369,7 @@ impl FileOpts { match self { Self::Save { .. } => { use std::time::SystemTime; + Some(format!( "map_{}.bin", SystemTime::now() @@ -377,7 +378,20 @@ impl FileOpts { .unwrap_or(0) )) }, - Self::CacheLoad(name, opts) => Some(format!("map_{}{:?}.bin", name, opts)), + Self::CacheLoad(name, opts) => { + use std::{ + collections::hash_map::DefaultHasher, + hash::{Hash, Hasher}, + }; + + // We convert opts to string, as opts isn't hasheable because + // of scale which is f64 + let opts = format!("{opts:?}"); + let mut hashed_opts = DefaultHasher::new(); + opts.hash(&mut hashed_opts); + + Some(format!("map_{}{:x}.bin", name, hashed_opts.finish())) + }, _ => None, } } From 6faba8a1e90cd198107186ad901ff2de17177e1f Mon Sep 17 00:00:00 2001 From: juliancoffee Date: Fri, 15 Jul 2022 18:28:35 +0300 Subject: [PATCH 04/11] Use hashable wrapper around gen opts --- world/src/sim/mod.rs | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/world/src/sim/mod.rs b/world/src/sim/mod.rs index 7d0ea59377..13ef7b36c7 100644 --- a/world/src/sim/mod.rs +++ b/world/src/sim/mod.rs @@ -384,9 +384,25 @@ impl FileOpts { hash::{Hash, Hasher}, }; - // We convert opts to string, as opts isn't hasheable because - // of scale which is f64 - let opts = format!("{opts:?}"); + // Hashable wrapper around gen opts + #[derive(Hash)] + struct GenOptsWrapper { + x_lg: u32, + y_lg: u32, + scale: ordered_float::OrderedFloat, + } + + // NOTE: we use pattern-matching to get opts fields + // so that when main opts strut gains new fields, + // compiler will bonk you into updating wrapper struct + let SizeOpts { x_lg, y_lg, scale } = opts; + + let opts = GenOptsWrapper { + x_lg: *x_lg, + y_lg: *y_lg, + scale: ordered_float::OrderedFloat(*scale), + }; + let mut hashed_opts = DefaultHasher::new(); opts.hash(&mut hashed_opts); From 6d9c7a5645e07d6841a8733a7afbe7ed93c2bf36 Mon Sep 17 00:00:00 2001 From: juliancoffee Date: Fri, 15 Jul 2022 20:21:48 +0300 Subject: [PATCH 05/11] Adressing review --- world/src/sim/mod.rs | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/world/src/sim/mod.rs b/world/src/sim/mod.rs index 13ef7b36c7..a6d68c16a6 100644 --- a/world/src/sim/mod.rs +++ b/world/src/sim/mod.rs @@ -167,7 +167,7 @@ pub enum FileOpts { /// Combination of Save and Load. /// Load map if exists or generate the world map and save the /// world file as - CacheLoad(String, SizeOpts), + LoadOrGenerate(String, SizeOpts), /// If set, load the world file from this path in legacy format (errors if /// path not found). This option may be removed at some point, since it /// only applies to maps generated before map saving was merged into @@ -188,14 +188,15 @@ impl Default for FileOpts { } impl FileOpts { - fn content(&self) -> (Option, MapSizeLg, f64) { - let parsed_world_file = self.try_to_map(); + fn load_content(&self) -> (Option, MapSizeLg, f64) { + let parsed_world_file = self.try_load_map(); let map_size_lg = parsed_world_file .as_ref() .and_then(|map| match MapSizeLg::new(map.map_size_lg) { Ok(map_size_lg) => Some(map_size_lg), Err(e) => { + // TODO: this probably just panic warn!("World size of map does not satisfy invariants: {:?}", e); None }, @@ -214,15 +215,17 @@ impl FileOpts { let continent_scale_hack = if let Some(map) = &parsed_world_file { map.continent_scale_hack } else { - self.continent_scale_hack(default_continent_scale_hack) + self.continent_scale_hack() + .unwrap_or(default_continent_scale_hack) }; (parsed_world_file, map_size_lg, continent_scale_hack) } + // TODO: this should return Option so that caller can choose fallback fn map_size(&self) -> MapSizeLg { match self { - Self::Generate(opts) | Self::Save(opts) | Self::CacheLoad(_, opts) => { + Self::Generate(opts) | Self::Save(opts) | Self::LoadOrGenerate(_, opts) => { MapSizeLg::new(Vec2 { x: opts.x_lg, y: opts.y_lg, @@ -236,14 +239,18 @@ impl FileOpts { } } - fn continent_scale_hack(&self, default: f64) -> f64 { + fn continent_scale_hack(&self) -> Option { match self { - Self::Generate(opts) | Self::Save(opts) | Self::CacheLoad(_, opts) => opts.scale, - _ => default, + Self::Generate(opts) | Self::Save(opts) | Self::LoadOrGenerate(_, opts) => { + Some(opts.scale) + }, + _ => None, } } - fn try_to_map(&self) -> Option { + // TODO: This should probably return a Result, so that caller can choose + // whether to log error + fn try_load_map(&self) -> Option { let map = match self { Self::LoadLegacy(ref path) => { let file = match File::open(path) { @@ -308,7 +315,7 @@ impl FileOpts { return None; }, }, - Self::CacheLoad { .. } => { + Self::LoadOrGenerate { .. } => { // FIXME: // We check if map exists by comparing name and gen opts. // But we also have another generation paramater that currently @@ -318,7 +325,7 @@ impl FileOpts { // moving worldgen seed to gen opts and use different sim seed from // server config or grab sim seed from world file. // - // `unwrap` is safe here, because CacheLoad has its path always defined + // `unwrap` is safe here, because LoadOrGenerate has its path always defined let path = self.map_path().unwrap(); let path = std::path::Path::new("./maps/").join(path); @@ -378,7 +385,7 @@ impl FileOpts { .unwrap_or(0) )) }, - Self::CacheLoad(name, opts) => { + Self::LoadOrGenerate(name, opts) => { use std::{ collections::hash_map::DefaultHasher, hash::{Hash, Hasher}, @@ -661,7 +668,7 @@ impl WorldSim { let world_file = opts.world_file; // Parse out the contents of various map formats into the values we need. - let (parsed_world_file, map_size_lg, continent_scale_hack) = world_file.content(); + let (parsed_world_file, map_size_lg, continent_scale_hack) = world_file.load_content(); let mut rng = ChaChaRng::from_seed(seed_expan::rng_state(seed)); let continent_scale = continent_scale_hack From a437d31205667a22b6cbdb1ebbe1973d6f086888 Mon Sep 17 00:00:00 2001 From: juliancoffee Date: Sat, 16 Jul 2022 09:49:06 +0300 Subject: [PATCH 06/11] Checking options from loaded map instead of hash --- world/src/sim/mod.rs | 73 +++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 42 deletions(-) diff --git a/world/src/sim/mod.rs b/world/src/sim/mod.rs index a6d68c16a6..1c2f7ac1a1 100644 --- a/world/src/sim/mod.rs +++ b/world/src/sim/mod.rs @@ -196,7 +196,7 @@ impl FileOpts { .and_then(|map| match MapSizeLg::new(map.map_size_lg) { Ok(map_size_lg) => Some(map_size_lg), Err(e) => { - // TODO: this probably just panic + // TODO: this probably should just panic warn!("World size of map does not satisfy invariants: {:?}", e); None }, @@ -315,17 +315,9 @@ impl FileOpts { return None; }, }, - Self::LoadOrGenerate { .. } => { - // FIXME: - // We check if map exists by comparing name and gen opts. - // But we also have another generation paramater that currently - // passed outside and used for both worldsim and worldgen. - // - // Ideally, we need to figure out how we want to use seed, i. e. - // moving worldgen seed to gen opts and use different sim seed from - // server config or grab sim seed from world file. - // - // `unwrap` is safe here, because LoadOrGenerate has its path always defined + Self::LoadOrGenerate(_name, opts) => { + // `unwrap` is safe here, because LoadOrGenerate has its path + // always defined let path = self.map_path().unwrap(); let path = std::path::Path::new("./maps/").join(path); @@ -353,6 +345,32 @@ impl FileOpts { }, }; + // FIXME: + // We check if we need to generate new map by comparing gen opts. + // But we also have another generation paramater that currently + // passed outside and used for both worldsim and worldgen. + // + // Ideally, we need to figure out how we want to use seed, i. e. + // moving worldgen seed to gen opts and use different sim seed from + // server config or grab sim seed from world file. + // + // NOTE: we intentionally use pattern-matching here to get + // options, so that when gen opts get another field, compiler + // will force you to update following logic + let SizeOpts { x_lg, y_lg, scale } = opts; + let map = match map { + WorldFile::Veloren0_7_0(map) => map, + WorldFile::Veloren0_5_0(_) => { + panic!("World file v0.5.0 isn't supported with LoadOrGenerate") + }, + }; + + if map.continent_scale_hack != *scale || map.map_size_lg != Vec2::new(*x_lg, *y_lg) + { + warn!("Specified options don't correspond these in loaded map file"); + return None; + } + map.into_modern() }, Self::Generate { .. } | Self::Save { .. } => return None, @@ -385,36 +403,7 @@ impl FileOpts { .unwrap_or(0) )) }, - Self::LoadOrGenerate(name, opts) => { - use std::{ - collections::hash_map::DefaultHasher, - hash::{Hash, Hasher}, - }; - - // Hashable wrapper around gen opts - #[derive(Hash)] - struct GenOptsWrapper { - x_lg: u32, - y_lg: u32, - scale: ordered_float::OrderedFloat, - } - - // NOTE: we use pattern-matching to get opts fields - // so that when main opts strut gains new fields, - // compiler will bonk you into updating wrapper struct - let SizeOpts { x_lg, y_lg, scale } = opts; - - let opts = GenOptsWrapper { - x_lg: *x_lg, - y_lg: *y_lg, - scale: ordered_float::OrderedFloat(*scale), - }; - - let mut hashed_opts = DefaultHasher::new(); - opts.hash(&mut hashed_opts); - - Some(format!("map_{}{:x}.bin", name, hashed_opts.finish())) - }, + Self::LoadOrGenerate(name, _opts) => Some(format!("map_{}.bin", name)), _ => None, } } From 60a9cce57f58c13401bceb2a1c69e9ba0f8832f0 Mon Sep 17 00:00:00 2001 From: juliancoffee Date: Sat, 16 Jul 2022 10:18:48 +0300 Subject: [PATCH 07/11] Change map_path to inslude map directory --- world/src/sim/mod.rs | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/world/src/sim/mod.rs b/world/src/sim/mod.rs index 1c2f7ac1a1..fbad1e853f 100644 --- a/world/src/sim/mod.rs +++ b/world/src/sim/mod.rs @@ -319,7 +319,6 @@ impl FileOpts { // `unwrap` is safe here, because LoadOrGenerate has its path // always defined let path = self.map_path().unwrap(); - let path = std::path::Path::new("./maps/").join(path); let file = match File::open(&path) { Ok(file) => file, @@ -361,13 +360,14 @@ impl FileOpts { let map = match map { WorldFile::Veloren0_7_0(map) => map, WorldFile::Veloren0_5_0(_) => { - panic!("World file v0.5.0 isn't supported with LoadOrGenerate") + panic!("World file v0.5.0 isn't supported with LoadOrGenerate.") }, }; if map.continent_scale_hack != *scale || map.map_size_lg != Vec2::new(*x_lg, *y_lg) { - warn!("Specified options don't correspond these in loaded map file"); + warn!("Specified options don't correspond these in loaded map file."); + warn!("Map will be regenerated and overwritten."); return None; } @@ -389,9 +389,10 @@ impl FileOpts { } } - fn map_path(&self) -> Option { + fn map_path(&self) -> Option { + const MAP_DIR: &str = "./maps"; // TODO: Work out a nice bincode file extension. - match self { + let file_name = match self { Self::Save { .. } => { use std::time::SystemTime; @@ -405,25 +406,27 @@ impl FileOpts { }, Self::LoadOrGenerate(name, _opts) => Some(format!("map_{}.bin", name)), _ => None, - } + }; + + file_name.map(|name| std::path::Path::new(MAP_DIR).join(name)) } fn save(&self, map: &WorldFile) { - let name = if let Some(name) = self.map_path() { - name + let path = if let Some(path) = self.map_path() { + path } else { return; }; // Check if folder exists and create it if it does not - let mut path = PathBuf::from("./maps"); - if !path.exists() { - if let Err(e) = std::fs::create_dir(&path) { - warn!(?e, ?path, "Couldn't create folder for map"); + let map_dir = path.parent().expect("failed to get map directory"); + if !map_dir.exists() { + if let Err(e) = std::fs::create_dir_all(map_dir) { + warn!(?e, ?map_dir, "Couldn't create folder for map"); return; } } - path.push(name); + let file = match File::create(path.clone()) { Ok(file) => file, Err(e) => { From d2962d544b27469d3fff23ee6c293aa35678e269 Mon Sep 17 00:00:00 2001 From: juliancoffee Date: Sat, 16 Jul 2022 11:28:44 +0300 Subject: [PATCH 08/11] Add overwrite option to LoadOrGenerate --- world/src/sim/mod.rs | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/world/src/sim/mod.rs b/world/src/sim/mod.rs index fbad1e853f..54346a8487 100644 --- a/world/src/sim/mod.rs +++ b/world/src/sim/mod.rs @@ -166,8 +166,14 @@ pub enum FileOpts { Save(SizeOpts), /// Combination of Save and Load. /// Load map if exists or generate the world map and save the - /// world file as - LoadOrGenerate(String, SizeOpts), + /// world file. + LoadOrGenerate { + name: String, + #[serde(default)] + opts: SizeOpts, + #[serde(default)] + overwrite: bool, + }, /// If set, load the world file from this path in legacy format (errors if /// path not found). This option may be removed at some point, since it /// only applies to maps generated before map saving was merged into @@ -225,7 +231,7 @@ impl FileOpts { // TODO: this should return Option so that caller can choose fallback fn map_size(&self) -> MapSizeLg { match self { - Self::Generate(opts) | Self::Save(opts) | Self::LoadOrGenerate(_, opts) => { + Self::Generate(opts) | Self::Save(opts) | Self::LoadOrGenerate { opts, .. } => { MapSizeLg::new(Vec2 { x: opts.x_lg, y: opts.y_lg, @@ -241,7 +247,7 @@ impl FileOpts { fn continent_scale_hack(&self) -> Option { match self { - Self::Generate(opts) | Self::Save(opts) | Self::LoadOrGenerate(_, opts) => { + Self::Generate(opts) | Self::Save(opts) | Self::LoadOrGenerate { opts, .. } => { Some(opts.scale) }, _ => None, @@ -315,7 +321,9 @@ impl FileOpts { return None; }, }, - Self::LoadOrGenerate(_name, opts) => { + Self::LoadOrGenerate { + opts, overwrite, .. + } => { // `unwrap` is safe here, because LoadOrGenerate has its path // always defined let path = self.map_path().unwrap(); @@ -323,11 +331,7 @@ impl FileOpts { let file = match File::open(&path) { Ok(file) => file, Err(e) => { - warn!( - ?e, - ?path, - "Couldn't find needed map. It will be regenerated." - ); + warn!(?e, ?path, "Couldn't find needed map. Generating..."); return None; }, }; @@ -366,8 +370,20 @@ impl FileOpts { if map.continent_scale_hack != *scale || map.map_size_lg != Vec2::new(*x_lg, *y_lg) { - warn!("Specified options don't correspond these in loaded map file."); - warn!("Map will be regenerated and overwritten."); + if *overwrite { + warn!( + "{}\n{}", + "Specified options don't correspond to these in loaded map.", + "Map will be regenerated and overwritten." + ); + } else { + panic!( + "{}\n{}", + "Specified options don't correspond to these in loaded map.", + "Use 'ovewrite' option, if you wish to regenerate map." + ); + } + return None; } @@ -404,7 +420,7 @@ impl FileOpts { .unwrap_or(0) )) }, - Self::LoadOrGenerate(name, _opts) => Some(format!("map_{}.bin", name)), + Self::LoadOrGenerate { name, .. } => Some(format!("map_{}.bin", name)), _ => None, }; From 5079c0007fdcb1e63db6a004fc3c104287e5cd6a Mon Sep 17 00:00:00 2001 From: juliancoffee Date: Sat, 16 Jul 2022 11:35:33 +0300 Subject: [PATCH 09/11] Remove prefix map_ for LoadOrGenerate maps --- world/src/sim/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/world/src/sim/mod.rs b/world/src/sim/mod.rs index 54346a8487..fc417e6646 100644 --- a/world/src/sim/mod.rs +++ b/world/src/sim/mod.rs @@ -420,7 +420,7 @@ impl FileOpts { .unwrap_or(0) )) }, - Self::LoadOrGenerate { name, .. } => Some(format!("map_{}.bin", name)), + Self::LoadOrGenerate { name, .. } => Some(format!("{}.bin", name)), _ => None, }; From 9d67d80a63f9b4cefae5a6880f19a70947fc044a Mon Sep 17 00:00:00 2001 From: juliancoffee Date: Sat, 16 Jul 2022 12:02:42 +0300 Subject: [PATCH 10/11] Only ovewrite LoadOrGenerate map if it's fresh map --- world/src/sim/mod.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/world/src/sim/mod.rs b/world/src/sim/mod.rs index fc417e6646..01d51f895a 100644 --- a/world/src/sim/mod.rs +++ b/world/src/sim/mod.rs @@ -677,6 +677,9 @@ impl WorldSim { // Parse out the contents of various map formats into the values we need. let (parsed_world_file, map_size_lg, continent_scale_hack) = world_file.load_content(); + // Currently only used with LoadOrGenerate to know if we need to + // overwrite world file + let fresh = parsed_world_file.is_none(); let mut rng = ChaChaRng::from_seed(seed_expan::rng_state(seed)); let continent_scale = continent_scale_hack @@ -1302,7 +1305,9 @@ impl WorldSim { alt, basement, }); - world_file.save(&map); + if fresh { + world_file.save(&map); + } // Skip validation--we just performed a no-op conversion for this map, so it had // better be valid! From fac67a40a1814ad15d0ddc278a470f47333378a2 Mon Sep 17 00:00:00 2001 From: juliancoffee Date: Sun, 17 Jul 2022 14:49:42 +0300 Subject: [PATCH 11/11] Make worldgen panic if loaded map has invalid size --- world/src/sim/mod.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/world/src/sim/mod.rs b/world/src/sim/mod.rs index 01d51f895a..07503e9c80 100644 --- a/world/src/sim/mod.rs +++ b/world/src/sim/mod.rs @@ -197,17 +197,12 @@ impl FileOpts { fn load_content(&self) -> (Option, MapSizeLg, f64) { let parsed_world_file = self.try_load_map(); - let map_size_lg = parsed_world_file - .as_ref() - .and_then(|map| match MapSizeLg::new(map.map_size_lg) { - Ok(map_size_lg) => Some(map_size_lg), - Err(e) => { - // TODO: this probably should just panic - warn!("World size of map does not satisfy invariants: {:?}", e); - None - }, - }) - .unwrap_or_else(|| self.map_size()); + let map_size_lg = if let Some(map) = &parsed_world_file { + MapSizeLg::new(map.map_size_lg) + .expect("World size of loaded map does not satisfy invariants.") + } else { + self.map_size() + }; // NOTE: Change 1.0 to 4.0 for a 4x // improvement in world detail. We also use this to automatically adjust