-
-
Notifications
You must be signed in to change notification settings - Fork 212
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
@rpc
support and examples
#784
Comments
We don't have such examples because there's no dedicated RPC support in gdext yet. Would you be interested in participating in a design discussion about |
Sure. I personally think this could be indeed almost the same macro as in gdnative's RPC, though with required (debatable) fields like packet reliability, remote/local code execution and channel ID. I'm new to rust's macros in general, though I think something like this could be possible: #[func(rpc=("authority", "reliable", "remote", 1))]
fn ping(&mut self); (Might not be convenient to paste default values ("authority" for example) each time though) |
For what it's worth, if you really need RPC right now, it is technically possible to do, it's just not type-checked very well. I think I found a thread in the Discord discussing how to do it, but the TLDR is that you can use code like this in your self.base_mut().rpc_config(
"proxy_request_to_server_internal".into(),
dict! {
"rpc_mode": RpcMode::ANY_PEER,
"transfer_mode": TransferMode::RELIABLE,
"call_local": false,
}
.to_variant(),
); In this case You can also call RPC methods on your class like this: self.base_mut().rpc_id(
1, // Send to server only, its ID is always 1.
"proxy_request_to_server_internal".into(),
&[
request_id.to_variant(),
serialized_request_body.into_godot().to_variant(),
],
); Again, it's all terribly un-ergonomic and not well-typed, but it gets the job done if you need it right this minute. |
I did some initial work for implementing an It's a parsing implementation detail and easily changed, but I've implemented and would propose this syntax: #[godot_api]
impl MyRefCounted {
#[rpc(mode = RpcMode::AUTHORITY, transfer_mode = TransferMode::RELIABLE, call_local = true, transfer_channel = 10)]
pub fn test() {}
} (it should be noted that In this case I'd be glad to carry the PR further into implementation and out of "here's how it could work and a half implemented base" but the syntax to target is an open question. Also I could use some clarification on how I should inject the logic that calls |
Thanks a lot, cool to see someone is tackling this! 💪
So, the arguments that you can provide to On the topic of #[godot_api]
impl MyRefCounted {
#[rpc(
mode = RpcMode::AUTHORITY,
transfer_mode = TransferMode::RELIABLE,
call_local = true,
transfer_channel = 10
)]
pub fn test() {}
} This looks great! You should be able to reuse most of I assume you have default values for those, which match Godot?
I'm currently not in front of my PC, can have a look towards the evening or so. |
The main reason I had Making the decision to require both isn't permanent.
Right now I have it forwarding the The main problem with my implementation right now is that the way it's implemented directly generates a Dictionary with the Example of the above#[godot_api]
impl MyRefCounted {
#[rpc(
mode = String::from("completely wrong value"),
transfer_mode = 0.1f32,
call_local = true,
transfer_channel = 10
)]
pub fn test() {}
} I've done some work with type safety in macros outside of this crate so I'm familiar with the general strategies but I'd like to know how you think I should approach this so it fits with the rest of the code.
I haven't tracked down the defaults or determined whether we can omit entries and have Godot provide its defaults. Honestly, it sounds like the path forward here would be determining the defaults and I'll do that. |
[Edit] I was originally suggesting a builder, but it's not even needed. Might be nice once we have a builder API, but we can always add that once it's actually necessary. Probably a small struct would be appropriate: // Private API
pub struct RpcArgs {
mode: RpcMode,
transfer_mode: TransferMode,
call_local: bool,
transfer_channel: u32,
} According to these docs, the defaults are: impl Default for RpcArgs {
fn default() -> Self {
Self {
mode: RpcMode::AUTHORITY,
transfer_mode: TransferMode::UNRELIABLE,
call_local: false,
transfer_channel: 0,
}
}
} Usage: // Generated code by the macro -- one call for each key:
let rpc = RpcArgs {
mode: RpcMode::AUTHORITY,
call_local: true,
..Default::default()
};
// The API that actually registers this with Godot then accesses the builder directly:
let transfer_mode = rpc.transfer_mode;
... |
The syntax with both must anyway be accepted: #[func]
#[rpc]
fn method() { ... } Simply for consistency, if people experiment with keys to But yeah, I think it's OK to assume empty |
I implemented the type safety using the struct as you suggested, feedback to the user on type mismatch looks great so no more concerns there. I'll get Planned syntaxJust to concretely describe the syntax I'm implementing right now in a concrete way, and as I said before its easy to change:The #[rpc]
pub fn test() {} #[rpc]
#[func]
pub fn test() {} To use features of the #[rpc]
#[func(rename = "foo")]
pub fn test() {} (a specific thing to call out here, the rpc macro will be aware of the renamed function and use the correct name) Unlike the gdscript and C# attributes for RPC, the Rust attribute only supports named parameters rather than additionally allowing positional parameters: This is supported #[rpc(
mode = RpcMode::AUTHORITY,
call_local = true,
transfer_mode = TransferMode::RELIABLE,
transfer_channel = 10
)]
pub fn test() {} This is not supported: #[rpc(RpcMode::Authority, true, TransferMode::RELIABLE, 10)]
pub fn test() {} I'd say it's better to use key-value for explicitness, and I worry about the potential to pick up gdscript syntax for the sake of picking up gdscript syntax. It should be noted C# supports both positional attribute parameters and named attribute parameters, so the C# API doesn't really offer any defense for not adding it. If we're adding multiple ways to express the #[rpc(authority, call_local, reliable, 10)]
pub fn test() {} I'd call it a mistake to support this, it negates the benefits of using actual types in a macro's invocation and then, as a result, requires us to decide the type in the macro code. Then we have to generate our own error when its not one of the correct values. Rust analyzer would provide 0 feedback while writing the code. Generated code after the latest changes#[godot_api]
impl MyRefCounted {
#[rpc(
mode = RpcMode::AUTHORITY,
transfer_mode = TransferMode::RELIABLE,
transfer_channel = 10,
call_local = true
)]
pub fn test() {}
#[rpc(
mode = RpcMode::ANY_PEER,
transfer_mode = TransferMode::UNRELIABLE_ORDERED,
transfer_channel = 15,
call_local = false
)]
pub fn foo() {}
} fn __register_rpc(base: &mut ::godot::obj::Gd<::godot::classes::Node>) {
let rpc_configuration = ::godot::obj::RpcArgs {
mode: RpcMode::AUTHORITY,
transfer_mode: TransferMode::RELIABLE,
call_local: true,
transfer_channel: 10,
..Default::default()
}
.into_dictionary();
base.rpc_config("test".into(), rpc_configuration.to_variant());
let rpc_configuration = ::godot::obj::RpcArgs {
mode: RpcMode::ANY_PEER,
transfer_mode: TransferMode::UNRELIABLE_ORDERED,
call_local: false,
transfer_channel: 15,
..Default::default()
}
.into_dictionary();
base.rpc_config("foo".into(), rpc_configuration.to_variant());
}
|
I was thinking about this, too; but I think key-value is better for two reasons:
I know it's a bit more wordy and there's some RTFM involved when translating GDScript for the first time, but I'm not sure if it's that bad. We should probably see how users work with this. Another thing I was briefly thinking about: #[rpc(
mode = AUTHORITY, // omit RpcMode::
transfer_mode = RELIABLE, // omit TransferMode::
transfer_channel = 10,
call_local = true
)] However I'm not sure if it's a good idea. While we can easily inject the enum prefixes, this prevents use cases like: const MY_MODE: RpcMode = RpcMode::AUTHORITY;
#[rpc(mode = MY_MODE)] which would btw partially address the "reuse config" point you mentioned. But I agree, if people find themselves repeating a lot, we could still later add a Could you also add tests where only some RPC config keys are specified? Is there a way to test this at all? There seems to be no getter counterpart to |
I didn't even think about that, we could make const MY_RPC_CONFIG: RpcConfig = RpcConfig {
mode: RpcMode::AUTHORITY,
transfer_mode: TransferMode::RELIABLE,
transfer_channel: 10,
call_local: true
}
#[rpc(mode = MY_RPC_CONFIG.mode, transfer_mode = MY_RPC_CONFIG.transfer_mode, transfer_channel = MY_RPC_CONFIG.transfer_channel, call_local = MY_RPC_CONFIG.call_local] There's... definitely a function in the Godot source code to get rpc configuration but it doesn't seem like it'll help. If we can write tests that actually use Godot's RPC (by being the host, so we don't need any networking), we can at the very least test calls where |
You're right, maybe we should just make it public then? I think the (The
Yes, this is going to be both development- and maintenance-heavy; I'd rather avoid it. Maybe we can open a PR to Godot to expose this functionality. I can try to find out if there's a reason why it's not exposed 🤔 |
Added Click to view syntaxconst BARE_CONST_CONFIG: RpcConfig = RpcConfig {
mode: RpcMode::AUTHORITY,
transfer_mode: TransferMode::UNRELIABLE,
call_local: false,
transfer_channel: 1,
};
#[godot_api]
impl MyRefCounted {
#[rpc(
mode = RpcMode::AUTHORITY,
transfer_mode = TransferMode::RELIABLE,
transfer_channel = 10,
call_local = true
)]
pub fn test() {}
#[rpc(
mode = RpcMode::ANY_PEER,
transfer_mode = TransferMode::UNRELIABLE_ORDERED,
transfer_channel = 15,
call_local = false
)]
pub fn foo() {}
// Rpc + func not supported yet
#[rpc(call_local = true)]
#[func(rename = "PascalCaseEndpoint")]
pub fn snake_case_endpoint() {
}
const ASSOCIATED_CONFIG: RpcConfig = RpcConfig {
mode: RpcMode::AUTHORITY,
transfer_mode: TransferMode::UNRELIABLE,
call_local: false,
transfer_channel: 1,
};
#[rpc(config = Self::ASSOCIATED_CONFIG)]
pub fn new_config_endpoint() {}
#[rpc(config = BARE_CONST_CONFIG)]
pub fn other_config_endpoint() {}
} This is where we're at syntax wise. I also need to add a check to prevent the I'll open the PR following the codegen changes for |
This looks great, thanks a lot! 👍 One small possibility is that Godot would extend the |
@ambeeeeee Have you had a chance to follow up on this yet? 🙂 |
I propose allowing the following, which aligns with GDScript's style: #[rpc("any_peer", "reliable", "call_local", channel = 0)]
fn my_rpc_call(&mut self) {
//..
}
const SHARED_RPC_ARGS: RpcArgs = RpcArgs { .. };
#[rpc(args = SHARED_RPC_ARGS)]
fn my_rpc_call(&mut self) {
//..
}
#[rpc(args = SHARED_RPC_ARGS, "any_peer")] // compiler error, can't have both args constant and individual
fn my_rpc_call(&mut self) {
//..
}
#[func(gd_self)] // #[func] can be specified to provide additional arguments, otherwise it's implied with default ones
#[rpc("any_peer")]
fn my_rpc_call(&mut self) {
//..
} I prefer having a separate attribute because:
If there's no #[func] attribute, then it's implied with default arguments. The RpcArgs struct would be the one agreeded on the previous answers. I can contribute this PR if a design is agreed upon. |
@ambeeeeee's previous suggestion included this syntax: #[godot_api]
impl MyRefCounted {
#[rpc(mode = RpcMode::AUTHORITY, transfer_mode = TransferMode::RELIABLE, call_local = true, transfer_channel = 10)]
pub fn test() {}
} What's your view on that vs. yours, @Houtamelo? If we keep it shorter, then we should not quote keys as strings; no other proc-macro API of gdext does this currently. Syntax cohesion with GDScript is less important than internal consistency within the library. That is, instead of: #[rpc("any_peer", "reliable", "call_local", channel = 0)]
fn my_rpc_call(&mut self) it would be #[rpc(any_peer, reliable, call_local, channel = 0)]
fn my_rpc_call(&mut self) Note that you essentially have 3 enum parameters with 2-3 variants each: And 2 of those are encoded in But maybe the shorter syntax is worth it for the sake of clarity and GDScript familiarity. In that case, people who want explicit enum orthogonality could still use |
Looks good, I'm in favor of that. |
Sounds good to me, you can gladly go ahead with a PR 😊 |
What about pub enum SyncMode {
CallLocal,
CallRemote,
} |
I think the idea was to follow the dictionary used in That also reminds me why the original key was named |
Hello, I've been trying to find any way to use godot's RPCs in
gdext
, but couldn't find any example of it (searched for it in the issues and features under this repo as well). It would be great to see a workinggdext
example like ingdnative
, or at least some code examples to see how to declare RPC functions? So far I was only able to find the Baserpc
method, but that's about it.The text was updated successfully, but these errors were encountered: