Move Deref/DerefMut out of T to Gd<T>; InstanceStorage safety

[Bromeon]

No more implicit access to base from inside the object. Access to base objects inside Gd<T> must go through the #[base] field. For detailed motivation, see #131. In summary:

• It's easier to keep the user object self and the Godot object self.base apart, when not all symbols are in one namespace.
• It's less tempting to do gd.bind_mut().get_position().
  • Now it would be gd.bind_mut().base.get_position() -- however one can as well do gd.get_position() directly.
• Users can implement their own custom Deref/DerefMut on T, we don't impose an implementation (see comment).
  • In particular, we don't fail compilation due to orphan rule, when T lives in another crate.

Instead, Deref and DerefMut are now implemented for all Gd<T> pointers, not just the engine ones. This means that calling Godot methods will be uniform, and there is always a clear separation between accessing the Godot object and the user object.

	user object	engine object
inside T	self	(not possible)
outside Gd<T>	gd.bind/bind_mut()	gd.method()

Also, InstanceStorage is now only accessed via shared-ref, which should fix some soundness issues with previously unbounded &mut. All its fields are immutable or have interior mutability.

Closes #131.

---

Outdated

Where I'm still not sure is, if we can provide Deref/DerefMut on Gd<T> into &T::Base and &mut T::Base.
This is different from the current behavior, where Deref/DerefMut was implemented on the user-object T.

A big unknown is especially the mutable case -- how would we avoid aliased &mut when two Gd<T> are concurrently mut-derefed?

#[derive(GodotClass)]
#[class(base=Node)]
struct MyClass {...}

let mut a: Gd<MyClass>= ...;
let mut b: Gd<MyClass> = a.share();

// using DerefMut:
let base_a: &mut Node = *a;
let base_b: &mut Node = *b;

We do already something similar for Gd<T> with engine types, but there, the engine instances (Node handles) have physically different locations. As of this PR's current state, they have one location (inside InstanceStorage) -- but maybe it would be possible to re-interpret the opaque object inside Gd directly (or if that doesn't work, create a new field with the Node itself).

(All in all it's rather an academic problem, as we're not actually mutating the engine object, but Rust's aliasing rules are potentially problematic here.)

---

Solution to the above: instead of storing a central Base<T> in the storage, as it was done in the earlier commit, we directly transmute the Gd.opaque field into T. This object is at a distinct memory location for each Gd pointer, as such there is no &mut aliasing when using DerefMut on aliased Gd pointers.

[GodotRust]

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-370

[lilizoey]

Cell isn't sync, so i dont think this is safe. we could make this an atomic integer instead.

[Bromeon]

Good catch! 👍

[Bromeon]

There's a cool trick for these things, btw:

fn enforce_sync<T: Sync>();

// never called
fn __static_type_check() {
    enforce_sync::<Type>();
}

I need to see if it's already applicable, if not I'd like to avoid spending time now to improve multi-threaded soundness, this needs more thought anyway...

[Bromeon]

I cannot apply this yet. Just discovered that T would need to be Sync and likely Send if used in a thread-shared Gd<T> under the threads feature. This will however mean adding those bounds in a ton of places, which I'm not gonna do now.

Possibly we could imply the bounds in something like

trait GodotClass: 'static + Sync + Send {
    ...
}

but then that would also affect engine types, not just user-defined T. Which I guess is the right move, anyway.

[lilizoey]

one possible solution to this is to have a sync version of Gd. which can only hold Sync rust objects (would we need Send? i guess the destructor takes the object out of the Gd, and i assume it can be called on another thread.)

which does sorta mirror how the std has sync and unsync versions of some types. like Rc vs Arc.

it also means we dont need a separate feature for thread safety, we'd just need people to use the right smart pointer. im not sure how this would need to work with Base though.

[Bromeon]

would we need Send? i guess the destructor takes the object out of the Gd, and i assume it can be called on another thread

Not just the destructor, but as long as bind_mut() still hands out a guard with DerefMut, one can do this:

// on thread 1:
let obj = MyClass { ... };
let ptr = Gd::new(obj);

// on thread 2:
let ptr: Gd<MyClass> // passed from thread 1
let guard = ptr.bind_mut();

let extracted = mem::take(*guard); // replaces with default
// do whatever on thread 2

Sync works with &T access + interior mutability, but not really with &mut because that is essentially a free pass to get the value...

---

it also means we dont need a separate feature for thread safety, we'd just need people to use the right smart pointer.

This whole topic probably needs more thought... In general, I'd also like to avoid accidental use of threads when there's no explicit opt-in, i.e. runtime validations in some places.

Having two types can also be unergonomic, requiring extra conversions, and less flexible API design. On GDScript there is no such distinction, so there may be friction at the boundary. Another option is also special impl <T: GodotClass + Sync> Gd<T> { ... } blocks provide functionality on top.