Added rpc_node_notfound signal to SceneTree and MultiplayerAPI

[alimalkhalifa]

rpc_node_notfound signal

Affected classes

• MultiplayerAPI
• SceneTree

Objective

I have added an rpc_node_notfound signal in SceneTree to make the multiplayer system more usable in real-life use cases. The main issue I keep facing is a Node Tree Desync between server and client, where the client will keep throwing Node Not Found exceptions when a node on the server uses an RPC. This is a typical problem for me with movement synchronization. I had found myself implementing multiplayer on a NetworkController node instead of on the game nodes themselves to avoid this issue.

The solution

I have added an rpc_node_notfound signal on the SceneTree which is emitted whenever the client receives an RPC for a node that it does not have. This allows you to connect the signal to a function that does an RPC to request missing nodes from the server.

Note

Please be gentle, this is my first PR to this project :) Let me know if I have overstepped somehow or if this PR is undesirable or if I can improve my PR somehow to be more in line with the project.

[akien-mga]

CC @Faless

[mhilbrunner]

Somewhat relevant: #24681

[zEh-]

Isn't it still spamming?

[zEh-]

Spam again?

[zEh-]

Is there anything left that's required to be able to use it purely on the MultiplayerAPI itself instead of the SceneTree? As it's the case when using CustomMultiplayer. SceneTree got a "ClassDB::bind_method" added - might that be missing?

[Faless]

Hi @alimalkhalifa , sorry about the very late reply.
This is interesting, but I think it's still far from the final solution.

There are at least 2 things to consider:

1. We likely need a more generic error signal, I don't think adding one signal for every possible error is a good solution.
   • I would probably split the errors 2 main signals. procol_error (malformed packets) and call_error (RPC/RSET error). Those signals should have a parameter specifying the error type (e.g. NODE_NOT_FOUND) and the relevant available informations (path to node, method name, args?).
2. I think errors should be suppressed if a method is connected to the signal, but I'm not sure how to do it.

Additionally, there's no need to modify SceneTree.
Accessing the Multiplayer API via SceneTree is going to be deprecated in 3.2 and likely completely dropped in 4.0

[akien-mga]

Moving to the next milestone as this won't be ready to merge for the imminent 3.2.

@alimalkhalifa see the review comments by @Faless above, there would be changes necessary.

[alimalkhalifa]

I had missed the reply.

I could use a bit of direction on where to implement this. I'll take a look and maybe make a new PR for 4.0 with a more well thought out solution

[realkotob]

@Faless The "node not found" errors can sometimes make the client hang if the server miscalculates and starts sending data before the client has finished populating all the game objects.

It can be worked around with some foresight, but preferably ~50 server-side objects should not cause the client to irretrievably hang, for a reasonably non-critical error.

Emitting signals from my experience also gets expensive on the main process thread (relative to direct method calls), with a large number of objects spamming emit causes a performance hit as well. Could it be possible to have a toggle in the project settings to ignore these errors and have them emitted?

[alimalkhalifa]

My work around has been to limit movement updates to a movement update node. It also consolidates the updates so that's a plus.

That gets rid of most issues.

[aaronfranke]

@alimalkhalifa Is this still desired? If so, it needs to be rebased on the latest master branch.

If not, abandoned pull requests will be closed in the future as announced here.

[alimalkhalifa]

@aaronfranke I will pose this question on Discord/IRC to see the general consensus. This is definitely something that can be handled in GDScript, but it is a lot of boiler plate for something that is a common issue. However, the counter argument is that different scenarios might require different solutions, and this is a non-issue in lobby->game type scenarios. That said, I still believe everyone will benefit from error signals on the NetworkPeer for node issues. Willing to give it another go from my end to sort out the above concerns and resubmit.

For the record, I forced a situation where a desync happens and causes a node to only exist on server. This is the output on client debug:

ERROR: _process_get_node: Failed to get cached path from RPC: Scene/@Doodad@2.
   At: core/io/multiplayer_api.cpp:272
ERROR: Invalid packet received. Requested node was not found.
   At: core/io/multiplayer_api.cpp:204
ERROR: Node not found: Scene/Doodad.
   At: scene/main/node.cpp:1381

[aaronfranke]

This PR has not received any new commits for over a year, closing.