mtkennerly / ludusavi-playnite

Playnite plugin for save backups via Ludusavi
MIT License
141 stars 9 forks source link

Use prefix for extension tags #32

Closed darklinkpower closed 2 years ago

darklinkpower commented 2 years ago

I noticed that tags used for functionality have been added for the extension, for example ludusavi-skip and ludusavi-backup were added in https://github.com/mtkennerly/ludusavi-playnite/commit/69dae3a39b8451ccf5969eabf5904781368675d7

As a suggestion, I was wondering if the extension could use a prefix. We've done this in other extensions so functionality tags and features are on top on the list and also to not make them be in the middle of "proper" tags in the list

Examples: image

image

mtkennerly commented 2 years ago

Thanks for the suggestion! I'm open to this for consistency with other extensions. Just two things to consider:

darklinkpower commented 2 years ago

Regarding the old tag, it could be renamed on the new updated. A setting value could be added to check if it's been migrated during the OnApplicationStarted, something like

if (!settings.TagsMigrated)
{
    var ludusaviSkipTag = PlayniteApi.Database.Tags.FirstOrDefault(x => x.Name == "ludusavi-skip");
    if (ludusaviSkipTag != null)
    {
        ludusaviSkipTag.Name = "[Ludusavi] Skip";
        PlayniteApi.Database.Tags.Update(ludusaviSkipTag);
    }

    SavePluginSettings(settings);
}

There's no standard set regarding tags naming but it's just something we just started doing in different extensions and everyone kinda followed using the format [<ExtensionName, Extension initials or similar>] <TagName>

Personally I think it's better to just create the tag when first needed. If you try to create a tag like var tag = PlayniteApi.Database.Tags. Add("SomeName") will return the existing tag if it already exists or also create it if it does't so I don't think it should be needed to create it beforehand as it can be done when using the function to add the tags.

mtkennerly commented 2 years ago

I like the migration idea :+1:

will return the existing tag if it already exists or also create it if it does't so I don't think it should be needed to create it beforehand as it can be done when using the function to add the tags

I was mainly thinking of users who want to add the tag from the dropdown (instead of using the extension's context menu). They'd have to double check the spelling of the tag to make sure they enter it correctly the first time, and there's always the risk of typos, so I thought it might be nice to automatically have it available to choose. On the other hand, if users aren't interested in using Ludusavi's tag functionality, then it's just clutter in their list.

darklinkpower commented 2 years ago

What I've opted in those cases is just create them when needed because I don't think it's a good idea to clutter tags and features with items the user may not want. I think manual adding, while possible, should not be a deciding factor as the extensions should provide functionality to do this by clicking a menu item.

To provide an example, in this extension I added menu items to add and remove features:

https://github.com/darklinkpower/PlayniteExtensionsCollection/blob/b29e507ae26e2266e310c62869a0f5663ca39473/source/Generic/PlayState/PlayState.cs#L456-L474