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 ping interval and timeout to player #304

Merged
merged 1 commit into from Jul 28, 2022

Conversation

MatteoH2O1999
Copy link
Contributor

Sometimes showdown server could suffer from a lag spike, expecially on less powerful machines. This allows to increase or disable timeouts for the keepalive mechanism used by websockets.

@hsahovic let me know if you think this could be useful

@MatteoH2O1999 MatteoH2O1999 force-pushed the websocket-timeout branch 3 times, most recently from 8c894fb to 0bca7f8 Compare July 25, 2022 08:46
@codecov
Copy link

codecov bot commented Jul 25, 2022

Codecov Report

Merging #304 (3ffa6f3) into master (ae58491) will increase coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head 3ffa6f3 differs from pull request most recent head cc462e6. Consider uploading reports for the commit cc462e6 to get more accurate results

@@            Coverage Diff             @@
##           master     #304      +/-   ##
==========================================
+ Coverage   88.57%   88.59%   +0.02%     
==========================================
  Files          36       36              
  Lines        3632     3640       +8     
==========================================
+ Hits         3217     3225       +8     
  Misses        415      415              

@MatteoH2O1999 MatteoH2O1999 force-pushed the websocket-timeout branch 5 times, most recently from 66b72ed to 0aa636a Compare July 25, 2022 09:40
@MatteoH2O1999
Copy link
Contributor Author

Added a fix for metaclass conflict as reported in #305

@MatteoH2O1999 MatteoH2O1999 changed the title Add ping interval and timeout to player Add ping interval and timeout to player and fix metaclass conflict Jul 25, 2022
@hsahovic
Copy link
Owner

@MatteoH2O1999 what's the default timeout ping? How did you encounter this issue?

@SomeRandomGuy009
Copy link

Added a fix for metaclass conflict as reported in #305

yep, this does fix the issue.

@MatteoH2O1999
Copy link
Contributor Author

@hsahovic,
default timeout is 20 seconds. For training I'm using an old computer so the server sometimes won't respond in time because of tensorflow keeping all the CPU occupied or because of RAM constraints forcing a lot of page swapping

@hsahovic
Copy link
Owner

@MatteoH2O1999 this looks good - can you remove the metaclass fix (I fixed the gym version instead)?

@MatteoH2O1999
Copy link
Contributor Author

Sure, but won't pinning the gym version be limiting?

@MatteoH2O1999 MatteoH2O1999 changed the title Add ping interval and timeout to player and fix metaclass conflict Add ping interval and timeout to player Jul 28, 2022
@hsahovic
Copy link
Owner

It is, but the new gym version made some deeper changes to their base API (eg. what step should be returning) and I'd prefer to wait before switching to it, so we can assess if it also requires other changes.

@MatteoH2O1999
Copy link
Contributor Author

Sure thing: I'll take a look at it and see if I can figure something out. In the meantime I already removed the metaclass fix

@hsahovic hsahovic merged commit a66c9cc into hsahovic:master Jul 28, 2022
@MatteoH2O1999 MatteoH2O1999 deleted the websocket-timeout branch September 8, 2022 22:41
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.

None yet

3 participants