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

finalize tournament lobby + add variable event cards #908

Merged
merged 18 commits into from
Nov 9, 2023

Conversation

sgfost
Copy link
Contributor

@sgfost sgfost commented Oct 31, 2023

keeping track of a queue (all clients not yet put into a group) in
addition to pending groups and a simplified client->group mapping makes
some of the logic a bit more straightforward. A mutex should be safer
than the simpler lock

- add timeout to clear a group and try again if all clients havent
  accepted in a timely manner
- misc. minor refactoring/renaming

ref virtualcommons/planning#51
@sgfost sgfost added the server Server Side label Oct 31, 2023
@sgfost sgfost added this to the Mars Madness Tournament 2023 milestone Oct 31, 2023
formGroup(): string {
const group = [];
const groupId = uuidv4();
_.shuffle(this.state.queue);
Copy link
Member

Choose a reason for hiding this comment

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

would it be more straightforward to shuffle once and drain all valid groups at once and then leave the unlucky leftovers in the queue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the queue isn't actually managed correctly, I'll look at this when doing so

// groups indexed by a unique group id
pendingGroups: Map<string, LobbyClient[]> = new Map();
// map client username to group id
clientToGroup: Map<string, string> = new Map();
Copy link
Member

Choose a reason for hiding this comment

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

Would a Group class with a fixed size, LobbyClient[] and id simplify things? Then clientToGroup = Map<string, Group>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Almost certainly, could also just store the group reference on the LobbyClient which I think was how the old tournament lobby did it but that'll either be a case of a really messy extension of the LobbyClient schema class or putting it in the base class to be a dead property elsewhere. The map is probably fine.. only needed for one operation

- remove all logic/private group-forming-related data from the state and
  put into GroupManager class which holds the waiting queue and set of
  pending groups
- remove group disbanding in favor of sending the group off anyways and
  assuming that any disconnected player will attempt to join back.
  timeout was repurposed to do this 'forceful' moving after 15s
- further renaming
add optional ?type param to /has-active/ to be able to check whether a
player has a specific type of game active and avoid putting them into a
freeplay game (that may have been accidentally started) when they are
attempting to join a tournament
client/server communication and invitation flow could potentially be
checked with the colyseus testing package if we can mock some of the
services relied upon:  https://docs.colyseus.io/tools/unit-testing

- fix queue shuffling
@sgfost sgfost changed the title validate/refactor tournament lobby functionality finalize tournament lobby + add variable event cards Nov 2, 2023
@sgfost
Copy link
Contributor Author

sgfost commented Nov 2, 2023

Unit tests for the grouping logic + a fair bit of mostly manual smoke-testing have me feeling fairly confident about the lobby working correctly. I'll move onto the other few requirements in order to finish up this week and leave the start of next for further testing

- fixed an issue where the exit survey was being given on the gameover
  screen of a freeplay game due to some helper functions not
  distinguishing between tournament/game types when searching for
  invites
if treatments for a game's tournament are found, it will cycle
through them evenly based on the last game's treatment for that specific
tournament

- add command for creating treatments
- includes some small fixes based on results
@sgfost sgfost marked this pull request as ready for review November 6, 2023 22:03
sgfost and others added 5 commits November 6, 2023 15:42
when the dynamic settings gets called in tests it causes everything to
hang, as well as not working correctly. A better solution might be to
mock the module for the test env with static defaults
bug led to a tournament only knowing about the most recently created
treatment
@sgfost
Copy link
Contributor Author

sgfost commented Nov 8, 2023

the problem with treatments during the pretest was that I somehow botched applying the changes in 14f8bfd which fixes the issue. Fix for the lobby not staying open long enough is correct as well. Better tests added for both

@alee alee linked an issue Nov 8, 2023 that may be closed by this pull request
Copy link
Member

@alee alee left a comment

Choose a reason for hiding this comment

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

great work as always @sgfost !

@alee alee merged commit 896e3d9 into virtualcommons:main Nov 9, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
server Server Side
Projects
None yet
Development

Successfully merging this pull request may close these issues.

include link to code of conduct in tournament lobby chat verify isLobbyOpen logic
2 participants