From 66189d279cd27c7df9f4fee8a5fa8e30fffd453e Mon Sep 17 00:00:00 2001 From: Matthias Schiffer Date: Fri, 14 Jun 2024 16:02:56 +0200 Subject: [PATCH 1/2] core: region_processor: fix crash due to incorrect counting in info message Fixes #52 --- CHANGELOG.md | 7 +++++ Cargo.lock | 21 +++++++++++++++ Cargo.toml | 1 + src/core/region_processor.rs | 52 +++++++++++++----------------------- 4 files changed, 47 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3b134e..efcf8d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ ## [Unreleased] - ReleaseDate +### Fixed + +- Fix crash due to incorrect counting in info message + + The calculation of the number of skipped regions could underflow when more invalid than valid + regions were encountered. + ## [2.1.0] - 2024-01-27 ### Added diff --git a/Cargo.lock b/Cargo.lock index b87c678..1780b1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -276,6 +276,26 @@ version = "1.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "11157ac094ffbdde99aa67b23417ebdd801842852b500e395a45a9c0aac03e4a" +[[package]] +name = "enum-map" +version = "2.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6866f3bfdf8207509a033af1a75a7b08abda06bbaaeae6669323fd5a097df2e9" +dependencies = [ + "enum-map-derive", +] + +[[package]] +name = "enum-map-derive" +version = "0.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f282cfdfe92516eb26c2af8589c274c7c17681f5ecc03c18255fe741c6aa64eb" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "enumflags2" version = "0.7.9" @@ -550,6 +570,7 @@ dependencies = [ "anyhow", "bincode", "clap", + "enum-map", "fastnbt", "futures-util", "git-version", diff --git a/Cargo.toml b/Cargo.toml index 082c5b6..f8bb35f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,6 +40,7 @@ pre-release-replacements = [ anyhow = "1.0.68" bincode = "1.3.3" clap = { version = "4.1.4", features = ["derive", "wrap_help"] } +enum-map = "2.7.3" fastnbt = "2.3.2" futures-util = "0.3.28" git-version = "0.3.5" diff --git a/src/core/region_processor.rs b/src/core/region_processor.rs index 63d5c09..9778061 100644 --- a/src/core/region_processor.rs +++ b/src/core/region_processor.rs @@ -1,16 +1,9 @@ //! The [RegionProcessor] and related functions -use std::{ - ffi::OsStr, - path::PathBuf, - sync::{ - atomic::{AtomicBool, Ordering}, - mpsc, - }, - time::SystemTime, -}; +use std::{ffi::OsStr, path::PathBuf, sync::mpsc, time::SystemTime}; use anyhow::{Context, Result}; +use enum_map::{Enum, EnumMap}; use rayon::prelude::*; use tracing::{debug, info, warn}; @@ -36,7 +29,7 @@ fn parse_region_filename(file_name: &OsStr) -> Option { } /// [RegionProcessor::process_region] return values -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Enum)] enum RegionProcessorStatus { /// Region was processed Ok, @@ -363,6 +356,8 @@ impl<'a> RegionProcessor<'a> { /// /// Returns a list of the coordinates of all processed regions pub fn run(self) -> Result> { + use RegionProcessorStatus as Status; + fs::create_dir_all(&self.config.processed_dir)?; fs::create_dir_all(&self.config.tile_dir(TileKind::Lightmap, 0))?; fs::create_dir_all(&self.config.entities_dir(0))?; @@ -370,31 +365,18 @@ impl<'a> RegionProcessor<'a> { info!("Processing region files..."); let (region_send, region_recv) = mpsc::channel(); - let (processed_send, processed_recv) = mpsc::channel(); - let (error_send, error_recv) = mpsc::channel(); - - let has_unknown = AtomicBool::new(false); + let (status_send, status_recv) = mpsc::channel(); self.collect_regions()?.par_iter().try_for_each(|&coords| { let ret = self .process_region(coords) .with_context(|| format!("Failed to process region {:?}", coords))?; - if ret != RegionProcessorStatus::ErrorMissing { + if ret != Status::ErrorMissing { region_send.send(coords).unwrap(); } - match ret { - RegionProcessorStatus::Ok => processed_send.send(()).unwrap(), - RegionProcessorStatus::OkWithUnknown => { - has_unknown.store(true, Ordering::Relaxed); - processed_send.send(()).unwrap(); - } - RegionProcessorStatus::Skipped => {} - RegionProcessorStatus::ErrorOk | RegionProcessorStatus::ErrorMissing => { - error_send.send(()).unwrap() - } - } + status_send.send(ret).unwrap(); anyhow::Ok(()) })?; @@ -402,19 +384,21 @@ impl<'a> RegionProcessor<'a> { drop(region_send); let mut regions: Vec<_> = region_recv.into_iter().collect(); - drop(processed_send); - let processed = processed_recv.into_iter().count(); - drop(error_send); - let errors = error_recv.into_iter().count(); + drop(status_send); + + let mut status = EnumMap::<_, usize>::default(); + for ret in status_recv { + status[ret] += 1; + } info!( "Processed region files ({} processed, {} unchanged, {} errors)", - processed, - regions.len() - processed - errors, - errors, + status[Status::Ok] + status[Status::OkWithUnknown], + status[Status::Skipped], + status[Status::ErrorOk] + status[Status::ErrorMissing], ); - if has_unknown.into_inner() { + if status[Status::OkWithUnknown] > 0 { warn!("Unknown block or biome types found during processing"); eprint!(concat!( "\n", From e74e7be68667f7562e57b1ed14bce25a796be336 Mon Sep 17 00:00:00 2001 From: Matthias Schiffer Date: Fri, 14 Jun 2024 16:15:56 +0200 Subject: [PATCH 2/2] core: region_processor: ignore empty region files Minecraft generates empty region files in some cases. Just ignore these files instead of printing an error message for each. --- CHANGELOG.md | 4 ++++ src/core/region_processor.rs | 19 ++++++++++++++----- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index efcf8d5..c010654 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ The calculation of the number of skipped regions could underflow when more invalid than valid regions were encountered. +- Ignore empty region files instead of treating them as invalid + + Minecraft generates empty region files in some cases. Just ignore them instead of printing an + error message every time. ## [2.1.0] - 2024-01-27 diff --git a/src/core/region_processor.rs b/src/core/region_processor.rs index 9778061..ce2d060 100644 --- a/src/core/region_processor.rs +++ b/src/core/region_processor.rs @@ -337,11 +337,20 @@ impl<'a> RegionProcessor<'a> { })? .filter_map(|entry| entry.ok()) .filter(|entry| { - // We are only interested in regular files - matches!( - entry.file_type().map(|file_type| file_type.is_file()), - Ok(true) - ) + (|| { + // We are only interested in regular files + let file_type = entry.file_type().ok()?; + if !file_type.is_file() { + return None; + } + + let metadata = entry.metadata().ok()?; + if metadata.len() == 0 { + return None; + } + Some(()) + })() + .is_some() }) .filter_map(|entry| parse_region_filename(&entry.file_name())) .collect())