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

[GH-1617] Unstable connection icon #1741

Closed
wants to merge 48 commits into from
Closed

Conversation

AngieDutra
Copy link
Contributor

@AngieDutra AngieDutra commented Apr 30, 2024

Closes #1617

Backend PR lambdaclass/mirra_backend#619

Motivation

When we have connectivity stability we have no feedback for the user

Summary of changes

This PR:

  • Adds a warning feature that shows the icon
  • Adds a disconnect feature for when the connection is lost
  • Adds a connectivity feature to check is the connection is stable
  • Add icon in game + it's requested animation
  • Block user's send action to backend until the stability is recovered, to avoid a queue of requests and bugs
  • Repositions the player once the connection is stable again
  • Supports airplane mode in game for both Android and IOS

DISCLAIMER: this doesn't support 3g testing. The issues when using 3g will be solved in a future iteration

Animation.mov

How has this been tested?

Setting for fake spikes:

  • Open 3 terminal tabs for the back repo
  • Open a terminal tab for the client PR
  • In the 1st back terminal tab run: toxiproxy-server
  • In the 2nd back terminal tab run: export USE_PROXY=true and then make start
  • In the client terminal tab run: toxiproxy-cli toxic add -n latency -t latency -a latency=300 game_proxy
  • In the 3rd back terminal tab run: ./lag_spikes.sh

In the backend repo, go to "lag_spikes.sh":
I used the following but you can alter each value to fake your own ping spikes

latencies=(150 500 150 100 500 100 500 100 500 100)
latency_durations=(1 1 1 1 1 1 1 1 1 1)

In the client repo:
Go to "GameServerConnectionManager.cs" and "ServerConnection.cs" and change in method "makeWebsocketUrl()"
int port = 5000;

Checklist

  • I have tested the changes locally.
  • I have tested the whole game after applying the changes, not only the affected areas.
  • I self-reviewed the changes on GitHub, line by line.
  • I have tested the changes in another devices.
    • Tested in iOS.
    • Tested in Android.
  • This change requires new documentation.
    • Documentation has been added/updated.

@jazimp
Copy link
Collaborator

jazimp commented May 3, 2024

Looks great! Visual approval ✅

@AngieDutra AngieDutra marked this pull request as draft May 3, 2024 21:32
@AngieDutra AngieDutra marked this pull request as ready for review May 6, 2024 14:18
@AngieDutra AngieDutra marked this pull request as draft May 15, 2024 16:08
@AngieDutra AngieDutra marked this pull request as ready for review May 16, 2024 13:25
Copy link
Contributor

@alfopisano alfopisano left a comment

Choose a reason for hiding this comment

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

Testing it as a user, I founded it a little difficult to play before the icon is displayed. I can see that other players are hurting me but I see them as far behind me, what to you think?
https://github.com/lambdaclass/curse_of_mirra/assets/116388676/2b1f98e4-b231-4413-a9c9-74c90f080960

@AngieDutra
Copy link
Contributor Author

I'll adjust the config for this

Copy link
Contributor

@alfopisano alfopisano left a comment

Choose a reason for hiding this comment

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

I am still encountering some problems as a user when playing with bad connection. As commented above I see that other players are hurting me but I am seeing them as far behind on the arena while the icon is still not showing up:

Untitled1.mov
Untitled2.mov

@tkz00 tkz00 marked this pull request as draft May 22, 2024 21:11
@Nico-Sanchez
Copy link
Contributor

We started all over in #1798

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Displaying connectivity Icon for connection issues
8 participants