Clean up top-level conversion utilities.

Penumbra/Import/Models/HavokConverter.cs

@@ -2,24 +2,19 @@ using FFXIVClientStructs.Havok;
 
 namespace Penumbra.Import.Models;
 
-// TODO: where should this live? interop i guess, in penum? or game data?
+public static unsafe class HavokConverter
-public unsafe class HavokConverter
 {
-    /// <summary>Creates a temporary file and returns its path.</summary>
+    /// <summary> Creates a temporary file and returns its path. </summary>
-    /// <returns>Path to a temporary file.</returns>
+    private static string CreateTempFile()
-    private string CreateTempFile()
     {
-        var s = File.Create(Path.GetTempFileName());
+        var stream = File.Create(Path.GetTempFileName());
-        s.Close();
+        stream.Close();
-        return s.Name;
+        return stream.Name;
     }
 
-    /// <summary>Converts a .hkx file to a .xml file.</summary>
+    /// <summary> Converts a .hkx file to a .xml file. </summary>
-    /// <param name="hkx">A byte array representing the .hkx file.</param>
+    /// <param name="hkx"> A byte array representing the .hkx file. </param> 
-    /// <returns>A string representing the .xml file.</returns>
+    public static string HkxToXml(byte[] hkx)
-    /// <exception cref="Exceptions.HavokReadException">Thrown if parsing the .hkx file fails.</exception>
-    /// <exception cref="Exceptions.HavokWriteException">Thrown if writing the .xml file fails.</exception>
-    public string HkxToXml(byte[] hkx)
     {
         var tempHkx = CreateTempFile();
         File.WriteAllBytes(tempHkx, hkx);
@@ -27,7 +22,7 @@ public unsafe class HavokConverter
         var resource = Read(tempHkx);
         File.Delete(tempHkx);
 
-        if (resource == null) throw new Exception("HavokReadException");
+        if (resource == null) throw new Exception("Failed to read havok file.");
 
         var options = hkSerializeUtil.SaveOptionBits.SerializeIgnoredMembers
             | hkSerializeUtil.SaveOptionBits.TextFormat
@@ -42,12 +37,9 @@ public unsafe class HavokConverter
         return bytes;
     }
 
-    /// <summary>Converts a .xml file to a .hkx file.</summary>
+    /// <summary> Converts an .xml file to a .hkx file. </summary>
-    /// <param name="xml">A string representing the .xml file.</param>
+    /// <param name="xml"> A string representing the .xml file. </param>
-    /// <returns>A byte array representing the .hkx file.</returns>
+    public static byte[] XmlToHkx(string xml)
-    /// <exception cref="Exceptions.HavokReadException">Thrown if parsing the .xml file fails.</exception>
-    /// <exception cref="Exceptions.HavokWriteException">Thrown if writing the .hkx file fails.</exception>
-    public byte[] XmlToHkx(string xml)
     {
         var tempXml = CreateTempFile();
         File.WriteAllText(tempXml, xml);
@@ -55,7 +47,7 @@ public unsafe class HavokConverter
         var resource = Read(tempXml);
         File.Delete(tempXml);
 
-        if (resource == null) throw new Exception("HavokReadException");
+        if (resource == null) throw new Exception("Failed to read havok file.");
 
         var options = hkSerializeUtil.SaveOptionBits.SerializeIgnoredMembers
             | hkSerializeUtil.SaveOptionBits.WriteAttributes;
@@ -74,9 +66,8 @@ public unsafe class HavokConverter
     /// The type is guessed automatically by Havok.
     /// This pointer might be null - you should check for that.
     /// </summary>
-    /// <param name="filePath">Path to a file on the filesystem.</param>
+    /// <param name="filePath"> Path to a file on the filesystem. </param>
-    /// <returns>A (potentially null) pointer to an hkResource.</returns>
+    private static hkResource* Read(string filePath)
-    private hkResource* Read(string filePath)
     {
         var path = Marshal.StringToHGlobalAnsi(filePath);
 
@@ -87,18 +78,15 @@ public unsafe class HavokConverter
         loadOptions->ClassNameRegistry = builtinTypeRegistry->GetClassNameRegistry();
         loadOptions->TypeInfoRegistry = builtinTypeRegistry->GetTypeInfoRegistry();
 
-        // TODO: probably can loadfrombuffer this
+        // TODO: probably can use LoadFromBuffer for this.
         var resource = hkSerializeUtil.LoadFromFile((byte*)path, null, loadOptions);
         return resource;
     }
 
-    /// <summary>Serializes an hkResource* to a temporary file.</summary>
+    /// <summary> Serializes an hkResource* to a temporary file. </summary>
-    /// <param name="resource">A pointer to the hkResource, opened through Read().</param>
+    /// <param name="resource"> A pointer to the hkResource, opened through Read(). </param>
-    /// <param name="optionBits">Flags representing how to serialize the file.</param>
+    /// <param name="optionBits"> Flags representing how to serialize the file. </param>
-    /// <returns>An opened FileStream of a temporary file. You are expected to read the file and delete it.</returns>
+    private static FileStream Write(
-    /// <exception cref="Exceptions.HavokWriteException">Thrown if accessing the root level container fails.</exception>
-    /// <exception cref="Exceptions.HavokFailureException">Thrown if an unknown failure in writing occurs.</exception>
-    private FileStream Write(
         hkResource* resource,
         hkSerializeUtil.SaveOptionBits optionBits
     )
@@ -125,16 +113,16 @@ public unsafe class HavokConverter
             var name = "hkRootLevelContainer";
 
             var resourcePtr = (hkRootLevelContainer*)resource->GetContentsPointer(name, typeInfoRegistry);
-            if (resourcePtr == null) throw new Exception("HavokWriteException");
+            if (resourcePtr == null) throw new Exception("Failed to retrieve havok root level container resource.");
 
             var hkRootLevelContainerClass = classNameRegistry->GetClassByName(name);
-            if (hkRootLevelContainerClass == null) throw new Exception("HavokWriteException");
+            if (hkRootLevelContainerClass == null) throw new Exception("Failed to retrieve havok root level container type.");
 
             hkSerializeUtil.Save(result, resourcePtr, hkRootLevelContainerClass, oStream.StreamWriter.ptr, saveOptions);
         }
         finally { oStream.Dtor(); }
 
-        if (result->Result == hkResult.hkResultEnum.Failure) throw new Exception("HavokFailureException");
+        if (result->Result == hkResult.hkResultEnum.Failure) throw new Exception("Failed to serialize havok file.");
 
         return new FileStream(tempFile, FileMode.Open);
     }

Penumbra/Import/Models/ModelManager.cs

@@ -89,17 +89,12 @@ public sealed class ModelManager : SingleTaskQueue, IDisposable
             if (_sklb == null)
                 return null;
 
-            // TODO: Consider making these static methods.
             // TODO: work out how i handle this havok deal. running it outside the framework causes an immediate ctd.
-            var havokConverter = new HavokConverter();
+            var xmlTask = _manager._framework.RunOnFrameworkThread(() => HavokConverter.HkxToXml(_sklb.Skeleton));
-            var xmlTask = _manager._framework.RunOnFrameworkThread(() => havokConverter.HkxToXml(_sklb.Skeleton));
             xmlTask.Wait(cancel);
             var xml = xmlTask.Result;
 
-            var skeletonConverter = new SkeletonConverter();
+            return SkeletonConverter.FromXml(xml);
-            var skeleton = skeletonConverter.FromXml(xml);
-
-            return skeleton;
         }
 
         public bool Equals(IAction? other)

Penumbra/Import/Models/SkeletonConverter.cs

@@ -4,10 +4,11 @@ using Penumbra.Import.Models.Export;
 
 namespace Penumbra.Import.Models;
 
-// TODO: tempted to say that this living here is more okay? that or next to havok converter, wherever that ends up.
+public static class SkeletonConverter
-public class SkeletonConverter
 {
-    public XivSkeleton FromXml(string xml)
+    /// <summary> Parse XIV skeleton data from a havok XML tagfile. </summary>
+    /// <param name="xml"> Havok XML tagfile containing skeleton data. </param>
+    public static XivSkeleton FromXml(string xml)
     {
         var document = new XmlDocument();
         document.LoadXml(xml);
@@ -16,14 +17,14 @@ public class SkeletonConverter
 
         var skeletonNode = document.SelectSingleNode($"/hktagfile/object[@type='hkaSkeleton'][@id='{mainSkeletonId}']");
         if (skeletonNode == null)
-            throw new InvalidDataException();
+            throw new InvalidDataException($"Failed to find skeleton with id {mainSkeletonId}.");
 
         var referencePose = ReadReferencePose(skeletonNode);
         var parentIndices = ReadParentIndices(skeletonNode);
         var boneNames = ReadBoneNames(skeletonNode);
 
         if (boneNames.Length != parentIndices.Length || boneNames.Length != referencePose.Length)
-            throw new InvalidDataException();
+            throw new InvalidDataException($"Mismatch in bone value array lengths: names({boneNames.Length}) parents({parentIndices.Length}) pose({referencePose.Length})");
 
         var bones = referencePose
             .Zip(parentIndices, boneNames)
@@ -38,27 +39,27 @@ public class SkeletonConverter
                 };
             })
             .ToArray();
