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

[Discussion] Load downloader by default #5940

Open
Flame442 opened this issue Dec 27, 2022 · 14 comments
Open

[Discussion] Load downloader by default #5940

Flame442 opened this issue Dec 27, 2022 · 14 comments
Labels
Category: Cogs - Downloader This is related to the Downloader cog. Category: Core - Bot Commands This is related to core commands (Core and CogManagerUI cog classes). Category: Core - Other Internals This is related to core internals that don't have a dedicated label. Category: Meta This is related to the repository maintenance. Status: Needs Discussion Needs more discussion. Type: Enhancement Something meant to enhance existing Red features. Type: Informational Aims to provide information or facilitate discussion.
Milestone

Comments

@Flame442
Copy link
Member

Flame442 commented Dec 27, 2022

What's the problem?

V3 has a philosophy of not loading any cogs by default, and only enabling the absolute bare minimum functionality in the bot's core on bot startup. This is to prevent users who have just installed Red from feeling overwhelmed by multiple pages of commands, many of which they may not want to use. An obvious example for this kind of thinking can be found looking at the Audio cog, and its 27 top level commands.

The black sheep of this philosophy has always been the Downloader cog, as is only has 4 top level commands which are intended to add the modularity that Red boasts as a major selling point. Currently, there is little reason to not have this cog loaded at all times, since almost all Red users will install at least 1 third party cog, and will want to be able to update that cog later.

How do we fix it?

There are a few different ways to approach making such a change:

Option 1 is to load Downloader by default in new installs, but is otherwise a normal cog after that. This will require the least amount of work to implement, and will not be nearly as much of a breaking change. Users can still decide to unload the Downloader cog if they for some reason wish to do so. However, this option will break the philosophy of V3, and may be slightly confusing for users who may not understand why this cog is loaded and no other.

Option 2 is to build Downloader into Core, similar to how CogManagerUI currently is. This will require more work, will be more of a breaking change, and will make it impossible for users to decide to unload the Downloader cog for whatever reason. However, it always being an available part of the bot will allow other cogs to more safely access its internals without having to also disable themselves when downloader is not loaded. It will also make it easier to make and support actual public APIs, rather than third party cogs like Neuro's updatechecker having to rely on internals. EDIT: It may make sense to consider #5943 at the same time this is done, while Downloader is receiving a major overhaul, if this option is picked.

Option 3 is to do nothing. Obviously this is the easiest option, but it means nothing will change.

What can I do?

We are looking for community feedback regarding this change. If you have any opinions or concerns regarding any of these options, or any options we may not have considered, please let us know in this issue. If we see enough support for this change, we may look in to making one of these changes for 3.6.0 (or some other breaking version thereafter).

@Flame442 Flame442 added Type: Enhancement Something meant to enhance existing Red features. Status: Needs Discussion Needs more discussion. Category: Bot Core Category: Cogs - Downloader This is related to the Downloader cog. Category: Meta This is related to the repository maintenance. labels Dec 27, 2022
@Flame442 Flame442 added this to the 3.6.0 milestone Dec 27, 2022
@Drapersniper
Copy link
Contributor

Drapersniper commented Dec 27, 2022

Not sure if the plan is still to merge @Twentysix26 Index cog with Downloader -

while the first would be the easiest is there any real work scenario where it makes sense to unload downloader?

the only thing I can think of is keeping the top level command namespace clear but not aware of any other commands that conflict with downloader anyways.

From a user perspective given how red is advertised it would make sense longterm to bundle it into core similar to CogManagerUI unless there is a clear benefit/reason to ever unload it removing the ability to unload would just simplify the majority of users experience - i.e no need to load Downloader when they inevitably want to install a new cog/update it etc

A quick and cheap way would be to go with option 1 first (potentially even as part of 3.5) and then merge into core as the next minor after

@Jackenmen
Copy link
Member

Not sure if the plan is to merge @ Twentysix26 Index cog with Downloader

That would be a separate issue entirely.

@Twentysix26
Copy link
Member

+1 on option 2 if there is real community interest in a Downloader public API. Otherwise, I don't see much reason to spend time changing a setup that works fine as is.

@wohali
Copy link

wohali commented Dec 29, 2022

I like Option 2, but could see the value in having a global disable of downloading functionality (possibly out-of-band, an env var or startup option). I can't see the downside of always including the downloading functionality as long as it can be hard-disabled somehow.

As-is is great too, every use case is catered for, even if it is a special case philosophically.

@ltzmax
Copy link
Contributor

ltzmax commented Dec 30, 2022

I'll say +1 for option 2.
It sounds pretty alright option since I nearly never see anyone actually unload downloader, You pretty much need it for cog installing, repo adding and cog update / repo update.

@qenu
Copy link
Contributor

qenu commented Dec 30, 2022

+1 for option 2
Imo, cogs are one of the core features for Red.
Moving Downloader to Core sounds like the intuitive option.

@Kuro-Rui
Copy link
Contributor

Kuro-Rui commented Jan 1, 2023

Imma say +1 for option 2
Since many people want to download another external cog, building Downloader into Core will reduce some work and this will be much more efficient.

@palmtree5
Copy link
Member

+1 on option 2 from me

Reading this, it's the option that makes the most sense, since we do advertise being fully modular and a lot of that modularity comes from installing third-party cogs through Downloader

@AAA3A-AAA3A
Copy link
Contributor

+1 for option 2 as well.

First of all, from a new user's perspective, putting the Downloader cog in Core would prevent new users from asking why the add repo command doesn't work.

Also, from a normal user's point of view, this cog is perhaps the most used over a long period of time by users. Running Red without the third-party cogs is only rarely done. It never needs to be unloaded and, since it is rigorously tested before each Red release, it doesn't cause critical errors in the bot (and the fact that it doesn't work would be a problem anyway). In case of an error caused by a loaded cog, the user can use the --no-cogs cli flag, but is often forced to load Downloader to uninstall the problematic cog or retrieve the url from the repo in order to open a problem.

Finally, from the cogs developer's point of view, loading Downloader by default ensures that the user is at the latest version of the code, unless of course he wants to pin the module. This way the user is aware that he can do it easily. Some cogs also use Downloader to retrieve the commit of an installed cog or repo to warn the user that the cog is out of date or calculate a version number. The fact that Downloader may not be loaded and prevents some cogs features from working.

So Downloader has no reason not to be loaded continuously, and I think Red should add it in Core. It provides necessary functionality to all users without exception, unlike default cogs which may not be used.

@Zephyrkul
Copy link
Contributor

+1 for option 1.

@vertyco
Copy link
Contributor

vertyco commented Jan 11, 2023

+1 for option 2, can't think of a reason not to build it into core.

@hatchcanon
Copy link

+1 for option 2

Downloader should be built-in

@Flame442 Flame442 added the Type: Informational Aims to provide information or facilitate discussion. label Jan 12, 2023
@Jackenmen Jackenmen added Category: Core - Bot Commands This is related to core commands (Core and CogManagerUI cog classes). Category: Core - Other Internals This is related to core internals that don't have a dedicated label. and removed Category: Bot Core labels Feb 12, 2023
@jump
Copy link

jump commented Apr 2, 2023

I'd like to see it added to the setup. Perhaps that and a few other things. Do you want ability to download cogs enabled by default? Something of that sort.

@Jackenmen
Copy link
Member

I don't think that's something we would be interested in doing. If Downloader is integrated into Core, I don't see us offering a way to cut it off completely. Users will just be able to choose not to use its functionality by disabling or simply not using those commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Cogs - Downloader This is related to the Downloader cog. Category: Core - Bot Commands This is related to core commands (Core and CogManagerUI cog classes). Category: Core - Other Internals This is related to core internals that don't have a dedicated label. Category: Meta This is related to the repository maintenance. Status: Needs Discussion Needs more discussion. Type: Enhancement Something meant to enhance existing Red features. Type: Informational Aims to provide information or facilitate discussion.
Projects
None yet
Development

No branches or pull requests