Skip to content
This repository has been archived by the owner on Mar 1, 2024. It is now read-only.

Rewriting a bunch of reconnect and disconnect handling. #419

Merged
merged 4 commits into from Nov 2, 2023

Conversation

mcottontensor
Copy link
Contributor

@mcottontensor mcottontensor commented Nov 1, 2023

Relevant components:

  • Signalling server
  • Frontend library
  • Frontend UI library
  • Matchmaker
  • Platform scripts
  • SFU

Problem statement:

In addressing #401 I noticed there was a lot of clumsiness when dealing with disconnections and reconnections. There were several situations that would lead to reconnection loops, or incorrect status reporting etc.

Solution

I have cleaned up and streamlined a bunch of the logic around when to connect and which streamers to auto subscribe to. I have also made changes to how the reporting is done on the overlay.
Fixes #401

Documentation

Test Plan and Compatibility

A lot of manual tests and the usual npm tests. Will need further QA.

@mcottontensor mcottontensor changed the title Rewriting a bunch of reconnect and disconnect handling. Fixes #401 Rewriting a bunch of reconnect and disconnect handling. Nov 1, 2023
Copy link
Contributor

@lukehb lukehb left a comment

Choose a reason for hiding this comment

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

Left a few minor comments.

@lukehb
Copy link
Contributor

lukehb commented Nov 1, 2023

@MWillWallT This PR changes some loading bearing code around reconnections and disconnections.

Can I get you to test the following:

  • Reconnection/disconnect working as expected with one streamer
  • Reconnection/disconnect working as expected with one streamer and multiple peers
  • AFK working as expected with one streamer and multiple peers
  • Killing the streamer midstream then putting it back up auto reconnects fine
  • Killing the streamer midstream then waiting for reconnects to exhaust, putting the streamer back up then clicking connect works fine
  • Change the reconnect time interval and attempts works as expected
  • Connecting/disconnecting/reconnecting works fine when there are multiple streamers
  • Matchmaker with multiple UEs works fine
  • Anything else you think that would make sense for reconnection flows.
  • Tested on Chrome, Firefox, Safari, Android, and iOS.

Also keep in mind some of the messages displayed on the frontend change in this PR. Might be worth seeing if all the common ones are worded correctly and make sense for the scenario they show up in.

@MWillWallT
Copy link
Contributor

I've gone through and tested as many re-connection configs as I can, as well as all the suggested you mentioned @lukehb
All good on this front

@lukehb
Copy link
Contributor

lukehb commented Nov 2, 2023

QA has passed, you can consider backporting to 5.2, 5.3 too with this one I think.

@mcottontensor mcottontensor merged commit 1e05186 into EpicGames:master Nov 2, 2023
1 check passed
@mcottontensor mcottontensor deleted the fix_reconnect_flow branch November 2, 2023 01:26
@mcottontensor mcottontensor restored the fix_reconnect_flow branch November 2, 2023 01:29
mcottontensor added a commit that referenced this pull request Nov 2, 2023
Rewriting a bunch of reconnect and disconnect handling.
@mcottontensor mcottontensor deleted the fix_reconnect_flow branch November 2, 2023 01:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] - MaxPlayerCount can result in a reconnection loop.
3 participants