-        
+
         return new XivSkeleton(bones);
     }
 
-    /// <summary>Get the main skeleton ID for a given skeleton document.</summary>
+    /// <summary> Get the main skeleton ID for a given skeleton document. </summary>
-    /// <param name="node">XML skeleton document.</param>
+    /// <param name="node"> XML skeleton document. </param>
-    private string GetMainSkeletonId(XmlNode node)
+    private static string GetMainSkeletonId(XmlNode node)
     {
         var animationSkeletons = node
             .SelectSingleNode("/hktagfile/object[@type='hkaAnimationContainer']/array[@name='skeletons']")?
             .ChildNodes;
 
         if (animationSkeletons?.Count != 1)
-            throw new Exception($"Assumption broken: Expected 1 hkaAnimationContainer skeleton, got {animationSkeletons?.Count ?? 0}");
+            throw new Exception($"Assumption broken: Expected 1 hkaAnimationContainer skeleton, got {animationSkeletons?.Count ?? 0}.");
 
         return animationSkeletons[0]!.InnerText;
     }
 
-    /// <summary>Read the reference pose transforms for a skeleton.</summary>
+    /// <summary> Read the reference pose transforms for a skeleton. </summary>
-    /// <param name="node">XML node for the skeleton.</param>
+    /// <param name="node"> XML node for the skeleton. </param>
-    private XivSkeleton.Transform[] ReadReferencePose(XmlNode node)
+    private static XivSkeleton.Transform[] ReadReferencePose(XmlNode node)
     {
         return ReadArray(
             CheckExists(node.SelectSingleNode("array[@name='referencePose']")),
@@ -75,7 +76,9 @@ public class SkeletonConverter
         );
     }
 
-    private float[] ReadVec12(XmlNode node)
+    /// <summary> Read a 12-item vector from a tagfile. </summary>
+    /// <param name="node"> Havok Vec12 XML node. </param>
+    private static float[] ReadVec12(XmlNode node)
     {
         var array = node.ChildNodes
             .Cast<XmlNode>()
@@ -89,12 +92,14 @@ public class SkeletonConverter
             .ToArray();
 
         if (array.Length != 12)
-            throw new InvalidDataException();
+            throw new InvalidDataException($"Unexpected Vector12 length ({array.Length}).");
 
         return array;
     }
 
-    private int[] ReadParentIndices(XmlNode node)
+    /// <summary> Read the bone parent relations for a skeleton. </summary>
+    /// <param name="node"> XML node for the skeleton. </param>
+    private static int[] ReadParentIndices(XmlNode node)
     {
         // todo: would be neat to genericise array between bare and children
         return CheckExists(node.SelectSingleNode("array[@name='parentIndices']"))
@@ -104,7 +109,9 @@ public class SkeletonConverter
             .ToArray();
     }
 
-    private string[] ReadBoneNames(XmlNode node)
+    /// <summary> Read the names of bones in a skeleton. </summary>
+    /// <param name="node"> XML node for the skeleton. </param>
+    private static string[] ReadBoneNames(XmlNode node)
     {
         return ReadArray(
             CheckExists(node.SelectSingleNode("array[@name='bones']")),
@@ -112,7 +119,10 @@ public class SkeletonConverter
         );
     }
 
-    private T[] ReadArray<T>(XmlNode node, Func<XmlNode, T> convert)
+    /// <summary> Read an XML tagfile array, converting it via the provided conversion function. </summary>
+    /// <param name="node"> Tagfile XML array node. </param>
+    /// <param name="convert"> Function to convert array item nodes to required data types. </param>
+    private static T[] ReadArray<T>(XmlNode node, Func<XmlNode, T> convert)
     {
         var element = (XmlElement)node;
 
@@ -125,6 +135,7 @@ public class SkeletonConverter
         return array;
     }
 
+    /// <summary> Check if the argument is null, returning a non-nullable value if it exists, and throwing if not. </summary>
     private static T CheckExists<T>(T? value)
     {
         ArgumentNullException.ThrowIfNull(value);