KOOKIIEStudios / PalCON-Discord

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

Revert "Optimization: Reduce Unnecessary Command Tree Sync Requests" #8

Closed Bratah123 closed 7 months ago

Bratah123 commented 7 months ago

Reverts KOOKIIEStudios/PalCON-Discord#5

Currently, with this code change, the bot never syncs in the first place, to allow users to use /sync

Consider another solution.

@alexraskin

KOOKIIEStudios commented 7 months ago

good catch ._. if i had to come up with one, it'll be to have an initial sync flag set either in the config toml or some other env file, which gets set on first run (i.e. syncs on-ready for first run only). seems cumbersome and over-engineered tho? mayhaps someone might have a more elegant solution

Bratah123 commented 7 months ago

good catch ._. if i had to come up with one, it'll be to have an initial sync flag set either in the config toml or some other env file, which gets set on first run (i.e. syncs on-ready for first run only). seems cumbersome and over-engineered tho? mayhaps someone might have a more elegant solution

I was thinking of making the sync command not a slash command and some type of "!" command, probably the best re-solution if we are sticking with sync on invoc

alexraskin commented 7 months ago

@Bratah123 lol thank you. I think the best solution is either making it a non slash command or passing a flag on bot start up. Personally making a sync command is more convenient but I'm up for anything. Let me know what you all think.

Bratah123 commented 7 months ago

@Bratah123 lol thank you. I think the best solution is either making it a non slash command or passing a flag on bot start up. Personally making a sync command is more convenient but I'm up for anything. Let me know what you all think.

I'm all for making it a non-slash command, if you're up for it

alexraskin commented 7 months ago

@Bratah123 lol thank you. I think the best solution is either making it a non slash command or passing a flag on bot start up. Personally making a sync command is more convenient but I'm up for anything. Let me know what you all think.

I'm all for making it a non-slash command, if you're up for it

Here is what I have come up with:

async def on_message(self, message):
    if message.author == self.user: # make sure its not the bot. 
        return
    if message.content.startswith("!sync"):
        await tree.sync()
        await message.channel.send("Syncing...")
    else:
        return

This is the simplest way because we are using discord.Client. The only thing I would need to add is a way to check if it's the owner syncing the commands. Which I could add a way to check using the config file and the users discord ID. Let me know your thoughts? @Bratah123 @KOOKIIEStudios

Bratah123 commented 7 months ago

Hmm, seems like discord.Client bots are only allowed application commands, so this is the only way.

I would just make the sync command be usable by all admins in the server, as we are still trying to be PnP with this discord bot. Having less configurations to fill out add more layers to set up.

I would remove the redundant else statement and add a more descriptive result message.

Something like this is fine with me:

async def on_message(self, message):
    if message.author == self.user: # make sure its not the bot. 
        return
    if message.content.startswith("!sync") and CHECK_ADMIN:
        await tree.sync()
        await message.channel.send("Application commands for this Discord Bot has been updated!")