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: