From b49085f9b91c8647db02f658dfc8530f757bda45 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Mon, 27 Apr 2026 19:50:58 +0100 Subject: [PATCH] Stop treating asset hash collisions as errors Feedback from Robert on Discord[1] is that hash collisions can be very common for certain mods. Since erroring causes that archive's assets to be ignored, it results in less accurate sorting behaviour than not erroring, and if there are many hash collisions then logging them just produces noise. Instead, count the number of collisions and debug log a single statement if it's non-zero, so there's still some indication to aid debugging any unexpected sorting behaviour it causes. [1]: https://discord.com/channels/473542112974077963/473542230095822848/1498060566344896673 The linked Discord message (and a few following messages) is: > pretty sure for Skyblivion we have > Many duplicates all over our files and i mean A LOT > but 100s of thousands of files that i'd expect > [...] > quirks of the hashing formula the game uses > > -- Robert in #support at 2026-04-27, 21:36 PM --- src/archive/ba2.rs | 18 ++++++++++++++---- src/archive/bsa.rs | 34 +++++++++++++++++++++++++++------- src/archive/error.rs | 12 ------------ src/archive/parse.rs | 4 ++-- 4 files changed, 43 insertions(+), 25 deletions(-) diff --git a/src/archive/ba2.rs b/src/archive/ba2.rs index 11956555..044fe35f 100644 --- a/src/archive/ba2.rs +++ b/src/archive/ba2.rs @@ -2,8 +2,11 @@ use std::{ collections::{BTreeMap, BTreeSet}, hash::{DefaultHasher, Hash, Hasher}, io::{BufRead, Seek}, + path::Path, }; +use crate::{escape_ascii, logging}; + use super::error::ArchiveParsingError; pub(super) const TYPE_ID: [u8; 4] = *b"BTDX"; @@ -59,6 +62,7 @@ impl TryFrom<[u8; HEADER_SIZE - TYPE_ID.len()]> for Header { pub(super) fn read_assets( mut reader: T, + archive_path: &Path, ) -> Result>, ArchiveParsingError> { let mut header_buffer = [0; HEADER_SIZE - TYPE_ID.len()]; @@ -70,6 +74,7 @@ pub(super) fn read_assets( reader.seek(std::io::SeekFrom::Start(header.file_paths_offset))?; + let mut collision_count: usize = 0; for _ in 0..header.file_count { let mut length_buf = [0; 2]; reader.read_exact(&mut length_buf)?; @@ -90,13 +95,18 @@ pub(super) fn read_assets( let file_hashes: &mut BTreeSet = assets.entry(folder_hash).or_default(); if !file_hashes.insert(file_hash) { - return Err(ArchiveParsingError::HashCollision { - folder_hash, - file_hash, - }); + collision_count += 1; } } + if collision_count > 0 { + logging::debug!( + "Encountered {} hash collisions for asset file paths while reading \"{}\"", + collision_count, + escape_ascii(archive_path) + ); + } + Ok(assets) } diff --git a/src/archive/bsa.rs b/src/archive/bsa.rs index 4e1f724b..639a7abe 100644 --- a/src/archive/bsa.rs +++ b/src/archive/bsa.rs @@ -1,8 +1,11 @@ use std::{ collections::{BTreeMap, BTreeSet, btree_map::Entry}, io::BufRead, + path::Path, }; +use crate::{escape_ascii, logging}; + use super::error::ArchiveParsingError; pub(super) const TYPE_ID: [u8; 4] = *b"BSA\0"; @@ -134,6 +137,7 @@ mod v105 { pub(super) fn read_assets( mut reader: T, + archive_path: &Path, ) -> Result>, ArchiveParsingError> { let mut header_buffer = [0; HEADER_SIZE - TYPE_ID.len()]; @@ -144,11 +148,13 @@ pub(super) fn read_assets( match header.version { 103 | 104 => read_assets_with_header::( reader, + archive_path, &header, v103::read_folder_record, ), 105 => read_assets_with_header::( reader, + archive_path, &header, v105::read_folder_record, ), @@ -160,6 +166,7 @@ pub(super) fn read_assets( fn read_assets_with_header( mut reader: T, + archive_path: &Path, header: &Header, read_folder_record: impl Fn(&[u8; U]) -> FolderRecord, ) -> Result>, ArchiveParsingError> { @@ -178,15 +185,15 @@ fn read_assets_with_header( let folder_record_offset_baseline = HEADER_SIZE + folders_buffer.len() + to_usize(header.total_file_names_length); + let mut folder_collision_count: usize = 0; + let mut asset_collision_count: usize = 0; let mut assets = BTreeMap::new(); for chunk in folders_buffer.as_chunks::().0 { let folder_record = read_folder_record(chunk); let entry = assets.entry(folder_record.name_hash); if let Entry::Occupied(_) = entry { - return Err(ArchiveParsingError::FolderHashCollision( - folder_record.name_hash, - )); + folder_collision_count += 1; } let file_records_offset = if (header.archive_flags & 0x1) == 0 { @@ -221,14 +228,27 @@ fn read_assets_with_header( let file_hash = file_record_hash(file_chunk); if !file_hashes.insert(file_hash) { - return Err(ArchiveParsingError::HashCollision { - folder_hash: folder_record.name_hash, - file_hash, - }); + asset_collision_count += 1; } } } + if folder_collision_count > 0 { + logging::debug!( + "Encountered {} hash collisions for asset folders while reading \"{}\"", + folder_collision_count, + escape_ascii(archive_path) + ); + } + + if asset_collision_count > 0 { + logging::debug!( + "Encountered {} hash collisions for asset file paths while reading \"{}\"", + asset_collision_count, + escape_ascii(archive_path) + ); + } + Ok(assets) } diff --git a/src/archive/error.rs b/src/archive/error.rs index 69d9ac6f..d6c9725a 100644 --- a/src/archive/error.rs +++ b/src/archive/error.rs @@ -47,8 +47,6 @@ pub(crate) enum ArchiveParsingError { InvalidFolderNameLengthOffset(usize), InvalidFileRecordsOffset(usize), UsesBigEndianNumbers, - FolderHashCollision(u64), - HashCollision { folder_hash: u64, file_hash: u64 }, } impl std::fmt::Display for ArchiveParsingError { @@ -70,16 +68,6 @@ impl std::fmt::Display for ArchiveParsingError { Self::UsesBigEndianNumbers => { write!(f, "archive uses big-endian numbers, which is unsupported") } - Self::FolderHashCollision(h) => { - write!(f, "unexpected collision for folder name hash {h:x}") - } - Self::HashCollision { - folder_hash, - file_hash, - } => write!( - f, - "unexpected collision for file name hash {file_hash:x} in set for folder name hash {folder_hash:x}" - ), } } } diff --git a/src/archive/parse.rs b/src/archive/parse.rs index 059fba58..383f01f4 100644 --- a/src/archive/parse.rs +++ b/src/archive/parse.rs @@ -64,9 +64,9 @@ fn get_assets_in_archive( .map_err(|e| ArchivePathParsingError::from_io_error(archive_path.into(), e))?; match type_id { - bsa::TYPE_ID => bsa::read_assets(reader) + bsa::TYPE_ID => bsa::read_assets(reader, archive_path) .map_err(|e| ArchivePathParsingError::new(archive_path.into(), e)), - ba2::TYPE_ID => ba2::read_assets(reader) + ba2::TYPE_ID => ba2::read_assets(reader, archive_path) .map_err(|e| ArchivePathParsingError::new(archive_path.into(), e)), _ => Err(ArchivePathParsingError::new( archive_path.into(),