Object notification API, engine intra-doc links

[Bromeon]

So far, there was no way to intercept the Godot _notification API, and also issuing notifications was very cumbersome, as the constants were spread across several classes.

This PR introduces multiple new APIs:

1. {Class}Notification enum, which contains all notifications relevant for a given class.

   • For example, Node3DNotification accumulates all NOTIFICATION_* constants across Node3D, Node and Object classes.
   • In GDScript, this is less of an issue because there is true inheritance, so one can type Node3D.NOTIF... and auto-complete will list all candidates. In Rust, constants are not inherited.
   • There is currently a workaround for bug godot#75839: in two cases, two enumerators cannot be distinguished as they share the same integer value. We address this by using a single enumerator to represent both (not ideal, but there's nothing else we can do).

2. Virtual method {Class}Virtual::on_notification(what: {NearestClass}Notification).

   • This is the equivalent to GDScript's _notification (with underscore), but more meaningfully named. It can be used to intercept low-level notifications for the given class.
   • The parameter type is either the notification enum for that class; or if the class itself does not have any notification constants, the nearest base class that does. This avoids proliferation of enum types with the exact same interface. It's possibly breaking when Godot adds such constants in new classes, but it helps reduce the API surface if not every single class needs its own enum (since there are relatively few classes working with notifications).

3. The method {Class}::issue_notification(what: {NearestClass}Notification).

   • This is the counterpart of on_notification to send notifications.
   • In GDScript, this is named notification (no underscore).

---

Apart from that, there are smaller changes:

• Display is now implemented on all Gd<T> types, by using the Godot string representation. T's Display impl is ignored.
• Several generated symbols in godot::engine are now slighly documented and contain intra-doc links to navigate easily:
  • Class <-> virtual trait
  • Class <-> notification enum
  • Class <-> sidecar module
  • Class -> Godot online docs
  • Modules global, utilities and notify

[ttencate]

Bikeshed time!

• In a vacuum, on_notification is definitely better than _notification. But all the other virtual methods don't have their names changed (except stripping off the leading underscore). We don't write on_ready or on_enter_tree for example. So I would vote for keeping the original name notification for consistency.

• For the same reason I wouldn't rename the sending method, except that now we have a naming conflict if both are called notification. Maybe send_notification instead of issue_notification? Shorter and clearer. Or consider notify, which is consistent with SceneTree::notify_group{,_flags}.

• I like type safety as much as anyone, but isn't using enums overly constraining here? What happens if I call notify_group and this results in a Node2D receiving a notification that is defined only for Node3D? I see you've added an Unknown(i32) but that seems like a bit of a code smell. In practice, people will always use a non-exhaustive match with an _ arm, so what is the added value of an enum instead of a set of i32 constants?

[Bromeon]

Thanks for the feedback!

• In a vacuum, on_notification is definitely better than _notification. But all the other virtual methods don't have their names changed (except stripping off the leading underscore). We don't write on_ready or on_enter_tree for example. So I would vote for keeping the original name notification for consistency.

What is unfortunate is that the virtual method NodeVirtual::notification (receiver) would then have the same name as notification (sender) in GDScript. Since trait methods are in scope when defining a custom struct, someone may do this:

#[godot_api]
impl MyClass {
    fn stuff(&mut self) {
        // Send a notification to the tree
        self.notification(NodeNotification::PhysicsProcess);
        // BUG: completely bypassed Godot, just called the Rust method directly
    }
}

A on_ prefix makes clearer that it's a callback in this case. I think it's less of a problem for ready(), process() etc, as they have more specific names and are well-known for any Godot developers, but in theory we could also consider on_ prefixes for those.

---

For the same reason I wouldn't rename the sending method, except that now we have a naming conflict if both are called notification. Maybe send_notification instead of issue_notification? Shorter and clearer. Or consider notify, which is consistent with SceneTree::notify_group{,_flags}.

"send" is not great because other such occurrences in Godot are networking- or server-related.

"notify" sounds good. I was considering it as well, but thought it might not be clear enough. I didn't think of notify_group though, thanks!

---

I like type safety as much as anyone, but isn't using enums overly constraining here? What happens if I call notify_group and this results in a Node2D receiving a notification that is defined only for Node3D? I see you've added an Unknown(i32) but that seems like a bit of a code smell. In practice, people will always use a non-exhaustive match with an _ arm, so what is the added value of an enum instead of a set of i32 constants?

There is no easy way to know the NOTIFICATION_* constants that are relevant for any given class.

If we consider Node3D as an example, we have such constants spread over Node3D, Node and Object. Unlike GDScript, Rust does not inherit constants, so auto-completing Node3D::NOTIF... will not give an exhaustive list of constants. So a user would need to manually dig through the docs of all relevant base classes.

And even if Rust did inherit constants (or duplicate them), we'd still have the problem that a user would not be informed about additions in Godot. If I have an enum, a match branch will force me to list all the relevant constants. If Godot 4.1 now adds NOTIFICATION_HOUSE_ON_FIRE, this means my code must be updated to account for it, because it doesn't fall into the Unknown(...) branch. I can always use _ if I don't care about others. (This is generally a double-edged sword; what can be a feature for type safety may be in the way of forward-compatibility. So there's no clear "better").

Then there's also the self-descriptiveness. Numbers are terrible for printing, debugging, etc; whereas enumerators can derive Debug to have a descriptive string representation. Possibly this could be addressed with constants as well, by defining them as something like:

