Read BambuStudio Nil Slots as Not Set Instead of Aborting the Load

BambuStudio and its forks save a nozzle variant that matches the parent
preset as "nil", including in keys OrcaSlicer can't leave empty, such as
retraction, z-hop and nozzle temperature. Reading one threw, which ended
the rest of the settings file: a project kept only the keys before it
alphabetically, and an embedded or user preset was dropped.

load_from_json now reads such a slot as not set when the caller opts in
(project settings, embedded presets, user presets). If every slot is nil,
the key is left out. Otherwise the slot holds the option default and is
recorded, and update_diff_values_to_child_config gives it the parent
preset's value. Other loaders still reject nil.

src/libslic3r/Config.cpp

@@ -873,10 +873,10 @@ int ConfigBase::load_from_json(const std::string &file, ConfigSubstitutionContex
 
     CNumericLocalesSetter locales_setter;
 
-    std::function<bool(const json::const_iterator&, const char,const char,const bool,std::string&)> parse_str_arr = [&parse_str_arr](const json::const_iterator& it, const char single_sep,const char array_sep,const bool escape_string_style,std::string& value_str)->bool {
+    std::function<bool(const json&, const char,const char,const bool,std::string&)> parse_str_arr = [&parse_str_arr](const json& arr, const char single_sep,const char array_sep,const bool escape_string_style,std::string& value_str)->bool {
         // must have consistent type name
         std::string consistent_type;
-        for (auto iter = it.value().begin(); iter != it.value().end(); ++iter) {
+        for (auto iter = arr.begin(); iter != arr.end(); ++iter) {
             if (consistent_type.empty())
                 consistent_type = iter.value().type_name();
             else {
@@ -886,13 +886,13 @@ int ConfigBase::load_from_json(const std::string &file, ConfigSubstitutionContex
         }
 
         bool first = true;
-        for (auto iter = it.value().begin(); iter != it.value().end(); iter++) {
+        for (auto iter = arr.begin(); iter != arr.end(); iter++) {
             if (iter.value().is_array()) {
                 if (!first)
                     value_str += array_sep;
                 else
                     first = false;
-                bool success = parse_str_arr(iter, single_sep, array_sep,escape_string_style, value_str);
+                bool success = parse_str_arr(iter.value(), single_sep, array_sep,escape_string_style, value_str);
                 if (!success)
                     return false;
             }
@@ -1038,8 +1038,39 @@ int ConfigBase::load_from_json(const std::string &file, ConfigSubstitutionContex
                         }
                     }
 
+                    // BambuStudio and its forks save a nozzle variant that matches the parent preset as "nil".
+                    // An option that can't hold nil gets its default in that slot, and the slot is reported so
+                    // the merge onto the parent keeps the parent's value. All slots nil means the key is not set.
+                    const json *values = &it.value();
+                    json values_with_defaults;
+                    if (substitution_context.accept_nil && optdef && !optdef->nullable && optdef->default_value &&
+                        (optdef->type == coFloats || optdef->type == coPercents || optdef->type == coFloatsOrPercents ||
+                         optdef->type == coInts || optdef->type == coEnums || optdef->type == coBools)) {
+                        auto is_nil = [](const json &v) { return v.is_string() && v.get<std::string>() == "nil"; };
+                        std::vector<size_t> nil_slots;
+                        for (size_t i = 0; i < values->size(); ++i)
+                            if (is_nil((*values)[i]))
+                                nil_slots.push_back(i);
+                        if (!nil_slots.empty()) {
+                            BOOST_LOG_TRIVIAL(warning) << __FUNCTION__ << ": " << file << ": " << it.key() << " is nil in "
+                                                       << nil_slots.size() << " of " << values->size()
+                                                       << " slots, read as not set (the parent preset's value, or the default)";
+                            if (nil_slots.size() == values->size())
+                                continue;
+                            // create_default_option() gives enums their names, which vserialize() needs.
+                            std::unique_ptr<ConfigOption> default_option(optdef->create_default_option());
+                            const std::vector<std::string> defaults = static_cast<const ConfigOptionVectorBase*>(default_option.get())->vserialize();
+                            values_with_defaults = *values;
+                            const json first_value = *std::find_if_not(values->begin(), values->end(), is_nil);
+                            for (size_t i : nil_slots)
+                                values_with_defaults[i] = defaults.empty() ? first_value : json(defaults[i % defaults.size()]);
+                            values = &values_with_defaults;
+                            substitution_context.nil_slots[opt_key] = std::move(nil_slots);
+                        }
+                    }
+
                     // BBS: we only support 2 depth array
-                    valid = parse_str_arr(it, single_sep, array_sep,escape_string_type, value_str);
+                    valid = parse_str_arr(*values, single_sep, array_sep,escape_string_type, value_str);
                     if (!valid) {
                         BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << ": parse " << file << " error, invalid json array for " << it.key();
                         break;

src/libslic3r/Config.hpp

@@ -267,6 +267,11 @@ struct ConfigSubstitutionContext
     ForwardCompatibilitySubstitutionRule 	rule;
     ConfigSubstitutions					    substitutions;
     std::vector<std::string>                unrecogized_keys;
+    // Read "nil" in an option that can't hold it as not set instead of failing. Set by callers that hand
+    // nil_slots to the merge onto the parent preset, or for which the option default is the right fallback.
+    bool                                    accept_nil = false;
+    // Slots of options that can't hold nil but were "nil" in the file; they hold the option default.
+    std::map<std::string, std::vector<size_t>> nil_slots;
 };
 
 // A generic value of a configuration option.

src/libslic3r/Format/bbs_3mf.cpp

@@ -2696,6 +2696,8 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result)
             }
             std::map<std::string, std::string> key_values;
             std::string reason;
+            // No parent preset here: a nil slot keeps the option default.
+            config_substitutions.accept_nil = true;
             int ret = config.load_from_json(dest_file, config_substitutions, true, key_values, reason);
             if (ret) {
                 add_error("Error load config from json:"+reason);
@@ -2736,7 +2738,13 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result)
             //ConfigSubstitutions config_substitutions = config.load_from_ini(dest_file, Enable);
             std::map<std::string, std::string> key_values;
             std::string reason;
-            ConfigSubstitutions config_substitutions = use_json? config.load_from_json(dest_file, Enable, key_values, reason) : config.load_from_ini(dest_file, Enable);
+            ConfigSubstitutionContext load_context(Enable);
+            load_context.accept_nil = true;
+            if (use_json)
+                config.load_from_json(dest_file, load_context, true, key_values, reason);
+            else
+                load_context.substitutions = config.load_from_ini(dest_file, Enable);
+            ConfigSubstitutions config_substitutions = std::move(load_context.substitutions);
             if (!reason.empty()) {
                 BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << boost::format(", load project embedded config from  %1% failed\n") % dest_file;
                 //skip this file
@@ -2785,6 +2793,7 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result)
             preset->is_project_embedded = true;
             preset->is_external = true;
             preset->is_dirty = false;
+            preset->nil_slots = std::move(load_context.nil_slots);
 
             std::string version_str = key_values[BBL_JSON_KEY_VERSION];
             boost::optional<Semver> version = Semver::parse(version_str);

src/libslic3r/Preset.cpp

@@ -1726,9 +1726,11 @@ PresetCollection::UserPresetLoad PresetCollection::resolve_user_preset(
         //ConfigSubstitutions config_substitutions = config.load_from_ini(preset.file, substitution_rule);
         std::map<std::string, std::string> key_values;
         std::string reason;
-        ConfigSubstitutions config_substitutions = config.load_from_json(preset.file, substitution_rule, key_values, reason);
-        if (! config_substitutions.empty())
-            out.substitutions.push_back({ preset.name, m_type, PresetConfigSubstitutions::Source::UserFile, preset.file, std::move(config_substitutions) });
+        ConfigSubstitutionContext load_context(substitution_rule);
+        load_context.accept_nil = true;
+        config.load_from_json(preset.file, load_context, true, key_values, reason);
+        if (! load_context.substitutions.empty())
+            out.substitutions.push_back({ preset.name, m_type, PresetConfigSubstitutions::Source::UserFile, preset.file, std::move(load_context.substitutions) });
         if (!reason.empty()) {
             out.discard_file = true;
             out.errors.push_back((boost::format("parse config %1% failed") % preset.file).str());
@@ -1764,7 +1766,7 @@ PresetCollection::UserPresetLoad PresetCollection::resolve_user_preset(
             preset.config = inherit_preset->config;
             preset.filament_id = inherit_preset->filament_id;
             extend_default_config_length(config, false, {});
-            preset.config.update_diff_values_to_child_config(config, extruder_id_name, extruder_variant_name, *key_set1, *key_set2);
+            preset.config.update_diff_values_to_child_config(config, extruder_id_name, extruder_variant_name, *key_set1, *key_set2, &load_context.nil_slots);
         }
         else {
             auto inherits_config2 = dynamic_cast<ConfigOptionString *>(inherits_config);
@@ -2137,7 +2139,8 @@ void PresetCollection::load_project_embedded_presets(std::vector<Preset*>& proje
                 BOOST_LOG_TRIVIAL(error) << boost::format("can not find parent for config %1%!")%preset->file;
                 continue;
             }
-            preset->config.update_diff_values_to_child_config(config, extruder_id_name, extruder_variant_name, *key_set1, *key_set2);
+            preset->config.update_diff_values_to_child_config(config, extruder_id_name, extruder_variant_name, *key_set1, *key_set2, &preset->nil_slots);
+            preset->nil_slots.clear();
             //preset->config.apply(std::move(config));
             Preset::normalize(preset->config);
             // Report configuration fields, which are misplaced into a wrong group.

src/libslic3r/Preset.hpp

@@ -269,6 +269,8 @@ public:
     //BBS: add type for project-embedded
     bool                is_project_embedded = false;
     ConfigSubstitutions *loading_substitutions{nullptr};
+    // Slots that were nil in the embedded preset's file, see ConfigSubstitutionContext::nil_slots.
+    std::map<std::string, std::vector<size_t>> nil_slots;
     bool                is_user() const { return ! this->is_default && ! this->is_system && ! this->is_project_embedded && ! this->is_from_bundle(); }
     bool                can_overwrite() const { return ! this->is_default && ! this->is_system && ! this->is_from_bundle(); }
     //bool                is_user() const { return ! this->is_default && ! this->is_system; }

src/libslic3r/PrintConfig.cpp

@@ -11596,7 +11596,8 @@ void DynamicPrintConfig::update_non_diff_values_to_base_config(DynamicPrintConfi
     return;
 }
 
-void DynamicPrintConfig::update_diff_values_to_child_config(DynamicPrintConfig& new_config, std::string extruder_id_name, std::string extruder_variant_name, std::set<std::string>& key_set1, std::set<std::string>& key_set2)
+void DynamicPrintConfig::update_diff_values_to_child_config(DynamicPrintConfig& new_config, std::string extruder_id_name, std::string extruder_variant_name, std::set<std::string>& key_set1, std::set<std::string>& key_set2,
+    const std::map<std::string, std::vector<size_t>>* nil_slots)
 {
     std::vector<int> cur_extruder_ids, target_extruder_ids, variant_index;
     std::vector<std::string> cur_extruder_variants, target_extruder_variants;
@@ -11657,6 +11658,13 @@ void DynamicPrintConfig::update_diff_values_to_child_config(DynamicPrintConfig&
         if (opt_src && opt_target && (*opt_src != *opt_target)) {
             BOOST_LOG_TRIVIAL(debug) << __FUNCTION__ << boost::format(" change key %1% from base_value %2% to child's value %3%")
                     %opt %(opt_src->serialize()) %(opt_target->serialize());
+            const std::vector<size_t> *unset_slots = nullptr;
+            if (nil_slots && opt_src->is_vector())
+                if (auto it = nil_slots->find(opt); it != nil_slots->end())
+                    unset_slots = &it->second;
+            std::unique_ptr<ConfigOption> base_value(unset_slots ? opt_src->clone() : nullptr);
+            int  stride            = 1;
+            bool merged_by_variant = false;
             if (opt_target->is_scalar()
                 || ((key_set1.find(opt) == key_set1.end()) && (key_set2.empty() || (key_set2.find(opt) == key_set2.end())))) {
                 //nothing to do, keep the original one
@@ -11665,7 +11673,6 @@ void DynamicPrintConfig::update_diff_values_to_child_config(DynamicPrintConfig&
             else {
                 ConfigOptionVectorBase* opt_vec_src = static_cast<ConfigOptionVectorBase*>(opt_src);
                 const ConfigOptionVectorBase* opt_vec_dest = static_cast<const ConfigOptionVectorBase*>(opt_target);
-                int stride = 1;
                 if (key_set2.find(opt) != key_set2.end())
                     stride = 2;
                 // set_only_diff() requires the base vector length to equal variant_index.size()*stride, where
@@ -11679,8 +11686,26 @@ void DynamicPrintConfig::update_diff_values_to_child_config(DynamicPrintConfig&
                 if (opt_vec_src->size() != variant_index.size() * size_t(stride)) {
                     opt_src->set(opt_target);
                 }
-                else
+                else {
                     opt_vec_src->set_only_diff(opt_vec_dest, variant_index, stride);
+                    merged_by_variant = true;
+                }
+            }
+            // A slot that was nil in the child's file is not set there, so it keeps this config's value.
+            if (unset_slots) {
+                auto       *merged    = static_cast<ConfigOptionVectorBase*>(opt_src);
+                const size_t base_size = static_cast<const ConfigOptionVectorBase*>(base_value.get())->size();
+                for (size_t slot = 0; slot < merged->size() && slot < base_size; ++slot) {
+                    size_t child_slot = slot;
+                    if (merged_by_variant) {
+                        const int child_variant = variant_index[slot / stride];
+                        if (child_variant == -1)
+                            continue;
+                        child_slot = size_t(child_variant) * stride + slot % stride;
+                    }
+                    if (std::find(unset_slots->begin(), unset_slots->end(), child_slot) != unset_slots->end())
+                        merged->set_at(base_value.get(), slot, slot);
+                }
             }
         }
     }

src/libslic3r/PrintConfig.hpp

@@ -859,7 +859,10 @@ public:
 
     void update_non_diff_values_to_base_config(DynamicPrintConfig& new_config, const t_config_option_keys& keys, const std::set<std::string>& different_keys, std::string extruder_id_name, std::string extruder_variant_name,
         std::set<std::string>& key_set1, std::set<std::string>& key_set2);
-    void update_diff_values_to_child_config(DynamicPrintConfig& new_config, std::string extruder_id_name, std::string extruder_variant_name, std::set<std::string>& key_set1, std::set<std::string>& key_set2);
+    // nil_slots: per option, the slots of new_config that were nil in its file (see ConfigSubstitutionContext::nil_slots);
+    // those variants keep this config's value.
+    void update_diff_values_to_child_config(DynamicPrintConfig& new_config, std::string extruder_id_name, std::string extruder_variant_name, std::set<std::string>& key_set1, std::set<std::string>& key_set2,
+        const std::map<std::string, std::vector<size_t>>* nil_slots = nullptr);
 
     int update_values_from_single_to_multi(DynamicPrintConfig& multi_config, std::set<std::string>& key_set, std::string id_name, std::string variant_name);
     int update_values_from_multi_to_multi(DynamicPrintConfig& new_config, std::set<std::string>& key_set, std::string id_name, std::string variant_name, std::vector<std::string>& extruder_variants);

tests/libslic3r/test_config.cpp

@@ -585,6 +585,81 @@ TEST_CASE("A BambuStudio project config loads its renamed settings without subst
     CHECK(config.validate().count("raft_first_layer_expansion") == 0);
 }
 
+TEST_CASE("load_from_json reads a BambuStudio nil slot as not set", "[Config]") {
+    ScopedTemporaryFile tmp(".json");
+    {
+        boost::nowide::ofstream ofs(tmp.string());
+        // Keys after retraction_length in file order must still load.
+        ofs << R"({"layer_height":"0.2","retraction_length":["0.8","nil"],"wall_loops":"3","z_hop":["nil","nil"],)"
+               R"("z_hop_types":["Spiral Lift","nil"]})";
+    }
+    std::map<std::string, std::string> key_values;
+    std::string reason;
+
+    SECTION("a loader that doesn't merge onto a parent still rejects nil") {
+        DynamicPrintConfig config;
+        ConfigSubstitutionContext context(ForwardCompatibilitySubstitutionRule::Enable);
+        config.load_from_json(tmp.string(), context, true, key_values, reason);
+        CHECK_FALSE(reason.empty());
+    }
+
+    SECTION("a loader that opts in reads it as not set") {
+        DynamicPrintConfig config;
+        ConfigSubstitutionContext context(ForwardCompatibilitySubstitutionRule::Enable);
+        context.accept_nil = true;
+        REQUIRE(config.load_from_json(tmp.string(), context, true, key_values, reason) == 0);
+        CHECK(reason.empty());
+
+        const auto *default_length = static_cast<const ConfigOptionFloats*>(print_config_def.get("retraction_length")->default_value.get());
+        const auto &retraction_length = config.option<ConfigOptionFloats>("retraction_length")->values;
+        REQUIRE(retraction_length.size() == 2);
+        CHECK_THAT(retraction_length[0], Catch::Matchers::WithinAbs(0.8, 1e-9));
+        CHECK_THAT(retraction_length[1], Catch::Matchers::WithinAbs(default_length->get_at(1), 1e-9));
+
+        std::unique_ptr<ConfigOption> default_types(print_config_def.get("z_hop_types")->create_default_option());
+        const std::vector<std::string> type_defaults = static_cast<const ConfigOptionVectorBase*>(default_types.get())->vserialize();
+        CHECK(static_cast<const ConfigOptionVectorBase*>(config.option("z_hop_types"))->vserialize() ==
+              std::vector<std::string>{"Spiral Lift", type_defaults[1 % type_defaults.size()]});
+
+        CHECK(context.nil_slots == std::map<std::string, std::vector<size_t>>{{"retraction_length", {1}}, {"z_hop_types", {1}}});
+        CHECK_FALSE(config.has("z_hop"));
+        CHECK(config.opt_int("wall_loops") == 3);
+    }
+}
+
+TEST_CASE("A nozzle variant that was nil keeps the parent preset's value", "[Config]") {
+    auto printer = [](std::vector<double> nozzle_diameter, std::vector<double> retraction_length, std::vector<double> max_speed_x) {
+        DynamicPrintConfig config;
+        config.set_key_value("printer_extruder_variant", new ConfigOptionStrings({"Direct Drive Standard", "Direct Drive High Flow"}));
+        config.set_key_value("printer_extruder_id", new ConfigOptionInts({1, 1}));
+        // Not a per-variant key, so the merge copies it whole.
+        config.set_key_value("nozzle_diameter", new ConfigOptionFloats(nozzle_diameter));
+        config.set_key_value("retraction_length", new ConfigOptionFloats(retraction_length));
+        // Two values per variant: normal and silent mode.
+        config.set_key_value("machine_max_speed_x", new ConfigOptionFloats(max_speed_x));
+        return config;
+    };
+    auto check_values = [](const DynamicPrintConfig &config, const char *key, const std::vector<double> &expected) {
+        const std::vector<double> &values = config.option<ConfigOptionFloats>(key)->values;
+        INFO(key);
+        REQUIRE(values.size() == expected.size());
+        for (size_t i = 0; i < expected.size(); ++i)
+            CHECK_THAT(values[i], Catch::Matchers::WithinAbs(expected[i], 1e-9));
+    };
+    DynamicPrintConfig parent = printer({0.4, 0.6}, {0.6, 0.5}, {500, 200, 400, 100});
+    // The nil slots of the child hold the option default after loading.
+    DynamicPrintConfig child = printer({0.2, 0.4}, {0.8, 0.4}, {500, 200, 300, 90});
+    const std::map<std::string, std::vector<size_t>> nil_slots{{"nozzle_diameter", {1}}, {"retraction_length", {1}}, {"machine_max_speed_x", {3}}};
+
+    parent.update_diff_values_to_child_config(child, "printer_extruder_id", "printer_extruder_variant",
+                                              printer_options_with_variant_1, printer_options_with_variant_2, &nil_slots);
+
+    check_values(parent, "nozzle_diameter", {0.2, 0.6});
+    check_values(parent, "retraction_length", {0.8, 0.5});
+    // Only the silent-mode value of the second variant was nil.
+    check_values(parent, "machine_max_speed_x", {500, 200, 300, 100});
+}
+
 TEST_CASE("save_to_json writes the same document to a stream as to a file", "[Config]") {
     DynamicPrintConfig config;
     config.set_key_value("layer_height", new ConfigOptionFloat(0.2));