Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "clamped" method to Vector3 #30058

Closed
ghost opened this issue Jun 25, 2019 · 8 comments · Fixed by #45624
Closed

Add "clamped" method to Vector3 #30058

ghost opened this issue Jun 25, 2019 · 8 comments · Fixed by #45624

Comments

@ghost
Copy link

ghost commented Jun 25, 2019

Godot version
3.2-dev 6fbd045

I noticed Vector2 has a method named clamped, which returns the Vector2 with its length clamped to the length parameter. It would be nice to have the same for Vector3.

@lawnjelly
Copy link
Member

Very simple to add, do you have a use case in mind? I had a look and the Vector2 version is only used twice I think in the source, both times in joints_2d_sw.cpp. It is also exposed in gdscript.

@ghost
Copy link
Author

ghost commented Jun 25, 2019

Well, I usually clamp input direction vectors to 1 (as one should) and velocities to a max speed, among other things. I've been using if-statements to do this, but having a dedicated helper method for this would turn 2-3 lines of code into 1. Nothing essential, but it would be pretty convenient...

@akien-mga
Copy link
Member

Well, I usually clamp input direction vectors to 1 (as one should)

For this you should use direction.normalized(), not clamping (as direction vectors should always have length 1).

@ghost
Copy link
Author

ghost commented Jun 25, 2019

You'd want to clamp if you also wanted to read directional inputs from an analog stick (ignoring the fact that some controllers' sticks can go beyond a magnitude of 1 at intercardinal directions).

@Goutte
Copy link
Contributor

Goutte commented Jun 25, 2019

do you have a use case in mind?

When building geometries (meshes) procedurally, I extensively use normalize and then scale, which clamp is a shortcut for if I'm not mistaken. I don't especially need the perf boost, since those are run during dev only, but it'd be nice to get rid of my custom normalizeAndScale(Vector3):Vector3.

@lawnjelly
Copy link
Member

lawnjelly commented Jun 25, 2019

I've certainly used 'Normalize and scale' more often than the clamped method, and I'd argue for that before clamped which seems to be a special case.

The existing clamped 2d function is this:

Vector2 Vector2::clamped(real_t p_len) const {

	real_t l = length();
	Vector2 v = *this;
	if (l > 0 && p_len < l) {

		v /= l;
		v *= p_len;
	}

	return v;
}

NormalizeToScale is more like this:

Vector2 Vector2::normalize_to_scale(real_t p_len) const {
	real_t l = length();
	Vector2 v = *this;
	// or zero
	if (l > EPSILON) {
		v *= (p_len / l);
	}
	return v;
}

Although in practice I'd probably return something to indicate whether it was too small to normalize to catch errors. Can clamped use the same (p_len / l)? It's doing more operations .. unless it is for precision reasons.

@aaronfranke
Copy link
Member

See also: #13926 (comment)

The name of clamped conflicts if we wanted to add clamping similar to clamping floats and ints, so we may rename this in the future.

@khinbaptista
Copy link

I agree with this addition. The use cases are the same as Vector2.clamp, which is already there. If anything, not including Vector3.clamp is an inconsistency (which is what led me to this issue, I'm used to using the method in 2D and now in 3D I was surprised to see it didn't exist)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants