Add Plane::invalid() (originally Default)

[you-win]

I checked the other Godot core types and they all look like they derive/impl default, so Plane is the only type that needed to be changed.

[GodotRust]

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

[Bromeon]

Thanks! Looks like setting all components to 0 also matches the GDScript behavior.

What I do wonder however, is this a good default to have? Such planes are always degenerate. What is the use case other than "others have it too"?

For Callable, we deliberately don't provide Default and instead have the Callable::invalid() constructor.

wrong button :D

[lilizoey]

What I do wonder however, is this a good default to have? Such planes are always degenerate. What is the use case other than "others have it too"?

Unlike Callable, Plane is a pure rust struct. And so imo it'd make sense for it to have a Default impl so it can work better with the rust ecosystem in general. Like for deriving Default on structs containing a plane or when storing say a Option<Plane> and using unwrap_or_default.

It might also for similar reasons make sense for Callable to implement Default, but it also doesn't make as much sense to use outside of godot.

On another note Rect2's Default impl returns a Rect2 with no size. which is also kinda useless.

[Bromeon]

Unlike Callable, Plane is a pure rust struct.

From the user point of view, that's an implementation detail. Having a default state is orthogonal to whether it's implemented in Rust or backed by the engine. We could use InnerPlane and the arguments would be the same.

---

And so imo it'd make sense for it to have a Default impl so it can work better with the rust ecosystem in general. Like for deriving Default on structs containing a plane or when storing say a Option<Plane> and using unwrap_or_default.

But when would you do that? If you have an Option<Plane>, you have already the perfect way to say "there is no plane", namely None. Returning an invalid plane is the Godot way of handling errors, and that's mostly because GDScript is limited. But it's annoying as a caller if we have language features like Option just for this purpose.

In general, having some traits not implemented potentially avoids errors in generic programming, because it forces the user to be conscious about invalid states. Sure, it can make a few situations slightly more complex, but I'd rather have people use Option<Plane> than Plane + abusing default as null state.

Think about it like this: if we designed our own Plane type from scratch, we'd likely not provide Default because there is no geometrically meaningful way to implement it (unlike vectors, for example). Even worse, subsequent operations like obtaining normal vectors yield invalid results, too. However we are binding to Godot, so maybe there is a need to construct invalid states for GDScript parity, but this could also be explicit like Callable::invalid(). This would strengthen the meaning of Default to "has a meaningful default value" and thus improve type safety.

[Bromeon]

On another note Rect2's Default impl returns a Rect2 with no size. which is also kinda useless.

The more I reflect upon it, the more I think we should revisit other builtin types.
Some spontaneous look:

Category	Default?	Meaning of default
Vectors	✔️	zero vector
Bounding boxes (Rects, AABB)	❔	maybe useful in situations where no area/volume is needed? however position is arbitrary
Matrices (basis, transform, projection, quat)	✔️	identity (transform returns input)
Plane	❌
Containers (array, packed array, dictionary)	✔️	empty
Strings	✔️	empty (although for node path, it's arguable; . might be a default, but that'd be different from Godot I think)
Callable	❌
Signal	❌
Rid	❔	possibly 0 or the invalid RID, but even this could be explicit
Variant	✔️	nil

[Bromeon]

@you-win Could you replace the Default derive with a named invalid() constructor + docs?

If you want, you can also do that for other types marked "❌", but it's also fine to stick to Plane in this PR.

[you-win]

Updated to add a new Plane::invalid() constructor and docs.

Default was manually implemented and it calls Plane::invalid(). This is due to the #[class(init)] attribute requiring Default be implemented on all bound fields.

[lilizoey]

Default was manually implemented and it calls Plane::invalid(). This is due to the #[class(init)] attribute requiring Default be implemented on all bound fields.

You can just do

#[derive(GodotClass)]
#[class(init)]
struct Foo {
  #[init(default = Plane::invalid())]
  plane: Plane,
}

like you would in other cases where there is no Default impl.

[Bromeon]

Yeah, this special need for #[init(default)] is not the nicest thing, but I still believe it's better to have this explicit.

If someone sees

  #[init(default = Plane::invalid())]
  plane: Plane,

, then it's immediately obvious that this field must be properly initialized somewhere. Otherwise, Option<Plane> is also possible.

[you-win]

Is the ask to remove the impl Default then?

I am personally not in favor since it removes the Plane() default constructor that I would expect from GDScript and instead replaces it with invalid().

[Bromeon]

Is the ask to remove the impl Default then?

Yes, exactly.

I am personally not in favor since it removes the Plane() default constructor that I would expect from GDScript and instead replaces it with invalid().

I presented the rationale in this reply. What are your arguments to keep it? To me, "because GDScript has it" is not a sufficient criterion for inclusion in Rust; we try to build a more type-safe API whenever possible.

[you-win]

What are your arguments to keep it? To me, "because GDScript has it" is not a sufficient criterion for inclusion in Rust; we try to build a more type-safe API whenever possible.

I am using Rust and GDext because I want to interact with Godot from Rust. I want things from Godot to work like they do in Godot. Type-safety is fine, but I do not think taking that choice away is the correct decision here.

Modeling Option<Plane> in GDScript is not possible, since Plane is one of Godot's core types. When doing something like var my_plane: Plane, the variable my_plane is implicitly set to Plane(). Using Option<Plane> is fine when going from Rust code -> Rust code, but what would you expect to happen when going from Rust -> GDScript?

Additionally, keeping the default impl keeps the API closer to the GDScript and C++ apis, which reduces the strain of context switching between all 3 languages. Coming from GDScript or C++ where Plane() is a valid constructor, there's unneeded friction between between using Plane::new(Vector3::default(), f32::default()) or having to find Plane::invalid().

