Merge pull request #53 from neocturne/empty-region-fixes

Empty region fixes

CHANGELOG.md

@@ -2,6 +2,17 @@
 
 ## [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.
+- 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
 
 ### Added

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",

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"

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<TileCoords> {
 }
 
 /// [RegionProcessor::process_region] return values
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Enum)]
 enum RegionProcessorStatus {
 	/// Region was processed
 	Ok,
@@ -344,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)
-				)
+					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())
@@ -363,6 +365,8 @@ impl<'a> RegionProcessor<'a> {
 	///
 	/// Returns a list of the coordinates of all processed regions
 	pub fn run(self) -> Result<Vec<TileCoords>> {
+		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 +374,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 +393,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",