Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sort plugins in settings dialog alphabetically #103

Conversation

radioactiveman
Copy link
Member

No description provided.

@radioactiveman
Copy link
Member Author

The "random" order is especially noticeable in the Effects tab with "Crossfade" and "Background Music" shown at the bottom.

Questions:

  • Is the current order determined by plugin_compare()?
  • Is there an easier way to copy an Index list?
  • Is auto & list or auto list correct for the libaudqt part?
  • Is the function name okay? It's not clear that the order is alphabetically.

static int plugin_compare(PluginHandle * const & a, PluginHandle * const & b)
{
if (a->type < b->type)
return -1;
if (a->type > b->type)
return 1;
if (a->priority < b->priority)
return -1;
if (a->priority > b->priority)
return 1;
int diff;
if ((diff = str_compare(dgettext(a->domain, a->name),
dgettext(b->domain, b->name))))
return diff;
return str_compare(a->path, b->path);
}

@jlindgren90
Copy link
Member

Yes, the current order uses plugin_compare(). This is priority-order.

I would suggest creating the alphabetically-sorted list once at startup rather than on demand. PluginListModel.index() (for example) can be called frequently and it would be better to not recreate the sorted list constantly.

If the list is created at startup, then aud_plugin_list_sorted can return a reference just like aud_plugin_list. Then auto & list will still be correct in libaudqt.

Something like this:

diff --git a/src/libaudcore/plugin-registry.cc b/src/libaudcore/plugin-registry.cc
index d5d72a603..1b894d77c 100644
--- a/src/libaudcore/plugin-registry.cc
+++ b/src/libaudcore/plugin-registry.cc
@@ -103,6 +103,7 @@ static constexpr aud::array<InputKey, const char *> input_key_names = {
 
 static aud::array<PluginType, Index<PluginHandle *>> plugins;
 static aud::array<PluginType, Index<PluginHandle *>> compatible;
+static aud::array<PluginType, Index<PluginHandle *>> sorted; /* by name */
 static aud::mutex mutex;
 static bool modified = false;
 
@@ -444,6 +445,10 @@ void plugin_registry_prune()
         plugins[type].sort(plugin_compare);
         compatible[type].insert(plugins[type].begin(), 0, plugins[type].len());
         compatible[type].remove_if(check_incompatible);
+        sorted[type].insert(compatible[type].begin(), 0, compatible[type].len());
+        sorted[type].sort([](PluginHandle * a, PluginHandle * b) {
+            return str_compare(aud_plugin_get_name(a), aud_plugin_get_name(b));
+        });
     }
 }
 
@@ -623,6 +628,11 @@ EXPORT const Index<PluginHandle *> & aud_plugin_list(PluginType type)
     return compatible[type];
 }
 
+EXPORT const Index<PluginHandle *> & aud_plugin_list_sorted(PluginType type)
+{
+    return sorted[type];
+}
+
 EXPORT const char * aud_plugin_get_name(PluginHandle * plugin)
 {
     return dgettext(plugin->domain, plugin->name);

@radioactiveman
Copy link
Member Author

radioactiveman commented Jan 20, 2024

Thanks, updated with the suggested changes. We also need to clear the list in plugin_registry_cleanup(). Ready to merge now?

@jlindgren90
Copy link
Member

Looks good. Thanks!

@jlindgren90 jlindgren90 merged commit d8f5366 into audacious-media-player:master Jan 21, 2024
8 checks passed
@radioactiveman radioactiveman deleted the sort-plugins-alphabetically branch January 21, 2024 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants