Skip to content

Loading…

addons: don't show a GUI notification for addons installed as a dependency of another addon installation #1398

Closed
wants to merge 1 commit into from

7 participants

@Montellese
Team Kodi member

So I recently installed a few addons and I got a GUI notification for every single dependency that was installed with it like the "Common Plugin Cache" or the "Simple Downloader for XBMC Plugins" which IMO can be a bit confusing if you just wanted to install youtube. These addons (services) will never be directly used by the user so why tell them that they have been installed?

@MartijnKaijser
Team Kodi member

+1 from me on this. When installing some skins you get tons of new scripts which the user doesn't have to know

@mkortstiege
Team Kodi member

+1. I think it's more user friendly.

@ghost

-1. if shit end up on my system I want to be told. what if apt did this?

@Montellese
Team Kodi member

I don't follow.

@dersphere
Team Kodi member

-1. A dependency can be another (visible) plugin. And a user could wonder why the required video plugin was installed. If he then disables/uninstalls the required plugin the original plugin stops working.

@MartijnKaijser
Team Kodi member

You cannot uninstall a dependency plugin and you are told why. Not sure about disable.

@Montellese
Team Kodi member

@Martijn: AFAIK you can uninstall any addon independent of whether it's a dependency of another addon.

@dersphere: And how does showing a notification during installation help me remember a year later that youtube addon requires Common Cache Plugin? It's not like showing the notification will make sure that you don't uninstall it by accident. For that we'd need some kind of message that warns you that uninstalling addon X might break addons Y and Z because they depend on it.

@dersphere
Team Kodi member

@MartijnKaijser: This is going to be offtopic but of course you can uninstall a via dependency installed addon. You can easy reproduce (with eden or master): install "TheHollywoodreporter" which installs "youtube". uninstall "youtube".

@Montellese: You are right. But at least it should be shown on required plugins to have any kind of notification on visible addons.

@arnova
Team Kodi member

In principle +1. And if we don't, we should at least make clear to the user that those are auto deps being installed...

@night199uk

-1 from me, I want to know which plugins get installed so I know when they cause issues.

@Montellese
Team Kodi member

Closing as there are too many -1s (although there are as many +1s).

@Montellese Montellese closed this
@da-anda
Team Kodi member

+1 -> see http://forum.xbmc.org/showthread.php?tid=151774&pid=1308555#pid1308555

We also have to deny disabling/uninstalling of addons that are part of a dependency and also show active dependencies in the addon info-dialog (both ways; "this addon depends on Foo, Bar, Baz;" "required by addons Foo, Bar"). But these features should be added in another PR.

Also I'd probably hide all notifications (e.g. "marked as broken") if a addon was installed as part of a dependency. I don't want to know if common.plugin.cache was updated or not - hell, I don't even know what it does. In doubt, let's add a history/log system and show performed actions in there for those that care, or add an advanced setting to enable the notification spam.

@Montellese Montellese deleted the Montellese:addon_dependency_notifications branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 9, 2012
  1. @Montellese

    addons: don't show a GUI notification for addons installed as a depen…

    Montellese committed
    …dency of another addon installation
Showing with 13 additions and 12 deletions.
  1. +9 −9 xbmc/addons/AddonInstaller.cpp
  2. +4 −3 xbmc/addons/AddonInstaller.h
View
18 xbmc/addons/AddonInstaller.cpp
@@ -205,7 +205,7 @@ bool CAddonInstaller::PromptForInstall(const CStdString &addonID, AddonPtr &addo
return false;
}
-bool CAddonInstaller::Install(const CStdString &addonID, bool force, const CStdString &referer, bool background)
+bool CAddonInstaller::Install(const CStdString &addonID, bool force, const CStdString &referer /* = "" */, bool background /* = true */, bool dependency /* = false */)
{
AddonPtr addon;
bool addonInstalled = CAddonMgr::Get().GetAddon(addonID, addon, ADDON_UNKNOWN, false);
@@ -225,12 +225,12 @@ bool CAddonInstaller::Install(const CStdString &addonID, bool force, const CStdS
CStdString hash;
if (therepo)
hash = therepo->GetAddonHash(addon);
- return DoInstall(addon, hash, addonInstalled, referer, background);
+ return DoInstall(addon, hash, addonInstalled, referer, background, dependency);
}
return false;
}
-bool CAddonInstaller::DoInstall(const AddonPtr &addon, const CStdString &hash, bool update, const CStdString &referer, bool background)
+bool CAddonInstaller::DoInstall(const AddonPtr &addon, const CStdString &hash, bool update, const CStdString &referer /* = "" */, bool background /* = true */, bool dependency /* = false */)
{
// check whether we already have the addon installing
CSingleLock lock(m_critSection);
@@ -249,14 +249,14 @@ bool CAddonInstaller::DoInstall(const AddonPtr &addon, const CStdString &hash, b
if (background)
{
- unsigned int jobID = CJobManager::GetInstance().AddJob(new CAddonInstallJob(addon, hash, update, referer), this);
+ unsigned int jobID = CJobManager::GetInstance().AddJob(new CAddonInstallJob(addon, hash, update, referer, dependency), this);
m_downloadJobs.insert(make_pair(addon->ID(), CDownloadJob(jobID)));
}
else
{
m_downloadJobs.insert(make_pair(addon->ID(), CDownloadJob(0)));
lock.Leave();
- CAddonInstallJob job(addon, hash, update, referer);
+ CAddonInstallJob job(addon, hash, update, referer, dependency);
if (!job.DoWork())
{ // TODO: dump something to debug log?
return false;
@@ -382,8 +382,8 @@ bool CAddonInstaller::HasJob(const CStdString& ID) const
return m_downloadJobs.find(ID) != m_downloadJobs.end();
}
-CAddonInstallJob::CAddonInstallJob(const AddonPtr &addon, const CStdString &hash, bool update, const CStdString &referer)
-: m_addon(addon), m_hash(hash), m_update(update), m_referer(referer)
+CAddonInstallJob::CAddonInstallJob(const AddonPtr &addon, const CStdString &hash, bool update, const CStdString &referer /* = "" */, bool dependency /* = false */)
+: m_addon(addon), m_hash(hash), m_update(update), m_referer(referer), m_dependency(dependency)
{
}
@@ -541,7 +541,7 @@ bool CAddonInstallJob::Install(const CStdString &installFrom)
force = false;
}
// don't have the addon or the addon isn't new enough - grab it (no new job for these)
- if (!CAddonInstaller::Get().Install(addonID, force, referer, false))
+ if (!CAddonInstaller::Get().Install(addonID, force, referer, false, true))
{
DeleteAddon(addonFolder);
return false;
@@ -553,7 +553,7 @@ bool CAddonInstallJob::Install(const CStdString &installFrom)
void CAddonInstallJob::OnPostInstall(bool reloadAddon)
{
- if (m_addon->Type() < ADDON_VIZ_LIBRARY && g_settings.m_bAddonNotifications)
+ if (m_addon->Type() < ADDON_VIZ_LIBRARY && g_settings.m_bAddonNotifications && !m_dependency)
{
CGUIDialogKaiToast::QueueNotification(m_addon->Icon(),
m_addon->Name(),
View
7 xbmc/addons/AddonInstaller.h
@@ -51,7 +51,7 @@ class CAddonInstaller : public IJobCallback
\return true on successful install, false on failure.
\sa DoInstall
*/
- bool Install(const CStdString &addonID, bool force = false, const CStdString &referer="", bool background = true);
+ bool Install(const CStdString &addonID, bool force = false, const CStdString &referer="", bool background = true, bool dependency = false);
/*! \brief Install an addon from the given zip path
\param path the zip file to install from
@@ -121,7 +121,7 @@ class CAddonInstaller : public IJobCallback
\param background whether to install in the background or not. Defaults to true.
\return true on successful install, false on failure.
*/
- bool DoInstall(const ADDON::AddonPtr &addon, const CStdString &hash = "", bool update = false, const CStdString &referer = "", bool background = true);
+ bool DoInstall(const ADDON::AddonPtr &addon, const CStdString &hash = "", bool update = false, const CStdString &referer = "", bool background = true, bool dependency = false);
CCriticalSection m_critSection;
JobMap m_downloadJobs;
@@ -133,7 +133,7 @@ class CAddonInstaller : public IJobCallback
class CAddonInstallJob : public CFileOperationJob
{
public:
- CAddonInstallJob(const ADDON::AddonPtr &addon, const CStdString &hash = "", bool update = false, const CStdString &referer = "");
+ CAddonInstallJob(const ADDON::AddonPtr &addon, const CStdString &hash = "", bool update = false, const CStdString &referer = "", bool dependency = false);
virtual bool DoWork();
@@ -168,6 +168,7 @@ class CAddonInstallJob : public CFileOperationJob
CStdString m_hash;
bool m_update;
CStdString m_referer;
+ bool m_dependency;
};
class CAddonUnInstallJob : public CFileOperationJob
Something went wrong with that request. Please try again.