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

Ephemeral Commands #3602

Open
Jackenmen opened this issue Feb 22, 2020 · 3 comments
Open

Ephemeral Commands #3602

Jackenmen opened this issue Feb 22, 2020 · 3 comments
Labels
Category: Core - API - Commands Package This is related to the `redbot.core.commands` package or `redbot.core.checks` module. Status: Needs Discussion Needs more discussion. Type: Feature New feature or request.

Comments

@Jackenmen
Copy link
Member

Copied from original issue (#3163):

Rationale

There are a lot of issues with our current handling of the core Alias and CustomCommand cogs.

Without enumerating all of these, the common thread is that we are dynamically creating command objects for these to get command like behavior without potentially causing a conflict with an existing command, or preventing loading a cog by registering the command object to the bot.

This comes with many issues, most noticeably a performance one.

We could go back to the old way of just waiting to see a non-command, and then issuing a response on these, but we would lose many benefits we gained from making these bahave as commands, such as restricting where they can be used, and who they can be used by with core settings.

I think we can do better than give up on this here.

How

If we allowed registering commands to a secondary command mapping for commands which should only exist while there isn't a real command by the same name, we could handle this.

Under the hood, this would use a ChainMap, and require some tweaks with our handling with permissions, adding and removing commands.

Ideally, this could be as simple as adding ephemeral=True to the command constructor from the cog developer side if they want a command to be easily overriden, or if they want to register a response handler, but only when there isn't a conflicting command.

Other benefits

We could make certain commands in core red which are frequently replaced with custom versions ephemeral commands, thereby making replacing their implementations easier to do without breaking other features.

There's also at least one 3rd party cog of which I'm aware which would be able to make use of this.

Limitations

To limit the complexities which may arise in handling this, only commands or command groups will actually use this, subcommands or subgroups will be attached to the parent.

@Jackenmen Jackenmen added Type: Feature New feature or request. Category: Core - API - Commands Package This is related to the `redbot.core.commands` package or `redbot.core.checks` module. labels Feb 22, 2020
@github-actions github-actions bot added the Status: Needs Triage This has not been labeled or discussed for handling yet. label Feb 22, 2020
@Jackenmen Jackenmen added Complex Issue This issue may require more than a surface level fix or be highly integrated with other components Status: Needs Discussion Needs more discussion. and removed Status: Needs Triage This has not been labeled or discussed for handling yet. Complex Issue This issue may require more than a surface level fix or be highly integrated with other components labels Feb 22, 2020
@Jackenmen Jackenmen added this to Dependencies of rewrite in Alias Rewrite Jun 10, 2020
@mikeshardmind
Copy link
Contributor

If I don't fill in the details of what I envisoned on this by this Sunday, mention me on discord and remind me I was going to put more detail as to what I originally envisioned here.

@Zephyrkul
Copy link
Contributor

While I ultimately discarded it a couple years ago in favor of a better rewrite, I did previously attempt to tackle something akin to this in 2018 in #2144. If the code there proves to be useful to someone tackling this issue, feel free to use it.

@mikeshardmind
Copy link
Contributor

There's a semi-non-trivial piece of work that needs to be done to support this. commands / all commands both need modifications that preserve the inability to directly access them but fill from a chain map of the normal and ephemeral commands separately.

This also (somewhat) necessitates us warning against using invoke to trigger other commands with an assumption that you got the command you expected (it's not safe right now either, but it becomes even less safe after a change like this), handling conflicts in ephemeral commands in a predictable manner (possible to just reject multiple by shared name), and providing an API to add ephemeral commands without them being attached to a cog directly (such as the dynamic CC/Alias cases)

@Zephyrkul Zephyrkul mentioned this issue Sep 16, 2020
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Core - API - Commands Package This is related to the `redbot.core.commands` package or `redbot.core.checks` module. Status: Needs Discussion Needs more discussion. Type: Feature New feature or request.
Projects
Alias Rewrite
  
Dependencies of rewrite and related i...
Development

No branches or pull requests

3 participants