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

[Net] ENet poll optimizations, fragmented unreliable transfer. #53129

Merged
merged 2 commits into from
Dec 15, 2021

Conversation

Faless
Copy link
Collaborator

@Faless Faless commented Sep 27, 2021

In this PR:

  • ENetMultiplayerPeer now sends fragmented packets unreliably too.
  • ENetMultiplayerPeer poll now only service the connection once.

4.0 port of 2 out of the 3 changes in #51466 .
The other change (sendmsg/WSASendTo) needs more testing on the 3.x branch because according to MS docs WSABUF is not available in UWP apps (though WSASendTo is?)

Bugsquad edit: master version of #53130.

@Faless Faless added this to the 4.0 milestone Sep 27, 2021
@Faless Faless requested a review from a team as a code owner September 27, 2021 13:32
@jordo
Copy link
Contributor

jordo commented Sep 30, 2021

if porting to 4.0 branch, mind throwing a co-author on the 4.0 commits?

It used to always send them reliably when transfer mode was unreliable
or ordered if the packet size was more then the enet host MTU (1400
bytes by default).

This commit also adds a warning when debug is enabled to explain the
effects of sending fragmented packets unreliably.
It used to call `enet_host_service` until all events were consumed, but
that also meant constantly polling the connection leading to potentially
unbounded processing time.

It now only service the connection once, and instead consumes all the
retrieved events via `enet_host_check_events`.
@Faless Faless merged commit c5879db into godotengine:master Dec 15, 2021
@mhilbrunner
Copy link
Member

Thanks :) And thanks to @jordo too!

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.

3 participants