Puppet functions callable locally from non-network master

[tomsharratt]

Godot version: 3.2.3
OS/device including version: Windows 10
Issue description: The puppet and puppetsync annotated functions still receive calls from other puppet clients, not just the master.

According to this line:

godot/core/io/multiplayer_api.cpp

Lines 92 to 94 in 00949f0

	case MultiplayerAPI::RPC_MODE_PUPPETSYNC:
	case MultiplayerAPI::RPC_MODE_PUPPET: {
	return !p_node->is_network_master() && p_remote_id == p_node->get_network_master();

puppet functions should only be callable from the network master but this is not the case.

Steps to reproduce:
Create a puppet function, try call an rpc from the server - this will work.
Try call this same puppet function from a client which isn't the network master - it will still call the puppet function

Example code:

puppet func puppet_rpc():
	print('rpc_sender: %d' % get_tree().get_rpc_sender_id())
	print('network master: %d' % get_network_master())

If I call this from the master (server) and then a client this is what I see in the logs of the client:

rpc_sender: 1
network master: 1
rpc_sender: 510900438
network master: 1

I would not expect the second call to happen as the the sender id != master id

[Calinou]

cc @Faless

[Faless]

@tomsharratt can you provide a minimum reproduction project? i.e. the project you used to generate that output?

[tomsharratt]

PuppetBug.zip
Here you go @Faless

If you start two copies up and have one create_client and the other create_server. Press the button on the server one first and you'll see it work as expected:

Client gets both rpcs called and the server just has the puppetsync called.

However, if you press the button on the client you'll see it doesn't get called on the server (this is correct) but then it does get both puppet and puppetsync called on the client you just sent the rpc from (this isn't correct? you shouldn't be accepting calls where the remote_id is not equal to the network master id?)

[Faless]

However, if you press the button on the client you'll see it doesn't get called on the server (this is correct) but then it does get both puppet and puppetsync called on the client

Yes, they are both called locally, but never accepted from the network (i.e. other connected clients will not call that function).
I can see how this is misleading, but the error here is that you should not call that RPC from the client, and if you do, you will notice it is not called across the network (spitting an error), and thus only the misbehaving client goes out of sync.

We could add extra checks that prints error and skip the call when the client attempts to call a function that it's not allowed to on itself.

[tomsharratt]

Totally agree with everything you said. I guess it would just be nice, like you said, to have it spit an error instead of running it locally and having that sync break. I found this issue because I accidentally coded in an RPC call from a client and it would be nice to have something warn me that I'm trying to call an RPC that I shouldn't be.