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

Unable to set multiplayer authority in _enter_tree #75067

Open
sygi opened this issue Mar 18, 2023 · 12 comments
Open

Unable to set multiplayer authority in _enter_tree #75067

sygi opened this issue Mar 18, 2023 · 12 comments

Comments

@sygi
Copy link
Contributor

sygi commented Mar 18, 2023

Godot version

4.0-stable

System information

Ubuntu 22.04

Issue description

I am trying to do a fairly standard p2p networking game with the new, Godot 4 networking system: two players, each controlling one character and synchronizing its position to the other one.

I have the following code in /root/players:

func connect_me():
    multiplayer.peer_connected.connect(add_player)

func add_player(id: int):
    var character = preload("res://players/player.tscn").instantiate()
    character.player = id
    character.name = str(id)
    add_child(character, true)

Each player instance has a MultiplayerSynchronizer, and a MultiplayerSpawner spawning players/player.tscn under /root/players. Once the "server" gets connected, it calls:

players.add_player(1)
players.connect_me()

This works, but doesn't transfer the multiplayer authority to the client node. I tried adding set_multiplayer_authority(player) call in:

  1. Character's _ready()
  2. Character's _enter_tree()
  3. players' _on_multiplayer_spawner_spawned
  4. after adding the child character in players

The first one gives an error:

on_replication_start: The MultiplayerSynchronizer at path "/root/root/549606282/MultiplayerSynchronizer" is unable to process the pending spawn since it has no network ID. This might happen when changing the multiplayer authority during the "_ready" callback. Make sure to only change the authority of multiplayer synchronizers during "_enter_tree" or the "_spawn_custom" callback of their multiplayer spawner

which is fair, however when I move the set_multiplayer_authority call to _enter_tree, the same error appears. To be able to set it on both sides, I needed to call it in 3 (for the client) and 4 (for the server).

It seems that the error's suggestion is wrong, and setting the multiplayer authority in _enter_tree doesn't work in all cases.

Steps to reproduce

Reproduce the simple node structure above. See reproduction project below.

Minimal reproduction project

networking_repro.tar.gz

@sygi
Copy link
Contributor Author

sygi commented Mar 18, 2023

I think I figured out why my code doesn't work, but I am still unclear on how to fix it. If I understand correctly (I'd be grateful if someone could confirm), the order of events during creation of a spawn is:
0. the name of the node is synchronized, always

  1. _enter_tree
  2. spawn synchronization of parameters starts
  3. _ready
  4. spawn synchronization of parameters ends

As I understand, changing multiplayer authority is forbidden after spawn synchronization of parameters starts until after _ready is finished. This is unfortunate, as it makes it impossible to set the authority at the right time:

  1. If I set it in _enter_tree, I: a) can't rely on value of player, as it's not yet synchronized, but even if I use the node name, b) the spawn synchronization won't work, as the server will lose the authority before the opportunity to synchronize the parameters on spawn.
  2. If I try to do it in _ready, I'll get an error that the spawn synchronization is in progress, and changing the authority now will make it lose access.
  3. If I try to do this in the player setter, I get the same error.

