search

KOOKIIEStudios / PalCON-Discord

A Python-based Discord bot for PalWorld server administration via RCON
MIT License
18 stars 3 forks source link

Optimization: Reduce Unnecessary Command Tree Sync Requests #5

Closed alexraskin closed 7 months ago

alexraskin commented 7 months ago

Invocation-Based Syncing: The bot now syncs its command tree with Discord exclusively when the sync command is invoked, aligning with best practices guidance.

The on_ready function is triggered each time the bot connects or reconnects to Discord. If command syncing is placed within this function, it might lead to duplicate syncs, especially in cases of network instability or bot reconnections. This can result in redundant API calls and potential rate limit issues. Excessive command syncing can waste limited API resources and might even temporarily disable bot functionality if rate limits are exceeded. It also risks inconsistency in command availability if syncs are inadvertently triggered multiple times.

TL;DR: Updated command syncing to trigger only on command invocation, optimizing API usage and enhancing user experience.

Also this

KOOKIIEStudios commented 7 months ago

Invocation-Based Syncing: The bot now syncs its command tree with Discord exclusively when the sync command is invoked, aligning with best practices guidance.

Hi, I can't help but notice that your best practices link is to a discord channel - those who aren't in the channel can't view it. Would appreciate if you could link us publicly viewable sources on this! Thanks!

alexraskin commented 7 months ago

Hey @KOOKIIEStudios sorry for the confusion, this is a post made from the dev of discord.py

Here is a screenshot of the post.

Image from Gyazo

I don't have a link or public page. That discord link is the public discord.py discord server.

Again, sorry for the confusion.

KOOKIIEStudios commented 7 months ago

That's great! Would appreciate if Danny put things like this into his official docs next time...

This PR looks good to me, but if we're switching to invoc-based syncing, the repo docs will need to be updated to include that. I'll merge it for now, and think about how to represent this in the docs after.

alexraskin commented 7 months ago

Yeah, I agree, strange it's not included.

I can work on updating the docs tomorrow.

KOOKIIEStudios commented 7 months ago

I've updated a section about updating code in the README - not sure if it helps

Thanks for the contribution!