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

Context Menus not being found #48

Closed
3 tasks done
Matyrobbrt opened this issue Jan 29, 2022 · 3 comments · Fixed by #49
Closed
3 tasks done

Context Menus not being found #48

Matyrobbrt opened this issue Jan 29, 2022 · 3 comments · Fixed by #49
Labels
bug Something isn't working
Milestone

Comments

@Matyrobbrt
Copy link

Matyrobbrt commented Jan 29, 2022

Issue Checklist

  • I have checked for similar issues on the Issue tracker
  • I have updated to the latest version of JDA-Chewtils.
  • I have checked the branches or the maintainer's PRs for upcoming bug fixes.

Affected Modules

Command

Description

Context menus can contain uppercase characters in their name, and when they are registered (using CommandClient#addContextMenu) in contextMenuIndex from CommandClientImpl the index is associated with the menu name, exactly as it is in the field (uppercased). However CommandClientImpl#onMessageContextMenu tries to get the index associated with the command name, lowercased, and such the command cannot be found if it contains uppercase characters.

Possible ways of fixing

  • Get rid of the toLowercase call when the command is searched. This fix is preferred, as there could be 2 menus with the same name, but different cases
  • Make addContextMenu lowercase the command name when inserting it in the map.
@Chew
Copy link
Owner

Chew commented Jan 29, 2022

I'm not sure how Discord handles case sensitivity. If it allows "rory" and "Rory" as valid names then we should do the former. Should probably get rid of the checks altogether since you could have a message and user context menu with the same name and be fine.

@Chew Chew added the bug Something isn't working label Jan 29, 2022
@Chew Chew added this to the v2 milestone Jan 29, 2022
@Matyrobbrt
Copy link
Author

From my testing, it seems to allow 2 different commands with different letter casings

@Chew
Copy link
Owner

Chew commented Jan 29, 2022

Indeed, this also caused issues when a User and Message context menu had the same name.

@Chew Chew closed this as completed in #49 Jan 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants