Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

a joke slash command #21

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

Closed
kfahn22 wants to merge 0 commits into CodingTrain:main from kfahn22:joke
Closed

a joke slash command #21

kfahn22 wants to merge 0 commits into CodingTrain:main from kfahn22:joke

Conversation

@kfahn22
Copy link
Contributor

@kfahn22 kfahn22 commented Oct 24, 2023

Oops, I didn't realize my auto-formatter apparently is set to use "" instead of ''

Copy link
Member

This is a great demo for quiz-like interactions which wait for the user response. (Though, the repo is being reworked for supplementing the videos)

Copy link
Collaborator

Have you tested this code? Some of the things in there don't look like they're going to do what you intended them to do 😅

Copy link
Contributor Author

kfahn22 commented Oct 24, 2023

I have tested it in my server and it seems to be doing what I intended. The one issue I am having is that the bot.js file asks about feelings. What are you concerned about?

);

const filter = (response) => {
return jokes[`${i}`].punchline.some(
Copy link
Collaborator

@supercrafter100 supercrafter100 Oct 24, 2023

Choose a reason for hiding this comment

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

You don't need to put i in a string! You can just do jokes[i], behind the scenes it will automatically convert it back to a number anyways as the index is a number here! So you're going from number -> string -> back to number

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I have updated that.

);
};

const answer = jokes[`${i}`].punchline;
Copy link
Collaborator

@supercrafter100 supercrafter100 Oct 24, 2023

Choose a reason for hiding this comment

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

Same as above

Comment on lines 41 to 62
await interaction
.reply({
content: jokes[`${i}`].joke,
fetchReply: true,
})
.then(() => {
interaction.channel
.awaitMessages({
filter,
max: 1,
time: 30000, // Allow user response for 30 seconds
errors: ["time"],
})
.then((collected) => {
interaction.followUp(`${collected.first().author} you got it right!`);
})
.catch((collected) => {
interaction.followUp(
`Sorry, that isn't right. The punchline is ${answer}`
);
});
});
Copy link
Collaborator

@supercrafter100 supercrafter100 Oct 24, 2023

Choose a reason for hiding this comment

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

Suggested change
await interaction
.reply({
content: jokes[`${i}`].joke,
fetchReply: true,
})
.then(() => {
interaction.channel
.awaitMessages({
filter,
max: 1,
time: 30000, // Allow user response for 30 seconds
errors: ["time"],
})
.then((collected) => {
interaction.followUp(`${collected.first().author} you got it right!`);
})
.catch((collected) => {
interaction.followUp(
`Sorry, that isn't right. The punchline is ${answer}`
);
});
});
await interaction
.reply({
content: jokes[i].joke,
fetchReply: true,
})
const collected = await interaction.channel
.awaitMessages({
filter,
max: 1,
time: 30000, // Allow user response for 30 seconds
errors: ["time"],
}).catch(() => interaction.followUp(
`Sorry, that isn't right. The punchline is ${answer}`
));
// If nothing got collected so it probably timed out and got handled by the catch above, do nothing...
if (!collected) return;
// They got it right!
await interaction.followUp(`${collected.first().author} you got it right!`);

Copy link
Collaborator

@supercrafter100 supercrafter100 Oct 24, 2023

Choose a reason for hiding this comment

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

You don't need all the .then() when we're working with async/await! Generally .then() is used when we don't have the luxury of an async function but we totally do in this case! My formatting might be off here as I used github it's internal editor to do this so you might want to perform those changes yourself. Ialso modified the reply code to address what I mentioned in my earlier review comments about the i variable being allowed in a string!

Comment on lines 18 to 32
const row = new ActionRowBuilder().addComponents(
new StringSelectMenuBuilder()
.setCustomId("random-joke")
.setPlaceholder("Can you answer the joke?")
.setMinValues(1)
.setMaxValues(1)
.addOptions(
new StringSelectMenuOptionBuilder()
.setLabel(`jokes[${i}]`)
.setDescription(`Joke ${i}`)
.setValue(
`jokes[${i}].punchline.some(answer => answer.toLowerCase() === response.content.toLowerCase())`
)
)
);
Copy link
Collaborator

@supercrafter100 supercrafter100 Oct 24, 2023

Choose a reason for hiding this comment

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

Is this code used anywhere? I'm either blind or it's just here without any actual reason?

Copy link
Collaborator

I have tested it in my server and it seems to be doing what I intended. The one issue I am having is that the bot.js file asks about feelings. What are you concerned about?

That's correct! In the current version of the repository the bot.js contains some additional code Dan tinkered with which did that when you mentioned the bot.

Copy link
Member

Hi @kfahn22 this is great! I was about to merge #20. The idea here is that this repo would have the code to match the videos precisely. Other bots or examples would come in future videos and then could be added to the repo. Also, we can create a section in the README to link to other examples or "passenger showcase" bots (though these would ultimately be on the website as well?).

How about linking to your own repo from the README here for now?

Copy link
Contributor Author

kfahn22 commented Oct 24, 2023

I really just wanted to share the code. I realized that you wanted a clean repo with just basics. I am not sure it makes sense to put a link in the README? Should I close pull request and send you a link instead just in case you want to use i?

Copy link
Member

I think this (closed) pr with Supercrafter's comments can be used as a reference later in case Dan wants to refer to it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@supercrafter100 supercrafter100 supercrafter100 left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /