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

Unify string parameters across the repo #1037

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
10 changes: 5 additions & 5 deletions gdnative-async/src/rt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::marker::PhantomData;

use func_state::FuncState;
use gdnative_bindings::Object;
use gdnative_core::core_types::{GodotError, Variant};
use gdnative_core::core_types::{GodotError, GodotString, Variant};
use gdnative_core::init::InitHandle;
use gdnative_core::object::{Instance, SubClass, TInstance, TRef};

Expand Down Expand Up @@ -77,13 +77,13 @@ impl Context {
pub fn signal<C>(
&self,
obj: TRef<'_, C>,
signal: &str,
signal: impl Into<GodotString>,
) -> Result<future::Yield<Vec<Variant>>, GodotError>
where
C: SubClass<Object>,
{
let (future, resume) = future::make();
bridge::SignalBridge::connect(obj.upcast(), signal, resume)?;
bridge::SignalBridge::connect(obj.upcast(), signal.into(), resume)?;
Ok(future)
}
}
Expand All @@ -107,8 +107,8 @@ pub fn register_runtime_with_prefix<S>(handle: &InitHandle, prefix: S)
where
S: Display,
{
handle.add_class_as::<bridge::SignalBridge>(format!("{prefix}SignalBridge"));
handle.add_class_as::<func_state::FuncState>(format!("{prefix}FuncState"));
handle.add_class_as::<bridge::SignalBridge>(&format!("{prefix}SignalBridge"));
handle.add_class_as::<func_state::FuncState>(&format!("{prefix}FuncState"));
}

/// Releases all observers still in use. This should be called in the
Expand Down
4 changes: 2 additions & 2 deletions gdnative-async/src/rt/bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use once_cell::sync::OnceCell;
use parking_lot::Mutex;

use gdnative_bindings::{Object, Reference};
use gdnative_core::core_types::{GodotError, Variant, VariantArray};
use gdnative_core::core_types::{GodotError, GodotString, Variant, VariantArray};
use gdnative_core::export::user_data::{ArcData, Map};
use gdnative_core::export::{ClassBuilder, Method, NativeClass, NativeClassMethods, Varargs};
use gdnative_core::godot_site;
Expand Down Expand Up @@ -58,7 +58,7 @@ impl NativeClass for SignalBridge {
impl SignalBridge {
pub(crate) fn connect(
source: TRef<Object>,
signal: &str,
signal: GodotString,
resume: Resume<Vec<Variant>>,
) -> Result<(), GodotError> {
let mut pool = BRIDGES.get_or_init(Mutex::default).lock();
Expand Down
2 changes: 1 addition & 1 deletion gdnative-bindings/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use super::generated::{Engine, Node, SceneTree};
/// invariants must be observed for the resulting node during `'a`, if any.
///
/// [thread-safety]: https://docs.godotengine.org/en/stable/tutorials/threads/thread_safe_apis.html
pub unsafe fn autoload<'a, T>(name: &str) -> Option<TRef<'a, T>>
pub unsafe fn autoload<'a, T>(name: impl Into<NodePath>) -> Option<TRef<'a, T>>
where
T: SubClass<Node>,
{
Expand Down
6 changes: 0 additions & 6 deletions gdnative-core/src/export/class_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@ use crate::export::*;
use crate::object::NewRef;
use crate::private::get_api;

// TODO(#996): unify string parameters across all buiders
// Potential candidates:
// * &str
// * impl Into<GodotString>
// * impl Into<Cow<'a, str>>

/// Allows registration of exported properties, methods and signals.
///
/// See member functions of this class for usage examples.
Expand Down
18 changes: 10 additions & 8 deletions gdnative-core/src/export/class_registry.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use std::any::TypeId;
use std::borrow::Cow;
use std::collections::hash_map::Entry;
use std::collections::HashMap;
use std::fmt;
Expand All @@ -15,7 +14,7 @@ static CLASS_REGISTRY: Lazy<RwLock<HashMap<TypeId, ClassInfo>>> =

#[derive(Clone, Debug)]
pub(crate) struct ClassInfo {
pub name: Cow<'static, str>,
pub name: String,
pub init_level: InitLevel,
}

Expand All @@ -31,7 +30,7 @@ where
/// Returns the NativeScript name of the class `C` if it is registered.
/// Can also be used to validate whether or not `C` has been added using `InitHandle::add_class<C>()`
#[inline]
pub(crate) fn class_name<C: NativeClass>() -> Option<Cow<'static, str>> {
pub(crate) fn class_name<C: NativeClass>() -> Option<String> {
with_class_info::<C, _, _>(|i| i.name.clone())
}

Expand All @@ -41,8 +40,8 @@ pub(crate) fn class_name<C: NativeClass>() -> Option<Cow<'static, str>> {
/// The returned string should only be used for diagnostic purposes, not for types that the user
/// has to name explicitly. The format is not guaranteed.
#[inline]
pub(crate) fn class_name_or_default<C: NativeClass>() -> Cow<'static, str> {
class_name::<C>().unwrap_or_else(|| Cow::Borrowed(std::any::type_name::<C>()))
pub(crate) fn class_name_or_default<C: NativeClass>() -> String {
class_name::<C>().unwrap_or_else(|| std::any::type_name::<C>().into())
}

/// Registers the class `C` in the class registry, using a custom name at the given level.
Expand All @@ -51,14 +50,17 @@ pub(crate) fn class_name_or_default<C: NativeClass>() -> Cow<'static, str> {
/// Returns an error with the old `ClassInfo` if a conflicting entry for `C` was already added.
#[inline]
pub(crate) fn register_class_as<C: NativeClass>(
name: Cow<'static, str>,
name: &str,
init_level: InitLevel,
) -> Result<bool, RegisterError> {
let type_id = TypeId::of::<C>();
let mut registry = CLASS_REGISTRY.write();
match registry.entry(type_id) {
Entry::Vacant(entry) => {
entry.insert(ClassInfo { name, init_level });
entry.insert(ClassInfo {
name: name.into(),
init_level,
});
Ok(true)
}
Entry::Occupied(entry) => {
Expand Down Expand Up @@ -86,7 +88,7 @@ pub(crate) fn register_class_as<C: NativeClass>(

#[inline]
#[allow(dead_code)] // Currently unused on platforms with inventory support
pub(crate) fn types_with_init_level(allow: InitLevel, deny: InitLevel) -> Vec<Cow<'static, str>> {
pub(crate) fn types_with_init_level(allow: InitLevel, deny: InitLevel) -> Vec<String> {
let registry = CLASS_REGISTRY.read();
let mut list = registry
.values()
Expand Down
4 changes: 2 additions & 2 deletions gdnative-core/src/export/property/invalid_accessor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ unsafe impl<'l, C: NativeClass, T> RawSetter<C, T> for InvalidSetter<'l> {
let mut set = sys::godot_property_set_func::default();

let data = Box::new(InvalidAccessorData {
class_name: class_registry::class_name_or_default::<C>().into_owned(),
class_name: class_registry::class_name_or_default::<C>(),
property_name: self.property_name.to_string(),
});

Expand All @@ -101,7 +101,7 @@ unsafe impl<'l, C: NativeClass, T> RawGetter<C, T> for InvalidGetter<'l> {
let mut get = sys::godot_property_get_func::default();

let data = Box::new(InvalidAccessorData {
class_name: class_registry::class_name_or_default::<C>().into_owned(),
class_name: class_registry::class_name_or_default::<C>(),
property_name: self.property_name.to_string(),
});

Expand Down
21 changes: 10 additions & 11 deletions gdnative-core/src/init/init_handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use crate::export::{
};
use crate::object::{GodotObject, RawObject, TRef};
use crate::private::get_api;
use std::borrow::Cow;
use std::ffi::CString;
use std::ptr;

Expand Down Expand Up @@ -43,7 +42,7 @@ impl InitHandle {
where
C: NativeClassMethods + StaticallyNamed,
{
self.add_maybe_tool_class_as_with::<C>(Cow::Borrowed(C::CLASS_NAME), false, |builder| {
self.add_maybe_tool_class_as_with::<C>(C::CLASS_NAME, false, |builder| {
C::nativeclass_register_monomorphized(builder);
f(builder);
})
Expand All @@ -64,7 +63,7 @@ impl InitHandle {
where
C: NativeClassMethods + StaticallyNamed,
{
self.add_maybe_tool_class_as_with::<C>(Cow::Borrowed(C::CLASS_NAME), true, |builder| {
self.add_maybe_tool_class_as_with::<C>(C::CLASS_NAME, true, |builder| {
C::nativeclass_register_monomorphized(builder);
f(builder);
})
Expand All @@ -75,7 +74,7 @@ impl InitHandle {
/// If the type implements [`StaticallyTyped`], that name is ignored in favor of the
/// name provided at registration.
#[inline]
pub fn add_class_as<C>(self, name: String)
pub fn add_class_as<C>(self, name: &str)
where
C: NativeClassMethods,
{
Expand All @@ -87,19 +86,19 @@ impl InitHandle {
/// If the type implements [`StaticallyTyped`], that name is ignored in favor of the
/// name provided at registration.
#[inline]
pub fn add_class_as_with<C>(self, name: String, f: impl FnOnce(&ClassBuilder<C>))
pub fn add_class_as_with<C>(self, name: &str, f: impl FnOnce(&ClassBuilder<C>))
where
C: NativeClassMethods,
{
self.add_maybe_tool_class_as_with::<C>(Cow::Owned(name), false, f)
self.add_maybe_tool_class_as_with::<C>(name, false, f)
}

/// Registers a new tool class to the engine
///
/// If the type implements [`StaticallyTyped`], that name is ignored in favor of the
/// name provided at registration.
#[inline]
pub fn add_tool_class_as<C>(self, name: String)
pub fn add_tool_class_as<C>(self, name: &str)
where
C: NativeClassMethods,
{
Expand All @@ -111,23 +110,23 @@ impl InitHandle {
/// If the type implements [`StaticallyTyped`], that name is ignored in favor of the
/// name provided at registration.
#[inline]
pub fn add_tool_class_as_with<C>(self, name: String, f: impl FnOnce(&ClassBuilder<C>))
pub fn add_tool_class_as_with<C>(self, name: &str, f: impl FnOnce(&ClassBuilder<C>))
where
C: NativeClassMethods,
{
self.add_maybe_tool_class_as_with::<C>(Cow::Owned(name), true, f)
self.add_maybe_tool_class_as_with::<C>(name, true, f)
}

#[inline]
fn add_maybe_tool_class_as_with<C>(
self,
name: Cow<'static, str>,
name: &str,
is_tool: bool,
f: impl FnOnce(&ClassBuilder<C>),
) where
C: NativeClassMethods,
{
let c_class_name = CString::new(&*name).unwrap();
let c_class_name = CString::new(name).unwrap();

match class_registry::register_class_as::<C>(name, self.init_level) {
Ok(true) => {}
Expand Down
3 changes: 2 additions & 1 deletion gdnative-core/src/object/instance.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::fmt::Debug;
use std::ptr::NonNull;

Expand Down Expand Up @@ -599,7 +600,7 @@ where
fn from_variant(variant: &Variant) -> Result<Self, FromVariantError> {
let owner = Ref::<T::Base, Shared>::from_variant(variant)?;
Self::from_base(owner).ok_or(FromVariantError::InvalidInstance {
expected: class_registry::class_name_or_default::<T>(),
expected: Cow::Owned(class_registry::class_name_or_default::<T>()),
})
}
}
Expand Down
4 changes: 2 additions & 2 deletions gdnative/src/globalscope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
//! [@GDScript]: https://docs.godotengine.org/en/stable/classes/[email protected]

use crate::api::{Resource, ResourceLoader};
use crate::core_types::NodePath;
use crate::core_types::GodotString;
use crate::object::{memory::RefCounted, GodotObject, Ref, SubClass};

#[doc(inline)]
Expand Down Expand Up @@ -52,7 +52,7 @@ pub use gdnative_core::globalscope::*;
/// let scene = load::<PackedScene>("res://path/to/Main.tscn").unwrap();
/// ```
#[inline]
pub fn load<T>(path: impl Into<NodePath>) -> Option<Ref<T>>
pub fn load<T>(path: impl Into<GodotString>) -> Option<Ref<T>>
where
T: SubClass<Resource> + GodotObject<Memory = RefCounted>,
{
Expand Down