Madlibs

[DMFriends]

Relevant Issues

Closes #847

Description

This PR is designed to add a fun Madlibs game to Sir Lancebot.

When the command, .madlibs, is run, the bot will ask the user to enter a word that fits a random part of speech (e.g. noun, adjective, verb, plural noun, etc.). The user will simply send a message with any word that fits the given part of speech.

The bot chooses a random number of user inputs to use for the game and a random story.

Once the user has finished entering all the input prompts, the bot should send a message with the completed story using the words that the user entered.

Did you:

• Join the Python Discord Community?
• Read all the comments in this template?
• Ensure there is an issue open, or link relevant discord discussions?
• Read and agree to the contributing guidelines?

[DMFriends]

Tysm for your review @TizzySaurus ! Your reviews are very thorough and it helps me a lot especially since this is my first time contributing to open-source!! I will implement your changes later today if I get the chance.

[Akarys42]

works as intended

[onerandomusername]

Should only allow a user to invoke the command once at a time.

Easy fix, decorate the command with this: https://discordpy.readthedocs.io/en/master/ext/commands/api.html#discord.ext.commands.max_concurrency

@commands.max_concurrency(1, per=commands.BucketType.user)

[onerandomusername]

It turns out that the bots error handler doesn't actually catch the error raised by a concurrency blocker.

To solve this, we can use local error handlers (method) (guide)

I've put an implementation together below, since some of this involves digging into our error handler implementation.

    @madlibs.error
    async def handle_madlibs_error(self, ctx: commands.Context, error: commands.CommandError) -> None:
        if isinstance(error, commands.MaxConcurrencyReached):
            await ctx.send("You are already playing madlibs!")
            error.handled = True

The reason we set error.handled to True is because in our error handler we use getattr(error, "handler", False) to see if we have already handled the error.

[DMFriends]

It turns out that the bots error handler doesn't actually catch the error raised by a concurrency blocker.

To solve this, we can use local error handlers (method) (guide)

I've put an implementation together below, since some of this involves digging into our error handler implementation.

    @madlibs.error
    async def handle_madlibs_error(self, ctx: commands.Context, error: commands.CommandError) -> None:
        if isinstance(error, commands.MaxConcurrencyReached):
            await ctx.send("You are already playing madlibs!")
            error.handled = True

The reason we set error.handled to True is because in our error handler we use getattr(error, "handler", False) to see if we have already handled the error.

So this would go in the error handler file in the repo, correct?

[DMFriends]

Should only allow a user to invoke the command once at a time.

Easy fix, decorate the command with this: https://discordpy.readthedocs.io/en/master/ext/commands/api.html#discord.ext.commands.max_concurrency

@commands.max_concurrency(1, per=commands.BucketType.user)

Would this go right after line 65?

[onerandomusername]

It turns out that the bots error handler doesn't actually catch the error raised by a concurrency blocker.
To solve this, we can use local error handlers (method) (guide)
I've put an implementation together below, since some of this involves digging into our error handler implementation.

    @madlibs.error
    async def handle_madlibs_error(self, ctx: commands.Context, error: commands.CommandError) -> None:
        if isinstance(error, commands.MaxConcurrencyReached):
            await ctx.send("You are already playing madlibs!")
            error.handled = True

The reason we set error.handled to True is because in our error handler we use getattr(error, "handler", False) to see if we have already handled the error.

So this would go in the error handler file in the repo, correct?

No, this would go in the madlibs file, since its local to the command.

[onerandomusername]

Should only allow a user to invoke the command once at a time.
Easy fix, decorate the command with this: https://discordpy.readthedocs.io/en/master/ext/commands/api.html#discord.ext.commands.max_concurrency

@commands.max_concurrency(1, per=commands.BucketType.user)

Would this go right after line 65?

Yeah.

[DMFriends]

It turns out that the bots error handler doesn't actually catch the error raised by a concurrency blocker.
To solve this, we can use local error handlers (method) (guide)
I've put an implementation together below, since some of this involves digging into our error handler implementation.

    @madlibs.error
    async def handle_madlibs_error(self, ctx: commands.Context, error: commands.CommandError) -> None:
        if isinstance(error, commands.MaxConcurrencyReached):
            await ctx.send("You are already playing madlibs!")
            error.handled = True

The reason we set error.handled to True is because in our error handler we use getattr(error, "handler", False) to see if we have already handled the error.

So this would go in the error handler file in the repo, correct?

No, this would go in the madlibs file, since its local to the command.

Ah, gotcha. So where exactly does this go? Right after the async def madlibs, correct?

[Shivansh-007]

Ah, gotcha. So where exactly does this go? Right after the async def madlibs, correct?

Yeah, it can go anywhere in the cog, it's just a special decorator like the on_command_error method but for a cog, which would be called whenever there is an error raised in the cog, also error.handled is set so the error handler doesn't handle the error again.

[DMFriends]

Ah, gotcha. So where exactly does this go? Right after the async def madlibs, correct?

Yeah, it can go anywhere in the cog, it's just a special decorator like the on_command_error method but for a cog, which would be called whenever there is an error raised in the cog, also error.handled is set so the error handler doesn't handle the error again.

Gotcha, thanks.

[onerandomusername]

lgtm :D

[onerandomusername]

Not changing my approval, but there is this error message on timeout which I don't fully think makes sense.

Additonally, I'm not sure about the random titles, as its somewhat unprompted.

Finally, I suggest mentioning the user with this error as its a timeout.

[DMFriends]

Not changing my approval, but there is this error message on timeout which I don't fully think makes sense.

Additonally, I'm not sure about the random titles, as its somewhat unprompted.

Finally, I suggest mentioning the user with this error as its a timeout.

As you mentioned on Discord, it does say "Please try again later" and yes I agree that that the user could try again right away. To be honest, I could probably change what the embed says as a whole. I'm going to think about this more. Also I think mentioning the user would be good, will add that.

About the random titles, that was something that Shom770 pointed out in a review so I added that. It sounds like this is a standard convention for similar projects so I decided to commit that suggestion. If you think I should not have it, I think it's fine either way.

[onerandomusername]

About the random titles, that was something that Shom770 pointed out in a review so I added that. It sounds like this is a standard convention for similar projects so I decided to commit that suggestion. If you think I should not have it, I think it's fine either way.

Well, we should keep them then, but I think that message should mention the user at minimum, and remove the please try again later :D

[DMFriends]

About the random titles, that was something that Shom770 pointed out in a review so I added that. It sounds like this is a standard convention for similar projects so I decided to commit that suggestion. If you think I should not have it, I think it's fine either way.

Well, we should keep them then, but I think that message should mention the user at minimum, and remove the please try again later :D

Yep, I already added that locally, will commit sometime soon :)
Everything else besides the "please try again later" is good, right? @onerandomusername

[onerandomusername]

Yep!

[Xithrius]

To whoever eventually hits the merge button: squash.