From e240cab5239084301e2fd5273a208c2877d6957a Mon Sep 17 00:00:00 2001 From: Woazboat Date: Mon, 27 Apr 2015 21:54:36 +0200 Subject: [PATCH 01/12] Removed redundant temp iterator. std::list.erase already returns iterator to next valid list element --- src/SetChunkData.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/SetChunkData.cpp b/src/SetChunkData.cpp index f2b58570d..e5afd96ed 100644 --- a/src/SetChunkData.cpp +++ b/src/SetChunkData.cpp @@ -131,11 +131,8 @@ void cSetChunkData::RemoveInvalidBlockEntities(void) ItemTypeToString(EntityBlockType).c_str(), EntityBlockType, ItemTypeToString(WorldBlockType).c_str(), WorldBlockType ); - cBlockEntityList::iterator itr2 = itr; - ++itr2; delete *itr; - m_BlockEntities.erase(itr); - itr = itr2; + itr = m_BlockEntities.erase(itr); } else { From 4e8b4981d877b28e0784f294341072fa3f279fb4 Mon Sep 17 00:00:00 2001 From: Woazboat Date: Tue, 28 Apr 2015 02:26:25 +0200 Subject: [PATCH 02/12] convert c style casts to c++ static casts --- src/SetChunkData.cpp | 2 +- src/Tracer.cpp | 29 +++++++++++++++-------------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/SetChunkData.cpp b/src/SetChunkData.cpp index e5afd96ed..7549b0dbf 100644 --- a/src/SetChunkData.cpp +++ b/src/SetChunkData.cpp @@ -103,7 +103,7 @@ void cSetChunkData::CalculateHeightMap(void) int index = cChunkDef::MakeIndexNoCheck(x, y, z); if (m_BlockTypes[index] != E_BLOCK_AIR) { - m_HeightMap[x + z * cChunkDef::Width] = (HEIGHTTYPE)y; + m_HeightMap[x + z * cChunkDef::Width] = static_cast(y); break; } } // for y diff --git a/src/Tracer.cpp b/src/Tracer.cpp index e604f4a5b..3f9501e1f 100644 --- a/src/Tracer.cpp +++ b/src/Tracer.cpp @@ -98,37 +98,38 @@ void cTracer::SetValues(const Vector3f & a_Start, const Vector3f & a_Direction) tDelta.z = 0; } + // start voxel coordinates - pos.x = (int)floorf(a_Start.x); - pos.y = (int)floorf(a_Start.y); - pos.z = (int)floorf(a_Start.z); + pos.x = static_cast(floorf(a_Start.x)); + pos.y = static_cast(floorf(a_Start.y)); + pos.z = static_cast(floorf(a_Start.z)); // calculate distance to first intersection in the voxel we start from if (dir.x < 0) { - tMax.x = ((float)pos.x - a_Start.x) / dir.x; + tMax.x = (static_cast(pos.x) - a_Start.x) / dir.x; } else { - tMax.x = (((float)pos.x + 1) - a_Start.x) / dir.x; + tMax.x = (static_cast(pos.x + 1) - a_Start.x) / dir.x; //TODO: Possible division by zero } if (dir.y < 0) { - tMax.y = ((float)pos.y - a_Start.y) / dir.y; + tMax.y = (static_cast(pos.y) - a_Start.y) / dir.y; } else { - tMax.y = (((float)pos.y + 1) - a_Start.y) / dir.y; + tMax.y = (static_cast(pos.y + 1) - a_Start.y) / dir.y; //TODO: Possible division by zero } if (dir.z < 0) { - tMax.z = ((float)pos.z - a_Start.z) / dir.z; + tMax.z = (static_cast(pos.z) - a_Start.z) / dir.z; } else { - tMax.z = (((float)pos.z + 1) - a_Start.z) / dir.z; + tMax.z = (static_cast(pos.z + 1) - a_Start.z) / dir.z; //TODO: Possible division by zero } } @@ -146,18 +147,18 @@ bool cTracer::Trace(const Vector3f & a_Start, const Vector3f & a_Direction, int SetValues(a_Start, a_Direction); - Vector3f End = a_Start + (dir * (float)a_Distance); + Vector3f End = a_Start + (dir * static_cast(a_Distance)); if (End.y < 0) { - float dist = -a_Start.y / dir.y; + float dist = -a_Start.y / dir.y; // No division by 0 possible End = a_Start + (dir * dist); } // end voxel coordinates - end1.x = (int)floorf(End.x); - end1.y = (int)floorf(End.y); - end1.z = (int)floorf(End.z); + end1.x = static_cast(floorf(End.x)); + end1.y = static_cast(floorf(End.y)); + end1.z = static_cast(floorf(End.z)); // check if first is occupied if (pos.Equals(end1)) From 797e3130d2ae721688b53dbaa2831d441ba0138a Mon Sep 17 00:00:00 2001 From: Woazboat Date: Tue, 28 Apr 2015 02:47:36 +0200 Subject: [PATCH 03/12] Check for zero length vector in Trace Added hasNonZeroLength member function to Vector3 --- src/Tracer.cpp | 14 ++++++++++---- src/Vector3.h | 5 +++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/Tracer.cpp b/src/Tracer.cpp index 3f9501e1f..816bc0fe8 100644 --- a/src/Tracer.cpp +++ b/src/Tracer.cpp @@ -56,6 +56,9 @@ float cTracer::SigNum(float a_Num) void cTracer::SetValues(const Vector3f & a_Start, const Vector3f & a_Direction) { + // Since this method should only be called by Trace, zero length vectors should already have been taken care of + ASSERT(a_Direction.hasNonZeroLength()); + // calculate the direction of the ray (linear algebra) dir = a_Direction; @@ -65,10 +68,8 @@ void cTracer::SetValues(const Vector3f & a_Start, const Vector3f & a_Direction) step.z = (int) SigNum(dir.z); // normalize the direction vector - if (dir.SqrLength() > 0.f) - { - dir.Normalize(); - } + dir.Normalize(); + // how far we must move in the ray direction before // we encounter a new voxel in x-direction @@ -139,6 +140,11 @@ void cTracer::SetValues(const Vector3f & a_Start, const Vector3f & a_Direction) bool cTracer::Trace(const Vector3f & a_Start, const Vector3f & a_Direction, int a_Distance, bool a_LineOfSight) { + if(!a_Direction.hasNonZeroLength()) + { + return false; + } + if ((a_Start.y < 0) || (a_Start.y >= cChunkDef::Height)) { LOGD("%s: Start Y is outside the world (%.2f), not tracing.", __FUNCTION__, a_Start.y); diff --git a/src/Vector3.h b/src/Vector3.h index 36f277ba4..f1fb2d257 100644 --- a/src/Vector3.h +++ b/src/Vector3.h @@ -78,6 +78,11 @@ public: ); } + inline bool hasNonZeroLength(void) const + { + return ((x != 0) || (y != 0) || (z != 0)); + } + inline double Length(void) const { return sqrt(static_cast(x * x + y * y + z * z)); From 496e5b10002900b4fb12b053523f83b943ef3806 Mon Sep 17 00:00:00 2001 From: Woazboat Date: Tue, 28 Apr 2015 02:51:21 +0200 Subject: [PATCH 04/12] Tracer signum function now returns int --- src/Tracer.cpp | 14 ++++++++------ src/Tracer.h | 3 ++- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/Tracer.cpp b/src/Tracer.cpp index 816bc0fe8..aa8689814 100644 --- a/src/Tracer.cpp +++ b/src/Tracer.cpp @@ -12,6 +12,8 @@ +const float FLOAT_EPSILON = 0.0001f; //TODO: Stash this in some header where it can be reused + cTracer::cTracer(cWorld * a_World): @@ -37,7 +39,7 @@ cTracer::~cTracer() -float cTracer::SigNum(float a_Num) +int cTracer::SigNum(float a_Num) { if (a_Num < 0.f) { @@ -63,9 +65,10 @@ void cTracer::SetValues(const Vector3f & a_Start, const Vector3f & a_Direction) dir = a_Direction; // decide which direction to start walking in - step.x = (int) SigNum(dir.x); - step.y = (int) SigNum(dir.y); - step.z = (int) SigNum(dir.z); + step.x = SigNum(dir.x); + step.y = SigNum(dir.y); + step.z = SigNum(dir.z); + // normalize the direction vector dir.Normalize(); @@ -302,8 +305,7 @@ int cTracer::intersect3D_SegmentPlane(const Vector3f & a_Origin, const Vector3f float D = a_PlaneNormal.Dot(u); // dot(Pn.n, u); float N = -(a_PlaneNormal.Dot(w)); // -dot(a_Plane.n, w); - const float EPSILON = 0.0001f; - if (fabs(D) < EPSILON) + if (fabs(D) < FLOAT_EPSILON) { // segment is parallel to plane if (N == 0.0) diff --git a/src/Tracer.h b/src/Tracer.h index ec87d449e..821131539 100644 --- a/src/Tracer.h +++ b/src/Tracer.h @@ -61,7 +61,8 @@ private: /// Return 1 through 6 for the following block faces, repectively: -x, -z, x, z, y, -y int GetHitNormal( const Vector3f & start, const Vector3f & end, const Vector3i & a_BlockPos); - float SigNum( float a_Num); + /// Signum function + int SigNum( float a_Num); cWorld* m_World; Vector3f m_NormalTable[6]; From 2f264dba7168f0e3519b3b53148442001642e309 Mon Sep 17 00:00:00 2001 From: Woazboat Date: Tue, 28 Apr 2015 02:54:45 +0200 Subject: [PATCH 05/12] Changed Vector3 Equals function to avoid using memcmp --- src/Vector3.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Vector3.h b/src/Vector3.h index f1fb2d257..ede6cc851 100644 --- a/src/Vector3.h +++ b/src/Vector3.h @@ -126,11 +126,7 @@ public: { // Perform a bitwise comparison of the contents - we want to know whether this object is exactly equal // To perform EPS-based comparison, use the EqualsEps() function - return ( - (memcmp(&x, &a_Rhs.x, sizeof(x)) == 0) && - (memcmp(&y, &a_Rhs.y, sizeof(y)) == 0) && - (memcmp(&z, &a_Rhs.z, sizeof(z)) == 0) - ); + return !((x != a_Rhs.x) || (y != a_Rhs.y) || (z != a_Rhs.z)); } inline bool EqualsEps(const Vector3 & a_Rhs, T a_Eps) const From 3d1bd544b08ab67a6d1e9c89fa66ad3c28426360 Mon Sep 17 00:00:00 2001 From: Woazboat Date: Wed, 29 Apr 2015 00:14:42 +0200 Subject: [PATCH 06/12] Changed Tracer::m_NormalTable to static array Was previously instantiated for every trace --- src/Tracer.cpp | 17 +++++++++++------ src/Tracer.h | 4 +++- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/Tracer.cpp b/src/Tracer.cpp index aa8689814..ad662aca7 100644 --- a/src/Tracer.cpp +++ b/src/Tracer.cpp @@ -15,16 +15,21 @@ const float FLOAT_EPSILON = 0.0001f; //TODO: Stash this in some header where it can be reused +const std::array cTracer::m_NormalTable = +{ + Vector3f(-1, 0, 0), // 1: -x + Vector3f( 0, 0, -1), // 2: -z + Vector3f( 1, 0, 0), // 3: +x + Vector3f( 0, 0, 1), // 4: +z + Vector3f( 0, 1, 0), // 5: +y + Vector3f( 0, -1, 0) // 6: -y +}; + + cTracer::cTracer(cWorld * a_World): m_World(a_World) { - m_NormalTable[0].Set(-1, 0, 0); - m_NormalTable[1].Set( 0, 0, -1); - m_NormalTable[2].Set( 1, 0, 0); - m_NormalTable[3].Set( 0, 0, 1); - m_NormalTable[4].Set( 0, 1, 0); - m_NormalTable[5].Set( 0, -1, 0); } diff --git a/src/Tracer.h b/src/Tracer.h index 821131539..ec23b340e 100644 --- a/src/Tracer.h +++ b/src/Tracer.h @@ -3,6 +3,8 @@ #include "Vector3.h" +#include + @@ -65,7 +67,7 @@ private: int SigNum( float a_Num); cWorld* m_World; - Vector3f m_NormalTable[6]; + static const std::array m_NormalTable; Vector3f dir; Vector3f tDelta; From 2359fc67055cadd369f6c52805778531efd47ecd Mon Sep 17 00:00:00 2001 From: Woazboat Date: Wed, 29 Apr 2015 00:27:15 +0200 Subject: [PATCH 07/12] Fix HasNonZeroLength name now 100% more cs compliant --- src/Tracer.cpp | 4 ++-- src/Vector3.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Tracer.cpp b/src/Tracer.cpp index ad662aca7..097726278 100644 --- a/src/Tracer.cpp +++ b/src/Tracer.cpp @@ -64,7 +64,7 @@ int cTracer::SigNum(float a_Num) void cTracer::SetValues(const Vector3f & a_Start, const Vector3f & a_Direction) { // Since this method should only be called by Trace, zero length vectors should already have been taken care of - ASSERT(a_Direction.hasNonZeroLength()); + ASSERT(a_Direction.HasNonZeroLength()); // calculate the direction of the ray (linear algebra) dir = a_Direction; @@ -148,7 +148,7 @@ void cTracer::SetValues(const Vector3f & a_Start, const Vector3f & a_Direction) bool cTracer::Trace(const Vector3f & a_Start, const Vector3f & a_Direction, int a_Distance, bool a_LineOfSight) { - if(!a_Direction.hasNonZeroLength()) + if(!a_Direction.HasNonZeroLength()) { return false; } diff --git a/src/Vector3.h b/src/Vector3.h index ede6cc851..6164721da 100644 --- a/src/Vector3.h +++ b/src/Vector3.h @@ -78,7 +78,7 @@ public: ); } - inline bool hasNonZeroLength(void) const + inline bool HasNonZeroLength(void) const { return ((x != 0) || (y != 0) || (z != 0)); } From ca5b3e5776c05a94c86b28b86227d90b8cef43c6 Mon Sep 17 00:00:00 2001 From: Woazboat Date: Wed, 29 Apr 2015 01:37:16 +0200 Subject: [PATCH 08/12] Fixed comments according to cs --- src/Tracer.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Tracer.cpp b/src/Tracer.cpp index 097726278..249d2a6d1 100644 --- a/src/Tracer.cpp +++ b/src/Tracer.cpp @@ -12,17 +12,17 @@ -const float FLOAT_EPSILON = 0.0001f; //TODO: Stash this in some header where it can be reused +const float FLOAT_EPSILON = 0.0001f; // TODO: Stash this in some header where it can be reused const std::array cTracer::m_NormalTable = { - Vector3f(-1, 0, 0), // 1: -x - Vector3f( 0, 0, -1), // 2: -z - Vector3f( 1, 0, 0), // 3: +x - Vector3f( 0, 0, 1), // 4: +z - Vector3f( 0, 1, 0), // 5: +y - Vector3f( 0, -1, 0) // 6: -y + Vector3f(-1, 0, 0), // 1: -x + Vector3f( 0, 0, -1), // 2: -z + Vector3f( 1, 0, 0), // 3: +x + Vector3f( 0, 0, 1), // 4: +z + Vector3f( 0, 1, 0), // 5: +y + Vector3f( 0, -1, 0) // 6: -y }; @@ -120,7 +120,7 @@ void cTracer::SetValues(const Vector3f & a_Start, const Vector3f & a_Direction) } else { - tMax.x = (static_cast(pos.x + 1) - a_Start.x) / dir.x; //TODO: Possible division by zero + tMax.x = (static_cast(pos.x + 1) - a_Start.x) / dir.x; // TODO: Possible division by zero } if (dir.y < 0) @@ -129,7 +129,7 @@ void cTracer::SetValues(const Vector3f & a_Start, const Vector3f & a_Direction) } else { - tMax.y = (static_cast(pos.y + 1) - a_Start.y) / dir.y; //TODO: Possible division by zero + tMax.y = (static_cast(pos.y + 1) - a_Start.y) / dir.y; // TODO: Possible division by zero } if (dir.z < 0) @@ -138,7 +138,7 @@ void cTracer::SetValues(const Vector3f & a_Start, const Vector3f & a_Direction) } else { - tMax.z = (static_cast(pos.z + 1) - a_Start.z) / dir.z; //TODO: Possible division by zero + tMax.z = (static_cast(pos.z + 1) - a_Start.z) / dir.z; // TODO: Possible division by zero } } @@ -165,7 +165,7 @@ bool cTracer::Trace(const Vector3f & a_Start, const Vector3f & a_Direction, int if (End.y < 0) { - float dist = -a_Start.y / dir.y; // No division by 0 possible + float dist = -a_Start.y / dir.y; // No division by 0 possible End = a_Start + (dir * dist); } From 723aca2bfa5b4baa019bcbd07199bdd5d58ca8ff Mon Sep 17 00:00:00 2001 From: Woazboat Date: Wed, 29 Apr 2015 01:39:08 +0200 Subject: [PATCH 09/12] And another cs fix I overlooked --- src/Tracer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tracer.cpp b/src/Tracer.cpp index 249d2a6d1..56229e443 100644 --- a/src/Tracer.cpp +++ b/src/Tracer.cpp @@ -148,7 +148,7 @@ void cTracer::SetValues(const Vector3f & a_Start, const Vector3f & a_Direction) bool cTracer::Trace(const Vector3f & a_Start, const Vector3f & a_Direction, int a_Distance, bool a_LineOfSight) { - if(!a_Direction.HasNonZeroLength()) + if (!a_Direction.HasNonZeroLength()) { return false; } From f7d88ff4e00d9ad61d7582f6b5fff97401bac1de Mon Sep 17 00:00:00 2001 From: Woazboat Date: Wed, 29 Apr 2015 02:21:37 +0200 Subject: [PATCH 10/12] Added extra braces to initialization of Tracer::m_NormalTable --- src/Tracer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Tracer.cpp b/src/Tracer.cpp index 56229e443..53daa4050 100644 --- a/src/Tracer.cpp +++ b/src/Tracer.cpp @@ -16,14 +16,14 @@ const float FLOAT_EPSILON = 0.0001f; // TODO: Stash this in some header where i const std::array cTracer::m_NormalTable = -{ +{{ Vector3f(-1, 0, 0), // 1: -x Vector3f( 0, 0, -1), // 2: -z Vector3f( 1, 0, 0), // 3: +x Vector3f( 0, 0, 1), // 4: +z Vector3f( 0, 1, 0), // 5: +y Vector3f( 0, -1, 0) // 6: -y -}; +}}; From d1ad08169e90ed3704d8e27aaa063fee4f6eed70 Mon Sep 17 00:00:00 2001 From: Woazboat Date: Wed, 29 Apr 2015 17:12:45 +0200 Subject: [PATCH 11/12] braces in Tracer::mNormalTable initializer in separate lines --- src/Tracer.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Tracer.cpp b/src/Tracer.cpp index 53daa4050..692772b04 100644 --- a/src/Tracer.cpp +++ b/src/Tracer.cpp @@ -16,14 +16,16 @@ const float FLOAT_EPSILON = 0.0001f; // TODO: Stash this in some header where i const std::array cTracer::m_NormalTable = -{{ +{ + { Vector3f(-1, 0, 0), // 1: -x Vector3f( 0, 0, -1), // 2: -z Vector3f( 1, 0, 0), // 3: +x Vector3f( 0, 0, 1), // 4: +z Vector3f( 0, 1, 0), // 5: +y Vector3f( 0, -1, 0) // 6: -y -}}; + } +}; From c31092c2d51087a495e44399a9d17375b168fcb1 Mon Sep 17 00:00:00 2001 From: Woazboat Date: Wed, 29 Apr 2015 17:17:43 +0200 Subject: [PATCH 12/12] Changed fabs() to std::abs() --- src/Tracer.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Tracer.cpp b/src/Tracer.cpp index 692772b04..b788d0861 100644 --- a/src/Tracer.cpp +++ b/src/Tracer.cpp @@ -86,7 +86,7 @@ void cTracer::SetValues(const Vector3f & a_Start, const Vector3f & a_Direction) // same but y-direction if (dir.x != 0.f) { - tDelta.x = 1 / fabs(dir.x); + tDelta.x = 1 / std::abs(dir.x); } else { @@ -94,7 +94,7 @@ void cTracer::SetValues(const Vector3f & a_Start, const Vector3f & a_Direction) } if (dir.y != 0.f) { - tDelta.y = 1 / fabs(dir.y); + tDelta.y = 1 / std::abs(dir.y); } else { @@ -102,7 +102,7 @@ void cTracer::SetValues(const Vector3f & a_Start, const Vector3f & a_Direction) } if (dir.z != 0.f) { - tDelta.z = 1 / fabs(dir.z); + tDelta.z = 1 / std::abs(dir.z); } else { @@ -312,7 +312,7 @@ int cTracer::intersect3D_SegmentPlane(const Vector3f & a_Origin, const Vector3f float D = a_PlaneNormal.Dot(u); // dot(Pn.n, u); float N = -(a_PlaneNormal.Dot(w)); // -dot(a_Plane.n, w); - if (fabs(D) < FLOAT_EPSILON) + if (std::abs(D) < FLOAT_EPSILON) { // segment is parallel to plane if (N == 0.0)