From c29ffdfc1af6e58c462f648b6e6e8b4fb6c016b0 Mon Sep 17 00:00:00 2001 From: Andrew <159703254+andrewsoonqn@users.noreply.github.com> Date: Mon, 13 Jul 2026 12:31:33 +0800 Subject: [PATCH] Extract script actions into an action source Move plugin capability enumeration, loader subscriptions, and action construction into ScriptActionSource. Keep ActionRegistry focused on generic action state, dispatch, and snapshots. Use source rather than package for generic origin metadata so future non-plugin providers share the same interface. Action IDs and persisted configuration remain unchanged. Subscribe before initial enumeration so plugin events cannot be missed between the snapshot and callback registration. Add a focused test for source startup. --- resources/web/dialog/SpeedDial/speeddial.js | 44 ++--- src/slic3r/CMakeLists.txt | 3 + src/slic3r/GUI/ActionRegistry.cpp | 164 +++------------- src/slic3r/GUI/ActionRegistry.hpp | 93 ++++----- src/slic3r/GUI/GUI_App.cpp | 33 +--- src/slic3r/GUI/IActionSource.hpp | 14 ++ src/slic3r/GUI/ScriptActionSource.cpp | 205 ++++++++++++++++++++ src/slic3r/GUI/ScriptActionSource.hpp | 41 ++++ tests/slic3rutils/CMakeLists.txt | 1 + tests/slic3rutils/test_action_source.cpp | 35 ++++ 10 files changed, 396 insertions(+), 237 deletions(-) create mode 100644 src/slic3r/GUI/IActionSource.hpp create mode 100644 src/slic3r/GUI/ScriptActionSource.cpp create mode 100644 src/slic3r/GUI/ScriptActionSource.hpp create mode 100644 tests/slic3rutils/test_action_source.cpp diff --git a/resources/web/dialog/SpeedDial/speeddial.js b/resources/web/dialog/SpeedDial/speeddial.js index 51ad65e1be..436a8c9a00 100644 --- a/resources/web/dialog/SpeedDial/speeddial.js +++ b/resources/web/dialog/SpeedDial/speeddial.js @@ -2,7 +2,7 @@ // node vm can exercise the pure helpers (filterActions / parseId / nextSel). // ---- state (populated by the C++ bridge via window.HandleStudio) ---- -var ACTIONS = []; // [{id,title,pkg,shortcut}], already frecency-sorted by C++ +var ACTIONS = []; // [{id,title,source,shortcut}], already frecency-sorted by C++ var FAVS = []; // [id...] var query = ""; var sel = { zone: "list", i: 0 }; // zone: 'list' | 'fav' @@ -27,10 +27,10 @@ function filterActions(actions, query) { for (var i = 0; i < list.length; i++) { var a = list[i]; var titleMatch = FuzzyRanges(a.title, q, false); - var pkgMatch = FuzzyRanges(a.pkg, q, false); - if (!titleMatch && !pkgMatch) + var sourceMatch = FuzzyRanges(a.source, q, false); + if (!titleMatch && !sourceMatch) continue; - matchIndex[a.id] = { title: titleMatch, pkg: pkgMatch, useTitle: !!titleMatch }; + matchIndex[a.id] = { title: titleMatch, source: sourceMatch, useTitle: !!titleMatch }; out.push(a); } return out; @@ -68,19 +68,19 @@ function selectedActionId(sel, actions, favIds) { function foldLabel(s) { return String(s || "").toLowerCase().replace(/[^a-z0-9]+/g, ""); } -// Title-case a package for display: "GCODE OPTIMIZER"/"iRoNiNg pRo" -> "Gcode Optimizer"/"Ironing Pro". -function prettyPkg(pkg) { - return String(pkg || "").toLowerCase().replace(/\b\w/g, function (c) { return c.toUpperCase(); }); +// Title-case a source for display: "GCODE OPTIMIZER"/"iRoNiNg pRo" -> "Gcode Optimizer"/"Ironing Pro". +function prettySource(source) { + return String(source || "").toLowerCase().replace(/\b\w/g, function (c) { return c.toUpperCase(); }); } -// Accessible label "Title from Pretty Pkg", disambiguated with the plugin key when another action -// shares the same title+pkg (case/separator-insensitive) - so two rows never read out identically. +// Accessible label "Title from Pretty Source", disambiguated with the plugin key when another action +// shares the same title+source (case/separator-insensitive) - so two rows never read out identically. function actionLabel(action, actions) { - var label = action.title + " from " + prettyPkg(action.pkg); + var label = action.title + " from " + prettySource(action.source); if (actions && actions.length) { - var mine = foldLabel(action.title) + "|" + foldLabel(action.pkg); + var mine = foldLabel(action.title) + "|" + foldLabel(action.source); var clash = actions.some(function (o) { - return o.id !== action.id && foldLabel(o.title) + "|" + foldLabel(o.pkg) === mine; + return o.id !== action.id && foldLabel(o.title) + "|" + foldLabel(o.source) === mine; }); if (clash) label += " (" + parseId(action.id).key + ")"; @@ -88,7 +88,7 @@ function actionLabel(action, actions) { return label; } -// Monogram code for a tile: title initial, escalated on collision by PREPENDING the pkg +// Monogram code for a tile: title initial, escalated on collision by PREPENDING the source // initial (pi+ti, e.g. "GC"), then a 1-based ordinal - so same-titled actions stay distinct. // why: ordinal is assigned by id, not by ACTIONS order - ACTIONS is frecency-sorted and // reshuffles as usage changes, which would otherwise flip who's "1" and who's "2" across runs. @@ -98,13 +98,13 @@ function tileCode(action, actions) { var sameTitle = list.filter(function (o) { return (o.title || " ").charAt(0).toUpperCase() === ti; }); if (sameTitle.length <= 1) return ti; - var pi = (action.pkg || " ").charAt(0).toUpperCase(); - var samePkg = sameTitle.filter(function (o) { return (o.pkg || " ").charAt(0).toUpperCase() === pi; }); - if (samePkg.length <= 1) + var pi = (action.source || " ").charAt(0).toUpperCase(); + var sameSource = sameTitle.filter(function (o) { return (o.source || " ").charAt(0).toUpperCase() === pi; }); + if (sameSource.length <= 1) return pi + ti; - samePkg.sort(function (a, b) { return a.id < b.id ? -1 : a.id > b.id ? 1 : 0; }); - for (var i = 0; i < samePkg.length; i++) - if (samePkg[i].id === action.id) + sameSource.sort(function (a, b) { return a.id < b.id ? -1 : a.id > b.id ? 1 : 0; }); + for (var i = 0; i < sameSource.length; i++) + if (sameSource[i].id === action.id) return pi + ti + (i + 1); return pi + ti; } @@ -203,7 +203,7 @@ function hue(id) { } // Build a
with the search-match ranges wrapped in . Used for both the -// title and the pkg eyebrow. Pure (only touches the document factory), so the node-vm test never +// title and the source eyebrow. Pure (only touches the document factory), so the node-vm test never // calls it and load-time stays DOM-free. function markedText(className, text, match) { var node = document.createElement("div"); @@ -359,7 +359,7 @@ function renderList() { var left = document.createElement("div"); left.className = "row-left"; var mi = matchIndex[a.id]; - var pkg = markedText("row-eyebrow", a.pkg, mi ? mi.pkg : null); + var sourceEl = markedText("row-eyebrow", a.source, mi ? mi.source : null); var line = document.createElement("div"); line.className = "row-line"; var name = markedText("row-name", a.title, mi ? mi.title : null); @@ -374,7 +374,7 @@ function renderList() { }); line.appendChild(sc); } - left.appendChild(pkg); + left.appendChild(sourceEl); left.appendChild(line); row.appendChild(tile); row.appendChild(left); diff --git a/src/slic3r/CMakeLists.txt b/src/slic3r/CMakeLists.txt index ffdc3d9830..605ac738bd 100644 --- a/src/slic3r/CMakeLists.txt +++ b/src/slic3r/CMakeLists.txt @@ -122,6 +122,9 @@ set(SLIC3R_GUI_SOURCES GUI/SpeedDialDialog.hpp GUI/ActionRegistry.cpp GUI/ActionRegistry.hpp + GUI/IActionSource.hpp + GUI/ScriptActionSource.cpp + GUI/ScriptActionSource.hpp GUI/SpeedDialActionId.hpp GUI/ProcessRunner.cpp GUI/ProcessRunner.hpp diff --git a/src/slic3r/GUI/ActionRegistry.cpp b/src/slic3r/GUI/ActionRegistry.cpp index f37023979b..d1b5b4b66b 100644 --- a/src/slic3r/GUI/ActionRegistry.cpp +++ b/src/slic3r/GUI/ActionRegistry.cpp @@ -1,20 +1,14 @@ #include "ActionRegistry.hpp" #include "GUI_App.hpp" -#include "SpeedDialActionId.hpp" -#include "slic3r/plugin/PluginManager.hpp" #include -#include -#include - #include #include #include #include -#include namespace Slic3r { namespace GUI { @@ -58,51 +52,41 @@ 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 -PluginScriptAction::PluginScriptAction(std::string plugin_key, std::string capability, std::string package_name) - : AppAction(speed_dial_action_id(plugin_key, capability), // id - capability.empty() ? plugin_key : capability, // title - std::move(package_name)), // pkg = source (package name) - plugin_key(std::move(plugin_key)), capability(std::move(capability)) -{} - -AppActionRunResult PluginScriptAction::run() const +void ActionRegistry::init() { - ScriptRunOutcome o = run_script_plugin_capability(plugin_key, capability); - AppActionRunResult::Level level = AppActionRunResult::Level::Info; - switch (o.level) { - case ScriptRunOutcome::Level::Success: level = AppActionRunResult::Level::Success; break; - case ScriptRunOutcome::Level::Error: level = AppActionRunResult::Level::Error; break; - case ScriptRunOutcome::Level::Busy: level = AppActionRunResult::Level::Busy; break; - case ScriptRunOutcome::Level::Info: break; - } - return { level, o.message }; + assert(wxThread::IsMain()); + for (auto& source : m_sources) + source->start(*this); } -// ---- build / enumeration ---------------------------------------------------- - -std::shared_ptr ActionRegistry::make_action(const std::string& plugin_key, const std::string& capability, - const std::function& package_name_for) const +void ActionRegistry::add_source(std::unique_ptr source) { - PluginLoader& loader = PluginManager::instance().get_loader(); - if (!loader.is_plugin_loaded(plugin_key)) - return nullptr; - auto cap = loader.get_plugin_capability_by_name(plugin_key, PluginCapabilityType::Script, capability); - if (!cap || !cap->enabled) - return nullptr; + assert(wxThread::IsMain()); + if (source) + m_sources.push_back(std::move(source)); +} - auto a = std::make_shared(plugin_key, capability, package_name_for(plugin_key)); - seed_state(*a); - return a; +void ActionRegistry::upsert(std::shared_ptr action) +{ + assert(wxThread::IsMain()); + if (!action) + return; + + seed_state(*action); + auto existing = std::find_if(m_actions.begin(), m_actions.end(), + [&](const auto& current) { return current->id == action->id; }); + if (existing == m_actions.end()) + m_actions.push_back(std::move(action)); + else + *existing = std::move(action); +} + +void ActionRegistry::remove(const std::string& id) +{ + assert(wxThread::IsMain()); + erase_id(id); } void ActionRegistry::seed_state(AppAction& a) const @@ -121,32 +105,6 @@ void ActionRegistry::seed_state(AppAction& a) const } } -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 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; - if (auto a = make_action(cap->plugin_key, cap->name, package_name_for)) - m_actions.push_back(std::move(a)); - } -} - // ---- read surface ----------------------------------------------------------- const AppAction* ActionRegistry::by_id(const std::string& id) const @@ -168,63 +126,6 @@ void ActionRegistry::erase_id(const std::string& id) m_actions.end()); } -// ---- incremental maintenance (UI thread, via CallAfter) --------------------- - -void ActionRegistry::refresh_capability(const std::string& plugin_key, const std::string& capability, ActionChange change) -{ - assert(wxThread::IsMain()); - const std::string id = speed_dial_action_id(plugin_key, capability); - if (change == ActionChange::Removed) { - 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; }; - - auto a = make_action(plugin_key, capability, package_name_for); - if (!a) { // why: disabled/unloaded capabilities leave the dial. - erase_id(id); - return; - } - auto it = std::find_if(m_actions.begin(), m_actions.end(), [&](const auto& e) { return e->id == id; }); - if (it != m_actions.end()) - *it = std::move(a); - else - m_actions.push_back(std::move(a)); -} - -void ActionRegistry::refresh_package(const std::string& plugin_key, ActionChange change) -{ - assert(wxThread::IsMain()); - // Drop every action of this package, then (on add/update) re-add its current script caps. - // why: this is the producer path (loader-driven), so it legitimately knows it deals with - // PluginScriptAction - the one type it builds; the cast is not the dispatch-time switch - // that run() polymorphism removed. - m_actions.erase(std::remove_if(m_actions.begin(), m_actions.end(), - [&](const auto& a) { - auto* p = dynamic_cast(a.get()); - return p && p->plugin_key == plugin_key; - }), - m_actions.end()); - 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; - if (auto a = make_action(plugin_key, cap->name, package_name_for)) - m_actions.push_back(std::move(a)); - } -} - // ---- dispatch + write-through ---------------------------------------------- AppActionRunResult ActionRegistry::run(const std::string& id) @@ -233,10 +134,9 @@ AppActionRunResult ActionRegistry::run(const std::string& id) auto it = std::find_if(m_actions.begin(), m_actions.end(), [&](const auto& e) { return e->id == id; }); if (it == m_actions.end()) return {}; // default Info, empty message - // why: hold a shared_ptr keep-alive, never a bare vector element. The script runner pumps - // a nested event loop (plugin modal UI); a queued refresh_* landing during that loop may - // erase this action from m_actions - the keep-alive keeps the heap object valid until run - // returns. (Replaces the old stack-copy; an abstract AppAction cannot be sliced.) + // why: hold a shared_ptr keep-alive, never a bare vector element. A runner may pump + // a nested event loop; a queued source refresh can erase this action from m_actions, + // while the keep-alive preserves the object until run returns. std::shared_ptr keep = *it; AppActionRunResult o = keep->run(); if (o.level == AppActionRunResult::Level::Busy) @@ -326,7 +226,7 @@ nlohmann::json ActionRegistry::snapshot() const for (const AppAction* a : sorted) actions.push_back({{"id", a->id}, {"title", a->title}, - {"pkg", a->pkg}, + {"source", a->source}, {"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 diff --git a/src/slic3r/GUI/ActionRegistry.hpp b/src/slic3r/GUI/ActionRegistry.hpp index 42bcf26b45..e6f6dd65cf 100644 --- a/src/slic3r/GUI/ActionRegistry.hpp +++ b/src/slic3r/GUI/ActionRegistry.hpp @@ -1,28 +1,22 @@ #pragma once -#include "PluginScriptRunner.hpp" // wxString + ScriptRunOutcome (mapped in the .cpp, not exposed here) +#include "IActionSource.hpp" #include +#include #include #include -#include #include #include +#include #include namespace Slic3r { namespace GUI { -enum class ActionChange { Added, Removed, Updated }; - -// Result of running an AppAction, in the ACTION layer's own vocabulary. -// why: run() must not leak plugin-specific types through the action abstraction, so -// this is deliberately NOT ScriptRunOutcome (the script/plugin layer's type) and is -// neither derived from nor composed with it. Each subclass POPULATES this from -// whatever its underlying runner returns (PluginScriptAction maps a ScriptRunOutcome -// into it). The shape mirrors ScriptRunOutcome for now; keeping them separate lets the -// two diverge freely as non-plugin action sources land. +// Result of running an AppAction, in the action layer's own vocabulary. Concrete +// actions translate their runner-specific result into this generic shape. struct AppActionRunResult { enum class Level { Success, Info, Error, Busy }; @@ -32,19 +26,17 @@ struct AppActionRunResult }; // A speed-dial action: identity + user-state seeded from config + how to run itself. -// Abstract base - the only virtual is run(); a concrete subclass (e.g. -// PluginScriptAction) knows how to run and what its pkg/source is. +// Abstract base - the only virtual is run(); concrete subclasses know how to run +// and what their source is. // note: named AppAction, not Action - Slic3r::GUI::Action is already taken by // UnsavedChangesDialog's exit-action enum, and this header reaches most GUI TUs. struct AppAction { std::string id; // speed_dial_action_id(...) - stable identity + AppConfig key std::string title; // display name - // pkg is the action's SOURCE - where it comes from. For a plugin action it is - // the owning package's display name (row eyebrow); a future app-command action - // would set its own label. Each subclass sets pkg in its constructor (below), - // so the base cannot be built without one. - std::string pkg; + // Where the action comes from. Each subclass sets it in its constructor, so the + // base cannot be built without one. + std::string source; // seeded from AppConfig for the snapshot / sort: bool favourite = false; @@ -55,45 +47,43 @@ struct AppAction virtual AppActionRunResult run() const = 0; // re-resolves + runs (UI thread) protected: - // why: protected + no default forces every subclass to supply id/title/pkg. A - // virtual compute_pkg() can't be used here - virtual dispatch from a base ctor - // resolves to the base, so this constructor-passing is the enforced substitute. - AppAction(std::string id, std::string title, std::string pkg) - : id(std::move(id)), title(std::move(title)), pkg(std::move(pkg)) {} + // why: protected + no default forces every subclass to supply id/title/source. + AppAction(std::string id, std::string title, std::string source) + : id(std::move(id)), title(std::move(title)), source(std::move(source)) {} }; -// The one action source today: a runnable SCRIPT plugin capability. Holds a weak -// (plugin_key, capability) handle, never a live capability pointer - run() -// re-resolves through the loader. -struct PluginScriptAction : AppAction -{ - std::string plugin_key; // weak handle: owning package - std::string capability; // weak handle: capability name - - PluginScriptAction(std::string plugin_key, std::string capability, std::string package_name); - AppActionRunResult run() const override; -}; - -// Single source of runnable actions for the app session, owned by GUI_App. -// 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. +// Generic sink and single owner of runnable actions for the app session. +// +// Workflow: +// 1. GUI_App transfers each concrete source to the registry with add_source(). +// 2. init() starts every source; each source enumerates its current actions and +// subscribes to future changes. +// 3. Sources push those changes through upsert() and remove(). The registry keeps +// the only action list and restores persisted user state as actions arrive. +// 4. Consumers use by_id(), snapshot(), and run() without knowing which source +// created an action. class ActionRegistry { public: - // Full sync from the loader. UI thread only. - void build(); + // Starts all registered sources. Call once on the UI thread after sources are + // added; source startup both populates the initial list and wires live updates. + void init(); + + // Transfers ownership of a source to the registry. The source remains dormant + // until init() starts it. + void add_source(std::unique_ptr source); + + // Seeds persisted state, then inserts the action or replaces the action with the + // same id. A null action is ignored. + void upsert(std::shared_ptr action); + + // Removes the action with this id. Missing ids are a harmless no-op. + void remove(const std::string& id); // Always-clean read surface. UI thread only. const std::vector>& 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. - // refresh_package syncs every script capability of one package (load/unload). - // refresh_capability syncs a single capability (enable/disable/key migration). - void refresh_package(const std::string& plugin_key, ActionChange change); - void refresh_capability(const std::string& plugin_key, const std::string& capability, ActionChange change); - // Dispatch + write-through (registry is the only thing that touches AppConfig). AppActionRunResult run(const std::string& id); // runs + bumps stats void set_favourite(const std::string& id, bool on); @@ -110,16 +100,13 @@ private: void seed_state(AppAction& a) const; // favourite/stats from config AppAction* find(const std::string& id); void erase_id(const std::string& id); - // Builds the action for one loaded+enabled script capability; returns nullptr - // when the capability is unloaded, missing, or disabled. - std::shared_ptr make_action(const std::string& plugin_key, const std::string& capability, - const std::function& package_name_for) const; // 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 + // serializing small action sets. Id lookups are event-driven and initialization is startup-only, so // a persistent map would add hashing/allocation cost without helping render. // shared_ptr: AppAction is abstract (stored by pointer), and run() takes a keep-alive - // copy so a queued refresh can't erase the object mid-run (nested plugin event loop). + // copy so a queued source refresh cannot erase the object mid-run. + std::vector> m_sources; std::vector> m_actions; // UI-thread confined; no lock }; diff --git a/src/slic3r/GUI/GUI_App.cpp b/src/slic3r/GUI/GUI_App.cpp index 7ed404e40e..27d000657c 100644 --- a/src/slic3r/GUI/GUI_App.cpp +++ b/src/slic3r/GUI/GUI_App.cpp @@ -142,6 +142,7 @@ #include "PluginsDialog.hpp" #include "SpeedDialDialog.hpp" +#include "ScriptActionSource.hpp" #include "TerminalDialog.hpp" //#ifdef WIN32 @@ -3171,32 +3172,6 @@ bool GUI_App::on_init_inner() refresh_plugins_dialog(); }); - // why: keep the speed-dial ActionRegistry always-clean. Each loader callback marshals - // a TARGETED refresh onto the UI thread (the callback may fire on a worker thread, so - // it must not touch the registry's vector directly). No custom event - CallAfter is - // the pubsub channel. - auto refresh_pkg = [](const std::string& key, ActionChange change) { - if (!wxTheApp || wxGetApp().is_closing()) - return; - wxGetApp().CallAfter([key, change] { - if (!wxGetApp().is_closing()) - wxGetApp().action_registry().refresh_package(key, change); - }); - }; - auto refresh_cap = [](const PluginCapabilityIdentifier& cap, ActionChange change) { - if (cap.type != PluginCapabilityType::Script || !wxTheApp || wxGetApp().is_closing()) - return; - const std::string key = cap.plugin_key, name = cap.name; - wxGetApp().CallAfter([key, name, change] { - if (!wxGetApp().is_closing()) - wxGetApp().action_registry().refresh_capability(key, name, change); - }); - }; - plugin_mgr.get_loader().subscribe_on_load_callback([refresh_pkg](const std::string& key) { refresh_pkg(key, ActionChange::Added); }); - plugin_mgr.get_loader().subscribe_on_unload_callback([refresh_pkg](const std::string& key) { refresh_pkg(key, ActionChange::Removed); }); - plugin_mgr.get_loader().subscribe_on_capability_load_callback([refresh_cap](const PluginCapabilityIdentifier& c) { refresh_cap(c, ActionChange::Added); }); - plugin_mgr.get_loader().subscribe_on_capability_unload_callback([refresh_cap](const PluginCapabilityIdentifier& c) { refresh_cap(c, ActionChange::Removed); }); - for (const std::string& plugin_key : plugin_mgr.get_catalog().get_enabled_plugin_keys()) { if (!plugin_mgr.get_loader().is_plugin_loaded(plugin_key)) { plugin_mgr.get_loader().load_plugin(plugin_mgr.get_catalog(), plugin_key, false); @@ -3204,10 +3179,8 @@ bool GUI_App::on_init_inner() } } - // why: seed the registry once on the UI thread after startup auto-load. The deferred - // per-plugin callbacks above are idempotent (upsert by id), so re-processing the same - // packages is harmless; this guarantees a populated set even before any callback lands. - m_action_registry.build(); + m_action_registry.add_source(std::make_unique()); + m_action_registry.init(); if (m_agent) plugin_mgr.set_cloud_agent(std::dynamic_pointer_cast(m_agent->get_cloud_agent())); diff --git a/src/slic3r/GUI/IActionSource.hpp b/src/slic3r/GUI/IActionSource.hpp new file mode 100644 index 0000000000..f76e6a281f --- /dev/null +++ b/src/slic3r/GUI/IActionSource.hpp @@ -0,0 +1,14 @@ +#pragma once + +namespace Slic3r { namespace GUI { + +class ActionRegistry; + +class IActionSource +{ +public: + virtual ~IActionSource() = default; + virtual void start(ActionRegistry& sink) = 0; +}; + +}} // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/ScriptActionSource.cpp b/src/slic3r/GUI/ScriptActionSource.cpp new file mode 100644 index 0000000000..8f5887b698 --- /dev/null +++ b/src/slic3r/GUI/ScriptActionSource.cpp @@ -0,0 +1,205 @@ +#include "ScriptActionSource.hpp" + +#include "GUI_App.hpp" +#include "PluginScriptRunner.hpp" +#include "SpeedDialActionId.hpp" +#include "slic3r/plugin/PluginManager.hpp" + +#include +#include + +#include + +#include +#include +#include + +namespace Slic3r { namespace GUI { + +namespace { + +std::string find_loaded_source_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 + +PluginScriptAction::PluginScriptAction(std::string plugin_key, std::string capability, std::string source) + : AppAction(speed_dial_action_id(plugin_key, capability), + capability.empty() ? plugin_key : capability, + std::move(source)), + plugin_key(std::move(plugin_key)), capability(std::move(capability)) +{} + +AppActionRunResult PluginScriptAction::run() const +{ + ScriptRunOutcome outcome = run_script_plugin_capability(plugin_key, capability); + AppActionRunResult::Level level = AppActionRunResult::Level::Info; + switch (outcome.level) { + case ScriptRunOutcome::Level::Success: level = AppActionRunResult::Level::Success; break; + case ScriptRunOutcome::Level::Error: level = AppActionRunResult::Level::Error; break; + case ScriptRunOutcome::Level::Busy: level = AppActionRunResult::Level::Busy; break; + case ScriptRunOutcome::Level::Info: break; + } + return {level, outcome.message}; +} + +void ScriptActionSource::start(ActionRegistry& sink) +{ + assert(wxThread::IsMain()); + assert(m_sink == nullptr); + m_sink = &sink; + + PluginLoader& loader = PluginManager::instance().get_loader(); + + auto refresh_source = [this](const std::string& plugin_key, ActionChange change) { + if (!wxTheApp || wxGetApp().is_closing()) + return; + wxGetApp().CallAfter([this, plugin_key, change] { + if (!wxGetApp().is_closing()) + this->refresh_source(plugin_key, change); + }); + }; + auto refresh_capability = [this](const PluginCapabilityIdentifier& capability, ActionChange change) { + if (capability.type != PluginCapabilityType::Script || !wxTheApp || wxGetApp().is_closing()) + return; + const std::string plugin_key = capability.plugin_key; + const std::string name = capability.name; + wxGetApp().CallAfter([this, plugin_key, name, change] { + if (!wxGetApp().is_closing()) + this->refresh_capability(plugin_key, name, change); + }); + }; + + // Subscribe before enumerating so a concurrent load cannot land in between the + // initial snapshot and callback registration. Duplicate notifications are safe: + // the sink upserts by id and tracking is de-duplicated. + loader.subscribe_on_load_callback( + [refresh_source](const std::string& key) { refresh_source(key, ActionChange::Added); }); + loader.subscribe_on_unload_callback( + [refresh_source](const std::string& key) { refresh_source(key, ActionChange::Removed); }); + loader.subscribe_on_capability_load_callback( + [refresh_capability](const PluginCapabilityIdentifier& capability) { + refresh_capability(capability, ActionChange::Added); + }); + loader.subscribe_on_capability_unload_callback( + [refresh_capability](const PluginCapabilityIdentifier& capability) { + refresh_capability(capability, ActionChange::Removed); + }); + + enumerate(); +} + +void ScriptActionSource::enumerate() +{ + assert(wxThread::IsMain()); + assert(m_sink != nullptr); + + PluginLoader& loader = PluginManager::instance().get_loader(); + std::unordered_map source_names; + for (const PluginDescriptor& desc : loader.get_all_loaded_plugin_descriptors()) + if (!desc.name.empty()) + source_names.emplace(desc.plugin_key, desc.name); + + for (const auto& capability : loader.get_plugin_capabilities_by_type(PluginCapabilityType::Script)) { + if (!capability || !capability->enabled) + continue; + auto source = source_names.find(capability->plugin_key); + const std::string& source_name = source == source_names.end() ? capability->plugin_key : source->second; + if (auto action = make_action(capability->plugin_key, capability->name, source_name)) { + track(capability->plugin_key, action->id); + m_sink->upsert(std::move(action)); + } + } +} + +std::shared_ptr ScriptActionSource::make_action(const std::string& plugin_key, + const std::string& capability, + const std::string& source) const +{ + PluginLoader& loader = PluginManager::instance().get_loader(); + if (!loader.is_plugin_loaded(plugin_key)) + return nullptr; + + auto loaded = loader.get_plugin_capability_by_name(plugin_key, PluginCapabilityType::Script, capability); + if (!loaded || !loaded->enabled) + return nullptr; + + return std::make_shared(plugin_key, capability, source); +} + +void ScriptActionSource::refresh_capability(const std::string& plugin_key, const std::string& capability, + ActionChange change) +{ + assert(wxThread::IsMain()); + assert(m_sink != nullptr); + + const std::string id = speed_dial_action_id(plugin_key, capability); + if (change == ActionChange::Removed) { + m_sink->remove(id); + untrack(plugin_key, id); + return; + } + + PluginLoader& loader = PluginManager::instance().get_loader(); + auto action = make_action(plugin_key, capability, find_loaded_source_name(loader, plugin_key)); + if (!action) { + m_sink->remove(id); + untrack(plugin_key, id); + return; + } + + track(plugin_key, id); + m_sink->upsert(std::move(action)); +} + +void ScriptActionSource::refresh_source(const std::string& plugin_key, ActionChange change) +{ + assert(wxThread::IsMain()); + assert(m_sink != nullptr); + + auto tracked = m_ids_by_plugin.find(plugin_key); + if (tracked != m_ids_by_plugin.end()) { + for (const std::string& id : tracked->second) + m_sink->remove(id); + m_ids_by_plugin.erase(tracked); + } + if (change == ActionChange::Removed) + return; + + PluginLoader& loader = PluginManager::instance().get_loader(); + const std::string source_name = find_loaded_source_name(loader, plugin_key); + for (const auto& capability : loader.get_plugin_capabilities_by_type(plugin_key, PluginCapabilityType::Script)) { + if (!capability || !capability->enabled) + continue; + if (auto action = make_action(plugin_key, capability->name, source_name)) { + track(plugin_key, action->id); + m_sink->upsert(std::move(action)); + } + } +} + +void ScriptActionSource::track(const std::string& plugin_key, const std::string& id) +{ + auto& ids = m_ids_by_plugin[plugin_key]; + if (std::find(ids.begin(), ids.end(), id) == ids.end()) + ids.push_back(id); +} + +void ScriptActionSource::untrack(const std::string& plugin_key, const std::string& id) +{ + auto tracked = m_ids_by_plugin.find(plugin_key); + if (tracked == m_ids_by_plugin.end()) + return; + + auto& ids = tracked->second; + ids.erase(std::remove(ids.begin(), ids.end(), id), ids.end()); + if (ids.empty()) + m_ids_by_plugin.erase(tracked); +} + +}} // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/ScriptActionSource.hpp b/src/slic3r/GUI/ScriptActionSource.hpp new file mode 100644 index 0000000000..85949f601d --- /dev/null +++ b/src/slic3r/GUI/ScriptActionSource.hpp @@ -0,0 +1,41 @@ +#pragma once + +#include "ActionRegistry.hpp" + +#include +#include +#include +#include + +namespace Slic3r { namespace GUI { + +enum class ActionChange { Added, Removed }; + +struct PluginScriptAction : AppAction +{ + std::string plugin_key; + std::string capability; + + PluginScriptAction(std::string plugin_key, std::string capability, std::string source); + AppActionRunResult run() const override; +}; + +class ScriptActionSource final : public IActionSource +{ +public: + void start(ActionRegistry& sink) override; + +private: + void enumerate(); + void refresh_source(const std::string& plugin_key, ActionChange change); + void refresh_capability(const std::string& plugin_key, const std::string& capability, ActionChange change); + std::shared_ptr make_action(const std::string& plugin_key, const std::string& capability, + const std::string& source) const; + void track(const std::string& plugin_key, const std::string& id); + void untrack(const std::string& plugin_key, const std::string& id); + + ActionRegistry* m_sink = nullptr; + std::unordered_map> m_ids_by_plugin; +}; + +}} // namespace Slic3r::GUI diff --git a/tests/slic3rutils/CMakeLists.txt b/tests/slic3rutils/CMakeLists.txt index 490f56a097..e122e4e760 100644 --- a/tests/slic3rutils/CMakeLists.txt +++ b/tests/slic3rutils/CMakeLists.txt @@ -1,6 +1,7 @@ get_filename_component(_TEST_NAME ${CMAKE_CURRENT_LIST_DIR} NAME) add_executable(${_TEST_NAME}_tests ${_TEST_NAME}_tests_main.cpp + test_action_source.cpp test_plugin_host_api.cpp test_plugin_capability_identifier.cpp test_plugin_install.cpp diff --git a/tests/slic3rutils/test_action_source.cpp b/tests/slic3rutils/test_action_source.cpp new file mode 100644 index 0000000000..8a90d2ccb1 --- /dev/null +++ b/tests/slic3rutils/test_action_source.cpp @@ -0,0 +1,35 @@ +#include + +#include "slic3r/GUI/ActionRegistry.hpp" +#include "slic3r/GUI/IActionSource.hpp" + +#include + +using Slic3r::GUI::ActionRegistry; +using Slic3r::GUI::IActionSource; + +namespace { + +class RecordingActionSource final : public IActionSource +{ +public: + explicit RecordingActionSource(bool& started) : m_started(started) {} + + void start(ActionRegistry&) override { m_started = true; } + +private: + bool& m_started; +}; + +} // namespace + +TEST_CASE("ActionRegistry starts registered action sources", "[speeddial][actions]") +{ + bool started = false; + ActionRegistry registry; + registry.add_source(std::make_unique(started)); + + registry.init(); + + CHECK(started); +}