Skip to content

Commit

Permalink
Merge pull request #577 from godot-rust/qol/baseless
Browse files Browse the repository at this point in the history
`#[base]` attribute is no longer necessary
  • Loading branch information
Bromeon authored Jan 25, 2024
2 parents 6918096 + 60a82c0 commit d873d8b
Show file tree
Hide file tree
Showing 22 changed files with 199 additions and 94 deletions.
1 change: 0 additions & 1 deletion examples/dodge-the-creeps/rust/src/hud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use godot::prelude::*;
#[derive(GodotClass)]
#[class(base=CanvasLayer)]
pub struct Hud {
#[base]
base: Base<CanvasLayer>,
}

Expand Down
1 change: 0 additions & 1 deletion examples/dodge-the-creeps/rust/src/main_scene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ pub struct Main {
music: Option<Gd<AudioStreamPlayer>>,
death_sound: Option<Gd<AudioStreamPlayer>>,
score: i64,
#[base]
base: Base<Node>,
}

Expand Down
1 change: 0 additions & 1 deletion examples/dodge-the-creeps/rust/src/mob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ pub struct Mob {
pub min_speed: real,
pub max_speed: real,

#[base]
base: Base<RigidBody2D>,
}

Expand Down
1 change: 0 additions & 1 deletion examples/dodge-the-creeps/rust/src/player.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ pub struct Player {
speed: real,
screen_size: Vector2,

#[base]
base: Base<Area2D>,
}

Expand Down
2 changes: 1 addition & 1 deletion godot-codegen/src/generator/virtual_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ fn special_virtual_methods(notification_enum_name: &Ident) -> TokenStream {

/// Godot constructor, accepting an injected `base` object.
///
/// `base` refers to the base instance of the class, which can either be stored in a `#[base]` field or discarded.
/// `base` refers to the base instance of the class, which can either be stored in a `Base<T>` field or discarded.
/// This method returns a fully-constructed instance, which will then be moved into a [`Gd<T>`][crate::obj::Gd] pointer.
///
/// If the class has a `#[class(init)]` attribute, this method will be auto-generated and must not be overridden.
Expand Down
23 changes: 23 additions & 0 deletions godot-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,29 @@ pub mod private {

/// Ensure `T` is an editor plugin.
pub const fn is_editor_plugin<T: crate::obj::Inherits<crate::engine::EditorPlugin>>() {}

// ------------------------------------------------------------------------------------------------------------------------------------------
// Compatibility

// Code generated by Rust derive macros cannot cause any deprecation warnings, due to questionable "feature"
// https://github.com/rust-lang/rust/pull/58994. Fortunately, an extra layer of indirection solves most problems: we generate a declarative
// macro that itself isn't deprecated, but _its_ expansion is. Since the expansion happens in a later step, the warning is emitted.

#[doc(hidden)]
#[inline(always)]
#[deprecated = "#[base] is no longer needed; Base<T> is recognized directly. \n\
More information on https://github.com/godot-rust/gdext/pull/577."]
pub const fn base_attribute_warning() {}

#[doc(hidden)]
#[macro_export]
macro_rules! __base_attribute_warning_expand {
() => {
const _: () = $crate::private::base_attribute_warning();
};
}

pub use crate::__base_attribute_warning_expand;
}

macro_rules! generate_gdextension_api_version {
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/obj/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl<T: GodotClass> Base<T> {
// This obj does not contribute to the strong count, otherwise we create a reference cycle:
// 1. RefCounted (dropped in GDScript)
// 2. holds user T (via extension instance and storage)
// 3. holds #[base] RefCounted (last ref, dropped in T destructor, but T is never destroyed because this ref keeps storage alive)
// 3. holds Base<T> RefCounted (last ref, dropped in T destructor, but T is never destroyed because this ref keeps storage alive)
// Note that if late-init never happened on self, we have the same behavior (still a raw pointer instead of weak Gd)
Base::from_obj(obj)
}
Expand Down
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
7 changes: 3 additions & 4 deletions godot-core/src/obj/gd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ use crate::{callbacks, engine, out};
///
/// * [`Gd::default()`] for reference-counted types that are constructible. For user types, this means they must expose an `init` function
/// or have a generated one. `Gd::<T>::default()` is equivalent to the shorter `T::new_gd()` and primarily useful for derives or generics.
/// * [`Gd::from_init_fn(function)`][Gd::from_init_fn] for Rust objects with `#[base]` field, which are constructed inside the smart pointer.
/// * [`Gd::from_init_fn(function)`][Gd::from_init_fn] for Rust objects with `Base<T>` field, which are constructed inside the smart pointer.
/// This is a very handy function if you want to pass extra parameters to your object upon construction.
/// * [`Gd::from_object(rust_obj)`][Gd::from_object] for existing Rust objects without a `#[base]` field that are moved _into_ the smart pointer.
/// * [`Gd::from_object(rust_obj)`][Gd::from_object] for existing Rust objects without a `Base<T>` field that are moved _into_ the smart pointer.
/// * [`Gd::from_instance_id(id)`][Gd::from_instance_id] and [`Gd::try_from_instance_id(id)`][Gd::try_from_instance_id]
/// to obtain a pointer to an object which is already alive in the engine.
///
Expand Down Expand Up @@ -108,7 +108,7 @@ where
{
/// Creates a `Gd<T>` using a function that constructs a `T` from a provided base.
///
/// Imagine you have a type `T`, which has a `#[base]` field that you cannot default-initialize.
/// Imagine you have a type `T`, which has a base field that you cannot default-initialize.
/// The `init` function provides you with a `Base<T::Base>` object that you can use inside your `T`, which
/// is then wrapped in a `Gd<T>`.
///
Expand All @@ -118,7 +118,6 @@ where
/// #[derive(GodotClass)]
/// #[class(init, base=Node2D)]
/// struct MyClass {
/// #[base]
/// my_base: Base<Node2D>,
/// other_field: i32,
/// }
Expand Down
10 changes: 4 additions & 6 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 Expand Up @@ -208,7 +210,7 @@ pub trait IndexEnum: EngineEnum {
}
}

/// Trait that's implemented for user-defined classes that provide a `#[base]` field.
/// Trait that's implemented for user-defined classes that provide a `Base<T>` field.
///
/// Gives direct access to the containing `Gd<Self>` from `Self`.
// Possible alternative for builder APIs, although even less ergonomic: Base<T> could be Base<T, Self> and return Gd<Self>.
Expand All @@ -232,7 +234,6 @@ pub trait WithBaseField: GodotClass + Bounds<Declarer = bounds::DeclUser> {
/// #[derive(GodotClass)]
/// #[class(init, base = Node)]
/// struct MyClass {
/// #[base]
/// base: Base<Node>,
/// }
///
Expand All @@ -259,8 +260,7 @@ pub trait WithBaseField: GodotClass + Bounds<Declarer = bounds::DeclUser> {
/// #[derive(GodotClass)]
/// #[class(init, base = Node)]
/// struct MyClass {
/// #[base]
/// base: Base<Node>,
/// /// base: Base<Node>,
/// }
///
/// #[godot_api]
Expand Down Expand Up @@ -298,7 +298,6 @@ pub trait WithBaseField: GodotClass + Bounds<Declarer = bounds::DeclUser> {
/// #[derive(GodotClass)]
/// #[class(init, base = Node)]
/// struct MyClass {
/// #[base]
/// base: Base<Node>,
/// }
///
Expand All @@ -324,7 +323,6 @@ pub trait WithBaseField: GodotClass + Bounds<Declarer = bounds::DeclUser> {
/// #[derive(GodotClass)]
/// #[class(init, base = Node)]
/// struct MyClass {
/// #[base]
/// base: Base<Node>,
/// }
///
Expand Down
5 changes: 4 additions & 1 deletion godot-macros/src/class/data_models/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ pub struct Fields {
/// All fields except `base_field`.
pub all_fields: Vec<Field>,

/// The field annotated with `#[base]`.
/// The field with type `Base<T>`, if available.
pub base_field: Option<Field>,

/// Whether a deprecated `#[base]` was used.
pub has_deprecated_base: bool,
}
92 changes: 75 additions & 17 deletions godot-macros/src/class/derive_godot_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ pub fn derive_godot_class(decl: Declaration) -> ParseResult<TokenStream> {
quote! {}
};

let deprecated_base_warning = if fields.has_deprecated_base {
quote! { ::godot::private::__base_attribute_warning_expand!(); }
} else {
TokenStream::new()
};

let (user_class_impl, has_default_virtual) =
make_user_class_impl(class_name, struct_cfg.is_tool, &fields.all_fields);

Expand Down Expand Up @@ -134,6 +140,7 @@ pub fn derive_godot_class(decl: Declaration) -> ParseResult<TokenStream> {
});

#prv::class_macros::#inherits_macro!(#class_name);
#deprecated_base_warning
})
}

Expand Down Expand Up @@ -228,8 +235,14 @@ fn parse_struct_attributes(class: &Struct) -> ParseResult<ClassAttributes> {
is_tool = true;
}

// TODO: better error message when using in Godot 4.0
if parser.handle_alone_ident("editor_plugin")?.is_some() {
if let Some(attr_key) = parser.handle_alone_ident("editor_plugin")? {
if cfg!(before_api = "4.1") {
return bail!(
attr_key,
"#[class(editor_plugin)] is not supported in Godot 4.0"
);
}

is_editor_plugin = true;
}

Expand All @@ -256,6 +269,7 @@ fn parse_struct_attributes(class: &Struct) -> ParseResult<ClassAttributes> {
fn parse_fields(class: &Struct) -> ParseResult<Fields> {
let mut all_fields = vec![];
let mut base_field = Option::<Field>::None;
let mut has_deprecated_base = false;

let named_fields: Vec<(NamedField, Punct)> = match &class.fields {
StructFields::Unit => {
Expand All @@ -273,17 +287,15 @@ fn parse_fields(class: &Struct) -> ParseResult<Fields> {
let mut is_base = false;
let mut field = Field::new(&named_field);

// #[base]
if let Some(parser) = KvParser::parse(&named_field.attributes, "base")? {
if let Some(prev_base) = base_field.as_ref() {
return bail!(
parser.span(),
"#[base] allowed for at most 1 field, already applied to `{}`",
prev_base.name
);
}
// Base<T> type inference
if path_ends_with_complex(&field.ty, "Base") {
is_base = true;
}

// deprecated #[base]
if KvParser::parse(&named_field.attributes, "base")?.is_some() {
has_deprecated_base = true;
is_base = true;
parser.finish()?;
}

// OnReady<T> type inference
Expand Down Expand Up @@ -312,23 +324,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
Loading

0 comments on commit d873d8b

Please sign in to comment.