From ad39bb3c3348c432f7edc7c2de5cd023a9d0c452 Mon Sep 17 00:00:00 2001
From: myk002 <myk002@yahoo.com>
Date: Fri, 22 Jan 2021 12:48:36 -0800
Subject: [PATCH] address review comments

---
 docs/guides/quickfort-user-guide.rst |  8 ++--
 plugins/buildingplan.cpp             | 58 +++++++++++++++++-----------
 2 files changed, 40 insertions(+), 26 deletions(-)

diff --git a/docs/guides/quickfort-user-guide.rst b/docs/guides/quickfort-user-guide.rst
index 0deedc192..66dc980aa 100644
--- a/docs/guides/quickfort-user-guide.rst
+++ b/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;
-quickfort will use buildingplan to manage everything designated in a ``#build``
-blueprint regardless of the buildingplan UI settings.
+building type, but that setting only applies to the buildingplan user interface.
+In addition, buildingplan has a "quickfort_mode" setting for compatibility with
+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
diff --git a/plugins/buildingplan.cpp b/plugins/buildingplan.cpp
index b6bef39bf..14c3a1918 100644
--- a/plugins/buildingplan.cpp
+++ b/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]));
-        out << "buildingplan debugging: " <<
-                ((show_debugging) ? "enabled" : "disabled") << endl;
+        show_debugging = isTrue(parameters[1]);
+        out.print("buildingplan debugging: %s\n",
+                  show_debugging ? "enabled" : "disabled");
     }
     else if (cmd == "set")
     {
         if (!is_enabled)
         {
-            out << "ERROR: buildingplan must be enabled before you can read or "
-                << "set buildingplan global settings." << endl;
+            out.printerr(
+                "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 "
-                << "buildingplan global settings. Try adding your "
-                << "'buildingplan set' commands to the onMapLoad.init file."
-                << endl;
+            out.printerr(
+                "ERROR: A map must be loaded before you can read or set"
+                "buildingplan global settings. Try adding your"
+                "'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 << " = "
-                    << (setting.second ? "true" : "false") << endl;
+                out.print("  %s = %s\n", setting.first.c_str(),
+                          setting.second ? "true" : "false");
             }
 
-            out << "  quickfort_mode = "
-                << (quickfort_mode ? "true" : "false") << endl;
+            out.print("  quickfort_mode = %s\n",
+                      quickfort_mode ? "true" : "false");
         }
         else if (parameters.size() == 3)
         {
             // set a setting
             std::string setting = toLower(parameters[1]);
-            bool val = isTrue(toLower(parameters[2]));
-            setSetting(setting, val);
+            bool val = isTrue(parameters[2]);
+            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 {