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

Make /coin check default to the caller #282

Merged
merged 6 commits into from
Aug 15, 2022
Merged

Conversation

hydrobeam
Copy link
Contributor

@hydrobeam hydrobeam commented Aug 13, 2022

Related Issues

N/A

Summary of Changes

In comparison to .coin, doing /coin check and having to look for the target user was pretty time consuming when somebody just wanted to check their own balance. This PR makes the user argument optional and defaults to checking the balance of the user who called the command if the arg is not provided.

  • Balance not present + coin check optional

image

  • .coin default behaviour checks out + message changes depending on how user is called

image

Steps to Reproduce

/coin check and pass in no arguments.
OR
.coin check and pass in no arguments.

Extra commentary

Not sure if messageFromUser.member?.user is the "right" way to get the user, since for some reason accessing member can return null from a Message?🤷

`/coin check` originally had a mandatory argument of a target user to
check the balance of. Now the argument defaults to the user who called
the command, making it more ergonomic to use.
@mcpenguin
Copy link
Collaborator

Also please leave screenshots :))

hydrobeam and others added 3 commits August 14, 2022 01:46
- Also generalizes getUserIDFromMessage to just get the User straight up
    - Updated previou references where relevant (leaderboard)
- Replaces "`user` has" with `You have` if the user is calling `/coin check`
on their own
@hydrobeam
Copy link
Contributor Author

hydrobeam commented Aug 14, 2022

Decided to remove the balance subcommand since having /coin check default to self makes it redundant. To maintain appearances, I added a check to see if the caller is running the method without arguments and changes the initial message to You have accordingly. Also reworked getUserIDFromMessage to be more general.

(added screenshots too)

Copy link
Collaborator

@mcpenguin mcpenguin left a comment

Choose a reason for hiding this comment

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

small changes

src/commandDetails/coin/check.ts Show resolved Hide resolved
- `'b', 'bal', 'balance'` due to the removal of the balance subcommand
Copy link
Collaborator

@mcpenguin mcpenguin left a comment

Choose a reason for hiding this comment

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

LGTM! Feel free to merge after you fix the linter.

@hydrobeam
Copy link
Contributor Author

LGTM! Feel free to merge after you fix the linter.

Fixed the linter, but don't have write access 🙈

@mcpenguin mcpenguin merged commit 570950c into uwcsc:master Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants