diff --git a/src/slic3r/CMakeLists.txt b/src/slic3r/CMakeLists.txt index c57f46add7..d9b8e00265 100644 --- a/src/slic3r/CMakeLists.txt +++ b/src/slic3r/CMakeLists.txt @@ -120,10 +120,6 @@ 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 GUI/TerminalDialog.cpp diff --git a/src/slic3r/GUI/ActionRegistry.cpp b/src/slic3r/GUI/ActionRegistry.cpp index 326bbbf6c5..89085de345 100644 --- a/src/slic3r/GUI/ActionRegistry.cpp +++ b/src/slic3r/GUI/ActionRegistry.cpp @@ -1,14 +1,19 @@ #include "ActionRegistry.hpp" #include "GUI_App.hpp" +#include "PluginScriptRunner.hpp" +#include "slic3r/plugin/PluginManager.hpp" #include +#include +#include #include #include #include #include +#include namespace Slic3r { namespace GUI { @@ -52,20 +57,172 @@ double frecency_score(int count, long long last, long long now) return count * std::pow(2.0, -age / HALF_LIFE_DAYS); } +// ---- script-plugin action source (the one and only source) ------------------ + +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; +} + +// A runnable script capability exposed as a speed-dial action. source_key = plugin_key +// (identity), so a plugin display-name change does not re-key the action. +struct PluginScriptAction : AppAction +{ + static constexpr const char* kIdPrefix = "plugin_script_action"; + + std::string plugin_key; + std::string capability; + + // The id an action for (plugin_key, capability) would have - lets refresh_capability + // remove a gone capability without materialising the action. + static std::string id_for(const std::string& plugin_key, const std::string& capability) + { + return AppAction::compose_id(kIdPrefix, capability.empty() ? plugin_key : capability, plugin_key); + } + + PluginScriptAction(std::string plugin_key_in, std::string capability_in, std::string source_name) + : AppAction(kIdPrefix, + capability_in.empty() ? plugin_key_in : capability_in, // title + plugin_key_in, // source_key + std::move(source_name)), + plugin_key(std::move(plugin_key_in)), capability(std::move(capability_in)) + {} + + AppActionRunResult run() const override + { + 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}; + } +}; + +// Builds an action for a capability, or nullptr if it is not a currently-loaded, +// enabled script capability. +std::unique_ptr make_action(const std::string& plugin_key, const std::string& capability, + const std::string& source_name) +{ + 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_unique(plugin_key, capability, source_name); +} + } // namespace void ActionRegistry::init() { assert(wxThread::IsMain()); - for (auto& source : m_sources) - source->start(*this); + assert(!m_started); + m_started = true; + + PluginLoader& loader = PluginManager::instance().get_loader(); + + auto on_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 on_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 between the initial + // snapshot and callback registration. Duplicate notifications are safe: upsert is by + // id and the m_actions scan in refresh_source is idempotent. + loader.subscribe_on_load_callback( + [on_source](const std::string& key) { on_source(key, ActionChange::Added); }); + loader.subscribe_on_unload_callback( + [on_source](const std::string& key) { on_source(key, ActionChange::Removed); }); + loader.subscribe_on_capability_load_callback( + [on_capability](const PluginCapabilityIdentifier& capability) { + on_capability(capability, ActionChange::Added); + }); + loader.subscribe_on_capability_unload_callback( + [on_capability](const PluginCapabilityIdentifier& capability) { + on_capability(capability, ActionChange::Removed); + }); + + // enumerate current script capabilities + 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 it = source_names.find(capability->plugin_key); + const std::string& source_name = it == source_names.end() ? capability->plugin_key : it->second; + if (auto action = make_action(capability->plugin_key, capability->name, source_name)) + upsert(std::move(action)); + } } -void ActionRegistry::add_source(std::unique_ptr source) +void ActionRegistry::refresh_source(const std::string& plugin_key, ActionChange change) { assert(wxThread::IsMain()); - if (source) - m_sources.push_back(std::move(source)); + + // Remove this source's current actions. Collect first - erasing from m_actions while + // iterating invalidates the iterator. why: m_actions (not the loader) is the source of + // truth, so this is correct even after the plugin has already unloaded. + std::vector stale; + for (const auto& [id, action] : m_actions) + if (action->source_key() == plugin_key) + stale.push_back(id); + for (const std::string& id : stale) + remove(id); + + 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)) + upsert(std::move(action)); + } +} + +void ActionRegistry::refresh_capability(const std::string& plugin_key, const std::string& capability, + ActionChange change) +{ + assert(wxThread::IsMain()); + + const std::string id = PluginScriptAction::id_for(plugin_key, capability); + if (change == ActionChange::Removed) { + remove(id); + return; + } + + PluginLoader& loader = PluginManager::instance().get_loader(); + if (auto action = make_action(plugin_key, capability, find_loaded_source_name(loader, plugin_key))) + upsert(std::move(action)); + else + remove(id); } void ActionRegistry::upsert(std::unique_ptr action) @@ -211,8 +368,8 @@ nlohmann::json ActionRegistry::snapshot() const return sa > sb; if (a->title() != b->title()) return a->title() < b->title(); - if (a->source() != b->source()) - return a->source() < b->source(); + if (a->source_name() != b->source_name()) + return a->source_name() < b->source_name(); return a->id() < b->id(); }); @@ -220,7 +377,7 @@ nlohmann::json ActionRegistry::snapshot() const for (const AppAction* a : sorted) actions.push_back({{"id", a->id()}, {"title", a->title()}, - {"source", a->source()}, + {"source", a->source_name()}, {"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 eb85d09737..c421297217 100644 --- a/src/slic3r/GUI/ActionRegistry.hpp +++ b/src/slic3r/GUI/ActionRegistry.hpp @@ -1,7 +1,5 @@ #pragma once -#include "IActionSource.hpp" - #include #include @@ -10,12 +8,16 @@ #include #include #include +#include #include #include #include namespace Slic3r { namespace GUI { +// How a source's action set changed. Drives the registry's refresh handlers. +enum class ActionChange { Added, Removed }; + // 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 @@ -35,7 +37,19 @@ struct AppAction { const std::string& id() const { return m_id; } const std::string& title() const { return m_title; } - const std::string& source() const { return m_source; } + const std::string& source_key() const { return m_source_key; } // stable source identity + const std::string& source_name() const { return m_source_name; } // source display name + + // Builds the stable id "::<source_key>". why: one place owns the + // format - both the base ctor and the one raw-key lookup (removing a capability + // without an action object) go through this; ids are never parsed back apart. + static std::string compose_id(std::string_view prefix, std::string_view title, std::string_view source_key) + { + std::string out; + out.reserve(prefix.size() + title.size() + source_key.size() + 2); + out.append(prefix).append(1, ':').append(title).append(1, ':').append(source_key); + return out; + } // seeded from AppConfig for the snapshot / sort: bool favourite = false; @@ -48,36 +62,41 @@ struct AppAction protected: // The definition is constructor-set and immutable. Refreshes replace an action // instead of mutating identity after the registry has indexed it by id. - AppAction(std::string id, std::string title, std::string source) - : m_id(std::move(id)), m_title(std::move(title)), m_source(std::move(source)) {} + // why: source_key (not the display name) carries identity, so renaming the source's + // display name leaves the id - and its persisted stats/favourite - intact. + AppAction(std::string_view prefix, std::string title, std::string source_key, std::string source_name) + : m_id(compose_id(prefix, title, source_key)), + m_title(std::move(title)), + m_source_key(std::move(source_key)), + m_source_name(std::move(source_name)) {} private: - std::string m_id; // speed_dial_action_id(...) - stable identity + AppConfig key - std::string m_title; // display name - std::string m_source; // display name of the action's source + std::string m_id; // <prefix>:<title>:<source_key> - stable identity + AppConfig key + std::string m_title; // display name + std::string m_source_key; // stable identity of the action's source (e.g. plugin_key) + std::string m_source_name; // display name of the action's source }; -// Generic sink and single owner of runnable actions for the app session. +// Self-contained 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. +// 1. init() (once, UI thread) subscribes to the plugin loader and enumerates the +// current script capabilities into actions. +// 2. Loader load/unload callbacks route through refresh_source()/refresh_capability(), +// which upsert()/remove() actions. The registry keeps the only action list and +// restores persisted user state as actions arrive. +// 3. Consumers use by_id(), snapshot(), and run() without knowing the source. +// +// note: there is exactly one source (script plugins), so it lives inline here rather +// than behind a polymorphic source interface. class ActionRegistry { public: - // 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. + // Subscribes to the plugin loader and enumerates its current actions. Call once + // on the UI thread after the plugin system is up; wires the initial list and live + // updates together. void init(); - // Transfers ownership of a source to the registry. The source remains dormant - // until init() starts it. - void add_source(std::unique_ptr<IActionSource> source); - // Takes ownership, seeds persisted state, then inserts the action or replaces // the action with the same id. A null action is ignored. void upsert(std::unique_ptr<AppAction> action); @@ -104,7 +123,12 @@ private: void seed_state(AppAction& a) const; // favourite/stats from config AppAction* find(const std::string& id); - std::vector<std::unique_ptr<IActionSource>> m_sources; + // Loader callbacks (marshalled to the UI thread) land here. refresh_source rebuilds + // one plugin's whole action set; refresh_capability touches a single capability. + void refresh_source(const std::string& plugin_key, ActionChange change); + void refresh_capability(const std::string& plugin_key, const std::string& capability, ActionChange change); + + bool m_started = false; // init() runs exactly once; guards double-subscription std::unordered_map<std::string, std::shared_ptr<AppAction>> m_actions; // UI-thread confined; no lock }; diff --git a/src/slic3r/GUI/GUI_App.cpp b/src/slic3r/GUI/GUI_App.cpp index d1c0e6695f..7dc10ed98a 100644 --- a/src/slic3r/GUI/GUI_App.cpp +++ b/src/slic3r/GUI/GUI_App.cpp @@ -142,7 +142,6 @@ #include "PluginsDialog.hpp" #include "SpeedDialDialog.hpp" -#include "ScriptActionSource.hpp" #include "TerminalDialog.hpp" //#ifdef WIN32 @@ -3172,15 +3171,7 @@ bool GUI_App::on_init_inner() init_plugin_gui_wiring(); - // Register all AppAction sources - m_action_registry.add_source(std::make_unique<ScriptActionSource>()); - // m_action_registry.add_source(std::make_unique<AnotherActionSource>()); - // m_action_registry.add_source(std::make_unique<YetAnotherActionSource>()); - // ... - - // Initialize with all added sources - // - register callbacks, - // - then enumerate: upsert all actions into registry + // Subscribe to the plugin loader and enumerate current actions (UI thread, once). m_action_registry.init(); for (const std::string& plugin_key : plugin_mgr.get_enabled_plugin_keys()) { diff --git a/src/slic3r/GUI/IActionSource.hpp b/src/slic3r/GUI/IActionSource.hpp deleted file mode 100644 index f76e6a281f..0000000000 --- a/src/slic3r/GUI/IActionSource.hpp +++ /dev/null @@ -1,14 +0,0 @@ -#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 index 1d71c68961..e69de29bb2 100644 --- a/src/slic3r/GUI/ScriptActionSource.cpp +++ b/src/slic3r/GUI/ScriptActionSource.cpp @@ -1,194 +0,0 @@ -#include "ScriptActionSource.hpp" - -#include "GUI_App.hpp" -#include "I18N.hpp" -#include "SpeedDialActionId.hpp" -#include "slic3r/plugin/PluginManager.hpp" - -#include <wx/thread.h> - -#include <algorithm> -#include <cassert> -#include <unordered_map> - -namespace Slic3r { namespace GUI { - -namespace { - -std::string find_loaded_source_name(PluginManager& manager, const std::string& plugin_key) -{ - PluginDescriptor descriptor; - if (manager.try_get_plugin_descriptor(plugin_key, descriptor) && !descriptor.name.empty()) - return descriptor.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 -{ - std::string error; - const ExecutionResult result = PluginManager::instance().run_script_capability(plugin_key, capability, error); - if (!error.empty()) - return {AppActionRunResult::Level::Error, from_u8(error)}; - - const bool skipped = result.status == PluginResult::Skipped; - const wxString fallback = skipped ? _L("Script plugin skipped.") : _L("Script plugin finished."); - return {skipped ? AppActionRunResult::Level::Info : AppActionRunResult::Level::Success, - result.message.empty() ? fallback : from_u8(result.message)}; -} - -void ScriptActionSource::start(ActionRegistry& sink) -{ - assert(wxThread::IsMain()); - assert(m_sink == nullptr); - m_sink = &sink; - - PluginManager& manager = PluginManager::instance(); - - 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 PluginCapabilityId& 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. - manager.subscribe_on_load_callback( - [refresh_source](const std::string& key) { refresh_source(key, ActionChange::Added); }); - manager.subscribe_on_unload_callback( - [refresh_source](const std::string& key) { refresh_source(key, ActionChange::Removed); }); - manager.subscribe_on_capability_load_callback( - [refresh_capability](const PluginCapabilityId& capability) { - refresh_capability(capability, ActionChange::Added); - }); - manager.subscribe_on_capability_unload_callback( - [refresh_capability](const PluginCapabilityId& capability) { - refresh_capability(capability, ActionChange::Removed); - }); - - // enumerate - std::unordered_map<std::string, std::string> source_names; - for (const PluginDescriptor& desc : manager.get_plugin_descriptors()) - if (!desc.name.empty()) - source_names.emplace(desc.plugin_key, desc.name); - - for (const auto& capability : manager.get_plugin_capabilities("", PluginCapabilityType::Script)) { - if (!capability) - continue; - auto source = source_names.find(capability->audit_plugin_key()); - const std::string& source_name = source == source_names.end() ? capability->audit_plugin_key() : source->second; - if (auto action = make_action(capability->audit_plugin_key(), capability->name(), source_name)) { - track(capability->audit_plugin_key(), action->id()); - m_sink->upsert(std::move(action)); - } - } -} - -std::unique_ptr<AppAction> ScriptActionSource::make_action(const std::string& plugin_key, - const std::string& capability, - const std::string& source) const -{ - PluginManager& manager = PluginManager::instance(); - if (!manager.is_plugin_loaded(plugin_key)) - return nullptr; - - auto loaded = manager.get_plugin_capability(plugin_key, capability, PluginCapabilityType::Script); - if (!loaded) - return nullptr; - - return std::make_unique<PluginScriptAction>(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; - } - - PluginManager& manager = PluginManager::instance(); - auto action = make_action(plugin_key, capability, find_loaded_source_name(manager, 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; - - PluginManager& manager = PluginManager::instance(); - const std::string source_name = find_loaded_source_name(manager, plugin_key); - for (const auto& capability : manager.get_plugin_capabilities(plugin_key, PluginCapabilityType::Script)) { - if (!capability) - 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 deleted file mode 100644 index a15e906e15..0000000000 --- a/src/slic3r/GUI/ScriptActionSource.hpp +++ /dev/null @@ -1,40 +0,0 @@ -#pragma once - -#include "ActionRegistry.hpp" - -#include <memory> -#include <string> -#include <unordered_map> -#include <vector> - -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 refresh_source(const std::string& plugin_key, ActionChange change); - void refresh_capability(const std::string& plugin_key, const std::string& capability, ActionChange change); - std::unique_ptr<AppAction> 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<std::string, std::vector<std::string>> m_ids_by_plugin; -}; - -}} // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/SpeedDialActionId.hpp b/src/slic3r/GUI/SpeedDialActionId.hpp deleted file mode 100644 index e900a76182..0000000000 --- a/src/slic3r/GUI/SpeedDialActionId.hpp +++ /dev/null @@ -1,36 +0,0 @@ -#pragma once - -#include <cstdint> -#include <string> - -namespace Slic3r { namespace GUI { - -// Stable, opaque identity for a speed-dial action, used as the AppConfig key, -// the JS/registry handle, and the favourite_actions/stats key. FNV-1a 64-bit over -// the UTF-8 bytes of `plugin_key 0x1f capability`, lowercase 16-char hex. -// -// why: this is PERSISTED, so it must be deterministic across runs and platforms. -// std::hash<std::string> is explicitly NOT usable here (unspecified, per-process). -// note: 64-bit hash -> collision is astronomically unlikely at the tens-of-actions -// scale; upgrade path (128-bit / full-string key) only if that ever changes. -inline std::string speed_dial_action_id(const std::string& plugin_key, const std::string& capability) -{ - constexpr std::uint64_t kOffset = 14695981039346656037ULL; // FNV offset basis (0xcbf29ce484222325) - constexpr std::uint64_t kPrime = 1099511628211ULL; // FNV prime - std::uint64_t h = kOffset; - // note: [&], not [&h] - MSVC rejects the constexpr kPrime read inside the lambda - // without a default capture mode (error C3493). - auto mix = [&](const std::string& s) { - for (unsigned char c : s) { h ^= c; h *= kPrime; } - }; - mix(plugin_key); - h ^= 0x1f; h *= kPrime; // 0x1f (unit separator) between the two fields - mix(capability); - - static const char* kHex = "0123456789abcdef"; - std::string out(16, '0'); - for (int i = 15; i >= 0; --i) { out[i] = kHex[h & 0xf]; h >>= 4; } - return out; -} - -}} // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/SpeedDialDialog.cpp b/src/slic3r/GUI/SpeedDialDialog.cpp index 19f03abacf..7d06c1d26d 100644 --- a/src/slic3r/GUI/SpeedDialDialog.cpp +++ b/src/slic3r/GUI/SpeedDialDialog.cpp @@ -270,7 +270,7 @@ private: // why: value-capture only. Script execution may outlive this popup if the app is closing. wxGetApp().CallAfter([id] { // why: the queue may drain during shutdown, after MainFrame/Plater teardown; - // skip like the ScriptActionSource callbacks do instead of running into it. + // skip like the registry's loader callbacks do instead of running into it. if (wxGetApp().is_closing()) return; AppActionRunResult o = wxGetApp().action_registry().run(id); diff --git a/tests/slic3rutils/CMakeLists.txt b/tests/slic3rutils/CMakeLists.txt index b1121103a2..8b3630ff95 100644 --- a/tests/slic3rutils/CMakeLists.txt +++ b/tests/slic3rutils/CMakeLists.txt @@ -8,7 +8,6 @@ add_executable(${_TEST_NAME}_tests test_plugin_lifecycle.cpp test_slicing_pipeline_bindings.cpp test_plugin_sort.cpp - test_speed_dial_action_id.cpp ) if (MSVC) diff --git a/tests/slic3rutils/test_action_source.cpp b/tests/slic3rutils/test_action_source.cpp index f85d1b42d9..b6721e22d6 100644 --- a/tests/slic3rutils/test_action_source.cpp +++ b/tests/slic3rutils/test_action_source.cpp @@ -1,7 +1,6 @@ #include <catch2/catch_test_macros.hpp> #include "slic3r/GUI/ActionRegistry.hpp" -#include "slic3r/GUI/IActionSource.hpp" #include <memory> #include <string> @@ -10,45 +9,42 @@ using Slic3r::GUI::AppAction; using Slic3r::GUI::AppActionRunResult; 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; -}; - -// AppAction is abstract; this minimal concrete action lets the tests exercise -// its constructor-set definition without involving a plugin runner. +// AppAction is abstract; this minimal concrete action lets the tests exercise its +// constructor-composed identity without involving a plugin runner. class TestAppAction final : public AppAction { public: - TestAppAction() : AppAction("action-id", "Action title", "Action source") {} + TestAppAction() : AppAction("test", "Action title", "src-key", "Action source") {} AppActionRunResult run() const override { return {}; } }; } // namespace +TEST_CASE("AppAction composes a stable id from prefix:title:source_key", "[speeddial][actions]") +{ + CHECK(AppAction::compose_id("test", "Action title", "src-key") == "test:Action title:src-key"); + // source_key (not the display name) carries identity, so it is the third field. + CHECK(AppAction::compose_id("script", "Do Thing", "pack.py") == "script:Do Thing:pack.py"); +} + TEST_CASE("AppAction definitions are immutable after construction", "[speeddial][actions]") { using StringAccessor = const std::string& (AppAction::*)() const; STATIC_CHECK(std::is_same_v<decltype(&AppAction::id), StringAccessor>); STATIC_CHECK(std::is_same_v<decltype(&AppAction::title), StringAccessor>); - STATIC_CHECK(std::is_same_v<decltype(&AppAction::source), StringAccessor>); + STATIC_CHECK(std::is_same_v<decltype(&AppAction::source_key), StringAccessor>); + STATIC_CHECK(std::is_same_v<decltype(&AppAction::source_name), StringAccessor>); const TestAppAction action; - CHECK(action.id() == "action-id"); + CHECK(action.id() == "test:Action title:src-key"); CHECK(action.title() == "Action title"); - CHECK(action.source() == "Action source"); + CHECK(action.source_key() == "src-key"); + CHECK(action.source_name() == "Action source"); } TEST_CASE("ActionRegistry takes exclusive ownership of published actions", "[speeddial][actions]") @@ -57,14 +53,3 @@ TEST_CASE("ActionRegistry takes exclusive ownership of published actions", "[spe STATIC_CHECK(std::is_same_v<decltype(&ActionRegistry::upsert), ExpectedUpsert>); } - -TEST_CASE("ActionRegistry starts registered action sources", "[speeddial][actions]") -{ - bool started = false; - ActionRegistry registry; - registry.add_source(std::make_unique<RecordingActionSource>(started)); - - registry.init(); - - CHECK(started); -} diff --git a/tests/slic3rutils/test_speed_dial_action_id.cpp b/tests/slic3rutils/test_speed_dial_action_id.cpp deleted file mode 100644 index c2667df839..0000000000 --- a/tests/slic3rutils/test_speed_dial_action_id.cpp +++ /dev/null @@ -1,28 +0,0 @@ -#include <catch2/catch_all.hpp> - -#include <slic3r/GUI/SpeedDialActionId.hpp> - -using Slic3r::GUI::speed_dial_action_id; - -TEST_CASE("speed_dial_action_id is deterministic and opaque", "[speeddial][id]") { - const std::string a = speed_dial_action_id("pack.py", "Do Thing"); - const std::string b = speed_dial_action_id("pack.py", "Do Thing"); - CHECK(a == b); // stable within a run - CHECK(a.size() == 16); // 64-bit -> 16 hex chars - CHECK(a.find_first_not_of("0123456789abcdef") == std::string::npos); -} - -TEST_CASE("speed_dial_action_id separates key and capability", "[speeddial][id]") { - // The 0x1f separator prevents (key+cap) run-together collisions: - // ("ab","c") must not equal ("a","bc"). - CHECK(speed_dial_action_id("ab", "c") != speed_dial_action_id("a", "bc")); - CHECK(speed_dial_action_id("k", "one") != speed_dial_action_id("k", "two")); - CHECK(speed_dial_action_id("k1", "cap") != speed_dial_action_id("k2", "cap")); -} - -TEST_CASE("speed_dial_action_id golden vector locks cross-run stability", "[speeddial][id]") { - // why: this value is PERSISTED as a config key; if the algorithm ever drifts, - // every saved favourite/stat silently orphans. Pin a known input->output. - // FNV-1a 64-bit of bytes: 'k','e','y',0x1f,'c','a','p' - CHECK(speed_dial_action_id("key", "cap") == "c54ad154fb2c9187"); -}