godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.15k stars 20.21k forks source link

Global RPC manager #24681

Closed ghost closed 4 years ago

ghost commented 5 years ago

Godot version: 3.1.dev git OS/device including version: Gentoo Linux 4.20 kernel

When separating client & server logic codes in different projects, we have to make sure we are sending and receiving RPCs from the same path. However, some players might want to reverse the networking protocol or send garbage data, which will result in a lot of errors serverside (no crash but still almost freezing the server). If we have a GDScript function (or something similar) "bool accept_rpc(int from_id, string rpc_function_name, string to_path)" we could handle this ourselves by dismissing malformed RPCs without flooding the terminal.

zEh- commented 5 years ago

I ran into the same problem. On the server I need to detect potential attacks. On the client I want my log to not get spammed because I receive updates on objects which do not exist yet.

Both would be solved by a callback like proposed by @myrage2000. (But where to put it, as the node may not be even found). My proposal is a signal on SceneTree like "received_invalid_rpc" when a node path is not found or the mode doesn't allow a call. (Ignoring rset here because I don't use them). That way it can either be ignored (by default) altogether(on the client) or detect potential attacks(on the server).

Otherwise I would need to implement a handshake to let my server know which nodes a client already set up which I'm currently refusing to do, as it complicates everything - Only to not get my log spammed and I still wouldn't be able to detect anything.

@Faless What do you think?

alimalkhalifa commented 5 years ago

My pull request at #23893 is relevant as @mhilbrunner has mentioned

menip commented 5 years ago

~Receiving rpc calls on objects that don't exist is not engine issue, but a logic issue in code that can be fixed.~

However, malicious users are still valid concern.

Edit: never mind me, forgot about obvious issues as result of async communication.

Faless commented 5 years ago

There is already a Global RPC manager, it's the MultiplayerAPI class, and can be accessed via the Node.multiplayer property (or the soon to be deprecated get_tree(),multiplayer). Signals should be added to that class, see this comment .

clayjohn commented 4 years ago

Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.

The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.

If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!