From 1c6d160a1a26ba266ebc5ac73d8d240cba18deb9 Mon Sep 17 00:00:00 2001
From: Lucas Schwiderski <lucas@lschwiderski.de>
Date: Wed, 7 Apr 2021 01:41:07 +0200
Subject: [PATCH] doc(import_bsi): Improve comments

Signed-off-by: Lucas Schwiderski <lucas@lschwiderski.de>
---
 addons/bitsquid/import_bsi.py | 52 +++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 21 deletions(-)

diff --git a/addons/bitsquid/import_bsi.py b/addons/bitsquid/import_bsi.py
index d0325f7..b775b46 100644
--- a/addons/bitsquid/import_bsi.py
+++ b/addons/bitsquid/import_bsi.py
@@ -20,15 +20,14 @@ import json
 import bpy
 import mathutils
 
-from bpy_extras.io_utils import unpack_list
-
 
 def parse_sjson(file_path, skip_editor_data=True):
     """
         Translate Bitsquid .bsi SJSON data to plain JSON,
         then parse into a python dictionary.
 
-        Taken from `bsi_import` in the Vermintide 2 SDK.
+        Taken from `bsi_import` in the Vermintide 2 SDK, but slightly
+        modified to fix some issues and improve readability.
     """
     return_dict = {}
     try:
@@ -167,16 +166,26 @@ def parse_sjson(file_path, skip_editor_data=True):
 
 
 def find(arr, f):
+    """
+    Find a value in a list by executing `f`
+    on each item until it returns `true`.
+    """
     for key, val in arr.items():
         if f(val, key):
             return val, key
 
 
 def create_object(self, context, name, node_data, geometries):
+    """
+    Create a Blender object from a BSI node definition
+    and additional data from the file.
+    """
     print("[create_object]", name, node_data)
+    # A list of tuples, where each tuple represents a Vector3(x, y, z).
     vertices = []
+    # A list of tuples, where each tuple contains three indices into `vertices`.
+    # Those three indices define the vertices that make up the face.
     faces = []
-    edges = []
 
     tri_count = int(geometries["indices"]["size"]) / 3
 
@@ -187,6 +196,10 @@ def create_object(self, context, name, node_data, geometries):
         for channel in data_stream['channels']:
             stream_data = data_stream["data"]
             if channel['name'] == 'POSITION':
+                # NOTE: Do we need to handle `stride != 3`?
+                # Since the value seems to be fixed per stream, a higher
+                # stride would only be possible for objects that can be built
+                # entirely from quads, which is very uncommon.
                 if stride != 3:
                     raise RuntimeError("stride != 3 cannot be handled")
 
@@ -209,38 +222,36 @@ def create_object(self, context, name, node_data, geometries):
                         index_stream[j + 2],
                     ))
             else:
+                # TODO: Implement other channel types
                 self.report(
                     {'WARNING'},
-                    "Unknown channel name: {}".format(channel["name"])
+                    "Unknown channel type: {}".format(channel["name"])
                 )
 
-    print(vertices, faces)
-
     mesh = bpy.data.meshes.new(name)
-    mesh.from_pydata(vertices, edges, faces)
+    mesh.from_pydata(vertices, [], faces)
 
     return mesh
 
 
 def import_node(self, context, name, node_data, global_data):
+    """Import a BSI node. Recurses into child nodes."""
     print("[import_node]", name, node_data)
     if "geometries" in node_data:
         geometry_name = node_data["geometries"][0]
         print("[import_node] Building geometry '{}' for object '{}'".format(geometry_name, name))
         geometries = global_data["geometries"][geometry_name]
         mesh = create_object(self, context, name, node_data, geometries)
-        print(mesh)
-
         obj = bpy.data.objects.new(mesh.name, mesh)
-        # TODO: Apply `local` tranformation
     else:
         print("[import_node] Adding empty for '{}'".format(name))
-        # TODO: Extract position and rotation from `local` matrix,
-        # if it's not the identity matrix
-        # NOTE: Low priority, Fatshark seems to stick with
-        # identity matrices here
         obj = bpy.data.objects.new(name, None)
 
+    # Needs to happen before scaling can be applied.
+    # TODO: Check if above is true, was seen on Stackoverflow.
+    obj.matrix_world = mathutils.Matrix()
+    # TODO: Apply tranformation matrix in `node_data["local"]`
+
     if "children" in node_data:
         for child_name, child_data in node_data["children"].items():
             if child_data["parent"] != name:
@@ -258,7 +269,8 @@ def import_node(self, context, name, node_data, global_data):
             )
             child_obj.parent = obj
 
-    # Make sure all objects are linked to the current collection
+    # Make sure all objects are linked to the current collection.
+    # Otherwise they won't show up in the outliner.
     collection = context.collection
     collection.objects.link(obj)
     return obj
@@ -275,15 +287,13 @@ def load(self, context, filepath, *, relpath=None):
         return {'CANCELLED'}
 
     view_layer = context.view_layer
-    global_matrix = mathutils.Matrix()
 
     for name, node_data in global_data["nodes"].items():
         obj = import_node(self, context, name, node_data, global_data)
 
-        view_layer.objects.active = obj
         obj.select_set(True)
-
-        # we could apply this anywhere before scaling.
-        obj.matrix_world = global_matrix
+        # One of the objects should be set as active.
+        # This is the easiest to implement and perfectly fine.
+        view_layer.objects.active = obj
 
     return {'FINISHED'}