Skip to content

Commit

Permalink
Add #[hint] as an escape hatch for Base<T> and OnReady<T> inference
Browse files Browse the repository at this point in the history
  • Loading branch information
Bromeon committed Jan 22, 2024
1 parent ef86a47 commit 9c77462
Show file tree
Hide file tree
Showing 7 changed files with 131 additions and 50 deletions.
2 changes: 1 addition & 1 deletion godot-core/src/obj/bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ pub(super) mod private {
/// <strong>Never</strong> implement this trait manually.
/// </div>
///
/// Most of the time, this trait is covered by [`#[derive(GodotClass)`](../bind/derive.GodotClass.html).
/// Most of the time, this trait is covered by [`#[derive(GodotClass)]`](../register/derive.GodotClass.html).
/// If you implement `GodotClass` manually, use the [`implement_godot_bounds!`][crate::implement_godot_bounds] macro.
///
/// There are two reasons to avoid a hand-written `impl Bounds`:
Expand Down
2 changes: 2 additions & 0 deletions godot-core/src/obj/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ use godot_ffi as sys;
/// Makes `T` eligible to be managed by Godot and stored in [`Gd<T>`][crate::obj::Gd] pointers.
///
/// The behavior of types implementing this trait is influenced by the associated types; check their documentation for information.
///
/// Normally, you don't need to implement this trait yourself; use [`#[derive(GodotClass)]`](../register/derive.GodotClass.html) instead.
// Above intra-doc link to the derive-macro only works as HTML, not as symbol link.
pub trait GodotClass: Bounds + 'static
where
Expand Down
63 changes: 50 additions & 13 deletions godot-macros/src/class/derive_godot_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,14 +289,6 @@ fn parse_fields(class: &Struct) -> ParseResult<Fields> {

// Base<T> type inference
if path_ends_with_complex(&field.ty, "Base") {
if let Some(prev_base) = base_field.as_ref() {
return bail!(
field.name,
"at most 1 field can have type Base<T>; previous is `{}`",
prev_base.name
);
}

is_base = true;
}

Expand Down Expand Up @@ -331,24 +323,69 @@ fn parse_fields(class: &Struct) -> ParseResult<Fields> {
parser.finish()?;
}

// Exported or Rust-only fields
// #[hint] to override type inference (must be at the end).
if let Some(mut parser) = KvParser::parse(&named_field.attributes, "hint")? {
if let Some(override_base) = handle_opposite_keys(&mut parser, "base")? {
is_base = override_base;
}

if let Some(override_onready) = handle_opposite_keys(&mut parser, "onready")? {
field.is_onready = override_onready;
}
parser.finish()?;
}

// Extra validation; eventually assign to base_fields or all_fields.
if is_base {
base_field = Some(field);
if field.is_onready
|| field.var.is_some()
|| field.export.is_some()
|| field.default.is_some()
{
return bail!(
&named_field,
"base field cannot have type `OnReady<T>` or attributes #[var], #[export] or #[init]"
);
}

if let Some(prev_base) = base_field.replace(field) {
// Ensure at most one Base<T>.
return bail!(
// base_field.unwrap().name,
named_field,
"at most 1 field can have type Base<T>; previous is `{}`",
prev_base.name
);
}
} else {
all_fields.push(field);
}
}

// TODO detect #[base] based on type instead of attribute
// Edge cases (type aliases, user types with same name, ...) could be handled with #[hint(base)] or #[hint(no_base)].

Ok(Fields {
all_fields,
base_field,
has_deprecated_base,
})
}

fn handle_opposite_keys(parser: &mut KvParser, key: &str) -> ParseResult<Option<bool>> {
let antikey = format!("no_{}", key);

let is_key = parser.handle_alone(key)?;
let is_no_key = parser.handle_alone(&antikey)?;

match (is_key, is_no_key) {
(true, false) => Ok(Some(true)),
(false, true) => Ok(Some(false)),
(false, false) => Ok(None),
(true, true) => bail!(
parser.span(),
"#[hint] attribute keys `{key}` and `{antikey}` are mutually exclusive",
),
}
}

// ----------------------------------------------------------------------------------------------------------------------------------------------
// General helpers

Expand Down
73 changes: 46 additions & 27 deletions godot-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@ use crate::util::ident;
/// your `struct`:
///
/// ```
/// use godot::prelude::*;
///
/// # use godot::prelude::*;
/// #[derive(GodotClass)]
/// #[class(base = Node2D)]
/// struct MyStruct {
Expand All @@ -94,8 +93,7 @@ use crate::util::ident;
/// your object accordingly. You can access it through `self.base()` and `self.base_mut()` methods.
///
/// ```
/// use godot::prelude::*;
///
/// # use godot::prelude::*;
/// #[derive(GodotClass)]
/// #[class(base = Node2D)]
/// struct MyStruct {
Expand All @@ -116,8 +114,7 @@ use crate::util::ident;
/// To create a property, you can use the `#[var]` annotation:
///
/// ```
/// use godot::prelude::*;
///
/// # use godot::prelude::*;
/// #[derive(GodotClass)]
/// struct MyStruct {
/// #[var]
Expand All @@ -134,8 +131,7 @@ use crate::util::ident;
/// `#[export(get = ..., set = ...)]`:
///
/// ```
/// use godot::prelude::*;
///
/// # use godot::prelude::*;
/// #[derive(GodotClass)]
/// struct MyStruct {
/// #[var(get = get_my_field, set = set_my_field)]
Expand All @@ -161,8 +157,7 @@ use crate::util::ident;
/// generated getter or setter in these cases anyway, use `get` or `set` without a value:
///
/// ```
/// use godot::prelude::*;
///
/// # use godot::prelude::*;
/// #[derive(GodotClass)]
/// struct MyStruct {
/// // Default getter, custom setter.
Expand All @@ -182,8 +177,7 @@ use crate::util::ident;
/// For exporting properties to the editor, you can use the `#[export]` attribute:
///
/// ```
/// use godot::prelude::*;
///
/// # use godot::prelude::*;
/// #[derive(GodotClass)]
/// struct MyStruct {
/// #[export]
Expand All @@ -206,8 +200,7 @@ use crate::util::ident;
/// As an example of some different export attributes:
///
/// ```
/// use godot::prelude::*;
///
/// # use godot::prelude::*;
/// #[derive(GodotClass)]
/// struct MyStruct {
/// // @export
Expand Down Expand Up @@ -249,8 +242,7 @@ use crate::util::ident;
/// the export attributes.
///
/// ```
/// use godot::prelude::*;
///
/// # use godot::prelude::*;
/// const MAX_HEALTH: f64 = 100.0;
///
/// #[derive(GodotClass)]
Expand All @@ -267,8 +259,7 @@ use crate::util::ident;
/// `hint`, `hint_string`, and `usage_flags` keys in the attribute:
///
/// ```
/// use godot::prelude::*;
///
/// # use godot::prelude::*;
/// #[derive(GodotClass)]
/// struct MyStruct {
/// // Treated as an enum with two values: "One" and "Two"
Expand All @@ -289,8 +280,7 @@ use crate::util::ident;
/// It will be fundamentally reworked.
///
/// ```no_run
/// use godot::prelude::*;
///
/// # use godot::prelude::*;
/// #[derive(GodotClass)]
/// struct MyClass {}
///
Expand All @@ -311,7 +301,7 @@ use crate::util::ident;
///
/// This is very similar to [GDScript's `@tool` feature](https://docs.godotengine.org/en/stable/tutorials/plugins/running_code_in_the_editor.html).
///
/// # Editor Plugins
/// # Editor plugins
///
/// If you annotate a class with `#[class(editor_plugin)]`, it will be turned into an editor plugin. The
/// class must then inherit from `EditorPlugin`, and an instance of that class will be automatically added
Expand All @@ -325,13 +315,13 @@ use crate::util::ident;
/// This should usually be combined with `#[class(tool)]` so that the code you write will actually run in the
/// editor.
///
/// # Class Renaming
/// # Class renaming
///
/// You may want to have structs with the same name. With Rust, this is allowed using `mod`. However in GDScript,
/// there are no modules, namespaces, or any such disambiguation. Therefore, you need to change the names before they
/// can get to Godot. You can use the `rename` key while defining your `GodotClass` for this.
///
/// ```
/// ```no_run
/// mod animal {
/// # use godot::prelude::*;
/// #[derive(GodotClass)]
Expand All @@ -349,7 +339,7 @@ use crate::util::ident;
///
/// These classes will appear in the Godot editor and GDScript as "AnimalToad" or "NpcToad".
///
/// # Hiding Classes
/// # Hiding classes
///
/// If you want to register a class with Godot, but not have it show up in the editor then you can use `#[class(hide)]`.
///
Expand All @@ -362,6 +352,35 @@ use crate::util::ident;
///
/// Even though this class is a `Node` and it has an init function, it still won't show up in the editor as a node you can add to a scene
/// because we have added a `hide` key to the class. This will also prevent it from showing up in documentation.
///
/// # Fine-grained inference hints
///
/// The derive macro is relatively smart about recognizing `Base<T>` and `OnReady<T>` types, and works also if those are qualified.
///
/// However, there may be situations where you need to help it out -- for example, if you have a type alias for `Base<T>`, or use an unrelated
/// `my_module::Base<T>` with a different meaning.
///
/// In this case, you can manually override the behavior with the `#[hint]` attribute. It takes multiple standalone keys:
/// - `base` and `no_base`
/// - `onready` and `no_onready`
///
/// ```no_run
/// use godot::engine::Node;
///
/// // There's no reason to do this, but for the sake of example:
/// type Super<T> = godot::obj::Base<T>;
/// type Base<T> = godot::obj::Gd<T>;
///
/// #[derive(godot::register::GodotClass)]
/// #[class(base = Node)]
/// struct MyStruct {
/// #[hint(base)]
/// base: Super<Node>,
///
/// #[hint(no_base)]
/// unbase: Base<Node>,
/// }
/// ```
#[proc_macro_derive(GodotClass, attributes(class, base, hint, var, export, init, signal))]
pub fn derive_godot_class(input: TokenStream) -> TokenStream {
translate(input, class::derive_godot_class)
Expand Down Expand Up @@ -486,7 +505,7 @@ pub fn derive_to_godot(input: TokenStream) -> TokenStream {
translate(input, derive::derive_to_godot)
}

/// Derive macro for [`FromGodot`](../builtin/meta/trait.FromVariant.html) on structs or enums.
/// Derive macro for [`FromGodot`](../builtin/meta/trait.FromGodot.html) on structs or enums.
///
/// # Example
///
Expand Down Expand Up @@ -520,7 +539,7 @@ pub fn derive_from_godot(input: TokenStream) -> TokenStream {
translate(input, derive::derive_from_godot)
}

/// Derive macro for [`Var`](../bind/property/trait.Var.html) on enums.
/// Derive macro for [`Var`](../register/property/trait.Var.html) on enums.
///
/// Currently has some tight requirements which are expected to be softened as implementation expands:
/// - Only works for enums, structs aren't supported by this derive macro at the moment.
Expand Down Expand Up @@ -560,7 +579,7 @@ pub fn derive_property(input: TokenStream) -> TokenStream {
translate(input, derive::derive_var)
}

/// Derive macro for [`Export`](../bind/property/trait.Export.html) on enums.
/// Derive macro for [`Export`](../register/property/trait.Export.html) on enums.
///
/// Currently has some tight requirements which are expected to be softened as implementation expands, see requirements for [`Var`].
#[proc_macro_derive(Export)]
Expand Down
24 changes: 18 additions & 6 deletions itest/rust/src/object_tests/base_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,24 @@ fn base_gd_self() {

// ----------------------------------------------------------------------------------------------------------------------------------------------

#[derive(GodotClass)]
#[class(init, base=Node2D)]
struct Based {
base: Base<Node2D>,

i: i32,
use renamed_bases::Based;
mod renamed_bases {
use super::{GodotClass, Node2D};

// Test #[hint].
type Super<T> = super::Base<T>;
type Base<T> = T;

#[derive(GodotClass)]
#[class(init, base = Node2D)]
pub struct Based {
#[hint(base)]
pub base: Super<Node2D>, // de-facto: Base<Node2D>.

// This can coexist because it's not really a base.
#[hint(no_base)]
pub i: Base<i32>, // de-facto: i32
}
}

impl Based {
Expand Down
14 changes: 12 additions & 2 deletions itest/rust/src/object_tests/onready_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ fn onready_lifecycle_with_impl_without_ready() {

*obj.auto = 77;
assert_eq!(*obj.auto, 77);

// Test #[hint(no_onready)]: we can still initialize it (would panic if already auto-initialized).
godot::private::auto_init(&mut obj.nothing);
}

obj.free();
Expand Down Expand Up @@ -199,20 +202,27 @@ impl OnReadyWithoutImpl {

// ----------------------------------------------------------------------------------------------------------------------------------------------

type Ordy<T> = OnReady<T>;

// Class that has a #[godot_api] impl, but does not override ready. Used to test whether variables are still initialized.
#[derive(GodotClass)]
#[class(base=Node)]
struct OnReadyWithImplWithoutReady {
auto: OnReady<i32>,
// Test also #[hint] at the same time.
#[hint(onready)]
auto: Ordy<i32>,
// No manual one, since those cannot be initialized without a ready() override.
// (Technically they _can_ at the moment, but in the future we might ensure initialization after ready, so this is not a supported workflow).
#[hint(no_onready)]
nothing: OnReady<i32>,
}

// Rust-only impl, no proc macros.
impl OnReadyWithImplWithoutReady {
fn create() -> Gd<OnReadyWithImplWithoutReady> {
let obj = Self {
auto: OnReady::new(|| 66),
auto: Ordy::new(|| 66),
nothing: Ordy::new(|| -111),
};

Gd::from_object(obj)
Expand Down
3 changes: 2 additions & 1 deletion itest/rust/src/object_tests/property_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,8 @@ pub struct DeriveExport {
#[export]
pub foo: TestEnum,

pub base: Base<RefCounted>,
// Tests also qualified base path (type inference of Base<T> without #[hint]).
pub base: godot::obj::Base<RefCounted>,
}

#[godot_api]
Expand Down

0 comments on commit 9c77462

Please sign in to comment.