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

[Mutes] Duration should take the first value provided #6274

Closed
EternalllZM opened this issue Dec 2, 2023 · 2 comments · Fixed by #6349
Closed

[Mutes] Duration should take the first value provided #6274

EternalllZM opened this issue Dec 2, 2023 · 2 comments · Fixed by #6349
Labels
Category: Cogs - Mutes This is related to the Mutes cog. Status: PRs Welcome No one is currently assigned to this issue, but we'd be grateful if anyone made a PR. Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing.

Comments

@EternalllZM
Copy link

What Red version are you using?

3.5.5

Cog name

Mutes

Command name

mute

What did you expect to happen?

[p]mute <user_id> 1h spamming 3 copypastas within 2 hours.

The first time mentioned is 1h, which means the intended mute time is 1 hour.

What actually happened?

Our example: [p]mute <user_id> 1h spamming 3 copypastas within 2 hours.

If you mention a time, even after one has been specified, it will use the last time mentioned. In this case, 2 hours is used as the duration instead of the intended 1h.

Per mutes help screen, the following examples are provided:

[p]mute @member1 @member2 spam 5 hours
[p]mute @member1 3 days

I would argue that the first mention of a time should be used as the base for the command's duration. In a reason you might need to explain that a certain amount of bad behavior took place within a time duration.

For further explanation, the mute help provides this:
[p]mute <users...> [time_and_reason]

Being picky, time is listed before reason. Therefore, the line of thinking is that you provide a time before the reason, even if it does not matter when reviewing listed examples.

How can we reproduce this error?

  1. Load mutes cog.
  2. Execute a mute command as such: [p]mute <user_id> 1h spamming 3 copypastas within 2 hours
  3. Mute will set duration as 2 hours instead of the 1h specified first.

Anything else?

This would not effect the ability to format mutes in the second example, except it would reverse the behavior (second time is now overwritten by first time mentioned).
[p]mute @member1 @member2 spam 5 hours

If no other time is passed before the 5 hours in the command, this would function just the same. Mostly this bug report is looking to change the behavior to accept the first mention of a time, rather than the last as it would make more sense.

@EternalllZM EternalllZM added the Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing. label Dec 2, 2023
@github-actions github-actions bot added the Status: Needs Triage This has not been labeled or discussed for handling yet. label Dec 2, 2023
@Flame442
Copy link
Member

Flame442 commented Dec 2, 2023

It's expected behavior that the time can be matched anywhere in the string, however it might not be expected behavior to use the last time in the string. Currently, it loops over all the matches front to back, and overrides the assumed time with each valid match, meaning the last time entered will be chosen. It might make sense to break on valid match, to ensure the first time entered is the one that is used.

for time in TIME_RE.finditer(maybe_time):
argument = argument.replace(time[0], "")
for k, v in time.groupdict().items():
if v:
time_data[k] = int(v)

@Flame442 Flame442 added the Category: Cogs - Mutes This is related to the Mutes cog. label Dec 2, 2023
@Flame442 Flame442 added Status: PRs Welcome No one is currently assigned to this issue, but we'd be grateful if anyone made a PR. and removed Status: Needs Triage This has not been labeled or discussed for handling yet. labels Jan 2, 2024
@Flame442 Flame442 added this to the 3.5.x milestone Jan 2, 2024
@Flame442 Flame442 modified the milestones: 3.5.x, 3.5.9 Apr 5, 2024
@Flame442
Copy link
Member

Flame442 commented Apr 5, 2024

Additionally, it seems like the regex may be overly ambitious in matching the single letter versions of times in some cases, such as 5 warnings being interpreted as 5 w.

Repro:

from redbot.cogs.mutes.converters import MuteTime

return await MuteTime().convert(ctx, "7 days Rule Violation - User has accumulated 5 warnings related to misuse of channels")
{'duration': datetime.timedelta(days=42), 'reason': 'Rule Violation - User has accumulated arnings related to misuse of channels'}

@Jackenmen Jackenmen removed this from the 3.5.9 milestone Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Cogs - Mutes This is related to the Mutes cog. Status: PRs Welcome No one is currently assigned to this issue, but we'd be grateful if anyone made a PR. Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants