Ownership model.

[godot-rust]

Ownership model.

[karroffel]

I think all calls to Godot that return objects that inherit Reference should automatically return the RefCounted<T> type.

For everything else I would use Temporary<T>, because the most common behavior is that you only access those things temporarily and then let them be handled by Godot again.

Some functions, like Node::new() or similar would then need to be explicitly made into owned. Consider following two scenarios:
1:

let p = self.godot_parent();
let node = Node::new().make_owned();
// do some tests with Node, imagine unit tests
// node freed here

and 2:

let p = self.godot_parent();
let node = Node::new();
node.set_name("MyNode");
p.add_child(node);
// don't free node here since it's now managed by Godot

It would be very nice if Node::add_child() would specify that it actually takes control over the parameter, but then again if you free a node then it would notify the parent and the children as well, so it's not only managed by Godot then.

I think a good way to go is to let the programmer decide how to deal with it, because the type desired depends on the usage and isn't necessarily obvious.

---

TL;DR: I like this proposal a lot, should probably make all Reference returns into RefCounted<T> and everything else into Temporary<T>, letting the caller still take control over it if desired.

[nical]

Actually, I think that rather than adding an Owned<T> and Temporary it should be possible to use rust's Box<T> pointer, and use raw pointers *mut T when handing off objects to the engine. There is no way to make a memory-safe pointer type on top of aliased mutable references that are exposed by the engine and owned y the engine so might as well just use the basic stuff if we can.
It's probable that the only place where we'll get Box<T> is the return type of T::new() but at least this way ownership is explicitly given away (have to call Box::into_raw to convert to an unsafe pointer which consumes the box).

Edit: ...and just as I wrote this I realized that we can't use Box because godot and rust have separate allocators. Owned<T> it is then.

[nical]

I was under the impression that Foo::new() of non-reference-counted types could always return an Owned<Foo> although as soon as we want to pass the object to the engine we'd have to convert the Owned into a Temporary/Ref.

[nical]

So I have been experimenting with this. It's quite a bit harder to implement than I hoped because in order to be safe to use, the non-owning Ref types need a lifetime. This makes code generation very tricky because the API description doesn't provide enough information about ownership to know the correct lifetime of the non-reference types in function parameter or return position.

I'll keep digging.

[karroffel]

Since Godot doesn't guarantee anything for non-Resource types I tried to look for things that are always true and one of them seemed useful enough:

If Godot calls a Rust-script function, all objects passed in will outlive the function scope unless the function explicitly frees the passed-in objects. (assuming one thread running and Godot doens't just free stuff from other threads, but let's just assume it doesn't)

Problem is how this can be made into a proper ownership model, since the only safe thing to do is to never store references in self, since that might outlive the referenced objects.

Not storing references is a bit of a problem and can also cause performance degradations (doing get_node() in every _process because you are not allowed to store references).

Another idea would be to use a kind of non-reference-counted Ref<T> wrapper, which tracks if the object was freed or not. This can be done properly with the new NativeScript 1.1 functionality "instance binding data", which has to set callbacks for the first encounter of an object and the deletion of the object. The bindings could maintain some state stored on the heap which could be something like

enum GodotRefState<T> {
    Empty,
    Filled(*mut sys::godot_object, PhantomData<T>)
}

struct GodotRef<T> {
    state: Rc<GodotRefState<T>>,
}

Trying to access an object that's freed would result in a panic. Alternatively each ref can be checked for existence.

---

This is just some thoughts I had, I would still like a kind of Owned<>, Temporary<> and Ref<> kind of system, but I don't know how that could be implemented properly for now :(

[nical]

The rust bindings should definitely let you do the unsafe things that matter for performance. What's important is that unsafe things must be properly annotated as unsafe.

Another thing that I'll try out when I have a bit of time is to treat the basic pointers to non-reference types as unsafe (all methods are unsafe), but have the reference-counted type pointers expose these methods as safe directly instead of using the deref trick to simulate inheritance.

That way any interaction with reference counted objects (including methods inherited from non-reference types) are safe, and interactions with pointers to non-reference types are unsafe as they should since we can't prove that they are safe.

[Type1J]

If there are changes that you need in Godot to make the Rust binding use unsafe less often, now might be a good time: https://godotengine.org/article/headsup-vulkan-merged-master-branch-unstable

[karroffel]

@Type1J the only real improvement that can be made would be to have most (or all) types inherit from Reference. This could be discussed, but I remember that there was some opposition to that :/

[Type1J]

If you have a link to the issue that you're talking about, I'd be interested in reading the discussion.

[karroffel]

I think those discussions were mostly on IRC or in person. I can't quickly find an issue about it right now, sorry. There might be one but I missed it in my quick search.

Closed in #449. The Ref<T, Access> and TRef<'a, T, Access> smart pointers track uniqueness using the Access typestate, allowing static enforcement of uniqueness, and safe usage of certain kinds of references as receivers/arguments.