Skip to content

Conversation

BobDotCom
Copy link
Contributor

@BobDotCom BobDotCom commented Jan 11, 2022

Summary

Rewrites command registration to actually be efficient and smart.

Checklist

Needs testing

  • Command groups

And maybe more...

Unfinished

  • Command permissions (v1)
  • Command permisisons (v2)
  • Documentation (New methods)
  • Documentation (Changed methods)
  • Documentation (Guides/examples if applicable)

And maybe more...

If there's anything I need to add just let me know.

If anyone would like to contribute, just make a PR on the feature/command-registration branch.

@BobDotCom BobDotCom added help wanted Extra attention is needed priority: high High Priority status: in progress Work in Progess Merge normally labels Jan 11, 2022
@BobDotCom BobDotCom added this to the v2.0 milestone Jan 11, 2022
@BobDotCom BobDotCom self-assigned this Jan 11, 2022
@BobDotCom BobDotCom linked an issue Jan 11, 2022 that may be closed by this pull request
@BobDotCom BobDotCom mentioned this pull request Jan 11, 2022
7 tasks
@Luc1412
Copy link
Contributor

Luc1412 commented Feb 2, 2022

Generally it currently it doesn't act as smart as it could. It still often re-creates all commands even trough I didn't modified any command. With a few restart I reached the 200 command creation limit.

An option for not deleting missing commands would be useful in case a cog isn't loaded or a command is added/updated later on runtime.

A perfect flow would be:
First startup: Bulk upsert all commands
Restart withou modifing cmds: No Action
Restart with modifing a command/updating a command on runtime: Only edit the command.

This also applies to multi process bots:
First Process starts: Register all commands
Seconds process starts: Just fetch the added commands
....

@BobDotCom BobDotCom marked this pull request as ready for review February 4, 2022 01:42
@BobDotCom
Copy link
Contributor Author

Generally it currently it doesn't act as smart as it could. It still often re-creates all commands even trough I didn't modified any command. With a few restart I reached the 200 command creation limit.

An option for not deleting missing commands would be useful in case a cog isn't loaded or a command is added/updated later on runtime.

A perfect flow would be: First startup: Bulk upsert all commands Restart withou modifing cmds: No Action Restart with modifing a command/updating a command on runtime: Only edit the command.

This also applies to multi process bots: First Process starts: Register all commands Seconds process starts: Just fetch the added commands ....

I haven't encountered this at all, are you sure? The whole reason this was rewritten was because the commands shouldn't be bulk registered every time if they don't need to be. This setup should utilize the minimum amount of registrations possible, though it will hit its limits if you start up without commands a couple times.

@BobDotCom BobDotCom linked an issue Feb 4, 2022 that may be closed by this pull request
VincentRPS
VincentRPS previously approved these changes Feb 4, 2022
Copy link
Contributor

@VincentRPS VincentRPS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Lulalaby
Lulalaby previously approved these changes Feb 4, 2022
@BobDotCom BobDotCom enabled auto-merge February 4, 2022 02:05
@BobDotCom BobDotCom disabled auto-merge February 4, 2022 02:12
@BobDotCom
Copy link
Contributor Author

Forgot to make one change

@BobDotCom BobDotCom dismissed stale reviews from Lulalaby and VincentRPS via 9205fc6 February 4, 2022 02:22
@BobDotCom BobDotCom enabled auto-merge February 4, 2022 02:22
@BobDotCom BobDotCom disabled auto-merge February 4, 2022 02:31
@BobDotCom BobDotCom enabled auto-merge (squash) February 4, 2022 02:31
Copy link
Member

@ChickenDevs ChickenDevs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@BobDotCom BobDotCom merged commit 396082a into master Feb 4, 2022
@BobDotCom BobDotCom deleted the feature/command-registration branch February 4, 2022 02:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed priority: high High Priority status: in progress Work in Progess
Projects
None yet
5 participants