mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-08-06 07:51:47 +00:00
Split update_bim_tool_props commit vs selection
tool.Parametric.refresh_post_commit was calling update_bim_tool_props after every IFC mutation. The function does two things — refresh read-only header values (extrusion_depth/length/x_angle) and re-target user-intent enums (ifc_class, relating_type_id) from the active object. Doing both on the commit path crashed on IfcAnnotation actives (the type isn't in the bim_tool ifc_class enum) and silently overwrote the user's "what to build next" choice on every other element. Split the function: update_bim_tool_props remains selection-driven and does both halves; new refresh_bim_tool_headers is header-only and is what refresh_post_commit now calls. Behaviour on selection change is preserved. Also ports the upstream PR #8136 try/except guard onto the props.ifc_class write for the selection-driven path. Adds test_handler_forward_compat.py to pin both contracts via AST. Generated with the assistance of an AI coding tool.
This commit is contained in:
@@ -118,19 +118,13 @@ def active_object_callback():
|
||||
|
||||
|
||||
def update_bim_tool_props():
|
||||
"""update BIM Tools props (such as extrusion_depth, length and x_angle) when active object changes"""
|
||||
obj = bpy.context.active_object
|
||||
|
||||
# bunch of checks to see if we're in a valid state
|
||||
if not obj:
|
||||
return
|
||||
mode = bpy.context.mode
|
||||
current_tool = bpy.context.workspace.tools.from_space_view3d_mode(mode)
|
||||
if not current_tool or current_tool.idname not in tool.Blender.get_list_of_tools():
|
||||
return
|
||||
element = tool.Ifc.get_entity(obj)
|
||||
if not element:
|
||||
"""Selection-driven BIM Tool sync: re-target user-intent enums
|
||||
(ifc_class, relating_type_id) AND refresh header values
|
||||
(extrusion_depth, length, x_angle) for the new active object."""
|
||||
ctx = _resolve_bim_tool_context()
|
||||
if ctx is None:
|
||||
return
|
||||
obj, current_tool, element = ctx
|
||||
|
||||
props = tool.Model.get_model_props()
|
||||
aprops = tool.Drawing.get_annotation_props()
|
||||
@@ -153,7 +147,14 @@ def update_bim_tool_props():
|
||||
return
|
||||
|
||||
if is_bim_tool:
|
||||
props.ifc_class = element_type.is_a()
|
||||
try:
|
||||
props.ifc_class = element_type.is_a()
|
||||
except TypeError:
|
||||
# ifc_class only lists element/space types present in the model, so an
|
||||
# unsupported type (e.g. a raw IfcTypeProduct) or a stale item list mid-
|
||||
# rebuild raises `enum "<class>" not found`. Skip rather than crash the
|
||||
# handler — it re-fires on the next selection and the panel resyncs.
|
||||
pass
|
||||
|
||||
# Only assign when the target enum is the one that lists this type — otherwise
|
||||
# we hit `enum "<id>" not found in (...)` if the user selects an element of a
|
||||
@@ -173,6 +174,43 @@ def update_bim_tool_props():
|
||||
if is_annotation_tool:
|
||||
return
|
||||
|
||||
_read_headers_into_props(obj, element)
|
||||
|
||||
|
||||
def refresh_bim_tool_headers():
|
||||
"""Commit-driven refresh of BIM Tool header values (extrusion_depth,
|
||||
length, x_angle) from the active object's IFC geometry. Must not
|
||||
write user-intent enums — those encode 'what to build next' and
|
||||
would silently reset on every IFC commit."""
|
||||
ctx = _resolve_bim_tool_context()
|
||||
if ctx is None:
|
||||
return
|
||||
obj, current_tool, element = ctx
|
||||
if current_tool.idname == "bim.annotation_tool":
|
||||
return
|
||||
_read_headers_into_props(obj, element)
|
||||
|
||||
|
||||
def _resolve_bim_tool_context():
|
||||
"""Return ``(obj, current_tool, element)`` when an active BIM workspace
|
||||
tool sees a resolvable IFC element; ``None`` otherwise."""
|
||||
obj = bpy.context.active_object
|
||||
if not obj:
|
||||
return None
|
||||
mode = bpy.context.mode
|
||||
current_tool = bpy.context.workspace.tools.from_space_view3d_mode(mode)
|
||||
if not current_tool or current_tool.idname not in tool.Blender.get_list_of_tools():
|
||||
return None
|
||||
element = tool.Ifc.get_entity(obj)
|
||||
if not element:
|
||||
return None
|
||||
return obj, current_tool, element
|
||||
|
||||
|
||||
def _read_headers_into_props(obj, element):
|
||||
"""Populate ``BIMModelProperties`` header values from the active
|
||||
object's IFC extrusion. Enum-safe: writes only header floats, never
|
||||
user-intent enum slots, so it is safe to call on the post-commit hook."""
|
||||
representation = ifcopenshell.util.representation.get_representation(element, "Model", "Body", "MODEL_VIEW")
|
||||
if not representation:
|
||||
return
|
||||
@@ -190,6 +228,7 @@ def update_bim_tool_props():
|
||||
if not AuthoringData.is_loaded:
|
||||
AuthoringData.load()
|
||||
|
||||
props = tool.Model.get_model_props()
|
||||
if AuthoringData.data["active_material_usage"] == "LAYER2":
|
||||
x_angle = get_x_angle(extrusion)
|
||||
axis = tool.Model.get_wall_axis(obj)["reference"]
|
||||
|
||||
@@ -183,11 +183,12 @@ class Parametric(bonsai.core.tool.Parametric):
|
||||
"""Post-commit hook for ``tool.Ifc.Operator``: re-syncs scene-level
|
||||
workspace-tool header fields from current IFC state and bumps the
|
||||
geometry generation counter so caches keyed off it drop stale
|
||||
entries on the next draw."""
|
||||
entries on the next draw. Header-only — user-intent enums are
|
||||
re-targeted on selection change, not here."""
|
||||
import bonsai.bim.handler # late import: bim.handler imports tool.*
|
||||
|
||||
cls._geom_generation += 1
|
||||
bonsai.bim.handler.update_bim_tool_props()
|
||||
bonsai.bim.handler.refresh_bim_tool_headers()
|
||||
tool.Blender.update_all_viewports()
|
||||
|
||||
@classmethod
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
# 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.
|
||||
|
||||
"""Forward-compat AST contracts for ``bonsai.bim.handler``.
|
||||
|
||||
Pins structural invariants on the post-commit refresh path that no
|
||||
behavioural test can catch on its own — specifically, that the commit-
|
||||
driven refresh never writes user-intent enum slots."""
|
||||
|
||||
import ast
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
pytestmark = pytest.mark.model
|
||||
|
||||
|
||||
HANDLER_PATH = Path(__file__).parent.parent.parent / "bonsai" / "bim" / "handler.py"
|
||||
|
||||
# User-intent enums: encode the user's "what to build next" choice on the
|
||||
# BIM Tool panel. Writing them from a commit-driven path silently resets
|
||||
# the user's selection on every IFC mutation — selection-change is the
|
||||
# only legitimate caller.
|
||||
USER_INTENT_ENUM_ATTRS = frozenset({"ifc_class", "relating_type_id"})
|
||||
|
||||
# Functions that must remain free of user-intent enum writes. Both are
|
||||
# reachable from ``tool.Parametric.refresh_post_commit``.
|
||||
ENUM_SAFE_FUNCTIONS = ("refresh_bim_tool_headers", "_read_headers_into_props")
|
||||
|
||||
|
||||
def _function_node(tree: ast.Module, name: str) -> ast.FunctionDef:
|
||||
for node in ast.walk(tree):
|
||||
if isinstance(node, ast.FunctionDef) and node.name == name:
|
||||
return node
|
||||
raise AssertionError(f"{name!r} not found in {HANDLER_PATH.name}")
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
def handler_tree() -> ast.Module:
|
||||
return ast.parse(HANDLER_PATH.read_text(encoding="utf-8"))
|
||||
|
||||
|
||||
@pytest.mark.parametrize("fn_name", ENUM_SAFE_FUNCTIONS)
|
||||
def test_commit_driven_function_does_not_write_user_intent_enums(handler_tree: ast.Module, fn_name: str) -> None:
|
||||
"""The commit-driven refresh path must never assign to user-intent
|
||||
enum slots (``ifc_class``, ``relating_type_id``). Re-targeting these
|
||||
from the post-commit hook silently overwrites the user's BIM Tool
|
||||
panel selection on every IFC mutation; only selection-change callers
|
||||
may write them."""
|
||||
fn = _function_node(handler_tree, fn_name)
|
||||
offenders = []
|
||||
for node in ast.walk(fn):
|
||||
if not isinstance(node, ast.Assign):
|
||||
continue
|
||||
for target in node.targets:
|
||||
if isinstance(target, ast.Attribute) and target.attr in USER_INTENT_ENUM_ATTRS:
|
||||
offenders.append((target.attr, node.lineno))
|
||||
if offenders:
|
||||
msgs = ", ".join(f"{attr} at line {line}" for attr, line in offenders)
|
||||
pytest.fail(
|
||||
f"{fn_name!r} assigns to user-intent enum slot(s): {msgs}. "
|
||||
f"Move this assignment to a selection-driven callback."
|
||||
)
|
||||
|
||||
|
||||
def test_refresh_post_commit_calls_header_only_entrypoint(handler_tree: ast.Module) -> None:
|
||||
"""``tool.Parametric.refresh_post_commit`` must dispatch into
|
||||
``refresh_bim_tool_headers``, not ``update_bim_tool_props``.
|
||||
The latter re-targets user-intent enums; routing the post-commit
|
||||
hook through it silently resets the user's BIM Tool selection on
|
||||
every IFC mutation and crashes on element types absent from the
|
||||
``ifc_class`` enum (e.g. ``IfcAnnotation``)."""
|
||||
parametric_path = HANDLER_PATH.parent.parent / "tool" / "parametric.py"
|
||||
parametric_tree = ast.parse(parametric_path.read_text(encoding="utf-8"))
|
||||
fn = _function_node(parametric_tree, "refresh_post_commit")
|
||||
called_handler_attrs = {
|
||||
node.func.attr
|
||||
for node in ast.walk(fn)
|
||||
if isinstance(node, ast.Call)
|
||||
and isinstance(node.func, ast.Attribute)
|
||||
and isinstance(node.func.value, ast.Attribute)
|
||||
and node.func.value.attr == "handler"
|
||||
}
|
||||
assert (
|
||||
"refresh_bim_tool_headers" in called_handler_attrs
|
||||
), "refresh_post_commit must call bonsai.bim.handler.refresh_bim_tool_headers"
|
||||
assert "update_bim_tool_props" not in called_handler_attrs, (
|
||||
"refresh_post_commit must not call update_bim_tool_props " "(re-targets user-intent enums on every commit)"
|
||||
)
|
||||
Reference in New Issue
Block a user