godot-rust / gdnative

Rust bindings for Godot 3
https://godot-rust.github.io
MIT License
3.61k stars 210 forks source link

`#[derive(NativeClass)]` does not support struct lifetimes #174

Closed WBosonic closed 1 year ago

WBosonic commented 5 years ago

When using lifetime parameters in a struct, I get these errors:

missing lifetime specifier at the declaration of the struct and use of undeclared lifetime name 'a at the impl block

minimal example:

#[macro_use]
extern crate gdnative

#[derive(gdnative::NativeClass)]
#[inherit(gdnative::Node)]
struct Test<'a> {
    this_is_a_reference: &'a i32,
}

#[methods]
impl<'a> Test<'a> {
    fn _init(_owner: gdnative::Node) -> Self {
        Test {
            this_is_a_reference: &1
        }
    }
}

Without the macros it works fine.

karroffel commented 5 years ago

Oh this is interesting. I think there is an implicit requirement that all types used as classes are 'static, which makes sense given that Godot controls the lifetime of those objects and is not aware of life-time mechanisms.

This should probably yield an error about the type having to be 'static but it also reminds me that it doesn't handle any types of arguments (type parameters, lifetimes, const generics) in any way.

WBosonic commented 5 years ago

Is it then recommended to use Rc<SomeType> instead of &'a SomeType?

I need the references in my struct to keep track of what needs to be updated, so I don't see a way to work around the lifetime issue without using Rc. It feels like a waste though, as a reference counted value is not needed.

karroffel commented 5 years ago

Who is owning the value you want to reference? Godot can't know the lifetimes of objects as they can be freely allocated and deallocated - it's not using RAII. With that in mind, it would be hard to prove that a reference inside an Object that's passed to Godot will definitely not outlive the owner. If you can't prove it you should use shared ownership, so Rc or Arc potentially with RefCell.

WBosonic commented 5 years ago

It looks like my idea was flawed anyways. My approach was originally this:

struct TerrainMesh<'a> {
    chunk_meshes: Hashmap<ChunkPosition, ChunkMesh>,
    needs_update: Vec<&'a ChunkMesh>
}

But it looks like rust doesn't like siblings that reference each other. Rc<Cell<ChunkMesh>> is probably the right choice here then.

karroffel commented 5 years ago

Ah yes, in this case you would need either Rc or use vector indices or something similar. Rust does not allow self-referential structs because moving would invalidate the pointers (references are just pointers). There is a new type, Pin, but I think even then you need some amount of unsafe to make this work.

So yes, Rc seems like a better choice here :)