diff --git a/src/bonsai/bonsai/bim/module/model/wall.py b/src/bonsai/bonsai/bim/module/model/wall.py index 428c999274..5ab2233a08 100644 --- a/src/bonsai/bonsai/bim/module/model/wall.py +++ b/src/bonsai/bonsai/bim/module/model/wall.py @@ -150,7 +150,7 @@ def _slab_connection_gizmo_poll_gate(context: bpy.types.Context, *, require_edit return False if require_editing and not tool.Model.get_slab_props(active).is_editing: return False - return any(True for _ in tool.Wall.iter_slab_wall_connections(element)) + return any(tool.Wall.iter_slab_wall_connections(element)) def _wall_topology_gizmo_poll_gate(context: bpy.types.Context) -> bool: @@ -739,12 +739,13 @@ class MergeWall(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.Ifc.Operat active_obj = context.active_object assert active_obj selected_objs = tool.Model.get_selected_mesh_objects() - # The merge deletes the second argument when the walls are collinear; - # only the first survives, so the resync targets the non-active wall. - surviving_obj = next(o for o in selected_objs if o != active_obj) - DumbWallJoiner().merge(surviving_obj, active_obj) - _maybe_resync_wall_props_from_ifc(surviving_obj) - _regenerate_walls([surviving_obj]) + # Active-is-survivor — matches Blender's Ctrl+J / "merge at last" + # convention so the wall a user clicks last absorbs the other. + # DumbWallJoiner.merge deletes its second argument. + other_obj = next(o for o in selected_objs if o != active_obj) + DumbWallJoiner().merge(active_obj, other_obj) + _maybe_resync_wall_props_from_ifc(active_obj) + _regenerate_walls([active_obj]) return {"FINISHED"} @@ -4229,9 +4230,7 @@ class GizmoSlabEdition(bpy.types.GizmoGroup, gizmo.BaseParametricGizmoGroup): @classmethod def is_element_type(cls, element: ifcopenshell.entity_instance) -> bool: - return tool.Parametric.is_slab(element) and any( - True for _ in tool.Wall.iter_slab_wall_connections(element) - ) + return tool.Parametric.is_slab(element) and any(tool.Wall.iter_slab_wall_connections(element)) class GizmoPairDisconnect(bpy.types.GizmoGroup, gizmo.BillboardingGizmoGroupMixin): diff --git a/src/bonsai/bonsai/tool/model.py b/src/bonsai/bonsai/tool/model.py index 87d8fd4210..bc6f49063b 100644 --- a/src/bonsai/bonsai/tool/model.py +++ b/src/bonsai/bonsai/tool/model.py @@ -933,9 +933,7 @@ class Model(bonsai.core.tool.Model): if not representation: return False chain = cls.get_booleans(wall, representation) - to_remove = [ - b for b in chain if (sec := b.SecondOperand) is not None and sec.is_a("IfcTessellatedFaceSet") - ] + to_remove = [b for b in chain if (sec := b.SecondOperand) is not None and sec.is_a("IfcTessellatedFaceSet")] for b in to_remove: tool.Geometry.remove_representation_item(b.SecondOperand, wall) # Sweep the now-stale BBIM_Boolean entries on the copy (their ids point diff --git a/src/bonsai/test/bim/module/model/test_merge_wall_convention.py b/src/bonsai/test/bim/module/model/test_merge_wall_convention.py new file mode 100644 index 0000000000..6447b8541b --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_merge_wall_convention.py @@ -0,0 +1,87 @@ +# 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. + +"""Pins the active-is-survivor merge convention. + +``bim.merge_wall`` must consume the non-active selection into the active +one — matching Blender's ``OBJECT_OT_join`` / ``MESH_OT_merge`` "at +last" convention. Users following Ctrl+J muscle-memory click the +surviving wall last; the operator must align with that expectation.""" + +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import pytest + +pytestmark = pytest.mark.wall + + +def _run_perform(active, other): + """Invoke ``MergeWall._perform`` as an unbound function with the + two wall stubs in the selection, patching the heavy IFC / Blender + side effects. Returns the ``(merger_arg_1, merger_arg_2)`` actually + passed to ``DumbWallJoiner.merge``.""" + from bonsai.bim.module.model.wall import MergeWall + + context = SimpleNamespace(active_object=active) + captured_call = {} + + def _capture_merge(self, a, b): + captured_call["wall1"] = a + captured_call["wall2"] = b + + fake_self = SimpleNamespace() + + with ( + patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=MagicMock(name="ifc_file")), + patch("bonsai.bim.module.model.wall.tool.Model.get_selected_mesh_objects", return_value=[active, other]), + patch("bonsai.bim.module.model.wall.DumbWallJoiner.__init__", return_value=None), + patch("bonsai.bim.module.model.wall.DumbWallJoiner.merge", new=_capture_merge), + patch("bonsai.bim.module.model.wall._maybe_resync_wall_props_from_ifc"), + patch("bonsai.bim.module.model.wall._regenerate_walls") as regen_walls, + ): + result = MergeWall._perform(fake_self, context) + + return captured_call, regen_walls, result + + +def test_active_wall_is_passed_as_survivor_to_merge(): + """The first argument to ``DumbWallJoiner.merge`` is the survivor; + the active object must occupy that slot so the wall the user clicked + last absorbs the other.""" + active = SimpleNamespace(name="active") + other = SimpleNamespace(name="other") + + captured, _regen, _ = _run_perform(active, other) + + assert captured["wall1"] is active + assert captured["wall2"] is other + + +def test_post_merge_resync_targets_active_not_consumed(): + """After the merge ``_regenerate_walls`` rebuilds the survivor's + body. Targeting the consumed wall would crash on a freed ``bpy_struct``; + the survivor (active) is the only valid target.""" + active = SimpleNamespace(name="active") + other = SimpleNamespace(name="other") + + _, regen_walls, _ = _run_perform(active, other) + + regen_walls.assert_called_once_with([active])