Add WeakGd

[beicause]

Address #1245, #557.

[Bromeon]

Thanks, but have you read my reply in #1273 (comment)?

There is no concept of "weak Gd" in godot-rust. Every public-facing Gd is always strong.

There are some places where we internally use weak Gds, like inside Base; but that's mostly an implementation detail because I can't be bothered having yet another pointer type just for that 😀

But if it's public-facing, it would likely need to be its own type due to very different semantics, just like Rc/Weak or Arc/Weak. And this would add a lot of API + implementation surface for what is now one use case, and in that only a performance optimization (and that because of a Godot quirk).

While I admittedly could have been more explicit, I don't think adding niche pointer types is a good idea at this point, especially not before we've sorted out the thread-safe Gd pointers (#18). We will end up with a proliferation of slightly different pointers otherwise:

• Gd
• DynGd
• UniqueGd for multi-threaded sending
• SharedGd/SyncGd for multi-threaded sharing (possibly)
• SiMut & Co. for script instances

WeakGd adds an entirely other dimension, but would only support Gd, not any of the others. Furthermore, we'd need to rule out any confusion with the WeakRef class, which is rather useless in GDExtension.

---

Weak pointers could make sense in the future, but they need a solid motivation, and I'd prefer addressing the threading pointers first. So far, no one has requested them outside of this base-init issue. For #1245 in particular, we should first try to solve the problem using regular Gd; requiring to use different pointers depending on context adds a lot of complexity for users.

In general, please follow this rule in the Contributing Guidelines -- not just for us, but also to save time on your side 🙂

Larger changes need design

If you plan to make bigger contributions, make sure to discuss them in a GitHub issue before opening a pull request (PR). Since the library is evolving quickly, this avoids that multiple people work on the same thing, or that features don't integrate well, causing a lot of rework. Also don't hesitate to talk to the developers in the #contrib-gdext channel on Discord!

[Bromeon]

Note that users can also implement WeakGd with a few lines of code:

struct WeakGd<T> {
    id: InstanceId,
    _ph: PhantomData<*mut T>,
}

impl<T: GodotClass> WeakGd<T> {
    fn downgrade_from(gd: &Gd<T>) -> Self {
        Self { id: gd.instance_id(), _ph: PhantomData }
    }

    fn upgrade(&self) -> Option<Gd<T>> {
        Gd::try_from_instance_id(self.id).ok()
    }
}

This may not work for the base-init problem, but would solve most other use cases for weak pointers, and follow a similar pattern as std::rc::Weak + std::sync::Weak. It supports both manually-managed and ref-counted classes, with upgrade() failing if the object is no longer alive.

[Yarwin]

I would also add that NOTIFICATION_PREDELETE should be preferred over user-defined drop fn for GDExtension classes – it is guaranteed to work without weird side effects and will always be emitted at some specified, concrete point in Object lifecycle (unless someone decide to invalidate tons of existing Godot's projects) while we can't really say the same about the drop.

[beicause]

I would also add that NOTIFICATION_PREDELETE should be preferred over user-defined drop fn for GDExtension classes – it is guaranteed to work without weird side effects and will always be emitted at some specified, concrete point in Object lifecycle (unless someone decide to invalidate tons of existing Godot's projects) while we can't really say the same about the drop.

It's ok to access base class pointer in drop (deconstructor in c++ is commonly used instead of NOTIFICATION_PREDELETE)

• Edit: it's UB, I opened GDExtension: Undefined behavior when calling base class methods in deconstructor godotengine/godot#109762

I used to try NOTIFICATION_PREDELETE but it seems unreliable during hot reloading.

[beicause]

Note that users can also implement WeakGd with a few lines of code:

struct WeakGd<T> {
    id: InstanceId,
    _ph: PhantomData<*mut T>,
}

impl<T: GodotClass> WeakGd<T> {
    fn downgrade_from(gd: &Gd<T>) -> Self {
        Self { id: gd.instance_id(), _ph: PhantomData }
    }

    fn upgrade(&self) -> Option<Gd<T>> {
        Gd::try_from_instance_id(self.id).ok()
    }
}

This may not work for the base-init problem, but would solve most other use cases for weak pointers, and follow a similar pattern as std::rc::Weak + std::sync::Weak. It supports both manually-managed and ref-counted classes, with upgrade() failing if the object is no longer alive.

This PR just wants to resolve the base-init and base-drop problems, not really for WeakRef.

[Bromeon]

I used to try NOTIFICATION_PREDELETE but it seems unreliable during hot reloading.

What does that mean? The notification isn't emitted during a reload? Or just sometimes?

---

This PR just wants to resolve the base-init and base-drop problems, not really for WeakRef.

I understand, but it still adds a new symbol to the public API. Before we do add something as central as WeakGd, I need to be 100% convinced there's no way to solve those problems with existing means. The base-init solution in particular is "good enough" for me, only improvements I could imagine on that side would be:

• do immediate destruction in Rust-side init, deferred in GDScript-side
• upstream feature in Godot to have "true postinit" callbacks

The base-drop problem I still need to look into. Thanks for opening the Godot issue!

[Yarwin]

I used to try NOTIFICATION_PREDELETE but it seems unreliable during hot reloading.

Right! It is not unreliable thought – it is not being run at all because base instance is not being deleted, only the "inner" GDExtension class (unless it is a plugin which must be recreated 🙃).

Behavior of tool class

// Editor creates one instance and frees it instantly for _stuff_ 
hello from init! Instance id: 2007108762555
hello from predelete! instance id: 2007108762555
Hello from drop! instance id: 2007108762555
---------------
hello from init! Instance id: 2156342099367
// Hot reload - GDEXtension Instance is being dropped.
Hello from drop! instance id: 2156342099367
---------------
// New GDExtension instance. Same ID! Same base instance!
hello from init! Instance id: 2156342099367

// Dummy instance which is instantly freed for editor stuff
hello from init! Instance id: 2301985111656
hello from predelete! instance id: 2301985111656
Hello from drop! instance id: 2301985111656
----------------

You can hook into on_level_deinit to run your own deinit:

#[gdextension]
unsafe impl ExtensionLibrary for MyExtension {
    fn on_level_deinit(level: InitLevel) {
        if level == InitLevel::Editor {
            godot_print!("Editor is being deinitialized!");
            // Technically we must be in the editor to deinitialize the editor but old habits die hard.
            
            if Engine::singleton().is_editor_hint() {
                if let Some(Ok(mut scene_tree)) = Engine::singleton().get_main_loop().map(Gd::try_cast::<SceneTree>)
                {
                    // (In case of refcounted you need to use other means than group, for example the signals)
                    scene_tree.call_group("EditorDeinitGroup", "editor_deinit", &[]);
                }
            }
        }
    }
}

(...)

#[godot_api]
impl INode for ClassWithGdSelfReceiver {
    fn on_notification(&mut self, what: NodeNotification) {
        if what == NodeNotification::PREDELETE {
            self.editor_deinit();
        }
    }

    fn ready(&mut self) {
        self.base_mut().add_to_group("EditorDeinitGroup");
    }
}

#[godot_api]
impl ClassWithGdSelfReceiver {
    #[func]
    fn editor_deinit(&mut self) {
        godot_print!("hello from deinit! Instance id: {}", self.base().instance_id_unchecked());
    }
}

to get desired results:

// Editor Placeholder, you know the drill
hello from init! Instance id: 2003837205240
hello from deinit! Instance id: 2003837205240
Hello from drop! instance id: 2003837205240
------------------
/// Our "tool" instance: 2151661255908
hello from init! Instance id: 2151661255908
// Hot reload
Editor is being deinitialized!
// Call nodes in group…
hello from deinit! Instance id: 2151661255908
// Drop "inner"
Hello from drop! instance id: 2151661255908
------------------
// Our "tool" instance - 2151661255908 - is getting new "inner".
hello from init! Instance id: 2151661255908

// Editor placeholder.
hello from init! Instance id: 2239557090725
hello from deinit! Instance id: 2239557090725
Hello from drop! instance id: 2239557090725
-------------------

// Hot reload 2.
Editor is being deinitialized!
hello from deinit! Instance id: 2151661255908
Hello from drop! instance id: 2151661255908
-------------------
hello from init! Instance id: 2151661255908

hello from init! Instance id: 2289133763916
hello from deinit! Instance id: 2289133763916
Hello from drop! instance id: 2289133763916
--------------------

My point is – we can't really guarantee any state of the app in drop, because it is fully dependent on upstream and minor details have been already changed in the past. Godot guarantees the order of an Object lifecycle, and I'm pretty sure that it won't rugpull our instances before we deinit our library (it would be ultra silly).