Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Matcha groups size #42

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Matcha groups size #42

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
edfbf9a
hi its rahil and I hacked faris's computer to add group sizes to matcha
farisashai May 28, 2023
8996f60
big error writer i am personally
farisashai May 28, 2023
23c7207
Update Logger.ts
greeenbeans May 29, 2023
461b72f
tested edge case and removed log messages
greeenbeans Jun 8, 2023
f7a9b86
added code organization
greeenbeans Jun 8, 2023
b593ff0
linting
greeenbeans Jun 8, 2023
697b688
Update old comment
skyflaren Nov 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 33 additions & 5 deletions src/commands/Matcha.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ export default class Matcha extends Command {
constructor(client: BotClient) {
const definition = new SlashCommandBuilder()
.setName('matcha')
.addIntegerOption(option =>
option.setName('groupsize').setDescription('Set the minimum group size').setRequired(true)
)
.setDescription(
'Triggers matching for Matcha! Threads will be created in the channel where this command is called.'
);
Expand Down Expand Up @@ -104,17 +107,31 @@ export default class Matcha extends Command {
const shuffledMembersList = Matcha.shuffle(users);
const memberPairings = [];

const groupsize = interaction.options.getInteger('groupsize', true);
Logger.error(`/matcha - ready to match ${shuffledMembersList}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this Logger.error here? Is this a logging statement we can remove?

while (shuffledMembersList.length > 0) {
let pairedMembers: GuildMember[];
if (shuffledMembersList.length % 2 === 1) {
// If there's an odd number of people, we start with a group of 3 to correct it.
pairedMembers = shuffledMembersList.splice(0, 3);
const extraMembers = shuffledMembersList.length % groupsize;
const numGroups = Math.floor(shuffledMembersList.length / groupsize);
Logger.error(`/matcha - numGroups ${numGroups}, extraMembers: ${extraMembers} groupsize ${groupsize}`);
if (extraMembers !== 0) {
// If length % size is off add people to the first group or first n groups
// for one group remaining add all extras
if (numGroups === 1) {
pairedMembers = shuffledMembersList.splice(0, shuffledMembersList.length);
Logger.error(`/matcha - pair ${pairedMembers}`);
}
// otherwise, disperse extra people across groupss
else {
const addedToGroup = Math.floor(extraMembers / numGroups);
pairedMembers = shuffledMembersList.splice(0, groupsize + addedToGroup);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I think we can make this code more clear by breaking the code out into a helper function (maybe splitIntoGroups()?)

Additionally, I think we need to consider edge cases here:

  • What if the group size is 3 and we have 5 total members? I think this code makes a pair of 4 and leaves the last person lonely

} else {
pairedMembers = shuffledMembersList.splice(0, 2);
pairedMembers = shuffledMembersList.splice(0, groupsize);
}
memberPairings.push(pairedMembers);
}

Logger.error(`/matcha - matching worked? ${memberPairings}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

again here, i don't think we need this logging anymore ^

/**
* To prevent ourselves from hitting Discord's API rate limit (50 requests/second),
* we add a small delay between each creation of a group thread and execute them
Expand Down Expand Up @@ -181,6 +198,17 @@ export default class Matcha extends Command {
await super.edit(interaction, { content: '/matcha was canceled!', components: [] });
return;
}
const groupsize = interaction.options.getInteger('groupsize', true);

if (groupsize <= 1) {
await super.edit(interaction, {
content: 'You need to enter an integer greater than 1 otherwise your only friend will be yourself :angry:',
components: [],
ephemeral: true,
});
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I think we can rewrite this error message to be more in line with what our other messages look like

Instead of integer, we can say "group size", and also add some punctuation

**/matcha** needs a group size greater than 1! Otherwise, your only friend will be yourself! :rage:


// Otherwise, the 'Confirm' button was called.
this.lastRun = DateTime.now();
// Remove the button so they can't press it again.
Expand Down
2 changes: 1 addition & 1 deletion src/utils/Logger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export default createLogger({
format: json(),
transports: [
new Transports.Console({
level: 'debug',
level: 'error',
Copy link
Member Author

Choose a reason for hiding this comment

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

delete

format:
process.env.NODE_ENV === 'development' ? combine(colorize(), consoleLogFormat) : combine(timestamp(), json()),
}),
Expand Down
16 changes: 13 additions & 3 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1505,6 +1505,11 @@ has-flag@^3.0.0:
resolved "https://registry.npmjs.org/has-flag/-/has-flag-3.0.0.tgz"
integrity sha1-tdRU3CGZriJWmfNGfloH87lVuv0=

has-flag@^4.0.0:
version "4.0.0"
resolved "https://registry.yarnpkg.com/has-flag/-/has-flag-4.0.0.tgz#944771fd9c81c81265c4d6941860da06bb59479b"
integrity sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ==

has-symbols@^1.0.1, has-symbols@^1.0.2:
version "1.0.2"
resolved "https://registry.npmjs.org/has-symbols/-/has-symbols-1.0.2.tgz"
Expand Down Expand Up @@ -2142,7 +2147,7 @@ ms@^2.1.1:
resolved "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz"
integrity sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==

nan@^2.17.0:
nan@^2.14.0, nan@^2.17.0:
version "2.17.0"
resolved "https://registry.npmjs.org/nan/-/nan-2.17.0.tgz"
integrity sha512-2ZTgtl0nJsO0KQCjEpxcIr5D+Yv90plTitZt9JBfQvVJDS5seMl3FOvsh3+9CoYWXf/1l5OaZzzF6nDm4cagaQ==
Expand Down Expand Up @@ -2234,9 +2239,9 @@ object-inspect@^1.11.0, object-inspect@^1.9.0:
resolved "https://registry.npmjs.org/object-inspect/-/object-inspect-1.11.0.tgz"
integrity sha512-jp7ikS6Sd3GxQfZJPyH3cjcbJF6GZPClgdV+EFygjFLQ5FmW/dRUnTd9PQ9k0JhoNDabWFbpF1yCdSWCC6gexg==

object-keys@^1.1.1:
object-keys@^1.0.12, object-keys@^1.1.1:
version "1.1.1"
resolved "https://registry.npmjs.org/object-keys/-/object-keys-1.1.1.tgz"
resolved "https://registry.yarnpkg.com/object-keys/-/object-keys-1.1.1.tgz#1c47f272df277f3b1daf061677d9c82e2322c60e"
integrity sha512-NuAESUOUMrlIXOfHKzD6bpPu3tYt3xvjNdRIQ+FeT0lNb4K8WR70CaDxhuNguS2XG+GjkyMwOzsN5ZktImfhLA==

object.assign@^4.1.2:
Expand Down Expand Up @@ -2388,6 +2393,11 @@ path-type@^3.0.0:
dependencies:
pify "^3.0.0"

path-type@^4.0.0:
version "4.0.0"
resolved "https://registry.yarnpkg.com/path-type/-/path-type-4.0.0.tgz#84ed01c0a7ba380afe09d90a8c180dcd9d03043b"
integrity sha512-gDKb8aZMDeD/tZWs9P6+q0J9Mwkdl6xMV8TjnGP3qJVJ06bdMgkbBlLU8IdfOsIsFz2BW1rNVT3XuNEl8zPAvw==

picocolors@^1.0.0:
version "1.0.0"
resolved "https://registry.npmjs.org/picocolors/-/picocolors-1.0.0.tgz"
Expand Down