-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
Support returning values from rpc_id()
calls or add something like rpc_id_with_return()
#2032
Comments
rpc_id()
calls or add something like rpc_id_with_return()
I have experience building multiplayer games with dedicated servers and i find the premise of this proposal flawed. There are actually very few instances in multiplayer games where having the server resoond directly to a clientbrequest makes sense--mostly due to the need to limit bandwidth and data usage. In fact, your example of authentication is also flawed. Authentication should be implemented using RPCs at all. A proper architecture would contain a separate lightweight auth server that worked over secure http using dome well estavlished orotocal to generate some auth token that would be used in subsequent requests for per request authN and authZ. I'm not totally agaibst the idea of supporting returning data from rpcs but it significantly complicates the implementation because rpcs are asynchronous in nature. If we were to implement sometging like this we'd need a really good use case or reason to do so and I can't think if a single one off the top of my head (not to say they don't exist) |
Maybe @Faless has some other thoughts here? |
@jonbonazza As stated in the OP, the code examples are the simplest possible to illustrate the need. They aren't intended to show best practice, just illustrate a use case where the ability for an RPC call to return values will greatly simplify server-side and client-side code. You are correct that using HTTPS for request/response is an option for authentication and maybe other functions. However, that requires adding a web-server to the server-side tech stack and the complexity of interfacing HTTPS requests with Godot coded servers. You point concerning bandwidth usage is valid if the rpc-with-return calls are used improperly. If available, they should only be used when the client cannot change state until a response from the server is received. That is why I used player authentication as an example. Additionally, HTTPS is one of the least performant protocols in existence due its bloated syntax. Using it for all request/response use cases will create far more bandwidth/data usage than an RPC call which serializes data. In my mind, without some type of simple request/response support in RPC, it will always be limited to use in small peer-to-peer games running on the same sub-net. |
I think there's a actually a decent number of instances where this would be useful. A few examples in the context of MMOs like World of Warcraft:
These are all examples, where the change to the client state should wait on server confirmation as latency isn't critical and as it would be very confusing to do a rollback of it afterwards. Allowing RPC calls to return values is simpler and more intuitive (see code examples by @tx350z). As the codebase grows, it's really nice for the developer to keep the logic in a single function instead of having to split part of it into a separate callback function. All developers understand that functions can return values, but many newcomers haven't used callbacks much and understandably get a bit confused by the concept. If the RPC API changes to a single keyword/function as proposed in #1939 (comment), then we wouldn't need to introduce |
The example code given here couldn't work exactly as written, without the game execution grinding to a halt when the RPC call is made, and without handling the possibility that the RPC call would fail or never return. Instead, you'd need to do something like:
Note the addition of the So, while it's maybe a bit simpler for the developer, it's doesn't really get that simple. I also share @jonbonazza's feeling that this won't really be useful very often. In the networked games I've made with Godot, I've never needed to have an RPC (at least with Godot's High-level Multiplayer API) return a value. In my games, I usually have two parts to the server side: (1) a part that handles authentication, matchmaking, user profiles, etc (for which I use Nakama) and (2) an instance of Godot that handles sync'ing of the real-time game via Godot's High-level Multiplayer API. The clients communicate with Nakama over HTTPS, and those calls have return values (but not direct ones, of course, because communicating over HTTP also needs to use signals or co-routines to not pause execution of the game, like described above) but the actual real-time game synchronization doesn't need to have return values. In my personal opinion, in the uncommon case of needing to have RPC's with return values, it can be implemented the way the OP is currently doing it. |
I actually don't think an RPC returning a value would work in almost any of these cases. Keep in mind that an RPC returning a value means calling a single method and returning a value by the end of the method. If the method needs to do anything asynchronously to achieve its result (ie. where it'd need to yield control to the engine, in order to draw something on the screen or send/receive other network requests, etc), then it won't be done by the end of the method, and can't just a return a value, because it doesn't have the result yet or isn't finished yet. I'll walk through an imaginary implementation of a couple of these examples below... Entering a dungeon
I don't see anywhere in there that an RPC could have returned a value, because all the operations initiated by the RPC on both the client and server needed to be done asynchronously to allow the client or server to keep running after the RPC call was made. Joining a group/party/guild of other players
Again, since so much needed to happen asynchronously, none of those RPC's could have return values. |
I was writing a comment about the asynchronous requirements of such a feature but I see @dsnopek already explained it quite well. In general, I too think that this is not worth it, given that it won't be commonly used, might end up leading developers into pitfalls, and when a result is actually needed, it can be worked around in GDScript (even as a reusable component with it's own semantics if needed). |
Thanks for the detailed response @dsnopek! I completely agree that this would need to be implemented with yield (or await in 4.0) and a configurable timeout. I might not have explained my examples clearly enough - the RPCs would return a quick success/error, not wait for other players' interactions. For instance, entering a dungeon would respond success along with any info needed to start streaming the dungeon data (your step 2) or respond with an error (e.g. "a party member is offline"). Similarly, joining a party would respond success and show the dialog to the party leader (your step 2) or respond with an error (e.g. "party is full"). Other common errors are due to race conditions such as "item already looted" or "NPC is busy". Handling errors and race conditions is surprisingly painful. If Godot doesn't have a way to return success/error, then you'd need extra server-callable functions on the client for each RPC that requires the server to communicate the response. More importantly, the developer then needs to handle the complexity of timeouts, the response function somehow called 2+ times at the same time, and many other weird cases. I agree that simple games could use a separate HTTPS server (although that adds complexity too). However, that wouldn't work for any multiplayer game, where the game server needs to communicate in-game success/error responses. All this being said, I think it's completely fair if this is out of scope for core :) |
This is just untrue. Pretty much every major MMO or FPS uses rpcs without return values. As I mentioned, there just arent very many cases where they are useful, and there is generally always a better alternative. The use cases that @bfelbo outlined are also not good ones for various reasons, sone of which having already been mentioned. |
Reading the responses I realize that the rabbit got lost along the way. Perhaps a more complicated example would help explain the need? I also realize that even if the capability for an rpc_id call to return a value (which is what my request is about) was to be implemented, it would not be available soon enough to meet my time line. Therefore, I'm closing the request. For my needs I've dedicated to create a server connector class that encapsulates a combination of RPC calls for push type communications (good application for RPC) and TCP connections that natively support request/response type communications. Thanks all. |
I don't think this should be closed. While it is an uncommon use case, there are cases where the resulting code is much more readable with this feature. In addition, there is no technical limitation preventing this from happening. Fetching chunks exampleClient requests chunk data for some Vector3 position. Workaround is trivial, since usually the logic for rendering the loaded chunk is self-contained. Current code needed: @rpc(any_peer) func fetch_chunk(pos: Vector3) -> void:
receive_chunk.rpc_id(get_multiplayer().get_remote_sender_id(), pos, PackedByteArray([0, 1, 2, 3]))
@rpc func receive_chunk(pos: Vector3, data: PackedByteArray) -> void:
pass # Chunk loading logic.
func test() -> void:
fetch_chunk(Vector3(0, 1, 2)) Proposed alternative: @rpc(any_peer func fetch_chunk(pos: Vector3) -> PackedByteArray:
return PackedByteArray([0, 1, 2, 3])
func test() -> void:
var data = await fetch_chunk(Vector3(0, 1, 2))
# Chunk loading logic Complex f(x) exampleClient requests a function result given an input, where the result needs to be passed to the same function that made the call. Current code needed: signal hash_value_received(id, result)
@rpc(any_peer) func hash_value(id: int, input: String) -> void:
# Pretend this is an expensive or secret function call. Calling on the client is either not feasible or not possible.
var result := input.hash()
receive_hash_value.rpc_id(get_multiplayer().get_remote_sender_id(), id, result)
@rpc func receive_hash_value(id: int, result: int) -> void:
hash_value_received.emit(id, result)
func test() -> void:
# var dialog = HelperClass.open_dialog("Please input some text")
var id := randi()
hash_value.rpc(id, "hello")
var result
while true:
var r = await hash_value_received
if r[0] == id:
result = r[1]
break
# This code needs to be here if we need some context from the last call, such as closing a dialog.
# dialog.queue_free()
print("Result is ", result) Proposed alternative: @rpc(any_peer) func hash_value(input: String) -> int:
# Pretend this is an expensive or secret function call. Calling on the client is either not feasible or not possible.
var result := input.hash()
return result
func test() -> void:
# var dialog = HelperClass.open_dialog("Please input some text")
var result = await hash_value("hello")
# This code needs to be here if we need some context from the last call, such as closing a dialog.
# dialog.queue_free()
print("Result is ", result) |
@nathanfranke How do we handle the "what if the server never responds" case with your co-routine proposal? |
It will just wait forever. Cases where this would happen:
Edit: So we'll need some safety, hmm |
|
Also:
Personally, I don't think we should introduce some engine-level networking functionality, where the developer can't provide a nice way for their game to handle network issues (not to mention the other reasons it may never respond). With what we have currently, the developer can (and should!) setup a system where they start a timer when making an RPC that they expect to followed by another RPC in response. And if it times out without a response, they can take some action that makes sense in context. Maybe they retry a couple times? Maybe they show an error message and ask the player to try again later? An addon to manage this could be created. I suppose they could still do that with your proposal, but it's not ideal. What if the server does end up responding, but way after the timeout? Then the developer needs to know to check some flag after the But mainly, I don't think we want to create an API that gives the illusion that the developer can depend on this, when in the real world, they can't. |
Network issues shouldn't be an issue with reliable rpc, and I don't think this should be supported with unreliable RPC.
I don't agree that the solution to X being potentially insecure is to rely on game developers to implement X. |
Network issues can still cause reliable RPC's to not arrive. |
I'll chime in since I wrote the OP. IMO, RPC as implemented is fine for smallish games where every node contains the entire game logic (where things like master, puppet, etc. make sense). Again IMO, the current RPC model does not easily support "ecosystem scale" MMO applications. But that is OK, most games will never be truly MMO. In the end I chose not to use RPC since it doesn't meet my needs. I chose instead to use mid-level communications with threaded classes, FIFO queues, custom event notification, etc. Works well, reliable, and superior error handling. However it requires 10x the amount of code with functional logic splattered across a large number of classes. |
So, I'm re-opening for now to keep the discussion going, even if I still don't think we should implement this feature, at least not in the short run. Some details on a potential implementation follow: First things, Godot doesn't really have the concept of co-routines, that's a GDScript thing, it's not exposed at Godot/ClassDB level. @rpc(any_peer func fetch_chunk(pos: Vector3) -> PackedByteArray:
return PackedByteArray([0, 1, 2, 3])
func test() -> void:
var result = fetch_chunk.rpc(Vector3(0, 1, 2))
await result.completed
if result.is_error():
print(result.error)
return
var data = result.data
# Chunk loading logic Then, to make the system kind of safe, we need to:
This is quite the work, and as I mentioned, I think the use case is limited, still, if we want to give this a try, all this should be implementable in GDScript. The RPCs will be a bit less performant, but given the use case I don't think that's a deal breaker. |
What is an example error message? I don't think explicit error handling is worth it if the only cause is a malicious client or engine bug. It would be better to have the errors in the engine with ERR_FAIL_etc. func test() -> void:
var result = await my_rpc.rpc()
# Is it possible to automatically "return" on an error? Edit: And if it isn't possible, we shouldn't immediately go to "how to work around this" but rather how to keep this intuitive for users, even if it involves modifying GDScript.
If |
Not true, if so should be documented
|
That would be exception handling. So no, it's not supported in GDScript.
Yeah, It doesn't have to be a message just detecting the error is fine (though differentiating between timeout and wrong return type might be nice): func test() -> void:
var result = fetch_chunk.rpc(Vector3(0, 1, 2)).completed
# await result.completed # EDIT: We could skip this and do it above, but will need to be careful about reference count
if result.is_error():
return
var data = result.data
# Chunk loading logic
Yeah, wrong terminology, let me rephrase: "how would it handle broadcast/multicast? I guess only unicast make sense?." |
Responding to @nathanfranke:
Imagine you send a reliable RPC, and then the user loses their internet connection. There's nothing that Godot can do to make the RPC arrive. If you're using ENet, for example, it won't immediately know that there is no connection because it uses UDP (a connectionless protocol), but it does have a mechanism to eventually detect that messages are no longer arriving. At that point, the But in the interim, between when the internet connection is lost and ENet figures that out, you could send any number of reliable RPCs, and they won't arrive.
Heh, yeah, that probobly should be changed! Godot does not possess any special magic that can make an RPC arrive "no matter what" :-) |
I create rpc with returnig. By callbacks and without waiting at first need to add it class to autoload. It generic rpc func
message is int, but you can change it to String if you want. Also data has type Dictionary, but maybe PackedByteArray is better also need this class for configuring callbacks and serializing custom messages
also need message types that extend Message Login message
login result message (in my case need only name)
also need configuring messages before using. For example in some autoloaded script
Usage Login screen
and server processing
What happened Client creates message with data for authorization and subscribes to response Server subscribes to message with authorization data and returns value (type that extends message too) Additional, when server creates response message, it can subscribe to response of response :) But this way have problems. It don't support channels, security ( But may be it can be solved. |
I'm building a server-authoritative system with WebRTC, where the server is simply the multiplayer authority in the network. When a client wants to emit an event, I need to be able to indicate whether the event is accepted or rejected by the server. Without return values on the RPC calls, I'd have to set up my own signalling system to resolve and reject different events on the emitting client. |
Describe the project you are working on
MMO game with scalable dedicated-server framework.
Describe the problem or limitation you are having in your project
Godot's RPC networking is great for small peer-to-peer applications. There are a few limitations that make games needing dedicated servers complicated. I have many cases where I need to make a request to a server and have a result returned. The most obvious example is user authentication shown in the code snippets below.
Describe the feature / enhancement and how it helps to overcome the problem or limitation
Currently rpc calls do not return values and I understand the reasoning. However, since rpc_id() is a call to a specific peer, there should be no case where multiple peers return values from that single call. An alternative is to have a separate "rpc_id_with_return" function or something similar.
In my application, there are multiple servers, each responsible for a "functional domain" such as user authentication, lobby, arena management, and multiple arenas ("arena" is the game play environment). The servers may be spread across multiple physical hosts and are managed by a "server_manager" server (one instance on each physical host). As the user moves through login, to lobby, to game play, the client disconnects from one server and connects to another server. The specific instance of each server the client connects to is assigned by the server_manager. For security reasons, the client never connects directly to the server manager. Only the functional domain servers communicate with the server_manager.
A very stripped down version of what I'm doing now is shown below. The client sends a user authentication request to the authentication server. If the authentication is successful, the authentication server sends a request to the server_manager for a lobby_server assignment. The authentication results along with the lobby_server assignment are then returned to the client.
DISCLAIMER: I haven't tried running this code since it is just a fraction of my actual client and server logic.
Current Client Code:
Current Server Code:
Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams
This is what I would like to have.
Client Code:
Server Code:
If this enhancement will not be used often, can it be worked around with a few lines of script?
There is a work around as shown above. However, it is far more than a few lines of code. And note that because the rpc_id call and the callback are decoupled, additional logic must be added to detect the failure to receive a response to the requests.
Is there a reason why this should be core and not an add-on in the asset library?
I don't believe this can be an add-on since the RPC networking is tightly integrated into the engine.
The text was updated successfully, but these errors were encountered: