diff --git a/examples/dodge-the-creeps/rust/src/main_scene.rs b/examples/dodge-the-creeps/rust/src/main_scene.rs index 6e1c0f803..901cb3790 100644 --- a/examples/dodge-the-creeps/rust/src/main_scene.rs +++ b/examples/dodge-the-creeps/rust/src/main_scene.rs @@ -100,13 +100,12 @@ impl Main { rng.gen_range(mob.min_speed..mob.max_speed) }; - let mut mob = mob.base_mut(); mob.set_linear_velocity(Vector2::new(range, 0.0)); let lin_vel = mob.get_linear_velocity().rotated(real::from_f32(direction)); mob.set_linear_velocity(lin_vel); let mut hud = self.base.get_node_as::("Hud"); - hud.base_mut().connect( + hud.connect( "start_game".into(), Callable::from_object_method(mob, "on_start_game"), ); diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs index 2d6ccd14e..48cc6c04b 100644 --- a/godot-core/src/obj/gd.rs +++ b/godot-core/src/obj/gd.rs @@ -160,36 +160,6 @@ where GdMut::from_cell(self.storage().get_mut()) } - /// Access base without locking the user object. - /// - /// If you need mutations, use [`base_mut()`][Self::base_mut]. If you need a value copy, use [`base().share()`][Share::share]. - pub fn base(&self) -> &Gd { - self.storage().base().deref() - } - - /// Access base mutably without locking the user object. - /// - /// The idea is to support the `gd.base_mut().mutating_method()` pattern, which is not allowed with `gd.base()`. - /// If you need a copy of a `Gd` object, use `base().share()`. - /// - /// The return type is currently `Gd` and not `&mut Gd` because of `&mut` aliasing restrictions. This may change in the future. - pub fn base_mut(&mut self) -> Gd { - // Note: we cannot give safe mutable access to a base without RefCell, because multiple Gd pointers could call base_mut(), - // leading to aliased &mut. And we don't want RefCell, as C++ objects (nodes etc.) don't underly Rust's exclusive-ref limitations. - // The whole point of the Gd::base*() methods are to not require runtime-exclusive access to the Rust object. - // - // This is not a problem when accessing the `#[base]` field of a user struct directly, because `self` is guarded by the - // RefCell/RwLock in the InstanceStorage. - // - // Here, we instead return a copy (for now), which for the user looks mostly the same. The idea is that: - // - gd.base().mutating_method() fails - // - gd.base_mut().mutating_method() works. - // - // Downside: small ref-counting overhead for `RefCounted` types. If this is an issue in a real game (highly unlikely), - // the user could always cache Gd base pointers. - self.storage().base().share() - } - /// Storage object associated with the extension instance. pub(crate) fn storage(&self) -> &InstanceStorage { // SAFETY: instance pointer belongs to this instance. We only get a shared reference, no exclusive access, so even @@ -541,16 +511,16 @@ where } } -impl Deref for Gd -where - T: GodotClass, -{ - type Target = T; +impl Deref for Gd { + // Target is always an engine class: + // * if T is an engine class => T + // * if T is a user class => T::Base + type Target = <::Declarer as dom::Domain>::DerefTarget; fn deref(&self) -> &Self::Target { // SAFETY: // - // This relies on `Gd` having the layout as `Node3D` (as an example), + // This relies on `Gd.opaque` having the layout as `Node3D` (as an example), // which also needs #[repr(transparent)]: // // struct Gd { @@ -560,22 +530,21 @@ where // struct Node3D { // object_ptr: sys::GDExtensionObjectPtr, // } - unsafe { std::mem::transmute::<&OpaqueObject, &T>(&self.opaque) } + unsafe { std::mem::transmute::<&OpaqueObject, &Self::Target>(&self.opaque) } } } -impl DerefMut for Gd -where - T: GodotClass, -{ - fn deref_mut(&mut self) -> &mut T { +impl DerefMut for Gd { + fn deref_mut(&mut self) -> &mut Self::Target { // SAFETY: see also Deref // // The resulting `&mut T` is transmuted from `&mut OpaqueObject`, i.e. a *pointer* to the `opaque` field. // `opaque` itself has a different *address* for each Gd instance, meaning that two simultaneous // DerefMut borrows on two Gd instances will not alias, *even if* the underlying Godot object is the // same (i.e. `opaque` has the same value, but not address). - unsafe { std::mem::transmute::<&mut OpaqueObject, &mut T>(&mut self.opaque) } + // + // The `&mut self` guarantees that no other base access can take place for *the same Gd instance* (access to other Gds is OK). + unsafe { std::mem::transmute::<&mut OpaqueObject, &mut Self::Target>(&mut self.opaque) } } } diff --git a/godot-core/src/obj/traits.rs b/godot-core/src/obj/traits.rs index 89e28445f..cd16b73c9 100644 --- a/godot-core/src/obj/traits.rs +++ b/godot-core/src/obj/traits.rs @@ -260,6 +260,8 @@ pub mod dom { /// Trait that specifies who declares a given `GodotClass`. pub trait Domain: Sealed { + type DerefTarget; + #[doc(hidden)] fn scoped_mut(obj: &mut Gd, closure: F) -> R where @@ -271,6 +273,8 @@ pub mod dom { pub enum EngineDomain {} impl Sealed for EngineDomain {} impl Domain for EngineDomain { + type DerefTarget = T; + fn scoped_mut(obj: &mut Gd, closure: F) -> R where T: GodotClass, @@ -284,6 +288,8 @@ pub mod dom { pub enum UserDomain {} impl Sealed for UserDomain {} impl Domain for UserDomain { + type DerefTarget = T::Base; + fn scoped_mut(obj: &mut Gd, closure: F) -> R where T: GodotClass, diff --git a/godot-core/src/storage.rs b/godot-core/src/storage.rs index b34de73ae..6fdf59119 100644 --- a/godot-core/src/storage.rs +++ b/godot-core/src/storage.rs @@ -12,16 +12,17 @@ use std::any::type_name; #[derive(Copy, Clone, Debug)] pub enum Lifecycle { + // Warning: when reordering/changing enumerators, update match in AtomicLifecycle below Alive, Destroying, Dead, // reading this would typically already be too late, only best-effort in case of UB } #[cfg(not(feature = "threads"))] -pub use single_thread::*; +pub(crate) use single_thread::*; #[cfg(feature = "threads")] -pub use multi_thread::*; +pub(crate) use multi_thread::*; #[cfg(not(feature = "threads"))] mod single_thread { @@ -115,21 +116,46 @@ mod single_thread { #[cfg(feature = "threads")] mod multi_thread { use std::any::type_name; + use std::sync; use std::sync::atomic::{AtomicU32, Ordering}; - use std::{cell, sync}; use crate::obj::{Base, GodotClass}; - use crate::{out, sys}; + use crate::out; use super::Lifecycle; + pub struct AtomicLifecycle { + atomic: AtomicU32, + } + + impl AtomicLifecycle { + pub fn new(value: Lifecycle) -> Self { + Self { + atomic: AtomicU32::new(value as u32), + } + } + + pub fn get(&self) -> Lifecycle { + match self.atomic.load(Ordering::Relaxed) { + 0 => Lifecycle::Alive, + 1 => Lifecycle::Dead, + 2 => Lifecycle::Destroying, + other => panic!("Invalid lifecycle {other}"), + } + } + + pub fn set(&self, value: Lifecycle) { + self.atomic.store(value as u32, Ordering::Relaxed); + } + } + /// Manages storage and lifecycle of user's extension class instances. pub struct InstanceStorage { user_instance: sync::RwLock, cached_base: Base, // Declared after `user_instance`, is dropped last - pub lifecycle: cell::Cell, + pub lifecycle: AtomicLifecycle, godot_ref_count: AtomicU32, } @@ -141,12 +167,12 @@ mod multi_thread { Self { user_instance: sync::RwLock::new(user_instance), cached_base, - lifecycle: cell::Cell::new(Lifecycle::Alive), + lifecycle: AtomicLifecycle::new(Lifecycle::Alive), godot_ref_count: AtomicU32::new(1), } } - pub(crate) fn on_inc_ref(&mut self) { + pub(crate) fn on_inc_ref(&self) { self.godot_ref_count.fetch_add(1, Ordering::Relaxed); out!( " Storage::on_inc_ref (rc={}) <{}>", // -- {:?}", @@ -156,7 +182,7 @@ mod multi_thread { ); } - pub(crate) fn on_dec_ref(&mut self) { + pub(crate) fn on_dec_ref(&self) { self.godot_ref_count.fetch_sub(1, Ordering::Relaxed); out!( " | Storage::on_dec_ref (rc={}) <{}>", // -- {:?}", @@ -195,7 +221,15 @@ mod multi_thread { pub(super) fn godot_ref_count(&self) -> u32 { self.godot_ref_count.load(Ordering::Relaxed) } + + // TODO enable once there is better thread support + //fn __static_type_check() { + // enforce_sync::>(); + //} } + + // Make sure storage is Sync in multi-threaded case, as it can be concurrently accessed through aliased Gd pointers. + //fn enforce_sync() {} } impl InstanceStorage { @@ -267,6 +301,9 @@ pub unsafe fn destroy_storage(instance_ptr: sys::GDExtensionClass let _drop = Box::from_raw(instance_ptr as *mut InstanceStorage); } +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// Callbacks + pub fn nop_instance_callbacks() -> sys::GDExtensionInstanceBindingCallbacks { // These could also be null pointers, if they are definitely not invoked (e.g. create_callback only passed to object_get_instance_binding(), // when there is already a binding). Current "empty but not null" impl corresponds to godot-cpp (wrapped.hpp). diff --git a/itest/rust/src/base_test.rs b/itest/rust/src/base_test.rs index 52e644458..494ae475b 100644 --- a/itest/rust/src/base_test.rs +++ b/itest/rust/src/base_test.rs @@ -27,12 +27,9 @@ fn base_instance_id() { fn base_access_unbound() { let mut obj = Gd::::new_default(); - { - let pos = Vector2::new(-5.5, 7.0); - obj.base_mut().set_position(pos); - - assert_eq!(obj.base().get_position(), pos); - } + let pos = Vector2::new(-5.5, 7.0); + obj.set_position(pos); + assert_eq!(obj.get_position(), pos); obj.free(); } @@ -42,12 +39,9 @@ fn base_access_unbound() { fn base_access_unbound_no_field() { let mut obj = Gd::::new_default(); - { - let pos = Vector2::new(-5.5, 7.0); - obj.base_mut().set_position(pos); - - assert_eq!(obj.base().get_position(), pos); - } + let pos = Vector2::new(-5.5, 7.0); + obj.set_position(pos); + assert_eq!(obj.get_position(), pos); obj.free(); } diff --git a/itest/rust/src/object_test.rs b/itest/rust/src/object_test.rs index c10cccba0..b9d3b4aa1 100644 --- a/itest/rust/src/object_test.rs +++ b/itest/rust/src/object_test.rs @@ -763,9 +763,6 @@ pub mod object_test_gd { #[derive(GodotClass)] #[class(base=Object)] pub struct CustomConstructor { - #[base] - base: Base, - #[var] pub val: i64, } @@ -774,7 +771,7 @@ pub mod object_test_gd { impl CustomConstructor { #[func] pub fn construct_object(val: i64) -> Gd { - Gd::with_base(|base| Self { base, val }) + Gd::with_base(|_base| Self { val }) } } } @@ -804,10 +801,7 @@ impl DoubleUse { #[derive(GodotClass)] #[class(init, base=Object)] -struct SignalEmitter { - #[base] - base: Base, -} +struct SignalEmitter {} #[godot_api] impl SignalEmitter { diff --git a/itest/rust/src/property_test.rs b/itest/rust/src/property_test.rs index 9c24617e3..1d401628e 100644 --- a/itest/rust/src/property_test.rs +++ b/itest/rust/src/property_test.rs @@ -15,28 +15,33 @@ use godot::{ #[derive(GodotClass)] #[class(base=Node)] struct HasProperty { - #[base] - base: Base, - #[var] int_val: i32, + #[var(get = get_int_val_read)] int_val_read: i32, + #[var(set = set_int_val_write)] int_val_write: i32, + #[var(get = get_int_val_rw, set = set_int_val_rw)] int_val_rw: i32, + #[var(get = get_int_val_getter, set)] int_val_getter: i32, + #[var(get, set = set_int_val_setter)] int_val_setter: i32, #[var(get = get_string_val, set = set_string_val)] string_val: GodotString, + #[var(get = get_object_val, set = set_object_val)] object_val: Option>, + #[var] texture_val: Gd, + #[var(get = get_texture_val, set = set_texture_val, hint = PROPERTY_HINT_RESOURCE_TYPE, hint_string = "Texture")] texture_val_rw: Option>, } @@ -120,7 +125,7 @@ impl HasProperty { #[godot_api] impl NodeVirtual for HasProperty { - fn init(base: Base) -> Self { + fn init(_base: Base) -> Self { HasProperty { int_val: 0, int_val_read: 2, @@ -132,7 +137,6 @@ impl NodeVirtual for HasProperty { string_val: GodotString::new(), texture_val: Texture::new(), texture_val_rw: None, - base, } } } diff --git a/itest/rust/src/signal_test.rs b/itest/rust/src/signal_test.rs index c0f834c6d..08f8c3c27 100644 --- a/itest/rust/src/signal_test.rs +++ b/itest/rust/src/signal_test.rs @@ -17,10 +17,7 @@ use crate::itest; #[derive(GodotClass)] #[class(init, base=Object)] -struct Emitter { - #[base] - base: Base, -} +struct Emitter {} #[godot_api] impl Emitter { @@ -81,11 +78,8 @@ fn signals() { let signal_name = format!("signal_{i}_arg"); let receiver_name = format!("receive_{i}_arg"); - emitter - .base_mut() - .connect(signal_name.clone().into(), receiver.callable(receiver_name)); - - emitter.base_mut().emit_signal(signal_name.into(), arg); + emitter.connect(signal_name.clone().into(), receiver.callable(receiver_name)); + emitter.emit_signal(signal_name.into(), arg); assert!(receiver.bind().used[i].get()); }