address review comments

docs/guides/quickfort-user-guide.rst

@@ -1010,9 +1010,11 @@ job but can't find the materials.
 
 As long as the `buildingplan` plugin is enabled, quickfort will use it to manage
 construction. The buildingplan plugin also has an "enabled" setting for each
-building type, but that setting only applies to the buildingplan user interface;
+building type, but that setting only applies to the buildingplan user interface.
-quickfort will use buildingplan to manage everything designated in a ``#build``
+In addition, buildingplan has a "quickfort_mode" setting for compatibility with
-blueprint regardless of the buildingplan UI settings.
+legacy Python Quickfort. This setting has no effect on DFHack Quickfort, which
+will use buildingplan to manage everything designated in a ``#build`` blueprint
+regardless of the buildingplan UI settings.
 
 However, quickfort *does* use buildingplan's filters for each building type. For
 example, you can use the buildingplan UI to set the type of stone you want your

plugins/buildingplan.cpp

@@ -17,7 +17,7 @@
 #include "buildingplan-lib.h"
 
 DFHACK_PLUGIN("buildingplan");
-#define PLUGIN_VERSION 2.0
+#define PLUGIN_VERSION "2.0"
 REQUIRE_GLOBAL(ui);
 REQUIRE_GLOBAL(ui_build_selector);
 REQUIRE_GLOBAL(world); // used in buildingplan library
@@ -913,11 +913,13 @@ IMPLEMENT_VMETHOD_INTERPOSE(buildingplan_room_hook, render);
 
 DFHACK_PLUGIN_IS_ENABLED(is_enabled);
 
-static void setSetting(std::string name, bool value);
+static bool setSetting(std::string name, bool value);
 
 static bool isTrue(std::string val)
 {
-    return val == "on" || val == "true" || val == "y" || val == "yes";
+    val = toLower(val);
+    return val == "on" || val == "true" || val == "y" || val == "yes"
+        || val == "1";
 }
 
 static command_result buildingplan_cmd(color_ostream &out, vector <string> & parameters)
@@ -929,51 +931,61 @@ static command_result buildingplan_cmd(color_ostream &out, vector <string> & par
 
     if (cmd.size() >= 1 && cmd[0] == 'v')
     {
-        out << "buildingplan version: " << PLUGIN_VERSION << endl;
+        out.print("buildingplan version: %s\n", PLUGIN_VERSION);
     }
     else if (parameters.size() >= 2 && cmd == "debug")
     {
-        show_debugging = isTrue(toLower(parameters[1]));
+        show_debugging = isTrue(parameters[1]);
-        out << "buildingplan debugging: " <<
+        out.print("buildingplan debugging: %s\n",
-                ((show_debugging) ? "enabled" : "disabled") << endl;
+                  show_debugging ? "enabled" : "disabled");
     }
     else if (cmd == "set")
     {
         if (!is_enabled)
         {
-            out << "ERROR: buildingplan must be enabled before you can read or "
+            out.printerr(
-                << "set buildingplan global settings." << endl;
+                "ERROR: buildingplan must be enabled before you can"
+                " read or set buildingplan global settings.");
             return CR_FAILURE;
         }
 
         if (!DFHack::Core::getInstance().isMapLoaded())
         {
-            out << "ERROR: A map must be loaded before you can read or set "
+            out.printerr(
-                << "buildingplan global settings. Try adding your "
+                "ERROR: A map must be loaded before you can read or set"
-                << "'buildingplan set' commands to the onMapLoad.init file."
+                "buildingplan global settings. Try adding your"
-                << endl;
+                "'buildingplan set' commands to the onMapLoad.init" "file.");
             return CR_FAILURE;
         }
 
         if (parameters.size() == 1)
         {
             // display current settings
-            out << "active settings:" << endl;
+            out.print("active settings:\n");
+
             for (auto & setting : planner.getGlobalSettings())
             {
-                out << "  " << setting.first << " = "
+                out.print("  %s = %s\n", setting.first.c_str(),
-                    << (setting.second ? "true" : "false") << endl;
+                          setting.second ? "true" : "false");
             }
 
-            out << "  quickfort_mode = "
+            out.print("  quickfort_mode = %s\n",
-                << (quickfort_mode ? "true" : "false") << endl;
+                      quickfort_mode ? "true" : "false");
         }
         else if (parameters.size() == 3)
         {
             // set a setting
             std::string setting = toLower(parameters[1]);
-            bool val = isTrue(toLower(parameters[2]));
+            bool val = isTrue(parameters[2]);
-            setSetting(setting, val);
+            if (!setSetting(setting, val))
+            {
+                out.printerr("ERROR: invalid parameter: '%s'\n",
+                    parameters[1].c_str());
+            }
+        }
+        else
+        {
+            out.printerr("ERROR: invalid syntax");
         }
     }
 
@@ -1089,14 +1101,14 @@ static void scheduleCycle() {
     cycle_requested = true;
 }
 
-static void setSetting(std::string name, bool value) {
+static bool setSetting(std::string name, bool value) {
     if (name == "quickfort_mode")
     {
         debug("setting quickfort_mode %d -> %d", quickfort_mode, value);
         quickfort_mode = value;
-        return;
+        return true;
     }
-    planner.setGlobalSetting(name, value);
+    return planner.setGlobalSetting(name, value);
 }
 
 DFHACK_PLUGIN_LUA_FUNCTIONS {