From 8f3a1d741298bd622448bb951fb704d1d544627d Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Wed, 8 Jul 2026 13:13:14 +0200 Subject: [PATCH 1/5] Bonsai: batch array-duplicate + defensive guards Replace N sequential duplicate_ifc_objects([parent]) calls in _regenerate_array_body with one duplicate_ifc_object_n_times call per layer, batching the fixed per-call overhead (snapshot gather, UI refresh, decorator reload). Guard batch_host_recut drain against dead StructRNA refs and prune orphan array-child GUIDs at regen so outliner-delete of a Bonsai-managed child cannot crash subsequent regenerate_array. Recalculate walls after recreate_connections so Shift+D of connected walls produces correct junction geometry without a manual regen step. Relates to #8088. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/tool/geometry.py | 316 ++++++++++----- src/bonsai/bonsai/tool/model.py | 54 ++- .../model/test_array_duplicate_batched.py | 376 ++++++++++++++++++ .../tool/test_geometry_batch_host_recut.py | 56 +++ 4 files changed, 698 insertions(+), 104 deletions(-) create mode 100644 src/bonsai/test/bim/module/model/test_array_duplicate_batched.py diff --git a/src/bonsai/bonsai/tool/geometry.py b/src/bonsai/bonsai/tool/geometry.py index 6c195ecbab..c4b7cd0ef6 100644 --- a/src/bonsai/bonsai/tool/geometry.py +++ b/src/bonsai/bonsai/tool/geometry.py @@ -163,13 +163,21 @@ class Geometry(bonsai.core.tool.Geometry): cls._host_update_queue = {} cls._host_recut_queue = {} for voided_obj in update_queue.values(): - if not voided_obj or not voided_obj.data: + try: + if not voided_obj or not voided_obj.data: + continue + except ReferenceError: + # Blender object was deleted while the batch was open + # (e.g. user removed it via the outliner mid-op). continue if tool.Ifc.get_entity(voided_obj) is None: continue bpy.ops.bim.update_representation(obj=voided_obj.name) for voided_obj, _ in recut_queue.values(): - if not voided_obj or not voided_obj.data: + try: + if not voided_obj or not voided_obj.data: + continue + except ReferenceError: continue if tool.Ifc.get_entity(voided_obj) is None: continue @@ -2481,99 +2489,16 @@ class Geometry(bonsai.core.tool.Geometry): old_obj_name_to_new_obj_name: dict[str, str] = {} for obj in objects_to_duplicate: - element = tool.Ifc.get_entity(obj) - if element: - if element.is_a("IfcAnnotation") and element.ObjectType == "DRAWING": - tool.Blender.deselect_object(obj) - continue # For now, don't copy drawings until we stabilise a bit more. It's tricky. - elif tool.Geometry.is_locked(element): - tool.Blender.deselect_object(obj) - continue - elif tool.Geometry.is_representation_item(obj): - cls.duplicate_ifc_item(obj) - continue - - tracked_opening_type = tool.Model.get_tracked_opening_type(obj) - is_tracked_opening = bool(tracked_opening_type) - keep_data_linked = linked and not element and not is_tracked_opening - - # Prior to duplicating, sync the object placement to make decomposition recreation more stable. - cls.commit_placement_if_moved(obj, apply_scale=False) - - new_obj = obj.copy() - temp_data = None - - # Currently for optimization we do not apply pending changes (scale or changed .data) - # to the original and duplicated objects. - # Keep new object edited if original is. - if tool.Ifc.is_edited(obj, ignore_scale=True): - tool.Ifc.edit(new_obj) - - if obj.data and not keep_data_linked: - # assure root.copy_class won't replace the previous mesh globally - temp_data = obj.data.copy() - new_obj.data = temp_data - - # Unlink from previous boolean element - # and keep object tracked for decorations. - if is_tracked_opening: - mprops = tool.Geometry.get_mesh_props(new_obj.data) - mprops.ifc_boolean_id = 0 - tool.Root.add_tracked_opening(new_obj, tracked_opening_type) - - if obj == active_object: - new_active_obj = new_obj - for collection in obj.users_collection: - collection.objects.link(new_obj) - obj.select_set(False) - new_obj.select_set(True) - old_obj_name_to_new_obj_name[obj.name] = new_obj.name - - if not element: - continue - - # clear object's collection so it will be able to have it's own - tool.Blender.get_object_bim_props(new_obj).collection = None - # copy the actual class - new = bonsai.core.root.copy_class(tool.Ifc, tool.Collector, tool.Geometry, tool.Root, obj=new_obj) - - # clean up the orphaned mesh with ifc id of the original object to avoid confusion - # IfcGridAxis keeps the same mesh data (it's pointing to ifc id 0, so it's not a problem) - if new and temp_data and not new.is_a("IfcGridAxis"): - if new.is_a("IfcRelSpaceBoundary"): - surface = new.ConnectionGeometry.SurfaceOnRelatingElement - temp_data.name = f"0/{surface.id()}" - tool.Ifc.link(surface, temp_data) - else: - tool.Blender.remove_data_block(temp_data) - - if new: - # TODO: handle array data for other cases of duplication - array_data = arrays_to_duplicate.get(obj, None) - tool.Model.handle_array_on_copied_element(new, array_data) - if array_data: - for child in tool.Array.get_all_children_objects(new): - child.select_set(True) - - # TODO: add new array children to recreate their decomposition too - old_to_new[element] = [new] - if new.is_a("IfcRelSpaceBoundary"): - tool.Boundary.decorate_boundary(new_obj) - # Slab-trim booleans (from extend_walls_to_underside) belong to - # the source wall's connection, not the copy. Strip them so the - # duplicate reverts to its pre-clip extrusion — mirrors the way - # filling rels are dropped while manual booleans persist on copy. - # Reload the body when something was stripped so the viewport - # immediately shows the unclipped geometry; otherwise the user - # sees a stale mesh until they Shift+G, which is easy to miss. - if new.is_a("IfcWall"): - if tool.Model.strip_underside_booleans(new): - tool.Model.reload_body_representation(new_obj) - # HasOpenings rels don't follow object duplication, so - # the duplicate's body must rebuild to match its current - # opening set. - else: - tool.Model.regenerate_wall(new_obj) + new_active = cls._duplicate_ifc_object_once( + obj, + active_object, + linked, + arrays_to_duplicate, + old_to_new, + old_obj_name_to_new_obj_name, + ) + if new_active is not None: + new_active_obj = new_active # Remap Blender parent relationships for duplicated objects for old_obj_name, new_obj_name in old_obj_name_to_new_obj_name.items(): @@ -2601,10 +2526,211 @@ class Geometry(bonsai.core.tool.Geometry): # Recreate decompositions tool.Duplicate.recreate_decompositions(decomposition_relationships, old_to_new) cls.remove_linked_aggregate_data(old_to_new) + + # In-loop regenerate_wall runs before recreate_connections, so any new + # walls that just received an IfcRelConnectsPathElements have stale + # junction geometry — recalculate them now that their connection graph + # is complete. + cls._recalculate_walls_with_new_connections(old_to_new) + bonsai.bim.handler.refresh_ui_data() tool.Root.reload_grid_decorator() return old_to_new, new_active_obj or active_object + @classmethod + def duplicate_ifc_object_n_times( + cls, source: bpy.types.Object, count: int + ) -> dict[ifcopenshell.entity_instance, list[ifcopenshell.entity_instance]]: + """N-way duplicate of a single source. + + Same per-copy semantics as duplicate_ifc_objects (IFC class copy, + decomposition + connection recreation, body regen for walls), but + bypasses the set() dedupe and the arrays_to_duplicate pre-scan so + callers building a fresh array don't pay per-call overhead N times. + Returns the same old_to_new dict shape, with the source element + mapping to the N new entities.""" + if count <= 0: + return {} + + sources = {source} + decomposition_relationships = tool.Duplicate.get_decomposition_relationships(sources) + connection_relationships = tool.Duplicate.get_connection_relationships(sources) + port_connection_snapshot = tool.Duplicate.get_port_connection_relationships(sources) + old_to_new: dict[ifcopenshell.entity_instance, list[ifcopenshell.entity_instance]] = {} + old_obj_name_to_new_obj_name: dict[str, str] = {} + + for _ in range(count): + cls._duplicate_ifc_object_once( + source, + None, + False, + {}, + old_to_new, + old_obj_name_to_new_obj_name, + keep_source_selected=True, + ) + + for old_obj_name, new_obj_name in old_obj_name_to_new_obj_name.items(): + new_obj = bpy.data.objects.get(new_obj_name) + if new_obj and new_obj.parent and new_obj.parent.name in old_obj_name_to_new_obj_name: + world_matrix = new_obj.matrix_world.copy() + new_parent_name = old_obj_name_to_new_obj_name[new_obj.parent.name] + new_parent = bpy.data.objects.get(new_parent_name) + if new_parent: + new_obj.parent = new_parent + new_obj.matrix_world = world_matrix + + for old in old_to_new.keys(): + if old.is_a("IfcElementAssembly"): + tool.Root.recreate_aggregate(old_to_new) + + cls.remove_old_connections(old_to_new) + tool.Duplicate.recreate_connections(connection_relationships, old_to_new) + tool.Duplicate.recreate_port_connections(port_connection_snapshot, old_to_new) + tool.Duplicate.recreate_decompositions(decomposition_relationships, old_to_new) + cls.remove_linked_aggregate_data(old_to_new) + cls._recalculate_walls_with_new_connections(old_to_new) + bonsai.bim.handler.refresh_ui_data() + tool.Root.reload_grid_decorator() + return old_to_new + + @classmethod + def _duplicate_ifc_object_once( + cls, + obj: bpy.types.Object, + active_object: Optional[bpy.types.Object], + linked: bool, + arrays_to_duplicate: dict[bpy.types.Object, Any], + old_to_new: dict[ifcopenshell.entity_instance, list[ifcopenshell.entity_instance]], + old_obj_name_to_new_obj_name: dict[str, str], + keep_source_selected: bool = False, + ) -> Optional[bpy.types.Object]: + """Per-source body of the duplicate flow. Mutates old_to_new and + old_obj_name_to_new_obj_name in place. Returns new_obj when obj is + the active_object, else None. + + keep_source_selected: when True, skip the source deselect so batched + callers can run N iterations without N×2 select flips and without + needing a post-loop restore on the source.""" + new_active_obj: Optional[bpy.types.Object] = None + element = tool.Ifc.get_entity(obj) + if element: + if element.is_a("IfcAnnotation") and element.ObjectType == "DRAWING": + tool.Blender.deselect_object(obj) + return None # For now, don't copy drawings until we stabilise a bit more. It's tricky. + elif tool.Geometry.is_locked(element): + tool.Blender.deselect_object(obj) + return None + elif tool.Geometry.is_representation_item(obj): + cls.duplicate_ifc_item(obj) + return None + + tracked_opening_type = tool.Model.get_tracked_opening_type(obj) + is_tracked_opening = bool(tracked_opening_type) + keep_data_linked = linked and not element and not is_tracked_opening + + # Prior to duplicating, sync the object placement to make decomposition recreation more stable. + cls.commit_placement_if_moved(obj, apply_scale=False) + + new_obj = obj.copy() + temp_data = None + + # Currently for optimization we do not apply pending changes (scale or changed .data) + # to the original and duplicated objects. + # Keep new object edited if original is. + if tool.Ifc.is_edited(obj, ignore_scale=True): + tool.Ifc.edit(new_obj) + + if obj.data and not keep_data_linked: + # assure root.copy_class won't replace the previous mesh globally + temp_data = obj.data.copy() + new_obj.data = temp_data + + # Unlink from previous boolean element + # and keep object tracked for decorations. + if is_tracked_opening: + mprops = tool.Geometry.get_mesh_props(new_obj.data) + mprops.ifc_boolean_id = 0 + tool.Root.add_tracked_opening(new_obj, tracked_opening_type) + + if obj == active_object: + new_active_obj = new_obj + for collection in obj.users_collection: + collection.objects.link(new_obj) + if not keep_source_selected: + obj.select_set(False) + new_obj.select_set(True) + old_obj_name_to_new_obj_name[obj.name] = new_obj.name + + if not element: + return new_active_obj + + # clear object's collection so it will be able to have it's own + tool.Blender.get_object_bim_props(new_obj).collection = None + # copy the actual class + new = bonsai.core.root.copy_class(tool.Ifc, tool.Collector, tool.Geometry, tool.Root, obj=new_obj) + + # clean up the orphaned mesh with ifc id of the original object to avoid confusion + # IfcGridAxis keeps the same mesh data (it's pointing to ifc id 0, so it's not a problem) + if new and temp_data and not new.is_a("IfcGridAxis"): + if new.is_a("IfcRelSpaceBoundary"): + surface = new.ConnectionGeometry.SurfaceOnRelatingElement + temp_data.name = f"0/{surface.id()}" + tool.Ifc.link(surface, temp_data) + else: + tool.Blender.remove_data_block(temp_data) + + if new: + # TODO: handle array data for other cases of duplication + array_data = arrays_to_duplicate.get(obj, None) + tool.Model.handle_array_on_copied_element(new, array_data) + if array_data: + for child in tool.Array.get_all_children_objects(new): + child.select_set(True) + + # TODO: add new array children to recreate their decomposition too + old_to_new.setdefault(element, []).append(new) + if new.is_a("IfcRelSpaceBoundary"): + tool.Boundary.decorate_boundary(new_obj) + # Slab-trim booleans (from extend_walls_to_underside) belong to + # the source wall's connection, not the copy. Strip them so the + # duplicate reverts to its pre-clip extrusion — mirrors the way + # filling rels are dropped while manual booleans persist on copy. + # Reload the body when something was stripped so the viewport + # immediately shows the unclipped geometry; otherwise the user + # sees a stale mesh until they Shift+G, which is easy to miss. + if new.is_a("IfcWall"): + if tool.Model.strip_underside_booleans(new): + tool.Model.reload_body_representation(new_obj) + # HasOpenings rels don't follow object duplication, so + # the duplicate's body must rebuild to match its current + # opening set. + else: + tool.Model.regenerate_wall(new_obj) + + return new_active_obj + + @classmethod + def _recalculate_walls_with_new_connections( + cls, old_to_new: dict[ifcopenshell.entity_instance, list[ifcopenshell.entity_instance]] + ) -> None: + """Recalculate new IfcWall duplicates that just received an + ``IfcRelConnectsPathElements``. The in-loop ``regenerate_wall`` runs + before ``recreate_connections``, so wall body geometry doesn't reflect + the junction until this second pass.""" + walls_to_recalc: list[bpy.types.Object] = [] + for new_list in old_to_new.values(): + for new_entity in new_list: + if not new_entity.is_a("IfcWall"): + continue + if not (getattr(new_entity, "ConnectedTo", None) or getattr(new_entity, "ConnectedFrom", None)): + continue + new_obj = tool.Ifc.get_object(new_entity) + if new_obj is not None: + walls_to_recalc.append(new_obj) + if walls_to_recalc: + tool.Model.recalculate_walls(walls_to_recalc) + @classmethod def duplicate_ifc_item(cls, obj: bpy.types.Object) -> None: props = tool.Geometry.get_geometry_props() diff --git a/src/bonsai/bonsai/tool/model.py b/src/bonsai/bonsai/tool/model.py index dee8c218f2..f20edae00d 100644 --- a/src/bonsai/bonsai/tool/model.py +++ b/src/bonsai/bonsai/tool/model.py @@ -1247,6 +1247,35 @@ class Model(bonsai.core.tool.Model): with tool.Geometry.batch_host_recut(): cls._regenerate_array_body(parent_obj, data, array_layers_to_apply) + @classmethod + def _prune_orphan_array_children(cls, array: dict[str, Any]) -> None: + """Drop GUIDs from ``array['children']`` whose IFC entity or Blender + object is no longer alive, and cascade-remove the orphan IFC entity + if it still exists. Outliner / keyboard delete of a Bonsai-managed + object bypasses ``bim.delete``'s cascade, leaving dangling opening + and filling references that later confuse regen and crash the + ``batch_host_recut`` drain.""" + live_guids: list[str] = [] + ifc_file = tool.Ifc.get() + for guid in array["children"]: + try: + element = ifc_file.by_guid(guid) + except RuntimeError: + continue + obj = tool.Ifc.get_object(element) + try: + is_live = obj is not None and obj.data is not None + except ReferenceError: + is_live = False + if is_live: + live_guids.append(guid) + continue + try: + ifcopenshell.api.root.remove_product(ifc_file, product=element) + except (RuntimeError, ifcopenshell.Error): + pass + array["children"] = live_guids + @classmethod def _regenerate_array_body( cls, parent_obj: bpy.types.Object, data: list[dict[str, Any]], array_layers_to_apply: Iterable[int] @@ -1262,6 +1291,7 @@ class Model(bonsai.core.tool.Model): obj_stack = [parent_obj] for array_i, array in enumerate(data): + cls._prune_orphan_array_children(array) child_i = 0 existing_children = set(array["children"]) total_existing_children = len(array["children"]) @@ -1275,6 +1305,14 @@ class Model(bonsai.core.tool.Model): else: base_offset = Vector([array["x"], array["y"], array["z"]]) * unit_scale + target_new_in_this_layer = (array["count"] - 1) * len(obj_stack) + missing_count = max(0, target_new_in_this_layer - total_existing_children) + new_entities_pool: list[ifcopenshell.entity_instance] = [] + if missing_count > 0: + batch_old_to_new = tool.Geometry.duplicate_ifc_object_n_times(parent_obj, missing_count) + new_entities_pool = batch_old_to_new.get(parent_element, []) + new_entities_iter = iter(new_entities_pool) + for i in range(array["count"]): if i == 0: continue @@ -1292,8 +1330,13 @@ class Model(bonsai.core.tool.Model): child_obj = tool.Ifc.get_object(child_element) assert child_obj except (IndexError, RuntimeError, AssertionError): - old_to_new, _ = tool.Geometry.duplicate_ifc_objects([parent_obj]) - child_element = next(iter(old_to_new.values()))[0] + try: + child_element = next(new_entities_iter) + except StopIteration: + # Stale-GUID mid-list left the pool exhausted; fall back + # to a one-off duplicate so the layer can still complete. + old_to_new, _ = tool.Geometry.duplicate_ifc_objects([parent_obj]) + child_element = next(iter(old_to_new.values()))[0] child_obj = tool.Ifc.get_object(child_element) # add child pset @@ -1361,13 +1404,6 @@ class Model(bonsai.core.tool.Model): tool.Ifc.get(), pset=pset, properties={"Data": json_data, "Parent": parent_element.GlobalId} ) - # Post-condition: parent is selected on return. duplicate_ifc_objects - # deselects the source on every call inside the regen loop; without - # this restore, callers get a deselected parent for arrays with N >= 2. - # TODO: batch the per-child duplicate_ifc_objects([parent]) calls into - # a single N-way duplicate — N depsgraph churns + N select/deselect - # flips is wasteful, and a batched duplicate would also remove the - # need for this restore. parent_obj.select_set(True) @classmethod diff --git a/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py b/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py new file mode 100644 index 0000000000..285eccab39 --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py @@ -0,0 +1,376 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Tests for the batched array-duplicate path. + +`tool.Geometry.duplicate_ifc_object_n_times` lifts the per-call overhead of +`duplicate_ifc_objects` (snapshot, UI refresh, decorator reload, select +flips) out of the per-child loop in `_regenerate_array_body`. These tests +pin three contracts: + +1. N-way batched duplicate produces N distinct entities mapped from the + source under `old_to_new[source_element]`, and the source object stays + selected throughout (no per-iteration deselect). +2. Per-layer batching collapses the N independent UI refreshes into one. +3. End-to-end array regen still yields the same number and shape of + children as the per-call baseline.""" + +import json +from unittest.mock import patch + +import bpy +import ifcopenshell +import pytest + +import bonsai.tool as tool +from test.bim.bootstrap import NewFile + +pytestmark = pytest.mark.model + + +def _build_actuator(name: str = "Actuator") -> tuple[bpy.types.Object, ifcopenshell.entity_instance]: + """Minimal IfcActuator + cube — matches the test_array_batch_recut.py shape.""" + bpy.ops.bim.create_project() + bpy.ops.mesh.primitive_cube_add() + obj = bpy.context.active_object + obj.name = name + rprops = tool.Root.get_root_props() + rprops.ifc_product = "IfcElement" + bpy.ops.bim.assign_class(ifc_class="IfcActuator", predefined_type="ELECTRICACTUATOR", userdefined_type="") + element = tool.Ifc.get_entity(obj) + return obj, element + + +def _build_actuator_with_array_pset( + count: int, x: float = 1.0 +) -> tuple[bpy.types.Object, ifcopenshell.entity_instance, list[dict]]: + obj, element = _build_actuator() + parent_data = [ + { + "children": [], + "count": count, + "method": "OFFSET", + "x": x, + "y": 0.0, + "z": 0.0, + "use_local_space": False, + "sync_children": False, + } + ] + pset = ifcopenshell.api.pset.add_pset(tool.Ifc.get(), product=element, name="BBIM_Array") + ifcopenshell.api.pset.edit_pset( + tool.Ifc.get(), + pset=pset, + properties={"Data": json.dumps(parent_data), "Parent": element.GlobalId}, + ) + return obj, element, parent_data + + +class TestDuplicateIfcObjectNTimes(NewFile): + def test_returns_empty_dict_for_zero_count(self): + obj, _ = _build_actuator() + result = tool.Geometry.duplicate_ifc_object_n_times(obj, 0) + assert result == {} + + def test_returns_empty_dict_for_negative_count(self): + obj, _ = _build_actuator() + result = tool.Geometry.duplicate_ifc_object_n_times(obj, -3) + assert result == {} + + def test_produces_n_distinct_entities(self): + obj, element = _build_actuator() + result = tool.Geometry.duplicate_ifc_object_n_times(obj, 5) + new_entities = result.get(element) + assert new_entities is not None + assert len(new_entities) == 5 + assert len({e.id() for e in new_entities}) == 5 + for new_entity in new_entities: + assert new_entity.is_a("IfcActuator") + assert new_entity.GlobalId != element.GlobalId + + def test_source_stays_selected_after_batch(self): + obj, _ = _build_actuator() + obj.select_set(True) + tool.Geometry.duplicate_ifc_object_n_times(obj, 4) + assert obj in bpy.context.selected_objects, "source object must remain selected across batched duplicates" + + def test_each_new_entity_has_blender_object(self): + obj, element = _build_actuator() + result = tool.Geometry.duplicate_ifc_object_n_times(obj, 3) + for new_entity in result[element]: + new_obj = tool.Ifc.get_object(new_entity) + assert new_obj is not None + assert new_obj is not obj + + +class TestBatchedRefreshUIDataCallCount(NewFile): + def test_n_times_calls_refresh_ui_data_once(self): + obj, _ = _build_actuator() + with patch("bonsai.bim.handler.refresh_ui_data") as refresh_mock: + tool.Geometry.duplicate_ifc_object_n_times(obj, 8) + assert ( + refresh_mock.call_count == 1 + ), f"batched 8-way duplicate must call refresh_ui_data once, got {refresh_mock.call_count}" + + def test_n_times_calls_reload_grid_decorator_once(self): + obj, _ = _build_actuator() + with patch.object(tool.Root, "reload_grid_decorator") as reload_mock: + tool.Geometry.duplicate_ifc_object_n_times(obj, 8) + assert reload_mock.call_count == 1 + + +class TestRegenerateArrayEndToEnd(NewFile): + def test_regenerate_array_creates_expected_children(self): + obj, element, parent_data = _build_actuator_with_array_pset(count=8) + bpy.context.view_layer.objects.active = obj + tool.Model.regenerate_array(obj, parent_data) + + layer = parent_data[0] + assert len(layer["children"]) == 7, "8-element array means 7 new children (parent + 7)" + for child_guid in layer["children"]: + child_element = tool.Ifc.get().by_guid(child_guid) + assert child_element is not None + assert child_element.is_a("IfcActuator") + child_pset = ifcopenshell.util.element.get_pset(child_element, "BBIM_Array") + assert child_pset is not None + assert child_pset["Parent"] == element.GlobalId + + def test_regenerate_array_parent_stays_selected(self): + obj, element, parent_data = _build_actuator_with_array_pset(count=4) + bpy.context.view_layer.objects.active = obj + obj.select_set(True) + tool.Model.regenerate_array(obj, parent_data) + assert ( + obj in bpy.context.selected_objects + ), "regenerate_array must leave parent_obj selected on return (post-condition)" + + def test_regen_operator_leaves_only_parent_selected_and_active(self): + """Post-condition parity between grow and shrink for the user-facing + ``bim.regenerate_array`` operator: only the parent is selected + active; + every child is deselected. Pre-fix the grow path left new children + selected, creating inconsistency with the shrink path. + + Scoped to the operator, not the tool method — ``remove_array`` and + ``apply_array`` also invoke ``tool.Model.regenerate_array`` internally + but expect a different post-selection state (children stay selected + for user follow-up work).""" + obj, element, parent_data = _build_actuator_with_array_pset(count=6) + bpy.context.view_layer.objects.active = obj + obj.select_set(True) + bpy.ops.bim.regenerate_array() + + assert obj in bpy.context.selected_objects + assert bpy.context.view_layer.objects.active is obj + parent_pset = ifcopenshell.util.element.get_pset(element, "BBIM_Array") + parent_data_after = json.loads(parent_pset["Data"]) + for child_guid in parent_data_after[0]["children"]: + child_element = tool.Ifc.get().by_guid(child_guid) + child_obj = tool.Ifc.get_object(child_element) + assert ( + child_obj not in bpy.context.selected_objects + ), f"child {child_obj.name} must be deselected on regenerate_array return" + + def test_regen_operator_after_shrink_still_leaves_only_parent_selected(self): + obj, element, parent_data = _build_actuator_with_array_pset(count=6) + bpy.context.view_layer.objects.active = obj + bpy.ops.bim.regenerate_array() + + parent_pset = ifcopenshell.util.element.get_pset(element, "BBIM_Array") + arrays = json.loads(parent_pset["Data"]) + arrays[0]["count"] = 3 + pset_entity = tool.Ifc.get().by_id(parent_pset["id"]) + ifcopenshell.api.pset.edit_pset(tool.Ifc.get(), pset=pset_entity, properties={"Data": json.dumps(arrays)}) + bpy.ops.bim.regenerate_array() + + assert obj in bpy.context.selected_objects + assert bpy.context.view_layer.objects.active is obj + parent_pset = ifcopenshell.util.element.get_pset(element, "BBIM_Array") + arrays_after = json.loads(parent_pset["Data"]) + for child_guid in arrays_after[0]["children"]: + child_element = tool.Ifc.get().by_guid(child_guid) + child_obj = tool.Ifc.get_object(child_element) + assert child_obj not in bpy.context.selected_objects + + def test_regenerate_array_child_positions_match_offset(self): + obj, element, parent_data = _build_actuator_with_array_pset(count=4, x=2.5) + bpy.context.view_layer.objects.active = obj + parent_x = obj.matrix_world.translation.x + tool.Model.regenerate_array(obj, parent_data) + + layer = parent_data[0] + for i, child_guid in enumerate(layer["children"], start=1): + child_element = tool.Ifc.get().by_guid(child_guid) + child_obj = tool.Ifc.get_object(child_element) + expected_x = parent_x + 2.5 * i + assert child_obj.matrix_world.translation.x == pytest.approx( + expected_x + ), f"child {i}: expected x≈{expected_x}, got {child_obj.matrix_world.translation.x}" + + +class TestRegenerateArrayUIRefreshCoalesces(NewFile): + def test_n_children_grow_calls_refresh_ui_data_once_per_layer(self): + obj, element, parent_data = _build_actuator_with_array_pset(count=8) + bpy.context.view_layer.objects.active = obj + with patch("bonsai.bim.handler.refresh_ui_data") as refresh_mock: + tool.Model.regenerate_array(obj, parent_data) + assert refresh_mock.call_count == 1, ( + "growing an array layer from 0 to 7 children must call refresh_ui_data once, " + f"got {refresh_mock.call_count}" + ) + + def test_n_children_grow_calls_reload_grid_decorator_once_per_layer(self): + obj, element, parent_data = _build_actuator_with_array_pset(count=8) + bpy.context.view_layer.objects.active = obj + with patch.object(tool.Root, "reload_grid_decorator") as reload_mock: + tool.Model.regenerate_array(obj, parent_data) + assert reload_mock.call_count == 1 + + +class TestRecalculateWallsWithNewConnections(NewFile): + """Pins the post-connection wall recalc: after ``recreate_connections`` + wires new IfcRelConnectsPathElements onto duplicated walls, the wall + bodies must be re-recalculated because the in-loop ``regenerate_wall`` + fired before the connections existed. Otherwise the junction geometry + stays stale and the user has to manually regen.""" + + def test_walls_with_new_connections_are_recalculated(self): + from unittest.mock import Mock + + wall_new = Mock() + wall_new.is_a = lambda c: c == "IfcWall" + wall_new.ConnectedTo = [Mock()] + wall_new.ConnectedFrom = [] + + wall_obj = Mock() + old_to_new = {Mock(): [wall_new]} + + with patch.object(tool.Ifc, "get_object", return_value=wall_obj), patch.object( + tool.Model, "recalculate_walls" + ) as recalc_mock: + tool.Geometry._recalculate_walls_with_new_connections(old_to_new) + + assert recalc_mock.call_count == 1 + assert recalc_mock.call_args.args[0] == [wall_obj] + + def test_walls_without_connections_are_skipped(self): + from unittest.mock import Mock + + wall_new = Mock() + wall_new.is_a = lambda c: c == "IfcWall" + wall_new.ConnectedTo = [] + wall_new.ConnectedFrom = [] + + old_to_new = {Mock(): [wall_new]} + + with patch.object(tool.Ifc, "get_object", return_value=Mock()), patch.object( + tool.Model, "recalculate_walls" + ) as recalc_mock: + tool.Geometry._recalculate_walls_with_new_connections(old_to_new) + + assert recalc_mock.call_count == 0, "walls with no new connections must not trigger a recalc pass" + + def test_non_wall_entities_are_skipped(self): + from unittest.mock import Mock + + actuator_new = Mock() + actuator_new.is_a = lambda c: c == "IfcActuator" + actuator_new.ConnectedTo = [Mock()] + + old_to_new = {Mock(): [actuator_new]} + + with patch.object(tool.Ifc, "get_object", return_value=Mock()), patch.object( + tool.Model, "recalculate_walls" + ) as recalc_mock: + tool.Geometry._recalculate_walls_with_new_connections(old_to_new) + + assert recalc_mock.call_count == 0 + + def test_multiple_new_walls_collected_into_one_call(self): + from unittest.mock import Mock + + wall_a_new = Mock() + wall_a_new.is_a = lambda c: c == "IfcWall" + wall_a_new.ConnectedTo = [Mock()] + wall_a_new.ConnectedFrom = [] + wall_b_new = Mock() + wall_b_new.is_a = lambda c: c == "IfcWall" + wall_b_new.ConnectedTo = [] + wall_b_new.ConnectedFrom = [Mock()] + + objs = {wall_a_new: Mock(), wall_b_new: Mock()} + old_to_new = {Mock(): [wall_a_new], Mock(): [wall_b_new]} + + with patch.object(tool.Ifc, "get_object", side_effect=lambda e: objs.get(e)), patch.object( + tool.Model, "recalculate_walls" + ) as recalc_mock: + tool.Geometry._recalculate_walls_with_new_connections(old_to_new) + + assert recalc_mock.call_count == 1 + assert set(recalc_mock.call_args.args[0]) == {objs[wall_a_new], objs[wall_b_new]} + + +class TestOrphanArrayChildPrune(NewFile): + """Outliner / keyboard delete of a Bonsai-managed array child bypasses + ``bim.delete``'s cascade, leaving the IFC entity and its opening / filling + refs behind. Regen must prune these orphans before the main loop or the + stale registry entry corrupts the ``batch_host_recut`` drain.""" + + def test_orphan_ifc_entity_pruned_from_children_list(self): + obj, element, parent_data = _build_actuator_with_array_pset(count=4) + bpy.context.view_layer.objects.active = obj + tool.Model.regenerate_array(obj, parent_data) + assert len(parent_data[0]["children"]) == 3 + + orphan_guid = parent_data[0]["children"][1] + orphan_element = tool.Ifc.get().by_guid(orphan_guid) + orphan_obj = tool.Ifc.get_object(orphan_element) + assert orphan_obj is not None + bpy.data.objects.remove(orphan_obj, do_unlink=True) + + tool.Model.regenerate_array(obj, parent_data) + + assert ( + orphan_guid not in parent_data[0]["children"] + ), "orphan GUID must be pruned from array['children'] once its Blender object is dead" + try: + still_there = tool.Ifc.get().by_guid(orphan_guid) + except RuntimeError: + still_there = None + assert still_there is None, "orphan IFC entity must be cascade-removed, not left as a leak" + + def test_regen_completes_when_child_deleted_outside_bim_cascade(self): + obj, element, parent_data = _build_actuator_with_array_pset(count=6) + bpy.context.view_layer.objects.active = obj + tool.Model.regenerate_array(obj, parent_data) + + victim_guid = parent_data[0]["children"][2] + victim_element = tool.Ifc.get().by_guid(victim_guid) + victim_obj = tool.Ifc.get_object(victim_element) + bpy.data.objects.remove(victim_obj, do_unlink=True) + + tool.Model.regenerate_array(obj, parent_data) + + assert len(parent_data[0]["children"]) == 5, "regen must rebuild to the target count after pruning the orphan" + for guid in parent_data[0]["children"]: + child = tool.Ifc.get().by_guid(guid) + child_obj = tool.Ifc.get_object(child) + assert child_obj is not None, "every surviving child must have a live Blender object" + + diff --git a/src/bonsai/test/tool/test_geometry_batch_host_recut.py b/src/bonsai/test/tool/test_geometry_batch_host_recut.py index e55af60d39..8e9108b555 100644 --- a/src/bonsai/test/tool/test_geometry_batch_host_recut.py +++ b/src/bonsai/test/tool/test_geometry_batch_host_recut.py @@ -164,6 +164,62 @@ def test_stale_element_skipped_at_drain(): assert recut.call_count == 0 +class _DeadStructRNA: + """Simulates a Blender object whose StructRNA has been removed — every + attribute access raises ReferenceError. Enqueue this as voided_obj to + reproduce the outliner-mid-batch-delete crash.""" + + def __getattr__(self, name): + raise ReferenceError("StructRNA of type Object has been removed") + + def __bool__(self): + raise ReferenceError("StructRNA of type Object has been removed") + + +def test_dead_structrna_recut_skipped_at_drain(): + """Blender object is deleted while the batch is open (outliner delete + + manual DEL bypass the bim.delete cascade). The drain must skip it silently + — not raise — so unrelated hosts in the same batch still get their recut.""" + from bonsai import tool + + dead_obj = _DeadStructRNA() + live_obj = _mock_voided_obj("LiveWall") + rep = Mock() + + def get_entity(obj): + # Called only when the guard clears — for the dead ref, guard short-circuits first. + return _mock_element(2) + + with patch("bonsai.core.geometry.switch_representation") as recut, patch.object( + tool.Ifc, "get_entity", side_effect=get_entity + ), patch.object(tool.Geometry, "get_active_representation", return_value=rep): + with tool.Geometry.batch_host_recut(): + tool.Geometry._host_recut_queue[999] = (dead_obj, rep) + tool.Geometry.recut_host(live_obj, rep) + + assert recut.call_count == 1, "live host must still get its recut despite a dead sibling in the queue" + drained_obj = recut.call_args.kwargs["obj"] + assert drained_obj is live_obj + + +def test_dead_structrna_update_skipped_at_drain(): + """Same guarantee for update_representation drain path.""" + from bonsai import tool + + dead_obj = _DeadStructRNA() + live_obj = _mock_voided_obj("LiveWall") + bpy_ops_mock = Mock() + + with patch("bonsai.tool.geometry.bpy.ops", new=bpy_ops_mock), patch.object( + tool.Ifc, "get_entity", return_value=_mock_element(42) + ), patch.object(tool.Geometry, "get_active_representation", return_value=Mock()): + with tool.Geometry.batch_host_recut(): + tool.Geometry._host_update_queue[999] = dead_obj + tool.Geometry.update_host_representation(live_obj) + + assert bpy_ops_mock.bim.update_representation.call_count == 1 + + def test_exception_inside_batch_still_resets_state(): from bonsai import tool From c299f0b191e4efa127946a1ea50bd10e57ee5e68 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Wed, 8 Jul 2026 13:19:26 +0200 Subject: [PATCH 2/5] Bonsai: iterate every duplicated entity in relationship recreation Sweep [0]-indexing in recreate_aggregate, recreate_connections, and recreate_port_connections so batched N-child duplicates recreate relationships on every new child, not just the first. Single-source callers unaffected (loop collapses to one iteration on 1-element lists). Relates to #8088. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/tool/duplicate.py | 113 +++++----- src/bonsai/bonsai/tool/root.py | 52 +++-- .../model/test_array_duplicate_batched.py | 213 ++++++++++++++++++ 3 files changed, 294 insertions(+), 84 deletions(-) diff --git a/src/bonsai/bonsai/tool/duplicate.py b/src/bonsai/bonsai/tool/duplicate.py index eb3630ccf1..fd8258672b 100644 --- a/src/bonsai/bonsai/tool/duplicate.py +++ b/src/bonsai/bonsai/tool/duplicate.py @@ -248,32 +248,30 @@ class Duplicate(bonsai.core.tool.Duplicate): old_to_new: dict[ifcopenshell.entity_instance, list[ifcopenshell.entity_instance]], ) -> None: for element, data in relationship.items(): - try: - new_relating_element = old_to_new.get(data.relating_element)[0] - new_related_element = old_to_new.get(data.related_element)[0] - except (KeyError, IndexError, TypeError): - continue - new_rel = tool.Ifc.run( - "geometry.connect_path", - relating_element=new_relating_element, - related_element=new_related_element, - relating_connection=data.relating_connection_type, - related_connection=data.related_connection_type, - ) + new_relating_elements = old_to_new.get(data.relating_element) or [] + new_related_elements = old_to_new.get(data.related_element) or [] # connect_path hardcodes priorities to []; restore them post-hoc. priority_attrs: dict[str, Any] = {} if data.relating_priorities: priority_attrs["RelatingPriorities"] = data.relating_priorities if data.related_priorities: priority_attrs["RelatedPriorities"] = data.related_priorities - if new_rel is not None and priority_attrs: - try: - tool.Ifc.run("attribute.edit_attributes", product=new_rel, attributes=priority_attrs) - except (RuntimeError, ifcopenshell.Error) as e: - cls._emit_warning( - f"connection priority restore failed for {new_rel}; " - f"duplicate has empty RelatingPriorities/RelatedPriorities: {e}" - ) + for new_relating_element, new_related_element in zip(new_relating_elements, new_related_elements): + new_rel = tool.Ifc.run( + "geometry.connect_path", + relating_element=new_relating_element, + related_element=new_related_element, + relating_connection=data.relating_connection_type, + related_connection=data.related_connection_type, + ) + if new_rel is not None and priority_attrs: + try: + tool.Ifc.run("attribute.edit_attributes", product=new_rel, attributes=priority_attrs) + except (RuntimeError, ifcopenshell.Error) as e: + cls._emit_warning( + f"connection priority restore failed for {new_rel}; " + f"duplicate has empty RelatingPriorities/RelatedPriorities: {e}" + ) @classmethod def recreate_port_connections( @@ -283,46 +281,43 @@ class Duplicate(bonsai.core.tool.Duplicate): ) -> None: """Recreate ``IfcRelConnectsPorts`` between duplicates; skip records whose duplicate's port count diverges from the snapshot.""" for relating_element, records in snapshot.by_element.items(): + new_relatings = old_to_new.get(relating_element) or [] + expected_relating = snapshot.port_counts.get(relating_element) for record in records: related_element = record.related_element - try: - new_relating = old_to_new[relating_element][0] - new_related = old_to_new[related_element][0] - except (KeyError, IndexError): - continue - - new_relating_ports = tool.System.get_ports(new_relating) - new_related_ports = tool.System.get_ports(new_related) - - expected_relating = snapshot.port_counts.get(relating_element) - if expected_relating is not None and len(new_relating_ports) != expected_relating: - cls._emit_warning( - f"port reconnect skipped — duplicate has {len(new_relating_ports)} ports, " - f"snapshot had {expected_relating}" - ) - continue + new_relateds = old_to_new.get(related_element) or [] expected_related = snapshot.port_counts.get(related_element) - if expected_related is not None and len(new_related_ports) != expected_related: - cls._emit_warning( - f"port reconnect skipped — duplicate has {len(new_related_ports)} ports, " - f"snapshot had {expected_related}" - ) - continue + for new_relating, new_related in zip(new_relatings, new_relateds): + new_relating_ports = tool.System.get_ports(new_relating) + new_related_ports = tool.System.get_ports(new_related) - try: - new_port_a = new_relating_ports[record.relating_port_index] - new_port_b = new_related_ports[record.related_port_index] - except IndexError: - cls._emit_warning( - f"port reconnect skipped — record references port index past the duplicate's port list" - ) - continue - try: - tool.Ifc.run( - "system.connect_port", - port1=new_port_a, - port2=new_port_b, - direction=record.direction or "NOTDEFINED", - ) - except (RuntimeError, ifcopenshell.Error) as e: - cls._emit_warning(f"port reconnect failed between duplicates: {e}") + if expected_relating is not None and len(new_relating_ports) != expected_relating: + cls._emit_warning( + f"port reconnect skipped — duplicate has {len(new_relating_ports)} ports, " + f"snapshot had {expected_relating}" + ) + continue + if expected_related is not None and len(new_related_ports) != expected_related: + cls._emit_warning( + f"port reconnect skipped — duplicate has {len(new_related_ports)} ports, " + f"snapshot had {expected_related}" + ) + continue + + try: + new_port_a = new_relating_ports[record.relating_port_index] + new_port_b = new_related_ports[record.related_port_index] + except IndexError: + cls._emit_warning( + f"port reconnect skipped — record references port index past the duplicate's port list" + ) + continue + try: + tool.Ifc.run( + "system.connect_port", + port1=new_port_a, + port2=new_port_b, + direction=record.direction or "NOTDEFINED", + ) + except (RuntimeError, ifcopenshell.Error) as e: + cls._emit_warning(f"port reconnect failed between duplicates: {e}") diff --git a/src/bonsai/bonsai/tool/root.py b/src/bonsai/bonsai/tool/root.py index d7b7e0596f..ef2464d964 100644 --- a/src/bonsai/bonsai/tool/root.py +++ b/src/bonsai/bonsai/tool/root.py @@ -373,35 +373,37 @@ class Root(bonsai.core.tool.Root): try: new_aggregate = old_to_new[old_aggregate] except: - bonsai.core.aggregate.unassign_object( - tool.Ifc, - tool.Aggregate, - tool.Collector, - relating_obj=tool.Ifc.get_object(old_aggregate), - related_obj=tool.Ifc.get_object(new[0]), - ) - continue - - bonsai.core.aggregate.assign_object( - tool.Ifc, - tool.Aggregate, - tool.Collector, - relating_obj=tool.Ifc.get_object(new_aggregate[0]), - related_obj=tool.Ifc.get_object(new[0]), - ) - - # Make sure that the array children also get reassigned to the correct aggregate - pset = ifcopenshell.util.element.get_pset(new[0], "BBIM_Array") - if pset: - array_children = tool.Array.get_all_children_objects(new[0]) - for obj in array_children: - bonsai.core.aggregate.assign_object( + for new_entity in new: + bonsai.core.aggregate.unassign_object( tool.Ifc, tool.Aggregate, tool.Collector, - relating_obj=tool.Ifc.get_object(new_aggregate[0]), - related_obj=tool.Ifc.get_object(tool.Ifc.get_entity(obj)), + relating_obj=tool.Ifc.get_object(old_aggregate), + related_obj=tool.Ifc.get_object(new_entity), ) + continue + + for new_entity in new: + bonsai.core.aggregate.assign_object( + tool.Ifc, + tool.Aggregate, + tool.Collector, + relating_obj=tool.Ifc.get_object(new_aggregate[0]), + related_obj=tool.Ifc.get_object(new_entity), + ) + + # Make sure that the array children also get reassigned to the correct aggregate + pset = ifcopenshell.util.element.get_pset(new_entity, "BBIM_Array") + if pset: + array_children = tool.Array.get_all_children_objects(new_entity) + for obj in array_children: + bonsai.core.aggregate.assign_object( + tool.Ifc, + tool.Aggregate, + tool.Collector, + relating_obj=tool.Ifc.get_object(new_aggregate[0]), + related_obj=tool.Ifc.get_object(tool.Ifc.get_entity(obj)), + ) if new_aggregate is None: return diff --git a/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py b/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py index 285eccab39..a3696fba9f 100644 --- a/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py +++ b/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py @@ -374,3 +374,216 @@ class TestOrphanArrayChildPrune(NewFile): assert child_obj is not None, "every surviving child must have a live Blender object" +class TestRecreateAggregateIteratesAllNew(NewFile): + """Pins the [0]-indexing sweep in tool/root.py recreate_aggregate. When the + new-list has N>1 entries (the batched-duplicate shape), every entry must be + aggregate-assigned, not just new[0].""" + + def test_iterates_assign_object_per_new_entity_when_old_has_aggregate(self): + from unittest.mock import Mock + + old_assembly = Mock() + old_assembly.is_a = lambda c: c == "IfcElementAssembly" + old_parent_aggregate = Mock() + old_parent_aggregate.is_a = lambda c: False + + new_assemblies = [Mock(), Mock(), Mock()] + new_parent_aggregate = [Mock()] + + old_to_new = {old_assembly: new_assemblies, old_parent_aggregate: new_parent_aggregate} + + with patch( + "ifcopenshell.util.element.get_aggregate", + side_effect=lambda e: old_parent_aggregate if e is old_assembly else None, + ), patch("bonsai.core.aggregate.assign_object") as assign_mock, patch( + "ifcopenshell.util.element.get_pset", return_value=None + ), patch.object( + tool.Ifc, "get_object", side_effect=lambda e: Mock() + ), patch.object( + tool.Blender, "select_and_activate_single_object" + ): + tool.Root.recreate_aggregate(old_to_new) + + assert ( + assign_mock.call_count == 3 + ), f"recreate_aggregate must assign each of N new entities (not just new[0]); got {assign_mock.call_count}" + + def test_iterates_unassign_object_per_new_entity_when_aggregate_missing(self): + from unittest.mock import Mock + + old_assembly = Mock() + old_assembly.is_a = lambda c: c == "IfcElementAssembly" + old_parent_aggregate = Mock() + + new_assemblies = [Mock(), Mock(), Mock()] + old_to_new = {old_assembly: new_assemblies} # parent aggregate NOT in old_to_new + + with patch( + "ifcopenshell.util.element.get_aggregate", + side_effect=lambda e: old_parent_aggregate if e is old_assembly else None, + ), patch("bonsai.core.aggregate.unassign_object") as unassign_mock, patch.object( + tool.Ifc, "get_object", side_effect=lambda e: Mock() + ): + tool.Root.recreate_aggregate(old_to_new) + + assert unassign_mock.call_count == 3, ( + f"recreate_aggregate must unassign each of N new entities when parent aggregate is missing; " + f"got {unassign_mock.call_count}" + ) + + +class TestRecreateConnectionsZipsPairs(NewFile): + """Pins the [0]-indexing sweep in tool/duplicate.py recreate_connections. When + both sides of a connection are duplicated N times, zip-pair the N new + relating with N new related; when only one side is duplicated, skip.""" + + def _make_connection_data(self): + from unittest.mock import Mock + + data = Mock() + data.relating_element = Mock() + data.related_element = Mock() + data.relating_connection_type = "ATSTART" + data.related_connection_type = "ATEND" + data.relating_priorities = [] + data.related_priorities = [] + return data + + def test_zips_n_pairs_when_both_sides_duplicated(self): + from unittest.mock import Mock + + data = self._make_connection_data() + old_to_new = { + data.relating_element: [Mock(), Mock(), Mock()], + data.related_element: [Mock(), Mock(), Mock()], + } + relationship = {Mock(): data} + + with patch.object(tool.Ifc, "run", return_value=None) as run_mock: + tool.Duplicate.recreate_connections(relationship, old_to_new) + + connect_calls = [c for c in run_mock.call_args_list if c.args and c.args[0] == "geometry.connect_path"] + assert ( + len(connect_calls) == 3 + ), f"zip-pair must create 3 connect_path calls for 3-vs-3 batched duplicate; got {len(connect_calls)}" + + def test_skips_when_other_side_not_duplicated(self): + from unittest.mock import Mock + + data = self._make_connection_data() + # Only relating side is in old_to_new; related side was NOT duplicated. + old_to_new = {data.relating_element: [Mock(), Mock(), Mock()]} + relationship = {Mock(): data} + + with patch.object(tool.Ifc, "run", return_value=None) as run_mock: + tool.Duplicate.recreate_connections(relationship, old_to_new) + + connect_calls = [c for c in run_mock.call_args_list if c.args and c.args[0] == "geometry.connect_path"] + assert ( + connect_calls == [] + ), "when only one side of a connection is in old_to_new, no connections should be recreated" + + def test_single_pair_case_unchanged(self): + """Pre-sweep behavior (1 source -> 1 new) must still work — zip with two 1-element lists.""" + from unittest.mock import Mock + + data = self._make_connection_data() + old_to_new = { + data.relating_element: [Mock()], + data.related_element: [Mock()], + } + relationship = {Mock(): data} + + with patch.object(tool.Ifc, "run", return_value=None) as run_mock: + tool.Duplicate.recreate_connections(relationship, old_to_new) + + connect_calls = [c for c in run_mock.call_args_list if c.args and c.args[0] == "geometry.connect_path"] + assert len(connect_calls) == 1 + + +class TestRecreatePortConnectionsZipsPairs(NewFile): + """Pins the [0]-indexing sweep in tool/duplicate.py recreate_port_connections. + When both sides of a port-to-port connection are duplicated N times, the + connection must be recreated on every pair of new siblings — not just the + first. Matters for arrayed MEP segments (pipes / ducts / cables) where each + child in the array should stay connected to its neighbour after regen.""" + + def _make_snapshot(self, relating_element, records, port_counts): + from bonsai.tool.duplicate import PortConnectionSnapshot + + return PortConnectionSnapshot( + by_element={relating_element: records}, + port_counts=port_counts, + ) + + def _make_record(self, related_element, relating_port_index=0, related_port_index=0, direction="SOURCE"): + from bonsai.tool.duplicate import PortConnectionRecord + + return PortConnectionRecord( + relating_port_index=relating_port_index, + related_element=related_element, + related_port_index=related_port_index, + direction=direction, + ) + + def test_zips_n_pairs_when_both_sides_duplicated(self): + from unittest.mock import Mock + + relating_old = Mock() + related_old = Mock() + record = self._make_record(related_old) + snapshot = self._make_snapshot(relating_old, [record], port_counts={}) + + old_to_new = { + relating_old: [Mock(), Mock(), Mock()], + related_old: [Mock(), Mock(), Mock()], + } + + fake_ports = [Mock(), Mock()] + with patch.object(tool.System, "get_ports", return_value=fake_ports), patch.object( + tool.Ifc, "run", return_value=None + ) as run_mock: + tool.Duplicate.recreate_port_connections(snapshot, old_to_new) + + connect_calls = [c for c in run_mock.call_args_list if c.args and c.args[0] == "system.connect_port"] + assert ( + len(connect_calls) == 3 + ), f"zip-pair must create 3 connect_port calls for 3-vs-3 batched MEP duplicate; got {len(connect_calls)}" + + def test_skips_when_other_side_not_duplicated(self): + from unittest.mock import Mock + + relating_old = Mock() + related_old = Mock() + record = self._make_record(related_old) + snapshot = self._make_snapshot(relating_old, [record], port_counts={}) + + # Only relating side is in old_to_new. + old_to_new = {relating_old: [Mock(), Mock(), Mock()]} + + with patch.object(tool.System, "get_ports", return_value=[Mock()]), patch.object( + tool.Ifc, "run", return_value=None + ) as run_mock: + tool.Duplicate.recreate_port_connections(snapshot, old_to_new) + + connect_calls = [c for c in run_mock.call_args_list if c.args and c.args[0] == "system.connect_port"] + assert connect_calls == [], "when only one side is in old_to_new, no port connections should be recreated" + + def test_single_pair_case_unchanged(self): + """Pre-sweep behavior (1 source -> 1 new) must still work — zip with two 1-element lists.""" + from unittest.mock import Mock + + relating_old = Mock() + related_old = Mock() + record = self._make_record(related_old) + snapshot = self._make_snapshot(relating_old, [record], port_counts={}) + + old_to_new = {relating_old: [Mock()], related_old: [Mock()]} + + with patch.object(tool.System, "get_ports", return_value=[Mock()]), patch.object( + tool.Ifc, "run", return_value=None + ) as run_mock: + tool.Duplicate.recreate_port_connections(snapshot, old_to_new) + + connect_calls = [c for c in run_mock.call_args_list if c.args and c.args[0] == "system.connect_port"] + assert len(connect_calls) == 1 From 9191baf0677c257369f5386f60226004e13c2c0a Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Wed, 8 Jul 2026 13:19:42 +0200 Subject: [PATCH 3/5] Bonsai: hide array-child gizmos + converge regen selection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add tool.Array.is_array_child helper. Port decorator and MEP action gizmos (lock, pen, join) hide on array children — writes on children get wiped by the next regen, and the port topology is inherited from the parent. Introduce tool.Array.select_only_parent and wire it into both bim.regenerate_array and bim.finish_editing_array so post-regen state converges on parent-only-selected + active. Grow and shrink paths otherwise diverge (grow left new children selected alongside the parent; shrink left only the parent). Relates to #8088. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/model/array.py | 9 +- src/bonsai/bonsai/bim/module/model/mep.py | 9 + src/bonsai/bonsai/tool/array.py | 33 ++ src/bonsai/bonsai/tool/system.py | 7 + .../model/test_array_duplicate_batched.py | 384 ++++++++++++------ .../model/test_mep_actions_visibility.py | 4 +- src/bonsai/test/tool/test_model.py | 8 +- 7 files changed, 315 insertions(+), 139 deletions(-) diff --git a/src/bonsai/bonsai/bim/module/model/array.py b/src/bonsai/bonsai/bim/module/model/array.py index c1265143ea..c9e9c1b4fe 100644 --- a/src/bonsai/bonsai/bim/module/model/array.py +++ b/src/bonsai/bonsai/bim/module/model/array.py @@ -329,6 +329,7 @@ class _ArrayEditMixin(ParametricEditMixinBase): # Unhide the (possibly newly-regenerated) children so the user sees # the committed result. Mirrors the hide in ``_enable_one``. cls._set_children_visibility(element, hidden=False) + tool.Array.select_only_parent(obj, element, context) @classmethod def _cancel_one(cls, obj: bpy.types.Object) -> None: @@ -421,9 +422,9 @@ class RegenerateArray(bpy.types.Operator, tool.Ifc.Operator): pset = ifcopenshell.util.element.get_pset(parent_element, "BBIM_Array") arrays = json.loads(pset["Data"]) pset = tool.Ifc.get().by_id(pset["id"]) - # Coalesce host recuts: the child-delete loop, the regenerate, and the - # per-child opening mirror all touch the same host body. Without batching, - # an N-child wipe-then-regen costs N+1 recuts; this collapses to one. + # Coalesce host recuts across the child-delete loop, the regenerate, + # and the per-child opening mirror: each fans out its own host body + # recut without the batch wrapper. with tool.Geometry.batch_host_recut(): for array in arrays: for child in set(array["children"]): @@ -442,6 +443,8 @@ class RegenerateArray(bpy.types.Operator, tool.Ifc.Operator): tool.Model.regenerate_array(parent, arrays) tool.Array.constrain_children_to_parent(parent_element) + tool.Array.select_only_parent(parent, parent_element, context) + class RemoveArray(bpy.types.Operator, tool.Ifc.Operator): bl_idname = "bim.remove_array" diff --git a/src/bonsai/bonsai/bim/module/model/mep.py b/src/bonsai/bonsai/bim/module/model/mep.py index 26a5a53de0..723ac75e46 100644 --- a/src/bonsai/bonsai/bim/module/model/mep.py +++ b/src/bonsai/bonsai/bim/module/model/mep.py @@ -1677,6 +1677,11 @@ def _n_mep_selected(n: int) -> bool: element = tool.Ifc.get_entity(selected_obj) if element is None or not tool.System.is_mep_element(element): return False + # Array children mirror their parent's port topology. Writable MEP + # actions on a child get wiped by the next array regen, so gate the + # icons out at the visibility layer. + if tool.Array.is_array_child(element): + return False return True @@ -2555,6 +2560,8 @@ def _active_is_flow_segment(obj: bpy.types.Object) -> bool: element = tool.Ifc.get_entity(obj) if element is None or not element.is_a("IfcFlowSegment"): return False + if tool.Array.is_array_child(element): + return False return tool.System.has_parametric_body(element) @@ -2584,6 +2591,8 @@ def _active_is_bend_fitting(obj: bpy.types.Object) -> bool: element = tool.Ifc.get_entity(obj) if not _is_bend_fitting(element): return False + if tool.Array.is_array_child(element): + return False element_type = ifcopenshell.util.element.get_type(element) if element_type is None: return False diff --git a/src/bonsai/bonsai/tool/array.py b/src/bonsai/bonsai/tool/array.py index d5e35bb6f9..0e77561f22 100644 --- a/src/bonsai/bonsai/tool/array.py +++ b/src/bonsai/bonsai/tool/array.py @@ -178,6 +178,39 @@ class Array(bonsai.core.tool.Array): element_root = cls.get_array_root_guid(element) return [o for o in occurrences if cls.get_array_root_guid(o) == element_root] + @classmethod + def select_only_parent( + cls, + parent_obj: bpy.types.Object, + parent_element: entity_instance, + context: bpy.types.Context, + ) -> None: + """Deselect every array child of ``parent_element``, then select and + activate ``parent_obj``. Post-condition for the user-facing regenerate + and finish-edit paths — grow and shrink otherwise diverge on which + objects stay selected, surfacing an inconsistency to the user.""" + for child_obj in cls.get_all_objects(parent_element): + if child_obj is parent_obj: + continue + try: + child_obj.select_set(False) + except (ReferenceError, RuntimeError): + continue + parent_obj.select_set(True) + context.view_layer.objects.active = parent_obj + + @classmethod + def is_array_child(cls, element: entity_instance) -> bool: + """True when ``element`` is a child of a parametric array — has a + BBIM_Array pset whose Parent GUID points to a different element. + Lighter than ``get_child_layer_index`` (no ``by_guid`` lookup, no + Data parse); suitable for per-element checks in draw handlers.""" + pset = ifcopenshell.util.element.get_pset(element, "BBIM_Array") + if not pset: + return False + parent_guid = pset.get("Parent") + return bool(parent_guid) and parent_guid != element.GlobalId + @classmethod def get_child_layer_index(cls, child_element: entity_instance) -> int | None: """Index of the layer that produced ``child_element``, or ``None`` diff --git a/src/bonsai/bonsai/tool/system.py b/src/bonsai/bonsai/tool/system.py index cf1ed3ab88..1deb454695 100644 --- a/src/bonsai/bonsai/tool/system.py +++ b/src/bonsai/bonsai/tool/system.py @@ -357,6 +357,13 @@ class System(bonsai.core.tool.System): if not cls.is_mep_element(element): continue + # Array children inherit port topology from their parent's IFC + # entity, but their positions are derived — drawing ports on every + # copy of an arrayed segment doubles up markers and misleads the + # user into thinking each copy has its own port network. + if tool.Array.is_array_child(element): + continue + selected_element = element in connected_elements verts_pos = [] diff --git a/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py b/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py index a3696fba9f..4f7f5eabec 100644 --- a/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py +++ b/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py @@ -243,137 +243,6 @@ class TestRegenerateArrayUIRefreshCoalesces(NewFile): assert reload_mock.call_count == 1 -class TestRecalculateWallsWithNewConnections(NewFile): - """Pins the post-connection wall recalc: after ``recreate_connections`` - wires new IfcRelConnectsPathElements onto duplicated walls, the wall - bodies must be re-recalculated because the in-loop ``regenerate_wall`` - fired before the connections existed. Otherwise the junction geometry - stays stale and the user has to manually regen.""" - - def test_walls_with_new_connections_are_recalculated(self): - from unittest.mock import Mock - - wall_new = Mock() - wall_new.is_a = lambda c: c == "IfcWall" - wall_new.ConnectedTo = [Mock()] - wall_new.ConnectedFrom = [] - - wall_obj = Mock() - old_to_new = {Mock(): [wall_new]} - - with patch.object(tool.Ifc, "get_object", return_value=wall_obj), patch.object( - tool.Model, "recalculate_walls" - ) as recalc_mock: - tool.Geometry._recalculate_walls_with_new_connections(old_to_new) - - assert recalc_mock.call_count == 1 - assert recalc_mock.call_args.args[0] == [wall_obj] - - def test_walls_without_connections_are_skipped(self): - from unittest.mock import Mock - - wall_new = Mock() - wall_new.is_a = lambda c: c == "IfcWall" - wall_new.ConnectedTo = [] - wall_new.ConnectedFrom = [] - - old_to_new = {Mock(): [wall_new]} - - with patch.object(tool.Ifc, "get_object", return_value=Mock()), patch.object( - tool.Model, "recalculate_walls" - ) as recalc_mock: - tool.Geometry._recalculate_walls_with_new_connections(old_to_new) - - assert recalc_mock.call_count == 0, "walls with no new connections must not trigger a recalc pass" - - def test_non_wall_entities_are_skipped(self): - from unittest.mock import Mock - - actuator_new = Mock() - actuator_new.is_a = lambda c: c == "IfcActuator" - actuator_new.ConnectedTo = [Mock()] - - old_to_new = {Mock(): [actuator_new]} - - with patch.object(tool.Ifc, "get_object", return_value=Mock()), patch.object( - tool.Model, "recalculate_walls" - ) as recalc_mock: - tool.Geometry._recalculate_walls_with_new_connections(old_to_new) - - assert recalc_mock.call_count == 0 - - def test_multiple_new_walls_collected_into_one_call(self): - from unittest.mock import Mock - - wall_a_new = Mock() - wall_a_new.is_a = lambda c: c == "IfcWall" - wall_a_new.ConnectedTo = [Mock()] - wall_a_new.ConnectedFrom = [] - wall_b_new = Mock() - wall_b_new.is_a = lambda c: c == "IfcWall" - wall_b_new.ConnectedTo = [] - wall_b_new.ConnectedFrom = [Mock()] - - objs = {wall_a_new: Mock(), wall_b_new: Mock()} - old_to_new = {Mock(): [wall_a_new], Mock(): [wall_b_new]} - - with patch.object(tool.Ifc, "get_object", side_effect=lambda e: objs.get(e)), patch.object( - tool.Model, "recalculate_walls" - ) as recalc_mock: - tool.Geometry._recalculate_walls_with_new_connections(old_to_new) - - assert recalc_mock.call_count == 1 - assert set(recalc_mock.call_args.args[0]) == {objs[wall_a_new], objs[wall_b_new]} - - -class TestOrphanArrayChildPrune(NewFile): - """Outliner / keyboard delete of a Bonsai-managed array child bypasses - ``bim.delete``'s cascade, leaving the IFC entity and its opening / filling - refs behind. Regen must prune these orphans before the main loop or the - stale registry entry corrupts the ``batch_host_recut`` drain.""" - - def test_orphan_ifc_entity_pruned_from_children_list(self): - obj, element, parent_data = _build_actuator_with_array_pset(count=4) - bpy.context.view_layer.objects.active = obj - tool.Model.regenerate_array(obj, parent_data) - assert len(parent_data[0]["children"]) == 3 - - orphan_guid = parent_data[0]["children"][1] - orphan_element = tool.Ifc.get().by_guid(orphan_guid) - orphan_obj = tool.Ifc.get_object(orphan_element) - assert orphan_obj is not None - bpy.data.objects.remove(orphan_obj, do_unlink=True) - - tool.Model.regenerate_array(obj, parent_data) - - assert ( - orphan_guid not in parent_data[0]["children"] - ), "orphan GUID must be pruned from array['children'] once its Blender object is dead" - try: - still_there = tool.Ifc.get().by_guid(orphan_guid) - except RuntimeError: - still_there = None - assert still_there is None, "orphan IFC entity must be cascade-removed, not left as a leak" - - def test_regen_completes_when_child_deleted_outside_bim_cascade(self): - obj, element, parent_data = _build_actuator_with_array_pset(count=6) - bpy.context.view_layer.objects.active = obj - tool.Model.regenerate_array(obj, parent_data) - - victim_guid = parent_data[0]["children"][2] - victim_element = tool.Ifc.get().by_guid(victim_guid) - victim_obj = tool.Ifc.get_object(victim_element) - bpy.data.objects.remove(victim_obj, do_unlink=True) - - tool.Model.regenerate_array(obj, parent_data) - - assert len(parent_data[0]["children"]) == 5, "regen must rebuild to the target count after pruning the orphan" - for guid in parent_data[0]["children"]: - child = tool.Ifc.get().by_guid(guid) - child_obj = tool.Ifc.get_object(child) - assert child_obj is not None, "every surviving child must have a live Blender object" - - class TestRecreateAggregateIteratesAllNew(NewFile): """Pins the [0]-indexing sweep in tool/root.py recreate_aggregate. When the new-list has N>1 entries (the batched-duplicate shape), every entry must be @@ -501,6 +370,259 @@ class TestRecreateConnectionsZipsPairs(NewFile): assert len(connect_calls) == 1 +class TestRecalculateWallsWithNewConnections(NewFile): + """Pins the post-connection wall recalc: after ``recreate_connections`` + wires new IfcRelConnectsPathElements onto duplicated walls, the wall + bodies must be re-recalculated because the in-loop ``regenerate_wall`` + fired before the connections existed. Otherwise the junction geometry + stays stale and the user has to manually regen.""" + + def test_walls_with_new_connections_are_recalculated(self): + from unittest.mock import Mock + + wall_new = Mock() + wall_new.is_a = lambda c: c == "IfcWall" + wall_new.ConnectedTo = [Mock()] + wall_new.ConnectedFrom = [] + + wall_obj = Mock() + old_to_new = {Mock(): [wall_new]} + + with patch.object(tool.Ifc, "get_object", return_value=wall_obj), patch.object( + tool.Model, "recalculate_walls" + ) as recalc_mock: + tool.Geometry._recalculate_walls_with_new_connections(old_to_new) + + assert recalc_mock.call_count == 1 + assert recalc_mock.call_args.args[0] == [wall_obj] + + def test_walls_without_connections_are_skipped(self): + from unittest.mock import Mock + + wall_new = Mock() + wall_new.is_a = lambda c: c == "IfcWall" + wall_new.ConnectedTo = [] + wall_new.ConnectedFrom = [] + + old_to_new = {Mock(): [wall_new]} + + with patch.object(tool.Ifc, "get_object", return_value=Mock()), patch.object( + tool.Model, "recalculate_walls" + ) as recalc_mock: + tool.Geometry._recalculate_walls_with_new_connections(old_to_new) + + assert recalc_mock.call_count == 0, "walls with no new connections must not trigger a recalc pass" + + def test_non_wall_entities_are_skipped(self): + from unittest.mock import Mock + + actuator_new = Mock() + actuator_new.is_a = lambda c: c == "IfcActuator" + actuator_new.ConnectedTo = [Mock()] + + old_to_new = {Mock(): [actuator_new]} + + with patch.object(tool.Ifc, "get_object", return_value=Mock()), patch.object( + tool.Model, "recalculate_walls" + ) as recalc_mock: + tool.Geometry._recalculate_walls_with_new_connections(old_to_new) + + assert recalc_mock.call_count == 0 + + def test_multiple_new_walls_collected_into_one_call(self): + from unittest.mock import Mock + + wall_a_new = Mock() + wall_a_new.is_a = lambda c: c == "IfcWall" + wall_a_new.ConnectedTo = [Mock()] + wall_a_new.ConnectedFrom = [] + wall_b_new = Mock() + wall_b_new.is_a = lambda c: c == "IfcWall" + wall_b_new.ConnectedTo = [] + wall_b_new.ConnectedFrom = [Mock()] + + objs = {wall_a_new: Mock(), wall_b_new: Mock()} + old_to_new = {Mock(): [wall_a_new], Mock(): [wall_b_new]} + + with patch.object(tool.Ifc, "get_object", side_effect=lambda e: objs.get(e)), patch.object( + tool.Model, "recalculate_walls" + ) as recalc_mock: + tool.Geometry._recalculate_walls_with_new_connections(old_to_new) + + assert recalc_mock.call_count == 1 + assert set(recalc_mock.call_args.args[0]) == {objs[wall_a_new], objs[wall_b_new]} + + +class TestMEPActionGuardsAgainstArrayChildren(NewFile): + """Pins the array-child guards on the three MEP-action visibility helpers. + Writable MEP actions (add fitting, remove terminal, join, re-edit bend) + applied to an array child get wiped by the next regen — gating the icons + at the visibility layer prevents that footgun.""" + + def test_active_is_flow_segment_returns_false_for_array_child(self): + from unittest.mock import Mock + + from bonsai.bim.module.model.mep import _active_is_flow_segment + + obj = Mock() + element = Mock() + element.is_a = lambda c: c == "IfcFlowSegment" + + with patch.object(tool.Ifc, "get_entity", return_value=element), patch.object( + tool.Array, "is_array_child", return_value=True + ), patch.object(tool.System, "has_parametric_body", return_value=True): + assert _active_is_flow_segment(obj) is False + + def test_active_is_flow_segment_true_for_non_array_parent(self): + from unittest.mock import Mock + + from bonsai.bim.module.model.mep import _active_is_flow_segment + + obj = Mock() + element = Mock() + element.is_a = lambda c: c == "IfcFlowSegment" + + with patch.object(tool.Ifc, "get_entity", return_value=element), patch.object( + tool.Array, "is_array_child", return_value=False + ), patch.object(tool.System, "has_parametric_body", return_value=True): + assert _active_is_flow_segment(obj) is True + + def test_active_is_bend_fitting_returns_false_for_array_child(self): + from unittest.mock import Mock + + from bonsai.bim.module.model.mep import _active_is_bend_fitting + + obj = Mock() + element = Mock() + + with patch.object(tool.Ifc, "get_entity", return_value=element), patch( + "bonsai.bim.module.model.mep._is_bend_fitting", return_value=True + ), patch.object(tool.Array, "is_array_child", return_value=True): + assert _active_is_bend_fitting(obj) is False + + def test_n_mep_selected_returns_false_when_any_selected_is_array_child(self): + from unittest.mock import Mock + + from bonsai.bim.module.model.mep import _n_mep_selected + + obj_a = Mock() + obj_b = Mock() + element_a = Mock() + element_b = Mock() + + def is_array_child(el): + return el is element_b + + with patch.object(tool.Blender, "get_selected_objects", return_value=[obj_a, obj_b]), patch.object( + tool.Ifc, "get_entity", side_effect=lambda o: element_a if o is obj_a else element_b + ), patch.object(tool.System, "is_mep_element", return_value=True), patch.object( + tool.Array, "is_array_child", side_effect=is_array_child + ): + assert _n_mep_selected(2) is False + + +class TestSelectOnlyParent(NewFile): + """Pins ``tool.Array.select_only_parent`` — the shared helper wired into + both ``bim.regenerate_array`` and ``bim.finish_editing_array`` so the + grow / shrink / edit-commit paths converge on the same post-condition: + only the parent is selected + active.""" + + def test_deselects_children_selects_and_activates_parent(self): + obj, element, parent_data = _build_actuator_with_array_pset(count=4) + bpy.context.view_layer.objects.active = obj + obj.select_set(True) + tool.Model.regenerate_array(obj, parent_data) + for child_guid in parent_data[0]["children"]: + child_element = tool.Ifc.get().by_guid(child_guid) + child_obj = tool.Ifc.get_object(child_element) + child_obj.select_set(True) + + tool.Array.select_only_parent(obj, element, bpy.context) + + assert obj in bpy.context.selected_objects + assert bpy.context.view_layer.objects.active is obj + for child_guid in parent_data[0]["children"]: + child_element = tool.Ifc.get().by_guid(child_guid) + child_obj = tool.Ifc.get_object(child_element) + assert child_obj not in bpy.context.selected_objects + + +class TestIsArrayChild(NewFile): + """Pins ``tool.Array.is_array_child`` — the light helper used by the port + decorator (and any future per-element guard) to skip array children.""" + + def test_returns_false_when_no_bbim_array_pset(self): + from unittest.mock import Mock + + element = Mock() + with patch("ifcopenshell.util.element.get_pset", return_value=None): + assert tool.Array.is_array_child(element) is False + + def test_returns_false_on_the_array_parent_itself(self): + from unittest.mock import Mock + + element = Mock() + element.GlobalId = "PARENT_GUID" + with patch("ifcopenshell.util.element.get_pset", return_value={"Parent": "PARENT_GUID"}): + assert tool.Array.is_array_child(element) is False + + def test_returns_true_when_parent_guid_points_elsewhere(self): + from unittest.mock import Mock + + element = Mock() + element.GlobalId = "CHILD_GUID" + with patch("ifcopenshell.util.element.get_pset", return_value={"Parent": "PARENT_GUID"}): + assert tool.Array.is_array_child(element) is True + + +class TestOrphanArrayChildPrune(NewFile): + """Outliner / keyboard delete of a Bonsai-managed array child bypasses + ``bim.delete``'s cascade, leaving the IFC entity and its opening / filling + refs behind. Regen must prune these orphans before the main loop or the + stale registry entry corrupts the ``batch_host_recut`` drain.""" + + def test_orphan_ifc_entity_pruned_from_children_list(self): + obj, element, parent_data = _build_actuator_with_array_pset(count=4) + bpy.context.view_layer.objects.active = obj + tool.Model.regenerate_array(obj, parent_data) + assert len(parent_data[0]["children"]) == 3 + + orphan_guid = parent_data[0]["children"][1] + orphan_element = tool.Ifc.get().by_guid(orphan_guid) + orphan_obj = tool.Ifc.get_object(orphan_element) + assert orphan_obj is not None + bpy.data.objects.remove(orphan_obj, do_unlink=True) + + tool.Model.regenerate_array(obj, parent_data) + + assert ( + orphan_guid not in parent_data[0]["children"] + ), "orphan GUID must be pruned from array['children'] once its Blender object is dead" + try: + still_there = tool.Ifc.get().by_guid(orphan_guid) + except RuntimeError: + still_there = None + assert still_there is None, "orphan IFC entity must be cascade-removed, not left as a leak" + + def test_regen_completes_when_child_deleted_outside_bim_cascade(self): + obj, element, parent_data = _build_actuator_with_array_pset(count=6) + bpy.context.view_layer.objects.active = obj + tool.Model.regenerate_array(obj, parent_data) + + victim_guid = parent_data[0]["children"][2] + victim_element = tool.Ifc.get().by_guid(victim_guid) + victim_obj = tool.Ifc.get_object(victim_element) + bpy.data.objects.remove(victim_obj, do_unlink=True) + + tool.Model.regenerate_array(obj, parent_data) + + assert len(parent_data[0]["children"]) == 5, "regen must rebuild to the target count after pruning the orphan" + for guid in parent_data[0]["children"]: + child = tool.Ifc.get().by_guid(guid) + child_obj = tool.Ifc.get_object(child) + assert child_obj is not None, "every surviving child must have a live Blender object" + + class TestRecreatePortConnectionsZipsPairs(NewFile): """Pins the [0]-indexing sweep in tool/duplicate.py recreate_port_connections. When both sides of a port-to-port connection are duplicated N times, the diff --git a/src/bonsai/test/bim/module/model/test_mep_actions_visibility.py b/src/bonsai/test/bim/module/model/test_mep_actions_visibility.py index 601ff8dfb9..dfff84da8a 100644 --- a/src/bonsai/test/bim/module/model/test_mep_actions_visibility.py +++ b/src/bonsai/test/bim/module/model/test_mep_actions_visibility.py @@ -346,7 +346,9 @@ def test_active_is_flow_segment_classifies_segment_vs_fitting(): fitting_elem.is_a = lambda c: c == "IfcFlowFitting" plain = Mock() - with patch("bonsai.bim.module.model.mep.tool.System.has_parametric_body", return_value=True): + with patch("bonsai.bim.module.model.mep.tool.System.has_parametric_body", return_value=True), patch( + "bonsai.bim.module.model.mep.tool.Array.is_array_child", return_value=False + ): with patch("bonsai.bim.module.model.mep.tool.Ifc.get_entity", return_value=segment_elem): assert _active_is_flow_segment(plain) is True with patch("bonsai.bim.module.model.mep.tool.Ifc.get_entity", return_value=fitting_elem): diff --git a/src/bonsai/test/tool/test_model.py b/src/bonsai/test/tool/test_model.py index 9d21aedc1a..e1b4601663 100644 --- a/src/bonsai/test/tool/test_model.py +++ b/src/bonsai/test/tool/test_model.py @@ -630,15 +630,15 @@ class TestUsingArrays(NewFile): def test_remove_array_first_to_last(self): self.setup_array(add_second_layer=True) bpy.ops.bim.remove_array(item=0) - assert len(bpy.context.selected_objects) == 3 + assert len(self._array_objects()) == 3 bpy.ops.bim.remove_array(item=0) - assert len(bpy.context.selected_objects) == 1 + assert len(self._array_objects()) == 1 def test_apply_array_1_layer(self): self.setup_array() bpy.ops.bim.apply_array() - objs = bpy.context.selected_objects + objs = self._array_objects() assert len(objs) == 4 # check BBIM_Array psets are removed for obj in objs: @@ -664,7 +664,7 @@ class TestUsingArrays(NewFile): self.setup_array(sync_children=True) bpy.ops.bim.apply_array() - objs = bpy.context.selected_objects + objs = self._array_objects() assert len(objs) == 4 # check BBIM_Array psets are removed for obj in objs: From da50d22ed5e33e575d9dbecbabb059d2d536e910 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Wed, 8 Jul 2026 14:53:29 +0200 Subject: [PATCH 4/5] Bonsai: route array-regen selection through tool.Blender utilities Rewrite tool.Array.select_only_parent as a thin call to tool.Blender.select_and_activate_single_object; drop the ad-hoc per-child deselect loop and the unused parent_element parameter. Replace the tail parent_obj.select_set(True) in _regenerate_array_body with tool.Blender.set_object_selection, which wraps select_set in the hidden-object try/except the utility already owns. Relates to #8088. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/model/array.py | 4 ++-- src/bonsai/bonsai/tool/array.py | 24 ++++--------------- src/bonsai/bonsai/tool/model.py | 2 +- .../model/test_array_duplicate_batched.py | 2 +- 4 files changed, 9 insertions(+), 23 deletions(-) diff --git a/src/bonsai/bonsai/bim/module/model/array.py b/src/bonsai/bonsai/bim/module/model/array.py index c9e9c1b4fe..5972189405 100644 --- a/src/bonsai/bonsai/bim/module/model/array.py +++ b/src/bonsai/bonsai/bim/module/model/array.py @@ -329,7 +329,7 @@ class _ArrayEditMixin(ParametricEditMixinBase): # Unhide the (possibly newly-regenerated) children so the user sees # the committed result. Mirrors the hide in ``_enable_one``. cls._set_children_visibility(element, hidden=False) - tool.Array.select_only_parent(obj, element, context) + tool.Array.select_only_parent(obj, context) @classmethod def _cancel_one(cls, obj: bpy.types.Object) -> None: @@ -443,7 +443,7 @@ class RegenerateArray(bpy.types.Operator, tool.Ifc.Operator): tool.Model.regenerate_array(parent, arrays) tool.Array.constrain_children_to_parent(parent_element) - tool.Array.select_only_parent(parent, parent_element, context) + tool.Array.select_only_parent(parent, context) class RemoveArray(bpy.types.Operator, tool.Ifc.Operator): diff --git a/src/bonsai/bonsai/tool/array.py b/src/bonsai/bonsai/tool/array.py index 0e77561f22..1b8c2a4f22 100644 --- a/src/bonsai/bonsai/tool/array.py +++ b/src/bonsai/bonsai/tool/array.py @@ -179,25 +179,11 @@ class Array(bonsai.core.tool.Array): return [o for o in occurrences if cls.get_array_root_guid(o) == element_root] @classmethod - def select_only_parent( - cls, - parent_obj: bpy.types.Object, - parent_element: entity_instance, - context: bpy.types.Context, - ) -> None: - """Deselect every array child of ``parent_element``, then select and - activate ``parent_obj``. Post-condition for the user-facing regenerate - and finish-edit paths — grow and shrink otherwise diverge on which - objects stay selected, surfacing an inconsistency to the user.""" - for child_obj in cls.get_all_objects(parent_element): - if child_obj is parent_obj: - continue - try: - child_obj.select_set(False) - except (ReferenceError, RuntimeError): - continue - parent_obj.select_set(True) - context.view_layer.objects.active = parent_obj + def select_only_parent(cls, parent_obj: bpy.types.Object, context: bpy.types.Context) -> None: + """Post-condition for the user-facing regenerate and finish-edit paths: + only ``parent_obj`` is selected + active. Grow and shrink otherwise + diverge on which objects stay selected, surfacing an inconsistency.""" + tool.Blender.select_and_activate_single_object(context, parent_obj) @classmethod def is_array_child(cls, element: entity_instance) -> bool: diff --git a/src/bonsai/bonsai/tool/model.py b/src/bonsai/bonsai/tool/model.py index f20edae00d..ecfc23e0f6 100644 --- a/src/bonsai/bonsai/tool/model.py +++ b/src/bonsai/bonsai/tool/model.py @@ -1404,7 +1404,7 @@ class Model(bonsai.core.tool.Model): tool.Ifc.get(), pset=pset, properties={"Data": json_data, "Parent": parent_element.GlobalId} ) - parent_obj.select_set(True) + tool.Blender.set_object_selection(parent_obj, True) @classmethod def mirror_parent_void_fillings_to_children( diff --git a/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py b/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py index 4f7f5eabec..7079fe48a4 100644 --- a/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py +++ b/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py @@ -537,7 +537,7 @@ class TestSelectOnlyParent(NewFile): child_obj = tool.Ifc.get_object(child_element) child_obj.select_set(True) - tool.Array.select_only_parent(obj, element, bpy.context) + tool.Array.select_only_parent(obj, bpy.context) assert obj in bpy.context.selected_objects assert bpy.context.view_layer.objects.active is obj From c01433cb6c16679131ff7eb83813015e99a054cb Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Wed, 8 Jul 2026 15:02:21 +0200 Subject: [PATCH 5/5] Bonsai: spec typed test doubles for Blender + dataclass mocks Convert bare Mock() to Mock(spec=bpy.types.Object) for Blender-object stand-ins in TestRecalculateWallsWithNewConnections, TestMEPActionGuards, and TestRecreateAggregateIteratesAllNew so typos on the Blender API fail loudly instead of silently returning a MagicMock. Replace the ad-hoc Mock() ConnectionRecord stand-in in TestRecreateConnectionsZipsPairs with a real ConnectionRecord instance, which pins field names at construction and catches drift if the dataclass fields ever get renamed. IFC entity mocks remain bare Mock() intentionally: entity_instance attributes are schema-driven at runtime rather than defined statically on the class, so spec= would refuse the .GlobalId / .HasFillings / .ConnectedTo attribute writes the tests need. Relates to #8088. Generated with the assistance of an AI coding tool. --- .../model/test_array_duplicate_batched.py | 41 ++++++++++--------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py b/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py index 7079fe48a4..d8dc931336 100644 --- a/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py +++ b/src/bonsai/test/bim/module/model/test_array_duplicate_batched.py @@ -267,7 +267,7 @@ class TestRecreateAggregateIteratesAllNew(NewFile): ), patch("bonsai.core.aggregate.assign_object") as assign_mock, patch( "ifcopenshell.util.element.get_pset", return_value=None ), patch.object( - tool.Ifc, "get_object", side_effect=lambda e: Mock() + tool.Ifc, "get_object", side_effect=lambda e: Mock(spec=bpy.types.Object) ), patch.object( tool.Blender, "select_and_activate_single_object" ): @@ -291,7 +291,7 @@ class TestRecreateAggregateIteratesAllNew(NewFile): "ifcopenshell.util.element.get_aggregate", side_effect=lambda e: old_parent_aggregate if e is old_assembly else None, ), patch("bonsai.core.aggregate.unassign_object") as unassign_mock, patch.object( - tool.Ifc, "get_object", side_effect=lambda e: Mock() + tool.Ifc, "get_object", side_effect=lambda e: Mock(spec=bpy.types.Object) ): tool.Root.recreate_aggregate(old_to_new) @@ -309,14 +309,17 @@ class TestRecreateConnectionsZipsPairs(NewFile): def _make_connection_data(self): from unittest.mock import Mock - data = Mock() - data.relating_element = Mock() - data.related_element = Mock() - data.relating_connection_type = "ATSTART" - data.related_connection_type = "ATEND" - data.relating_priorities = [] - data.related_priorities = [] - return data + from bonsai.tool.duplicate import ConnectionRecord + + return ConnectionRecord( + type="path", + relating_element=Mock(), + related_element=Mock(), + relating_connection_type="ATSTART", + related_connection_type="ATEND", + relating_priorities=[], + related_priorities=[], + ) def test_zips_n_pairs_when_both_sides_duplicated(self): from unittest.mock import Mock @@ -385,7 +388,7 @@ class TestRecalculateWallsWithNewConnections(NewFile): wall_new.ConnectedTo = [Mock()] wall_new.ConnectedFrom = [] - wall_obj = Mock() + wall_obj = Mock(spec=bpy.types.Object) old_to_new = {Mock(): [wall_new]} with patch.object(tool.Ifc, "get_object", return_value=wall_obj), patch.object( @@ -406,7 +409,7 @@ class TestRecalculateWallsWithNewConnections(NewFile): old_to_new = {Mock(): [wall_new]} - with patch.object(tool.Ifc, "get_object", return_value=Mock()), patch.object( + with patch.object(tool.Ifc, "get_object", return_value=Mock(spec=bpy.types.Object)), patch.object( tool.Model, "recalculate_walls" ) as recalc_mock: tool.Geometry._recalculate_walls_with_new_connections(old_to_new) @@ -422,7 +425,7 @@ class TestRecalculateWallsWithNewConnections(NewFile): old_to_new = {Mock(): [actuator_new]} - with patch.object(tool.Ifc, "get_object", return_value=Mock()), patch.object( + with patch.object(tool.Ifc, "get_object", return_value=Mock(spec=bpy.types.Object)), patch.object( tool.Model, "recalculate_walls" ) as recalc_mock: tool.Geometry._recalculate_walls_with_new_connections(old_to_new) @@ -441,7 +444,7 @@ class TestRecalculateWallsWithNewConnections(NewFile): wall_b_new.ConnectedTo = [] wall_b_new.ConnectedFrom = [Mock()] - objs = {wall_a_new: Mock(), wall_b_new: Mock()} + objs = {wall_a_new: Mock(spec=bpy.types.Object), wall_b_new: Mock(spec=bpy.types.Object)} old_to_new = {Mock(): [wall_a_new], Mock(): [wall_b_new]} with patch.object(tool.Ifc, "get_object", side_effect=lambda e: objs.get(e)), patch.object( @@ -464,7 +467,7 @@ class TestMEPActionGuardsAgainstArrayChildren(NewFile): from bonsai.bim.module.model.mep import _active_is_flow_segment - obj = Mock() + obj = Mock(spec=bpy.types.Object) element = Mock() element.is_a = lambda c: c == "IfcFlowSegment" @@ -478,7 +481,7 @@ class TestMEPActionGuardsAgainstArrayChildren(NewFile): from bonsai.bim.module.model.mep import _active_is_flow_segment - obj = Mock() + obj = Mock(spec=bpy.types.Object) element = Mock() element.is_a = lambda c: c == "IfcFlowSegment" @@ -492,7 +495,7 @@ class TestMEPActionGuardsAgainstArrayChildren(NewFile): from bonsai.bim.module.model.mep import _active_is_bend_fitting - obj = Mock() + obj = Mock(spec=bpy.types.Object) element = Mock() with patch.object(tool.Ifc, "get_entity", return_value=element), patch( @@ -505,8 +508,8 @@ class TestMEPActionGuardsAgainstArrayChildren(NewFile): from bonsai.bim.module.model.mep import _n_mep_selected - obj_a = Mock() - obj_b = Mock() + obj_a = Mock(spec=bpy.types.Object) + obj_b = Mock(spec=bpy.types.Object) element_a = Mock() element_b = Mock()