From 7291026bec6bc516e36c5ebbde3591079871ed24 Mon Sep 17 00:00:00 2001 From: Lipu Fei Date: Fri, 22 Feb 2019 15:13:50 +0100 Subject: [PATCH 1/2] Fix material container removal CURA-6237 --- cura/Machines/MaterialManager.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/cura/Machines/MaterialManager.py b/cura/Machines/MaterialManager.py index 160508e7a6..aa5af9f01d 100644 --- a/cura/Machines/MaterialManager.py +++ b/cura/Machines/MaterialManager.py @@ -537,6 +537,24 @@ class MaterialManager(QObject): return nodes_to_remove = [material_group.root_material_node] + material_group.derived_material_node_list + # FIXME: CURA-6237 + # Sort all nodes with respect to the container ID lengths in the ascending order so the base material container + # will be the last one to remove. Adding this is because in ContainerRegistry.removeContainer(), the container + # will be loaded if it has not been loaded before. If the base material has been removed before this happens, + # it will not be able to find and load the material container, resulting in a crash. + # We may need to consider changing how the signal ContainerRegistry.containerRemoved works: Now it requires + # the container that's being removed, meaning that in order to remove a container, it must be loaded first. + # But it can also be that for ContainerRegistry.containerRemoved, we just need to notify the id, name, and/or + # type of the container that's being removed, thus removing removeContainer()'s dependency on a container being + # loaded first. + nodes_to_remove = sorted(nodes_to_remove, key = lambda x: len(x.getMetaDataEntry("id", "")), reverse = True) + # Try to load all containers first. If there is any faulty ones, they will be put into the faulty container + # list, so removeContainer() can ignore those ones. + for node in nodes_to_remove: + container_id = node.getMetaDataEntry("id", "") + results = self._container_registry.findContainers(id = container_id) + if not results: + self._container_registry.addWrongContainerId(container_id) for node in nodes_to_remove: self._container_registry.removeContainer(node.getMetaDataEntry("id", "")) From 2af3ae8efba15b6549e21c70f7026ab0eb894c94 Mon Sep 17 00:00:00 2001 From: Jaime van Kessel Date: Fri, 1 Mar 2019 10:53:55 +0100 Subject: [PATCH 2/2] Ensure that the materials get removed in order If we remove the materials in order, it doesn't cause any issues. This is probably because loading the base profile causes the others to be properly instantiated (and subsequently deleted) CURA-6237 --- cura/Machines/MaterialManager.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/cura/Machines/MaterialManager.py b/cura/Machines/MaterialManager.py index aa5af9f01d..ae6dbbd9bd 100644 --- a/cura/Machines/MaterialManager.py +++ b/cura/Machines/MaterialManager.py @@ -537,17 +537,9 @@ class MaterialManager(QObject): return nodes_to_remove = [material_group.root_material_node] + material_group.derived_material_node_list - # FIXME: CURA-6237 # Sort all nodes with respect to the container ID lengths in the ascending order so the base material container - # will be the last one to remove. Adding this is because in ContainerRegistry.removeContainer(), the container - # will be loaded if it has not been loaded before. If the base material has been removed before this happens, - # it will not be able to find and load the material container, resulting in a crash. - # We may need to consider changing how the signal ContainerRegistry.containerRemoved works: Now it requires - # the container that's being removed, meaning that in order to remove a container, it must be loaded first. - # But it can also be that for ContainerRegistry.containerRemoved, we just need to notify the id, name, and/or - # type of the container that's being removed, thus removing removeContainer()'s dependency on a container being - # loaded first. - nodes_to_remove = sorted(nodes_to_remove, key = lambda x: len(x.getMetaDataEntry("id", "")), reverse = True) + # will be the first one to be removed. We need to do this to ensure that all containers get loaded & deleted. + nodes_to_remove = sorted(nodes_to_remove, key = lambda x: len(x.getMetaDataEntry("id", ""))) # Try to load all containers first. If there is any faulty ones, they will be put into the faulty container # list, so removeContainer() can ignore those ones. for node in nodes_to_remove: