From 1961cd905ee2d333f10a14e76d61e8aba5c02fe9 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Thu, 28 May 2026 13:48:15 +0200 Subject: [PATCH] Fix parametric framework live-session regressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bundle of bugs surfaced when exercising the new gizmo framework end-to-end in a live Blender session after the bim/module/drawing/gizmos.py refactor + TypeAccessor/CycleType/PickType mixins landed. Register / annotation resolution * parametric_lifecycle.py: hoist `entity_instance` import out of TYPE_CHECKING so typing.get_type_hints resolves the Callable[[entity_instance], bool] annotation at operator registration (CycleDoorType, CycleWindowType, CycleStairType failed with NameError). Clarify the INTERFACE return contract on the picker entry-point so readers see why the gizmo step stays off the undo stack. Framework callable contracts * model/wall.py, door.py, window.py, stair.py: migrate `props_getter` and `element_checker` from bl_idname strings to bound classmethods on tool.Model / tool.Parametric. BaseParametricGizmoGroup.get_props expects a callable; the string form raised TypeError on first gizmo poll. * model/door.py, model/stair.py: drop the dead `prop_path=` operator kwarg from create_arc_gizmo / create_icon_gizmo call sites. The framework helper blindly setattrs every kwarg onto the operator's OperatorProperties, but ToggleDoorSwing / ToggleStairProperty don't declare prop_path — the setattr raised mid-setup_element_specific_gizmos, so self.gizmo_door_type / self.lock_gizmo never got assigned and every subsequent draw_prepare tornadoed AttributeError. Nothing reads op.prop_path anywhere; the kwarg was dead data. Dispatcher operators * model/array.py: add EnableEditingParametric (the framework pen-icon dispatcher that routes to a per-feature edit operator by bl_idname string) and AddArrayFromFeatureEdit (binds the framework's array icon to bim.add_array on the current parametric draft). * model/__init__.py: register both new operators. Per-frame robustness * drawing/gizmos.py: guard BaseParametricGizmoGroup.draw_prepare with is_setup_complete() — matches the existing guard in refresh() and in BaseSchematicGizmoGroup.draw_prepare(). Defense-in-depth: when any subclass's setup raises mid-way, draw_prepare now no-ops cleanly instead of per-frame AttributeError-tornadoing on whatever attribute the failed setup phase was meant to populate. * model/decorator.py: guard ProfileDecorator.__call__ against context.active_object is None. The decorator is a per-frame viewport draw handler; deselecting or deleting the active object while it's installed crashed on obj.mode access. Treat None the same as "no longer in edit mode" — uninstall + fire the exit callback if present. * geometry/data.py: ViewportData.load() populates `data` before flipping `is_loaded`, so a raise from cls.mode() no longer leaves the class flag-set but data-empty for subsequent reads. Generated with the assistance of an AI coding tool. --- .../bonsai/bim/module/drawing/gizmos.py | 2 + src/bonsai/bonsai/bim/module/geometry/data.py | 6 +- .../bonsai/bim/module/model/__init__.py | 2 + src/bonsai/bonsai/bim/module/model/array.py | 126 ++++++++++++++++++ .../bonsai/bim/module/model/decorator.py | 2 +- src/bonsai/bonsai/bim/module/model/door.py | 8 +- src/bonsai/bonsai/bim/module/model/stair.py | 6 +- src/bonsai/bonsai/bim/module/model/wall.py | 2 +- src/bonsai/bonsai/bim/module/model/window.py | 6 +- src/bonsai/bonsai/bim/parametric_lifecycle.py | 18 ++- 10 files changed, 156 insertions(+), 22 deletions(-) diff --git a/src/bonsai/bonsai/bim/module/drawing/gizmos.py b/src/bonsai/bonsai/bim/module/drawing/gizmos.py index 55096bc6a0..519451d47b 100644 --- a/src/bonsai/bonsai/bim/module/drawing/gizmos.py +++ b/src/bonsai/bonsai/bim/module/drawing/gizmos.py @@ -5948,6 +5948,8 @@ class BaseParametricGizmoGroup: customize dimension gizmo positioning, and _refresh_element_specific() to re-billboard element-specific gizmos per frame. """ + if not self.is_setup_complete(): + return obj = context.active_object if not obj: return diff --git a/src/bonsai/bonsai/bim/module/geometry/data.py b/src/bonsai/bonsai/bim/module/geometry/data.py index 05344ab87e..481426d25b 100644 --- a/src/bonsai/bonsai/bim/module/geometry/data.py +++ b/src/bonsai/bonsai/bim/module/geometry/data.py @@ -44,8 +44,12 @@ class ViewportData: @classmethod def load(cls): - cls.is_loaded = True + # Populate data BEFORE flipping is_loaded so a raising ``mode()`` + # call doesn't leave the class half-loaded (flag set, dict empty). + # Subsequent items-callback invocations skip load() on a True flag + # and would hit ``cls.data["mode"]`` → KeyError. cls.data = {"mode": cls.mode()} + cls.is_loaded = True @classmethod def mode(cls) -> tool.Blender.BLENDER_ENUM_ITEMS: diff --git a/src/bonsai/bonsai/bim/module/model/__init__.py b/src/bonsai/bonsai/bim/module/model/__init__.py index 26dca1984d..84e4c7f886 100644 --- a/src/bonsai/bonsai/bim/module/model/__init__.py +++ b/src/bonsai/bonsai/bim/module/model/__init__.py @@ -61,6 +61,8 @@ classes = ( array.Input3DCursorXArray, array.Input3DCursorYArray, array.Input3DCursorZArray, + array.EnableEditingParametric, + array.AddArrayFromFeatureEdit, product.AddDefaultType, product.AddEmptyType, product.AddOccurrence, diff --git a/src/bonsai/bonsai/bim/module/model/array.py b/src/bonsai/bonsai/bim/module/model/array.py index dd54bf0ab3..e4bfb8fedd 100644 --- a/src/bonsai/bonsai/bim/module/model/array.py +++ b/src/bonsai/bonsai/bim/module/model/array.py @@ -379,3 +379,129 @@ class Input3DCursorZArray(bpy.types.Operator): else: props.z = cursor.location.z - obj.location.z return {"FINISHED"} + + +class EnableEditingParametric(bpy.types.Operator): + """Pen-icon dispatcher: fires the gizmo group's per-feature edit operator. + + Bound to every parametric gizmo group's pen icon. The gizmo group's own + ``enable_editing_operator`` (``bim.enable_editing_door``, ``…_wall``, …) + is passed as ``feature_enable_op`` at setup time and invoked here. The + indirection lets one gizmo class serve all features without per-feature + subclasses.""" + + bl_idname = "bim.enable_editing_parametric" + bl_label = "Enable Editing" + bl_description = "Edit this object's parameters" + bl_options = {"REGISTER", "UNDO"} + + feature_enable_op: bpy.props.StringProperty( + default="", + description="Operator bl_idname to invoke (e.g., 'bim.enable_editing_door').", + ) + + def execute(self, context): + # Malformed ``feature_enable_op`` (missing dot) would otherwise crash + # the unpack with ValueError; treat the same as the empty-string case. + parts = self.feature_enable_op.split(".", 1) + if len(parts) != 2: + return {"CANCELLED"} + domain, opname = parts + return getattr(getattr(bpy.ops, domain), opname)("INVOKE_DEFAULT") + + +class AddArrayFromFeatureEdit(bpy.types.Operator, tool.Ifc.Operator): + """Commit any in-progress feature edit and add an array with + gizmo-friendly defaults (count=2, offset = bbox extent along the axis). + + Modifier-aware: plain click → X, Shift → Y, Ctrl → Z. Callers can pass + ``axis="X"`` via EXEC_DEFAULT to bypass the modifier read. + + All three chained operators (feature finish + add_array + enable_editing) + run inside one transaction for a single undo step.""" + + bl_idname = "bim.add_array_from_feature_edit" + bl_label = "Add Array" + bl_description = ( + "Click: add an array along X.\n" "Shift+Click: add an array along Y.\n" "Ctrl+Click: add an array along Z" + ) + bl_options = {"REGISTER", "UNDO"} + + axis: bpy.props.EnumProperty( + name="Offset Axis", + items=[ + ("X", "X", "Offset along the object's X axis (bbox X extent)"), + ("Y", "Y", "Offset along the object's Y axis (bbox Y extent)"), + ("Z", "Z", "Offset along the object's Z axis (bbox Z extent)"), + ], + default="X", + ) + + # Minimum offset to use when the object's bbox extent is tiny — prevents + # the second instance from visually overlapping the parent on small + # annotations / openings (0.3m ≈ a clearly-separated next-instance distance). + MIN_DEFAULT_OFFSET = 0.3 + + def invoke(self, context, event): + # Modifier-aware axis pick: X by default, Shift → Y, Ctrl → Z. + if event.shift: + self.axis = "Y" + elif event.ctrl: + self.axis = "Z" + else: + self.axis = "X" + return self.execute(context) + + def _execute(self, context): + obj = context.active_object + if obj is None: + return {"CANCELLED"} + # Commit any in-progress parametric edit lifecycle on this object first — the + # user expects "Add Array" to also finalise whatever they were editing + # so they don't lose their draft changes. + editing = tool.Parametric.is_object_editing(obj, skip_name="array") + if editing is not None: + finish_op_name = editing.finish_op.removeprefix("bim.") + getattr(bpy.ops.bim, finish_op_name)("INVOKE_DEFAULT") + # Bounding-box derived offset along the chosen axis, converted from + # Blender SI (meters) to IFC project units (which is what + # ``BBIM_Array.Data`` stores; the regenerator multiplies by + # unit_scale on the way out). + axis_idx = "XYZ".index(self.axis) + if obj.bound_box: + bbox_extent_si = max(c[axis_idx] for c in obj.bound_box) - min(c[axis_idx] for c in obj.bound_box) + else: + bbox_extent_si = 1.0 + bbox_extent_si = max(bbox_extent_si, self.MIN_DEFAULT_OFFSET) + si_conversion = ifcopenshell.util.unit.calculate_unit_scale(tool.Ifc.get()) + offset_project = bbox_extent_si / si_conversion if si_conversion else bbox_extent_si + add_kwargs = {"count": 2, "x": 0.0, "y": 0.0, "z": 0.0} + add_kwargs[self.axis.lower()] = offset_project + result = bpy.ops.bim.add_array(**add_kwargs) + if result != {"FINISHED"}: + return result + # Restore selection to just the parent. ``regenerate_array`` calls + # ``tool.Geometry.duplicate_ifc_objects`` which leaves the newly-created + # child selected alongside the parent. The edit-lifecycle gizmos poll on a + # single-selected parent, so with both selected the gizmos wouldn't + # surface and "ARRAY → enter edit" would feel broken. + tool.Blender.select_and_activate_single_object(context, active_object=obj) + # Chain straight into array edit for the newly-added layer (always the + # last entry in the pset's Data list, by AddArray's append semantics). + # The user's expectation after clicking ARRAY is "I want to tweak this + # array now" — entering edit mode immediately collapses the 2-click + # discover-then-edit flow into one. + element = tool.Ifc.get_entity(obj) + if element is None: + return {"FINISHED"} + data_text = ifcopenshell.util.element.get_pset(element, "BBIM_Array", "Data") + if not data_text: + return {"FINISHED"} + try: + layers = json.loads(data_text) + except (ValueError, TypeError): + return {"FINISHED"} + if not layers: + return {"FINISHED"} + bpy.ops.bim.enable_editing_array("INVOKE_DEFAULT", item=len(layers) - 1) + return {"FINISHED"} diff --git a/src/bonsai/bonsai/bim/module/model/decorator.py b/src/bonsai/bonsai/bim/module/model/decorator.py index 49090e2c9f..149d91b68b 100644 --- a/src/bonsai/bonsai/bim/module/model/decorator.py +++ b/src/bonsai/bonsai/bim/module/model/decorator.py @@ -108,7 +108,7 @@ class ProfileDecorator: obj = context.active_object - if obj.mode != "EDIT": + if obj is None or obj.mode != "EDIT": if exit_edit_mode_callback: ProfileDecorator.uninstall() exit_edit_mode_callback() diff --git a/src/bonsai/bonsai/bim/module/model/door.py b/src/bonsai/bonsai/bim/module/model/door.py index d6a619f429..6ccdf23c97 100644 --- a/src/bonsai/bonsai/bim/module/model/door.py +++ b/src/bonsai/bonsai/bim/module/model/door.py @@ -707,8 +707,8 @@ class CycleDoorType(bpy.types.Operator, tool.Ifc.Operator, gizmo.CycleTypeMixin) bl_label = "Cycle Door Type" bl_options = {"REGISTER", "UNDO"} - element_checker = "is_door" - props_getter = "get_door_props" + element_checker = tool.Parametric.is_door + props_getter = tool.Model.get_door_props type_literal = tool.Model.DoorType type_attr = "door_type" @@ -835,7 +835,7 @@ class GizmoDoorEdition(bpy.types.GizmoGroup, gizmo.BaseParametricGizmoGroup): ), ] - props_getter = "get_door_props" + props_getter = tool.Model.get_door_props gizmo_pref_name = "door" @classmethod @@ -866,13 +866,11 @@ class GizmoDoorEdition(bpy.types.GizmoGroup, gizmo.BaseParametricGizmoGroup): self.gizmo_door_type = self.create_arc_gizmo( special_color, "bim.toggle_door_swing", - prop_path="BIMDoorProperties.door_type", flip_geometry=False, ) self.gizmo_flip_arc = self.create_arc_gizmo( inactive_color, "bim.toggle_door_swing", - prop_path="BIMDoorProperties.door_type", flip_geometry=True, flip_local_axes="XY", ) diff --git a/src/bonsai/bonsai/bim/module/model/stair.py b/src/bonsai/bonsai/bim/module/model/stair.py index 0834263552..ef765ba53c 100644 --- a/src/bonsai/bonsai/bim/module/model/stair.py +++ b/src/bonsai/bonsai/bim/module/model/stair.py @@ -430,7 +430,7 @@ class CycleStairType(bpy.types.Operator, gizmo.CycleTypeMixin): bl_label = "Cycle Stair Type" bl_options = {"REGISTER", "UNDO"} - props_getter = "get_stair_props" + props_getter = tool.Model.get_stair_props type_literal = tool.Model.StairType type_attr = "stair_type" skip_element_check = True @@ -580,7 +580,7 @@ class GizmoStairEdition(bpy.types.GizmoGroup, gizmo.BaseParametricGizmoGroup): ] # Metadata-driven dispatch for props and preferences - props_getter = "get_stair_props" + props_getter = tool.Model.get_stair_props gizmo_pref_name = "stair" @classmethod @@ -593,14 +593,12 @@ class GizmoStairEdition(bpy.types.GizmoGroup, gizmo.BaseParametricGizmoGroup): "VIEW3D_GT_lock", self.COLOR_BLUE, "bim.toggle_stair_property", - prop_path="BIMStairProperties.total_length_lock", property_name="total_length_lock", ) self.tread_lock_gizmo = self.create_icon_gizmo( "VIEW3D_GT_lock", (1.0, 1.0, 1.0), "bim.toggle_stair_property", - prop_path="BIMStairProperties.custom_tread_lock", property_name="custom_tread_lock", ) self.plus_gizmo = self.create_icon_gizmo( diff --git a/src/bonsai/bonsai/bim/module/model/wall.py b/src/bonsai/bonsai/bim/module/model/wall.py index 0bedb86fc6..104e521082 100644 --- a/src/bonsai/bonsai/bim/module/model/wall.py +++ b/src/bonsai/bonsai/bim/module/model/wall.py @@ -1829,7 +1829,7 @@ class GizmoWallEdition(bpy.types.GizmoGroup, gizmo.BaseParametricGizmoGroup): ), ] - props_getter = "get_wall_props" + props_getter = tool.Model.get_wall_props gizmo_pref_name = "wall" @classmethod diff --git a/src/bonsai/bonsai/bim/module/model/window.py b/src/bonsai/bonsai/bim/module/model/window.py index 2432549661..a14f3322c4 100644 --- a/src/bonsai/bonsai/bim/module/model/window.py +++ b/src/bonsai/bonsai/bim/module/model/window.py @@ -558,8 +558,8 @@ class CycleWindowType(bpy.types.Operator, tool.Ifc.Operator, gizmo.CycleTypeMixi bl_label = "Cycle Window Type" bl_options = {"REGISTER", "UNDO"} - element_checker = "is_window" - props_getter = "get_window_props" + element_checker = tool.Parametric.is_window + props_getter = tool.Model.get_window_props type_literal = tool.Model.WindowType type_attr = "window_type" @@ -745,7 +745,7 @@ class GizmoWindowEdition(bpy.types.GizmoGroup, gizmo.BaseParametricGizmoGroup): DimensionGizmoConfig(attr_name="lining_offset", axis=(0, 1, 0), min_value=-10.0), ] - props_getter = "get_window_props" + props_getter = tool.Model.get_window_props gizmo_pref_name = "window" @classmethod diff --git a/src/bonsai/bonsai/bim/parametric_lifecycle.py b/src/bonsai/bonsai/bim/parametric_lifecycle.py index 6fa74ab81d..b247e3e4bc 100644 --- a/src/bonsai/bonsai/bim/parametric_lifecycle.py +++ b/src/bonsai/bonsai/bim/parametric_lifecycle.py @@ -71,18 +71,16 @@ from __future__ import annotations import json from collections.abc import Callable -from typing import TYPE_CHECKING, ClassVar, get_args +from typing import ClassVar, get_args import bpy import ifcopenshell.util.element from bpy.app.handlers import persistent +from ifcopenshell import entity_instance import bonsai.core.geometry import bonsai.tool as tool -if TYPE_CHECKING: - from ifcopenshell import entity_instance - class ParametricEditMixinBase: """Common scaffolding for parametric edit-lifecycle mixins. @@ -500,9 +498,15 @@ class PickTypeMixin(TypeAccessorBase): op.value = v context.window_manager.popup_menu(draw, title=self.bl_label, icon="MENU_PANEL") - # INTERFACE (not FINISHED) keeps the menu-opening invocation out of the - # undo stack; the picked-value write below returns FINISHED, so the - # type change remains undoable as a single step. + # The type change is a two-step interaction: this invocation just OPENS + # the menu (no state change yet); a SECOND invocation fires when the + # user clicks a menu item — that one writes ``props.`` and + # returns FINISHED. By returning INTERFACE here (and not FINISHED), the + # menu-open step is excluded from Blender's undo stack so the user + # gets exactly ONE undo entry per type change. If we returned FINISHED + # here too, the stack would gain a no-op "opened the menu" entry that + # Ctrl+Z would dismiss before reverting the actual type change — + # confusing UX where the first Ctrl+Z appears to do nothing. return {"INTERFACE"} def _pick_type(self, context: bpy.types.Context) -> set[str]: