Skip to content

Add a 2 minute cooldown to the topic command#880

Merged
Xithrius merged 2 commits into
mainfrom
topic-command-cooldown
Oct 7, 2021
Merged

Add a 2 minute cooldown to the topic command#880
Xithrius merged 2 commits into
mainfrom
topic-command-cooldown

Conversation

@ChrisLovering
Copy link
Copy Markdown
Member

Relevant Issues

Closes #868

Description

Using the command while it's on cooldown will hit the error handler, which sends an error message showing how long is left on the cooldown, which is deleted after 7.5 seconds.
image

Did you:

@Akarys42
Copy link
Copy Markdown
Contributor

I'm not a fan of this solution. I think there are relevant reasons to use two topic commands in a five minutes span. I'd propose one of the following:

  • Reduce to one minute
  • Add a re-roll reaction that can be only used by the command runner

Comment thread bot/exts/utilities/conversationstarters.py Outdated
Copy link
Copy Markdown
Contributor

@Bluenix2 Bluenix2 left a comment

Choose a reason for hiding this comment

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

I don't really have much else to comment, looks good otherwise.

Allows the refresh of a topic by pressing an emoji.
"""
message = await ctx.send(embed=self._build_topic_embed(ctx.channel.id))
self.bot.loop.create_task(self._listen_for_refresh(message))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't the command itself a task?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

My reasoning for adding this as a task onto the loop, rather than awaiting it directly was so that the refresh loop isn't considered as part of the command invocation directly, this mostly matters for things like timing how long a command takes to complete,

Comment thread bot/exts/utilities/conversationstarters.py Outdated
@ChrisLovering ChrisLovering force-pushed the topic-command-cooldown branch from d3ee3df to 1bd844f Compare October 3, 2021 16:16
Copy link
Copy Markdown
Contributor

@Bluenix2 Bluenix2 left a comment

Choose a reason for hiding this comment

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

I would perhaps advocate for a 3 minute cooldown. The cooldown still does its job but we limit the "false positives" (as in, times where the cooldown prohibits usage we want to support).

Either way, it works!

@Xithrius Xithrius added area: backend Related to internal functionality and utilities category: utilities Related to utilities status: needs review Author is waiting for someone to review and approve type: enhancement Changes or improvements to existing features labels Oct 7, 2021
ChrisLovering and others added 2 commits October 7, 2021 20:31
Using the command while it's on cooldown will hit the error handler, which sends an error message showing how long is left on the cooldown, which is deleted after 7.5 seconds.
This is done via an emoji as buttons are too big

Co-authored-by: Bluenix <bluenixdev@gmail.com>
@ChrisLovering ChrisLovering force-pushed the topic-command-cooldown branch from 1bd844f to 515c639 Compare October 7, 2021 19:33
@ChrisLovering ChrisLovering changed the title Add a 5 minute cooldown to the topic command Add a 2 minute cooldown to the topic command Oct 7, 2021
Copy link
Copy Markdown
Contributor

@Xithrius Xithrius left a comment

Choose a reason for hiding this comment

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

Y e s.

@Xithrius Xithrius merged commit e8e6067 into main Oct 7, 2021
@Xithrius Xithrius deleted the topic-command-cooldown branch October 7, 2021 19:34
@TizzySaurus TizzySaurus mentioned this pull request Oct 8, 2021
4 tasks
@Xithrius Xithrius removed the status: needs review Author is waiting for someone to review and approve label Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: backend Related to internal functionality and utilities category: utilities Related to utilities type: enhancement Changes or improvements to existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a per-channel cooldown for .topic

5 participants