Fix some confirm plugin issues and add a simple configuration UI

- Detect null pointers in trade-related functions
- Fix typo/issues pointed out by @dscorbett
- Reorder includes

plugins/confirm.cpp

@@ -1,15 +1,18 @@
-#include <set>
 #include <map>
+#include <set>
+#include <queue>
+
 #include "Console.h"
 #include "Core.h"
 #include "DataDefs.h"
+#include "Error.h"
 #include "Export.h"
-#include "PluginManager.h"
-#include "VTableInterpose.h"
 #include "LuaTools.h"
 #include "LuaWrapper.h"
-#include "uicommon.h"
+#include "PluginManager.h"
+#include "VTableInterpose.h"
 #include "modules/Gui.h"
+#include "uicommon.h"
 
 #include "df/building_tradedepotst.h"
 #include "df/general_ref.h"
@@ -32,7 +35,9 @@ typedef std::set<df::interface_key> ikey_set;
 command_result df_confirm (color_ostream &out, vector <string> & parameters);
 
 struct conf_wrapper;
-static std::map<std::string, conf_wrapper*> confirmations;
+static std::map<string, conf_wrapper*> confirmations;
+string active_id;
+std::queue<string> cmds;
 
 template <typename VT, typename FT>
 inline bool in_vector (std::vector<VT> &vec, FT item)
@@ -50,6 +55,35 @@ string char_replace (string s, char a, char b)
     return res;
 }
 
