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

Turn AoC commands into hybrid commands #101

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

n0Oo0Oo0b
Copy link

@n0Oo0Oo0b n0Oo0Oo0b commented Nov 8, 2023

Closes #97

image

I feel like duplicating the code isn't the best way to go about this, but I couldn't really think of anything else. Any suggestions to avoid it are welcome.

Copy link
Contributor

@Robin5605 Robin5605 left a comment

Choose a reason for hiding this comment

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

Making this into a hybrid command should cut down on code duplication. I believe Kat wanted them to be hybrid anyway, so that works out nicely. Do you mind condensing the slash command and text command into a hybrid command?

@n0Oo0Oo0b
Copy link
Author

n0Oo0Oo0b commented Nov 9, 2023

You can't turn individual commandss into hybrid commands afaict, but using a hybrid group should turn all the commands into hybrid commands, which should work anyway.
I was initially avoiding doing this since I didn't know you could send ephemeral messages with them. But the ephemeral kwarg in ctx.send can do that, so I think this should be the best way to go.

@n0Oo0Oo0b n0Oo0Oo0b changed the title Create slash command for &aoc countdown Turn AoC commands into hybrid commands Nov 9, 2023
@n0Oo0Oo0b
Copy link
Author

The latest commits should turn all the AoC commands into hybrid commands. Everything looks ok from my testing other than the channel restrictions, which don't work for text commands on my bot either, so it seems like something to do with my environment.

@n0Oo0Oo0b n0Oo0Oo0b requested a review from Robin5605 November 9, 2023 12:40
Copy link
Collaborator

@janine9vn janine9vn left a comment

Choose a reason for hiding this comment

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

So an issue with the current approach, is that &aoc link still works as a regular command and is not ephemeral only. I would like to convert this completely over to a slash-only is possible. If it's not the most feasible then that's fine.

Additionally, with the hybrid group, I would prefer the prefix to be /aoc instead of the wordy /adventofcode

For the channel restrictions, that's not working because we're missing code that wraps the extensions with the whitelist command.
Sir-Lancebot runs very restricted and each command is restricted and then given an channel overrides: https://github.com/python-discord/sir-lancebot/blob/main/bot/__main__.py#L76-L80

I'm okay with taking a similar approach and stealing that functionality/code-snippet from Sir-Lancebot.
You will run into an issue with a missing code jam constant if you do run it, so feel free to create a new constant for the Code Jam categories:

class Categories(NamedTuple):
    codejam_categories_name = "CODE JAM"

We will have to adapt the output when a command is run doesn't pass the role check. Currently this is the output:
image

@Robin5605
Copy link
Contributor

Pushed commit 619a1db that makes link/unlink commands slash-command only as requested by Kat.

Would like some feedback here on the technical side - I've used the app_command property of discord.ext.commands.HybridGroup, in order to make a slash-only command under the existing hybrid group.
However, the issue here is that this app_command property is not publicly documented, and seems to be an "internal" attribute (I'm not 100% sure it's not prefixed with _ as most of discord.py's other private attributes are):
https://github.com/Rapptz/discord.py/blob/536cf5c01fa4237306eadb29fc32abd65b485205/discord/ext/commands/hybrid.py#L656-L663:

            self.app_command = app_commands.Group(
                name=self._locale_name or self.name,
                description=self._locale_description or self.description or self.short_doc or '…',
                guild_ids=guild_ids,
                guild_only=guild_only,
                default_permissions=default_permissions,
                nsfw=nsfw,
            )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AoC: Convert commands to hybrid slash commands (ephemeral where reasonable)
3 participants