I found the solution that I had above (setting the authority independently on the server after ready and in the _spawn_custom on the client a bit hacky, as I shouldn't have to do this in two places. Is there a better solution?

If not, can we hide this behind an API more, so that setting the authority is more of a first-class citizen, eg. to something like:

var player = Player.instantiate()
player.authority_on_spawn = id
add_child(player)

which would simulate the above, ie. add the player to the tree, synchronize the spawn parameters, and set the authority on both ends once the synchronization is finished?

@Faless
Copy link
Collaborator

Faless commented Mar 19, 2023

Is there a better solution?

The quickest solution is to use the node name, you already do character.name = str(id) so in _enter_tree you can just do:

func _enter_tree():
  $sync.set_multiplayer_authority(str(name).to_int())

You can alternatively use a second synchronizer (with authority = 1) to set the spawn parameters in case you want the server to control the spawning process and pass the control of the other synchronizer to the client.
There is an example of that option in the blog post: https://godotengine.org/article/multiplayer-in-godot-4-0-scene-replication/

@sygi
Copy link
Contributor Author

sygi commented Mar 19, 2023

Thanks! Setting the multiplayer authority based on the name won't work, as the properties won't be synchronized on spawn (as the authority will be lost before the parameters are synced).

I'm not sure I get how the other solution would work. The tutorial splits the node into two parts: most of it stays under the authority of the server, and only the keystrokes have the authority changed. How would this work in my case, where I'd like to pass the authority over the whole player's node?

Are you saying I can have one global MultiplayerSynchronizer that only synchronizes "at spawn" and never changes authority from the server, and another one as it is (under player) that synchronizes at sync? I tried setting it up, but the editor's UI doesn't allow me to set the synchronization for MultiplayerSynchronizer before the player nodes have instantiated.

Even if I managed to make it work (curious to see how it'd look like), setting two synchronizers sounds hacky: in my mind, changing the authority is a fundamental feature, and using it shouldn't require passing the client id via node name, rearranging the node tree, or breaking the synchronizer's contract.

@Faless
Copy link
Collaborator

Faless commented Mar 19, 2023

as the properties won't be synchronized on spawn (as the authority will be lost before the parameters are synced).

The name is set by the spawner and not the synchronizer, so that will be correctly set (if you use the spawner).

Even if I managed to make it work (curious to see how it'd look like),

The blog post include the link to the full project if you want to check it out: https://godotengine.org/storage/blog/multiplayer-in-godot-4-0-scene-replication/project.zip

@sygi
Copy link
Contributor Author

sygi commented Mar 19, 2023

The name is set by the spawner and not the synchronizer, so that will be correctly set (if you use the spawner).

Yes, the name will be, but all the other properties (like :position) won't.

I understand how the project from the tutorial works, but I'm not sure how to apply this in the case we're discussing: where full nodes (in particular with state that needs to be synchronized on spawn) can be replicated. and get their authority transferred.

@chrisl8
Copy link
Contributor

chrisl8 commented Sep 12, 2023

@sygi Have you come up with any further understanding on this?

I am running into the same issue, and everything you have said resonates with the way it appears to work to me as well.

The demo assumes that the server will be the authority for the player character, but attempting to set the authority to the player themselves produces this strange error that sends me down a wrong path.

It is strange that it suggests using _enter_tree when that runs long before _ready and doesn't seem like it would ever work. It certainly does not work for me.
Using the trigger on the MultiplayerSpawner works for the clients, but I haven't found a consistently reliable way to set it for the server itself. There seems to be some timing issue involved. Sometimes just setting it to the player after adding it works and other times it complains.

@Cammymoop
Copy link
Contributor

I have a (somewhat) reliable workaround that I came up with making This silly tank game

Part of the complexity is because I need to handle players joining when any number of players are active and I also made ownership transferable over time but this is just the part that does the initial ownership.
Also IDK how much of this is actually required.

Firstly I set the ownership of the player node (which is spawned from a MultiplayerSpawner) using call_deferred from it's _ready() method. I only call this from the client who is taking control, but I use an rpc to do the actual mp authority update so the other peers all update it at the same time.

But in order for the client to know that the player node is their own player node in _ready I make use of a MultiplayerSyncronizer with some replications enabled only for spawn to synchronize some values I need at spawn time, I call it SpawnSynchronizer and I make sure it's mp authority is always set to 1, which in my case is always the authority of the spawner node but I think it just needs to be the same as the spawner that spawns it. Setting it to something else later seems to be no good either so just leave it alone. So when a player joins mid-game it can spawn the other player nodes and immediately know what their authority is, so it can set them to the correct authority (again with a deferred call, but this time without the rpc). And also know that their own player node is actually themselves.

Since I do want to synchronize stuff during gameplay I handle that with another synchronizer node (both of them are children of the spawned player node) This one I just call "Synchronizer" and only has "Sync" replications, no "Spawn" replications. It will actually cause problems for the spawn synchronization code if it exists and has the same mp authority as the spawner when it tries to deserialize the spawn values and finds this extra synchronizer. I avoid this by immediately setting the mp authority on the "Synchronizer" to the id of the target player before returning it from the custom spawning function in my spawner script.

Finally, that Synchronizer is a bit too eager and starts sending things to other peers before they've noticed what it's mp authority should be or something like that, which was getting me some errors in the log but I don't remember whether or not it was causing actual bugs. But regardless I fixed that by having each peer keep track of all the connected peers that were "ready" to receive synchronization data, and I used a visibility filter that I add to "Synchronizer" inside the _ready of the player node so by default it wasn't sending anything to any of them. When any peer got notified of a new peer connecting it sends a rpc_id back to that client with it's own peer_id so it adds it to the list of peers that are "ready" to get synchronization data.

That seemed to fix the majority of problems I was having but again I don't actually know how much of it is strictly necessary, especially if you require players to be connected before starting a game (and spawning the player nodes). And also mine was over complicated by the fact I was later changing the authority again in certain conditions which came with another carton of worm-cans that I wont get into.

@Sch1nken
Copy link
Contributor

I've just come across this issue in my own project. Currently I don't have any good ideas on how to implement it (other than maybe pass all current attribute values over to the other peers when the MultiplayerSpawner does its job).

This could also help with setting a "spawn position" and similiar things.

@vorletzter
Copy link

While doing a simple Learning Project i ran into the same issue.
Just leaving it here, because this Issue pops up, when googling :)

My goal was to keep the Authority (as in the the Movement) completely on the Client side.

Using @Faless approach i got it working.

Using the https://godotengine.org/storage/blog/multiplayer-in-godot-4-0-scene-replication/project.zip as base, i just added added the _enter_tree() func to the player_scene and removed the InputSync from the Scene.

In my Project i renamed "player"to player_id, because i got confused in the beginning.

func _enter_tree():
	# We set the authority after just before the player enters the scene
	player_id = str(name).to_int()
	$MultiplayerSynchronizer.set_multiplayer_authority(str(name).to_int())

and removed the authority line from the player/player_id setter.
After changing the Properties for the remaining Synchronizer to
position (spawn:True and Always) and "player_id" (was player in the example) to "spawn:True and Never" and some cleaning up/removing/rewiring/ everything worked as expected.

No two Sync needed.

@dmitrijbes
Copy link

Stumbled upon the same issue and solved it using ready signal, which is called after _ready and so properties are already synced.
Kudos to @sygi for a great research, which helped a lot.

func _on_ready():
	set_multiplayer_authority(peer_id)

func _ready():
	self.ready.connect(_on_ready)

@RichardEllicott
Copy link

RichardEllicott commented May 15, 2024

i found this issue to, it would be nice if the Spawner could handle this automaticly, if you set it before using add_child

Faless answer woked for me:

func _enter_tree():
  $sync.set_multiplayer_authority(str(name).to_int())

but also since you probabally want to write a spawner or wrapper anyway, to get rpc signals, you could use the signal child_entered_tree.... this is great as you don't need to write the code in each node, it will get run on anything that is added to the tree

@tomytomy1020
Copy link

Stumbled upon the same issue and solved it using ready signal, which is called after _ready and so properties are already synced. Kudos to @sygi for a great research, which helped a lot.

func _on_ready():
set_multiplayer_authority(peer_id)

func _ready():
self.ready.connect(_on_ready)


many thanks, your solution works for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants