Make speed dial actions polymorphic

Decouple registry dispatch from plugin-specific
result types so future action types can provide
their own run implementations.

Store actions behind shared pointers to keep them
alive while plugin UI pumps a nested event loop
and queued refreshes mutate the registry. Scope
"don't ask again" to an individual action instead
of its whole plugin.
This commit is contained in:
Andrew
2026-07-10 20:46:50 +08:00
parent 7b19145226
commit 2cef101ffe
3 changed files with 127 additions and 71 deletions

View File

@@ -68,31 +68,41 @@ std::string find_loaded_package_name(PluginLoader& loader, const std::string& pl
} // namespace } // namespace
ScriptRunOutcome AppAction::run() const 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
{ {
return run_script_plugin_capability(plugin_key, capability); 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 };
} }
// ---- build / enumeration ---------------------------------------------------- // ---- build / enumeration ----------------------------------------------------
bool ActionRegistry::make_action(const std::string& plugin_key, const std::string& capability, std::shared_ptr<AppAction> ActionRegistry::make_action(const std::string& plugin_key, const std::string& capability,
const std::function<std::string(const std::string&)>& package_name_for, const std::function<std::string(const std::string&)>& package_name_for) const
AppAction& out) const
{ {
PluginLoader& loader = PluginManager::instance().get_loader(); PluginLoader& loader = PluginManager::instance().get_loader();
if (!loader.is_plugin_loaded(plugin_key)) if (!loader.is_plugin_loaded(plugin_key))
return false; return nullptr;
auto cap = loader.get_plugin_capability_by_name(plugin_key, PluginCapabilityType::Script, capability); auto cap = loader.get_plugin_capability_by_name(plugin_key, PluginCapabilityType::Script, capability);
if (!cap || !cap->enabled) if (!cap || !cap->enabled)
return false; return nullptr;
out.id = speed_dial_action_id(plugin_key, capability); auto a = std::make_shared<PluginScriptAction>(plugin_key, capability, package_name_for(plugin_key));
out.title = capability.empty() ? plugin_key : capability; seed_state(*a);
out.pkg = package_name_for(plugin_key); return a;
out.plugin_key = plugin_key;
out.capability = capability;
seed_state(out);
return true;
} }
void ActionRegistry::seed_state(AppAction& a) const void ActionRegistry::seed_state(AppAction& a) const
@@ -132,8 +142,7 @@ void ActionRegistry::build()
for (const auto& cap : loader.get_plugin_capabilities_by_type(PluginCapabilityType::Script)) { for (const auto& cap : loader.get_plugin_capabilities_by_type(PluginCapabilityType::Script)) {
if (!cap || !cap->enabled) if (!cap || !cap->enabled)
continue; continue;
AppAction a; if (auto a = make_action(cap->plugin_key, cap->name, package_name_for))
if (make_action(cap->plugin_key, cap->name, package_name_for, a))
m_actions.push_back(std::move(a)); m_actions.push_back(std::move(a));
} }
} }
@@ -143,8 +152,8 @@ void ActionRegistry::build()
const AppAction* ActionRegistry::by_id(const std::string& id) const const AppAction* ActionRegistry::by_id(const std::string& id) const
{ {
assert(wxThread::IsMain()); assert(wxThread::IsMain());
auto it = std::find_if(m_actions.begin(), m_actions.end(), [&](const AppAction& a) { return a.id == id; }); auto it = std::find_if(m_actions.begin(), m_actions.end(), [&](const auto& a) { return a->id == id; });
return it == m_actions.end() ? nullptr : &*it; return it == m_actions.end() ? nullptr : it->get();
} }
AppAction* ActionRegistry::find(const std::string& id) AppAction* ActionRegistry::find(const std::string& id)
@@ -155,7 +164,7 @@ AppAction* ActionRegistry::find(const std::string& id)
void ActionRegistry::erase_id(const std::string& id) void ActionRegistry::erase_id(const std::string& id)
{ {
m_actions.erase(std::remove_if(m_actions.begin(), m_actions.end(), m_actions.erase(std::remove_if(m_actions.begin(), m_actions.end(),
[&](const AppAction& a) { return a.id == id; }), [&](const auto& a) { return a->id == id; }),
m_actions.end()); m_actions.end());
} }
@@ -175,13 +184,14 @@ void ActionRegistry::refresh_capability(const std::string& plugin_key, const std
// multi-plugin lookup, so the callback can just return the already-resolved name. // 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 package_name_for = [&package_name](const std::string&) { return package_name; };
AppAction a; auto a = make_action(plugin_key, capability, package_name_for);
if (!make_action(plugin_key, capability, package_name_for, a)) { // why: disabled/unloaded capabilities leave the dial. if (!a) { // why: disabled/unloaded capabilities leave the dial.
erase_id(id); erase_id(id);
return; return;
} }
if (AppAction* existing = find(id)) auto it = std::find_if(m_actions.begin(), m_actions.end(), [&](const auto& e) { return e->id == id; });
*existing = std::move(a); if (it != m_actions.end())
*it = std::move(a);
else else
m_actions.push_back(std::move(a)); m_actions.push_back(std::move(a));
} }
@@ -190,8 +200,14 @@ void ActionRegistry::refresh_package(const std::string& plugin_key, ActionChange
{ {
assert(wxThread::IsMain()); assert(wxThread::IsMain());
// Drop every action of this package, then (on add/update) re-add its current script caps. // 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(), m_actions.erase(std::remove_if(m_actions.begin(), m_actions.end(),
[&](const AppAction& a) { return a.plugin_key == plugin_key; }), [&](const auto& a) {
auto* p = dynamic_cast<PluginScriptAction*>(a.get());
return p && p->plugin_key == plugin_key;
}),
m_actions.end()); m_actions.end());
if (change == ActionChange::Removed) if (change == ActionChange::Removed)
return; return;
@@ -204,26 +220,26 @@ void ActionRegistry::refresh_package(const std::string& plugin_key, ActionChange
for (const auto& cap : loader.get_plugin_capabilities_by_type(plugin_key, PluginCapabilityType::Script)) { for (const auto& cap : loader.get_plugin_capabilities_by_type(plugin_key, PluginCapabilityType::Script)) {
if (!cap || !cap->enabled) if (!cap || !cap->enabled)
continue; continue;
AppAction a; if (auto a = make_action(plugin_key, cap->name, package_name_for))
if (make_action(plugin_key, cap->name, package_name_for, a))
m_actions.push_back(std::move(a)); m_actions.push_back(std::move(a));
} }
} }
// ---- dispatch + write-through ---------------------------------------------- // ---- dispatch + write-through ----------------------------------------------
ScriptRunOutcome ActionRegistry::run(const std::string& id) AppActionRunResult ActionRegistry::run(const std::string& id)
{ {
assert(wxThread::IsMain()); assert(wxThread::IsMain());
const AppAction* a = by_id(id); auto it = std::find_if(m_actions.begin(), m_actions.end(), [&](const auto& e) { return e->id == id; });
if (!a) if (it == m_actions.end())
return ScriptRunOutcome{ScriptRunOutcome::Level::Info, {}}; return {}; // default Info, empty message
// why: run a stack COPY, never the vector element. The script runner pumps a nested // why: hold a shared_ptr keep-alive, never a bare vector element. The script runner pumps
// event loop (plugin modal UI) and holds the action's strings by reference; a queued // a nested event loop (plugin modal UI); a queued refresh_* landing during that loop may
// refresh_* landing during that loop may reallocate m_actions and dangle them. // erase this action from m_actions - the keep-alive keeps the heap object valid until run
const AppAction action = *a; // returns. (Replaces the old stack-copy; an abstract AppAction cannot be sliced.)
ScriptRunOutcome o = action.run(); std::shared_ptr<AppAction> keep = *it;
if (o.level == ScriptRunOutcome::Level::Busy) AppActionRunResult o = keep->run();
if (o.level == AppActionRunResult::Level::Busy)
return o; return o;
// Bump stats (write-through). Re-read to avoid clobbering a concurrent field. // Bump stats (write-through). Re-read to avoid clobbering a concurrent field.
@@ -271,19 +287,19 @@ void ActionRegistry::reorder_favourites(const std::vector<std::string>& ids)
write_section("favourite_actions", nlohmann::json(next)); write_section("favourite_actions", nlohmann::json(next));
} }
bool ActionRegistry::should_ask(const std::string& plugin_key) const bool ActionRegistry::should_ask(const std::string& id) const
{ {
assert(wxThread::IsMain()); assert(wxThread::IsMain());
auto arr = read_string_array("ask_suppressed"); auto arr = read_string_array("ask_suppressed");
return std::find(arr.begin(), arr.end(), plugin_key) == arr.end(); return std::find(arr.begin(), arr.end(), id) == arr.end();
} }
void ActionRegistry::suppress_ask(const std::string& plugin_key) void ActionRegistry::suppress_ask(const std::string& id)
{ {
assert(wxThread::IsMain()); assert(wxThread::IsMain());
auto arr = read_string_array("ask_suppressed"); auto arr = read_string_array("ask_suppressed");
if (std::find(arr.begin(), arr.end(), plugin_key) == arr.end()) if (std::find(arr.begin(), arr.end(), id) == arr.end())
arr.push_back(plugin_key); arr.push_back(id);
write_section("ask_suppressed", nlohmann::json(arr)); write_section("ask_suppressed", nlohmann::json(arr));
} }
@@ -294,8 +310,8 @@ nlohmann::json ActionRegistry::snapshot() const
assert(wxThread::IsMain()); assert(wxThread::IsMain());
std::vector<const AppAction*> sorted; std::vector<const AppAction*> sorted;
sorted.reserve(m_actions.size()); sorted.reserve(m_actions.size());
for (const AppAction& a : m_actions) for (const auto& a : m_actions)
sorted.push_back(&a); sorted.push_back(a.get());
const long long now = (long long) std::time(nullptr); const long long now = (long long) std::time(nullptr);
std::stable_sort(sorted.begin(), sorted.end(), [&](const AppAction* a, const AppAction* b) { std::stable_sort(sorted.begin(), sorted.end(), [&](const AppAction* a, const AppAction* b) {

View File

@@ -1,6 +1,6 @@
#pragma once #pragma once
#include "PluginScriptRunner.hpp" // ScriptRunOutcome #include "PluginScriptRunner.hpp" // wxString + ScriptRunOutcome (mapped in the .cpp, not exposed here)
#include <nlohmann/json.hpp> #include <nlohmann/json.hpp>
@@ -8,6 +8,7 @@
#include <cassert> #include <cassert>
#include <functional> #include <functional>
#include <memory>
#include <string> #include <string>
#include <vector> #include <vector>
@@ -15,24 +16,62 @@ namespace Slic3r { namespace GUI {
enum class ActionChange { Added, Removed, Updated }; 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.
struct AppActionRunResult
{
enum class Level { Success, Info, Error, Busy };
Level level = Level::Info;
wxString message; // empty = "nothing worth showing"
};
// A speed-dial action: identity + user-state seeded from config + how to run itself. // A speed-dial action: identity + user-state seeded from config + how to run itself.
// Holds a weak (plugin_key, capability) handle, never a live capability pointer. // Abstract base - the only virtual is run(); a concrete subclass (e.g.
// PluginScriptAction) knows how to run and what its pkg/source is.
// note: named AppAction, not Action - Slic3r::GUI::Action is already taken by // note: named AppAction, not Action - Slic3r::GUI::Action is already taken by
// UnsavedChangesDialog's exit-action enum, and this header reaches most GUI TUs. // UnsavedChangesDialog's exit-action enum, and this header reaches most GUI TUs.
struct AppAction struct AppAction
{ {
std::string id; // speed_dial_action_id(plugin_key, capability) std::string id; // speed_dial_action_id(...) - stable identity + AppConfig key
std::string title; // capability display name std::string title; // display name
std::string pkg; // owning package display name (row eyebrow) // pkg is the action's SOURCE - where it comes from. For a plugin action it is
std::string plugin_key; // weak handle: owning package // the owning package's display name (row eyebrow); a future app-command action
std::string capability; // weak handle: capability name // would set its own label. Each subclass sets pkg in its constructor (below),
// so the base cannot be built without one.
std::string pkg;
// seeded from AppConfig for the snapshot / sort: // seeded from AppConfig for the snapshot / sort:
bool favourite = false; bool favourite = false;
int count = 0; int count = 0;
long long last = 0; // epoch seconds long long last = 0; // epoch seconds
ScriptRunOutcome run() const; // re-resolves the capability and runs it (UI thread) virtual ~AppAction() = default;
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)) {}
};
// 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. // Single source of runnable actions for the app session, owned by GUI_App.
@@ -46,8 +85,8 @@ public:
void build(); void build();
// Always-clean read surface. UI thread only. // Always-clean read surface. UI thread only.
const std::vector<AppAction>& all() const { assert(wxThread::IsMain()); return m_actions; } const std::vector<std::shared_ptr<AppAction>>& all() const { assert(wxThread::IsMain()); return m_actions; }
const AppAction* by_id(const std::string& id) const; const AppAction* by_id(const std::string& id) const;
// Targeted incremental maintenance, reached via CallAfter from loader callbacks. // Targeted incremental maintenance, reached via CallAfter from loader callbacks.
// refresh_package syncs every script capability of one package (load/unload). // refresh_package syncs every script capability of one package (load/unload).
@@ -56,12 +95,13 @@ public:
void refresh_capability(const std::string& plugin_key, const std::string& capability, 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). // Dispatch + write-through (registry is the only thing that touches AppConfig).
ScriptRunOutcome run(const std::string& id); // runs + bumps stats AppActionRunResult run(const std::string& id); // runs + bumps stats
void set_favourite(const std::string& id, bool on); void set_favourite(const std::string& id, bool on);
void reorder_favourites(const std::vector<std::string>& ids); // persist a new bar order void reorder_favourites(const std::vector<std::string>& ids); // persist a new bar order
bool should_ask(const std::string& plugin_key) const; // run-confirm gate // Run-confirm gate, keyed by action id (per-action "don't ask again").
void suppress_ask(const std::string& plugin_key); bool should_ask(const std::string& id) const;
void suppress_ask(const std::string& id);
// Flat, frecency-sorted snapshot for the webview: {actions:[...], favourites:[...]}. // Flat, frecency-sorted snapshot for the webview: {actions:[...], favourites:[...]}.
nlohmann::json snapshot() const; nlohmann::json snapshot() const;
@@ -70,16 +110,17 @@ private:
void seed_state(AppAction& a) const; // favourite/stats from config void seed_state(AppAction& a) const; // favourite/stats from config
AppAction* find(const std::string& id); AppAction* find(const std::string& id);
void erase_id(const std::string& id); void erase_id(const std::string& id);
// Builds the AppAction for one loaded+enabled script capability; returns false // Builds the action for one loaded+enabled script capability; returns nullptr
// (and builds nothing) when the capability is unloaded, missing, or disabled. // when the capability is unloaded, missing, or disabled.
bool make_action(const std::string& plugin_key, const std::string& capability, std::shared_ptr<AppAction> make_action(const std::string& plugin_key, const std::string& capability,
const std::function<std::string(const std::string&)>& package_name_for, const std::function<std::string(const std::string&)>& package_name_for) const;
AppAction& out) const;
// why: vector storage favors the popup path, which snapshots by iterating, sorting, and // 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 build is startup-only, so
// a persistent map would add hashing/allocation cost without helping render. // a persistent map would add hashing/allocation cost without helping render.
std::vector<AppAction> m_actions; // UI-thread confined; no lock // 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).
std::vector<std::shared_ptr<AppAction>> m_actions; // UI-thread confined; no lock
}; };
}} // namespace Slic3r::GUI }} // namespace Slic3r::GUI

View File

@@ -254,30 +254,29 @@ private:
const AppAction* a = reg.by_id(id); const AppAction* a = reg.by_id(id);
if (!a) if (!a)
return; return;
const std::string key = a->plugin_key; const bool ask = reg.should_ask(id);
const std::string cap = a->capability; const std::string atitle = a->title; // capture before Dismiss; `a` may not outlive it
const bool ask = reg.should_ask(key);
Dismiss(); // launcher gone before the modal / the script's own UI Dismiss(); // launcher gone before the modal / the script's own UI
if (ask) { if (ask) {
const wxString label = title.empty() ? from_u8(cap) : from_u8(title); const wxString label = title.empty() ? from_u8(atitle) : from_u8(title);
RichMessageDialog dlg(wxGetApp().mainframe, wxString::Format(_L("Run \"%s\"?"), label), RichMessageDialog dlg(wxGetApp().mainframe, wxString::Format(_L("Run \"%s\"?"), label),
_L("Run plugin"), wxOK | wxCANCEL); _L("Run plugin"), wxOK | wxCANCEL);
dlg.ShowCheckBox(_L("Don't ask again for this plugin")); dlg.ShowCheckBox(_L("Don't ask again for this action"));
if (dlg.ShowModal() != wxID_OK) if (dlg.ShowModal() != wxID_OK)
return; return;
if (dlg.IsCheckBoxChecked()) if (dlg.IsCheckBoxChecked())
wxGetApp().action_registry().suppress_ask(key); wxGetApp().action_registry().suppress_ask(id);
} }
// why: value-capture only. Script execution may outlive this popup if the app is closing. // why: value-capture only. Script execution may outlive this popup if the app is closing.
wxGetApp().CallAfter([id] { wxGetApp().CallAfter([id] {
ScriptRunOutcome o = wxGetApp().action_registry().run(id); AppActionRunResult o = wxGetApp().action_registry().run(id);
if (o.level == ScriptRunOutcome::Level::Busy) if (o.level == AppActionRunResult::Level::Busy)
return; return;
if (!o.message.IsEmpty()) if (!o.message.IsEmpty())
wxGetApp().plater()->get_notification_manager()->push_notification( wxGetApp().plater()->get_notification_manager()->push_notification(
NotificationType::CustomNotification, NotificationType::CustomNotification,
o.level == ScriptRunOutcome::Level::Error ? NotificationManager::NotificationLevel::ErrorNotificationLevel : o.level == AppActionRunResult::Level::Error ? NotificationManager::NotificationLevel::ErrorNotificationLevel :
NotificationManager::NotificationLevel::RegularNotificationLevel, NotificationManager::NotificationLevel::RegularNotificationLevel,
into_u8(o.message)); into_u8(o.message));
}); });