+bool set_conf_state (string name, bool state);
+
+struct conf_wrapper {
+private:
+    bool enabled;
+    std::set<VMethodInterposeLinkBase*> hooks;
+public:
+    conf_wrapper()
+        :enabled(false)
+    {}
+    void add_hook(VMethodInterposeLinkBase *hook)
+    {
+        if (!hooks.count(hook))
+            hooks.insert(hook);
+    }
+    bool apply (bool state) {
+        if (state == enabled)
+            return true;
+        for (auto h = hooks.begin(); h != hooks.end(); ++h)
+        {
+            if (!(**h).apply(state))
+                return false;
+        }
+        enabled = state;
+        return true;
+    }
+    inline bool is_enabled() { return enabled; }
+};
+
 namespace trade {
     static bool goods_selected (const std::vector<char> &selected)
     {
@@ -60,10 +94,12 @@ namespace trade {
     }
     inline bool trader_goods_selected (df::viewscreen_tradegoodsst *screen)
     {
+        CHECK_NULL_POINTER(screen);
         return goods_selected(screen->trader_selected);
     }
     inline bool broker_goods_selected (df::viewscreen_tradegoodsst *screen)
     {
+        CHECK_NULL_POINTER(screen);
         return goods_selected(screen->broker_selected);
     }
 
@@ -93,10 +129,12 @@ namespace trade {
     }
     inline bool trader_goods_all_selected(df::viewscreen_tradegoodsst *screen)
     {
+        CHECK_NULL_POINTER(screen);
         return goods_all_selected(screen->trader_selected, screen->trader_items);
     }
     inline bool broker_goods_all_selected(df::viewscreen_tradegoodsst *screen)
     {
+        CHECK_NULL_POINTER(screen);
         return goods_all_selected(screen->broker_selected, screen->broker_items);
     }
 }
@@ -127,6 +165,11 @@ namespace conf_lua {
             lua_insert(l_state, lua_gettop(l_state) - nargs);
         return Lua::SafeCall(*out, l_state, nargs, nres);
     }
+    bool simple_call (const char *func)
+    {
+        Lua::StackUnwinder top(l_state);
+        return call(func, 0, 0);
+    }
     template <typename T>
     void push (T val)
     {
@@ -147,11 +190,34 @@ namespace conf_lua {
                 table_set(L, it->first, true);
             return 1;
         }
+        int get_conf_data (lua_State *L)
+        {
+            lua_newtable(L);
+            int i = 1;
+            for (auto it = confirmations.begin(); it != confirmations.end(); ++it)
+            {
+                Lua::Push(L, i++);
+                lua_newtable(L);
+                table_set(L, "id", it->first);
+                table_set(L, "enabled", it->second->is_enabled());
+                lua_settable(L, -3);
+            }
+            return 1;
+        }
+        int get_active_id (lua_State *L)
+        {
+            if (active_id.size())
+                Lua::Push(L, active_id);
+            else
+                lua_pushnil(L);
+            return 1;
+        }
     }
 }
 
 #define CONF_LUA_FUNC(ns, name) {#name, df::wrap_function(ns::name, true)}
 DFHACK_PLUGIN_LUA_FUNCTIONS {
+    CONF_LUA_FUNC( , set_conf_state),
     CONF_LUA_FUNC(trade, broker_goods_selected),
     CONF_LUA_FUNC(trade, broker_goods_all_selected),
     CONF_LUA_FUNC(trade, trader_goods_selected),
@@ -162,15 +228,30 @@ DFHACK_PLUGIN_LUA_FUNCTIONS {
 #define CONF_LUA_CMD(name) {#name, conf_lua::api::name}
 DFHACK_PLUGIN_LUA_COMMANDS {
     CONF_LUA_CMD(get_ids),
+    CONF_LUA_CMD(get_conf_data),
+    CONF_LUA_CMD(get_active_id),
     DFHACK_LUA_END
 };
 
+void show_options()
+{
+    cmds.push("gui/confirm-opts");
+}
+
 template <class T>
 class confirmation {
 public:
     enum cstate { INACTIVE, ACTIVE, SELECTED };
     typedef T screen_type;
     screen_type *screen;
+    void set_state (cstate s)
+    {
+        state = s;
+        if (s == INACTIVE)
+            active_id = "";
+        else
+            active_id = get_id();
+    }
     bool feed (ikey_set *input) {
         if (state == INACTIVE)
         {
@@ -179,7 +260,7 @@ public:
                 if (intercept_key(*it))
                 {
                     last_key = *it;
-                    state = ACTIVE;
+                    set_state(ACTIVE);
                     return true;
                 }
             }
@@ -188,9 +269,11 @@ public:
         else if (state == ACTIVE)
         {
             if (input->count(df::interface_key::LEAVESCREEN))
-                state = INACTIVE;
+                set_state(INACTIVE);
             else if (input->count(df::interface_key::SELECT))
-                state = SELECTED;
+                set_state(SELECTED);
+            else if (input->count(df::interface_key::CUSTOM_S))
+                show_options();
             return true;
         }
         return false;
@@ -212,7 +295,7 @@ public:
         if (state == ACTIVE)
         {
             split_string(&lines, get_message(), "\n");
-            size_t max_length = 30;
+            size_t max_length = 40;
             for (auto it = lines.begin(); it != lines.end(); ++it)
                 max_length = std::max(max_length, it->size());
             int width = max_length + 4;
@@ -241,11 +324,15 @@ public:
             Screen::paintString(Screen::Pen(' ', COLOR_BLACK, COLOR_GREY),
                 (gps->dimx / 2) - (title.size() / 2), y1, title);
             int x = x1 + 2;
-            OutputString(COLOR_LIGHTGREEN, x, y2, Screen::getKeyDisplay(df::interface_key::LEAVESCREEN));
+            int y = y2;
-            OutputString(COLOR_WHITE, x, y2, ": Cancel");
+            OutputString(COLOR_LIGHTRED, x, y, Screen::getKeyDisplay(df::interface_key::LEAVESCREEN));
+            OutputString(COLOR_WHITE, x, y, ": Cancel");
+            x = (gps->dimx - (Screen::getKeyDisplay(df::interface_key::CUSTOM_S) + ": Settings").size()) / 2 + 1;
+            OutputString(COLOR_LIGHTRED, x, y, Screen::getKeyDisplay(df::interface_key::CUSTOM_S));
+            OutputString(COLOR_WHITE, x, y, ": Settings");
             x = x2 - 2 - 3 - Screen::getKeyDisplay(df::interface_key::SELECT).size();
-            OutputString(COLOR_LIGHTGREEN, x, y2, Screen::getKeyDisplay(df::interface_key::SELECT));
+            OutputString(COLOR_LIGHTRED, x, y, Screen::getKeyDisplay(df::interface_key::SELECT));
-            OutputString(COLOR_WHITE, x, y2, ": Ok");
+            OutputString(COLOR_WHITE, x, y, ": Ok");
             Screen::fillRect(Screen::Pen(' ', COLOR_BLACK, COLOR_BLACK), x1 + 1, y1 + 1, x2 - 1, y2 - 1);
             for (size_t i = 0; i < lines.size(); i++)
             {
@@ -257,7 +344,7 @@ public:
             ikey_set tmp;
             tmp.insert(last_key);
             screen->feed(&tmp);
-            state = INACTIVE;
+            set_state(INACTIVE);
         }
     }
     virtual string get_id() = 0;
@@ -301,37 +388,10 @@ protected:
     df::interface_key last_key;
 };
 
-struct conf_wrapper {
-private:
-    bool enabled;
-    std::set<VMethodInterposeLinkBase*> hooks;
-public:
-    conf_wrapper()
-        :enabled(false)
-    {}
-    void add_hook(VMethodInterposeLinkBase *hook)
-    {
-        if (!hooks.count(hook))
-            hooks.insert(hook);
-    }
-    bool apply (bool state) {
-        if (state == enabled)
-            return true;
-        for (auto h = hooks.begin(); h != hooks.end(); ++h)
-        {
-            if (!(**h).apply(state))
-                return false;
-        }
-        enabled = state;
-        return true;
-    }
-    inline bool is_enabled() { return enabled; }
-};
-
 template<typename T>
 int conf_register(confirmation<T> *c, ...)
 {
-    conf_wrapper *w = new conf_wrapper;
+    conf_wrapper *w = new conf_wrapper();
     confirmations[c->get_id()] = w;
     va_list args;
     va_start(args, c);
@@ -423,9 +483,7 @@ DFhackCExport command_result plugin_enable (color_ostream &out, bool enable)
     }
     if (is_enabled)
     {
-        using namespace conf_lua;
+        conf_lua::simple_call("check");
-        Lua::StackUnwinder unwind(l_state);
-        call("check");
     }
     return CR_OK;
 }
@@ -438,7 +496,17 @@ DFhackCExport command_result plugin_shutdown (color_ostream &out)
     return CR_OK;
 }
 
-void enable_conf (color_ostream &out, string name, bool state)
+DFhackCExport command_result plugin_onupdate (color_ostream &out)
+{
+    while (!cmds.empty())
+    {
+        Core::getInstance().runCommand(out, cmds.front());
+        cmds.pop();
+    }
+    return CR_OK;
+}
+
+bool set_conf_state (string name, bool state)
 {
     bool found = false;
     for (auto it = confirmations.begin(); it != confirmations.end(); ++it)
@@ -449,7 +517,12 @@ void enable_conf (color_ostream &out, string name, bool state)
             it->second->apply(state);
         }
     }
-    if (!found)
+    return found;
+}
+
+void enable_conf (color_ostream &out, string name, bool state)
+{
+    if (!set_conf_state(name, state))
         out.printerr("Unrecognized option: %s\n", name.c_str());
 }
 

plugins/lua/confirm.lua

@@ -7,7 +7,7 @@ local confs = {}
 keys = {}
 setmetatable(keys, {
     __index = function(self, k)
-        return df.interface_key[k] and df.interface_key[k] or error('Invalid key: ' .. tostring(k))
+        return df.interface_key[k] or error('Invalid key: ' .. tostring(k))
     end,
     __newindex = function() error('Table is read-only') end
 })
@@ -95,7 +95,7 @@ function trade_seize.intercept_key(key)
         key == keys.TRADE_SEIZE
 end
 trade_seize.title = "Confirm seize"
-trade_seize.message = "Are you sure you want to sieze these goods?"
+trade_seize.message = "Are you sure you want to seize these goods?"
 
 trade_offer = defconf('trade-offer')
 function trade_offer.intercept_key(key)

scripts/gui/confirm-opts.lua

@@ -0,0 +1,74 @@
+-- confirm plugin options
+--[[=begin
+
+gui/confirm-opts
+================
+A basic configuration interface for the `confirm` plugin.
+
+=end]]
+
+
+confirm = require 'plugins.confirm'
+gui = require 'gui'
+
+Opts = defclass(Opts, gui.FramedScreen)
+Opts.ATTRS = {
+    frame_style = gui.GREY_LINE_FRAME,
+    frame_title = 'Confirmation dialogs',
+    frame_width = 32,
+    frame_height = 20,
+    frame_inset = 1,
+    focus_path = 'confirm/opts',
+}
+
+function Opts:init()
+    self:refresh()
+    self.cursor = 1
+    local active_id = confirm.get_active_id()
+    for i, c in pairs(self.data) do
+        if c.id == active_id then
+            self.cursor = i
+            break
+        end
+    end
+end
+
+function Opts:refresh()
+    self.data = confirm.get_conf_data()
+    self.frame_height = #self.data
+end
+
+function Opts:onRenderBody(p)
+    for i, c in pairs(self.data) do
+        local highlight = (i == self.cursor and 8 or 0)
+        p:pen(COLOR_GREY + highlight)
+        p:string(c.id .. ': ')
+        p:pen((c.enabled and COLOR_GREEN or COLOR_RED) + highlight)
+        p:string(c.enabled and 'Enabled' or 'Disabled')
+        p:newline()
+    end
+end
+
+function Opts:onInput(keys)
+    local conf = self.data[self.cursor]
+    if keys.LEAVESCREEN then
+        self:dismiss()
+    elseif keys.SELECT then
+        confirm.set_conf_state(conf.id, not conf.enabled)
+        self:refresh()
+    elseif keys.SEC_SELECT then
+        for _, c in pairs(self.data) do
+            confirm.set_conf_state(c.id, not conf.enabled)
+        end
+        self:refresh()
+    elseif keys.STANDARDSCROLL_UP or keys.STANDARDSCROLL_DOWN then
+        self.cursor = self.cursor + (keys.STANDARDSCROLL_UP and -1 or 1)
+        if self.cursor < 1 then
+            self.cursor = #self.data
+        elseif self.cursor > #self.data then
+            self.cursor = 1
+        end
+    end
+end
+
+Opts():show()