From e7e7b22fd5c475fba9854daa5de7adc4e6faf587 Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Fri, 2 Jul 2021 19:10:51 +1000 Subject: [PATCH] Implement undo support for all root operations. Clean up implementation, making passing transaction keys unnecessary. See #1475. --- src/blenderbim/blenderbim/bim/handler.py | 4 ++ src/blenderbim/blenderbim/bim/ifc.py | 42 +++++++++++------ .../bim/module/aggregate/operator.py | 12 ++--- .../bim/module/attribute/operator.py | 1 - .../bim/module/geometry/operator.py | 1 - .../blenderbim/bim/module/project/operator.py | 47 ++++++++----------- .../blenderbim/bim/module/root/operator.py | 23 +++++++-- .../docs/blenderbim/undo_system.rst | 41 ++++++++-------- 8 files changed, 92 insertions(+), 79 deletions(-) diff --git a/src/blenderbim/blenderbim/bim/handler.py b/src/blenderbim/blenderbim/bim/handler.py index 28eb81fcd0..515f3cf504 100644 --- a/src/blenderbim/blenderbim/bim/handler.py +++ b/src/blenderbim/blenderbim/bim/handler.py @@ -33,6 +33,10 @@ def mode_callback(obj, data): def name_callback(obj, data): + try: + oby.type + except: + return # In case the object RNA is gone during an undo / redo operation # Blender material names are up to 63 UTF-8 bytes if not obj.BIMObjectProperties.ifc_definition_id or "/" not in obj.name or len(bytes(obj.name, "utf-8")) >= 63: return diff --git a/src/blenderbim/blenderbim/bim/ifc.py b/src/blenderbim/blenderbim/bim/ifc.py index fee0af8f7f..fb6b5a91aa 100644 --- a/src/blenderbim/blenderbim/bim/ifc.py +++ b/src/blenderbim/blenderbim/bim/ifc.py @@ -16,6 +16,7 @@ class IfcStore: library_path = "" library_file = None element_listeners = set() + current_transaction = "" last_transaction = "" history = [] future = [] @@ -99,7 +100,7 @@ class IfcStore: if IfcStore.history: data = {"id": element.id(), "guid": getattr(element, "GlobalId", None), "obj": obj.name} - IfcStore.history[-1]["transactions"].append( + IfcStore.history[-1]["operations"].append( {"rollback": IfcStore.rollback_link_element, "commit": IfcStore.commit_link_element, "data": data} ) @@ -111,9 +112,12 @@ class IfcStore: @staticmethod def commit_link_element(data): - IfcStore.id_map[data["id"]] = bpy.data.objects.get(data["obj"]) + obj = bpy.data.objects.get(data["obj"]) + IfcStore.id_map[data["id"]] = obj if data["guid"]: - IfcStore.guid_map[data["guid"]] = bpy.data.objects.get(data["obj"]) + IfcStore.guid_map[data["guid"]] = obj + blenderbim.bim.handler.subscribe_to(obj, "mode", blenderbim.bim.handler.mode_callback) + blenderbim.bim.handler.subscribe_to(obj, "name", blenderbim.bim.handler.name_callback) @staticmethod def unlink_element(element=None, obj=None): @@ -142,20 +146,24 @@ class IfcStore: @staticmethod def execute_ifc_operator(operator, context): - is_top_level_operator = not bool(operator.transaction_key) + is_top_level_operator = not bool(IfcStore.current_transaction) if is_top_level_operator: + IfcStore.begin_transaction(operator) IfcStore.get_file().begin_transaction() # This empty transaction ensures that each operator has at least one transaction - IfcStore.add_transaction(operator, rollback=lambda data: True, commit=lambda data: True) + IfcStore.add_transaction_operation(operator, rollback=lambda data: True, commit=lambda data: True) + else: + operator.transaction_key = IfcStore.current_transaction result = getattr(operator, "_execute")(context) if is_top_level_operator: IfcStore.get_file().end_transaction() - IfcStore.add_transaction( + IfcStore.add_transaction_operation( operator, rollback=IfcStore.rollback_ifc_operator, commit=IfcStore.commit_ifc_operator ) + IfcStore.end_transaction(operator) return result @@ -170,13 +178,17 @@ class IfcStore: blenderbim.bim.handler.purge_module_data() @staticmethod - def generate_transaction_key(operator): - if not getattr(operator, "transaction_key", None): - setattr(operator, "transaction_key", str(uuid.uuid4())) + def begin_transaction(operator): + IfcStore.current_transaction = str(uuid.uuid4()) + operator.transaction_key = IfcStore.current_transaction @staticmethod - def add_transaction(operator, rollback=None, commit=None): - IfcStore.generate_transaction_key(operator) + def end_transaction(operator): + IfcStore.current_transaction = "" + operator.transaction_key = "" + + @staticmethod + def add_transaction_operation(operator, rollback=None, commit=None): key = getattr(operator, "transaction_key", None) data = getattr(operator, "transaction_data", None) bpy.context.scene.BIMProperties.last_transaction = key @@ -184,10 +196,10 @@ class IfcStore: rollback = rollback or getattr(operator, "rollback", lambda data: True) commit = commit or getattr(operator, "commit", lambda data: True) if IfcStore.history and IfcStore.history[-1]["key"] == key: - IfcStore.history[-1]["transactions"].append({"rollback": rollback, "commit": commit, "data": data}) + IfcStore.history[-1]["operations"].append({"rollback": rollback, "commit": commit, "data": data}) else: IfcStore.history.append( - {"key": key, "transactions": [{"rollback": rollback, "commit": commit, "data": data}]} + {"key": key, "operations": [{"rollback": rollback, "commit": commit, "data": data}]} ) IfcStore.future = [] @@ -196,7 +208,7 @@ class IfcStore: if not IfcStore.history: return event = IfcStore.history.pop() - for transaction in event["transactions"][::-1]: + for transaction in event["operations"][::-1]: transaction["rollback"](transaction["data"]) IfcStore.future.append(event) @@ -205,6 +217,6 @@ class IfcStore: if not IfcStore.future: return event = IfcStore.future.pop() - for transaction in event["transactions"]: + for transaction in event["operations"]: transaction["commit"](transaction["data"]) IfcStore.history.append(event) diff --git a/src/blenderbim/blenderbim/bim/module/aggregate/operator.py b/src/blenderbim/blenderbim/bim/module/aggregate/operator.py index 941210aaa6..fe85844eb4 100644 --- a/src/blenderbim/blenderbim/bim/module/aggregate/operator.py +++ b/src/blenderbim/blenderbim/bim/module/aggregate/operator.py @@ -8,7 +8,6 @@ class AssignObject(bpy.types.Operator): bl_idname = "bim.assign_object" bl_label = "Assign Object" bl_options = {"REGISTER", "UNDO"} - transaction_key: bpy.props.StringProperty() relating_object: bpy.props.StringProperty() related_object: bpy.props.StringProperty() @@ -34,7 +33,7 @@ class AssignObject(bpy.types.Operator): "relating_object": self.file.by_id(relating_object.BIMObjectProperties.ifc_definition_id), }, ) - bpy.ops.bim.edit_object_placement(transaction_key=self.transaction_key, obj=related_object.name) + bpy.ops.bim.edit_object_placement(obj=related_object.name) Data.load(IfcStore.get_file(), oprops.ifc_definition_id) bpy.ops.bim.disable_editing_aggregate(obj=related_object.name) @@ -89,7 +88,6 @@ class AddAggregate(bpy.types.Operator): bl_idname = "bim.add_aggregate" bl_label = "Add Aggregate" bl_options = {"REGISTER", "UNDO"} - transaction_key: bpy.props.StringProperty() obj: bpy.props.StringProperty() def execute(self, context): @@ -101,10 +99,6 @@ class AddAggregate(bpy.types.Operator): bpy.context.scene.collection.children.link(aggregate_collection) aggregate = bpy.data.objects.new("Assembly", None) aggregate_collection.objects.link(aggregate) - bpy.ops.bim.assign_class( - transaction_key=self.transaction_key, obj=aggregate.name, ifc_class="IfcElementAssembly" - ) - bpy.ops.bim.assign_object( - transaction_key=self.transaction_key, related_object=obj.name, relating_object=aggregate.name - ) + bpy.ops.bim.assign_class(obj=aggregate.name, ifc_class="IfcElementAssembly") + bpy.ops.bim.assign_object(related_object=obj.name, relating_object=aggregate.name) return {"FINISHED"} diff --git a/src/blenderbim/blenderbim/bim/module/attribute/operator.py b/src/blenderbim/blenderbim/bim/module/attribute/operator.py index 9d05731d3c..8d0432da77 100644 --- a/src/blenderbim/blenderbim/bim/module/attribute/operator.py +++ b/src/blenderbim/blenderbim/bim/module/attribute/operator.py @@ -64,7 +64,6 @@ class EditAttributes(bpy.types.Operator): bl_idname = "bim.edit_attributes" bl_label = "Edit Attributes" bl_options = {"REGISTER", "UNDO"} - transaction_key: bpy.props.StringProperty() obj: bpy.props.StringProperty() obj_type: bpy.props.StringProperty() diff --git a/src/blenderbim/blenderbim/bim/module/geometry/operator.py b/src/blenderbim/blenderbim/bim/module/geometry/operator.py index 0e1dbb1c29..0b5cd5f698 100644 --- a/src/blenderbim/blenderbim/bim/module/geometry/operator.py +++ b/src/blenderbim/blenderbim/bim/module/geometry/operator.py @@ -18,7 +18,6 @@ class EditObjectPlacement(bpy.types.Operator): bl_idname = "bim.edit_object_placement" bl_label = "Edit Object Placement" bl_options = {"REGISTER", "UNDO"} - transaction_key: bpy.props.StringProperty() obj: bpy.props.StringProperty() def execute(self, context): diff --git a/src/blenderbim/blenderbim/bim/module/project/operator.py b/src/blenderbim/blenderbim/bim/module/project/operator.py index f63ae42cef..f01e937fdb 100644 --- a/src/blenderbim/blenderbim/bim/module/project/operator.py +++ b/src/blenderbim/blenderbim/bim/module/project/operator.py @@ -13,14 +13,14 @@ class CreateProject(bpy.types.Operator): bl_idname = "bim.create_project" bl_label = "Create Project" bl_options = {"REGISTER", "UNDO"} - transaction_key: bpy.props.StringProperty() def execute(self, context): - IfcStore.generate_transaction_key(self) - IfcStore.add_transaction(self, rollback=self.rollback, commit=lambda data: True) + IfcStore.begin_transaction(self) + IfcStore.add_transaction_operation(self, rollback=self.rollback, commit=lambda data: True) result = self._execute(context) self.transaction_data = {"file": self.file} - IfcStore.add_transaction(self, rollback=lambda data: True, commit=self.commit) + IfcStore.add_transaction_operation(self, rollback=lambda data: True, commit=self.commit) + IfcStore.end_transaction(self) return result def _execute(self, context): @@ -41,7 +41,7 @@ class CreateProject(bpy.types.Operator): building = bpy.data.objects.new("My Building", None) building_storey = bpy.data.objects.new("Ground Floor", None) - bpy.ops.bim.assign_class(transaction_key=self.transaction_key, obj=project.name, ifc_class="IfcProject") + bpy.ops.bim.assign_class(obj=project.name, ifc_class="IfcProject") bpy.ops.bim.assign_unit() bpy.ops.bim.add_subcontext(context="Model") bpy.ops.bim.add_subcontext(context="Model", subcontext="Body", target_view="MODEL_VIEW") @@ -53,14 +53,12 @@ class CreateProject(bpy.types.Operator): ifcopenshell.util.representation.get_context(self.file, "Model", "Body", "MODEL_VIEW").id() ) - bpy.ops.bim.assign_class(transaction_key=self.transaction_key, obj=site.name, ifc_class="IfcSite") - bpy.ops.bim.assign_class(transaction_key=self.transaction_key, obj=building.name, ifc_class="IfcBuilding") - bpy.ops.bim.assign_class( - transaction_key=self.transaction_key, obj=building_storey.name, ifc_class="IfcBuildingStorey" - ) - bpy.ops.bim.assign_object(transaction_key=self.transaction_key, related_object=site.name, relating_object=project.name) - bpy.ops.bim.assign_object(transaction_key=self.transaction_key, related_object=building.name, relating_object=site.name) - bpy.ops.bim.assign_object(transaction_key=self.transaction_key, related_object=building_storey.name, relating_object=building.name) + bpy.ops.bim.assign_class(obj=site.name, ifc_class="IfcSite") + bpy.ops.bim.assign_class(obj=building.name, ifc_class="IfcBuilding") + bpy.ops.bim.assign_class(obj=building_storey.name, ifc_class="IfcBuildingStorey") + bpy.ops.bim.assign_object(related_object=site.name, relating_object=project.name) + bpy.ops.bim.assign_object(related_object=building.name, relating_object=site.name) + bpy.ops.bim.assign_object(related_object=building_storey.name, relating_object=building.name) return {"FINISHED"} @@ -77,14 +75,14 @@ class CreateProjectLibrary(bpy.types.Operator): bl_idname = "bim.create_project_library" bl_label = "Create Project Library" bl_options = {"REGISTER", "UNDO"} - transaction_key: bpy.props.StringProperty() def execute(self, context): - IfcStore.generate_transaction_key(self) - IfcStore.add_transaction(self, rollback=self.rollback, commit=lambda data: True) + IfcStore.begin_transaction(self) + IfcStore.add_transaction_operation(self, rollback=self.rollback, commit=lambda data: True) result = self._execute(context) self.transaction_data = {"file": self.file} - IfcStore.add_transaction(self, rollback=lambda data: True, commit=self.commit) + IfcStore.add_transaction_operation(self, rollback=lambda data: True, commit=self.commit) + IfcStore.end_transaction(self) return result def _execute(self, context): @@ -102,9 +100,7 @@ class CreateProjectLibrary(bpy.types.Operator): bpy.ops.bim.add_organisation() project_library = bpy.data.objects.new("My Project Library", None) - bpy.ops.bim.assign_class( - transaction_key=self.transaction_key, obj=project_library.name, ifc_class="IfcProjectLibrary" - ) + bpy.ops.bim.assign_class(obj=project_library.name, ifc_class="IfcProjectLibrary") bpy.ops.bim.assign_unit() return {"FINISHED"} @@ -128,7 +124,7 @@ class SelectLibraryFile(bpy.types.Operator): old_filepath = IfcStore.library_path result = self._execute(context) self.transaction_data = {"old_filepath": old_filepath, "filepath": self.filepath} - IfcStore.add_transaction(self) + IfcStore.add_transaction_operation(self) return result def _execute(self, context): @@ -230,14 +226,13 @@ class AssignLibraryDeclaration(bpy.types.Operator): bl_idname = "bim.assign_library_declaration" bl_label = "Assign Library Declaration" bl_options = {"REGISTER", "UNDO"} - transaction_key: bpy.props.StringProperty() definition: bpy.props.IntProperty() def execute(self, context): IfcStore.library_file.begin_transaction() result = self._execute(context) IfcStore.library_file.end_transaction() - IfcStore.add_transaction(self) + IfcStore.add_transaction_operation(self) return result def _execute(self, context): @@ -265,14 +260,13 @@ class UnassignLibraryDeclaration(bpy.types.Operator): bl_idname = "bim.unassign_library_declaration" bl_label = "Unassign Library Declaration" bl_options = {"REGISTER", "UNDO"} - transaction_key: bpy.props.StringProperty() definition: bpy.props.IntProperty() def execute(self, context): IfcStore.library_file.begin_transaction() result = self._execute(context) IfcStore.library_file.end_transaction() - IfcStore.add_transaction(self) + IfcStore.add_transaction_operation(self) return result def _execute(self, context): @@ -309,7 +303,6 @@ class AppendLibraryElement(bpy.types.Operator): bl_idname = "bim.append_library_element" bl_label = "Append Library Element" bl_options = {"REGISTER", "UNDO"} - transaction_key: bpy.props.StringProperty() definition: bpy.props.IntProperty() def execute(self, context): @@ -389,7 +382,7 @@ class EditHeader(bpy.types.Operator): self.transaction_data["old"] = self.record_state() result = self._execute(context) self.transaction_data["new"] = self.record_state() - IfcStore.add_transaction(self) + IfcStore.add_transaction_operation(self) return result def _execute(self, context): diff --git a/src/blenderbim/blenderbim/bim/module/root/operator.py b/src/blenderbim/blenderbim/bim/module/root/operator.py index 9797a88dc6..f6bffb0e3d 100644 --- a/src/blenderbim/blenderbim/bim/module/root/operator.py +++ b/src/blenderbim/blenderbim/bim/module/root/operator.py @@ -12,6 +12,7 @@ from blenderbim.bim.ifc import IfcStore class EnableReassignClass(bpy.types.Operator): bl_idname = "bim.enable_reassign_class" bl_label = "Enable Reassign IFC Class" + bl_options = {"REGISTER", "UNDO"} def execute(self, context): obj = bpy.context.active_object @@ -41,6 +42,7 @@ class EnableReassignClass(bpy.types.Operator): class DisableReassignClass(bpy.types.Operator): bl_idname = "bim.disable_reassign_class" bl_label = "Disable Reassign IFC Class" + bl_options = {"REGISTER", "UNDO"} def execute(self, context): bpy.context.active_object.BIMObjectProperties.is_reassigning_class = False @@ -50,9 +52,13 @@ class DisableReassignClass(bpy.types.Operator): class ReassignClass(bpy.types.Operator): bl_idname = "bim.reassign_class" bl_label = "Reassign IFC Class" + bl_options = {"REGISTER", "UNDO"} obj: bpy.props.StringProperty() def execute(self, context): + return IfcStore.execute_ifc_operator(self, context) + + def _execute(self, context): objects = [bpy.data.objects.get(self.obj)] if self.obj else bpy.context.selected_objects self.file = IfcStore.get_file() predefined_type = bpy.context.scene.BIMRootProperties.ifc_predefined_type @@ -78,7 +84,6 @@ class AssignClass(bpy.types.Operator): bl_idname = "bim.assign_class" bl_label = "Assign IFC Class" bl_options = {"REGISTER", "UNDO"} - transaction_key: bpy.props.StringProperty() obj: bpy.props.StringProperty() ifc_class: bpy.props.StringProperty() predefined_type: bpy.props.StringProperty() @@ -160,9 +165,7 @@ class AssignClass(bpy.types.Operator): collection.objects.link(obj) if parent_collection: parent_collection.children.link(collection) - bpy.ops.bim.assign_object( - transaction_key=self.transaction_key, related_object=obj.name, relating_object=parent_collection.name - ) + bpy.ops.bim.assign_object(related_object=obj.name, relating_object=parent_collection.name) else: bpy.context.scene.collection.children.link(collection) @@ -181,9 +184,13 @@ class AssignClass(bpy.types.Operator): class UnassignClass(bpy.types.Operator): bl_idname = "bim.unassign_class" bl_label = "Unassign IFC Class" + bl_options = {"REGISTER", "UNDO"} obj: bpy.props.StringProperty() def execute(self, context): + return IfcStore.execute_ifc_operator(self, context) + + def _execute(self, context): self.file = IfcStore.get_file() if self.obj: objects = [bpy.data.objects.get(self.obj)] @@ -220,9 +227,13 @@ class UnassignClass(bpy.types.Operator): class UnlinkObject(bpy.types.Operator): bl_idname = "bim.unlink_object" bl_label = "Unlink Object" + bl_options = {"REGISTER", "UNDO"} obj: bpy.props.StringProperty() def execute(self, context): + return IfcStore.execute_ifc_operator(self, context) + + def _execute(self, context): self.file = IfcStore.get_file() if self.obj: objects = [bpy.data.objects.get(self.obj)] @@ -239,9 +250,13 @@ class UnlinkObject(bpy.types.Operator): class CopyClass(bpy.types.Operator): bl_idname = "bim.copy_class" bl_label = "Copy Class" + bl_options = {"REGISTER", "UNDO"} obj: bpy.props.StringProperty() def execute(self, context): + return IfcStore.execute_ifc_operator(self, context) + + def _execute(self, context): self.file = IfcStore.get_file() if self.obj: objects = [bpy.data.objects.get(self.obj)] diff --git a/src/blenderbim/docs/blenderbim/undo_system.rst b/src/blenderbim/docs/blenderbim/undo_system.rst index 84e750c7d4..1bc389b384 100644 --- a/src/blenderbim/docs/blenderbim/undo_system.rst +++ b/src/blenderbim/docs/blenderbim/undo_system.rst @@ -48,7 +48,7 @@ want. Since IfcOpenShell has no interface, you manually run code like ``model.undo()`` and ``model.redo()`` to undo and redo. This scenario where there is pure IfcOpenShell never occurs with the BlenderBIM -Add-on, so let's put both of them together. +Add-on. Instead, stuff happens in Blender operators. .. code-block:: python :emphasize-lines: 5,7,8,10,13 @@ -57,7 +57,6 @@ Add-on, so let's put both of them together. bl_idname = "foobar" bl_label = "Foobar" bl_options = {"REGISTER", "UNDO"} - transaction_key: bpy.props.StringProperty() def execute(self, context): return IfcStore.execute_ifc_operator(self, context) @@ -65,25 +64,22 @@ Add-on, so let's put both of them together. def _execute(self, context): ifcopenshell.api.run("foo.bar", IfcStore.get_file()) context.scene.name = "Foobar" - bpy.ops.foobaz(transaction_key=self.transaction_key) return {"FINISHED"} When your operator manipulates (creates, removes, or edits) IFC data directly or -indirectly (i.e. through calling another operator), your operator must have a -``transaction_key`` property assigned. The purpose of this transaction key is so -that if the operator is ever called from another operator, it can identify that -changes in the IFC data belong to a parent transaction. In this example, it is -also calling another operator ``bpy.ops.foobaz``, which manipulates IFC as well, -so we have to pass along the transaction key so ``bpy.ops.foobaz`` gets treated -as a sub-transaction. +indirectly (i.e. through calling another operator), your operator must be +wrapped in an ``IfcStore.execute_ifc_operator`` call. This wrapper will: -In this example, we call ``return IfcStore.execute_ifc_operator(self, context)`` -in the ``execute`` function. This is a special wrapper which begins an IFC -transaction, runs your operator's ``_execute`` function, then ends the IFC -transaction. This wrapper ensures that any IFC data manipulations within your -actual ``_execute`` function gets captured in a single transaction. In addition -to tracking all changes in the IFC data, it also tracks changes in the -``id_map`` and ``guid_map``. +1. Begin a BlenderBIM Add-on transaction +2. Begin an IfcOpenShell transaction +3. Runs your operator's ``_execute``. +4. Ends the IfcOpenShell transaction +5. Ends the BlenderBIM Add-on transaction + +The IfcOpenShell transaction keeps track of IFC data changes, and the BlenderBIM +Add-on transaction keeps track of all other custom data changes, like changes in +the ``id_map`` and ``guid_map``. For the vast majority of operations, this +wrapper provides everything that you need. If, however, your operator manipulates data that is not tracked by Blender, is not tracked in the IFC data, and is not tracked in the element map, then you @@ -96,14 +92,15 @@ operator. Here is an example. bl_idname = "foobar" bl_label = "Foobar" bl_options = {"REGISTER", "UNDO"} - transaction_key: bpy.props.StringProperty() def execute(self, context): + IfcStore.begin_transaction(operator) old_value = Foo.bar result = self._execute(context) new_value = Foo.bar self.transaction_data = {"old_value": old_value, "new_value": new_value} - IfcStore.add_transaction(self) + IfcStore.add_transaction_operation(self) + IfcStore.end_transaction(operator) return result def _execute(self, context): @@ -116,6 +113,6 @@ operator. Here is an example. def commit(self, data): Foo.baz = data["new_value"] -Note that there is a distinction between ``execute`` and ``_execute``. This -recommended convention allows you to quickly discern undo state tracking code -from regular operation code. +Note that there is still a distinction between ``execute`` and ``_execute``. +This recommended convention allows you to quickly discern undo state tracking +code from regular operation code.