GeorgeCiesinski / poke-guesser-bot

Discord bot that lets you guess pokemon
MIT License
0 stars 3 forks source link

/mod score set... is unintuitive #109

Closed GeorgeCiesinski closed 11 months ago

GeorgeCiesinski commented 1 year ago

Describe the bug

/mod score set requires you to type score after providing user mention. For example, you have to type:

/mod score set @mention score (type in number)

This is unusual because score is already selected as an option once, and then you have to select it again. I know this isn't a bug, but it results in the command failing if the user doesn't type it in correctly, so we should try to make it as intuitive to use as possible.

Expected behavior

If possible, the commands should be:

/mod score set @mention (type in number)

Or alternatively:

/mod score @mention set: (number)

Wissididom commented 1 year ago

That is basically how Discord slash commands work when you have an optional option. You have to choose that you want to specify a number by either typing score or selecting it from the menu. Actually, currently it is /mod score set user:@mention score:<number>, just Discord already opens the parameter for user because it is required. I think the easiest way to solve it would be to just make the number required too. Another way I see is creating the /mod score command and have an action option with preconfigured choices. For that way, I see something like /mod score user:@mention action:<add|set|remove> score/set:<number> being possible. And a completly different implementation I see is having a /mod score or just /mod command that sends a message with buttons and popups to configure it.

Wissididom commented 1 year ago

Oh, another idea I just have on how it might be possible to improve it is to make the command like /mod score user:@mentionOrId add:True/False value:(number that may be negative) (would imply setting it to 0 with add:False to remove it from the leaderboard, but currently it just writes a 0 in the DB) or maybe a command like /mod score args:add/set/remove @mentionOrId (number)

Those 2 comments being made I think it is a matter of decision on how we want to have it. Also I think the first step would be to decide if we want to make the score parameter required. After that there is a decision to make how we want to have the command. Here are the summerized options I see from my post 3 weeks ago and today:

We can discuss it further either here or in Discord, if you want.

Wissididom commented 11 months ago

I just have another idea on how the command could look like additionally to my last post: /mod score user:@mentionOrId score:(number as string. If it starts with '-' it'll subtract, if it starts with '+' it'll add, if you omit the '+' or '-' it'll set) Examples:

One disadvantage I see here is, that we cannot assume anymore that it is a number but needs to be checked manually before converting to number. Currently, the score option of the slash command is configured to be an integer. But that shouldn't really be difficult. We probably just need to trim whitespaces, check startsWith and after that we should be able to use Number.isInteger and fail if it returns false and parseInt the number and do what the command is supposed to do, if it returns true.

GeorgeCiesinski commented 11 months ago

I modified one of your suggestions which I liked the most:

/mod score user:@mention action:<add|set|remove> amount:<number>

This follows the most logical progression:

Mod > score > user > function (+/-/set) > amount

The benefit here is we don't reuse score which could cause confusion. I also like that the function and amount are beside each other because George + 5 makes more sense in a glance than + George 5. So from a flow perspective I think this is ideal.

I also am not sure if discord allows for integer inputs with an automatic check, but if it does than it could save us from building our own logic which is a minor convenience as well.

Wissididom commented 11 months ago

It does support integer options, yes. The amount option (that's currently called score) is an integer option. That was just a thought if we wanted to put the Action in the same field as the amount. I'll look into modifying the code in to the version you prefer later, probably today.