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.

Sign up for GitHub

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

Jump to bottom

[Net] Low level ENet wrappers, ENet meshes for multiplayer. #50710

Merged
merged 3 commits into from
Jul 29, 2021
Merged

[Net] Low level ENet wrappers, ENet meshes for multiplayer. #50710

merged 3 commits into from
Jul 29, 2021

Conversation

Faless
Copy link
Collaborator

@Faless Faless commented Jul 21, 2021 •
edited
Loading

In this PR:

  • Move random ID generation function to MultiplayerPeer.
  • Add ENetConnection as a low level wrapper for ENetHost.
  • Add ENetPacketPeer as a low level wrapper for ENetPeer.
  • ENetMultiplayer now uses the low level wrappers internally (and exposes them for extra configuration).
  • Optimize ENetMultiplayer server packet allocations (avoid duplicating packets as much as possible).
  • Add create_mesh and add_mesh_peer to ENetMultiplayer to implement a full mesh network:
    • Mesh is immediately connected and has the given unique ID.
    • add_mesh_peer takes 2 parameters, the remote peer ID, and a ENetConnection, which must have exactly one peer, in the connected state (service it manually via service until connection, and only then add it to the mesh).

Closes #40626

ENetMultiplayer

ENetPacketPeer

ENetConnection

@Faless Faless added this to the 4.0 milestone Jul 21, 2021
@Faless Faless force-pushed the enet/4.x_refactor branch from 6e0dd31 to e8a2721 Compare July 21, 2021 19:00
@Calinou
Copy link
Member

Calinou commented Jul 21, 2021

What are the differences and use cases for mesh vs non-mesh peers? Is this about avoiding the need for users to forward ports when they host a server?

@Faless
Copy link
Collaborator Author

Faless commented Jul 21, 2021 •
edited
Loading

What are the differences and use cases for mesh vs non-mesh peers? Is this about avoiding the need for users to forward ports when they host a server?

No, in the mesh configuration each peer is directly connected to each other, so they can communicate directly instead of having to passing through a "server" peer.

They still need to open ports in some way and could do so by port forwarding, using NAT punch-trough (which I guess would be the most common case), etc.

EDIT:
To clarify, a good use case for mesh is peer-to-peer multiplayer.
While in the usual client/server configuration a self-hosting player (i.e. acting as server) can potentially read all the communication between other peers, in the mesh configuration an rpcs will go directly from the sending peer to the destination peer(s). This also helps balancing the bandwidth across peers (the server would otherwise need more bandwidth then the clients).

@Faless Faless force-pushed the enet/4.x_refactor branch 2 times, most recently from 509429d to 342bdda Compare July 23, 2021 13:35
@Faless Faless marked this pull request as ready for review July 23, 2021 13:36
@Faless Faless requested review from a team as code owners July 23, 2021 13:36
@Faless Faless changed the title [WIP] [Net] Low level ENet wrappers, ENet meshes for multiplayer. [Net] Low level ENet wrappers, ENet meshes for multiplayer. Jul 23, 2021
@Faless Faless force-pushed the enet/4.x_refactor branch from 342bdda to ff197bd Compare July 26, 2021 09:10
@Faless Faless requested a review from a team as a code owner July 26, 2021 09:10
Calinou
Calinou reviewed Jul 26, 2021
View reviewed changes
doc/classes/MultiplayerPeer.xml
<return type="int">
</return>
<description>
Returns a randomly generated integer that can be used as a network unique ID.
Copy link
Member

@Calinou Calinou Jul 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Is this ID persistent across restarts or client reconnections? This should be documented.

Copy link
Collaborator Author

@Faless Faless Jul 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Is this ID persistent across restarts or client reconnections?

No, it's a pseudo-random number as mentioned.

modules/enet/doc_classes/ENetPacketPeer.xml Outdated
Comment on lines 173 to 199
<constant name="PEER_PACKET_THROTTLE" value="7" enum="PeerStatistic">
</constant>
<constant name="PEER_PACKET_THROTTLE_LIMIT" value="8" enum="PeerStatistic">
</constant>
<constant name="PEER_PACKET_THROTTLE_COUNTER" value="9" enum="PeerStatistic">
</constant>
<constant name="PEER_PACKET_THROTTLE_EPOCH" value="10" enum="PeerStatistic">
</constant>
<constant name="PEER_PACKET_THROTTLE_ACCELERATION" value="11" enum="PeerStatistic">
</constant>
<constant name="PEER_PACKET_THROTTLE_DECELERATION" value="12" enum="PeerStatistic">
</constant>
<constant name="PEER_PACKET_THROTTLE_INTERVAL" value="13" enum="PeerStatistic">
</constant>
<constant name="PACKET_LOSS_SCALE" value="65536">
</constant>
<constant name="PACKET_THROTTLE_SCALE" value="32">
</constant>
Copy link
Member

Choose a reason for hiding this comment

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

These constants are missing documentation.

modules/enet/doc_classes/ENetPacketPeer.xml
Comment on lines +159 to +167
<constant name="PEER_PACKET_LOSS_EPOCH" value="2" enum="PeerStatistic">
</constant>
Copy link
Member

Choose a reason for hiding this comment

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

This constant is missing documentation.

modules/enet/doc_classes/ENetPacketPeer.xml
Comment on lines +133 to +159
<constant name="STATE_DISCONNECTED" value="0" enum="PeerState">
</constant>
<constant name="STATE_CONNECTING" value="1" enum="PeerState">
</constant>
<constant name="STATE_ACKNOWLEDGING_CONNECT" value="2" enum="PeerState">
</constant>
<constant name="STATE_CONNECTION_PENDING" value="3" enum="PeerState">
</constant>
<constant name="STATE_CONNECTION_SUCCEEDED" value="4" enum="PeerState">
</constant>
<constant name="STATE_CONNECTED" value="5" enum="PeerState">
</constant>
<constant name="STATE_DISCONNECT_LATER" value="6" enum="PeerState">
</constant>
<constant name="STATE_DISCONNECTING" value="7" enum="PeerState">
</constant>
<constant name="STATE_ACKNOWLEDGING_DISCONNECT" value="8" enum="PeerState">
</constant>
<constant name="STATE_ZOMBIE" value="9" enum="PeerState">
</constant>
Copy link
Member

Choose a reason for hiding this comment

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

These constants are missing documentation.

@Faless
Copy link
Collaborator Author

Faless commented Jul 26, 2021

@Calinou About the constants that are missing documentation, they are also missing in ENet. I really don't feel like second-guessing the ENet author here.

Faless added 3 commits July 29, 2021 10:40
@Faless
[Net] Add generate_unique_id to MultiplayerPeer.
1e8bf86 
Used by ENetMultiplayerPeer and WebSocketServer to generate network IDs,
and exposed to the user for p2p networks (e.g. WebRTCMultiplayerPeer)
and custom MultiplayerPeer implementations.
@Faless
[Net] Implement lower level ENet wrappers.
42a1777
@Faless
[Net] Refactor ENetMultiplayerPeer to use ENet wrappers.
f39547b
@Faless Faless force-pushed the enet/4.x_refactor branch from ff197bd to f39547b Compare July 29, 2021 08:59
@Faless Faless mentioned this pull request Jul 29, 2021
Merged
@akien-mga akien-mga merged commit 1c68be9 into godotengine:master Jul 29, 2021
@akien-mga
Copy link
Member

Thanks!

@Faless Faless deleted the enet/4.x_refactor branch July 29, 2021 11:07
@Faless Faless mentioned this pull request Aug 9, 2021
Merged
@akien-mga akien-mga mentioned this pull request Aug 9, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Calinou Calinou Calinou left review comments

@akien-mga akien-mga akien-mga approved these changes

Assignees
No one assigned
Labels
enhancement topic:network
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.
3 participants
@Faless @Calinou @akien-mga