Decode C++ namespace syntax in NIF block type names

[MANOLOV02]

Certain NIF block types are serialized with C++ namespace syntax: BSSkin::Instance, BSSkin::BoneData, BSConnectPoint::Parents, BSConnectPoint::Children. C# class identifiers can't contain ::, so the corresponding NiflySharp classes are named with _ (e.g. BSSkin_Instance).

In NifFile.Load, blocks are instantiated via Type.GetType("NiflySharp.Blocks." + blockTypeStr) using the binary string directly. That call returns null for any ::-named type, and the block falls through to the NiUnknown catch-all.

Net effect: any FO4 NIF with a skin rig (BSSkin::Instance / BSSkin::BoneData) or connect points loads those blocks as NiUnknown, and GetBlock<BSSkin_Instance>(...) returns null — silent data loss.

C++ nifly avoids the problem because each block class declares its binary name explicitly:

• nifly/include/Skin.hpp:238 — static constexpr const char* BlockName = "BSSkin::Instance";
• nifly/include/Skin.hpp:224 — static constexpr const char* BlockName = "BSSkin::BoneData";

The C++ class identifiers are independent of the binary name (BSSkinInstance the class vs "BSSkin::Instance" the string), so no mapping is needed there.

Fix: introduce a small bidirectional table NifFile.NamespacedBlockTypes and apply it at the two boundaries:

• Load path (NifFile.Load): decode binary → C# name before Type.GetType.
• Save path (NiHeader.AddBlockInfo): encode C# → binary before serializing the header block-type string.

The list is centralized; new ::-named types only need one entry.

Alternative considered: an attribute-based approach mirroring C++ ([NifBlockName("BSSkin::Instance")] on the class). Rejected for this PR to keep the change minimal and reversible — an attribute scan would also need a fallback for types without the attribute and would touch the source generator. Happy to switch to that approach if you'd prefer.

Not in issue #15 (was originally classified as a C# language workaround). It is a real bug that affects any C# consumer loading FO4 NIFs with skin rigs.

[ousnius]

@MANOLOV02 There's an error in one of the tests here. To fix the issue with the "::" naming, I would prefer having the source generator create a new type of attribute into the declarations of these classes that list the "real" class name and checking that attribute in places where the real name is necessary.

So the suggested alternative solution from your description.

For example:

[NifBlockName("BSSkin::Instance")]
partial class BSSkin_Instance

[MANOLOV02]

Pushed the source-generator + attribute approach as suggested. Summary of the changes:

• New NifBlockNameAttribute (records the original binary name on the class, plus a GetBinaryName(Type) helper).
• The generator emits [global::NiflySharp.NifBlockName("...")] on every class whose nif.xml name differs from its C# identifier (i.e. those with ::). No hand-maintained list.
• NifFile.Save (line 355) and NiHeader.AddBlockInfo / ReplaceBlockInfo use NifBlockNameAttribute.GetBinaryName(type) for the binary write.
• NifFile.Load builds a one-shot binaryName -> Type lookup over the assembly's NiflySharp.Blocks namespace, with a fallback to Type.GetType for the (typical) case where the names already match.

---

About the LoadAndSave_Skinned_SF test failure

The CI failure here is a pre-existing latent bug that this PR exposes rather than introduces. Before this PR, BSSkin::Instance and BSSkin::BoneData failed Type.GetType("NiflySharp.Blocks.BSSkin::Instance") and fell through to NiUnknown, which round-trips as an opaque byte blob — so the test passed by accident. Once they are loaded as their typed classes, an unrelated FinalizeData issue surfaces:

• Skinned.nif (Starfield) is a standalone skinned mesh: every entry of BSSkin_Instance.Bones is NPOS because the bones live in the external skeleton, to be resolved at runtime.
• FinalizeData calls CleanInvalidRefs() on every reference array of every block.
• NiBlockRefArray.CleanInvalidRefs strips entries whose Index == NPOS unless KeepEmptyRefs is set, shrinking Bones from 38 → 0.
• BSSkin_Instance.Sync (write) then emits NumBones=0 and no pointers, so the saved block is 152 bytes shorter than the input and the byte-for-byte test fails.

There is precedent for handling this kind of NiBlockPtrArray pointing at externally-referenced objects: NiMultiTargetTransformController.ExtraTargets already opts in with KeepEmptyRefs = true in its hand-written partial. Applying the same pattern to BSSkin_Instance.Bones makes the SF test pass locally (34/34) without any other changes.

Proposed follow-up (not in this PR)

I have a small follow-up branch ready if you'd prefer it as a separate PR (so this one stays focused on the namespace decoding). It contains only:

• BSSkin_Instance partial: hand-written constructor that sets _bones = new NiBlockPtrArray<NiNode> { KeepEmptyRefs = true }, mirroring NiMultiTargetTransformController.
• SourceGenUtil.ClassesWithHandWrittenDefaultCtor: adds "BSSkin_Instance" so the generator does not emit a competing default ctor.

Let me know whether you'd like me to open that as its own PR, fold it into this one, or take a different approach (e.g. defaulting KeepEmptyRefs = true on NiBlockPtrArray itself, which would handle the broader category of Ptr arrays whose NPOS slots are legitimate). Happy to align with whichever direction you prefer.

[ousnius]

@MANOLOV02 You can set KeepEmptyRefs and include it in this PR. But what makes it harder is that it would be better if KeepEmptyRefs is only set for Starfield and only for BSSkin::Instance.

[MANOLOV02]

Folded in the KeepEmptyRefs fix per your guidance — scoped to Starfield and to BSSkin_Instance only.

Implementation in NiflySharp/Blocks/BSSkin_Instance.cs:

public new void BeforeSync(NiStreamReversible stream)
{
    if (_bones == null)
        _bones = new NiBlockPtrArray<NiNode>();

    _bones.KeepEmptyRefs = stream.Version.IsSF();
}

Followed two existing patterns in the codebase: the BeforeSync hook from NiMultiTargetTransformController (which sets KeepEmptyRefs on its own NiBlockPtrArray), and the version gating from BSTriShape.BeforeSync which uses stream.Version.IsSSE() for SSE-specific logic. Here IsSF() keeps the flag off for FO4 / FO76 standalone files where the Bones pointers are not NPOS, matching the constraint you suggested.

No changes needed to the source generator side — the partial only adds BeforeSync, so the generator-emitted default constructor still works.

Local test run is 34/34 green, including both LoadAndSave_Skinned_SF and LoadAndSave_Skinned_FO4.