Logic error in BlbFile.cs:Blb3File.ReadBlocksInfoAndDirectory regarding BundleFile.Node.flags parsing

[ihzgniqgnem]

Describe the bug
There is a logical flaw in the Blb3File constructor, specifically within the ReadBlocksInfoAndDirectory method when parsing BundleFile.Node.flags.
The current implementation incorrectly reads file flags for nodes with an index $i \ge 64$ and produces nonsensical flag values (flags value should not depend on sequence) for all nodes due to incorrect bitwise operations.

To be specific, when $i \ge 0x20(32)$, the control flow will enter this branch

// In file AnimeStudio\BlbFile.cs:113
reader.Position = flagInfoOffset;
var flag = reader.ReadUInt32();
if (i >= 0x20)
{
    flag = reader.ReadUInt32();//<-
}
m_DirectoryInfo[i].flags = (uint)(flag & (1 << i)) * 4;

This might be intentional, but when $i \ge 64$, the result of flag & (1 << i) is meaningless, because the value of variable flag is still the value when $64 > i \ge 32$, so it will return the flag value of flagInfo[32 + i % 32]. This is not what we wanted.
If nodesCount is never greater than 64, this issue may never surface.
However, one thing is certain, flag & (1 << i) makes the flags value depend on sequence, which is completely wrong.

To Reproduce
N/A

Expected behavior
Since I haven't found any cases of using BundleFile.Node.flags in the code yet, perhaps the loading of the flag here can be removed, or can be changed to

// This code fragment is not verified, please check it
reader.Position = flagInfoOffset + ((i >> 5) << 2)
var flag = reader.ReadUInt32();
m_DirectoryInfo[i].flags = (flag >> (i & 0x1f) & 1) << 2;

Screenshots
N/A

Version
master branch 4d758ef

Unity Version
N/A

Game/Sample
N/A

Additional context
The right shift wrap-around of integer literals in C# is also implicit and not easy to understand. Perhaps it can be explicitly written as 1 << (i % 32), 1 << (i & 0x1f) or at least write a comment to explain it.

[Escartem]

The second flag read is because it becomes uint64, it is not used under but like most of studio code for parsing files, a lot of unused variables are here to respect the original file structure and make it clearer at the byte arrangement. Does this causes any bug in itself ?

[ihzgniqgnem]

Thank you for your reply. Before we delve into discussing the fix, I would like to confirm a key question first: in the actual game files, is it possible for nodesCount to exceed 64?

[Escartem]

Technically yes, it's stored as a uint32 so could be any value in here, though in general nodes count is usually 2, one for the serialized file and one for the resources files

Also another thing to mention, BlbFile is derived from the usual BundleFile, this one has others derivations such as MhyFile, HygFile, VFSFile and such, as far as I remember this flag extension is specific to mihoyo's blb files but could it impact the other structures too from the others variants of a bundle

[ihzgniqgnem]

But in any case, it is abnormal for flags to be related to storage order or index.
Flags should be inherent properties of the node itself, not determined by its position in the array. If we cannot delete the field, perhaps we can leave it unset? It might be better than assigning the wrong value.

[Escartem]

I guess sure, do you want to make a PR for it ?

[ihzgniqgnem]

I have already written a PR to remove unused and incorrect logic to fix it.