Red-DiscordBot icon indicating copy to clipboard operation
Red-DiscordBot copied to clipboard

Ephemeral Commands

Open Jackenmen opened this issue 5 years ago • 3 comments

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 avatar Feb 22 '20 00:02 Jackenmen

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.

mikeshardmind avatar Aug 06 '20 01:08 mikeshardmind

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.

Zephyrkul avatar Aug 06 '20 01:08 Zephyrkul

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)

mikeshardmind avatar Aug 17 '20 01:08 mikeshardmind