Skip to content
This repository has been archived by the owner on Nov 15, 2021. It is now read-only.

Speedrank #33

Merged
merged 5 commits into from
Nov 28, 2018
Merged

Speedrank #33

merged 5 commits into from
Nov 28, 2018

Conversation

drgoonic
Copy link
Contributor

Added the "rank" command to allow speedrunners to pull their specific stats in the general categories.

The new command has the form:
rank
playername = name in the speedrun sheet
category = 3star, 4star, 5star, or Torment (defaults to 4star)
type = overall or no-csb (defaults to overall)

Added the "rank" command to allow speedrunners to pull their specific stats in the general categories.

The new command has the form:
rank <playername> <category> <type>
playername = name in the speedrun sheet
category = 3star, 4star, 5star, or Torment (defaults to 4star)
type = overall or no-csb (defaults to overall)
@drgoonic
Copy link
Contributor Author

Oh I forgot to mention this is intended to resolve #29

ffrkbot.js Outdated
@@ -18,7 +18,7 @@ const client = new Commando.Client({

client.on('ready', () => {
console.log(`Logged in as ${client.user.username}!`);
client.user.setGame(',help');
Copy link
Owner

Choose a reason for hiding this comment

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

This change isn't related and should be in a separate pull request.

}
switch (category) {
case '3star':
category = 'GL 3* '+version+ ' rankings';
Copy link
Owner

Choose a reason for hiding this comment

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

For consistency's sake, I'd like for you to use Javascript's template literals to match the rest of the documentation for anything here. For example, instead of:

category = 'GL 3* '+version+ ' rankings';

Try:

`category = GL 3* ${version} rankings;`

Removed some extraneous changes from commit

Brought rank command in line with template literal usage
Copy link
Owner

@brasstax brasstax left a comment

Choose a reason for hiding this comment

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

I've made some comments with change requests.

As a final note, you don't have to do this for this particular pull request, but you'll notice that there's a lot of repeated code right now in this and in speedrun-utils.js. If you want to be really fly, make a separate pull request in which you combine common functions into one file, then export those functions so you can import them elsewhere. That way, you aren't duplicating functions across files and you don't have to maintain two versions of the same function.

commands/misc/speedrank.js Outdated Show resolved Hide resolved
utilities/speedrank-utils.js Outdated Show resolved Hide resolved
1) Cleaned up rank command examples

2) The spreadsheet formatting of the playername will now be used in the table output
@brasstax brasstax merged commit 654f959 into brasstax:master Nov 28, 2018
drgoonic added a commit to drgoonic/ffrkbot that referenced this pull request Nov 29, 2018
drgoonic added a commit to drgoonic/ffrkbot that referenced this pull request Nov 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants