Skip to content

Schema rework | Update to roll command | DB functions updated #16

Open
sjkd23 wants to merge 15 commits intomainfrom
rolling
Open

Schema rework | Update to roll command | DB functions updated #16
sjkd23 wants to merge 15 commits intomainfrom
rolling

Conversation

@sjkd23
Copy link
Copy Markdown
Collaborator

@sjkd23 sjkd23 commented Sep 11, 2023

Roll command updates:

  • Optional 'number' option for /roll, which allows you to roll for multiple Gachas at once.
    - When rolling multiple times, you can cycle through the Gachas you got using the 'Previous' and 'Next' buttons.
    - Only interaction user can use the buttons

  • 'Roll Again!' button which prompts you to choose a number of rolls
    - User must send a message containing ONLY a number

  • Each roll cost 10 coins/balance.

Database functions updated:

  • Used findOrCreate in a lot of places, instead of making it two seperate 'find' and 'create' functions.
  • Added documentation to new functions.
  • Function names are (hopefully) intuitive.

New Database Schema:

  • Added GACHA_URL_LIST array to the 'Gacha' database, instead of using the array itself in the code.

image

Copy link
Copy Markdown
Member

@ewang2002 ewang2002 left a comment

Choose a reason for hiding this comment

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

Few changes, but looks good!

Comment thread src/DBMain.ts
Comment thread src/DBMain.ts Outdated
Comment thread src/DBMain.ts Outdated
Comment thread src/DBMain.ts Outdated
Comment thread src/DBMain.ts Outdated
Comment thread src/index.ts Outdated
Comment thread src/utils.ts
Comment thread src/utils.ts
Comment thread src/utils.ts Outdated
Comment thread src/utils.ts Outdated
@sjkd23 sjkd23 requested a review from ewang2002 September 18, 2023 06:41
Comment thread src/utils.ts
* @param {number} max - the highest possible number
* @returns - returns the random number
*/
export async function rng(min: number, max: number): Promise<number> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This function does not need to be async

Comment thread src/DBUtils.ts
rarity = 'uncommon';
} else if (rndm > 90 && rndm <= 99) {
rarity = 'rare';
} else if (rndm >= 100) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
} else if (rndm >= 100) {
} else {

Comment thread src/DBUtils.ts
} else if (rndm >= 100) {
rarity = 'legendary';
}
const randomGacha = await getRandomGachaOfRarity(rarity!);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
const randomGacha = await getRandomGachaOfRarity(rarity!);
const randomGacha = await getRandomGachaOfRarity(rarity);

Comment thread src/commands/poll.ts
if (questionString.length > 256) {
interaction.reply(`Question is too long. (max 256 char.)`);
await interaction.reply(`Question is too long. (max 256 char.)`);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you mean to return here?

Comment thread src/commands/roll.ts

if (i.customId === BUTTONS.ROLL_AGAIN_ID) {
if (rollAgainInUse) {
await i.update({})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Try something like deferReply or deferUpdate

Also, be consistent w/ the semicolon usage

Comment thread src/commands/roll.ts
if (!isNaN(parsedNumber)) {
msgCollector.stop();
rollAgainInUse = false;
await Roll.run(interaction, parsedNumber);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Try not to recursively call the run function. You can do this in some other function, but not here.

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