document multiplayer replication classes, small changes to multiplayer spawner

6 files changed, 78 insertions, 17 deletions

diff --git a/modules/multiplayer/doc_classes/MultiplayerSpawner.xml b/modules/multiplayer/doc_classes/MultiplayerSpawner.xml
index 44ab34f52c..881796ed26 100644
--- a/modules/multiplayer/doc_classes/MultiplayerSpawner.xml
+++ b/modules/multiplayer/doc_classes/MultiplayerSpawner.xml
@@ -1,65 +1,83 @@
 <?xml version="1.0" encoding="UTF-8" ?>
 <class name="MultiplayerSpawner" inherits="Node" version="4.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../../../doc/class.xsd">
 	<brief_description>
+		Automatically replicates spawnable nodes from the authority to other multiplayer peers.
 	</brief_description>
 	<description>
-		This node uses [method MultiplayerAPI.object_configuration_add] to notify spawns passing the spawned node as the [code]object[/code] and itself as the [code]configuration[/code], and [method MultiplayerAPI.object_configuration_remove] to notify despawns in a similar way.
+		Spawnable scenes can be configured in the editor or through code (see [method add_spawnable_scene]).
+		Also supports custom node spawns through [method spawn], calling [method _spawn_custom] on all peers.
+
+		Internally, [MultiplayerSpawner] uses [method MultiplayerAPI.object_configuration_add] to notify spawns passing the spawned node as the [code]object[/code] and itself as the [code]configuration[/code], and [method MultiplayerAPI.object_configuration_remove] to notify despawns in a similar way.
 	</description>
 	<tutorials>
 	</tutorials>
 	<methods>
 		<method name="_spawn_custom" qualifiers="virtual">
-			<return type="Object" />
+			<return type="Node" />
 			<argument index="0" name="data" type="Variant" />
 			<description>
+				Method called on all peers when a custom spawn was requested by the authority using [method spawn]. Should return a [Node] that is not in the scene tree.
+
+				[b]Note:[/b] Spawned nodes should [b]not[/b] be added to the scene with `add_child`. This is done automatically.
 			</description>
 		</method>
 		<method name="add_spawnable_scene">
 			<return type="void" />
 			<argument index="0" name="path" type="String" />
 			<description>
+				Adds a scene path to spawnable scenes, making it automatically replicated from the multiplayer authority to other peers when added as children of the node pointed by [member spawn_path].
 			</description>
 		</method>
 		<method name="clear_spawnable_scenes">
 			<return type="void" />
 			<description>
+				Clears all spawnable scenes. Does not despawn existing instances on remote peers.
 			</description>
 		</method>
 		<method name="get_spawnable_scene" qualifiers="const">
 			<return type="String" />
-			<argument index="0" name="path" type="int" />
+			<argument index="0" name="index" type="int" />
 			<description>
+				Returns the spawnable scene path by index.
 			</description>
 		</method>
 		<method name="get_spawnable_scene_count" qualifiers="const">
 			<return type="int" />
 			<description>
+				Returns the count of spawnable scene paths.
 			</description>
 		</method>
 		<method name="spawn">
 			<return type="Node" />
 			<argument index="0" name="data" type="Variant" default="null" />
 			<description>
+				Requests a custom spawn, with [code]data[/code] passed to [method _spawn_custom] on all peers. Returns the locally spawned node instance already inside the scene tree, and added as a child of the node pointed by [member spawn_path].
+
+				[b]Note:[/b] Spawnable scenes are spawned automatically. [method spawn] is only needed for custom spawns.
 			</description>
 		</method>
 	</methods>
 	<members>
 		<member name="spawn_limit" type="int" setter="set_spawn_limit" getter="get_spawn_limit" default="0">
+			Maximum nodes that is allowed to be spawned by this spawner. Includes both spawnable scenes and custom spawns.
+
+			When set to [code]0[/code] (the default), there is no limit.
 		</member>
 		<member name="spawn_path" type="NodePath" setter="set_spawn_path" getter="get_spawn_path" default="NodePath(&quot;&quot;)">
+			Path to the spawn root. Spawnable scenes that are added as direct children are replicated to other peers.
 		</member>
 	</members>
 	<signals>
 		<signal name="despawned">
-			<argument index="0" name="scene_id" type="int" />
-			<argument index="1" name="node" type="Node" />
+			<argument index="0" name="node" type="Node" />
 			<description>
+				Emitted when a spawnable scene or custom spawn was despawned by the multiplayer authority. Only called on puppets.
 			</description>
 		</signal>
 		<signal name="spawned">
-			<argument index="0" name="scene_id" type="int" />
-			<argument index="1" name="node" type="Node" />
+			<argument index="0" name="node" type="Node" />
 			<description>
+				Emitted when a spawnable scene or custom spawn was spawned by the multiplayer authority. Only called on puppets.
 			</description>
 		</signal>
 	</signals>
diff --git a/modules/multiplayer/doc_classes/MultiplayerSynchronizer.xml b/modules/multiplayer/doc_classes/MultiplayerSynchronizer.xml
index ebd1b50201..a2ea64061c 100644
--- a/modules/multiplayer/doc_classes/MultiplayerSynchronizer.xml
+++ b/modules/multiplayer/doc_classes/MultiplayerSynchronizer.xml
@@ -1,9 +1,15 @@
 <?xml version="1.0" encoding="UTF-8" ?>
 <class name="MultiplayerSynchronizer" inherits="Node" version="4.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../../../doc/class.xsd">
 	<brief_description>
+		Synchronizes properties from the multiplayer authority to the remote peers.
 	</brief_description>
 	<description>
-		The [MultiplayerSynchronizer] uses [method MultiplayerAPI.object_configuration_add] to notify synchronization start passing the [Node] at [member root_path] as the [code]object[/code] and itself as the [code]configuration[/code], and uses [method MultiplayerAPI.object_configuration_remove] to notify synchronization end in a similar way.
+		By default, [MultiplayerSynchronizer] synchronizes configured properties to all peers.
+		Visiblity can be handled directly with [method set_visibility_for] or as-needed with [method add_visibility_filter] and [method update_visibility].
+
+		[MultiplayerSpawner]s will handle nodes according to visibility of synchronizers as long as the node at [member root_path] was spawned by one.
+
+		Internally, [MultiplayerSynchronizer] uses [method MultiplayerAPI.object_configuration_add] to notify synchronization start passing the [Node] at [member root_path] as the [code]object[/code] and itself as the [code]configuration[/code], and uses [method MultiplayerAPI.object_configuration_remove] to notify synchronization end in a similar way.
 	</description>
 	<tutorials>
 	</tutorials>
@@ -12,18 +18,23 @@
 			<return type="void" />
 			<argument index="0" name="filter" type="Callable" />
 			<description>
+				Adds a peer visibility filter for this synchronizer.
+
+				[code]filter[/code] should take a peer id [int] and return a [bool].
 			</description>
 		</method>
 		<method name="get_visibility_for" qualifiers="const">
 			<return type="bool" />
 			<argument index="0" name="peer" type="int" />
 			<description>
+				Queries the current visibility for peer [code]peer[/code].
 			</description>
 		</method>
 		<method name="remove_visibility_filter">
 			<return type="void" />
 			<argument index="0" name="filter" type="Callable" />
 			<description>
+				Removes a peer visiblity filter from this synchronizer.
 			</description>
 		</method>
 		<method name="set_visibility_for">
@@ -31,40 +42,52 @@
 			<argument index="0" name="peer" type="int" />
 			<argument index="1" name="visible" type="bool" />
 			<description>
+				Sets the visibility of [code]peer[/code] to [code]visible[/code]. If [code]peer[/code] is [code]0[/code], the value of [member public_visibility] will be updated instead.
 			</description>
 		</method>
 		<method name="update_visibility">
 			<return type="void" />
 			<argument index="0" name="for_peer" type="int" default="0" />
 			<description>
+				Updates the visibility of [code]peer[/code] according to visibility filters. If [code]peer[/code] is [code]0[/code] (the default), all peers' visibilties are updated.
 			</description>
 		</method>
 	</methods>
 	<members>
 		<member name="public_visibility" type="bool" setter="set_visibility_public" getter="is_visibility_public" default="true">
+			Whether synchronization should be visible to all peers by default. See [method set_visibility_for] and [method add_visibility_filter] for ways of configuring fine-grained visibility options.
 		</member>
 		<member name="replication_config" type="SceneReplicationConfig" setter="set_replication_config" getter="get_replication_config">
+			Resource containing which properties to synchronize.
 		</member>
 		<member name="replication_interval" type="float" setter="set_replication_interval" getter="get_replication_interval" default="0.0">
+			Time interval between synchronizes. When set to [code]0.0[/code] (the default), synchronizes happen every network process frame.
 		</member>
 		<member name="root_path" type="NodePath" setter="set_root_path" getter="get_root_path" default="NodePath(&quot;..&quot;)">
+			Node path that replicated properties are relative to.
+			If [member root_path] was spawned by a [MultiplayerSpawner], the node will be also be spawned and despawned based on this synchronizer visibility options.
 		</member>
 		<member name="visibility_update_mode" type="int" setter="set_visibility_update_mode" getter="get_visibility_update_mode" enum="MultiplayerSynchronizer.VisibilityUpdateMode" default="0">
+			Specifies when visibility filters are updated (see [enum VisibilityUpdateMode] for options).
 		</member>
 	</members>
 	<signals>
 		<signal name="visibility_changed">
 			<argument index="0" name="for_peer" type="int" />
 			<description>
+				Emitted when visibility of [code]for_peer[/code] is updated. See [method update_visibility].
 			</description>
 		</signal>
 	</signals>
 	<constants>
 		<constant name="VISIBILITY_PROCESS_IDLE" value="0" enum="VisibilityUpdateMode">
+			Visibility filters are updated every idle process frame.
 		</constant>
 		<constant name="VISIBILITY_PROCESS_PHYSICS" value="1" enum="VisibilityUpdateMode">
+			Visibility filters are updated every physics process frame.
 		</constant>
 		<constant name="VISIBILITY_PROCESS_NONE" value="2" enum="VisibilityUpdateMode">
+			Visibility filters are not updated automatically, and must be updated manually by calling [method update_visibility].
 		</constant>
 	</constants>
 </class>
diff --git a/modules/multiplayer/doc_classes/SceneReplicationConfig.xml b/modules/multiplayer/doc_classes/SceneReplicationConfig.xml
index 1d6dec2f92..fc91592c7a 100644
--- a/modules/multiplayer/doc_classes/SceneReplicationConfig.xml
+++ b/modules/multiplayer/doc_classes/SceneReplicationConfig.xml
@@ -1,6 +1,7 @@
 <?xml version="1.0" encoding="UTF-8" ?>
 <class name="SceneReplicationConfig" inherits="Resource" version="4.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../../../doc/class.xsd">
 	<brief_description>
+		Configuration for properties to synchronize with a [MultiplayerSynchronizer].
 	</brief_description>
 	<description>
 	</description>
@@ -12,35 +13,41 @@
 			<argument index="0" name="path" type="NodePath" />
 			<argument index="1" name="index" type="int" default="-1" />
 			<description>
+				Adds the property identified by the given [code]path[/code] to the list of the properties being synchronized, optionally passing an [code]index[/code].
 			</description>
 		</method>
 		<method name="get_properties" qualifiers="const">
 			<return type="NodePath[]" />
 			<description>
+				Returns a list of synchronized property [NodePath]s.
 			</description>
 		</method>
 		<method name="has_property" qualifiers="const">
 			<return type="bool" />
 			<argument index="0" name="path" type="NodePath" />
 			<description>
+				Returns whether the given [code]path[/code] is configured for synchronization.
 			</description>
 		</method>
 		<method name="property_get_index" qualifiers="const">
 			<return type="int" />
 			<argument index="0" name="path" type="NodePath" />
 			<description>
+				Finds the index of the given [code]path[/code].
 			</description>
 		</method>
 		<method name="property_get_spawn">
 			<return type="bool" />
 			<argument index="0" name="path" type="NodePath" />
 			<description>
+				Returns whether the property identified by the given [code]path[/code] is configured to be synchronized on spawn.
 			</description>
 		</method>
 		<method name="property_get_sync">
 			<return type="bool" />
 			<argument index="0" name="path" type="NodePath" />
 			<description>
+				Returns whether the property identified by the given [code]path[/code] is configured to be synchronized on process.
 			</description>
 		</method>
 		<method name="property_set_spawn">
@@ -48,6 +55,7 @@
 			<argument index="0" name="path" type="NodePath" />
 			<argument index="1" name="enabled" type="bool" />
 			<description>
+				Sets whether the property identified by the given [code]path[/code] is configured to be synchronized on spawn.
 			</description>
 		</method>
 		<method name="property_set_sync">
@@ -55,12 +63,14 @@
 			<argument index="0" name="path" type="NodePath" />
 			<argument index="1" name="enabled" type="bool" />
 			<description>
+				Sets whether the property identified by the given [code]path[/code] is configured to be synchronized on process.
 			</description>
 		</method>
 		<method name="remove_property">
 			<return type="void" />
 			<argument index="0" name="path" type="NodePath" />
 			<description>
+				Removes the property identified by the given [code]path[/code] from the configuration.
 			</description>
 		</method>
 	</methods>
diff --git a/modules/multiplayer/multiplayer_spawner.cpp b/modules/multiplayer/multiplayer_spawner.cpp
index f5edffbb0c..e8f3aecc69 100644
--- a/modules/multiplayer/multiplayer_spawner.cpp
+++ b/modules/multiplayer/multiplayer_spawner.cpp
@@ -126,7 +126,7 @@ void MultiplayerSpawner::_set_spawnable_scenes(const Vector<String> &p_scenes) {
 void MultiplayerSpawner::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("add_spawnable_scene", "path"), &MultiplayerSpawner::add_spawnable_scene);
 	ClassDB::bind_method(D_METHOD("get_spawnable_scene_count"), &MultiplayerSpawner::get_spawnable_scene_count);
-	ClassDB::bind_method(D_METHOD("get_spawnable_scene", "path"), &MultiplayerSpawner::get_spawnable_scene);
+	ClassDB::bind_method(D_METHOD("get_spawnable_scene", "index"), &MultiplayerSpawner::get_spawnable_scene);
 	ClassDB::bind_method(D_METHOD("clear_spawnable_scenes"), &MultiplayerSpawner::clear_spawnable_scenes);
 
 	ClassDB::bind_method(D_METHOD("_get_spawnable_scenes"), &MultiplayerSpawner::_get_spawnable_scenes);
@@ -146,8 +146,8 @@ void MultiplayerSpawner::_bind_methods() {
 
 	GDVIRTUAL_BIND(_spawn_custom, "data");
 
-	ADD_SIGNAL(MethodInfo("despawned", PropertyInfo(Variant::INT, "scene_id"), PropertyInfo(Variant::OBJECT, "node", PROPERTY_HINT_RESOURCE_TYPE, "Node")));
-	ADD_SIGNAL(MethodInfo("spawned", PropertyInfo(Variant::INT, "scene_id"), PropertyInfo(Variant::OBJECT, "node", PROPERTY_HINT_RESOURCE_TYPE, "Node")));
+	ADD_SIGNAL(MethodInfo("despawned", PropertyInfo(Variant::OBJECT, "node", PROPERTY_HINT_RESOURCE_TYPE, "Node")));
+	ADD_SIGNAL(MethodInfo("spawned", PropertyInfo(Variant::OBJECT, "node", PROPERTY_HINT_RESOURCE_TYPE, "Node")));
 }
 
 void MultiplayerSpawner::_update_spawn_node() {
@@ -277,12 +277,11 @@ Node *MultiplayerSpawner::instantiate_scene(int p_id) {
 
 Node *MultiplayerSpawner::instantiate_custom(const Variant &p_data) {
 	ERR_FAIL_COND_V_MSG(spawn_limit && spawn_limit <= tracked_nodes.size(), nullptr, "Spawn limit reached!");
-	Object *obj = nullptr;
 	Node *node = nullptr;
-	if (GDVIRTUAL_CALL(_spawn_custom, p_data, obj)) {
-		node = Object::cast_to<Node>(obj);
+	if (GDVIRTUAL_CALL(_spawn_custom, p_data, node)) {
+		return node;
 	}
-	return node;
+	ERR_FAIL_V_MSG(nullptr, "Method '_spawn_custom' is not implemented on this peer.");
 }
 
 Node *MultiplayerSpawner::spawn(const Variant &p_data) {
diff --git a/modules/multiplayer/multiplayer_spawner.h b/modules/multiplayer/multiplayer_spawner.h
index 80bb878a74..fc3befc2d4 100644
--- a/modules/multiplayer/multiplayer_spawner.h
+++ b/modules/multiplayer/multiplayer_spawner.h
@@ -91,7 +91,9 @@ protected:
 	void _get_property_list(List<PropertyInfo> *p_list) const;
 #endif
 public:
-	Node *get_spawn_node() const { return spawn_node.is_valid() ? Object::cast_to<Node>(ObjectDB::get_instance(spawn_node)) : nullptr; }
+	Node *get_spawn_node() const {
+		return spawn_node.is_valid() ? Object::cast_to<Node>(ObjectDB::get_instance(spawn_node)) : nullptr;
+	}
 
 	void add_spawnable_scene(const String &p_path);
 	int get_spawnable_scene_count() const;
@@ -110,7 +112,7 @@ public:
 	Node *instantiate_custom(const Variant &p_data);
 	Node *instantiate_scene(int p_idx);
 
-	GDVIRTUAL1R(Object *, _spawn_custom, const Variant &);
+	GDVIRTUAL1R(Node *, _spawn_custom, const Variant &);
 
 	MultiplayerSpawner() {}
 };
diff --git a/modules/multiplayer/scene_replication_interface.cpp b/modules/multiplayer/scene_replication_interface.cpp
index c89270fbe8..6e3dbfab47 100644
--- a/modules/multiplayer/scene_replication_interface.cpp
+++ b/modules/multiplayer/scene_replication_interface.cpp
@@ -396,6 +396,8 @@ Error SceneReplicationInterface::on_spawn_receive(int p_from, const uint8_t *p_b
 		pending_buffer_size = state_len;
 	}
 	parent->add_child(node);
+	spawner->emit_signal(SNAME("spawned"), node);
+
 	pending_spawn = ObjectID();
 	pending_buffer = nullptr;
 	pending_buffer_size = 0;
@@ -411,10 +413,17 @@ Error SceneReplicationInterface::on_despawn_receive(int p_from, const uint8_t *p
 	Error err = rep_state->peer_del_remote(p_from, net_id, &node);
 	ERR_FAIL_COND_V(err != OK, err);
 	ERR_FAIL_COND_V(!node, ERR_BUG);
+
+	MultiplayerSpawner *spawner = rep_state->get_spawner(node->get_instance_id());
+	ERR_FAIL_COND_V(!spawner, ERR_DOES_NOT_EXIST);
+	ERR_FAIL_COND_V(p_from != spawner->get_multiplayer_authority(), ERR_UNAUTHORIZED);
+
 	if (node->get_parent() != nullptr) {
 		node->get_parent()->remove_child(node);
 	}
 	node->queue_delete();
+	spawner->emit_signal(SNAME("despawned"), node);
+
 	return OK;
 }