fix: custom G-code keyword check uses the wrong list and skips the editor (#14908)

src/libslic3r/GCode.cpp

@@ -2378,9 +2378,9 @@ namespace DoExport {
         static const unsigned int MAX_TAGS_COUNT = 5;
         std::vector<std::pair<std::string, std::string>> ret;
 
-        auto check = [&ret](const std::string& source, const std::string& gcode) {
+        auto check = [&ret, is_bbl_printer = print.is_BBL_printer()](const std::string& source, const std::string& gcode) {
             std::vector<std::string> tags;
-            if (GCodeProcessor::contains_reserved_tags(gcode, MAX_TAGS_COUNT, tags)) {
+            if (GCodeProcessor::contains_reserved_tags(gcode, MAX_TAGS_COUNT, tags, is_bbl_printer)) {
                 if (!tags.empty()) {
                     size_t i = 0;
                     while (ret.size() < MAX_TAGS_COUNT && i < tags.size()) {

src/libslic3r/GCode/GCodeProcessor.cpp

@@ -2609,7 +2609,7 @@ bool GCodeProcessor::contains_reserved_tag(const std::string& gcode, std::string
     return ret;
 }
 
-bool GCodeProcessor::contains_reserved_tags(const std::string& gcode, unsigned int max_count, std::vector<std::string>& found_tag)
+bool GCodeProcessor::contains_reserved_tags(const std::string& gcode, unsigned int max_count, std::vector<std::string>& found_tag, bool is_bbl_printer)
 {
     max_count = std::max(max_count, 1U);
 
@@ -2618,7 +2618,7 @@ bool GCodeProcessor::contains_reserved_tags(const std::string& gcode, unsigned i
     CNumericLocalesSetter locales_setter;
 
     GCodeReader parser;
-    auto& _tags = s_IsBBLPrinter ? Reserved_Tags : Reserved_Tags_compatible;
+    auto& _tags = is_bbl_printer ? Reserved_Tags : Reserved_Tags_compatible;
     parser.parse_buffer(gcode, [&ret, &found_tag, max_count, _tags](GCodeReader& parser, const GCodeReader::GCodeLine& line) {
         std::string comment = line.raw();
         if (comment.length() > 2 && comment.front() == ';') {

src/libslic3r/GCode/GCodeProcessor.hpp

@@ -521,7 +521,7 @@ class Print;
         static bool contains_reserved_tag(const std::string& gcode, std::string& found_tag);
         // checks the given gcode for reserved tags and returns true when finding any
         // (the first max_count found tags are returned into found_tag)
-        static bool contains_reserved_tags(const std::string& gcode, unsigned int max_count, std::vector<std::string>& found_tag);
+        static bool contains_reserved_tags(const std::string& gcode, unsigned int max_count, std::vector<std::string>& found_tag, bool is_bbl_printer);
 
         static int get_gcode_last_filament(const std::string &gcode_str);
         static bool get_last_z_from_gcode(const std::string& gcode_str, double& z);

src/slic3r/GUI/Tab.cpp

@@ -3991,7 +3991,7 @@ void TabPrintLayer::update_custom_dirty(std::vector<std::string> &dirty_options,
 bool Tab::validate_custom_gcode(const wxString& title, const std::string& gcode)
 {
     std::vector<std::string> tags;
-    bool invalid = GCodeProcessor::contains_reserved_tags(gcode, 5, tags);
+    bool invalid = GCodeProcessor::contains_reserved_tags(gcode, 5, tags, wxGetApp().preset_bundle->is_bbl_vendor());
     if (invalid) {
         std::string lines = ":\n";
         for (const std::string& keyword : tags)
@@ -4020,11 +4020,27 @@ static void validate_custom_gcode_cb(Tab* tab, ConfigOptionsGroupShp opt_group,
     tab->on_value_change(opt_key, value);
 }
 
+// Orca: names the field the way its page does. The option label alone is ambiguous, as the machine's
+// and the filament's custom G-code are both labelled "Start G-code".
+static wxString custom_gcode_group_title(const Page* page, const t_config_option_key& opt_key)
+{
+    if (page)
+        for (const auto& opt_group : page->m_optgroups)
+            for (const auto& opt : opt_group->opt_map())
+                if (opt.second.first == opt_key)
+                    return opt_group->title;
+    return from_u8(opt_key);
+}
+
 void Tab::edit_custom_gcode(const t_config_option_key& opt_key)
 {
     EditGCodeDialog dlg = EditGCodeDialog(this, opt_key, get_custom_gcode(opt_key));
     if (dlg.ShowModal() == wxID_OK) {
-        set_custom_gcode(opt_key, dlg.get_edited_gcode());
+        const std::string edited_gcode = dlg.get_edited_gcode();
+        // Orca: this dialog writes the value straight into the config, bypassing the field's change
+        // handler, so the reserved keyword check has to run here as it does when editing in place.
+        validate_custom_gcodes_was_shown = !validate_custom_gcode(custom_gcode_group_title(m_active_page, opt_key), edited_gcode);
+        set_custom_gcode(opt_key, edited_gcode);
         update_dirty();
         update();
     }

tests/fff_print/CMakeLists.txt

@@ -9,6 +9,7 @@ add_executable(${_TEST_NAME}_tests
 	test_fill.cpp
 	test_flow.cpp
 	test_gcode_timing.cpp
+	test_gcodeprocessor.cpp
 	test_gcodewriter.cpp
 	test_model.cpp
 	test_multifilament.cpp

tests/fff_print/test_gcodeprocessor.cpp

@@ -0,0 +1,79 @@
+#include <catch2/catch_all.hpp>
+
+#include "libslic3r/libslic3r.h"
+#include "libslic3r/GCode/GCodeProcessor.hpp"
+
+#include <string>
+#include <vector>
+
+using namespace Slic3r;
+
+// Bambu firmware uses the " FEATURE: " style reserved tags, everything else the Slic3r-compatible
+// "TYPE:" style, so which list applies depends on the printer kind passed in.
+TEST_CASE("Reserved keyword detection follows the printer kind it is given", "[GCodeProcessor]")
+{
+    struct Case
+    {
+        const char* name;
+        std::string gcode;
+        bool        reserved_on_bbl;
+        bool        reserved_on_non_bbl;
+    };
+
+    const auto test_case = GENERATE(values<Case>({
+        {"compatible feature tag", ";TYPE:Prime tower", false, true},
+        {"compatible layer tag", ";LAYER_CHANGE", false, true},
+        {"bbl feature tag", "; FEATURE: Outer wall", true, false},
+        {"tag shared by both lists", ";_GP_FIRST_LINE_M73_PLACEHOLDER", true, true},
+        {"bbl spells this one with a leading space", ";COLOR_CHANGE", false, true},
+        {"ordinary comment", "; heat the bed", false, false},
+        {"not a comment at all", "G1 X10 Y10 F3000", false, false},
+        // A tag counts only as the whole comment's prefix, so neither a tag mentioned mid-comment
+        // nor one trailing a real command is a reserved use.
+        {"tag text later in the comment", "; the TYPE:Prime tower marker", false, false},
+        {"tag trailing a command", "G1 X10 ;TYPE:Prime tower", false, false},
+    }));
+
+    DYNAMIC_SECTION(test_case.name)
+    {
+        std::vector<std::string> tags;
+        REQUIRE(GCodeProcessor::contains_reserved_tags(test_case.gcode, 5, tags, true) == test_case.reserved_on_bbl);
+
+        tags.clear();
+        REQUIRE(GCodeProcessor::contains_reserved_tags(test_case.gcode, 5, tags, false) == test_case.reserved_on_non_bbl);
+    }
+}
+
+TEST_CASE("Reserved keyword detection reports every offending line", "[GCodeProcessor]")
+{
+    const std::string gcode = ";TYPE:Prime tower\nG1 X10\n;LAYER_CHANGE\n";
+
+    std::vector<std::string> tags;
+    REQUIRE(GCodeProcessor::contains_reserved_tags(gcode, 5, tags, false));
+    REQUIRE(tags.size() == 2);
+    // Reported in the order they appear, which is what makes the max_count cut-off meaningful.
+    CHECK(tags[0] == "TYPE:Prime tower");
+    CHECK(tags[1] == "LAYER_CHANGE");
+
+    SECTION("the reported count is capped at max_count")
+    {
+        tags.clear();
+        REQUIRE(GCodeProcessor::contains_reserved_tags(gcode, 1, tags, false));
+        CHECK(tags.size() == 1);
+        CHECK(tags[0] == "TYPE:Prime tower");
+    }
+
+    SECTION("a max_count of zero still reports the first tag")
+    {
+        tags.clear();
+        REQUIRE(GCodeProcessor::contains_reserved_tags(gcode, 0, tags, false));
+        CHECK(tags.size() == 1);
+    }
+
+    SECTION("g-code with nothing reserved in it reports nothing")
+    {
+        tags.clear();
+        CHECK_FALSE(GCodeProcessor::contains_reserved_tags("G28\n; home all axes\n", 5, tags, false));
+        CHECK(tags.empty());
+    }
+}