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

[MP] Move packet relay and peer signaling code to SceneMultiplayer. #67094

Merged
merged 1 commit into from Oct 27, 2022

Conversation

Faless
Copy link
Collaborator

@Faless Faless commented Oct 8, 2022

Fixes godotengine/godot-proposals#3208 .

In this PR:

MultiplayerPeer can now report if server relaying is allowed.

If this method returns true, the upper MultiplayerAPI layer is allowed to signal peers connected to the server to other clients, and perform packet relaying among them.

Add support for packet relaying and peer signaling in SceneMultiplayer.

When the MultiplayerPeer supports relaying packet, the high level multiplayer API will now advertise connected peers, and relay peer packets as requested.

Update ENet, WebRTC, WebSocket to support server relay.

Remove custom code for relaying from WebSocket and ENet, and let it be handled by the upper layer.

Update WebRTC to split create_client, create_server, and create_mesh, with the latter behaving like the old initialize with server_compatibility = false, and the first two supporting the upper layer relaying protocol.

@Faless Faless added this to the 4.0 milestone Oct 8, 2022
@Faless Faless requested review from a team as code owners October 8, 2022 20:36
@Faless Faless marked this pull request as draft October 9, 2022 22:20
@Faless
Copy link
Collaborator Author

Faless commented Oct 9, 2022

Converted to draft since it doesn't currently respect channel and trasfer mode when relaying.

@Faless Faless marked this pull request as ready for review October 10, 2022 22:22
@Faless
Copy link
Collaborator Author

Faless commented Oct 10, 2022

Converted to draft since it doesn't currently respect channel and trasfer mode when relaying.

Fixed now, and ready for review.
I've been going back and forth between the implementation in the last commit and a much simpler solution where we trust the client to ask for the proper channel and mode, but in the end I think two extra functions to MultiplayerPeer are worth the sanity check.

@Faless
Copy link
Collaborator Author

Faless commented Oct 11, 2022

Rebased after #66594

Copy link
Contributor

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

This looks great! I particularly like the WebRTC changes: the new method of doing "server compatibility" is easier to understand both internally and externally, and now that messages can be relayed through the server, it's really actually compatible.

Reading through the code, I only found little nitpicky things. I tested with ENet, but I haven't done a proper test with WebRTC or WebSockets yet.

modules/webrtc/webrtc_multiplayer_peer.cpp Outdated Show resolved Hide resolved
modules/websocket/websocket_multiplayer_peer.cpp Outdated Show resolved Hide resolved
modules/websocket/websocket_multiplayer_peer.cpp Outdated Show resolved Hide resolved
Copy link
Member

@mhilbrunner mhilbrunner left a comment

Choose a reason for hiding this comment

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

Great work, agree with the feature and implementation, left a few nitpicks.

MultiplayerPeer changes:

- Adds is_server_relay_supported virtual method

Informs the upper MultiplayerAPI layer if it can signal peers connected
to the server to other clients, and perform packet relaying among them.

- Adds get_packet_channel and get_packet_mode virtual methods

Allows the MultiplayerAPI to retrieve the channel and transfer modes to
use when relaying the last received packet.

SceneMultiplayerPeer changes:

- Implement peer signaling and packet relaying when the MultiplayerPeer
  advertise they are supported.

ENet, WebRTC, WebSocket changes:

- Removed custom code for relaying from WebSocket and ENet, and let it
  be handled by the upper layer.
- Update WebRTC to split create_client, create_server, and create_mesh,
  with the latter behaving like the old initialize with
  "server_compatibility = false", and the first two supporting the upper
  layer relaying protocol.
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.

Add server_relay option to WebRTCMultiplayer
3 participants