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

Use distinct annotations for command-specific properties #146

Merged
merged 29 commits into from
Jan 16, 2024

Conversation

freya022
Copy link
Owner

@freya022 freya022 commented Jan 10, 2024

Problem

Annotated text commands as well as slash commands suffer from several issues do to all the data being written in a single annotation:

  • For multiple commands with the same top-level name, you must specify the same data everytime (like the scope), which causes hard to diagnose issues if the values are not the same everywhere, without even mentioning the maintainability problem
  • The description of subcommand groups had a chance to not be applied on the right group
    • This is due to the annotation applying the description to the top-level command, the subcommand group, as well as the subcommand, as all of them share the same description
    • This is also what causes unnecessary application command updates, as the description of subcommand groups would sometimes alternate (partially due an unstable order of reflectively-obtained methods), this did not cause an issue as subcommand groups are not visible on Discord (yet)
  • Text commands has a generalDescription property, but this only applies to the text command itself, i.e., not on the text command variation
    • The user may receive a warning if such properties were set more than once on a same command
    • This required a way to sort text command variations, determining which variation would be above every other variation, from which you could take the data

Solution

Properties which are attribuable to only a part of a command (such as the scope which only can be set on the top-level command) will be in a different annotation:

  • Slash commands will have 3 annotations:
    • [Required on subcommands] Top-level (scope, defaultLocked, nsfw, description)
    • [Optional] Subcommand group (description)
    • [Required] Slash command (name, group, subcommand, description)
  • Text commands will have 2 annotations:
    • [Optional] Text command (path, aliases, description) (path is the path this annotation applies to, if empty, apply on the variation's path)
    • [Required] Variation (path, order, description, usage, example)

Annotations which applies to text commands may appear on a single variation, but they affect the command as a whole, and are not allowed more than once per path

The description for a top-level command can be only be set on either annotations

  • Update live templates
  • Test built-in help content, see if all descriptions go to the right levels

@freya022 freya022 added type: enhancement type: breaking Contains a backwards incompatible change V3 Targets BotCommands 3.x.x labels Jan 10, 2024
@freya022 freya022 added this to the v3.0 milestone Jan 10, 2024

This comment has been minimized.

@freya022 freya022 changed the title Use distinct annotations for top-level commands and other non-assignable properties Use distinct annotations for command-specific properties Jan 14, 2024
@freya022 freya022 merged commit 7387dda into 3.X Jan 16, 2024
4 checks passed
@freya022 freya022 deleted the v3/refactor/top-level-commands branch January 16, 2024 21:30
freya022 added a commit that referenced this pull request Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: breaking Contains a backwards incompatible change type: enhancement V3 Targets BotCommands 3.x.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant