Skip to content

Ingame Verification with Discord Bot#16

Open
RexBluefox wants to merge 6 commits intonullsoepic:mainfrom
RexBluefox:Verification
Open

Ingame Verification with Discord Bot#16
RexBluefox wants to merge 6 commits intonullsoepic:mainfrom
RexBluefox:Verification

Conversation

@RexBluefox
Copy link

@RexBluefox RexBluefox commented Dec 17, 2022

This will resolve #9.
So its the Ingame Verification System.

I will add checking for completed challenges later

@RexBluefox
Copy link
Author

Found a typo. In the verify command i mistyped verify as yerify

Comment on lines +27 to +29



Copy link
Collaborator

Choose a reason for hiding this comment

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

There are a few unnecessary newlines here.

return createHash('sha256').update(string).digest('hex');
}

export function makeid(length) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The type isn't defined, from what I can tell it is meant to be a number.

@@ -0,0 +1,15 @@
const { createHash } = require('crypto');

export function hash(string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

No type here either

const type = interaction.options.getString("type") || "null"
const user = interaction.options.getString("user") || "null"
let users = fs.readFileSync(__dirname + "/../../Verification/doneVerification.json","utf-8");
var usersData = JSON.parse(users)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This variable isn't being reassigned maybe it could be a const

Copy link
Author

Choose a reason for hiding this comment

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

Is this necessary?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No

switch (type.toLowerCase()){
case "minecraft":{
let discord = getKeyByValue(usersData,user)
if (discord == undefined){
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be compacted to if(!discord) instead of if(discord == undefined) also there's no space in-between the if and {

Comment on lines +1 to +12
import { SlashCommandBuilder, EmbedBuilder, ChatInputCommandInteraction, User, Colors } from 'discord.js';
import { DiscordClient } from '../../Utils/DiscordClient';
import fs from 'fs';
function getKeyByValue(object, value) {
return Object.keys(object).find(key => object[key] === value);
}
export const data = new SlashCommandBuilder()
.setName('whois')
.setDescription('Lookup a user by their IGN or Discord Name')
.addStringOption((o) => o.setName(`type`).setDescription(`Lookup by minecraft or discord`).setRequired(true).addChoices({name:"Minecraft",value:"minecraft"},{name:"Discord",value:"discord"}))
.addStringOption((o) => o.setName(`user`).setDescription(`If you chose minecraft: IGN; if you chose discord: Username#Discriminator`).setRequired(true))
export function execute(interaction: ChatInputCommandInteraction, client: DiscordClient) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another not so necessary but for readability and good practice, maybe add a new line in-between the imports, the getKeyByValue function, the data export, and the execute function export. Also I think getKeyByValue would be more fit as it's own file in the Utils folder in the future event we may need to use it elsewhere.

Comment on lines +4 to +5
import { whisperChat } from '../../Utils/IngameChat';
export const data = new SlashCommandBuilder()
Copy link
Collaborator

Choose a reason for hiding this comment

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

new line in-between pls

Comment on lines +12 to +16
let pending = fs.readFileSync(__dirname + "/../../Verification/pendingVerification.json","utf-8");
let done = fs.readFileSync(__dirname + "/../../Verification/doneVerification.json","utf-8");
var pendingData = JSON.parse(pending);
var doneData = JSON.parse(done)
var user = `${interaction.user.username}#${interaction.user.discriminator}`
Copy link
Collaborator

Choose a reason for hiding this comment

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

All of these could be constants except for pendingData but I will talk about a potential solution to that. Additionally instead of making more separate variables you could just do const pending = JSON.parse(fs.readFileSync(__dirname + "/../../Verification/pendingVerification.json", "utf-8")); or if it's too long for your liking you could just make a function in the Utils folder that reads & parses it that way it is shorter.

Copy link
Author

Choose a reason for hiding this comment

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

I will probably just change this and...

Comment on lines +43 to +46
pendingData = JSON.stringify(pendingData)
doneData = JSON.stringify(doneData)
fs.writeFileSync(__dirname + "/../../Verification/pendingVerification.json",pendingData, { encoding: "utf-8" })
fs.writeFileSync(__dirname + "/../../Verification/doneVerification.json",doneData, { encoding: "utf-8" })
Copy link
Collaborator

Choose a reason for hiding this comment

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

So if you were to make pendingData and doneData constants a way to make this valid is you could make these seperate variables or just directly pass JSON.stringify(data) as one of the arguments in fs.writeFileSync

Copy link
Author

Choose a reason for hiding this comment

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

... And this

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.

[IDEA] In-Game verification

2 participants