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

Disallow Export if class doesn't inherit Node or Resource #839

Merged
merged 2 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions godot-codegen/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,13 +329,32 @@ impl InheritanceTree {

/// Returns all base classes, without the class itself, in order from nearest to furthest (object).
pub fn collect_all_bases(&self, derived_name: &TyName) -> Vec<TyName> {
let mut maybe_base = derived_name;
let mut upgoer = derived_name;
let mut result = vec![];

while let Some(base) = self.derived_to_base.get(maybe_base) {
while let Some(base) = self.derived_to_base.get(upgoer) {
result.push(base.clone());
maybe_base = base;
upgoer = base;
}
result
}

/// Whether a class is a direct or indirect subclass of another (true for derived == base).
pub fn inherits(&self, derived: &TyName, base_name: &str) -> bool {
// Reflexive: T inherits T.
if derived.godot_ty == base_name {
return true;
}

let mut upgoer = derived;

while let Some(next_base) = self.derived_to_base.get(upgoer) {
if next_base.godot_ty == base_name {
return true;
}
upgoer = next_base;
}

false
}
}
18 changes: 14 additions & 4 deletions godot-codegen/src/generator/classes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ fn make_class(class: &Class, ctx: &mut Context, view: &ApiView) -> GeneratedClas
// notify() and notify_reversed() are added after other methods, to list others first in docs.
let notify_methods = notifications::make_notify_methods(class_name, ctx);

let (assoc_memory, assoc_dyn_memory) = make_bounds(class);
let (assoc_memory, assoc_dyn_memory, is_exportable) = make_bounds(class, ctx);

let internal_methods = quote! {
fn __checked_id(&self) -> Option<crate::obj::InstanceId> {
Expand Down Expand Up @@ -228,6 +228,7 @@ fn make_class(class: &Class, ctx: &mut Context, view: &ApiView) -> GeneratedClas
type Memory = crate::obj::bounds::#assoc_memory;
type DynMemory = crate::obj::bounds::#assoc_dyn_memory;
type Declarer = crate::obj::bounds::DeclEngine;
type Exportable = crate::obj::bounds::#is_exportable;
}

#(
Expand Down Expand Up @@ -420,8 +421,10 @@ fn make_deref_impl(class_name: &TyName, base_ty: &TokenStream) -> TokenStream {
}
}

fn make_bounds(class: &Class) -> (Ident, Ident) {
let assoc_dyn_memory = if class.name().rust_ty == "Object" {
fn make_bounds(class: &Class, ctx: &mut Context) -> (Ident, Ident, Ident) {
let c = class.name();

let assoc_dyn_memory = if c.rust_ty == "Object" {
ident("MemDynamic")
} else if class.is_refcounted {
ident("MemRefCounted")
Expand All @@ -435,7 +438,14 @@ fn make_bounds(class: &Class) -> (Ident, Ident) {
ident("MemManual")
};

(assoc_memory, assoc_dyn_memory)
let tree = ctx.inheritance_tree();
let is_exportable = if tree.inherits(c, "Node") || tree.inherits(c, "Resource") {
ident("Yes")
} else {
ident("No")
};

(assoc_memory, assoc_dyn_memory, is_exportable)
}

fn make_class_methods(
Expand Down
25 changes: 24 additions & 1 deletion godot-core/src/obj/bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ use private::Sealed;
// Sealed trait

pub(super) mod private {
use super::{Declarer, DynMemory, Memory};
use super::{Declarer, DynMemory, Exportable, Memory};

// Bounds trait declared here for code locality; re-exported in crate::obj.

Expand Down Expand Up @@ -97,6 +97,12 @@ pub(super) mod private {
/// Whether this class is a core Godot class provided by the engine, or declared by the user as a Rust struct.
// TODO what about GDScript user classes?
type Declarer: Declarer;

/// True if *either* `T: Inherits<Node>` *or* `T: Inherits<Resource>` is fulfilled.
///
/// Enables `#[export]` for those classes.
#[doc(hidden)]
type Exportable: Exportable;
}

/// Implements [`Bounds`] for a user-defined class.
Expand Down Expand Up @@ -130,6 +136,7 @@ pub(super) mod private {
type Memory = <<$UserClass as $crate::obj::GodotClass>::Base as $crate::obj::Bounds>::Memory;
type DynMemory = <<$UserClass as $crate::obj::GodotClass>::Base as $crate::obj::Bounds>::DynMemory;
type Declarer = $crate::obj::bounds::DeclUser;
type Exportable = <<$UserClass as $crate::obj::GodotClass>::Base as $crate::obj::Bounds>::Exportable;
}
};
}
Expand Down Expand Up @@ -417,3 +424,19 @@ impl Declarer for DeclUser {
}
}
}

// ----------------------------------------------------------------------------------------------------------------------------------------------
// Exportable bounds (still hidden)

#[doc(hidden)]
pub trait Exportable: Sealed {}

#[doc(hidden)]
pub enum Yes {}
impl Sealed for Yes {}
impl Exportable for Yes {}

#[doc(hidden)]
pub enum No {}
impl Sealed for No {}
impl Exportable for No {}
38 changes: 26 additions & 12 deletions godot-core/src/obj/gd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -738,16 +738,27 @@ impl<T: GodotClass> GodotType for Gd<T> {

impl<T: GodotClass> ArrayElement for Gd<T> {
fn element_type_string() -> String {
match Self::export_hint().hint {
hint @ (PropertyHint::RESOURCE_TYPE | PropertyHint::NODE_TYPE) => {
format!(
"{}/{}:{}",
VariantType::OBJECT.ord(),
hint.ord(),
T::class_name()
)
}
_ => format!("{}:", VariantType::OBJECT.ord()),
// See also impl Export for Gd<T>.

let hint = if T::inherits::<classes::Resource>() {
Some(PropertyHint::RESOURCE_TYPE)
} else if T::inherits::<classes::Node>() {
Some(PropertyHint::NODE_TYPE)
} else {
None
};

// Exportable classes (Resource/Node based) include the {RESOURCE|NODE}_TYPE hint + the class name.
if let Some(export_hint) = hint {
format!(
"{variant}/{hint}:{class}",
variant = VariantType::OBJECT.ord(),
hint = export_hint.ord(),
class = T::class_name()
)
} else {
// Previous impl: format!("{variant}:", variant = VariantType::OBJECT.ord())
unreachable!("element_type_string() should only be invoked for exportable classes")
}
}
}
Expand Down Expand Up @@ -793,14 +804,17 @@ impl<T: GodotClass> Var for Gd<T> {
}
}

impl<T: GodotClass> Export for Gd<T> {
impl<T> Export for Gd<T>
where
T: GodotClass + Bounds<Exportable = bounds::Yes>,
{
fn export_hint() -> PropertyHintInfo {
let hint = if T::inherits::<classes::Resource>() {
PropertyHint::RESOURCE_TYPE
} else if T::inherits::<classes::Node>() {
PropertyHint::NODE_TYPE
} else {
PropertyHint::NONE
unreachable!("classes not inheriting from Resource or Node should not be exportable")
};

// Godot does this by default too; the hint is needed when the class is a resource/node,
Expand Down
1 change: 1 addition & 0 deletions godot-core/src/obj/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ unsafe impl Bounds for NoBase {
type Memory = bounds::MemManual;
type DynMemory = bounds::MemManual;
type Declarer = bounds::DeclEngine;
type Exportable = bounds::No;
}

/// Non-strict inheritance relationship in the Godot class hierarchy.
Expand Down
10 changes: 7 additions & 3 deletions godot-core/src/registry/property.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ use crate::meta::{FromGodot, GodotConvert, GodotType, PropertyHintInfo, ToGodot}
/// This does not require [`FromGodot`] or [`ToGodot`], so that something can be used as a property even if it can't be used in function
/// arguments/return types.

// We also mention #[export] here, because we can't control the order of error messages. Missing Export often also means missing Var trait,
// and so the Var error message appears first.
// on_unimplemented: we also mention #[export] here, because we can't control the order of error messages.
// Missing Export often also means missing Var trait, and so the Var error message appears first.
#[diagnostic::on_unimplemented(
message = "`#[var]` properties require `Var` trait; #[export] ones require `Export` trait",
label = "type cannot be used as a property",
Expand All @@ -41,7 +41,10 @@ pub trait Var: GodotConvert {
}

/// Trait implemented for types that can be used as `#[export]` fields.
// Mentioning both Var + Export: see above.
///
/// `Export` is only implemented for objects `Gd<T>` if either `T: Inherits<Node>` or `T: Inherits<Resource>`, just like GDScript.
/// This means you cannot use `#[export]` with `Gd<RefCounted>`, for example.
// on_unimplemented: mentioning both Var + Export; see above.
#[diagnostic::on_unimplemented(
message = "`#[var]` properties require `Var` trait; #[export] ones require `Export` trait",
label = "type cannot be used as a property",
Expand Down Expand Up @@ -414,6 +417,7 @@ mod export_impls {

// Dictionary: will need to be done manually once they become typed.
impl_property_by_godot_convert!(Dictionary);
impl_property_by_godot_convert!(Variant);

// Packed arrays: we manually implement `Export`.
impl_property_by_godot_convert!(PackedByteArray, no_export);
Expand Down
1 change: 1 addition & 0 deletions godot-macros/src/class/derive_godot_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult<TokenStream> {
type Memory = <<Self as ::godot::obj::GodotClass>::Base as ::godot::obj::Bounds>::Memory;
type DynMemory = <<Self as ::godot::obj::GodotClass>::Base as ::godot::obj::Bounds>::DynMemory;
type Declarer = ::godot::obj::bounds::DeclUser;
type Exportable = <<Self as ::godot::obj::GodotClass>::Base as ::godot::obj::Bounds>::Exportable;
}

#godot_init_impl
Expand Down
Loading