From ac6b477aa2d0d41e1e032c7231e51c71215e9cc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Kalinowski?= Date: Wed, 20 Mar 2019 20:58:15 +0100 Subject: [PATCH] Simplify statements -Simplify return statements across the code --- AI/VCAI/AIUtility.cpp | 13 ++------- AI/VCAI/BuildingManager.cpp | 5 +--- AI/VCAI/VCAI.cpp | 5 +--- lib/CStack.cpp | 9 +----- lib/battle/BattleInfo.cpp | 5 +--- lib/battle/CBattleInfoCallback.cpp | 14 ++++----- lib/battle/CBattleInfoEssentials.cpp | 5 +--- lib/battle/CUnitState.cpp | 5 +--- lib/filesystem/CCompressedStream.cpp | 2 +- lib/mapObjects/CGTownInstance.cpp | 4 +-- lib/mapObjects/CommonConstructors.cpp | 4 +-- lib/mapObjects/MiscObjects.cpp | 8 ++--- lib/mapObjects/ObjectTemplate.cpp | 20 ++++--------- lib/mapping/CDrawRoadsOperation.cpp | 11 +------ lib/rmg/CMapGenerator.cpp | 5 +--- lib/spells/CSpellHandler.cpp | 5 +--- lib/spells/TargetCondition.cpp | 42 ++++++++------------------- lib/spells/effects/Catapult.cpp | 7 ++--- lib/spells/effects/Dispel.cpp | 4 +-- lib/spells/effects/Sacrifice.cpp | 7 ++--- lib/spells/effects/UnitEffect.cpp | 13 ++------- 21 files changed, 45 insertions(+), 148 deletions(-) diff --git a/AI/VCAI/AIUtility.cpp b/AI/VCAI/AIUtility.cpp index 932b93000..b94531faa 100644 --- a/AI/VCAI/AIUtility.cpp +++ b/AI/VCAI/AIUtility.cpp @@ -205,14 +205,7 @@ bool isSafeToVisit(HeroPtr h, uint64_t dangerStrength) if(dangerStrength) { - if(heroStrength / SAFE_ATTACK_CONSTANT > dangerStrength) - { - return true; - } - else - { - return false; - } + return heroStrength / SAFE_ATTACK_CONSTANT > dangerStrength; } return true; //there's no danger @@ -314,8 +307,6 @@ bool compareArtifacts(const CArtifactInstance * a1, const CArtifactInstance * a2 if(art1->price == art2->price) return art1->valOfBonuses(Bonus::PRIMARY_SKILL) > art2->valOfBonuses(Bonus::PRIMARY_SKILL); - else if(art1->price > art2->price) - return true; else - return false; + return art1->price > art2->price; } diff --git a/AI/VCAI/BuildingManager.cpp b/AI/VCAI/BuildingManager.cpp index 4e28be6bd..698b33f97 100644 --- a/AI/VCAI/BuildingManager.cpp +++ b/AI/VCAI/BuildingManager.cpp @@ -225,10 +225,7 @@ bool BuildingManager::getBuildingOptions(const CGTownInstance * t) if (buildingInfo.first > 43) extraBuildings.push_back(buildingInfo.first); } - if (tryBuildAnyStructure(t, extraBuildings)) - return true; - - return false; + return tryBuildAnyStructure(t, extraBuildings); } BuildingID BuildingManager::getMaxPossibleGoldBuilding(const CGTownInstance * t) diff --git a/AI/VCAI/VCAI.cpp b/AI/VCAI/VCAI.cpp index 3a383212a..989189050 100644 --- a/AI/VCAI/VCAI.cpp +++ b/AI/VCAI/VCAI.cpp @@ -1282,10 +1282,7 @@ bool VCAI::isGoodForVisit(const CGObjectInstance * obj, HeroPtr h, const AIPath const CGObjectInstance * topObj = cb->getVisitableObjs(obj->visitablePos()).back(); //it may be hero visiting this obj //we don't try visiting object on which allied or owned hero stands // -> it will just trigger exchange windows and AI will be confused that obj behind doesn't get visited - if (topObj->ID == Obj::HERO && cb->getPlayerRelations(h->tempOwner, topObj->tempOwner) != PlayerRelations::ENEMIES) - return false; - else - return true; //all of the following is met + return !(topObj->ID == Obj::HERO && cb->getPlayerRelations(h->tempOwner, topObj->tempOwner) != PlayerRelations::ENEMIES); //all of the following is met } bool VCAI::isTileNotReserved(const CGHeroInstance * h, int3 t) const diff --git a/lib/CStack.cpp b/lib/CStack.cpp index fdbb1a204..8db9e2fcf 100644 --- a/lib/CStack.cpp +++ b/lib/CStack.cpp @@ -308,14 +308,7 @@ bool CStack::unitHasAmmoCart(const battle::Unit * unit) const { if(battle->battleMatchOwner(st, unit, true) && st->getCreature()->idNumber == CreatureID::AMMO_CART) { - if(st->alive()) - { - return true; - } - else - { - return false; - } + return st->alive(); } } //ammo cart works during creature bank battle while not on battlefield diff --git a/lib/battle/BattleInfo.cpp b/lib/battle/BattleInfo.cpp index 28aac3939..c2e6a5cce 100644 --- a/lib/battle/BattleInfo.cpp +++ b/lib/battle/BattleInfo.cpp @@ -876,10 +876,7 @@ void BattleInfo::setUnitState(uint32_t id, const JsonNode & data, int64_t health auto selector = [](const Bonus * b) { //Special case: DISRUPTING_RAY is absolutely permanent - if(b->source == Bonus::SPELL_EFFECT) - return b->sid != SpellID::DISRUPTING_RAY; - else - return false; + return b->source == Bonus::SPELL_EFFECT && b->sid != SpellID::DISRUPTING_RAY; }; changedStack->removeBonusesRecursive(selector); } diff --git a/lib/battle/CBattleInfoCallback.cpp b/lib/battle/CBattleInfoCallback.cpp index 9968460b7..f5018d599 100644 --- a/lib/battle/CBattleInfoCallback.cpp +++ b/lib/battle/CBattleInfoCallback.cpp @@ -577,11 +577,8 @@ bool CBattleInfoCallback::battleCanAttack(const CStack * stack, const CStack * t auto &id = stack->getCreature()->idNumber; if (id == CreatureID::FIRST_AID_TENT || id == CreatureID::CATAPULT) return false; - - if (!target->alive()) - return false; - - return true; + + return target->alive(); } bool CBattleInfoCallback::battleCanShoot(const battle::Unit * attacker, BattleHex dest) const @@ -610,12 +607,11 @@ bool CBattleInfoCallback::battleCanShoot(const battle::Unit * attacker, BattleHe if(attacker->creatureIndex() == CreatureID::CATAPULT && defender) //catapult cannot attack creatures return false; - if(attacker->canShoot() + return attacker->canShoot() && battleMatchOwner(attacker, defender) && defender->alive() - && (!battleIsUnitBlocked(attacker) || attacker->hasBonusOfType(Bonus::FREE_SHOOTING))) - return true; - return false; + && (!battleIsUnitBlocked(attacker) + || attacker->hasBonusOfType(Bonus::FREE_SHOOTING)); } TDmgRange CBattleInfoCallback::calculateDmgRange(const BattleAttackInfo & info) const diff --git a/lib/battle/CBattleInfoEssentials.cpp b/lib/battle/CBattleInfoEssentials.cpp index a24c6bb5b..ba8a9202c 100644 --- a/lib/battle/CBattleInfoEssentials.cpp +++ b/lib/battle/CBattleInfoEssentials.cpp @@ -422,8 +422,5 @@ bool CBattleInfoEssentials::battleMatchOwner(const PlayerColor & attacker, const PlayerColor initialOwner = getBattle()->getSidePlayer(defender->unitSide()); - if(boost::logic::indeterminate(positivness)) - return true; - else - return (attacker == initialOwner) == (bool)positivness; + return boost::logic::indeterminate(positivness) || (attacker == initialOwner) == (bool)positivness; } diff --git a/lib/battle/CUnitState.cpp b/lib/battle/CUnitState.cpp index 544f5c132..2d2ecbe3e 100644 --- a/lib/battle/CUnitState.cpp +++ b/lib/battle/CUnitState.cpp @@ -721,10 +721,7 @@ bool CUnitState::canMove(int turn) const bool CUnitState::defended(int turn) const { - if(!turn) - return defending; - else - return false; + return !turn && defending; } bool CUnitState::moved(int turn) const diff --git a/lib/filesystem/CCompressedStream.cpp b/lib/filesystem/CCompressedStream.cpp index e44d083d4..b5a6515e4 100644 --- a/lib/filesystem/CCompressedStream.cpp +++ b/lib/filesystem/CCompressedStream.cpp @@ -165,7 +165,7 @@ si64 CCompressedStream::readMore(ui8 *data, si64 size) throw std::runtime_error(std::string("Decompression error: ") + inflateState->msg); } } - while (endLoop == false && inflateState->avail_out != 0 ); + while (!endLoop && inflateState->avail_out != 0 ); decompressed = inflateState->total_out - decompressed; diff --git a/lib/mapObjects/CGTownInstance.cpp b/lib/mapObjects/CGTownInstance.cpp index 79d1e7730..8a4eb8045 100644 --- a/lib/mapObjects/CGTownInstance.cpp +++ b/lib/mapObjects/CGTownInstance.cpp @@ -859,9 +859,7 @@ bool CGTownInstance::passableFor(PlayerColor color) const if ( tempOwner == PlayerColor::NEUTRAL )//neutral guarded - no one can visit return false; - if (cb->getPlayerRelations(tempOwner, color) != PlayerRelations::ENEMIES) - return true; - return false; + return cb->getPlayerRelations(tempOwner, color) != PlayerRelations::ENEMIES; } void CGTownInstance::getOutOffsets( std::vector &offsets ) const diff --git a/lib/mapObjects/CommonConstructors.cpp b/lib/mapObjects/CommonConstructors.cpp index 6f61bd524..381fc28e7 100644 --- a/lib/mapObjects/CommonConstructors.cpp +++ b/lib/mapObjects/CommonConstructors.cpp @@ -65,9 +65,7 @@ bool CTownInstanceConstructor::objectFilter(const CGObjectInstance * object, con return town->hasBuilt(id); }; - if (filters.count(templ.stringID)) - return filters.at(templ.stringID).test(buildTest); - return false; + return filters.count(templ.stringID) != 0 && filters.at(templ.stringID).test(buildTest); } CGObjectInstance * CTownInstanceConstructor::create(const ObjectTemplate & tmpl) const diff --git a/lib/mapObjects/MiscObjects.cpp b/lib/mapObjects/MiscObjects.cpp index 667a14b59..7a93a4c8b 100644 --- a/lib/mapObjects/MiscObjects.cpp +++ b/lib/mapObjects/MiscObjects.cpp @@ -1279,12 +1279,8 @@ void CGWhirlpool::teleportDialogAnswered(const CGHeroInstance *hero, ui32 answer bool CGWhirlpool::isProtected(const CGHeroInstance * h) { - if(h->hasBonusOfType(Bonus::WHIRLPOOL_PROTECTION) || - (h->stacksCount() == 1 && h->Slots().begin()->second->count == 1)) //we can't remove last unit - { - return true; - } - return false; + return h->hasBonusOfType(Bonus::WHIRLPOOL_PROTECTION) + || (h->stacksCount() == 1 && h->Slots().begin()->second->count == 1); } void CGArtifact::initObj(CRandomGenerator & rand) diff --git a/lib/mapObjects/ObjectTemplate.cpp b/lib/mapObjects/ObjectTemplate.cpp index 9f35bf5c2..5cc2ccf98 100644 --- a/lib/mapObjects/ObjectTemplate.cpp +++ b/lib/mapObjects/ObjectTemplate.cpp @@ -44,9 +44,7 @@ static bool isOnVisitableFromTopList(int identifier, int type) Obj::QUEST_GUARD, Obj::CORPSE }; - if (vstd::find_pos(visitableFromTop, identifier) != -1) - return true; - return false; + return vstd::find_pos(visitableFromTop, identifier) != -1; } ObjectTemplate::ObjectTemplate(): @@ -430,30 +428,22 @@ bool ObjectTemplate::isWithin(si32 X, si32 Y) const { if (X < 0 || Y < 0) return false; - if (X >= getWidth() || Y >= getHeight()) - return false; - return true; + return !(X >= getWidth() || Y >= getHeight()); } bool ObjectTemplate::isVisitableAt(si32 X, si32 Y) const { - if (isWithin(X, Y)) - return usedTiles[Y][X] & VISITABLE; - return false; + return isWithin(X, Y) && usedTiles[Y][X] & VISITABLE; } bool ObjectTemplate::isVisibleAt(si32 X, si32 Y) const { - if (isWithin(X, Y)) - return usedTiles[Y][X] & VISIBLE; - return false; + return isWithin(X, Y) && usedTiles[Y][X] & VISIBLE; } bool ObjectTemplate::isBlockedAt(si32 X, si32 Y) const { - if (isWithin(X, Y)) - return usedTiles[Y][X] & BLOCKED; - return false; + return isWithin(X, Y) && usedTiles[Y][X] & BLOCKED; } std::set ObjectTemplate::getBlockedOffsets() const diff --git a/lib/mapping/CDrawRoadsOperation.cpp b/lib/mapping/CDrawRoadsOperation.cpp index 5e8ffb21b..b39f8418c 100644 --- a/lib/mapping/CDrawRoadsOperation.cpp +++ b/lib/mapping/CDrawRoadsOperation.cpp @@ -309,16 +309,7 @@ CDrawRoadsOperation::ValidationResult CDrawRoadsOperation::validateTile(const Ro int3 currentPos(cx, cy, pos.z); - bool hasSomething; - - if(!map->isInTheMap(currentPos)) - { - hasSomething = true; //road/river can go out of map - } - else - { - hasSomething = tileHasSomething(currentPos); - } + bool hasSomething = !map->isInTheMap(currentPos) || tileHasSomething(currentPos); if(ruleIsSomething(flipped.data[i])) { diff --git a/lib/rmg/CMapGenerator.cpp b/lib/rmg/CMapGenerator.cpp index c8f07b11b..e14d76ea1 100644 --- a/lib/rmg/CMapGenerator.cpp +++ b/lib/rmg/CMapGenerator.cpp @@ -617,10 +617,7 @@ void CMapGenerator::createConnections2() boost::set_intersection(tilesA, tilesB, std::back_inserter(commonTiles), [](const int3 &lhs, const int3 &rhs) -> bool { //ignore z coordinate - if (lhs.x < rhs.x) - return true; - else - return lhs.y < rhs.y; + return lhs.x < rhs.x || lhs.y < rhs.y; }); vstd::erase_if(commonTiles, [](const int3 &tile) -> bool diff --git a/lib/spells/CSpellHandler.cpp b/lib/spells/CSpellHandler.cpp index 44c5b079c..4f34564fb 100644 --- a/lib/spells/CSpellHandler.cpp +++ b/lib/spells/CSpellHandler.cpp @@ -594,10 +594,7 @@ CSpell::TargetInfo::TargetInfo(const CSpell * spell, const int level, spells::Mo bool DLL_LINKAGE isInScreenRange(const int3 & center, const int3 & pos) { int3 diff = pos - center; - if(diff.x >= -9 && diff.x <= 9 && diff.y >= -8 && diff.y <= 8) - return true; - else - return false; + return diff.x >= -9 && diff.x <= 9 && diff.y >= -8 && diff.y <= 8; } ///CSpellHandler diff --git a/lib/spells/TargetCondition.cpp b/lib/spells/TargetCondition.cpp index c0f521982..61bf0b43c 100644 --- a/lib/spells/TargetCondition.cpp +++ b/lib/spells/TargetCondition.cpp @@ -57,11 +57,7 @@ public: bool isReceptive(const Mechanics * m, const battle::Unit * target) const override { bool result = check(m, target); - - if(inverted) - return !result; - else - return result; + return inverted != result; } protected: @@ -120,10 +116,10 @@ protected: cachingStr << "type_" << Bonus::LEVEL_SPELL_IMMUNITY << "addInfo_1"; TBonusListPtr levelImmunities = target->getBonuses(Selector::type(Bonus::LEVEL_SPELL_IMMUNITY).And(Selector::info(1)), cachingStr.str()); - - if(levelImmunities->size() > 0 && levelImmunities->totalValue() >= m->getSpellLevel() && m->getSpellLevel() > 0) - return false; - return true; + + return !(levelImmunities->size() > 0 + && levelImmunities->totalValue() >= m->getSpellLevel() + && m->getSpellLevel() > 0); } }; @@ -141,9 +137,7 @@ protected: { std::stringstream cachingStr; cachingStr << "type_" << Bonus::SPELL_IMMUNITY << "subtype_" << m->getSpellIndex() << "addInfo_1"; - if(target->hasBonus(Selector::typeSubtypeInfo(Bonus::SPELL_IMMUNITY, m->getSpellIndex(), 1), cachingStr.str())) - return false; - return true; + return !target->hasBonus(Selector::typeSubtypeInfo(Bonus::SPELL_IMMUNITY, m->getSpellIndex(), 1), cachingStr.str()); } }; @@ -197,10 +191,9 @@ protected: bool check(const Mechanics * m, const battle::Unit * target) const override { TBonusListPtr levelImmunities = target->getBonuses(Selector::type(Bonus::LEVEL_SPELL_IMMUNITY)); - - if(levelImmunities->size() > 0 && levelImmunities->totalValue() >= m->getSpellLevel() && m->getSpellLevel() > 0) - return false; - return true; + return !(levelImmunities->size() > 0 && + levelImmunities->totalValue() >= m->getSpellLevel() && + m->getSpellLevel() > 0); } }; @@ -236,10 +229,7 @@ protected: int64_t subjectHealth = target->getAvailableHealth(); //apply 'damage' bonus for hypnotize, including hero specialty auto maxHealth = m->applySpellBonus(m->getEffectValue(), target); - if(subjectHealth > maxHealth) - return false; - - return true; + return subjectHealth <= maxHealth; } }; @@ -426,11 +416,7 @@ bool TargetCondition::isReceptive(const Mechanics * m, const battle::Unit * targ if(item->isReceptive(m, target)) return true; } - - if(!check(normal, m, target)) - return false; - - return true; + return check(normal, m, target); } void TargetCondition::serializeJson(JsonSerializeFormat & handler, const ItemFactory * itemFactory) @@ -488,11 +474,7 @@ bool TargetCondition::check(const ItemVector & condition, const Mechanics * m, c nonExclusiveExits = true; } } - - if(nonExclusiveExits) - return nonExclusiveCheck; - else - return true; + return !nonExclusiveExits || nonExclusiveCheck; } void TargetCondition::loadConditions(const JsonNode & source, bool exclusive, bool inverted, const ItemFactory * itemFactory) diff --git a/lib/spells/effects/Catapult.cpp b/lib/spells/effects/Catapult.cpp index 5db9efe3d..f94a32b97 100644 --- a/lib/spells/effects/Catapult.cpp +++ b/lib/spells/effects/Catapult.cpp @@ -59,11 +59,8 @@ bool Catapult::applicable(Problem & problem, const Mechanics * m) const } const auto attackableBattleHexes = m->cb->getAttackableBattleHexes(); - - if(attackableBattleHexes.empty()) - return m->adaptProblem(ESpellCastProblem::NO_APPROPRIATE_TARGET, problem); - - return true; + + return !attackableBattleHexes.empty() || m->adaptProblem(ESpellCastProblem::NO_APPROPRIATE_TARGET, problem); } void Catapult::apply(BattleStateProxy * battleState, RNG & rng, const Mechanics * m, const EffectTarget & /* eTarget */) const diff --git a/lib/spells/effects/Dispel.cpp b/lib/spells/effects/Dispel.cpp index 820781b2f..576bd25e2 100644 --- a/lib/spells/effects/Dispel.cpp +++ b/lib/spells/effects/Dispel.cpp @@ -102,9 +102,7 @@ bool Dispel::mainSelector(const Bonus * bonus) if(sourceSpell->id == SpellID::CLONE) return false; //stack may have inherited effects - if(sourceSpell->isAdventureSpell()) - return false; - return true; + return !sourceSpell->isAdventureSpell(); } //not spell effect return false; diff --git a/lib/spells/effects/Sacrifice.cpp b/lib/spells/effects/Sacrifice.cpp index 4a105b3ac..1e8347952 100644 --- a/lib/spells/effects/Sacrifice.cpp +++ b/lib/spells/effects/Sacrifice.cpp @@ -110,11 +110,8 @@ bool Sacrifice::applicable(Problem & problem, const Mechanics * m, const EffectT auto victim = target.at(1).unitValue; if(!victim) return false; - - if(!victim->alive() || !getStackFilter(m, false, victim) || !isReceptive(m, victim)) - return false; - - return true; + + return !(!victim->alive() || !getStackFilter(m, false, victim) || !isReceptive(m, victim)); } return true; diff --git a/lib/spells/effects/UnitEffect.cpp b/lib/spells/effects/UnitEffect.cpp index ec6a1d39e..83daf36c5 100644 --- a/lib/spells/effects/UnitEffect.cpp +++ b/lib/spells/effects/UnitEffect.cpp @@ -93,13 +93,7 @@ EffectTarget UnitEffect::filterTarget(const Mechanics * m, const EffectTarget & EffectTarget res; vstd::copy_if(target, std::back_inserter(res), [this, m](const Destination & d) { - if(!d.unitValue) - return false; - if(!isValidTarget(m, d.unitValue)) - return false; - if(!isReceptive(m, d.unitValue)) - return false; - return true; + return d.unitValue && isValidTarget(m, d.unitValue) && isReceptive(m, d.unitValue); }); return res; } @@ -271,10 +265,7 @@ bool UnitEffect::isReceptive(const Mechanics * m, const battle::Unit * unit) con //SPELL_IMMUNITY absolute case std::stringstream cachingStr; cachingStr << "type_" << Bonus::SPELL_IMMUNITY << "subtype_" << m->getSpellIndex() << "addInfo_1"; - if(unit->hasBonus(Selector::typeSubtypeInfo(Bonus::SPELL_IMMUNITY, m->getSpellIndex(), 1), cachingStr.str())) - return false; - - return true; + return !unit->hasBonus(Selector::typeSubtypeInfo(Bonus::SPELL_IMMUNITY, m->getSpellIndex(), 1), cachingStr.str()); } else {