Missing builder method for Basis::looking_at()

[NanamiNakano]

Ref: https://docs.godotengine.org/en/stable/classes/class_basis.html#class-basis-method-looking-at

basis.rs is manually written instead of generated, so it does not have a looking_at_ex() method.

This isn't a bug, but it would be very convenient to have it.

[Bromeon]

How do you currently use this method?

Same for Transform3D, by the way 🙂

[NanamiNakano]

I am currently manually filling fields with default values.

[NanamiNakano]

I've considered submitting a PR directly to manually add the missing builder to these methods. However, this change would be a little large, so I opened an issue here to discuss.

[Bromeon]

Some prior art:

Method with defaults	Rust API	Approach
GString::find/rfind/findn/rfindn	find_ex	Manual builder
GString::split/rsplit	split_ex	Manual builder
GString::count/countn	count, countn	Take all parameters
GString::format	format + format_with_placeholder	2 methods
GString::erase(pos, count=1)	erase(range)	Rust idiomatic range
GString::split_floats	split_floats	Take all parameters
GString::strip_edges	strip_edges	Take all parameters
Vector2::limit_length	limit_length(Some(float))	Option
Rect2::intersects	intersects + intersects_exclude_borders	2 methods
Plane::has_point(point, tol)	contains_point	Option

So you see, we decide case-by-case what's the most idiomatic in Rust. Builtins are a bit different from classes here as they tend to be used more commonly, at the same time there's a slight proliferation of very similar methods (like the GString::find* variants) that would lead to a mess with separate methods and builders.

Today, the default approach of the code generator for builtins is not to create the _ex builders, but rather provide the full signature.

[NanamiNakano]

Got it 👌. I either use all the fields or only fill in the fields without default values. So for me, I can simply write a helper method. No need to change the existing API.

[Bromeon]

So for me, I can simply write a helper method. No need to change the existing API.

No, that's not what I meant -- I agree that looking_at shouldn't require people to always specify use_model_front, as it's rarely needed.

It's more that we need to see which is the most appropriate API in this case 🙂

[NanamiNakano]

This looks good to me.

(pesudocode)

let basis = Basis::looking_at_ex(Vector3)
	.use_model_front() // Set to true, default false
	.up(Vector3) // Set to passed value, default (0,1,0)
	.done()

[Bromeon]

Sorry, this won't make it into v0.4 anymore. Due to its breaking change, we need to target the 0.5.0 release.

[Bromeon]

Looking at the table above, I wonder if we shouldn't choose the same approach as we do with classes, namely auto-generated _ex() methods + builder structs. We could still have manual definitions for APIs that are more idiomatic, like GString::find*/split*/erase, but these would be exceptions rather than the rule.

This would have the advantage that newly added methods or default parameters are automatically supported (we won't miss some), and the APIs of builtins are more consistent with those of classes, reducing mental overhead for users.