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 rotate_toward method to Vector2, Vector3, Quaternion, and Basis #82699

Closed
wants to merge 1 commit into from

Conversation

fire
Copy link
Member

@fire fire commented Oct 3, 2023

Trial pr to implement godotengine/godot-proposals#7965

core/math/vector2.h Outdated Show resolved Hide resolved
core/math/vector2.h Outdated Show resolved Hide resolved
@Chaosus Chaosus added this to the 4.2 milestone Oct 3, 2023
@ettiSurreal
Copy link
Contributor

Vector functions with separate magnitude move taken out.

	_FORCE_INLINE_ Vector2 rotate_toward(Vector2 p_to, real_t p_delta) {

		if (Math::is_zero_approx(p_rotation_delta)) {
			return *this;
		}

		if (p_delta < 0.0f) {
			p_delta = -p_delta;
			p_to = -p_to;
		}

		real_t angle = angle_to(p_to);

		if (angle < p_delta) {
			return p_to;
		}

		return slerp(p_to, p_delta / angle);
	}
	_FORCE_INLINE_ Vector3 rotate_toward(Vector3 p_to, float p_delta) {

		if (Math::is_zero_approx(p_delta)) {
			return *this;
		}

		if (p_delta < 0.0f) {
			p_delta = -p_delta;
			p_to = -p_to;
		}

		float angle = angle_to(p_to);

		if (angle < p_delta) {
			return p_to;
		}

		return slerp(p_to, p_delta / angle);
	}

@fire fire force-pushed the rotate_towards branch 4 times, most recently from 5a5826c to d90f714 Compare October 3, 2023 21:37
@ettiSurreal
Copy link
Contributor

@fire you mentioned on the chat (before I left the reviews yesterday) that you are abandoning this PR and were hoping the original author (I assume referring to me) would be able to take it on. I am supposed to make my own PR on this right?

@@ -189,6 +189,10 @@ struct _NO_DISCARD_ Basis {
rows[2].zero();
}

_FORCE_INLINE_ Basis rotate_toward(Basis p_to_basis, real_t p_delta) const {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_FORCE_INLINE_ Basis rotate_toward(Basis p_to_basis, real_t p_delta) const {
_FORCE_INLINE_ Basis rotate_toward(const Basis &p_to_basis, real_t p_delta) const {

<param index="0" name="to" type="Vector2" />
<param index="1" name="delta" type="float" />
<description>
Returns the result of rotating this vector towards [param to], by increment [param delta] (in radians).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Returns the result of rotating this vector towards [param to], by increment [param delta] (in radians).
Returns the result of rotating this vector toward [param to], by increment [param delta] (in radians).

"towards" is generally considered UK English, and "toward" is used in the line below so best to keep uniform

<param index="0" name="to" type="Vector3" />
<param index="1" name="delta" type="float" />
<description>
Returns the result of rotating this vector towards [param to], by increment [param delta] (in radians).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Returns the result of rotating this vector towards [param to], by increment [param delta] (in radians).
Returns the result of rotating this vector toward [param to], by increment [param delta] (in radians).

@fire
Copy link
Member Author

fire commented Oct 7, 2023

Superseded by: #82926

@fire fire closed this Oct 7, 2023
@fire fire added the archived label Oct 7, 2023
@AThousandShips AThousandShips removed this from the 4.2 milestone Oct 7, 2023
@fire fire deleted the rotate_towards branch November 30, 2023 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants