WIP: Cleanup MachineSettingsAction

3 files changed, 50 insertions, 82 deletions

diff --git a/plugins/MachineSettingsAction/MachineSettingsAction.py b/plugins/MachineSettingsAction/MachineSettingsAction.py
index afd7aac86d..cddc4e5fe8 100755
--- a/plugins/MachineSettingsAction/MachineSettingsAction.py
+++ b/plugins/MachineSettingsAction/MachineSettingsAction.py
@@ -1,16 +1,21 @@
-# Copyright (c) 2017 Ultimaker B.V.
+# Copyright (c) 2019 Ultimaker B.V.
 # Cura is released under the terms of the LGPLv3 or higher.
 
-from PyQt5.QtCore import pyqtProperty, pyqtSignal
+from typing import Optional, TYPE_CHECKING
+
+from PyQt5.QtCore import pyqtProperty
 
 import UM.i18n
 from UM.FlameProfiler import pyqtSlot
-from UM.Application import Application
 from UM.Settings.ContainerRegistry import ContainerRegistry
 from UM.Settings.DefinitionContainer import DefinitionContainer
 
 from cura.MachineAction import MachineAction
 from cura.Settings.CuraStackBuilder import CuraStackBuilder
+from cura.Settings.cura_empty_instance_containers import isEmptyContainer
+
+if TYPE_CHECKING:
+    from PyQt5.QtCore import QObject
 
 catalog = UM.i18n.i18nCatalog("cura")
 
@@ -18,139 +23,102 @@ catalog = UM.i18n.i18nCatalog("cura")
 ##  This action allows for certain settings that are "machine only") to be modified.
 #   It automatically detects machine definitions that it knows how to change and attaches itself to those.
 class MachineSettingsAction(MachineAction):
-    def __init__(self, parent = None):
+    def __init__(self, parent: Optional["QObject"] = None) -> None:
         super().__init__("MachineSettingsAction", catalog.i18nc("@action", "Machine Settings"))
         self._qml_url = "MachineSettingsAction.qml"
 
-        self._application = Application.getInstance()
-
-        self._global_container_stack = None
+        from cura.CuraApplication import CuraApplication
+        self._application = CuraApplication.getInstance()
 
         from cura.Settings.CuraContainerStack import _ContainerIndexes
-        self._container_index = _ContainerIndexes.DefinitionChanges
+        self._store_container_index = _ContainerIndexes.DefinitionChanges
 
         self._container_registry = ContainerRegistry.getInstance()
         self._container_registry.containerAdded.connect(self._onContainerAdded)
-        self._container_registry.containerRemoved.connect(self._onContainerRemoved)
-        self._application.globalContainerStackChanged.connect(self._onGlobalContainerChanged)
 
+        # The machine settings dialog blocks auto-slicing when it's shown, and re-enables it when it's finished.
         self._backend = self._application.getBackend()
+        self.onFinished.connect(self._onFinished)
 
-        self._empty_definition_container_id_list = []
-
-    def _isEmptyDefinitionChanges(self, container_id: str):
-        if not self._empty_definition_container_id_list:
-            self._empty_definition_container_id_list = [self._application.empty_container.getId(),
-                                                        self._application.empty_definition_changes_container.getId()]
-        return container_id in self._empty_definition_container_id_list
+    # Which container index in a stack to store machine setting changes.
+    @pyqtProperty(int, constant = True)
+    def storeContainerIndex(self) -> int:
+        return self._store_container_index
 
     def _onContainerAdded(self, container):
         # Add this action as a supported action to all machine definitions
         if isinstance(container, DefinitionContainer) and container.getMetaDataEntry("type") == "machine":
             self._application.getMachineActionManager().addSupportedAction(container.getId(), self.getKey())
 
-    def _onContainerRemoved(self, container):
-        # Remove definition_changes containers when a stack is removed
-        if container.getMetaDataEntry("type") in ["machine", "extruder_train"]:
-            definition_changes_id = container.definitionChanges.getId()
-            if self._isEmptyDefinitionChanges(definition_changes_id):
-                return
-
     def _reset(self):
-        if not self._global_container_stack:
+        global_stack = self._application.getMachineManager().activeMachine
+        if not global_stack:
             return
 
         # Make sure there is a definition_changes container to store the machine settings
-        definition_changes_id = self._global_container_stack.definitionChanges.getId()
-        if self._isEmptyDefinitionChanges(definition_changes_id):
-            CuraStackBuilder.createDefinitionChangesContainer(self._global_container_stack,
-                                                              self._global_container_stack.getName() + "_settings")
-
-        # Notify the UI in which container to store the machine settings data
-        from cura.Settings.CuraContainerStack import _ContainerIndexes
-
-        container_index = _ContainerIndexes.DefinitionChanges
-        if container_index != self._container_index:
-            self._container_index = container_index
-            self.containerIndexChanged.emit()
+        definition_changes_id = global_stack.definitionChanges.getId()
+        if isEmptyContainer(definition_changes_id):
+            CuraStackBuilder.createDefinitionChangesContainer(global_stack,
+                                                              global_stack.getName() + "_settings")
 
         # Disable auto-slicing while the MachineAction is showing
         if self._backend:  # This sometimes triggers before backend is loaded.
             self._backend.disableTimer()
 
-    @pyqtSlot()
-    def onFinishAction(self):
-        # Restore autoslicing when the machineaction is dismissed
+    def _onFinished(self):
+        # Restore auto-slicing when the machine action is dismissed
         if self._backend and self._backend.determineAutoSlicing():
+            self._backend.enableTimer()
             self._backend.tickle()
 
-    containerIndexChanged = pyqtSignal()
-
-    @pyqtProperty(int, notify = containerIndexChanged)
-    def containerIndex(self):
-        return self._container_index
-
-    def _onGlobalContainerChanged(self):
-        self._global_container_stack = Application.getInstance().getGlobalContainerStack()
-
-        # This additional emit is needed because we cannot connect a UM.Signal directly to a pyqtSignal
-        self.globalContainerChanged.emit()
-
-    globalContainerChanged = pyqtSignal()
-
-    @pyqtProperty(int, notify = globalContainerChanged)
-    def definedExtruderCount(self):
-        if not self._global_container_stack:
-            return 0
-
-        return len(self._global_container_stack.getMetaDataEntry("machine_extruder_trains"))
-
     @pyqtSlot(int)
-    def setMachineExtruderCount(self, extruder_count):
+    def setMachineExtruderCount(self, extruder_count: int) -> None:
         # Note: this method was in this class before, but since it's quite generic and other plugins also need it
         # it was moved to the machine manager instead. Now this method just calls the machine manager.
         self._application.getMachineManager().setActiveMachineExtruderCount(extruder_count)
 
     @pyqtSlot()
-    def forceUpdate(self):
+    def forceUpdate(self) -> None:
         # Force rebuilding the build volume by reloading the global container stack.
         # This is a bit of a hack, but it seems quick enough.
-        self._application.globalContainerStackChanged.emit()
+        self._application.getMachineManager().globalContainerChanged.emit()
 
     @pyqtSlot()
-    def updateHasMaterialsMetadata(self):
+    def updateHasMaterialsMetadata(self) -> None:
+        global_stack = self._application.getMachineManager().activeMachine
+
         # Updates the has_materials metadata flag after switching gcode flavor
-        if not self._global_container_stack:
+        if not global_stack:
             return
 
-        definition = self._global_container_stack.getBottom()
+        definition = global_stack.getDefinition()
         if definition.getProperty("machine_gcode_flavor", "value") != "UltiGCode" or definition.getMetaDataEntry("has_materials", False):
             # In other words: only continue for the UM2 (extended), but not for the UM2+
             return
 
         machine_manager = self._application.getMachineManager()
         material_manager = self._application.getMaterialManager()
-        extruder_positions = list(self._global_container_stack.extruders.keys())
-        has_materials = self._global_container_stack.getProperty("machine_gcode_flavor", "value") != "UltiGCode"
+        extruder_positions = list(global_stack.extruders.keys())
+        has_materials = global_stack.getProperty("machine_gcode_flavor", "value") != "UltiGCode"
 
         material_node = None
         if has_materials:
-            self._global_container_stack.setMetaDataEntry("has_materials", True)
+            global_stack.setMetaDataEntry("has_materials", True)
         else:
             # The metadata entry is stored in an ini, and ini files are parsed as strings only.
             # Because any non-empty string evaluates to a boolean True, we have to remove the entry to make it False.
-            if "has_materials" in self._global_container_stack.getMetaData():
-                self._global_container_stack.removeMetaDataEntry("has_materials")
+            if "has_materials" in global_stack.getMetaData():
+                global_stack.removeMetaDataEntry("has_materials")
 
         # set materials
         for position in extruder_positions:
             if has_materials:
-                material_node = material_manager.getDefaultMaterial(self._global_container_stack, position, None)
+                material_node = material_manager.getDefaultMaterial(global_stack, position, None)
             machine_manager.setMaterial(position, material_node)
 
         self._application.globalContainerStackChanged.emit()
 
     @pyqtSlot(int)
-    def updateMaterialForDiameter(self, extruder_position: int):
+    def updateMaterialForDiameter(self, extruder_position: int) -> None:
         # Updates the material container to a material that matches the material diameter set for the printer
         self._application.getMachineManager().updateMaterialWithVariant(str(extruder_position))
diff --git a/plugins/MachineSettingsAction/MachineSettingsExtruderTab.qml b/plugins/MachineSettingsAction/MachineSettingsExtruderTab.qml
index 270bd7e828..f941d13561 100644
--- a/plugins/MachineSettingsAction/MachineSettingsExtruderTab.qml
+++ b/plugins/MachineSettingsAction/MachineSettingsExtruderTab.qml
@@ -26,15 +26,15 @@ Item
 
     property int columnWidth: (parent.width - 2 * UM.Theme.getSize("default_margin").width) / 2
     property int columnSpacing: 3
-    property int propertyStoreIndex: 5  // definition_changes
+    property int propertyStoreIndex: manager.storeContainerIndex  // definition_changes
 
     property string extruderStackId: ""
     property int extruderPosition: 0
-    property var forceUpdateFunction: CuraApplication.getMachineSettingsManager().forceUpdate
+    property var forceUpdateFunction: manager.forceUpdate
 
     function updateMaterialDiameter()
     {
-        CuraApplication.getMachineSettingsManager().updateMaterialForDiameter(extruderPosition)
+        manager.updateMaterialForDiameter(extruderPosition)
     }
 
     Item
diff --git a/plugins/MachineSettingsAction/MachineSettingsPrinterTab.qml b/plugins/MachineSettingsAction/MachineSettingsPrinterTab.qml
index c952e4cfb3..d9c6bcf539 100644
--- a/plugins/MachineSettingsAction/MachineSettingsPrinterTab.qml
+++ b/plugins/MachineSettingsAction/MachineSettingsPrinterTab.qml
@@ -26,11 +26,11 @@ Item
 
     property int columnWidth: (parent.width - 2 * UM.Theme.getSize("default_margin").width) / 2
     property int columnSpacing: 3
-    property int propertyStoreIndex: 5  // definition_changes
+    property int propertyStoreIndex: manager.storeContainerIndex  // definition_changes
 
     property string machineStackId: Cura.MachineManager.activeMachineId
 
-    property var forceUpdateFunction: CuraApplication.getMachineSettingsManager().forceUpdate
+    property var forceUpdateFunction: manager.forceUpdate
 
     Item
     {
@@ -153,7 +153,7 @@ Item
                 // FIXME(Lipu): better document this.
                 // This has something to do with UM2 and UM2+ regarding "has_material" and the gcode flavor settings.
                 // I don't remember exactly what.
-                afterOnEditingFinishedFunction: CuraApplication.getMachineSettingsManager().updateHasMaterialsMetadata
+                afterOnEditingFinishedFunction: manager.updateHasMaterialsMetadata
             }
         }
 
@@ -277,8 +277,8 @@ Item
                 // FIXME(Lipu): better document this.
                 // This has something to do with UM2 and UM2+ regarding "has_material" and the gcode flavor settings.
                 // I don't remember exactly what.
-                afterOnEditingFinishedFunction: CuraApplication.getMachineSettingsManager().updateHasMaterialsMetadata
-                setValueFunction: CuraApplication.getMachineSettingsManager().setMachineExtruderCount
+                afterOnEditingFinishedFunction: manager.updateHasMaterialsMetadata
+                setValueFunction: manager.setMachineExtruderCount
 
                 optionModel: ListModel
                 {