pub const NOTIFICATION_XY: Constant = Constant(73, "XY");

Last, there's also type safety. You cannot accidentally send a CanvasItem notification to a Node3D receiver.

---

But yeah, maybe some of the parts are overkill... notify_group also doesn't offer this level of type safety, however by its nature it can't (dynamic reflection call).

[ttencate]

#[godot_api]
impl MyClass {
    fn stuff(&mut self) {
        // Send a notification to the tree
        self.notification(NodeNotification::PhysicsProcess);
        // BUG: completely bypassed Godot, just called the Rust method directly
    }
}

Hmm, yes, that's nasty. If we name both of them notification, this will actually do the right thing, because the inherent impl takes precedence over the trait. Not sure if that is better, or creates new footguns and confusion... Edit Actually we are going through a Deref here, and then the trait seems to take precedence.

If I have an enum, a match branch will force me to list all the relevant constants. If Godot 4.1 now adds NOTIFICATION_HOUSE_ON_FIRE, this means my code must be updated to account for it, because it doesn't fall into the Unknown(...) branch. I can always use _ if I don't care about others.

My argument was that this is rarely, if ever, how _notification is used in practice. Nobody cares about handling all possible notifications. (Well, except maybe for debugging, but then they wouldn't match on it.) You just implement _notification because there's one specific notification you need to handle, and you discard all the others.

For similar reasons, a Debug implementation is also not very valuable. The code never asks "what notification is this?". It only asks "is it notification XY?". But if we really want a useful Debug, we can instead define struct Notification(pub i32); and generate a Debug impl that matches on the inner i32.

Incidentally, such a type would also give you a place to put all the constants, i.e. Notification::ENTER_TREE and so on, which is somewhat discoverable in autocomplete too. You'd also see all notifications that aren't relevant to your current class, but I don't think that's a problem.

At this point I'm one step away from inventing a single, global enum Notification type with an Unknown(i32) branch, which would also be fine I guess.

[Bromeon]

Hmm, yes, that's nasty. If we name both of them notification, this will actually do the right thing, because the inherent impl takes precedence over the trait. Not sure if that is better, or creates new footguns and confusion... Edit Actually we are going through a Deref here, and then the trait seems to take precedence.

Independent of Rust's resolution mechanism, 2 names in the "same" scope is a recipe for confusion. This might change dependent on use statements or whether Deref is applied, and it also makes it harder to identify a function call or find it in the documentation.

Having two separate names is a much smaller deal in my opinion. I mean, Godot's notification and _notification naming is not great in the first place... 😉

---

My argument was that this is rarely, if ever, how _notification is used in practice. Nobody cares about handling all possible notifications. (Well, except maybe for debugging, but then they wouldn't match on it.) You just implement _notification because there's one specific notification you need to handle, and you discard all the others.

Thanks, that's actually a very good point. Maybe the exhaustiveness doesn't matter at all.

---

Incidentally, such a type would also give you a place to put all the constants, i.e. Notification::ENTER_TREE and so on, which is somewhat discoverable in autocomplete too. You'd also see all notifications that aren't relevant to your current class, but I don't think that's a problem.

At this point I'm one step away from inventing a single, global enum Notification type with an Unknown(i32) branch, which would also be fine I guess.

This was my first thought and is completely impossible due to godotengine/godot#75839: there are loads of notifications that share the same integer value. Limiting the scope to a single class + its bases addresses most of the problem, but not the entire one (there are still collisions, but they are recognized as a bug and likely fixed at some point).

[ttencate]

So it can't work as a single enum, but can still work as a single newtype struct, albeit without a sensible Debug impl, right?

[Bromeon]

But that encounters all the problems I previously mentioned:

1. No way to easily know which notifications are relevant to be sent/received in a given class.
2. Multiple classes can define the same constant, with different values.
   • Window.NOTIFICATION_VISIBILITY_CHANGED == 30
   • CanvasItem.NOTIFICATION_VISIBILITY_CHANGED == 31
   • Node3D.NOTIFICATION_VISIBILITY_CHANGED == 43
3. No type safety, you can easily pass a notification of one class to another, and be left wondering at runtime why nothing happened.

---

In other words, we clearly need a scoped approach, because Godot's constants are not unique.

We can use constants instead of enums, but then:

• we need to have them per-class (for the scoping)
• we need to duplicate base constants in every single derived class (so the user knows which may be relevant)
  • hundreds of times the same symbol, good luck with reading docs
• we need to either give them a per-class type (for type safety) or say we don't care about intermixing
  • "don't care" is an option -- but why risk choosing the wrong constant, especially with colliding names?
  • It can easily happen that a user picks Window.NOTIFICATION_VISIBILITY_CHANGED instead of CanvasItem.NOTIFICATION_VISIBILITY_CHANGED, because of IDE suggestions, refactorings, whatever. This is an instant runtime bug, that would have been caught with my approach.

To me it seems while the enum solution I suggested may be a bit overkill, I don't see an alternative that solves these problems elegantly.

[ttencate]

Ow geez, I didn't realize number 2. What a mess. Yeah then your solution is probably best.

[Bromeon]

Renamed the two methods from issue_notification, issue_notification_reversed to notify, notify_reversed; left the other changes as-is. I wanted to clean up the commits a bit more, but with the merge conflicts, a rebase results in broken intermediate commits which all depict a different picture than at development time, so I don't think it's worth it.

bors r+

[bors]

Build succeeded:

• full-ci