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

Large Refactor, Disabled Multiplayer For Now #711

Closed
wants to merge 3 commits into from

Conversation

Ragudos
Copy link
Contributor

@Ragudos Ragudos commented Sep 6, 2023


title: Large Refactor, Disabled Multiplayer For Now

Discord Username: @aaronragudos

What type of PR is this? (select all that apply)

  • πŸ• Feature
  • [z ] πŸ› Bug Fix
  • [z ] 🚧 Breaking Change
  • [z ] πŸ§‘β€πŸ’» Code Refactor
  • πŸ“ Documentation Update

Description

  • I got intrigued by socket.io, so I went and looked at our code.
  • I found some things:
  1. Database calls are done both in the client and the websocket server (which should be in the server I believe).
  2. There is no proper cleanup functions for socket.io listeners.
  3. The client/app automatically connects to the websocket server. This should be disabled (autoConnect: false) since we will only connect to the server if the user plays multiplayer.
  4. Complicated libraries are used for simple passing of data like a language. This can be handled with a simple handleSubmit function and using zod to parse the chosen item.
  5. Little abstraction for the websocket server. We are both handling events and memory inside one class/constructor. I made a separate file for in-memory handling (to check for races and whatnot).

NOTE: This PR is just the beginning to my changes, and since I realized the changes needed are huge, I decided to just disable the multiplayer for now. If you are willing to hack at it, then please feel free to do so. Thank you.

Related Tickets & Documents

  • Related Issue #
  • Closes #

QA Instructions, Screenshots, Recordings

The video is too big for GitHub (8mins) where I explained what changes I made and such. Here it is on YT (unlisted, if it's not allowed, please comment so):

UI accessibility concerns?

Added/updated tests?

  • πŸ‘ yes
  • πŸ™… no, because they aren't needed
  • [x ] πŸ™‹ no, because I need help (Help please >w<)

[optional] Are there any post deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

@Ragudos
Copy link
Contributor Author

Ragudos commented Sep 8, 2023

Closing this pull request. Currently working on the refactor. I will submit a PR once I complete the prototype (by that, I mean once I finish and manually test everything).

Current state:

  • A middleware for getting information about the user trying to connect to the socket.
  • Finished room mechanism of being able to join
  • There is a room owner idea now, so a race does not start automatically in rooms.
  • When a room owner leaves, the player that joined after them will be the new owner.
  • If there is only one player in a room and they leave, the room will be deleted/removed, thus no one will be able to join the room (a new one must be created).
  • If a the status of the race is not "WAITING" and a player leaves, check if the amount of players left is only one. If so, reset the game.
  • We can now see the list of players that joined and left a room
  • Countdown (will happen on the client, useEffect & timeout) when starting a game
  • After countdown finishes, game will start

Notes:

  • Sessions and rooms are stored in the memory of the server, not on the database
  • The client will now only connect to the socket if they are going to play multiplayer, and they will be disconnected if they navigate away from a room they joined or when they refresh a page (but since refreshing a page means that they are on the same page, then they would automatically reconnect to the room if the room still exists.).

Todos:

  • Make the race functionality of the private rooms version of the multiplayer feature.

Plans:

  • A database call will only happen if a race has finished.

Refer to the branch of my fork, "refactor wss" (along those lines), to keep track of my changes. Thanks!

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.

1 participant