Implement undo support for all root operations. Clean up implementation, making passing transaction keys unnecessary. See #1475.

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

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):
+    def begin_transaction(operator):
-        if not getattr(operator, "transaction_key", None):
+        IfcStore.current_transaction = str(uuid.uuid4())
-            setattr(operator, "transaction_key", str(uuid.uuid4()))
+        operator.transaction_key = IfcStore.current_transaction
 
     @staticmethod
-    def add_transaction(operator, rollback=None, commit=None):
+    def end_transaction(operator):
-        IfcStore.generate_transaction_key(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)

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(
+        bpy.ops.bim.assign_class(obj=aggregate.name, ifc_class="IfcElementAssembly")
-            transaction_key=self.transaction_key, obj=aggregate.name, ifc_class="IfcElementAssembly"
+        bpy.ops.bim.assign_object(related_object=obj.name, relating_object=aggregate.name)
-        )
-        bpy.ops.bim.assign_object(
-            transaction_key=self.transaction_key, related_object=obj.name, relating_object=aggregate.name
-        )
         return {"FINISHED"}

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()
 

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):

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.begin_transaction(self)
-        IfcStore.add_transaction(self, rollback=self.rollback, commit=lambda data: True)
+        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(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(obj=building.name, ifc_class="IfcBuilding")
-        bpy.ops.bim.assign_class(
+        bpy.ops.bim.assign_class(obj=building_storey.name, ifc_class="IfcBuildingStorey")
-            transaction_key=self.transaction_key, 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(transaction_key=self.transaction_key, related_object=site.name, relating_object=project.name)
+        bpy.ops.bim.assign_object(related_object=building_storey.name, relating_object=building.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)
 
         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.begin_transaction(self)
-        IfcStore.add_transaction(self, rollback=self.rollback, commit=lambda data: True)
+        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(
+        bpy.ops.bim.assign_class(obj=project_library.name, ifc_class="IfcProjectLibrary")
-            transaction_key=self.transaction_key, 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):

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(
+            bpy.ops.bim.assign_object(related_object=obj.name, relating_object=parent_collection.name)
-                transaction_key=self.transaction_key, 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)]

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
+indirectly (i.e. through calling another operator), your operator must be
-``transaction_key`` property assigned. The purpose of this transaction key is so
+wrapped in an ``IfcStore.execute_ifc_operator`` call. This wrapper will:
-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.
 
-In this example, we call ``return IfcStore.execute_ifc_operator(self, context)``
+1. Begin a BlenderBIM Add-on transaction
-in the ``execute`` function. This is a special wrapper which begins an IFC
+2. Begin an IfcOpenShell transaction
-transaction, runs your operator's ``_execute`` function, then ends the IFC
+3. Runs your operator's ``_execute``.
-transaction. This wrapper ensures that any IFC data manipulations within your
+4. Ends the IfcOpenShell transaction
-actual ``_execute`` function gets captured in a single transaction. In addition
+5. Ends the BlenderBIM Add-on transaction
-to tracking all changes in the IFC data, it also tracks changes in the
+
-``id_map`` and ``guid_map``.
+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
+Note that there is still a distinction between ``execute`` and ``_execute``.
-recommended convention allows you to quickly discern undo state tracking code
+This recommended convention allows you to quickly discern undo state tracking
-from regular operation code.
+code from regular operation code.