Route the speed dial popup through the ActionRegistry

src/slic3r/GUI/ActionRegistry.cpp

@@ -14,6 +14,7 @@
 #include <algorithm>
 #include <cmath>
 #include <ctime>
+#include <unordered_map>
 
 namespace Slic3r { namespace GUI {
 
@@ -57,6 +58,14 @@ double frecency_score(int count, long long last, long long now)
     return count * std::pow(2.0, -age / HALF_LIFE_DAYS);
 }
 
+std::string find_loaded_package_name(PluginLoader& loader, const std::string& plugin_key)
+{
+    for (const PluginDescriptor& desc : loader.get_all_loaded_plugin_descriptors())
+        if (desc.plugin_key == plugin_key)
+            return desc.name.empty() ? plugin_key : desc.name;
+    return plugin_key;
+}
+
 } // namespace
 
 ScriptRunOutcome AppAction::run() const
@@ -66,7 +75,9 @@ ScriptRunOutcome AppAction::run() const
 
 // ---- build / enumeration ----------------------------------------------------
 
-bool ActionRegistry::make_action(const std::string& plugin_key, const std::string& capability, AppAction& out) const
+bool ActionRegistry::make_action(const std::string& plugin_key, const std::string& capability,
+                                 const std::function<std::string(const std::string&)>& package_name_for,
+                                 AppAction& out) const
 {
     PluginLoader& loader = PluginManager::instance().get_loader();
     if (!loader.is_plugin_loaded(plugin_key))
@@ -75,13 +86,9 @@ bool ActionRegistry::make_action(const std::string& plugin_key, const std::strin
     if (!cap || !cap->enabled)
         return false;
 
-    std::string pkg = plugin_key;
-    for (const PluginDescriptor& desc : loader.get_all_loaded_plugin_descriptors())
-        if (desc.plugin_key == plugin_key) { if (!desc.name.empty()) pkg = desc.name; break; }
-
     out.id         = speed_dial_action_id(plugin_key, capability);
     out.title      = capability.empty() ? plugin_key : capability;
-    out.pkg        = pkg;
+    out.pkg        = package_name_for(plugin_key);
     out.plugin_key = plugin_key;
     out.capability = capability;
     out.runnable   = true;
@@ -110,23 +117,30 @@ void ActionRegistry::build()
     assert(wxThread::IsMain()); // why: UI-thread only; m_actions is unguarded (confinement)
     m_actions.clear();
     PluginLoader& loader = PluginManager::instance().get_loader();
+
+    std::unordered_map<std::string, std::string> package_names;
+    for (const PluginDescriptor& desc : loader.get_all_loaded_plugin_descriptors())
+        if (!desc.name.empty())
+            package_names.emplace(desc.plugin_key, desc.name);
+
+    // why: build sees many capabilities, so package display names are indexed once instead of
+    // scanning the loaded descriptors for every capability.
+    auto package_name_for = [&package_names](const std::string& plugin_key) {
+        auto it = package_names.find(plugin_key);
+        return it == package_names.end() ? plugin_key : it->second;
+    };
+
     for (const auto& cap : loader.get_plugin_capabilities_by_type(PluginCapabilityType::Script)) {
         if (!cap || !cap->enabled)
             continue;
         AppAction a;
-        if (make_action(cap->plugin_key, cap->name, a))
+        if (make_action(cap->plugin_key, cap->name, package_name_for, a))
             m_actions.push_back(std::move(a));
     }
 }
 
 // ---- read surface -----------------------------------------------------------
 
-const std::vector<AppAction>& ActionRegistry::all() const
-{
-    assert(wxThread::IsMain());
-    return m_actions;
-}
-
 const AppAction* ActionRegistry::by_id(const std::string& id) const
 {
     assert(wxThread::IsMain());
@@ -136,8 +150,7 @@ const AppAction* ActionRegistry::by_id(const std::string& id) const
 
 AppAction* ActionRegistry::find(const std::string& id)
 {
-    auto it = std::find_if(m_actions.begin(), m_actions.end(), [&](const AppAction& a) { return a.id == id; });
+    return const_cast<AppAction*>(by_id(id));
-    return it == m_actions.end() ? nullptr : &*it;
 }
 
 void ActionRegistry::erase_id(const std::string& id)
@@ -157,8 +170,14 @@ void ActionRegistry::refresh_capability(const std::string& plugin_key, const std
         erase_id(id);
         return;
     }
+    PluginLoader& loader = PluginManager::instance().get_loader();
+    const std::string package_name = find_loaded_package_name(loader, plugin_key);
+    // why: make_action only ever resolves this single plugin_key here, unlike build()'s
+    // multi-plugin lookup, so the callback can just return the already-resolved name.
+    auto package_name_for = [&package_name](const std::string&) { return package_name; };
+
     AppAction a;
-    if (!make_action(plugin_key, capability, a)) { // disabled / unloaded -> treat as gone
+    if (!make_action(plugin_key, capability, package_name_for, a)) { // why: disabled/unloaded capabilities leave the dial.
         erase_id(id);
         return;
     }
@@ -178,11 +197,16 @@ void ActionRegistry::refresh_package(const std::string& plugin_key, ActionChange
     if (change == ActionChange::Removed)
         return;
     PluginLoader& loader = PluginManager::instance().get_loader();
+    const std::string package_name = find_loaded_package_name(loader, plugin_key);
+    // why: every capability iterated below belongs to this single plugin_key, so the
+    // callback can just return the already-resolved name (see refresh_capability).
+    auto package_name_for = [&package_name](const std::string&) { return package_name; };
+
     for (const auto& cap : loader.get_plugin_capabilities_by_type(plugin_key, PluginCapabilityType::Script)) {
         if (!cap || !cap->enabled)
             continue;
         AppAction a;
-        if (make_action(plugin_key, cap->name, a))
+        if (make_action(plugin_key, cap->name, package_name_for, a))
             m_actions.push_back(std::move(a));
     }
 }
@@ -275,8 +299,7 @@ nlohmann::json ActionRegistry::snapshot() const
                            {"shortcut", ""}});
     // why: favourites is the ORDERED pin list - it must come from favourite_actions
     // as stored, not be re-derived from the frecency-sorted actions (that would
-    // reorder the favourites bar and renumber its Alt+1..9 targets). The page
+    // reorder the favourites bar). The page (js) filters out ids with no live action itself.
-    // filters out ids with no live action itself.
     nlohmann::json favourites(read_string_array("favourite_actions"));
     return {{"actions", std::move(actions)}, {"favourites", std::move(favourites)}};
 }

src/slic3r/GUI/ActionRegistry.hpp

@@ -4,6 +4,10 @@
 
 #include <nlohmann/json.hpp>
 
+#include <wx/thread.h>
+
+#include <cassert>
+#include <functional>
 #include <string>
 #include <vector>
 
@@ -36,9 +40,6 @@ struct AppAction
 // Always clean when read: built once at startup, then maintained incrementally by
 // loader callbacks (via CallAfter on the UI thread). The action vector is UI-thread
 // confined and deliberately unguarded.
-//
-// ponytail: AppAction is a plain struct with run(), not a polymorphic base — one action
-// type exists today. Reintroduce the hierarchy when a second action type shows up.
 class ActionRegistry
 {
 public:
@@ -46,7 +47,7 @@ public:
     void build();
 
     // Always-clean read surface. UI thread only.
-    const std::vector<AppAction>& all() const;
+    const std::vector<AppAction>& all() const { assert(wxThread::IsMain()); return m_actions; }
     const AppAction*              by_id(const std::string& id) const;
 
     // Targeted incremental maintenance, reached via CallAfter from loader callbacks.
@@ -71,9 +72,14 @@ private:
     void         erase_id(const std::string& id);
     // Builds the AppAction for one loaded+enabled script capability; returns false
     // (and builds nothing) when the capability is unloaded, missing, or disabled.
-    bool         make_action(const std::string& plugin_key, const std::string& capability, AppAction& out) const;
+    bool         make_action(const std::string& plugin_key, const std::string& capability,
+                             const std::function<std::string(const std::string&)>& package_name_for,
+                             AppAction& out) const;
 
-    std::vector<AppAction> m_actions;  // UI-thread confined; no lock (see spec, thread confinement)
+    // why: vector storage favors the popup path, which snapshots by iterating, sorting, and
+    // serializing small action sets. Id lookups are event-driven and build is startup-only, so
+    // a persistent map would add hashing/allocation cost without helping render.
+    std::vector<AppAction> m_actions;  // UI-thread confined; no lock
 };
 
 }} // namespace Slic3r::GUI

src/slic3r/GUI/SpeedDialDialog.cpp

@@ -1,5 +1,6 @@
 #include "SpeedDialDialog.hpp"
 
+#include "ActionRegistry.hpp"
 #include "GUI.hpp"
 #include "GUI_App.hpp"
 #include "MainFrame.hpp"
@@ -11,19 +12,10 @@
 #include "Widgets/WebView.hpp"
 #include "Widgets/WebViewHostDialog.hpp"
 #include "Widgets/StateColor.hpp"
-#include "slic3r/plugin/PluginManager.hpp"
-
-#include <libslic3r/AppConfig.hpp>
-
-#include <slic3r/plugin/PluginLoader.hpp>
-#include <slic3r/plugin/PythonPluginInterface.hpp>
 
 #include <boost/filesystem.hpp>
 
 #include <algorithm>
-#include <cmath>
-#include <ctime>
-#include <map>
 
 #include <wx/display.h>
 #include <wx/sizer.h>
@@ -34,50 +26,12 @@ namespace Slic3r { namespace GUI {
 
 namespace {
 
-// AppConfig section for the speed dial (favourites, run stats, ask suppression).
-constexpr const char* kConfigSection = "speed_dial";
 // ADJUST WIDTH HERE (DIP px). Fixed popup width; was 360, now 1.5x. Height is not set here -
 // the popup auto-resizes to the page content (see resize_to_content + the list max-height in style.css).
 constexpr int kPopupWidth = 540;
 constexpr int kPopupMinHeight = 60;   // just above the bare search-bar height, so the popup hugs content
 constexpr int kPopupMaxHeight = 282;
 
-// Tolerant parse of a config-stored JSON value; anything unreadable degrades to `fallback`
-// (the section is brand new, so damaged/absent values just mean empty defaults).
-nlohmann::json parse_config_json(const std::string& value, nlohmann::json fallback)
-{
-    nlohmann::json parsed = nlohmann::json::parse(value, nullptr, false);
-    return parsed.is_discarded() ? std::move(fallback) : parsed;
-}
-
-// Stable action identity for the page: type-prefixed (plugin_key, capability) pair.
-std::string action_id(const std::string& key, const std::string& cap) { return "script:" + key + "::" + cap; }
-
-// frecency = frequency + recency
-double frecency_score(int count, long long last, long long now)
-{
-    if (count <= 0)
-        return 0.0;
-    constexpr double HALF_LIFE_DAYS = 30.0; // tunable knob: score halves every 30 idle days
-    double age = std::max(0.0, double(now - last) / 86400.0);
-    return count * std::pow(2.0, -age / HALF_LIFE_DAYS);
-}
-
-// why: STATIC free function, not a member - run_action Dismisses (destroys) the popup before
-// the deferred run, so there is no `this` left to bump on. Reads/writes config directly.
-void bump_frecency(const std::string& id)
-{
-    auto stats = parse_config_json(wxGetApp().app_config->get(kConfigSection, "stats"), nlohmann::json::object());
-    // why: stats[id] on a new id yields a null node; value("count",...) throws type_error.306 on a
-    // non-object, so seed it to {} first (the first run of every action hit this before the toast).
-    nlohmann::json& e = stats[id];
-    if (!e.is_object())
-        e = nlohmann::json::object();
-    e["count"] = e.value("count", 0) + 1;
-    e["last"]  = (long long) std::time(nullptr);
-    wxGetApp().app_config->set(kConfigSection, "stats", stats.dump());
-}
-
 wxString dialog_url()
 {
     boost::filesystem::path path = boost::filesystem::path(resources_dir()) / "web/dialog/SpeedDial/index.html";
@@ -234,7 +188,7 @@ private:
             m_page_ready = true;
             send_actions();
         }
-        else if (cmd == "toggle_favourite") toggle_favourite(p.value("id", ""), p.value("fav", false));
+        else if (cmd == "toggle_favourite") wxGetApp().action_registry().set_favourite(p.value("id", ""), p.value("fav", false));
         else if (cmd == "run_action")       run_action(p.value("id", ""), p.value("title", ""));
         else if (cmd == "resize")           resize_to_content(json_int_or(p, "height", 0));
         else if (cmd == "close_page")       Dismiss();
@@ -285,65 +239,18 @@ private:
         m_pending_show = false;
     }
 
-    // Config readers/writer for the persisted dial state (favourites, run stats, ask suppression).
+    // Hide FIRST (scripts may open their own modals that must not stack under this
-    std::vector<std::string> read_favourites() const
+    // launcher), gate on a native run-confirm unless suppressed, then run on the next
-    {
+    // tick via the registry (which re-resolves the capability and bumps stats).
-        auto j = parse_config_json(wxGetApp().app_config->get(kConfigSection, "favourites"), nlohmann::json::array());
-        std::vector<std::string> v;
-        for (auto& e : j)
-            if (e.is_string())
-                v.push_back(e.get<std::string>());
-        return v;
-    }
-    nlohmann::json read_stats() const
-    {
-        return parse_config_json(wxGetApp().app_config->get(kConfigSection, "stats"), nlohmann::json::object());
-    }
-    std::vector<std::string> read_ask_suppressed() const
-    {
-        auto j = parse_config_json(wxGetApp().app_config->get(kConfigSection, "ask_suppressed"), nlohmann::json::array());
-        std::vector<std::string> v;
-        for (auto& e : j)
-            if (e.is_string())
-                v.push_back(e.get<std::string>());
-        return v;
-    }
-    static void write_json(const char* key, const nlohmann::json& j) { wxGetApp().app_config->set(kConfigSection, key, j.dump()); }
-
-    void toggle_favourite(const std::string& id, bool fav)
-    {
-        auto favs = read_favourites();
-        auto it   = std::find(favs.begin(), favs.end(), id);
-        if (fav && it == favs.end())
-            favs.push_back(id);
-        if (!fav && it != favs.end())
-            favs.erase(it);
-        write_json("favourites", nlohmann::json(favs));
-    }
-
-    void suppress_ask_for(const std::string& key)
-    {
-        auto arr = read_ask_suppressed();
-        if (std::find(arr.begin(), arr.end(), key) == arr.end())
-            arr.push_back(key);
-        write_json("ask_suppressed", nlohmann::json(arr));
-    }
-
-    // id == "script:<key>::<cap>". Hide FIRST (scripts may open their own modals that must not stack
-    // under this launcher), gate on a native run-confirm unless suppressed, then run on the next tick.
     void run_action(const std::string& id, const std::string& title)
     {
-        const std::string prefix = "script:";
+        ActionRegistry& reg = wxGetApp().action_registry();
-        if (id.rfind(prefix, 0) != 0)
+        const AppAction* a = reg.by_id(id);
+        if (!a)
             return;
-        auto body = id.substr(prefix.size());
+        const std::string key = a->plugin_key;
-        auto sep  = body.find("::");
+        const std::string cap = a->capability;
-        if (sep == std::string::npos)
+        const bool ask = reg.should_ask(key);
-            return;
-        std::string key = body.substr(0, sep), cap = body.substr(sep + 2);
-
-        auto suppressed = read_ask_suppressed();
-        const bool ask  = std::find(suppressed.begin(), suppressed.end(), key) == suppressed.end();
         Dismiss(); // launcher gone before the modal / the script's own UI
 
         if (ask) {
@@ -354,14 +261,13 @@ private:
             if (dlg.ShowModal() != wxID_OK)
                 return;
             if (dlg.IsCheckBoxChecked())
-                suppress_ask_for(key);
+                wxGetApp().action_registry().suppress_ask(key);
         }
         // why: value-capture only. Script execution may outlive this popup if the app is closing.
-        wxGetApp().CallAfter([id, key, cap] {
+        wxGetApp().CallAfter([id] {
-            ScriptRunOutcome o = run_script_plugin_capability(key, cap);
+            ScriptRunOutcome o = wxGetApp().action_registry().run(id);
             if (o.level == ScriptRunOutcome::Level::Busy)
                 return;
-            bump_frecency(id);
             if (!o.message.IsEmpty())
                 wxGetApp().plater()->get_notification_manager()->push_notification(
                     NotificationType::CustomNotification,
@@ -371,26 +277,6 @@ private:
         });
     }
 
-    // Frecency ordering so the page shows most-reached-for first; ties broken alphabetically by title.
-    void sort_by_frecency(nlohmann::json& actions) const
-    {
-        nlohmann::json stats = read_stats();
-        long long now        = (long long) std::time(nullptr);
-        std::stable_sort(actions.begin(), actions.end(), [&](const nlohmann::json& a, const nlohmann::json& b) {
-            auto sc = [&](const nlohmann::json& x) {
-                auto it = stats.find(x["id"].get<std::string>());
-                // why: guard is_object() too - value() throws on a malformed (non-object) stats entry.
-                if (it == stats.end() || !it->is_object())
-                    return 0.0;
-                return frecency_score(it->value("count", 0), it->value("last", 0LL), now);
-            };
-            double sa = sc(a), sb = sc(b);
-            if (sa != sb)
-                return sa > sb;
-            return a["title"].get<std::string>() < b["title"].get<std::string>();
-        });
-    }
-
     // C++ -> JS push. Same escaping as WebViewHostDialog::call_web_handler: the ignore error
     // handler keeps a stray non-UTF-8 byte in a plugin name from throwing and aborting the send;
     // concatenation (not wxString::Format) avoids a '%' in a title being read as a format token.
@@ -402,40 +288,12 @@ private:
         WebView::RunScript(m_browser, wxT("window.HandleStudio(") + payload + wxT(")"));
     }
 
-    // Only runnable actions are offered: enabled AND loaded SCRIPT capabilities. Unloaded or
-    // invalid ones stay hidden (this transient launcher has no greyed/edit state yet).
-    nlohmann::json build_actions() const
-    {
-        PluginLoader& loader = PluginManager::instance().get_loader();
-
-        // plugin_key -> package display name (the row eyebrow); falls back to the key.
-        std::map<std::string, std::string> pkg_name;
-        for (const PluginDescriptor& desc : loader.get_all_loaded_plugin_descriptors())
-            pkg_name[desc.plugin_key] = desc.name;
-
-        nlohmann::json arr = nlohmann::json::array();
-        for (const auto& cap : loader.get_plugin_capabilities_by_type(PluginCapabilityType::Script)) {
-            if (!cap || !cap->enabled)
-                continue;
-            if (!loader.is_plugin_loaded(cap->plugin_key))
-                continue;
-            const auto it = pkg_name.find(cap->plugin_key);
-            arr.push_back({{"id", action_id(cap->plugin_key, cap->name)},
-                           {"title", cap->name.empty() ? cap->plugin_key : cap->name},
-                           {"pkg", (it != pkg_name.end() && !it->second.empty()) ? it->second : cap->plugin_key},
-                           {"runnable", true},
-                           {"shortcut", ""}});
-        }
-        return arr;
-    }
-
     void send_actions()
     {
-        nlohmann::json actions = build_actions();
+        nlohmann::json snap = wxGetApp().action_registry().snapshot();
-        sort_by_frecency(actions);
         push({{"command", "list_actions"},
-              {"actions", std::move(actions)},
+              {"actions", std::move(snap["actions"])},
-              {"favourites", read_favourites()}});
+              {"favourites", std::move(snap["favourites"])}});
     }
 
     wxWebView* m_browser{nullptr};