Skip to content
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

Make AsArg usable for custom functions #961

Open
aekobear opened this issue Dec 4, 2024 · 4 comments
Open

Make AsArg usable for custom functions #961

aekobear opened this issue Dec 4, 2024 · 4 comments

Comments

@aekobear
Copy link
Contributor

aekobear commented Dec 4, 2024

gdext 0.2 gave us the power to pass strings without .into() for godot APIs:

node.base().get_node_as::<Node2D>("my/node");

Which is a really awesome feature! But when implementing custom functions that take godot string values, we still need to do:

custom_node.bind().custom_function("string-name".into());

Thinking as a plugin creator, long-term it would be valuable to allow custom node APIs to also declare their arguments using the AsArg trait so that users of plugins do not experience inconsistent apis (IE why do I only have to call into() for strings sometimes?)

Not sure how feasible this actually is, but I wanted to float the idea just in case.

@Bromeon
Copy link
Member

Bromeon commented Dec 4, 2024

You can already declare impl AsArg<T> parameters, but only forward their values to Godot APIs.

What makes this a bit cumbersome (and the reason why AsArg offers no public methods in its initial version) is that you either get a value or a reference, but you cannot just move out of the parameter without invalidating that reference. So you need something like Cow, and often 2 method calls (impl AsArg -> Cow -> &T). The library itself currently does this via macro, but it's not very elegant.

As for consistent APIs, I agree that would be nice. Note that due to binding, you likely won't have 100% similarity, but we can at least strive towards it 🙂

@aekobear
Copy link
Contributor Author

aekobear commented Dec 4, 2024

You can already declare impl AsArg<T> parameters, but only forward their values to Godot APIs.

Hm maybe I'm doing something wrong with the syntax. What I'm seeing is:

#[godot_api]
impl CustomNode {
  #[func]
  pub fn test(&self, action: impl AsArg<StringName>) {}
}

This throws an error on the #[godot_api] macro

bz_in::_::__init::__inner_init::{closure#0}::Sig::{opaque#0}cannot be formatted using{:?}because it doesn't implementDebug`

@Bromeon
Copy link
Member

Bromeon commented Dec 5, 2024

Ah yes, I thought you talked about Rust-only APIs. #[func] doesn't support generics (yet?).

I assume you'd want impl AsArg<T> to behave the same as T? One issue is that it doesn't need to be an owned value, so FromGodot is a bit trickier...

Maybe a first step would be to forget about references and just treat things as values in #[func]. May need extra ref-count, but would avoid lifetimes and other headaches.

@aekobear
Copy link
Contributor Author

aekobear commented Dec 5, 2024

I assume you'd want impl AsArg<T> to behave the same as T? One issue is that it doesn't need to be an owned value, so FromGodot is a bit trickier...

That would be awesome if possible, but I personally don't mind a bit of boilerplate (like your current macro) inside the function definition as long as it's seamless for the caller. To me it's analogous to:

fn send<S: AsRef<str>>(msg: S) {
  let owned: String = msg.as_ref().into();
 }

Where there's some ref juggling and boilerplate for the sake of a cleaner external API.

I don't think refcounting is a bad idea though. My only concern would be performance, but I'm guessing the overhead of an extra ref-count would be negligible here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants