From 3b6480c5b0c968ad9f5a7cfb7ca494989be03629 Mon Sep 17 00:00:00 2001
From: Craig Robbins <kde.psych@gmail.com>
Date: Mon, 23 Feb 2015 16:25:14 +1000
Subject: [PATCH] Fix wrapDegrees family of functions

wrapDegrees() (renamed to modulo360f)
wrapDegrees_0_360
wrapDegrees_180

Minor errors were present in previous versions; see issue #2328
---
 src/network/packethandlers/server.cpp |  4 +-
 src/test.cpp                          | 44 +++++++++++---
 src/util/numeric.h                    | 83 +++++++++++++--------------
 3 files changed, 80 insertions(+), 51 deletions(-)

diff --git a/src/network/packethandlers/server.cpp b/src/network/packethandlers/server.cpp
index feb8d66be..137327e0b 100644
--- a/src/network/packethandlers/server.cpp
+++ b/src/network/packethandlers/server.cpp
@@ -543,8 +543,8 @@ void Server::handleCommand_PlayerPos(NetworkPacket* pkt)
 	v3f position((f32)ps.X / 100.0, (f32)ps.Y / 100.0, (f32)ps.Z / 100.0);
 	v3f speed((f32)ss.X / 100.0, (f32)ss.Y / 100.0, (f32)ss.Z / 100.0);
 
