-
Notifications
You must be signed in to change notification settings - Fork 18
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
V14 #473
V14 #473
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a large diff but the logic is fairly repetitive and easy to understand. Let me know if you have questions!
@@ -19,47 +19,48 @@ | |||
"author": "", | |||
"license": "ISC", | |||
"dependencies": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update sapphire
"@types/lodash": "^4.14.168", | ||
"@types/node": "^15.0.1", | ||
"@types/sqlite3": "^3.1.7", | ||
"@tsconfig/node14": "^1.0.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yarn update pulled these in. I confirmed they are stable
'GUILD_MESSAGES', | ||
'GUILD_MESSAGE_REACTIONS', | ||
'DIRECT_MESSAGES', | ||
'DIRECT_MESSAGE_REACTIONS', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We must use GatewayIntentBits instead of strings
Partials.Channel, | ||
Partials.GuildMember, | ||
Partials.Message, | ||
Partials.Reaction, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We must use these Partials
MENTIONABLE = 'mentionable', | ||
NUMBER = 'number', | ||
ATTACHMENT = 'attachment', | ||
STRING = ApplicationCommandOptionType.String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use ApplicationCommandOptionType
@@ -46,10 +44,11 @@ export class ConnectFourHandler extends InteractionHandler { | |||
if ( | |||
interaction.user.id !== connectFourGameTracker.getGameFromId(result.gameId)!.state.player1Id | |||
) { | |||
return await interaction.reply({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function definition returns null
gameId: number; | ||
sign: RpsGameSign; | ||
}> { | ||
public override parse(interaction: ButtonInteraction): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Sapphirev4, Maybe no longer exists. We use Option.None and Option.Some instead.
transferId: string; | ||
sign: TransferSign; | ||
}> { | ||
public override parse(interaction: ButtonInteraction): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Sapphirev4, Maybe no longer exists. We use Option.None and Option.Some instead.
.setColor(DEFAULT_EMBED_COLOUR) | ||
.addField('User', `${user.tag} (${user.id})`) | ||
.addFields([{ name: 'User', value: `${user.tag} (${user.id})` }]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DiscordJS requires name and value keys for object
if (message.content) kickEmbed.addField('Content', `${message.content}`); | ||
if (reason) kickEmbed.addFields([{ name: 'Reason', value: reason }]); | ||
kickEmbed.addFields([ | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DiscordJS requires name and value keys for object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Testing of commands seems to work as expected. In the future we should add unit tests so that errors can be mitigated as well.
Summary of Changes
Updates Discord.js to v14.9 and Sapphire to corresponding latest versions
Motivation and Explanation
Using the latest Discord JS API will help people more easily contribute by being able to rely on the latest Discord JS and Sapphire docs. It will also be much easier to ask for help on those respective Discords now
Related Issues
I was so excited I forgot to make one
Steps to Reproduce
Run Codey and verify he's stable and happy
Demonstration of Changes
No functional changes
Further Information and Comments
Since this is a large commit I've commented on each change