refactor!: command sync #112
Conversation
VincentRPS
added
enhancement
| tasks.extend(self._process_global_command_for(command)) | ||
| await asyncio.gather(*tasks) | ||
|
|
||
| async def _process_global_command_for(self, command: Command) -> list[Coroutine]: |
There was a problem hiding this comment.
Why is command type hinted as Command if this is for application commands?
There was a problem hiding this comment.
people could make their own application command class without subclassing app command itself.
There was a problem hiding this comment.
No they shouldn't? The subclass wouldn't be an application command, it would just be a janky normal command.
There was a problem hiding this comment.
No they shouldn't? The subclass wouldn't be an application command, it would just be a janky normal command.
ApplicationCommand isn't a base class, so using it would mean having to do some jank stuff to override functions and everything. It would be much easier for the user to just sub class Command which is a base class
There was a problem hiding this comment.
From a logical perspective, it doesn't make sense. Whenever a user makes their own "custom application command" class, it's not going to even have the ApplicationCommand class as a parent. That means that the "custom application command" class isn't a custom application command class, it's a custom command class.
There was a problem hiding this comment.
It doesn't need to be a custom application command, these just sync up commands which act like them
| self.user.id, command.guild_id, True | ||
| ) | ||
|
|
||
| for app_cmd in guild_commands: |
There was a problem hiding this comment.
I'm confused about this logic. Why is it looping through every application command found in a guild and performing operations on those application commands? Aren't we supposed to focus on the one application command this function was provided?
There was a problem hiding this comment.
this mainly: checks if a command named like that already exists in this guild, and if so edits instead of creating, and deletes commands which the bot shouldn't have.
There was a problem hiding this comment.
It would be nice if the user could disable the behavior of deleting unknown application commands. Otherwise it would ruin bot setups where application commands are split between multiple different instances of the bot.
There was a problem hiding this comment.
It would be nice if the user could disable the behavior of deleting unknown application commands. Otherwise it would ruin bot setups where application commands are split between multiple different instances of the bot.
How? v3 supports that, and in any world this instance going up means that it's new and probably needs app commands globally refreshed.
There was a problem hiding this comment.
It would be nice if the user could disable the behavior of deleting unknown application commands. Otherwise it would ruin bot setups where application commands are split between multiple different instances of the bot.
How? v3 supports that, and in any world this instance going up means that it's new and probably needs app commands globally refreshed.
Have you heard of booleans, the client, and the state? The client's initializer should have a parameter that can enable or disable deleting unknown application commands. Then pass that to the state and you're done. I don't see what the problem is?
There was a problem hiding this comment.
No I wasn't asking how to do it, I was asking how it would ruin instance-based bot setups
Co-authored-by: baronkobama <baronkobama@gmail.com> Signed-off-by: VincentRPS <vincentbusiness55@gmail.com>
Co-authored-by: baronkobama <baronkobama@gmail.com> Signed-off-by: VincentRPS <vincentbusiness55@gmail.com>
Co-authored-by: Emre Terzioglu <50607143+EmreTech@users.noreply.github.com> Signed-off-by: VincentRPS <vincentbusiness55@gmail.com>
| return | ||
|
|
||
| if self._created is True: | ||
| await self.http.delete_global_application_command( |
There was a problem hiding this comment.
I'm confused. Why does this delete the global command and not recreate it? After calling this HTTP function, the method just returns.
| else: | ||
| return | ||
|
|
||
| if not created: |
There was a problem hiding this comment.
When is created ever False anyways? All of the pathways the code takes where created is still False all return prematurely without this code being ran.
| ) | ||
|
|
||
| for app_cmd in guild_commands: | ||
| if app_cmd['name'] not in self._application_command_names: |
There was a problem hiding this comment.
The attribute _application_command_names doesn't exist.
Co-authored-by: Emre Terzioglu <50607143+EmreTech@users.noreply.github.com> Signed-off-by: VincentRPS <vincentbusiness55@gmail.com>
Co-authored-by: Emre Terzioglu <50607143+EmreTech@users.noreply.github.com> Signed-off-by: VincentRPS <vincentbusiness55@gmail.com>
|
Please add a changelog entry |
Summary
This PR refactors and moves the previous logic of
ApplicationCommand.instantiatetostate.sync_commandto make command syncing and parsing easier.I have not tested this yet.
Checklist