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.

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

[WIP] Matchmaking Queue #559

Closed
wants to merge 3 commits into from
Closed

Conversation

kev306
Copy link
Collaborator

@kev306 kev306 commented Dec 15, 2023

Added a matchmaking class to implement a queue system


// Add user to queue
addUserToQueue(username: string): void {
if (!(username in this.userQueue)) {
Copy link
Owner

Choose a reason for hiding this comment

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

// Checks if number users in queue >= specified roomSize.
// If yes, returns the list of users and removes them from the queue.
// If < specified roomSize, returns null
checkRoomPossible(roomSize: number): string[] | null {
Copy link
Owner

Choose a reason for hiding this comment

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

Let's remove the roomSize input here. If we need to configure anything for the lifetime of this object, let's do it in the constructor.

For this PR, lets just hard code to 6 players in this file.

const users = this.userQueue.slice(0 , roomSize);
this.userQueue = this.userQueue.slice(roomSize);
return users;
} else return null;
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of returning null, let's return empty array.

@@ -0,0 +1,33 @@
export class Matchmaking {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
export class Matchmaking {
export class MatchmakingQueue {

@@ -0,0 +1,33 @@
export class Matchmaking {
userQueue: string[];
Copy link
Owner

Choose a reason for hiding this comment

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

Let's make this private.

it('should allow a user to be added to the queue', () => {
matchmaking.addUserToQueue('pronub');

expect(matchmaking.userQueue).toContain('pronub');
Copy link
Owner

Choose a reason for hiding this comment

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

Update tests to not read into private member userQueue


matchmaking.removeUser('pronub');

expect(matchmaking.userQueue).not.toContain('pronub');
Copy link
Owner

Choose a reason for hiding this comment

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

Replace test with:

  • Add 5 users
  • No match
  • Add 6th user
  • Match
  • Try to find another match, should be empty

expect(matchmaking.userQueue).not.toContain('pronub');
});

it('should do nothing if the user is not in the queue', () => {
Copy link
Owner

Choose a reason for hiding this comment

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

Doesn't crash if we remove a user who was never added (you can paraphrase this).

@vck3000 vck3000 closed this Feb 15, 2024
@kev306 kev306 deleted the feat/matchmaking branch March 19, 2024 05:05
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