[Net] Modularize multiplayer, expose MultiplayerAPI to extensions.

[Faless]

TL; DR;

Most of the Multiplayer code has been extracted from core/scene to modules/multiplayer which can be compiled out (see below for details).

A new MultiplayerAPIExtension class allows extending the MultiplayerAPI by overriding specific methods to manage peers, handling RPCs, and even replacing the replication behavior (via the extra configuration methods _object_configuration_add/_object_configuration_remove).

Script.get_rpc_config (get_rpc_methods) now returns a Variant so we can keep compatibility in the future (the module expects a Dictionary for now).

In code

extends MultiplayerAPIExtension
class_name LogMultiplayer

# We want to augment the default SceneMultiplayer.
var base_multiplayer = SceneMultiplayer.new()

func _init():
	# Just passthourgh base signals (copied to var to avoid cyclic reference)
	var cts = connected_to_server
	var cf = connection_failed
	var pc = peer_connected
	var pd = peer_disconnected
	base_multiplayer.connected_to_server.connect(func(): cts.emit())
	base_multiplayer.connection_failed.connect(func(): cf.emit())
	base_multiplayer.peer_connected.connect(func(id): pc.emit(id))
	base_multiplayer.peer_disconnected.connect(func(id): pd.emit(id))

# Log RPC being made and forward it to the default multiplayer.
func _rpc(peer: int, object: Object, method: StringName, args: Array) -> int: # Error
	print("Got RPC for %d: %s::%s(%s)" % [peer, object, method, args])
	return base_multiplayer.rpc(peer, object, method, args)

# Log configuration add. E.g. root path (nullptr, NodePath), replication (Node, Spawner|Synchronizer), custom.
func _object_configuration_add(object, config: Variant) -> int: # Error
	if config is MultiplayerSynchronizer:
		print("Adding synchronization configuration for %s. Synchronizer: %s" % [object, config])
	elif config is MultiplayerSpawner:
		print("Adding node %s to the spawn list. Spawner: %s" % [object, config])
	return base_multiplayer.object_configuration_add(object, config)

# Log configuration remove. E.g. root path (nullptr, NodePath), replication (Node, Spawner|Synchronizer), custom.
func _object_configuration_remove(object, config: Variant) -> int: # Error
	if config is MultiplayerSynchronizer:
		print("Removing synchronization configuration for %s. Synchronizer: %s" % [object, config])
	elif config is MultiplayerSpawner:
		print("Removing node %s from the spawn list. Spawner: %s" % [object, config])
	return base_multiplayer.object_configuration_remove(object, config)

# These can be optional, but in our case we want to augment SceneMultiplayer, so forward everything.
func _set_multiplayer_peer(p_peer: MultiplayerPeer):
	base_multiplayer.multiplayer_peer = p_peer

func _get_multiplayer_peer() -> MultiplayerPeer:
	return base_multiplayer.multiplayer_peer

func _get_unique_id() -> int:
	return base_multiplayer.get_unique_id()

func _get_peer_ids() -> PackedInt32Array:
	return base_multiplayer.get_peers()

And the main:

extends Node

func _enter_tree():
	# Sets our custom multiplayer as the main one in SceneTree.
	get_tree().set_multiplayer(LogMultiplayer.new())

func _ready():
	# Starts a server.
	var peer = ENetMultiplayerPeer.new()
	peer.create_server(1234)
	multiplayer.set_multiplayer_peer(peer)

	test.rpc(1, 2, 0.42)
	# Same as
	multiplayer.rpc(0, self, "test", [1, 2, 0.42])

@rpc
func test():
	pass

Review considerations

• Script.get_rpc_config (get_rpc_methods) now returns a Variant and should probably be exposed to scripting, the current implementation expects a Dictionary, and I should have implemented them properly in both gdscript/mono (CC @vnen @neikeq ).
• The remaining MultiplayerAPI should probably be moved to scene/main/ (and MultiplayerPeer moved back to core/io) EDIT: same for MultiplayerPeer (cc @reduz ).
• MultiplayerPeer should probably use Variant instead of int in methods and signals (so it supports hashes etc) and the mapping should just be implemented by the MultiplayerAPI. Deserves its own PR
• Expose Multiplayer.[de|en]code_variant[s] (cc @dsnopek ). Also deserves its own PR
• Depends on Add peer visibility to MultiplayerSynchronizer. #62961
• Expose a faster RPC callback in extensions/languages that support variable arguments (callable?) Needs more discussion.
• - Document the new methods in MultiplayerAPI and the new MultiplayerAPIExtension.

Supersedes (closes #62870)
Partly address #38294

[Faless]

One other thing to note is that we could also move _[set|get|has]_multiplayer_peer to the module implementation (and you could still implement the method yourself, at least in GDScript).

[raulsntos]

I only looked a bit into the C# side.

[Faless]

Thanks a lot for the review @raulsntos , I fixed the typo, and did the renaming.
There seem to still be an error when building the mono glue in modules/mono/glue/GodotSharp/GodotSharp/Core/Attributes/RPCAttribute.cs which I'm not sure how to fix with proper imports. So help is welcome there :) .

[raulsntos]

That's because the C# attribute uses the RPC enums that used to be core constants and now are enums in MultiplayerAPI and MultiplayerPeer. This should fix it:

diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/Attributes/RPCAttribute.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/Attributes/RPCAttribute.cs
index 0a1c8322d7..fb37838ffa 100644
--- a/modules/mono/glue/GodotSharp/GodotSharp/Core/Attributes/RPCAttribute.cs
+++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/Attributes/RPCAttribute.cs
@@ -5,8 +5,8 @@ namespace Godot
     /// <summary>
     /// Attribute that changes the RPC mode for the annotated <c>method</c> to the given <see cref="Mode"/>,
     /// optionally specifying the <see cref="TransferMode"/> and <see cref="TransferChannel"/> (on supported peers).
-    /// See <see cref="RPCMode"/> and <see cref="TransferMode"/>. By default, methods are not exposed to networking
-    /// (and RPCs).
+    /// See <see cref="MultiplayerAPI.RPCMode"/> and <see cref="MultiplayerPeer.TransferModeEnum"/>.
+    /// By default, methods are not exposed to networking (and RPCs).
     /// </summary>
     [AttributeUsage(AttributeTargets.Method, AllowMultiple = false)]
     public class RPCAttribute : Attribute
@@ -14,7 +14,7 @@ namespace Godot
         /// <summary>
         /// RPC mode for the annotated method.
         /// </summary>
-        public RPCMode Mode { get; } = RPCMode.Disabled;
+        public MultiplayerAPI.RPCMode Mode { get; } = MultiplayerAPI.RPCMode.Disabled;
 
         /// <summary>
         /// If the method will also be called locally; otherwise, it is only called remotely.
@@ -24,7 +24,7 @@ namespace Godot
         /// <summary>
         /// Transfer mode for the annotated method.
         /// </summary>
-        public TransferMode TransferMode { get; set; } = TransferMode.Reliable;
+        public MultiplayerPeer.TransferModeEnum TransferMode { get; set; } = MultiplayerPeer.TransferModeEnum.Reliable;
 
         /// <summary>
         /// Transfer channel for the annotated mode.
@@ -35,7 +35,7 @@ namespace Godot
         /// Constructs a <see cref="RPCAttribute"/> instance.
         /// </summary>
         /// <param name="mode">The RPC mode to use.</param>
-        public RPCAttribute(RPCMode mode = RPCMode.Authority)
+        public RPCAttribute(MultiplayerAPI.RPCMode mode = MultiplayerAPI.RPCMode.Authority)
         {
             Mode = mode;
         }

[Faless]

Thanks a lot ❤️ . This should do it then, I had some issues with msbuild and was using CI to test the glue and couldn't figure out the Enum part in TransferModeEnum 😅 . Thanks again!

[dsnopek]

@Faless This is almost enough to implement what I was looking for in godotengine/godot-proposals#4873

My goal (as described in more detail there) is for RPC's (and any other messages that SceneMultiplayer sends) to be relayed via a match in Nakama, so that developers can transparently use the High-Level Multiplayer API or the new scene replication stuff (same as they would with ENet, for example).

Extending MultiplayerAPI in GDScript, and not using any MultiplayerPeer, would get me part of the way there. By implementing _rpc() I could get any RPC's the developer sends, and send them via Nakama. Then the receiving peer can use that information to call a method on a node in the scene tree. However, there's lots of stuff from SceneMultiplayer that I'd need to re-implement myself, like calling the RPC's locally when necessary. Also, it doesn't look like any of the scene replication stuff would work.

Given this refactor, it still seems like implementing a custom MultiplayerPeer from GDScript (like I described in my proposaal) would work better, because then all the stuff in SceneMultiplayer would still work, the data would just be transmitted via Nakama.

But maybe further refactoring to SceneMultiplayer could maybe allow this to work? Similar to the ForwardMultiplayer example in the issue description, if my custom MultiplayerAPI could use SceneMultiplayer in such a way that it could get or submit the binary data it uses (without it trying to directly use the MultiplayerPeer), then it could perhaps rely on it to do most of the actual work? Currently, SceneMultiplayer and its helper classes (like SceneReplicationInterface, SceneRPCInterface, etc) are still directly interacting with MultiplayerPeer for lots of stuff.

Unless I'm totally missing/misunderstanding something?

[Faless]

@dsnopek I think you are right and after seeing you implementation I think I misunderstood the scope of your original proposal.

I think we should also have something like you describe in 4.x, probably just extending MultiplayerPeerExtension and overriding the few methods that can't be overridden in gdscript making them use the appropriate format (get_packet/put_packet?).

I'm a bit busy this week, but I'll try to look into it, it should be really just those 2 functions as far as I can tell since the other methods should be already extendable in GDScript.

[dsnopek]

@Faless:

probably just extending MultiplayerPeerExtension and overriding the few methods that can't be overridden in gdscript making them use the appropriate format (get_packet/put_packet?)

Yeah, I think it is just get_packet/put_packet.

By extending, do you mean making a new class (for example,MultiplayerPeerCustom) that descends from MultiplayerPeerExtension to provide a better interface for GDScript for those two methods? Or, changing MultiplayerPeerExtension so that it exposes a different interface to GDScript and GDExtension? I actually don't know if the 2nd option is possible, but it'd be interesting to not need another class.

Given a direction, I could try and make a PR for it!

[Faless]

@dsnopek well, I had both in mind, not sure which one is better.

If we just modify MultiplayerPeerExtension, we could have put_packet/get_packet like:

if (GDVIRTUAL_IS_OVERRIDDEN(_put_packet)) {
  // current fast route.
} else if (GDVIRTUAL_IS_OVERRIDDEN(_put_packet_script)) {
  // Make PackedByteArray and GDVIRTUAL_CALL _put_packet_script
}

Which is nice, because we don't need a new class, but we add a bunch of checks to every packet sent (probably fine).

Actually, I think we could likely also simplify it to just:

if (GDVIRTUAL_CALL(_put_packet)) { // should return false when not overridden
  // current fast route.
} else if (GDVIRTUAL_IS_OVERRIDDEN(_put_packet_script)) { // check is_overridden only in the gdscript route.
  // Make PackedByteArray and GDVIRTUAL_CALL _put_packet_script
}

I think if this works and do not spam errors about the first call we should go this route.

Given a direction, I could try and make a PR for it!

Sure, go ahead and ping me for review, thanks :)

[dsnopek]

@Faless Here's a PR implementing your suggestion: #63262

I ported my Nakama code to use it and it works without any errors or warnings about the first call!

[Faless]

Okay, I think this is generally ready for review. I've left out a few things which probably should go in their own PRs:

• MultiplayerPeer should probably use Variant instead of int in methods and signals (so it supports hashes etc) and the mapping should just be implemented by the MultiplayerAPI.
• Expose Multiplayer.[de|en]code_variant[s]
• Expose a faster RPC callback in extensions/languages that support variable arguments (not sure what's the best solution here).

I've added documentation for MultiplayerAPI and MultiplayerAPIExtension.
Documentation for the module is still missing (beside the two added notes so the knowledge is not lost).
I know @nathanfranke was working on some documentation to add on top of this so I'll drop a friendly ping but I can work on it next if he doesn't have time.
In the meantime, I'm off getting RPC visibility to work :).

P.S.: Some stats:

$ git diff HEAD~1 --stat -- core/ scene/ editor/
39 files changed, 638 insertions(+), 4893 deletions(-)

$ git diff HEAD~1 --stat -- modules/
51 files changed, 4862 insertions(+), 137 deletions(-)

[akien-mga]

Thanks!

[nathanfranke]

Are we sure this PR has enough topic labels? 😆

[aaronfranke]

I'm getting a linker error when using this method in a module, but it works if I move the code back into the header. I've tested that it's broken on macOS and Linux, both GCC and Clang. Any ideas? GameNetworking/NetworkSynchronizer#16 (comment)

If I move these back into the headers, it compiles fine: https://github.com/aaronfranke/godot/tree/node-rpc-header

[nathanfranke]

Opened #63820

[ClockRate]

Would this new addition allow for an interchangable transport implementation (usage of HLAPI but with custom low-level LAN/Thirdparty P2P transport as an example)? Because if it would, then this is a huge breakthrough, since there are only two other engines that provides this flexibility out of the box by having HLAPI seperate from interchangable LLAPI (LAN/Third-party P2P) transport implementation: Source and Unreal. In every other engine that is available right now you have to build your own complete (LLAPI + HLAPI) networking codebase from scratch (Which I had to do for Godot, Unity and a bunch of other engines because of this) around this missing low level piece that not a lot of people seem to realise is missing until you are trying to make a multiplayer like Left 4 Dead 2, where LAN and P2P are both valid working options for a play session.

[dsnopek]

@ClockRate It should always have been possible to implement a custom NetworkedMultiplayerPeer (Godot 3.x) or MultiplayerPeerExtension (Godot 4.x) in C++ via GDNative/GDExtension to allow you to use Steam's networking as the backend for Godot's High-Level Multiplayer API.

This change in this PR opens up the possibility for taking over more of what the High-Level Multiplayer API does, at a higher level. If we're eventually able to use this to assign something other than integers for peer ids, that would definitely make that process easier!

However, if you're looking for a way to implement a custom NetworkedMultiplayerPeer/MultiplayerPeerExtension from GDScript (rather than C++), that was recently added to both Godot 3 and 4 in these PRs:

• [3.x] Add NetworkedMultiplayerCustom so high-level multiplayer backends can be added from GDScript #63163
• [4.x] Allow extending MultiplayerPeerExtension from GDScript #63262

[ClockRate]

@dsnopek Thanks for the heads up, I tried using NetworkedMultiplayerPeer with GDNative in the past, but I could not figure out how to override some of the methods that were essential for implementation of the LLAPI (writing/reading packets), maybe stuff has changed in the 3.X branch since then, I'll give it a shot.