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

#[var(set = function_name)] is not compatible with #[func(rename = gdscript_name)] #863

Open
Houtamelo opened this issue Aug 18, 2024 · 6 comments
Labels
bug c: register Register classes, functions and other symbols to GDScript

Comments

@Houtamelo
Copy link
Contributor

Example:

#[derive(GodotClass)]
#[class(init, base = CharacterBody2D)]
pub struct Herbivore {
	#[var(get, set = gdscript_set_health)]
	pub(super) health: f64,
}

#[godot_api]
impl Herbivore {
	#[func(rename = set_health)]
	fn gdscript_set_health(&mut self, value: f64) { 
		self.set_health(value);
	}
}

This compiles in Rust but in Godot we get a runtime error when trying to invoke the property:
image

This happens because inside Godot its using the original name, not the renamed one.

Not sure if this is by design or a niche bug.

@Bromeon
Copy link
Member

Bromeon commented Aug 18, 2024

Interesting! Certainly not intended.

Might be a bit tricky because those are in two separate macros, and I don't know if the #[func(rename)] info is "stored" somewhere to be accessible by code generated from #[derive(Godot)]. It should be possible, but might require an extra indirection for each get/set function reference.

@Bromeon Bromeon added bug c: register Register classes, functions and other symbols to GDScript labels Aug 18, 2024
@Bromeon
Copy link
Member

Bromeon commented Nov 23, 2024

One idea to address this: #[godot_api] generates a compile-time lookup table with all #[func]s and their registered names. So

#[godot_api]
impl Herbivore {
	#[func]
	fn eat_herbs(&mut self, value: f64) {...}

	#[func(rename = set_health)]
	fn gdscript_set_health(&mut self, value: f64) {...}
}

generates something like

mod __funcs_Herbivore {
      pub const eat_herbs: &str = "eat_herbs";
      pub const gdscript_set_health: &str = "set_health";
}

Then, the custom setter

#[var(get, set = gdscript_set_health)]
pub(super) health: f64,

would, instead of registering the setter as "gdscript_set_health" in Godot, look up the name as __funcs_Herbivore::gdscript_set_health. This solves two problems:

  1. It takes into account renamed methods.
  2. If someone forgets #[func] above gdscript_set_health, this causes a compile-time error (at the moment: runtime error).

@0x53A
Copy link
Contributor

0x53A commented Dec 5, 2024

That got kinda harder with my PR, where you can now have multiple #[godot_api].

Since you can't contribute to one mod from multiple places, the constants would have to live in the surrounding module.

#[godot_api]
impl Herbivore {
	#[func]
	fn eat_herbs(&mut self, value: f64) {...}
}

// generated
pub const __funcs_Herbivore_eat_herbs: &str = "eat_herbs";


#[godot_api]
impl Herbivore {
	#[func(rename = set_health)]
	fn gdscript_set_health(&mut self, value: f64) {...}
}

// generated
pub const __funcs_Herbivore_gdscript_set_health: &str  = "set_health";

@Bromeon
Copy link
Member

Bromeon commented Dec 5, 2024

Ugh... back to C namespacing then 😔

(It could be impl scoped in this case... but generally I don't like polluting neither the struct's nor the surrounding scope)

@0x53A
Copy link
Contributor

0x53A commented Dec 6, 2024

would you merge a PR for this anyway? if yes, what pattern exactly?

/// Generated by the #[func] macro of gdext. Maps the name under which it is exported to godot to the rust name of the function.
#[doc(hidden)]
pub(self) const __func_{class}_{rust-fn-name}: &str  = "{godot-fn-name}";

(I could take a stab at implementing it next week or so)

@Bromeon
Copy link
Member

Bromeon commented Dec 6, 2024

Yes, a PR would still be appreciated! It would already fix the issue, and if we find another approach in the future, we can always change it 🙂

You could maybe use the __gdext_ prefix like here:

pub unsafe fn __gdext_load_library<E: ExtensionLibrary>(

Also, pub(self) is redundant , so it could be along:

#[doc(hidden)]
const __gdext_func_{class}_{rust-fn-name}: &str  = "{godot-fn-name}";

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c: register Register classes, functions and other symbols to GDScript
Projects
None yet
Development

No branches or pull requests

3 participants