mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-08-09 17:31:45 +00:00
Swap merge convention to active-is-survivor
bim.merge_wall now consumes the non-active selection into the active one — matching Blender's OBJECT_OT_join (Ctrl+J) and MESH_OT_merge "at last" convention. The wall the user clicks last absorbs the other; users following Blender muscle-memory get the result they expect. DumbWallJoiner.merge is already structurally asymmetric (wall1 = survivor); only the caller in MergeWall._perform needed flipping. Audit confirmed the previous call site was the sole caller of DumbWallJoiner.merge in production code. Drive-by tidies on adjacent code: collapse two over-length comprehensions under black's 120-char budget, and switch ``any(True for _ in gen)`` to ``any(gen)`` since the iterable yields tuples that are always truthy. Generated with the assistance of an AI coding tool.
This commit is contained in:
@@ -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):
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 <http://www.gnu.org/licenses/>.
|
||||
#
|
||||
# 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])
|
||||
Reference in New Issue
Block a user