Having a consistent API surface with the option for additional safety is great. Taking away that option is too opinionated, especially when the library is a binding to something else.

[Bromeon]

This reply tries to answer some considerations from a general API design perspective, and hopefully helps at understanding why certain things are done the way they are. For Plane specific discussion, see next reply.

---

Having a consistent API surface with the option for additional safety is great. Taking away that option is too opinionated, especially when the library is a binding to something else.

I see that point, and tend to agree that we should generally strive for exposing all possible Godot functionality in Rust.

Where I disagree however is that the API must be the same. This is already not the case for a ton of things:

• We don't have inheritance but Deref and upcasting.
• We don't have default parameters but fluent builders.
• Global-scope functions like sin, pow etc. may have subtle differences in implementation.
• Callable also has invalid() instead of Default::default().
• Gd<T> is non-nullable; GDScript objects are null by default.
• In GDScript, double free() shoots yourself in the foot; in Rust it panics.
• Rect2::intersection() returns Option<Rect2>, while GDScript returns a Rect2 that is degenerate.

This is by the way no different for any of the other Plane constructors. In Godot those are true constructors, while in Rust, they are associated functions such as new, from_normal_at_origin, from_points etc. You can consider invalid as just one of these constructors with a better name than default.

As I see it, the reason why GDScript must have a default for every built-in type is because it keeps the language simple, and GDScript is simply not powerful enough to express something like Option<T>. But in Rust we don't have this limitation, and I'm not sure if bringing our type safety to the level of GDScript is the right choice.

---

or having to find Plane::invalid()

That's a matter of documentation and discoverability. We can mention this at the top of Plane docs, if it helps.

Or even in the book, and make sure the invalid() is called the same for all ❌ builtin types.

---

I am using Rust and GDext because I want to interact with Godot from Rust. I want things from Godot to work like they do in Godot. Type-safety is fine, but I do not think taking that choice away is the correct decision here.

You, yes. One thing to consider though is that there's a balance between "how close are we to GDScript" and "how close are we to idiomatic Rust". A lot of users approach godot-rust from the Rust angle, meaning they expect Rust idioms and are equally surprised if we do things in a Godot-specific way that feels strange in Rust.

There's no general answer to this question; it's a case-by-case trade-off.

[Bromeon]

Modeling Option<Plane> in GDScript is not possible, since Plane is one of Godot's core types. When doing something like var my_plane: Plane, the variable my_plane is implicitly set to Plane(). Using Option<Plane> is fine when going from Rust code -> Rust code, but what would you expect to happen when going from Rust -> GDScript?

When mapping Option::<Plane>::None to GDScript, intuitively I would expect null (the nil variant) on the other side. But maybe others see this different.

The thing is, if you have null you immediately know that the plane is invalid. How do you test whether a Plane is invalid? Check whether d is zero? Compare it against Plane()? Certainly possible, but there's already 2 ways and both are less expressive than if plane == null.

---

TLDR: I'm not categorically saying Default makes no sense for Plane, but:

• we need to have it consistent for all built-in types
• there needs to be a good reason why it simplifies things
• there needs to be very little chance that people use it wrong and have to deal with invalid values

The 2nd and 3rd point are somewhat hard to estimate without seeing real-world code. So I'm interested, could you elaborate a bit on the concrete scenarios that made you miss a Default impl?

[lilizoey]

Modeling Option<Plane> in GDScript is not possible, since Plane is one of Godot's core types. When doing something like var my_plane: Plane, the variable my_plane is implicitly set to Plane(). Using Option<Plane> is fine when going from Rust code -> Rust code, but what would you expect to happen when going from Rust -> GDScript?

We can actually pretty easily support this now by implementing GodotFfiNullable like for instance:

impl GodotFfiNullable for Plane {
    fn flatten_option(opt: Option<Self>) -> Self {
        opt.unwrap_or(Plane::invalid())
    }
    fn is_null(&self) -> bool {
        self.normal.length() == 0
    }
}

Which would make Option<Plane> work in exporting such that None is Plane().

[you-win]

Updated to just contain the Plane::invalid method. I will not argue my case anymore but hope you will reconsider in the future.

[Bromeon]

We can actually pretty easily support this now by implementing GodotFfiNullable like for instance:

Interesting idea! And cool that the trait would support that 🙂

We'd probably still need to see if that makes things actually easier in practice, or if it's rather confusing because Option::None maps to null everywhere else. But good to know this would be an option (no pun intended).

---

Updated to just contain the Plane::invalid method. I will not argue my case anymore but hope you will reconsider in the future.

Thanks! I'm definitely interested in more use cases here, so feel free to raise them in the future as well.

---

Some more thoughts (no need to respond, but for reference later on):

Default docs explicitly mention:

A trait for giving a type a useful default value.

Generally, I don't think writing Plane::invalid() instead of Plane::default() is too much to ask, but I see that it can become more cumbersome when dealing with #[var] declarations, for example.

These are the problems that arise (this is not current proc-macro syntax):

// Would this be allowed?
#[export]
plane: Plane,

// But this is OK because explicit:
#[export(default = Plane::invalid())]
plane: Plane,

// Or even special syntax:
#[export(invalid_default)]
plane: Plane,

// This is also OK because default is None:
#[export]
plane: Option<Plane>,

We also have the exact same problem with Gd<T> currently, whose GDScript default null is not even representable as Gd<T>, only Option<Gd<T>>.

So this should be seen in a wider context. I'm also working on some onready syntax, and with all these things, initialization should probably be reconsidered as a whole. I'd like to be as ergonomic as possible while also prevent "forgotten initialization" errors.

[Bromeon]

Thanks a lot!