From df8cce0d617b18aa2b8eb72959f26250162511fc Mon Sep 17 00:00:00 2001 From: karol57 Date: Sun, 25 May 2014 20:12:53 +0200 Subject: [PATCH 1/5] Declarations and definitions are now separated. Added explicit int3(si32) c-tor. Operators like += now return reference to object. Operator != has now own implementation. Added dist2SQ for optimization proposes. areNeighbours now uses dist2SQ Removed unnecessary comments --- lib/int3.h | 245 +++++++++++++++++++++++++++++++++-------------------- 1 file changed, 154 insertions(+), 91 deletions(-) diff --git a/lib/int3.h b/lib/int3.h index 4b2a8ca22..4d2b6db81 100644 --- a/lib/int3.h +++ b/lib/int3.h @@ -1,109 +1,172 @@ #pragma once /* - * int3.h, part of VCMI engine - * - * Authors: listed in file AUTHORS in main folder - * - * License: GNU General Public License v2.0 or later - * Full text of license available in license.txt file, in main folder - * - */ +* int3.h, part of VCMI engine +* +* Authors: listed in file AUTHORS in main folder +* +* License: GNU General Public License v2.0 or later +* Full text of license available in license.txt file, in main folder +* +*/ + /// Class which consists of three integer values. Represents position on adventure map. class int3 { public: - si32 x,y,z; - inline int3():x(0),y(0),z(0){}; //c-tor, x/y/z initialized to 0 - inline int3(const si32 X, const si32 Y, const si32 Z):x(X),y(Y),z(Z){}; //c-tor - inline int3(const int3 & val) : x(val.x), y(val.y), z(val.z){} //copy c-tor - inline int3 & operator=(const int3 & val) {x = val.x; y = val.y; z = val.z; return *this;} //assignemt operator - ~int3() {} // d-tor - does nothing - inline int3 operator+(const int3 & i) const //returns int3 with coordinates increased by corresponding coordinate of given int3 - {return int3(x+i.x,y+i.y,z+i.z);} - inline int3 operator+(const si32 i) const //returns int3 with coordinates increased by given numer - {return int3(x+i,y+i,z+i);} - inline int3 operator-(const int3 & i) const //returns int3 with coordinates decreased by corresponding coordinate of given int3 - {return int3(x-i.x,y-i.y,z-i.z);} - inline int3 operator-(const si32 i) const //returns int3 with coordinates decreased by given numer - {return int3(x-i,y-i,z-i);} - inline int3 operator-() const //returns opposite position - {return int3(-x,-y,-z);} - inline double dist2d(const int3 &other) const //distance (z coord is not used) - {return std::sqrt((double)(x-other.x)*(x-other.x) + (y-other.y)*(y-other.y));} - inline bool areNeighbours(const int3 &other) const - {return dist2d(other) < 2. && z == other.z;} - inline void operator+=(const int3 & i) - { - x+=i.x; - y+=i.y; - z+=i.z; - } - inline void operator+=(const si32 & i) - { - x+=i; - y+=i; - z+=i; - } - inline void operator-=(const int3 & i) - { - x-=i.x; - y-=i.y; - z-=i.z; - } - inline void operator-=(const si32 & i) - { - x+=i; - y+=i; - z+=i; - } - inline bool operator==(const int3 & i) const - {return (x==i.x) && (y==i.y) && (z==i.z);} - inline bool operator!=(const int3 & i) const - {return !(*this==i);} - inline bool operator<(const int3 & i) const - { - if (zi.z) - return false; - if (yi.y) - return false; - if (xi.x) - return false; - return false; - } - inline std::string operator ()() const - { - return "(" + boost::lexical_cast(x) + - " " + boost::lexical_cast(y) + - " " + boost::lexical_cast(z) + ")"; - } - inline bool valid() const - { - return z >= 0; //minimal condition that needs to be fulfilled for tiles in the map - } - template void serialize(Handler &h, const int version) - { - h & x & y & z; - } - + si32 x, y, z; + + int3(); //c-tor, x/y/z initialized to 0 + explicit int3(const si32 i); //c-tor, x/y/z initialized to i + int3(const si32 X, const si32 Y, const si32 Z); //c-tor, x/y/z initialized to X/Y/Z + int3(const int3 & copy); + + int3 & operator=(const int3 & copy); + + int3 operator-() const; + + int3 operator+(const int3 & i) const; + int3 operator-(const int3 & i) const; + int3 operator+(const si32 i) const; //returns int3 with coordinates increased by given number + int3 operator-(const si32 i) const; //returns int3 with coordinates decreased by given number + + int3 & operator+=(const int3 & i); + int3 & operator-=(const int3 & i); + int3 & operator+=(const si32 i); //increases coordinates by given number + int3 & operator-=(const si32 i); //decreases coordinates by given number + + bool operator==(const int3 & i) const; + bool operator!=(const int3 & i) const; + + bool operator<(const int3 & i) const; + + si32 dist2dSQ(const int3 & other) const; //returns squared distance on Oxy plane (z coord is not used) + double dist2d(const int3 & other) const; //returns distance on Oxy plane + + bool areNeighbours(const int3 & other) const; + + std::string operator ()() const; //returns "(x y z)" string + + bool valid() const; + + template + void serialize(Handler &h, const int version); }; + +inline int3::int3() : x(0), y(0), z(0) {} // I think that x, y, z should be left uninitialized. +inline int3::int3(const si32 i) : x(i), y(i), z(i) {} +inline int3::int3(const si32 X, const si32 Y, const si32 Z) : x(X), y(Y), z(Z) {} +inline int3::int3(const int3 & c) : x(c.x), y(c.y), z(c.z) {} // Should be set to default (C++11)? + +inline int3 & int3::operator=(const int3 & c) // Should be set to default (C++11)? +{ + x = c.x; + y = c.y; + z = c.z; + + return *this; +} + +inline int3 int3::operator-() const { return int3(-x, -y, -z); } + +inline int3 int3::operator+(const int3 & i) const { return int3(x + i.x, y + i.y, z + i.z); } +inline int3 int3::operator-(const int3 & i) const { return int3(x - i.x, y - i.y, z - i.z); } +inline int3 int3::operator+(const si32 i) const { return int3(x + i, y + i, z + i); } +inline int3 int3::operator-(const si32 i) const { return int3(x - i, y - i, z - i); } + +inline int3 & int3::operator+=(const int3 & i) +{ + x += i.x; + y += i.y; + z += i.z; + return *this; +} +inline int3 & int3::operator-=(const int3 & i) +{ + x -= i.x; + y -= i.y; + z -= i.z; + return *this; +} +inline int3 & int3::operator+=(const si32 i) +{ + x += i; + y += i; + z += i; + return *this; +} +inline int3 & int3::operator-=(const si32 i) +{ + x -= i; + y -= i; + z -= i; + return *this; +} + +inline bool int3::operator==(const int3 & i) const { return (x == i.x && y == i.y && z == i.z); } +inline bool int3::operator!=(const int3 & i) const { return (x != i.x || y != i.y || z != i.z); } + +inline bool int3::operator<(const int3 & i) const +{ + if (z < i.z) + return true; + if (z > i.z) + return false; + if (y < i.y) + return true; + if (y > i.y) + return false; + if (x < i.x) + return true; + if (x > i.x) + return false; + return false; +} + +inline si32 int3::dist2dSQ(const int3 &o) const +{ + const si32 dx = (x - o.x); + const si32 dy = (y - o.y); + return dx*dx + dy*dy; +} +inline double int3::dist2d(const int3 &o) const +{ + return std::sqrt((double)dist2dSQ(o)); +} + +inline bool int3::areNeighbours(const int3 &o) const +{ + return (dist2dSQ(o) < 4) && (z == o.z); +} + +inline std::string int3::operator ()() const //Change to int3::toString()? +{ + std::string result("("); + result += boost::lexical_cast(x); result += ' '; + result += boost::lexical_cast(y); result += ' '; + result += boost::lexical_cast(z); result += ')'; + return result; +} + +template +inline void int3::serialize(Handler &h, const int version) { h & x & y & z; } + +inline bool int3::valid() const //Change name to isValid? +{ + return z >= 0; //minimal condition that needs to be fulfilled for tiles in the map +} + inline std::istream & operator>>(std::istream & str, int3 & dest) { - str>>dest.x>>dest.y>>dest.z; - return str; + return str >> dest.x >> dest.y >> dest.z; } inline std::ostream & operator<<(std::ostream & str, const int3 & sth) { - return str< Date: Sun, 25 May 2014 20:42:25 +0200 Subject: [PATCH 2/5] int3 dist2d microoptimization --- client/CSpellWindow.cpp | 21 ++++++++++++--------- lib/CGameState.cpp | 2 +- lib/CObjectHandler.cpp | 6 ++---- lib/mapping/CMap.cpp | 2 +- server/CGameHandler.cpp | 4 ++-- 5 files changed, 18 insertions(+), 17 deletions(-) diff --git a/client/CSpellWindow.cpp b/client/CSpellWindow.cpp index 8c43b9dae..1f88df9e2 100644 --- a/client/CSpellWindow.cpp +++ b/client/CSpellWindow.cpp @@ -744,23 +744,26 @@ void CSpellWindow::SpellArea::clickLeft(tribool down, bool previousState) if (h->getSpellSchoolLevel(CGI->spellh->objects[spell]) < 2) //not advanced or expert - teleport to nearest available city { - int nearest = -1; //nearest town's ID - double dist = -1; - for (int g=0; gcb->getTown((*nearest)->id)->pos.dist2dSQ(h->pos); + + for (auto i = nearest + 1; i != Towns.cend(); ++i) { - const CGTownInstance * dest = LOCPLINT->cb->getTown(Towns[g]->id); - double curDist = dest->pos.dist2d(h->pos); - if (nearest == -1 || curDist < dist) + const CGTownInstance * dest = LOCPLINT->cb->getTown((*i)->id); + si32 curDist = dest->pos.dist2dSQ(h->pos); + + if (curDist < dist) { - nearest = g; + nearest = i; dist = curDist; } } - if ( Towns[nearest]->visitingHero ) + + if ((*nearest)->visitingHero) LOCPLINT->showInfoDialog(CGI->generaltexth->allTexts[123]); else { - const CGTownInstance * town = LOCPLINT->cb->getTown(Towns[nearest]->id); + const CGTownInstance * town = LOCPLINT->cb->getTown((*nearest)->id); LOCPLINT->cb->castSpell(h, spell, town->visitablePos());// - town->getVisitableOffset()); } } diff --git a/lib/CGameState.cpp b/lib/CGameState.cpp index 137852662..3e9165e0d 100644 --- a/lib/CGameState.cpp +++ b/lib/CGameState.cpp @@ -1032,7 +1032,7 @@ void CGameState::initGrailPosition() && !t.visitable && t.terType != ETerrainType::WATER && t.terType != ETerrainType::ROCK - && map->grailPos.dist2d(int3(i,j,k)) <= map->grailRadious) + && map->grailPos.dist2dSQ(int3(i, j, k)) <= (map->grailRadious * map->grailRadious)) allowedPos.push_back(int3(i,j,k)); } } diff --git a/lib/CObjectHandler.cpp b/lib/CObjectHandler.cpp index 8e4a38861..75d2ce506 100644 --- a/lib/CObjectHandler.cpp +++ b/lib/CObjectHandler.cpp @@ -4173,13 +4173,13 @@ void CGTeleport::postInit() //matches subterranean gates into pairs const CGObjectInstance *cur = gatesSplit[0][i]; //find nearest underground exit - std::pair best(-1,150000); //pair + std::pair best(-1, 0x7FFFFFFF); //pair for(int j = 0; j < gatesSplit[1].size(); j++) { const CGObjectInstance *checked = gatesSplit[1][j]; if(!checked) continue; - double hlp = checked->pos.dist2d(cur->pos); + si32 hlp = checked->pos.dist2dSQ(cur->pos); if(hlp < best.second) { best.first = j; @@ -4193,9 +4193,7 @@ void CGTeleport::postInit() //matches subterranean gates into pairs gatesSplit[1][best.first] = nullptr; } else - { gates.push_back(std::make_pair(cur->id, ObjectInstanceID())); - } } objs.erase(Obj::SUBTERRANEAN_GATE); } diff --git a/lib/mapping/CMap.cpp b/lib/mapping/CMap.cpp index bd97fd117..2231fd5f2 100644 --- a/lib/mapping/CMap.cpp +++ b/lib/mapping/CMap.cpp @@ -424,7 +424,7 @@ const CGObjectInstance * CMap::getObjectiveObjectFrom(int3 pos, Obj::EObj type) bestMatch = object; else { - if (object->pos.dist2d(pos) < bestMatch->pos.dist2d(pos)) + if (object->pos.dist2dSQ(pos) < bestMatch->pos.dist2dSQ(pos)) bestMatch = object;// closer than one we already found } } diff --git a/server/CGameHandler.cpp b/server/CGameHandler.cpp index 3e833a7eb..30d709c33 100644 --- a/server/CGameHandler.cpp +++ b/server/CGameHandler.cpp @@ -5592,11 +5592,11 @@ bool CGameHandler::castSpell(const CGHeroInstance *h, SpellID spellID, const int if (h->getSpellSchoolLevel(s) < 2) { - double dist = town->pos.dist2d(h->pos); + si32 dist = town->pos.dist2dSQ(h->pos); ObjectInstanceID nearest = town->id; //nearest town's ID for(const CGTownInstance * currTown : gs->getPlayer(h->tempOwner)->towns) { - double curDist = currTown->pos.dist2d(h->pos); + si32 curDist = currTown->pos.dist2dSQ(h->pos); if (nearest == ObjectInstanceID() || curDist < dist) { nearest = town->id; From c676c5da82068d88ad38f0ae372ecf4a20ffdf35 Mon Sep 17 00:00:00 2001 From: karol57 Date: Sun, 25 May 2014 21:53:25 +0200 Subject: [PATCH 3/5] Fixed small error in nearest town finding algorithm. --- server/CGameHandler.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/CGameHandler.cpp b/server/CGameHandler.cpp index 30d709c33..19e91c912 100644 --- a/server/CGameHandler.cpp +++ b/server/CGameHandler.cpp @@ -5596,11 +5596,11 @@ bool CGameHandler::castSpell(const CGHeroInstance *h, SpellID spellID, const int ObjectInstanceID nearest = town->id; //nearest town's ID for(const CGTownInstance * currTown : gs->getPlayer(h->tempOwner)->towns) { - si32 curDist = currTown->pos.dist2dSQ(h->pos); - if (nearest == ObjectInstanceID() || curDist < dist) + si32 currDist = currTown->pos.dist2dSQ(h->pos); + if (currDist < dist) { - nearest = town->id; - dist = curDist; + nearest = currTown->id; + dist = currDist; } } if (town->id != nearest) From 5907735676eb746e267df7bcc07a95d3c1eab1ef Mon Sep 17 00:00:00 2001 From: karol57 Date: Mon, 26 May 2014 10:37:04 +0200 Subject: [PATCH 4/5] All inline functions are now part of definition. --- lib/int3.h | 256 ++++++++++++++++++++++++----------------------------- 1 file changed, 116 insertions(+), 140 deletions(-) diff --git a/lib/int3.h b/lib/int3.h index 4d2b6db81..ad62e4e8b 100644 --- a/lib/int3.h +++ b/lib/int3.h @@ -1,15 +1,14 @@ #pragma once /* -* int3.h, part of VCMI engine -* -* Authors: listed in file AUTHORS in main folder -* -* License: GNU General Public License v2.0 or later -* Full text of license available in license.txt file, in main folder -* -*/ - + * int3.h, part of VCMI engine + * + * Authors: listed in file AUTHORS in main folder + * + * License: GNU General Public License v2.0 or later + * Full text of license available in license.txt file, in main folder + * + */ /// Class which consists of three integer values. Represents position on adventure map. class int3 @@ -17,156 +16,133 @@ class int3 public: si32 x, y, z; - int3(); //c-tor, x/y/z initialized to 0 - explicit int3(const si32 i); //c-tor, x/y/z initialized to i - int3(const si32 X, const si32 Y, const si32 Z); //c-tor, x/y/z initialized to X/Y/Z - int3(const int3 & copy); + //c-tor: x, y, z initialized to 0 + int3() : x(0), y(0), z(0) {} // I think that x, y, z should be left uninitialized. + //c-tor: x, y, z initialized to i + explicit int3(const si32 i) : x(i), y(i), z(i) {} + //c-tor: x, y, z initialized to X, Y, Z + int3(const si32 X, const si32 Y, const si32 Z) : x(X), y(Y), z(Z) {} + int3(const int3 & c) : x(c.x), y(c.y), z(c.z) {} // Should be set to default (C++11)? - int3 & operator=(const int3 & copy); + int3 & operator=(const int3 & c) // Should be set to default (C++11)? + { + x = c.x; + y = c.y; + z = c.z; - int3 operator-() const; + return *this; + } + int3 operator-() const { return int3(-x, -y, -z); } - int3 operator+(const int3 & i) const; - int3 operator-(const int3 & i) const; - int3 operator+(const si32 i) const; //returns int3 with coordinates increased by given number - int3 operator-(const si32 i) const; //returns int3 with coordinates decreased by given number + int3 operator+(const int3 & i) const { return int3(x + i.x, y + i.y, z + i.z); } + int3 operator-(const int3 & i) const { return int3(x - i.x, y - i.y, z - i.z); } + //returns int3 with coordinates increased by given number + int3 operator+(const si32 i) const { return int3(x + i, y + i, z + i); } + //returns int3 with coordinates decreased by given number + int3 operator-(const si32 i) const { return int3(x - i, y - i, z - i); } - int3 & operator+=(const int3 & i); - int3 & operator-=(const int3 & i); - int3 & operator+=(const si32 i); //increases coordinates by given number - int3 & operator-=(const si32 i); //decreases coordinates by given number + int3 & operator+=(const int3 & i) + { + x += i.x; + y += i.y; + z += i.z; + return *this; + } + int3 & operator-=(const int3 & i) + { + x -= i.x; + y -= i.y; + z -= i.z; + return *this; + } + + //increases all coordinates by given number + int3 & operator+=(const si32 i) + { + x += i; + y += i; + z += i; + return *this; + } + //decreases all coordinates by given number + int3 & operator-=(const si32 i) + { + x -= i; + y -= i; + z -= i; + return *this; + } - bool operator==(const int3 & i) const; - bool operator!=(const int3 & i) const; + bool operator==(const int3 & i) const { return (x == i.x && y == i.y && z == i.z); } + bool operator!=(const int3 & i) const { return (x != i.x || y != i.y || z != i.z); } - bool operator<(const int3 & i) const; + bool operator<(const int3 & i) const + { + if (z < i.z) + return true; + if (z > i.z) + return false; + if (y < i.y) + return true; + if (y > i.y) + return false; + if (x < i.x) + return true; + if (x > i.x) + return false; + return false; + } - si32 dist2dSQ(const int3 & other) const; //returns squared distance on Oxy plane (z coord is not used) - double dist2d(const int3 & other) const; //returns distance on Oxy plane + //returns squared distance on Oxy plane (z coord is not used) + si32 dist2dSQ(const int3 & o) const + { + const si32 dx = (x - o.x); + const si32 dy = (y - o.y); + return dx*dx + dy*dy; + } + //returns distance on Oxy plane (z coord is not used) + double dist2d(const int3 & o) const + { + return std::sqrt((double)dist2dSQ(o)); + } - bool areNeighbours(const int3 & other) const; + bool areNeighbours(const int3 & o) const + { + return (dist2dSQ(o) < 4) && (z == o.z); + } - std::string operator ()() const; //returns "(x y z)" string + //returns "(x y z)" string + std::string operator ()() const //Change to int3::toString()? + { + std::string result("("); + result += boost::lexical_cast(x); result += ' '; + result += boost::lexical_cast(y); result += ' '; + result += boost::lexical_cast(z); result += ')'; + return result; + } - bool valid() const; + bool valid() const //Should be named "isValid"? + { + return z >= 0; //minimal condition that needs to be fulfilled for tiles in the map + } template - void serialize(Handler &h, const int version); + void serialize(Handler &h, const int version) + { + h & x & y & z; + } }; -inline int3::int3() : x(0), y(0), z(0) {} // I think that x, y, z should be left uninitialized. -inline int3::int3(const si32 i) : x(i), y(i), z(i) {} -inline int3::int3(const si32 X, const si32 Y, const si32 Z) : x(X), y(Y), z(Z) {} -inline int3::int3(const int3 & c) : x(c.x), y(c.y), z(c.z) {} // Should be set to default (C++11)? - -inline int3 & int3::operator=(const int3 & c) // Should be set to default (C++11)? -{ - x = c.x; - y = c.y; - z = c.z; - - return *this; -} - -inline int3 int3::operator-() const { return int3(-x, -y, -z); } - -inline int3 int3::operator+(const int3 & i) const { return int3(x + i.x, y + i.y, z + i.z); } -inline int3 int3::operator-(const int3 & i) const { return int3(x - i.x, y - i.y, z - i.z); } -inline int3 int3::operator+(const si32 i) const { return int3(x + i, y + i, z + i); } -inline int3 int3::operator-(const si32 i) const { return int3(x - i, y - i, z - i); } - -inline int3 & int3::operator+=(const int3 & i) -{ - x += i.x; - y += i.y; - z += i.z; - return *this; -} -inline int3 & int3::operator-=(const int3 & i) -{ - x -= i.x; - y -= i.y; - z -= i.z; - return *this; -} -inline int3 & int3::operator+=(const si32 i) -{ - x += i; - y += i; - z += i; - return *this; -} -inline int3 & int3::operator-=(const si32 i) -{ - x -= i; - y -= i; - z -= i; - return *this; -} - -inline bool int3::operator==(const int3 & i) const { return (x == i.x && y == i.y && z == i.z); } -inline bool int3::operator!=(const int3 & i) const { return (x != i.x || y != i.y || z != i.z); } - -inline bool int3::operator<(const int3 & i) const -{ - if (z < i.z) - return true; - if (z > i.z) - return false; - if (y < i.y) - return true; - if (y > i.y) - return false; - if (x < i.x) - return true; - if (x > i.x) - return false; - return false; -} - -inline si32 int3::dist2dSQ(const int3 &o) const -{ - const si32 dx = (x - o.x); - const si32 dy = (y - o.y); - return dx*dx + dy*dy; -} -inline double int3::dist2d(const int3 &o) const -{ - return std::sqrt((double)dist2dSQ(o)); -} - -inline bool int3::areNeighbours(const int3 &o) const -{ - return (dist2dSQ(o) < 4) && (z == o.z); -} - -inline std::string int3::operator ()() const //Change to int3::toString()? -{ - std::string result("("); - result += boost::lexical_cast(x); result += ' '; - result += boost::lexical_cast(y); result += ' '; - result += boost::lexical_cast(z); result += ')'; - return result; -} - -template -inline void int3::serialize(Handler &h, const int version) { h & x & y & z; } - -inline bool int3::valid() const //Change name to isValid? -{ - return z >= 0; //minimal condition that needs to be fulfilled for tiles in the map -} - -inline std::istream & operator>>(std::istream & str, int3 & dest) -{ - return str >> dest.x >> dest.y >> dest.z; -} inline std::ostream & operator<<(std::ostream & str, const int3 & sth) { return str << sth.x << ' ' << sth.y << ' ' << sth.z; } +inline std::istream & operator>>(std::istream & str, int3 & dest) +{ + return str >> dest.x >> dest.y >> dest.z; +} -//Change to normal function? +//Why not normal function? struct ShashInt3 { size_t operator()(int3 const& pos) const From 6a65802f2265e36f2cff4ab102e23155ef8794bf Mon Sep 17 00:00:00 2001 From: karol57 Date: Mon, 26 May 2014 11:52:27 +0200 Subject: [PATCH 5/5] Removed hardcoded value --- lib/CObjectHandler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/CObjectHandler.cpp b/lib/CObjectHandler.cpp index 75d2ce506..477331735 100644 --- a/lib/CObjectHandler.cpp +++ b/lib/CObjectHandler.cpp @@ -4173,7 +4173,7 @@ void CGTeleport::postInit() //matches subterranean gates into pairs const CGObjectInstance *cur = gatesSplit[0][i]; //find nearest underground exit - std::pair best(-1, 0x7FFFFFFF); //pair + std::pair best(-1, std::numeric_limits::max()); //pair for(int j = 0; j < gatesSplit[1].size(); j++) { const CGObjectInstance *checked = gatesSplit[1][j];