Fix #4946. Refactor model wall join operators into core for better error reporting

The operator is no longer needed and all the variations can be called exactly at the workspace hotkey level now (and thus nested operator errors won't cause runtime exceptions)

src/blenderbim/blenderbim/bim/module/model/__init__.py

@@ -70,10 +70,10 @@ classes = (
     wall.ChangeExtrusionXAngle,
     wall.ChangeLayerLength,
     wall.FlipWall,
-    wall.JoinWall,
     wall.MergeWall,
     wall.RecalculateWall,
     wall.SplitWall,
+    wall.UnjoinWalls,
     opening.AddBoolean,
     opening.AddFilledOpening,
     opening.AddPotentialHalfSpaceSolid,

src/blenderbim/blenderbim/bim/module/model/wall.py

@@ -41,74 +41,17 @@ from blenderbim.bim.module.model.opening import FilledOpeningGenerator
 from typing import Optional
 
 
-class JoinWall(bpy.types.Operator, tool.Ifc.Operator):
-    bl_idname = "bim.join_wall"
-    bl_label = "Join Wall"
+class UnjoinWalls(bpy.types.Operator, tool.Ifc.Operator):
+    bl_idname = "bim.unjoin_walls"
+    bl_label = "Unjoin Walls"
     bl_options = {"REGISTER", "UNDO"}
-    bl_description = """ Trim/Extend the selected walls to the last selected wall:
-    'T' mode: Trim/Extend to a selected wall, slab, or 3D target
-    'L' mode: Butt join two selected walls
-    'V' mode: Mitre join two selected wall
-    '' (empty) mode: Unjoin selected walls
-    """
-    join_type: bpy.props.StringProperty()
 
     @classmethod
     def poll(cls, context):
         return context.selected_objects
 
     def _execute(self, context):
-        selected_objs = [o for o in context.selected_objects if o.BIMObjectProperties.ifc_definition_id]
-        joiner = DumbWallJoiner()
-        if not self.join_type:
-            for obj in selected_objs:
-                joiner.unjoin(obj)
-            return {"FINISHED"}
-
-        if not context.active_object or not context.active_object.BIMObjectProperties.ifc_definition_id:
-            self.report({"ERROR"}, f"No active object selected")
-            return {"CANCELLED"}
-
-        for obj in selected_objs:
-            tool.Geometry.clear_scale(obj)
-
-        if not selected_objs:
-            self.report({"ERROR"}, f"No IFC objects selected")
-            return {"CANCELLED"}
-
-        if len(selected_objs) == 1:
-            joiner.join_E(context.active_object, context.scene.cursor.location)
-            return {"FINISHED"}
-
-        if self.join_type in ("L", "V"):
-            try:
-                core.join_wall_LV(tool.Blender, joiner, join_type=self.join_type)
-            except core.RequireTwoObjectsError:
-                join_type_name = {"L": "butt", "V": "mitre"}[self.join_type]
-                self.report({"ERROR"}, f"Please select 2 objects to do a {join_type_name} joint")
-                return {"CANCELLED"}
-            return {"FINISHED"}
-
-        if self.join_type == "T":
-            elements = [tool.Ifc.get_entity(o) for o in context.selected_objects]
-            layer2_elements = []
-            layer3_elements = []
-            for element in elements:
-                usage = tool.Model.get_usage_type(element)
-                if usage == "LAYER2":
-                    layer2_elements.append(element)
-                elif usage == "LAYER3":
-                    layer3_elements.append(element)
-            if layer3_elements:
-                target = tool.Ifc.get_object(layer3_elements[0])
-                for element in layer2_elements:
-                    joiner.join_Z(tool.Ifc.get_object(element), target)
-            else:
-                for obj in selected_objs:
-                    if obj == context.active_object:
-                        continue
-                    joiner.join_T(obj, context.active_object)
-        return {"FINISHED"}
+        core.unjoin_walls(tool.Ifc, tool.Blender, tool.Geometry, DumbWallJoiner(), tool.Model)
 
 
 class AlignWall(bpy.types.Operator):
@@ -1049,8 +992,6 @@ class DumbWallJoiner:
 
     def join_E(self, wall1, target):
         element1 = tool.Ifc.get_entity(wall1)
-        if not element1:
-            return
 
         axis1 = tool.Model.get_wall_axis(wall1)
         intersect, connection = mathutils.geometry.intersect_point_line(target.to_2d(), *axis1["reference"])

src/blenderbim/blenderbim/bim/module/model/workspace.py

@@ -18,10 +18,8 @@
 
 import os
 import bpy
-import ifcopenshell
-import ifcopenshell.util.unit
 import blenderbim.tool as tool
-import blenderbim.core.model
+import blenderbim.core.model as core
 from blenderbim.bim.module.model.wall import DumbWallJoiner
 from blenderbim.bim.helper import prop_with_search
 from bpy.types import WorkSpaceTool
@@ -350,7 +348,7 @@ class BimToolUI:
             add_layout_hotkey_operator(cls.layout, "Split", "S_K", bpy.ops.bim.split_wall.__doc__)
             add_layout_hotkey_operator(cls.layout, "Rotate 90", "S_R", bpy.ops.bim.rotate_90.__doc__)
             add_layout_hotkey_operator(cls.layout, "Regen", "S_G", bpy.ops.bim.recalculate_wall.__doc__)
-            row.operator("bim.join_wall", icon="X", text="").join_type = ""
+            row.operator("bim.unjoin_walls", icon="X", text="")
 
         elif AuthoringData.data["active_material_usage"] == "LAYER3":
             if len(context.selected_objects) == 1:
@@ -680,7 +678,14 @@ class Hotkey(bpy.types.Operator, tool.Ifc.Operator):
                     bpy.ops.bim.enable_editing_extrusion_profile()
             elif self.active_material_usage == "LAYER2":
                 # Extend LAYER2 to cursor
-                bpy.ops.bim.join_wall(join_type="T")
+                core.extend_walls(
+                    tool.Ifc,
+                    tool.Blender,
+                    tool.Geometry,
+                    DumbWallJoiner(),
+                    tool.Model,
+                    bpy.context.scene.cursor.location,
+                )
             elif self.active_material_usage == "PROFILE":
                 # Extend PROFILE to cursor
                 bpy.ops.bim.extend_profile(join_type="T")
@@ -698,13 +703,19 @@ class Hotkey(bpy.types.Operator, tool.Ifc.Operator):
             # Extend LAYER2s to LAYER3
             [o.select_set(False) for o in selected_usages.get("PROFILE", [])]
             [o.select_set(False) for o in selected_usages.get("LAYER3", []) if o != bpy.context.active_object]
-            bpy.ops.bim.join_wall(join_type="T")
+            try:
+                core.join_walls_TZ(tool.Ifc, tool.Blender, tool.Geometry, DumbWallJoiner(), tool.Model)
+            except core.RequireAtLeastTwoLayeredElements as e:
+                self.report({"ERROR"}, str(e))
 
         elif self.active_material_usage == "LAYER2":
             # Extend LAYER2s to LAYER2
             [o.select_set(False) for o in selected_usages.get("LAYER3", [])]
             [o.select_set(False) for o in selected_usages.get("PROFILE", [])]
-            bpy.ops.bim.join_wall(join_type="T")
+            try:
+                core.join_walls_TZ(tool.Ifc, tool.Blender, tool.Geometry, DumbWallJoiner(), tool.Model)
+            except core.RequireAtLeastTwoLayeredElements as e:
+                self.report({"ERROR"}, str(e))
 
         elif self.active_material_usage == "PROFILE":
             # Extend PROFILEs to PROFILE
@@ -722,7 +733,6 @@ class Hotkey(bpy.types.Operator, tool.Ifc.Operator):
         elif self.active_material_usage == "PROFILE":
             bpy.ops.bim.flip_object(flip_local_axes="XZ")
 
-
     def hotkey_S_G(self):
         obj = bpy.context.active_object
         element = tool.Ifc.get_entity(obj)
@@ -755,7 +765,7 @@ class Hotkey(bpy.types.Operator, tool.Ifc.Operator):
         else:
             if len(bpy.context.selected_objects) == 1:
                 self.report(
-                    {"INFO"},
+                    {"ERROR"},
                     "At least two objects must be selected: an object to be mirrored, and a mirror axis as the active object.",
                 )
             else:
@@ -782,10 +792,9 @@ class Hotkey(bpy.types.Operator, tool.Ifc.Operator):
             return
         if self.active_material_usage == "LAYER2":
             try:
-                blenderbim.core.model.join_wall_LV(tool.Blender, DumbWallJoiner(), join_type="L")
-            except blenderbim.core.model.RequireTwoObjectsError:
-                self.report({"ERROR"}, "Please select 2 objects to do a butt joint")
-                return {"CANCELLED"}
+                core.join_walls_LV(tool.Ifc, tool.Blender, tool.Geometry, DumbWallJoiner(), tool.Model, join_type="L")
+            except core.RequireTwoWallsError as e:
+                self.report({"ERROR"}, str(e))
         elif self.active_material_usage == "PROFILE":
             bpy.ops.bim.extend_profile(join_type="L")
 
@@ -811,16 +820,14 @@ class Hotkey(bpy.types.Operator, tool.Ifc.Operator):
             return
         if self.active_material_usage == "LAYER2":
             try:
-                blenderbim.core.model.join_wall_LV(tool.Blender, DumbWallJoiner(), join_type="V")
-            except blenderbim.core.model.RequireTwoObjectsError:
-                self.report({"ERROR"}, "Please select 2 objects to do a mitre joint")
-                return {"CANCELLED"}
+                core.join_walls_LV(tool.Ifc, tool.Blender, tool.Geometry, DumbWallJoiner(), tool.Model, join_type="V")
+            except core.RequireTwoWallsError as e:
+                self.report({"ERROR"}, str(e))
         elif self.active_class in ("IfcDuctSegment", "IfcPipeSegment", "IfcCableCarrierSegment", "IfcCableSegment"):
             bpy.ops.bim.fit_flow_segments()
         elif self.active_material_usage == "PROFILE":
             bpy.ops.bim.extend_profile(join_type="V")
 
-
     def hotkey_S_B(self):
         bpy.ops.bim.add_boundary()
 
@@ -828,7 +835,6 @@ class Hotkey(bpy.types.Operator, tool.Ifc.Operator):
         if len(bpy.context.selected_objects) == 2:
             bpy.ops.bim.add_opening()
         else:
-            unit_scale = ifcopenshell.util.unit.calculate_unit_scale(tool.Ifc.get())
             bpy.ops.bim.add_potential_opening(x=self.x, y=self.y, z=self.z)
             self.props.x = self.x
             self.props.y = self.y

src/blenderbim/blenderbim/core/model.py

@@ -20,17 +20,91 @@ import blenderbim.core.tool as tool
 from typing import Literal
 
 
-def join_wall_LV(blender: tool.Blender, joiner, join_type: Literal["L", "V"] = "L") -> None:
-    if len(selected_objs := blender.get_selected_objects()) != 2:
-        raise RequireTwoObjectsError()
+def unjoin_walls(ifc: tool.Ifc, blender: tool.Blender, geometry: tool.Geometry, joiner, model: tool.Model) -> None:
+    for obj in blender.get_selected_objects():
+        if not (element := ifc.get_entity(obj)) or model.get_usage_type(element) != "LAYER2":
+            continue
+        geometry.clear_scale(obj)
+        joiner.unjoin(obj)
+
+
+def extend_walls(
+    ifc: tool.Ifc, blender: tool.Blender, geometry: tool.Geometry, joiner, model: tool.Model, target
+) -> None:
+    for obj in blender.get_selected_objects():
+        if not (element := ifc.get_entity(obj)) or model.get_usage_type(element) != "LAYER2":
+            continue
+        geometry.clear_scale(obj)
+        joiner.join_E(obj, target)
+
+
+def join_walls_LV(
+    ifc: tool.Ifc,
+    blender: tool.Blender,
+    geometry: tool.Geometry,
+    joiner,
+    model: tool.Model,
+    join_type: Literal["L", "V"] = "L",
+) -> None:
+    selected_objs = [
+        o for o in blender.get_selected_objects() if (e := ifc.get_entity(o)) and model.get_usage_type(e) == "LAYER2"
+    ]
+    if len(selected_objs) != 2:
+        raise RequireTwoWallsError("Two vertically layered elements must be selected to connect their paths together")
+
+    if active_obj := blender.get_active_object():
+        another_selected_object = next(o for o in selected_objs if o != active_obj)
+    else:
+        active_obj, another_selected_object = selected_objs
+
+    for obj in selected_objs:
+        geometry.clear_scale(obj)
 
-    active_obj = blender.get_active_object()
-    another_selected_object = next(o for o in selected_objs if o != active_obj)
     if join_type == "L":
         joiner.join_L(another_selected_object, active_obj)
     elif join_type == "V":
         joiner.join_V(another_selected_object, active_obj)
 
 
-class RequireTwoObjectsError(Exception):
+def join_walls_TZ(ifc: tool.Ifc, blender: tool.Blender, geometry: tool.Geometry, joiner, model: tool.Model) -> None:
+    selected_objs = [
+        o
+        for o in blender.get_selected_objects()
+        if (e := ifc.get_entity(o)) and model.get_usage_type(e) in ("LAYER2", "LAYER3")
+    ]
+    if len(selected_objs) != 2:
+        raise RequireAtLeastTwoLayeredElements(
+            "Two or more vertically or horizontally layered elements must be selected to connect their paths together"
+        )
+
+    for obj in selected_objs:
+        geometry.clear_scale(obj)
+
+    elements = [ifc.get_entity(o) for o in blender.get_selected_objects()]
+    layer2_elements = []
+    layer3_elements = []
+    for element in elements:
+        usage = model.get_usage_type(element)
+        if usage == "LAYER2":
+            layer2_elements.append(element)
+        elif usage == "LAYER3":
+            layer3_elements.append(element)
+    if layer3_elements:
+        target = ifc.get_object(layer3_elements[0])
+        for element in layer2_elements:
+            joiner.join_Z(ifc.get_object(element), target)
+    else:
+        if not (active_obj := blender.get_active_object()):
+            active_obj = selected_objs[0]
+        for obj in selected_objs:
+            if obj == active_obj:
+                continue
+            joiner.join_T(obj, active_obj)
+
+
+class RequireTwoWallsError(Exception):
+    pass
+
+
+class RequireAtLeastTwoLayeredElements(Exception):
     pass

src/blenderbim/blenderbim/core/tool.py

@@ -537,6 +537,7 @@ class Model:
     def import_rectangle(cls, obj, position, profile): pass
     def load_openings(cls, openings): pass
     def clear_scene_openings(cls): pass
+    def get_usage_type(cls, element): pass
     def get_material_layer_parameters(cls, element): pass
     def get_manual_booleans(cls, element): pass
     def get_wall_axis(cls, obj, layers=None): pass

src/blenderbim/blenderbim/tool/model.py

@@ -41,7 +41,7 @@ from blenderbim.bim.module.geometry.helper import Helper
 from blenderbim.bim.module.model.data import AuthoringData, RailingData, RoofData, WindowData, DoorData
 from blenderbim.bim.module.model.opening import FilledOpeningGenerator
 from ifcopenshell.util.shape_builder import V, ShapeBuilder
-from typing import Optional, Union, TypeVar, Any, Iterable
+from typing import Optional, Union, TypeVar, Any, Iterable, Literal
 
 T = TypeVar("T")
 
@@ -570,7 +570,9 @@ class Model(blenderbim.core.tool.Model):
             return material.MaterialProfiles[0].Profile
 
     @classmethod
-    def get_usage_type(cls, element):
+    def get_usage_type(
+        cls, element: ifcopenshell.entity_instance
+    ) -> Optional[Literal["LAYER1", "LAYER2", "LAYER3", "PROFILE"]]:
         material = ifcopenshell.util.element.get_material(element, should_inherit=False)
         if material:
             if material.is_a("IfcMaterialLayerSetUsage"):