mirror of
https://github.com/loot/libloot.git
synced 2026-07-27 14:16:01 -07:00
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
This commit is contained in:
+14
-4
@@ -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<T: BufRead + Seek>(
|
||||
mut reader: T,
|
||||
archive_path: &Path,
|
||||
) -> Result<BTreeMap<u64, BTreeSet<u64>>, ArchiveParsingError> {
|
||||
let mut header_buffer = [0; HEADER_SIZE - TYPE_ID.len()];
|
||||
|
||||
@@ -70,6 +74,7 @@ pub(super) fn read_assets<T: BufRead + Seek>(
|
||||
|
||||
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<T: BufRead + Seek>(
|
||||
let file_hashes: &mut BTreeSet<u64> = 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)
|
||||
}
|
||||
|
||||
|
||||
+27
-7
@@ -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<T: BufRead>(
|
||||
mut reader: T,
|
||||
archive_path: &Path,
|
||||
) -> Result<BTreeMap<u64, BTreeSet<u64>>, ArchiveParsingError> {
|
||||
let mut header_buffer = [0; HEADER_SIZE - TYPE_ID.len()];
|
||||
|
||||
@@ -144,11 +148,13 @@ pub(super) fn read_assets<T: BufRead>(
|
||||
match header.version {
|
||||
103 | 104 => read_assets_with_header::<T, { v103::FOLDER_RECORD_SIZE }>(
|
||||
reader,
|
||||
archive_path,
|
||||
&header,
|
||||
v103::read_folder_record,
|
||||
),
|
||||
105 => read_assets_with_header::<T, { v105::FOLDER_RECORD_SIZE }>(
|
||||
reader,
|
||||
archive_path,
|
||||
&header,
|
||||
v105::read_folder_record,
|
||||
),
|
||||
@@ -160,6 +166,7 @@ pub(super) fn read_assets<T: BufRead>(
|
||||
|
||||
fn read_assets_with_header<T: BufRead, const U: usize>(
|
||||
mut reader: T,
|
||||
archive_path: &Path,
|
||||
header: &Header,
|
||||
read_folder_record: impl Fn(&[u8; U]) -> FolderRecord,
|
||||
) -> Result<BTreeMap<u64, BTreeSet<u64>>, ArchiveParsingError> {
|
||||
@@ -178,15 +185,15 @@ fn read_assets_with_header<T: BufRead, const U: usize>(
|
||||
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::<U>().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<T: BufRead, const U: usize>(
|
||||
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)
|
||||
}
|
||||
|
||||
|
||||
@@ -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}"
|
||||
),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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(),
|
||||
|
||||
Reference in New Issue
Block a user