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

[Godot] Websocket dropping data violates TCP reliability and congestion control #31

Open
joshmossas opened this issue Oct 1, 2024 · 4 comments · May be fixed by #697
Open

[Godot] Websocket dropping data violates TCP reliability and congestion control #31

joshmossas opened this issue Oct 1, 2024 · 4 comments · May be fixed by #697
Labels
bug topic:network tracked:godot Issue already tracker on the godot issue tracker

Comments

@joshmossas
Copy link

Tested versions

v4.0.beta10.official

System information

N/A

Issue description

See original issue in the Godot repo:
godotengine/godot#70738

The issue has been open since Dec 30, 2022

From the original post:

When the socket is being fed data (receiving) faster than your game can deal with it the WebSocketPeer poll-call will continue to read data despite the buffer filling up and then be forced to drop the data. This is not correct behavior for any application working with any protocol based on TCP.

This is not related to godotengine/godot#22496. If you're receiving websocket messages that are very large, then obviously you'll need to have a buffer large enough to fit such messages. Instead, this is regarding "normal" sized messages but a frame freeze or other performance reducing condition causing the websocket buffer to fill up when it eventually gets to the poll-call.

Dropping data violates the TCP reliability upon which websocket is built which means it's no longer possible to rely on data sent actually being received, causing one to have to implement data re-transmission mechanisms despite those already being present in TCP.

Steps to reproduce

N/A

Minimal reproduction project (MRP)

N/A

@Salvakiya Salvakiya added the bug label Oct 1, 2024
@Norrox
Copy link
Contributor

Norrox commented Oct 2, 2024

a semi workaround is changing the buffer size manually, but it wont stop the websocketpeer from stalling when its full.

func _ready():
	socket = WebSocketPeer.new()
	socket.connect_to_url("ws://localhost:8765/")
	socket.inbound_buffer_size = NEW SIZE
	socket.outbound_buffer_size = NEW SIZE

@Redot-Engine Redot-Engine deleted a comment Oct 6, 2024
This was referenced Oct 7, 2024
RadenTheFolf added a commit to RadenTheFolf/redot-engine that referenced this issue Oct 8, 2024
@Spartan322 Spartan322 added topic:network tracked:godot Issue already tracker on the godot issue tracker labels Oct 14, 2024
@Spartan322 Spartan322 changed the title Websocket dropping data violates TCP reliability and congestion control (Ported from Godot) [Godot] Websocket dropping data violates TCP reliability and congestion control Oct 14, 2024
@SkogiB
Copy link
Contributor

SkogiB commented Oct 14, 2024

We have some work being done to fix this, we made a PR upstream to see if Godot will take it first. godotengine/godot#98167

@Redot-Engine Redot-Engine deleted a comment from jawbroken Oct 16, 2024
@Redot-Engine Redot-Engine deleted a comment from joshmossas Oct 16, 2024
@Redot-Engine Redot-Engine deleted a comment from jawbroken Oct 16, 2024
@RadenTheFolf RadenTheFolf linked a pull request Oct 16, 2024 that will close this issue
@joshmossas
Copy link
Author

joshmossas commented Oct 28, 2024

@SkogiB Out of curiosity, have you guys determined a criteria for how long is reasonable to wait for a PR to be merged upstream by Godot?

I think it's good that y'all are trying to get some of your fixes merged upstream. As long as it doesn't cause too long of a delay. Having an upper limit of "waiting for Godot" time would probably be a good thing to consider.

Maybe it could even be a variable limit depending on the severity of the issue. Lower severity issues can prolly afford to wait a bit longer, while higher severity issues have a shorter time limit.

IDK just spitballing ideas.

@Norrox
Copy link
Contributor

Norrox commented Oct 29, 2024

@SkogiB Out of curiosity, have you guys determined a criteria for how long is reasonable to wait for a PR to be merged upstream by Godot?

I think it's good that y'all are trying to get some of your fixes merged upstream. As long as it doesn't cause too long of a delay. Having an upper limit of "waiting for Godot" time would probably be a good thing to consider.

Maybe it could even be a variable limit depending on the severity of the issue. Lower severity issues can prolly afford to wait a bit longer, while higher severity issues have a shorter time limit.

IDK just spitballing ideas.

#709

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug topic:network tracked:godot Issue already tracker on the godot issue tracker
Projects
Status: Waiting On Godot
Development

Successfully merging a pull request may close this issue.

5 participants