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

Add player reconnecting using name #343

Merged
merged 8 commits into from
Jan 22, 2022
Merged

Add player reconnecting using name #343

merged 8 commits into from
Jan 22, 2022

Conversation

I3uckwheat
Copy link
Owner

@I3uckwheat I3uckwheat commented Jan 10, 2022

0. Make sure your issue is linked:

Closes: #333
Mentions: #344

1. Purpose:

To allow clients to reconnect if they have accidentally disconnected

2. Implementation:

Remove the logic that removes a card from the Czar's hand and adjust the player ID when they reconnect.
The reconnect is simply using the player name. This also allows for changing devices mid-game.

3. Possible Issues:

There are missing tests for the re-connection of players.
There could be a specific edge case around players reconnecting at the same time the Czar selects
It is unknown what happens when the Czar disconnects currently.

4. How to Test:

Start a game with 3 players
select cards
disconnect a player
reconnect the player
czar selects cards
ensure that all winners are valid.

Try different combinations of disconnecting and reconnecting.

5. Cleanup

  • I've added all TODOs needed
  • I've removed all unneeded comments
  • I've updated the snapshot tests and checked to make sure they're accurate if they changed
  • I've removed all unnecessary console.logs

@I3uckwheat I3uckwheat marked this pull request as ready for review January 20, 2022 05:42
Copy link
Collaborator

@sher-s7 sher-s7 left a comment

Choose a reason for hiding this comment

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

Following the steps on how to test, everything seems to work perfectly 👌

I'm not sure if this is out of scope, but if one player submits their cards and another player leaves, it looks like it puts the game in an unplayable state. If the same player joins back again, it still remains unplayable

chrome_XUS14gEGXa_Trim.mp4

@I3uckwheat
Copy link
Owner Author

@sher-s7 This is definitely an issue. Is this something that can be fixed on the host side by skipping the round?

@sher-s7
Copy link
Collaborator

sher-s7 commented Jan 21, 2022

@sher-s7 This is definitely an issue. Is this something that can be fixed on the host side by skipping the round?

@I3uckwheat When I ran into the issue that was the first thing that came to mind. I think it might be a good solution

@I3uckwheat
Copy link
Owner Author

@sher-s7 I've updated the code, it should have some things rounded out now.

Copy link
Collaborator

@zachmmeyer zachmmeyer left a comment

Choose a reason for hiding this comment

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

I watched this happen live, looks good to me.

@sher-s7 sher-s7 self-requested a review January 21, 2022 06:52
Copy link
Collaborator

@sher-s7 sher-s7 left a comment

Choose a reason for hiding this comment

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

Nice 🔥

@I3uckwheat I3uckwheat merged commit 1b04eea into main Jan 22, 2022
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.

Add client reconnects
3 participants