TL;DR: The warning added a lot more noise than expected, doesn't really
add value, and improving it isn't worth the effort.
The warning was logged when the same pair of folder hash and file hash
appeared more than once within the set of archive files being loaded
(as the function was used, the set would be the archives loaded by a
single plugin).
Hashes are used directly instead of asset file paths because the paths
are not necessarily present in BSA files, and while they are present in
BA2 files (at least, I'm not aware of any option to omit them or any
files that do omit them), hashes are calculated from the file paths they
contain for consistency with the approach needed for BSA files.
Without the file paths it's not possible to determine if the repeated
hashes represent the same asset file (possibly containing different
data) or different files that have colliding hashes.
I had assumed that it would be unusual for a single plugin to load more
than one archive file containing the same asset file path, so any
repeated hash pairs would likely indicate hash collisions, but feedback
from Pickysaurus on behalf of Vortex users indicates that's not true,
and that logging all the warnings significantly slows down fully loading
plugins.
There are a few ways that the logged warning could be improved: tracking
which archive file existing hashes were inserted from would make it
easier to identify the pair of archive files that might need
investigating; reading BA2 files could defer transforming their asset file
paths into hashes until after all the files for a plugin have been read
(or even past that, to account for hash collisions between different
plugins' assets); and reading BSA files could opportunistically store the
asset file paths if they are present, and fall back to comparing using
hashes if not.
However, even if the warning was logged for only true positive hash
collisions, this is all in service of a sorting heuristic that is only
used when adding overlap edges and a pair of plugins do not have
overlapping records but do both load assets, and a collision would mean
that a plugin might seem to load fewer assets than it does, and could
also appear to overlap with a plugin that doesn't actually load assets
with the same file paths. That in turn might result in the two plugins
loading in one order instead of the other, causing one's assets to
override the other's. If that's a problem, then it can be fixed using
load after metadata, and you can only really tell if it's a problem by
spotting something wrong in game, so the warning doesn't really add much
value, and although it indicates that there might be a problem, there's
enough conditions between the warning and there actually being an issue
that logging it as a warning is excessive anyway.
Some tests have been updated because UTF-8 is used as the native
path encoding with MinGW/Wine, unlike MSVC/Windows.
Some of the tests fail:
- 4 Rust tests fail because long paths are not enabled and so the
paths used when creating symlinks and junction paths are too
long. I've tested them with x86_64-pc-windows-gnu and
x86_64-pc-windows-gnullvm, and both see the same behaviour. The
tests pass when the MinGW-built executable is run on Windows, so
this is a Wine limitation.
- 12 C++ tests fail because directory symlink creation is not
implemented. They fail whether the MinGW-built executable is run
in Wine or on Windows, so this is a MinGW limitation.
- 1 C++ filesystem test fails because long paths are not enabled.
The failing tests are skipped at runtime when built with MinGW,
aside from the one test for long paths being enabled, which expects
them to be disabled when built with MinGW.
If long paths are enabled, e.g. by running
wine reg add HKLM\\System\\CurrentControlSet\\Control\\Filesystem /v LongPathsEnabled /t REG_DWORD /d 1 /f
then many more tests fail because the C++ tests create long paths
when that Registry value is set, but it doesn't seem to actually
enable long path support in Wine, so various filesystem operations
fail.
Oliver Hamlet
Oliver HamletandGitHub
dependabot[bot]andGitHub