-	pitch = wrapDegrees(pitch);
-	yaw = wrapDegrees(yaw);
+	pitch = modulo360f(pitch);
+	yaw = modulo360f(yaw);
 
 	Player *player = m_env->getPlayer(pkt->getPeerId());
 	if (player == NULL) {
diff --git a/src/test.cpp b/src/test.cpp
index 3b7c75c6e..350cab127 100644
--- a/src/test.cpp
+++ b/src/test.cpp
@@ -147,15 +147,45 @@ struct TestBase
 
 struct TestUtilities: public TestBase
 {
+	inline float ref_WrapDegrees180(float f)
+	{
+		// This is a slower alternative to the wrapDegrees_180() function;
+		// used as a reference for testing
+		float value = fmodf(f + 180, 360);
+		if (value < 0)
+			value += 360;
+		return value - 180;
+	}
+
+	inline float ref_WrapDegrees_0_360(float f)
+	{
+		// This is a slower alternative to the wrapDegrees_0_360() function;
+		// used as a reference for testing
+		float value = fmodf(f, 360);
+		if (value < 0)
+			value += 360;
+		return value < 0 ? value + 360 : value;
+	}
+
+
 	void Run()
 	{
-		/*infostream<<"wrapDegrees(100.0) = "<<wrapDegrees(100.0)<<std::endl;
-		infostream<<"wrapDegrees(720.5) = "<<wrapDegrees(720.5)<<std::endl;
-		infostream<<"wrapDegrees(-0.5) = "<<wrapDegrees(-0.5)<<std::endl;*/
-		UASSERT(fabs(wrapDegrees(100.0) - 100.0) < 0.001);
-		UASSERT(fabs(wrapDegrees(720.5) - 0.5) < 0.001);
-		UASSERT(fabs(wrapDegrees(-0.5) - (-0.5)) < 0.001);
-		UASSERT(fabs(wrapDegrees(-365.5) - (-5.5)) < 0.001);
+		UASSERT(fabs(modulo360f(100.0) - 100.0) < 0.001);
+		UASSERT(fabs(modulo360f(720.5) - 0.5) < 0.001);
+		UASSERT(fabs(modulo360f(-0.5) - (-0.5)) < 0.001);
+		UASSERT(fabs(modulo360f(-365.5) - (-5.5)) < 0.001);
+
+		for (float f = -720; f <= -360; f += 0.25) {
+			UASSERT(fabs(modulo360f(f) - modulo360f(f + 360)) < 0.001);
+		}
+
+		for (float f = -1440; f <= 1440; f += 0.25) {
+			UASSERT(fabs(modulo360f(f) - fmodf(f, 360)) < 0.001);
+			UASSERT(fabs(wrapDegrees_180(f) - ref_WrapDegrees180(f)) < 0.001);
+			UASSERT(fabs(wrapDegrees_0_360(f) - ref_WrapDegrees_0_360(f)) < 0.001);
+			UASSERT(wrapDegrees_0_360(fabs(wrapDegrees_180(f) - wrapDegrees_0_360(f))) < 0.001);
+		}
+
 		UASSERT(lowercase("Foo bAR") == "foo bar");
 		UASSERT(trim("\n \t\r  Foo bAR  \r\n\t\t  ") == "Foo bAR");
 		UASSERT(trim("\n \t\r    \r\n\t\t  ") == "");
diff --git a/src/util/numeric.h b/src/util/numeric.h
index 7fc3d2167..42d9a87a9 100644
--- a/src/util/numeric.h
+++ b/src/util/numeric.h
@@ -184,57 +184,56 @@ inline void sortBoxVerticies(v3s16 &p1, v3s16 &p2) {
 }
 
 
-/*
-	See test.cpp for example cases.
-	wraps degrees to the range of -360...360
-	NOTE: Wrapping to 0...360 is not used because pitch needs negative values.
-*/
-inline float wrapDegrees(float f)
+/** Returns \p f wrapped to the range [-360, 360]
+ *
+ *  See test.cpp for example cases.
+ *
+ *  \note This is also used in cases where degrees wrapped to the range [0, 360]
+ *  is innapropriate (e.g. pitch needs negative values)
+ *
+ *  \internal functionally equivalent -- although precision may vary slightly --
+ *  to fmodf((f), 360.0f) however empirical tests indicate that this approach is
+ *  faster.
+ */
+inline float modulo360f(float f)
 {
-	// Take examples of f=10, f=720.5, f=-0.5, f=-360.5
-	// This results in
-	// 10, 720, -1, -361
-	int i = floor(f);
-	// 0, 2, 0, -1
-	int l = i / 360;
-	// NOTE: This would be used for wrapping to 0...360
-	// 0, 2, -1, -2
-	/*if(i < 0)
-		l -= 1;*/
-	// 0, 720, 0, -360
-	int k = l * 360;
-	// 10, 0.5, -0.5, -0.5
-	f -= float(k);
-	return f;
+	int sign;
+	int whole;
+	float fraction;
+
+	if (f < 0) {
+		f = -f;
+		sign = -1;
+	} else {
+		sign = 1;
+	}
+
+	whole = f;
+
+	fraction = f - whole;
+	whole %= 360;
+
+	return sign * (whole + fraction);
 }
 
-/* Wrap to 0...360 */
+
+/** Returns \p f wrapped to the range [0, 360]
+  */
 inline float wrapDegrees_0_360(float f)
 {
-	// Take examples of f=10, f=720.5, f=-0.5, f=-360.5
-	// This results in
-	// 10, 720, -1, -361
-	int i = floor(f);
-	// 0, 2, 0, -1
-	int l = i / 360;
-	// Wrap to 0...360
-	// 0, 2, -1, -2
-	if(i < 0)
-		l -= 1;
-	// 0, 720, 0, -360
-	int k = l * 360;
-	// 10, 0.5, -0.5, -0.5
-	f -= float(k);
-	return f;
+	float value = modulo360f(f);
+	return value < 0 ? value + 360 : value;
 }
 
-/* Wrap to -180...180 */
+
+/** Returns \p f wrapped to the range [-180, 180]
+  */
 inline float wrapDegrees_180(float f)
 {
-	f += 180;
-	f = wrapDegrees_0_360(f);
-	f -= 180;
-	return f;
+	float value = modulo360f(f + 180);
+	if (value < 0)
+		value += 360;
+	return value - 180;
 }
 
 /*