Merge pull request #3614 from IvanSavenko/sonarcloud_fixes_2

Sonarcloud fixes 2

AI/BattleAI/AttackPossibility.cpp

@@ -62,16 +62,12 @@ void DamageCache::buildDamageCache(std::shared_ptr<HypotheticBattle> hb, int sid
 
 int64_t DamageCache::getDamage(const battle::Unit * attacker, const battle::Unit * defender, std::shared_ptr<CBattleInfoCallback> hb)
 {
-	auto damage = damageCache[attacker->unitId()][defender->unitId()] * attacker->getCount();
+	bool wasComputedBefore = damageCache[attacker->unitId()].count(defender->unitId());
 
-	if(damage == 0)
+	if (!wasComputedBefore)
-	{
 		cacheDamage(attacker, defender, hb);
 
-		damage = damageCache[attacker->unitId()][defender->unitId()] * attacker->getCount();
+	return damageCache[attacker->unitId()][defender->unitId()] * attacker->getCount();
-	}
-
-	return static_cast<int64_t>(damage);
 }
 
 int64_t DamageCache::getOriginalDamage(const battle::Unit * attacker, const battle::Unit * defender, std::shared_ptr<CBattleInfoCallback> hb)

AI/BattleAI/BattleExchangeVariant.cpp

@@ -270,7 +270,7 @@ EvaluationResult BattleExchangeEvaluator::findBestTarget(
 	{
 		float score = evaluateExchange(ap, 0, targets, damageCache, hb);
 
-		if(score > result.score || (score == result.score && result.wait))
+		if(score > result.score || (vstd::isAlmostEqual(score, result.score) && result.wait))
 		{
 			result.score = score;
 			result.bestAttack = ap;

AI/Nullkiller/AIGateway.cpp

@@ -632,7 +632,8 @@ void AIGateway::showBlockingDialog(const std::string & text, const std::vector<C
 				auto topObj = objects.front()->id == hero->id ? objects.back() : objects.front();
 				auto objType = topObj->ID; // top object should be our hero
 				auto goalObjectID = nullkiller->getTargetObject();
-				auto ratio = (float)nullkiller->dangerEvaluator->evaluateDanger(target, hero.get()) / (float)hero->getTotalStrength();
+				auto danger = nullkiller->dangerEvaluator->evaluateDanger(target, hero.get());
+				auto ratio = static_cast<float>(danger) / hero->getTotalStrength();
 
 				answer = topObj->id == goalObjectID; // no if we do not aim to visit this object
 				logAi->trace("Query hook: %s(%s) by %s danger ratio %f", target.toString(), topObj->getObjectName(), hero.name, ratio);
@@ -648,7 +649,7 @@ void AIGateway::showBlockingDialog(const std::string & text, const std::vector<C
 				}
 				else if(objType == Obj::ARTIFACT || objType == Obj::RESOURCE)
 				{
-					bool dangerUnknown = ratio == 0;
+					bool dangerUnknown = danger == 0;
 					bool dangerTooHigh = ratio > (1 / SAFE_ATTACK_CONSTANT);
 
 					answer = !dangerUnknown && !dangerTooHigh;
@@ -1406,7 +1407,7 @@ void AIGateway::tryRealize(Goals::Trade & g) //trade
 	int accquiredResources = 0;
 	if(const CGObjectInstance * obj = cb->getObj(ObjectInstanceID(g.objid), false))
 	{
-		if(const IMarket * m = IMarket::castFrom(obj, false))
+		if(const auto * m = dynamic_cast<const IMarket*>(obj))
 		{
 			auto freeRes = cb->getResourceAmount(); //trade only resources which are not reserved
 			for(auto it = ResourceSet::nziterator(freeRes); it.valid(); it++)

AI/Nullkiller/Analyzers/DangerHitMapAnalyzer.cpp

@@ -226,7 +226,7 @@ void DangerHitMapAnalyzer::calculateTileOwners()
 				}
 			}
 
-			if(ourDistance == enemyDistance)
+			if(vstd::isAlmostEqual(ourDistance, enemyDistance))
 			{
 				hitMap[pos.x][pos.y][pos.z].closestTown = nullptr;
 			}

AI/Nullkiller/Goals/AbstractGoal.h

@@ -180,11 +180,7 @@ public:
 	{
 	}
 
-	virtual ~cannotFulfillGoalException() throw ()
+	const char * what() const noexcept override
-	{
-	};
-
-	const char * what() const throw () override
 	{
 		return msg.c_str();
 	}
@@ -203,11 +199,7 @@ public:
 		msg = goal->toString();
 	}
 
-	virtual ~goalFulfilledException() throw ()
+	const char * what() const noexcept override
-	{
-	};
-
-	const char * what() const throw () override
 	{
 		return msg.c_str();
 	}

AI/Nullkiller/Pathfinding/Rules/AILayerTransitionRule.cpp

@@ -131,7 +131,7 @@ namespace AIPathfinding
 		{
 			if(obj->ID != Obj::TOWN) //towns were handled in the previous loop
 			{
-				if(const IShipyard * shipyard = IShipyard::castFrom(obj))
+				if(const auto * shipyard = dynamic_cast<const IShipyard *>(obj))
 					shipyards.push_back(shipyard);
 			}
 		}

AI/VCAI/Goals/CollectRes.cpp

@@ -124,7 +124,9 @@ TSubgoal CollectRes::whatToDoToTrade()
 	ai->retrieveVisitableObjs(visObjs, true);
 	for(const CGObjectInstance * obj : visObjs)
 	{
-		if(const IMarket * m = IMarket::castFrom(obj, false); m && m->allowsTrade(EMarketMode::RESOURCE_RESOURCE))
+		const auto * m = dynamic_cast<const IMarket*>(obj);
+
+		if(m && m->allowsTrade(EMarketMode::RESOURCE_RESOURCE))
 		{
 			if(obj->ID == Obj::TOWN)
 			{

AI/VCAI/Pathfinding/Rules/AILayerTransitionRule.cpp

@@ -58,7 +58,7 @@ namespace AIPathfinding
 		{
 			if(obj->ID != Obj::TOWN) //towns were handled in the previous loop
 			{
-				if(const IShipyard * shipyard = IShipyard::castFrom(obj))
+				if(const auto * shipyard = dynamic_cast<const IShipyard *>(obj))
 					shipyards.push_back(shipyard);
 			}
 		}

AI/VCAI/VCAI.cpp

@@ -2134,7 +2134,7 @@ void VCAI::tryRealize(Goals::Trade & g) //trade
 	int accquiredResources = 0;
 	if(const CGObjectInstance * obj = cb->getObj(ObjectInstanceID(g.objid), false))
 	{
-		if(const IMarket * m = IMarket::castFrom(obj, false))
+		if(const auto * m = dynamic_cast<const IMarket*>(obj))
 		{
 			auto freeRes = ah->freeResources(); //trade only resources which are not reserved
 			for(auto it = ResourceSet::nziterator(freeRes); it.valid(); it++)

AI/VCAI/VCAI.h

@@ -371,11 +371,7 @@ public:
 	{
 	}
 
-	virtual ~cannotFulfillGoalException() throw ()
+	const char * what() const noexcept override
-	{
-	};
-
-	const char * what() const throw () override
 	{
 		return msg.c_str();
 	}
@@ -394,11 +390,7 @@ public:
 		msg = goal->name();
 	}
 
-	virtual ~goalFulfilledException() throw ()
+	const char * what() const noexcept override
-	{
-	};
-
-	const char * what() const throw () override
 	{
 		return msg.c_str();
 	}

Global.h

@@ -685,6 +685,23 @@ namespace vstd
 		return a + (b - a) * f;
 	}
 
+	template<typename Floating>
+	bool isAlmostZero(const Floating & value)
+	{
+		constexpr Floating epsilon(0.00001);
+		return std::abs(value) <= epsilon;
+	}
+
+	template<typename Floating1, typename Floating2>
+	bool isAlmostEqual(const Floating1 & left, const Floating2 & right)
+	{
+		using Floating = decltype(left + right);
+		constexpr Floating epsilon(0.00001);
+		const Floating relativeEpsilon = std::max(std::abs(left), std::abs(right)) * epsilon;
+		const Floating value = std::abs(left - right);
+		return value <= relativeEpsilon;
+	}
+
 	///compile-time version of std::abs for ints for int3, in clang++15 std::abs is constexpr
 	static constexpr int abs(int i) {
 		if(i < 0) return -i;

client/NetPacksClient.cpp

@@ -958,7 +958,7 @@ void ApplyClientNetPackVisitor::visitOpenWindow(OpenWindow & pack)
 	case EOpenWindowMode::SHIPYARD_WINDOW:
 		{
 			assert(pack.queryID == QueryID::NONE);
-			const IShipyard *sy = IShipyard::castFrom(cl.getObj(ObjectInstanceID(pack.object)));
+			const auto * sy = dynamic_cast<const IShipyard *>(cl.getObj(ObjectInstanceID(pack.object)));
 			callInterfaceIfPresent(cl, sy->getObject()->getOwner(), &IGameEventsReceiver::showShipyardDialog, sy);
 		}
 		break;
@@ -974,7 +974,7 @@ void ApplyClientNetPackVisitor::visitOpenWindow(OpenWindow & pack)
 	case EOpenWindowMode::UNIVERSITY_WINDOW:
 		{
 			//displays University window (when hero enters University on adventure map)
-			const IMarket *market = IMarket::castFrom(cl.getObj(ObjectInstanceID(pack.object)));
+			const auto * market = dynamic_cast<const IMarket*>(cl.getObj(ObjectInstanceID(pack.object)));
 			const CGHeroInstance *hero = cl.getHero(ObjectInstanceID(pack.visitor));
 			callInterfaceIfPresent(cl, hero->tempOwner, &IGameEventsReceiver::showUniversityWindow, market, hero, pack.queryID);
 		}
@@ -984,7 +984,7 @@ void ApplyClientNetPackVisitor::visitOpenWindow(OpenWindow & pack)
 			//displays Thieves' Guild window (when hero enters Den of Thieves)
 			const CGObjectInstance *obj = cl.getObj(ObjectInstanceID(pack.object));
 			const CGHeroInstance *hero = cl.getHero(ObjectInstanceID(pack.visitor));
-			const IMarket *market = IMarket::castFrom(obj);
+			const auto *market = dynamic_cast<const IMarket*>(obj);
 			callInterfaceIfPresent(cl, cl.getTile(obj->visitablePos())->visitableObjects.back()->tempOwner, &IGameEventsReceiver::showMarketWindow, market, hero, pack.queryID);
 		}
 		break;

client/adventureMap/AdventureMapInterface.cpp

@@ -852,7 +852,7 @@ Rect AdventureMapInterface::terrainAreaPixels() const
 
 const IShipyard * AdventureMapInterface::ourInaccessibleShipyard(const CGObjectInstance *obj) const
 {
-	const IShipyard *ret = IShipyard::castFrom(obj);
+	const auto *ret = dynamic_cast<const IShipyard *>(obj);
 
 	if(!ret ||
 		obj->tempOwner != currentPlayerID ||

client/mapView/MapViewCache.cpp

@@ -141,7 +141,7 @@ void MapViewCache::update(const std::shared_ptr<IMapRendererContext> & context)
 void MapViewCache::render(const std::shared_ptr<IMapRendererContext> & context, Canvas & target, bool fullRedraw)
 {
 	bool mapMoved = (cachedPosition != model->getMapViewCenter());
-	bool lazyUpdate = !mapMoved && !fullRedraw && context->viewTransitionProgress() == 0;
+	bool lazyUpdate = !mapMoved && !fullRedraw && vstd::isAlmostZero(context->viewTransitionProgress());
 
 	Rect dimensions = model->getTilesTotalRect();
 
@@ -184,7 +184,7 @@ void MapViewCache::render(const std::shared_ptr<IMapRendererContext> & context,
 		}
 	}
 
-	if(context->viewTransitionProgress() != 0)
+	if(!vstd::isAlmostZero(context->viewTransitionProgress()))
 		target.drawTransparent(*terrainTransition, Point(0, 0), 1.0 - context->viewTransitionProgress());
 
 	cachedPosition = model->getMapViewCenter();

client/render/ColorFilter.cpp

@@ -41,10 +41,10 @@ bool ColorFilter::operator != (const ColorFilter & other) const
 bool ColorFilter::operator == (const ColorFilter & other) const
 {
 	return
-		r.r == other.r.r && r.g && other.r.g && r.b == other.r.b && r.a == other.r.a &&
+		vstd::isAlmostEqual(r.r, other.r.r) && vstd::isAlmostEqual(r.g, other.r.g) && vstd::isAlmostEqual(r.b, other.r.b) && vstd::isAlmostEqual(r.a, other.r.a) &&
-		g.r == other.g.r && g.g && other.g.g && g.b == other.g.b && g.a == other.g.a &&
+		vstd::isAlmostEqual(g.r, other.g.r) && vstd::isAlmostEqual(g.g, other.g.g) && vstd::isAlmostEqual(g.b, other.g.b) && vstd::isAlmostEqual(g.a, other.g.a) &&
-		b.r == other.b.r && b.g && other.b.g && b.b == other.b.b && b.a == other.b.a &&
+		vstd::isAlmostEqual(b.r, other.b.r) && vstd::isAlmostEqual(b.g, other.b.g) && vstd::isAlmostEqual(b.b, other.b.b) && vstd::isAlmostEqual(b.a, other.b.a) &&
-		a == other.a;
+		vstd::isAlmostEqual(a, other.a);
 }
 
 ColorFilter ColorFilter::genEmptyShifter( )

client/widgets/Slider.cpp

@@ -21,23 +21,24 @@
 
 void CSlider::mouseDragged(const Point & cursorPosition, const Point & lastUpdateDistance)
 {
-	double v = 0;
+	double newPosition = 0;
 	if(getOrientation() == Orientation::HORIZONTAL)
 	{
-		v = cursorPosition.x - pos.x - 24;
+		newPosition = cursorPosition.x - pos.x - 24;
-		v *= positions;
+		newPosition *= positions;
-		v /= (pos.w - 48);
+		newPosition /= (pos.w - 48);
 	}
 	else
 	{
-		v = cursorPosition.y - pos.y - 24;
+		newPosition = cursorPosition.y - pos.y - 24;
-		v *= positions;
+		newPosition *= positions;
-		v /= (pos.h - 48);
+		newPosition /= (pos.h - 48);
 	}
-	v += 0.5;
+
-	if(v!=value)
+	int positionInteger = std::round(newPosition);
+	if(positionInteger != value)
 	{
-		scrollTo(static_cast<int>(v));
+		scrollTo(static_cast<int>(newPosition));
 	}
 }
 

lib/CCreatureHandler.cpp

@@ -845,8 +845,8 @@ void CCreatureHandler::loadUnitAnimInfo(JsonNode & graphics, CLegacyConfigParser
 	missile["attackClimaxFrame"].Float() = parser.readNumber();
 
 	// assume that creature is not a shooter and should not have whole missile field
-	if (missile["frameAngles"].Vector()[0].Float() == 0 &&
+	if (missile["frameAngles"].Vector()[0].Integer() == 0 &&
-	    missile["attackClimaxFrame"].Float() == 0)
+		missile["attackClimaxFrame"].Integer() == 0)
 		graphics.Struct().erase("missile");
 }
 
@@ -988,10 +988,10 @@ void CCreatureHandler::loadStackExperience(CCreature * creature, const JsonNode
 			int lastVal = 0;
 			for (const JsonNode &val : values)
 			{
-				if (val.Float() != lastVal)
+				if (val.Integer() != lastVal)
 				{
 					JsonNode bonusInput = exp["bonus"];
-					bonusInput["val"].Float() = static_cast<int>(val.Float()) - lastVal;
+					bonusInput["val"].Float() = val.Integer() - lastVal;
 
 					auto bonus = JsonUtils::parseBonus (bonusInput);
 					bonus->source = BonusSource::STACK_EXPERIENCE;

lib/gameState/CGameState.cpp

@@ -1385,10 +1385,8 @@ bool CGameState::checkForVictory(const PlayerColor & player, const EventConditio
 			int total = 0; //creature counter
 			for(auto object : map->objects)
 			{
-				const CArmedInstance *ai = nullptr;
+				const auto * ai = dynamic_cast<const CArmedInstance *>(object.get());
-				if(object
+				if(ai && ai->getOwner() == player)
-					&& object->tempOwner == player //object controlled by player
-					&& (ai = dynamic_cast<const CArmedInstance *>(object.get()))) //contains army
 				{
 					for(const auto & elem : ai->Slots()) //iterate through army
 						if(elem.second->getId() == condition.objectType.as<CreatureID>()) //it's searched creature

lib/mapObjects/IMarket.cpp

@@ -154,20 +154,6 @@ std::vector<TradeItemBuy> IMarket::availableItemsIds(EMarketMode mode) const
 	return ret;
 }
 
-const IMarket * IMarket::castFrom(const CGObjectInstance *obj, bool verbose)
-{
-	auto * imarket = dynamic_cast<const IMarket *>(obj);
-	if(verbose && !imarket)
-	{
-		logGlobal->error("Cannot cast to IMarket");
-		if(obj)
-		{
-			logGlobal->error("Object type %s", obj->typeName);
-		}
-	}
-	return imarket;
-}
-
 IMarket::IMarket()
 {
 }

lib/mapObjects/IMarket.h

@@ -29,8 +29,6 @@ public:
 
 	bool getOffer(int id1, int id2, int &val1, int &val2, EMarketMode mode) const; //val1 - how many units of id1 player has to give to receive val2 units
 	std::vector<EMarketMode> availableModes() const;
-
-	static const IMarket *castFrom(const CGObjectInstance *obj, bool verbose = true);
 };
 
 VCMI_LIB_NAMESPACE_END

lib/mapObjects/IObjectInterface.cpp

@@ -156,9 +156,4 @@ void IShipyard::getBoatCost(TResources & cost) const
 	cost[EGameResID::GOLD] = 1000;
 }
 
-const IShipyard * IShipyard::castFrom( const CGObjectInstance *obj )
-{
-	return dynamic_cast<const IShipyard *>(obj);
-}
-
 VCMI_LIB_NAMESPACE_END

lib/mapObjects/IObjectInterface.h

@@ -101,8 +101,6 @@ class DLL_LINKAGE IShipyard : public IBoatGenerator
 {
 public:
 	virtual void getBoatCost(ResourceSet & cost) const;
-
-	static const IShipyard *castFrom(const CGObjectInstance *obj);
 };
 
 VCMI_LIB_NAMESPACE_END

lib/rmg/Functions.h

@@ -28,12 +28,8 @@ public:
 	explicit rmgException(const std::string& _Message) : msg(_Message)
 	{
 	}
-	
+
-	virtual ~rmgException() throw ()
+	const char *what() const noexcept override
-	{
-	};
-	
-	const char *what() const throw () override
 	{
 		return msg.c_str();
 	}

lib/rmg/float3.h

@@ -113,23 +113,6 @@ public:
 		return *this;
 	}
 
-	bool operator==(const float3 & i) const { return (x == i.x) && (y == i.y) && (z == i.z); }
-	bool operator!=(const float3 & i) const { return (x != i.x) || (y != i.y) || (z != i.z); }
-
-	bool operator<(const float3 & 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;
-
-		return x<i.x;
-	}
-
 	std::string toString() const
 	{
 		return	"(" + std::to_string(x) +

server/CGameHandler.cpp

@@ -3528,7 +3528,7 @@ void CGameHandler::objectVisitEnded(const CObjectVisitQuery & query)
 
 bool CGameHandler::buildBoat(ObjectInstanceID objid, PlayerColor playerID)
 {
-	const IShipyard *obj = IShipyard::castFrom(getObj(objid));
+	const auto *obj = dynamic_cast<const IShipyard *>(getObj(objid));
 
 	if (obj->shipyardStatus() != IBoatGenerator::GOOD)
 	{

server/NetPacksServer.cpp

@@ -170,7 +170,7 @@ void ApplyGhNetPackVisitor::visitTradeOnMarketplace(TradeOnMarketplace & pack)
 {
 	const CGObjectInstance * object = gh.getObj(pack.marketId);
 	const CGHeroInstance * hero = gh.getHero(pack.heroId);
-	const IMarket * market = IMarket::castFrom(object);
+	const auto * market = dynamic_cast<const IMarket*>(object);
 
 	gh.throwIfWrongPlayer(&pack);