Simplified handling of the "compatible_printers_condition" and

"inherits" configuration values.
Implemented correct setting of the "inherits" flag for the profiles
loaded from AMF/3MF/Config files.

3 files changed, 54 insertions, 34 deletions

diff --git a/xs/src/slic3r/GUI/Preset.cpp b/xs/src/slic3r/GUI/Preset.cpp
index d52ea6215..c0b02d460 100644
--- a/xs/src/slic3r/GUI/Preset.cpp
+++ b/xs/src/slic3r/GUI/Preset.cpp
@@ -234,12 +234,12 @@ std::string Preset::label() const
 
 bool Preset::is_compatible_with_printer(const Preset &active_printer, const DynamicPrintConfig *extra_config) const
 {
-    auto *condition               = dynamic_cast<const ConfigOptionStrings*>(this->config.option("compatible_printers_condition"));
+    auto &condition               = this->compatible_printers_condition();
     auto *compatible_printers     = dynamic_cast<const ConfigOptionStrings*>(this->config.option("compatible_printers"));
     bool  has_compatible_printers = compatible_printers != nullptr && ! compatible_printers->values.empty();
-    if (! has_compatible_printers && condition != nullptr && ! condition->values.empty() && ! condition->values.front().empty()) {
+    if (! has_compatible_printers && ! condition.empty()) {
         try {
-            return PlaceholderParser::evaluate_boolean_expression(condition->values.front(), active_printer.config, extra_config);
+            return PlaceholderParser::evaluate_boolean_expression(condition, active_printer.config, extra_config);
         } catch (const std::runtime_error &err) {
             //FIXME in case of an error, return "compatible with everything".
             printf("Preset::is_compatible_with_printer - parsing error of compatible_printers_condition %s:\n%s\n", active_printer.name.c_str(), err.what());
@@ -466,6 +466,15 @@ Preset& PresetCollection::load_external_preset(
             this->select_preset(it - m_presets.begin());
         return *it;
     }
+    // Update the "inherits" field.
+    std::string &inherits = Preset::inherits(cfg);
+    if (it != m_presets.end() && inherits.empty()) {
+        // There is a profile with the same name already loaded. Should we update the "inherits" field?
+        if (it->vendor == nullptr)
+            inherits = it->inherits();
+        else
+            inherits = it->name;
+    }
     // The external preset does not match an internal preset, load the external preset.
     std::string new_name;
     for (size_t idx = 0;; ++ idx) {
@@ -531,10 +540,7 @@ void PresetCollection::save_current_preset(const std::string &new_name)
     } else {
         // Creating a new preset.
 		Preset       &preset   = *m_presets.insert(it, m_edited_preset);
-        ConfigOptionStrings *opt_inherits = preset.config.option<ConfigOptionStrings>("inherits", true);
-        if (opt_inherits->values.empty())
-            opt_inherits->values.emplace_back(std::string());
-        std::string  &inherits = opt_inherits->values.front();
+        std::string  &inherits = preset.inherits();
         std::string   old_name = preset.name;
         preset.name = new_name;
 		preset.file = this->path_from_name(new_name);
@@ -549,7 +555,6 @@ void PresetCollection::save_current_preset(const std::string &new_name)
             // Inherited from a user preset. Just maintain the "inherited" flag, 
             // meaning it will inherit from either the system preset, or the inherited user preset.
         }
-        preset.inherits = inherits;
         preset.is_default  = false;
         preset.is_system   = false;
         preset.is_external = false;
@@ -587,20 +592,20 @@ bool PresetCollection::load_bitmap_default(const std::string &file_name)
 
 const Preset* PresetCollection::get_selected_preset_parent() const
 {
-    auto *inherits = dynamic_cast<const ConfigOptionStrings*>(this->get_edited_preset().config.option("inherits"));
-	if (inherits == nullptr || inherits->values.empty() || inherits->values.front().empty())
+    const std::string &inherits = this->get_edited_preset().inherits();
+    if (inherits.empty())
 		return this->get_selected_preset().is_system ? &this->get_selected_preset() : nullptr; 
-    const Preset* preset = this->find_preset(inherits->values.front(), false);
+    const Preset* preset = this->find_preset(inherits, false);
     return (preset == nullptr || preset->is_default || preset->is_external) ? nullptr : preset;
 }
 
 const Preset* PresetCollection::get_preset_parent(const Preset& child) const
 {
-    auto *inherits = dynamic_cast<const ConfigOptionStrings*>(child.config.option("inherits"));
-    if (inherits == nullptr || inherits->values.empty() || inherits->values.front().empty())
+    const std::string &inherits = child.inherits();
+    if (inherits.empty())
 // 		return this->get_selected_preset().is_system ? &this->get_selected_preset() : nullptr; 
 		return nullptr; 
-    const Preset* preset = this->find_preset(inherits->values.front(), false);
+    const Preset* preset = this->find_preset(inherits, false);
     return (preset == nullptr/* || preset->is_default */|| preset->is_external) ? nullptr : preset;
 }
 
@@ -837,7 +842,7 @@ std::vector<std::string> PresetCollection::dirty_options(const Preset *edited, c
         // The "compatible_printers" option key is handled differently from the others:
         // It is not mandatory. If the key is missing, it means it is compatible with any printer.
         // If the key exists and it is empty, it means it is compatible with no printer.
-        std::initializer_list<const char*> optional_keys { "compatible_printers", "compatible_printers_condition" };
+        std::initializer_list<const char*> optional_keys { "compatible_printers" };
         for (auto &opt_key : optional_keys) {
             if (reference->config.has(opt_key) != edited->config.has(opt_key))
                 changed.emplace_back(opt_key);
diff --git a/xs/src/slic3r/GUI/Preset.hpp b/xs/src/slic3r/GUI/Preset.hpp
index ee0cdc18b..42ef6ceee 100644
--- a/xs/src/slic3r/GUI/Preset.hpp
+++ b/xs/src/slic3r/GUI/Preset.hpp
@@ -113,9 +113,6 @@ public:
     // or a Configuration file bundling the Print + Filament + Printer presets (in that case is_external and possibly is_system will be true),
     // or it could be a G-code (again, is_external will be true).
     std::string         file;
-    // A user profile may inherit its settings either from a system profile, or from a user profile.
-    // A system profile shall never derive from any other profile, as the system profile hierarchy is being flattened during loading.
-    std::string         inherits;
     // If this is a system profile, then there should be a vendor data available to display at the UI.
     const VendorProfile *vendor      = nullptr;
 
@@ -142,6 +139,28 @@ public:
     bool                is_compatible_with_printer(const Preset &active_printer, const DynamicPrintConfig *extra_config) const;
     bool                is_compatible_with_printer(const Preset &active_printer) const;
 
+    // Returns the name of the preset, from which this preset inherits.
+    static std::string& inherits(DynamicPrintConfig &cfg)
+    {
+        auto option = cfg.option<ConfigOptionStrings>("inherits", true);
+        if (option->values.empty())
+            option->values.emplace_back(std::string());
+        return option->values.front();
+    }
+    std::string&        inherits() { return Preset::inherits(this->config); }
+    const std::string&  inherits() const { return Preset::inherits(const_cast<Preset*>(this)->config); }
+
+    // Returns the "compatible_printers_condition".
+    static std::string& compatible_printers_condition(DynamicPrintConfig &cfg)
+    {
+        auto option = cfg.option<ConfigOptionStrings>("compatible_printers_condition", true);
+        if (option->values.empty())
+            option->values.emplace_back(std::string());
+        return option->values.front();
+    }
+    std::string&        compatible_printers_condition() { return Preset::compatible_printers_condition(this->config); }
+    const std::string&  compatible_printers_condition() const { return Preset::compatible_printers_condition(const_cast<Preset*>(this)->config); }
+
     // Mark this preset as compatible if it is compatible with active_printer.
     bool                update_compatible_with_printer(const Preset &active_printer, const DynamicPrintConfig *extra_config);
 
diff --git a/xs/src/slic3r/GUI/PresetBundle.cpp b/xs/src/slic3r/GUI/PresetBundle.cpp
index fcf8ce859..c11816d06 100644
--- a/xs/src/slic3r/GUI/PresetBundle.cpp
+++ b/xs/src/slic3r/GUI/PresetBundle.cpp
@@ -65,12 +65,12 @@ PresetBundle::PresetBundle() :
 
     // Create the ID config keys, as they are not part of the Static print config classes.
     this->prints.default_preset().config.optptr("print_settings_id", true);
-    this->prints.default_preset().config.option<ConfigOptionStrings>("compatible_printers_condition", true)->values = { "" };
-    this->prints.default_preset().config.option<ConfigOptionStrings>("inherits", true)->values = { "" };
+    this->prints.default_preset().compatible_printers_condition();
+    this->prints.default_preset().inherits();
 
     this->filaments.default_preset().config.option<ConfigOptionStrings>("filament_settings_id", true)->values = { "" };
-    this->filaments.default_preset().config.option<ConfigOptionStrings>("compatible_printers_condition", true)->values = { "" };
-    this->filaments.default_preset().config.option<ConfigOptionStrings>("inherits", true)->values = { "" };
+    this->filaments.default_preset().compatible_printers_condition();
+    this->filaments.default_preset().inherits();
 
     this->printers.default_preset().config.optptr("printer_settings_id", true);
     this->printers.default_preset().config.optptr("printer_vendor", true);
@@ -78,7 +78,7 @@ PresetBundle::PresetBundle() :
     this->printers.default_preset().config.optptr("printer_variant", true);
     this->printers.default_preset().config.optptr("default_print_profile", true);
     this->printers.default_preset().config.optptr("default_filament_profile", true);
-	this->printers.default_preset().config.option<ConfigOptionStrings>("inherits", true)->values = { "" };
+	this->printers.default_preset().inherits();
 
 	// Load the default preset bitmaps.
     this->prints   .load_bitmap_default("cog.png");
@@ -387,17 +387,13 @@ DynamicPrintConfig PresetBundle::full_config() const
     // Collect the "compatible_printers_condition" and "inherits" values over all presets (print, filaments, printers) into a single vector.
     std::vector<std::string> compatible_printers_condition;
     std::vector<std::string> inherits;
-    auto                     append_config_string = [](const DynamicConfig &cfg, const std::string &key, std::vector<std::string> &dst) {
-        const ConfigOptionStrings *opt = cfg.opt<ConfigOptionStrings>(key);
-        dst.emplace_back((opt == nullptr || opt->values.empty()) ? "" : opt->values.front());
-    };
-    append_config_string(this->prints.get_edited_preset().config, "compatible_printers_condition", compatible_printers_condition);
-    append_config_string(this->prints.get_edited_preset().config, "inherits",                      inherits);
+    compatible_printers_condition.emplace_back(this->prints.get_edited_preset().compatible_printers_condition());
+    inherits                     .emplace_back(this->prints.get_edited_preset().inherits());
 
     if (num_extruders <= 1) {
         out.apply(this->filaments.get_edited_preset().config);
-        append_config_string(this->filaments.get_edited_preset().config, "compatible_printers_condition", compatible_printers_condition);
-        append_config_string(this->filaments.get_edited_preset().config, "inherits",                      inherits);
+        compatible_printers_condition.emplace_back(this->filaments.get_edited_preset().compatible_printers_condition());
+        inherits                     .emplace_back(this->filaments.get_edited_preset().inherits());
     } else {
         // Retrieve filament presets and build a single config object for them.
         // First collect the filament configurations based on the user selection of this->filament_presets.
@@ -408,8 +404,8 @@ DynamicPrintConfig PresetBundle::full_config() const
 		while (filament_configs.size() < num_extruders)
             filament_configs.emplace_back(&this->filaments.first_visible().config);
         for (const DynamicPrintConfig *cfg : filament_configs) {
-            append_config_string(*cfg, "compatible_printers_condition", compatible_printers_condition);
-            append_config_string(*cfg, "inherits",                      inherits);
+            compatible_printers_condition.emplace_back(Preset::compatible_printers_condition(*const_cast<DynamicPrintConfig*>(cfg)));
+            inherits                     .emplace_back(Preset::inherits(*const_cast<DynamicPrintConfig*>(cfg)));
         }
         // Option values to set a ConfigOptionVector from.
         std::vector<const ConfigOption*> filament_opts(num_extruders, nullptr);
@@ -434,7 +430,7 @@ DynamicPrintConfig PresetBundle::full_config() const
     }
 
 	// Don't store the "compatible_printers_condition" for the printer profile, there is none.
-    append_config_string(this->printers.get_edited_preset().config, "inherits", inherits);
+    inherits.emplace_back(this->printers.get_edited_preset().inherits());
 
     // These two value types clash between the print and filament profiles. They should be renamed.
     out.erase("compatible_printers");