From 473fb7442be03c57926eb3046a9d0644e990fde3 Mon Sep 17 00:00:00 2001 From: Jarod42 Date: Thu, 2 May 2024 12:42:13 +0200 Subject: [PATCH 1/4] Clean up: - use `std::optional` instead of output parameter - Add projection to `ranges::sort` --- src/ai/ai_building.cpp | 279 ++++++++++++++++++++++------------------- src/ai/ai_force.cpp | 165 +++++++++--------------- src/ai/ai_local.h | 4 +- src/ai/ai_resource.cpp | 9 +- src/include/util.h | 8 +- 5 files changed, 224 insertions(+), 241 deletions(-) diff --git a/src/ai/ai_building.cpp b/src/ai/ai_building.cpp index 6c28ff7c14..cb79ab99ee 100644 --- a/src/ai/ai_building.cpp +++ b/src/ai/ai_building.cpp @@ -143,18 +143,74 @@ static bool AiCheckSurrounding(const CUnit &worker, const CUnitType &type, const class BuildingPlaceFinder { public: - BuildingPlaceFinder(const CUnit &worker, const CUnitType &type, bool checkSurround, Vec2i *resultPos) : + friend TerrainTraversal; + /** + ** Find free building place. (flood fill version) + ** + ** @param worker Worker to build building. + ** @param type Type of building. + ** @param startPos Original position to try building + ** @param checkSurround Check if the perimeter of the building is free + ** + ** @return place found, std::nullopt if no found. + */ + static std::optional + find(const CUnit &worker, const CUnitType &type, const Vec2i &startPos, bool checkSurround) + { + TerrainTraversal terrainTraversal; + + terrainTraversal.SetSize(Map.Info.MapWidth, Map.Info.MapHeight); + terrainTraversal.Init(); + + Assert(Map.Info.IsPointOnMap(startPos)); + terrainTraversal.PushPos(startPos); + + BuildingPlaceFinder buildingPlaceFinder(worker, type, checkSurround); + + return terrainTraversal.Run(buildingPlaceFinder) + ? std::make_optional(buildingPlaceFinder.resultPos) + : std::nullopt; + } + + /** + ** Find free building place. (flood fill version) + ** + ** @param worker Worker to build building. + ** @param type Type of building. + ** @param startUnit Original position to try building + ** @param checkSurround Check if the perimeter of the building is free + ** + ** @return place found, std::nullopt if no found. + */ + static std::optional find(const CUnit &worker, + const CUnitType &type, + const CUnit &startUnit, + bool checkSurround) + { + TerrainTraversal terrainTraversal; + + terrainTraversal.SetSize(Map.Info.MapWidth, Map.Info.MapHeight); + terrainTraversal.Init(); + terrainTraversal.PushUnitPosAndNeighboor(startUnit); + + BuildingPlaceFinder buildingPlaceFinder(worker, type, checkSurround); + + return terrainTraversal.Run(buildingPlaceFinder) + ? std::make_optional(buildingPlaceFinder.resultPos) + : std::nullopt; + } + + +private: + BuildingPlaceFinder(const CUnit &worker, const CUnitType &type, bool checkSurround) : worker(worker), type(type), movemask( worker.Type->MovementMask & ~((type.BoolFlag[SHOREBUILDING_INDEX].value ? (MapFieldCoastAllowed | MapFieldLandUnit | MapFieldAirUnit | MapFieldSeaUnit) : (MapFieldLandUnit | MapFieldAirUnit | MapFieldSeaUnit)))), - checkSurround(checkSurround), - resultPos(resultPos) + checkSurround(checkSurround) { - resultPos->x = -1; - resultPos->y = -1; } VisitResult Visit(TerrainTraversal &terrainTraversal, const Vec2i &pos, const Vec2i &from); private: @@ -162,7 +218,7 @@ class BuildingPlaceFinder const CUnitType &type; unsigned int movemask; bool checkSurround; - Vec2i *resultPos; + Vec2i resultPos{-1, -1}; }; VisitResult BuildingPlaceFinder::Visit(TerrainTraversal &terrainTraversal, const Vec2i &pos, const Vec2i &from) @@ -176,10 +232,10 @@ VisitResult BuildingPlaceFinder::Visit(TerrainTraversal &terrainTraversal, const && !AiEnemyUnitsInDistance(*worker.Player, nullptr, pos, 8)) { bool backupok; if (AiCheckSurrounding(worker, type, pos, backupok) && checkSurround) { - *resultPos = pos; + resultPos = pos; return VisitResult::Finished; - } else if (backupok && resultPos->x == -1) { - *resultPos = pos; + } else if (backupok && resultPos.x == -1) { + resultPos = pos; } } if (CanMoveToMask(pos, movemask) @@ -190,41 +246,53 @@ VisitResult BuildingPlaceFinder::Visit(TerrainTraversal &terrainTraversal, const } } -/** -** Find free building place. (flood fill version) -** -** @param worker Worker to build building. -** @param type Type of building. -** @param startPos Original position to try building -** @param checkSurround Check if the perimeter of the building is free -** @param resultPos OUT: Pointer for position returned. -** -** @return True if place found, false if no found. -*/ -static bool AiFindBuildingPlace2(const CUnit &worker, const CUnitType &type, const Vec2i &startPos, const CUnit *startUnit, bool checkSurround, Vec2i *resultPos) +class HallPlaceFinder { - TerrainTraversal terrainTraversal; +public: + friend TerrainTraversal; + + /** + ** Find building place for hall. (flood fill version) + ** + ** The best place: + ** 1) near to resource. + ** !2) near to wood. + ** !3) near to worker and must be reachable. + ** 4) no enemy near it. + ** 5) no hall already near + ** !6) enough gold in mine + ** + ** @param worker Worker to build building. + ** @param type Type of building. + ** @param startPos Start search position (if == -1 then unit X pos used). + ** @param resource resource to be near. + ** + ** @return place found, std::nullopt if not found. + ** + ** @todo FIXME: This is slow really slow, using + ** two flood fills, is not a perfect solution. + */ + static std::optional + find(const CUnit &worker, const CUnitType &type, const Vec2i &startPos, int resource) + { + TerrainTraversal terrainTraversal; - terrainTraversal.SetSize(Map.Info.MapWidth, Map.Info.MapHeight); - terrainTraversal.Init(); + terrainTraversal.SetSize(Map.Info.MapWidth, Map.Info.MapHeight); + terrainTraversal.Init(); - if (startUnit != nullptr) { - terrainTraversal.PushUnitPosAndNeighboor(*startUnit); - } else { Assert(Map.Info.IsPointOnMap(startPos)); terrainTraversal.PushPos(startPos); - } - BuildingPlaceFinder buildingPlaceFinder(worker, type, checkSurround, resultPos); + HallPlaceFinder hallPlaceFinder(worker, type, resource); - terrainTraversal.Run(buildingPlaceFinder); - return Map.Info.IsPointOnMap(*resultPos); -} + if (terrainTraversal.Run(hallPlaceFinder)) { + return hallPlaceFinder.resultPos; + } + return BuildingPlaceFinder::find(worker, type, startPos, true); + } -class HallPlaceFinder -{ -public: - HallPlaceFinder(const CUnit &worker, const CUnitType &type, int resource, Vec2i *resultPos) : +private: + HallPlaceFinder(const CUnit &worker, const CUnitType &type, int resource) : worker(worker), type(type), movemask( @@ -232,8 +300,7 @@ class HallPlaceFinder & ~((type.BoolFlag[SHOREBUILDING_INDEX].value ? (MapFieldCoastAllowed | MapFieldLandUnit | MapFieldAirUnit | MapFieldSeaUnit) : (MapFieldLandUnit | MapFieldAirUnit | MapFieldSeaUnit)))), - resource(resource), - resultPos(resultPos) + resource(resource) {} VisitResult Visit(TerrainTraversal &terrainTraversal, const Vec2i &pos, const Vec2i &from); private: @@ -243,7 +310,7 @@ class HallPlaceFinder const CUnitType &type; const unsigned int movemask; const int resource; - Vec2i *resultPos; + Vec2i resultPos{-1, -1}; }; bool HallPlaceFinder::IsAUsableMine(const CUnit &mine) const @@ -288,7 +355,8 @@ VisitResult HallPlaceFinder::Visit(TerrainTraversal &terrainTraversal, const Vec #endif CUnit *mine = ResourceOnMap(pos, resource); if (mine && IsAUsableMine(*mine)) { - if (AiFindBuildingPlace2(worker, type, pos, mine, true, resultPos)) { + if (auto place = BuildingPlaceFinder::find(worker, type, *mine, true)) { + resultPos = *place; return VisitResult::Finished; } } @@ -299,63 +367,49 @@ VisitResult HallPlaceFinder::Visit(TerrainTraversal &terrainTraversal, const Vec } } -/** -** Find building place for hall. (flood fill version) -** -** The best place: -** 1) near to resource. -** !2) near to wood. -** !3) near to worker and must be reachable. -** 4) no enemy near it. -** 5) no hall already near -** !6) enough gold in mine -** -** @param worker Worker to build building. -** @param type Type of building. -** @param startPos Start search position (if == -1 then unit X pos used). -** @param resource resource to be near. -** @param resultPos OUT: Pointer for position returned. -** -** @return True if place found, false if not found. -** -** @todo FIXME: This is slow really slow, using -** two flood fills, is not a perfect solution. -*/ -static bool AiFindHallPlace(const CUnit &worker, - const CUnitType &type, - const Vec2i &startPos, - int resource, - Vec2i *resultPos) +class LumberMillPlaceFinder { - TerrainTraversal terrainTraversal; +public: + friend TerrainTraversal; + /** + ** Find free building place for lumber mill. (flood fill version) + ** + ** @param worker Worker to build building. + ** @param type Type of building. + ** @param resource resource terrain to be near. + ** @param startPos Start search X position (if == -1 then unit X pos used). + ** + ** @return place found, std::nullopt if not found. + ** + ** @todo FIXME: This is slow really slow, using two flood fills, is not a perfect solution. + */ + static std::optional + find(const CUnit &worker, const CUnitType &type, const Vec2i &startPos, int resource) + { + TerrainTraversal terrainTraversal; - terrainTraversal.SetSize(Map.Info.MapWidth, Map.Info.MapHeight); - terrainTraversal.Init(); + terrainTraversal.SetSize(Map.Info.MapWidth, Map.Info.MapHeight); + terrainTraversal.Init(); - Assert(Map.Info.IsPointOnMap(startPos)); - terrainTraversal.PushPos(startPos); + Assert(Map.Info.IsPointOnMap(startPos)); + terrainTraversal.PushPos(startPos); - HallPlaceFinder hallPlaceFinder(worker, type, resource, resultPos); + LumberMillPlaceFinder lumberMillPlaceFinder(worker, type, resource); - if (terrainTraversal.Run(hallPlaceFinder)) { - return true; + return terrainTraversal.Run(lumberMillPlaceFinder) + ? std::make_optional(lumberMillPlaceFinder.resultPos) + : std::nullopt; } - return AiFindBuildingPlace2(worker, type, startPos, nullptr, true, resultPos); -} -class LumberMillPlaceFinder -{ -public: +private: LumberMillPlaceFinder(const CUnit &worker, const CUnitType &type, - int resource, - Vec2i *resultPos) : + int resource) : worker(worker), type(type), movemask(worker.Type->MovementMask & ~(MapFieldLandUnit | MapFieldAirUnit | MapFieldSeaUnit)), - resource(resource), - resultPos(resultPos) + resource(resource) {} VisitResult Visit(TerrainTraversal &terrainTraversal, const Vec2i &pos, const Vec2i &from); @@ -364,7 +418,7 @@ class LumberMillPlaceFinder const CUnitType &type; unsigned int movemask; int resource; - Vec2i *resultPos; + Vec2i resultPos{-1, -1}; }; VisitResult LumberMillPlaceFinder::Visit(TerrainTraversal &terrainTraversal, const Vec2i &pos, const Vec2i &from) @@ -375,7 +429,8 @@ VisitResult LumberMillPlaceFinder::Visit(TerrainTraversal &terrainTraversal, con } #endif if (Map.Field(pos)->IsTerrainResourceOnMap(resource)) { - if (AiFindBuildingPlace2(worker, type, from, nullptr, true, resultPos)) { + if (auto place = BuildingPlaceFinder::find(worker, type, from, true)) { + resultPos = *place; return VisitResult::Finished; } } @@ -386,42 +441,11 @@ VisitResult LumberMillPlaceFinder::Visit(TerrainTraversal &terrainTraversal, con } } -/** -** Find free building place for lumber mill. (flood fill version) -** -** @param worker Worker to build building. -** @param type Type of building. -** @param resource resource terrain to be near. -** @param startPos Start search X position (if == -1 then unit X pos used). -** @param resultPos OUT: Pointer for position returned. -** -** @return True if place found, false if not found. -** -** @todo FIXME: This is slow really slow, using two flood fills, is not a perfect solution. -*/ -static bool AiFindLumberMillPlace(const CUnit &worker, const CUnitType &type, const Vec2i &startPos, int resource, Vec2i *resultPos) -{ - TerrainTraversal terrainTraversal; - - terrainTraversal.SetSize(Map.Info.MapWidth, Map.Info.MapHeight); - terrainTraversal.Init(); - - Assert(Map.Info.IsPointOnMap(startPos)); - terrainTraversal.PushPos(startPos); - - LumberMillPlaceFinder lumberMillPlaceFinder(worker, type, resource, resultPos); - - return terrainTraversal.Run(lumberMillPlaceFinder); -} - -static bool AiFindMiningPlace(const CUnit &worker, - const CUnitType &type, - const Vec2i &startPos, - int resource, - Vec2i *resultPos) +static std::optional +AiFindMiningPlace(const CUnit &worker, const CUnitType &type, const Vec2i &startPos, int resource) { // look near (mine = ResourceOnMap(pos, resource, false) ? - return AiFindBuildingPlace2(worker, type, startPos, nullptr, false, resultPos); + return BuildingPlaceFinder::find(worker, type, startPos, false); } /** @@ -430,14 +454,13 @@ static bool AiFindMiningPlace(const CUnit &worker, ** @param worker Worker to build building. ** @param type Type of building. ** @param nearPos Start search near nearPos position (or worker->X if nearPos is invalid). -** @param resultPos Pointer for position returned. ** -** @return True if place found, false if no found. +** @return place found, std::nullopt if no found. ** ** @todo Better and faster way to find building place of oil ** platforms Special routines for special buildings. */ -bool AiFindBuildingPlace(const CUnit &worker, const CUnitType &type, const Vec2i &nearPos, Vec2i *resultPos) +std::optional AiFindBuildingPlace(const CUnit &worker, const CUnitType &type, const Vec2i &nearPos) { // Find a good place for a new hall DebugPrint("%d: Want to build a %s(%s)\n", @@ -453,24 +476,24 @@ bool AiFindBuildingPlace(const CUnit &worker, const CUnitType &type, const Vec2i //Depots if (type.CanStore[i]) { if (resinfo && resinfo->TerrainHarvester) { - return AiFindLumberMillPlace(worker, type, startPos, i, resultPos); + return LumberMillPlaceFinder::find(worker, type, startPos, i); } else { - return AiFindHallPlace(worker, type, startPos, i, resultPos); + return HallPlaceFinder::find(worker, type, startPos, i); } } else { //mines if (type.GivesResource == i) { if (resinfo && resinfo->RefineryHarvester) { //Mine have to be build ONTOP resources - return AiFindMiningPlace(worker, type, startPos, i, resultPos); + return AiFindMiningPlace(worker, type, startPos, i); } else { //Mine can be build without resource restrictions: solar panels, etc - return AiFindBuildingPlace2(worker, type, startPos, nullptr, true, resultPos); + return BuildingPlaceFinder::find(worker, type, startPos, true); } } } } - return AiFindBuildingPlace2(worker, type, startPos, nullptr, true, resultPos); + return BuildingPlaceFinder::find(worker, type, startPos, true); } //@} diff --git a/src/ai/ai_force.cpp b/src/ai/ai_force.cpp index abfcba055a..b31ee80278 100644 --- a/src/ai/ai_force.cpp +++ b/src/ai/ai_force.cpp @@ -60,15 +60,26 @@ class EnemyUnitFinder { public: - EnemyUnitFinder(const CUnit &unit, CUnit **result_unit, int find_type) : - //Wyrmgus end + friend TerrainTraversal; + + static CUnit *find(const CUnit& unit, int find_type) { + // Terrain traversal by Andrettin + TerrainTraversal terrainTraversal; + terrainTraversal.SetSize(Map.Info.MapWidth, Map.Info.MapHeight); + terrainTraversal.Init(); + terrainTraversal.PushUnitPosAndNeighboor(unit); + EnemyUnitFinder enemyUnitFinder(unit, find_type); + terrainTraversal.Run(enemyUnitFinder); + return enemyUnitFinder.result_unit; + } + +private: + EnemyUnitFinder(const CUnit &unit, int find_type) : unit(unit), movemask(unit.Type->MovementMask & ~(MapFieldLandUnit | MapFieldAirUnit | MapFieldSeaUnit)), attackrange(unit.Stats->Variables[ATTACKRANGE_INDEX].Max), - find_type(find_type), - result_unit(result_unit) + find_type(find_type) { - *result_unit = nullptr; } VisitResult Visit(TerrainTraversal &terrainTraversal, const Vec2i &pos, const Vec2i &from); private: @@ -76,7 +87,7 @@ class EnemyUnitFinder unsigned int movemask; const int attackrange; const int find_type; - CUnit **result_unit; + CUnit *result_unit = nullptr; }; VisitResult EnemyUnitFinder::Visit(TerrainTraversal &terrainTraversal, const Vec2i &pos, const Vec2i &from) @@ -102,10 +113,10 @@ VisitResult EnemyUnitFinder::Visit(TerrainTraversal &terrainTraversal, const Vec } if ((find_type != AIATTACK_BUILDING || dtype.BoolFlag[BUILDING_INDEX].value) && (find_type != AIATTACK_AGRESSIVE || dest->IsAgressive())) { - *result_unit = dest; + result_unit = dest; return VisitResult::Finished; - } else if (*result_unit == nullptr) { // if trying to search for buildings or aggressive units specifically, still put the first found unit (even if it doesn't fit those parameters) as the result unit, so that it can be returned if no unit with the specified parameters is found - *result_unit = dest; + } else if (result_unit == nullptr) { // if trying to search for buildings or aggressive units specifically, still put the first found unit (even if it doesn't fit those parameters) as the result unit, so that it can be returned if no unit with the specified parameters is found + result_unit = dest; } } return VisitResult::Ok; @@ -115,21 +126,16 @@ template class AiForceEnemyFinder { public: - AiForceEnemyFinder(int force, const CUnit **enemy) : enemy(enemy) - { - Assert(enemy != nullptr); - *enemy = nullptr; - for (const CUnit *unit : AiPlayer->Force[force].Units) { - if (!(*this)(unit)) { - break; - } - } + static const CUnit* find(AiForce& force) { + return AiForceEnemyFinder(force).enemy; } - AiForceEnemyFinder(AiForce &force, const CUnit **enemy) : enemy(enemy) + static const CUnit *find(int force) { return find(AiPlayer->Force[force]); } + +private: + + AiForceEnemyFinder(AiForce &force) { - Assert(enemy != nullptr); - *enemy = nullptr; for (const CUnit* unit : force.Units) { if (!(*this)(unit)) { break; @@ -137,48 +143,20 @@ class AiForceEnemyFinder } } - bool found() const { return *enemy != nullptr; } - - bool operator()(const CUnit *const unit) const + bool operator()(const CUnit *const unit) { if (unit->Type->CanAttack == false) { - return *enemy == nullptr; + return enemy == nullptr; } if constexpr (FIND_TYPE == AIATTACK_RANGE) { - *enemy = AttackUnitsInReactRange(*unit); + enemy = AttackUnitsInReactRange(*unit); } else { - // Terrain traversal by Andrettin - TerrainTraversal terrainTraversal; - terrainTraversal.SetSize(Map.Info.MapWidth, Map.Info.MapHeight); - terrainTraversal.Init(); - terrainTraversal.PushUnitPosAndNeighboor(*unit); - CUnit *result_unit = nullptr; - EnemyUnitFinder enemyUnitFinder(*unit, &result_unit, FIND_TYPE); - terrainTraversal.Run(enemyUnitFinder); - *enemy = result_unit; + enemy = EnemyUnitFinder::find(*unit, FIND_TYPE); } - // Previous attack finding code before we added TerrainTraversal for all - // of these. Here for prosperity - - // } else if (FIND_TYPE == AIATTACK_ALLMAP) { - // *enemy = AttackUnitsInDistance(*unit, MaxMapWidth); - // } else if (FIND_TYPE == AIATTACK_BUILDING) { - // *enemy = AttackUnitsInDistance(*unit, MaxMapWidth, IsBuildingType()); - // Assert(!*enemy); - // if (*enemy == nullptr || !(*enemy)->Type->Building) { - // *enemy = AttackUnitsInDistance(*unit, MaxMapWidth); - // } - // } else if (FIND_TYPE == AIATTACK_AGRESSIVE) { - // *enemy = AttackUnitsInDistance(*unit, MaxMapWidth, IsAggresiveUnit()); - // Assert(!*enemy || (*enemy)->IsAgressive()); - // if (*enemy == nullptr) { - // *enemy = AttackUnitsInDistance(*unit, MaxMapWidth); - // } - // } - return *enemy == nullptr; + return enemy == nullptr; } private: - const CUnit **enemy; + const CUnit *enemy = nullptr; }; class IsAnAlliedUnitOf @@ -258,15 +236,6 @@ std::vector AiFindUnitTypeEquiv(const CUnitType &unittype) return result; } -class UnitTypePrioritySorter_Decreasing -{ -public: - bool operator()(int lhs, int rhs) const - { - return UnitTypes[lhs]->MapDefaultStat.Variables[PRIORITY_INDEX].Value > UnitTypes[rhs]->MapDefaultStat.Variables[PRIORITY_INDEX].Value; - } -}; - /** ** Find All unittypes equivalent to a given one, and which are available ** UnitType are returned in the preferred order (ie paladin >> knight...) @@ -284,7 +253,9 @@ std::vector AiFindAvailableUnitTypeEquiv(const CUnitType &unittype) return !CheckDependByIdent(*AiPlayer->Player, UnitTypes[typeIndex]->Ident); }); // 3 - Sort by level - ranges::sort(usableTypes, UnitTypePrioritySorter_Decreasing()); + ranges::sort(usableTypes, std::greater<>(), [](int index) { + return UnitTypes[index]->MapDefaultStat.Variables[PRIORITY_INDEX].Value; + }); return usableTypes; } @@ -310,7 +281,7 @@ std::vector AiForce::CountTypes() const bool AiForce::IsBelongsTo(const CUnitType &type) { bool flag = false; - auto counter = CountTypes(); + const auto counter = CountTypes(); // Look what should be in the force. Completed = true; @@ -389,7 +360,7 @@ VisitResult AiForceRallyPointFinder::Visit(TerrainTraversal &terrainTraversal, c } } -bool AiForce::NewRallyPoint(const Vec2i &startPos, Vec2i *resultPos) +std::optional AiForce::NewRallyPoint(const Vec2i &startPos) { Assert(this->Units.size() > 0); const CUnit &leader = *(this->Units[0]); @@ -405,9 +376,11 @@ bool AiForce::NewRallyPoint(const Vec2i &startPos, Vec2i *resultPos) Assert(Map.Info.IsPointOnMap(startPos)); terrainTraversal.PushPos(startPos); - AiForceRallyPointFinder aiForceRallyPointFinder(leader, distance, leader.tilePos, resultPos); + Vec2i resultPos; + AiForceRallyPointFinder aiForceRallyPointFinder(leader, distance, leader.tilePos, &resultPos); - return terrainTraversal.Run(aiForceRallyPointFinder); + const bool found = terrainTraversal.Run(aiForceRallyPointFinder); + return found ? std::make_optional(resultPos) : std::nullopt; } void AiForce::Attack(const Vec2i &pos) @@ -438,14 +411,9 @@ void AiForce::Attack(const Vec2i &pos) bool isDefenceForce = false; if (Map.Info.IsPointOnMap(goalPos) == false) { /* Search in entire map */ - const CUnit *enemy = nullptr; - if (isTransporter) { - AiForceEnemyFinder(*this, &enemy); - } else if (isNaval) { - AiForceEnemyFinder(*this, &enemy); - } else { - AiForceEnemyFinder(*this, &enemy); - } + const CUnit *enemy = isTransporter ? AiForceEnemyFinder::find(*this) + : isNaval ? AiForceEnemyFinder::find(*this) + : AiForceEnemyFinder::find(*this); if (enemy) { goalPos = enemy->tilePos; } @@ -461,10 +429,9 @@ void AiForce::Attack(const Vec2i &pos) return; } if (this->State == AiForceAttackingState::Waiting && isDefenceForce == false) { - Vec2i resultPos {-1, -1}; - NewRallyPoint(goalPos, &resultPos); - if (resultPos.x != -1 && resultPos.y != -1) { - this->GoalPos = resultPos; + const auto rallyPoint = NewRallyPoint(goalPos); + if (rallyPoint) { + this->GoalPos = *rallyPoint; this->State = AiForceAttackingState::GoingToRallyPoint; } else { this->GoalPos = goalPos; @@ -516,7 +483,7 @@ void AiForce::ReturnToHome() AiForceManager::AiForceManager() { forces.resize(AI_MAX_FORCES); - memset(script, -1, AI_MAX_FORCES * sizeof(char)); + ranges::fill(script, -1); } unsigned int AiForceManager::FindFreeForce(AiForceRole role, int begin) @@ -861,9 +828,9 @@ void AiForce::Update() if (Size() == 0) { Attacking = false; if (!Defending && State > AiForceAttackingState::Waiting) { - DebugPrint("%d: Attack force #%lu was destroyed, giving up\n", - AiPlayer->Player->Index, - (long unsigned int) (this - &(AiPlayer->Force[0]))); + DebugPrint("%d: Attack force #%lu was destroyed, giving up\n", + AiPlayer->Player->Index, + (long unsigned int) (this - &(AiPlayer->Force[0]))); Reset(true); } return; @@ -948,12 +915,10 @@ void AiForce::Update() --WaitOnRallyPoint; } if (maxDist <= thresholdDist || !WaitOnRallyPoint) { - const CUnit *unit = nullptr; - - AiForceEnemyFinder(*this, &unit); - if (!unit) { - AiForceEnemyFinder(*this, &unit); - if (!unit) { + const CUnit *unit = AiForceEnemyFinder::find(*this); + if (unit == nullptr) { + unit = AiForceEnemyFinder::find(*this); + if (unit == nullptr) { // No enemy found, give up // FIXME: should the force go home or keep trying to attack? DebugPrint("%d: Attack force #%lu can't find a target, giving up\n", @@ -995,12 +960,8 @@ void AiForce::Update() const bool isNaval = ranges::any_of(this->Units, [](const CUnit *unit) { return unit->Type->MoveType == EMovement::Naval && unit->Type->CanAttack; }); - const CUnit *unit = nullptr; - if (isNaval) { - AiForceEnemyFinder(*this, &unit); - } else { - AiForceEnemyFinder(*this, &unit); - } + const CUnit *unit = isNaval ? AiForceEnemyFinder::find(*this) + : AiForceEnemyFinder::find(*this); if (!unit) { // No enemy found, give up // FIXME: should the force go home or keep trying to attack? @@ -1011,10 +972,9 @@ void AiForce::Update() State = AiForceAttackingState::Waiting; return; } else { - Vec2i resultPos {-1, -1}; - NewRallyPoint(unit->tilePos, &resultPos); - if (resultPos.x != -1 && resultPos.y != -1) { - this->GoalPos = resultPos; + const auto rallyPoint = NewRallyPoint(unit->tilePos); + if (rallyPoint) { + this->GoalPos = *rallyPoint; this->State = AiForceAttackingState::GoingToRallyPoint; } else { this->GoalPos = unit->tilePos; @@ -1075,9 +1035,8 @@ void AiForceManager::Update() for (const CUnit *aiunit : force.Units) { if (aiunit->MapDistanceTo(force.GoalPos) <= nearDist) { // Look if still enemies in attack range. - const CUnit *dummy = nullptr; maxPathing--; - if (!AiForceEnemyFinder(force, &dummy).found()) { + if (AiForceEnemyFinder::find(force) == nullptr) { force.ReturnToHome(); } } diff --git a/src/ai/ai_local.h b/src/ai/ai_local.h index b37e1d38b4..f7f833cfee 100644 --- a/src/ai/ai_local.h +++ b/src/ai/ai_local.h @@ -162,7 +162,7 @@ class AiForce bool PlanAttack(); void ReturnToHome(); - bool NewRallyPoint(const Vec2i &startPos, Vec2i *resultPos); + std::optional NewRallyPoint(const Vec2i &startPos); void Insert(CUnit &unit); private: @@ -419,7 +419,7 @@ extern CUnit *AiGetSuitableDepot(const CUnit &worker, const CUnit &oldDepot, CUn // Buildings // /// Find nice building place -extern bool AiFindBuildingPlace(const CUnit &worker, const CUnitType &type, const Vec2i &nearPos, Vec2i *resultPos); +extern std::optional AiFindBuildingPlace(const CUnit &worker, const CUnitType &type, const Vec2i &nearPos); // // Forces diff --git a/src/ai/ai_resource.cpp b/src/ai/ai_resource.cpp index cd2a78d424..780f7ea873 100644 --- a/src/ai/ai_resource.cpp +++ b/src/ai/ai_resource.cpp @@ -272,10 +272,9 @@ static bool AiBuildBuilding(const CUnitType &type, CUnitType &building, const Ve CUnit &candidate = (table.size() == 1) ? *table[0] : *table[SyncRand() % table.size()]; - Vec2i pos; // Find a place to build. - if (AiFindBuildingPlace(candidate, building, nearPos, &pos)) { - CommandBuildBuilding(candidate, pos, building, FlushCommands); + if (auto pos = AiFindBuildingPlace(candidate, building, nearPos)) { + CommandBuildBuilding(candidate, *pos, building, FlushCommands); return true; } else { //when first worker can't build then rest also won't be able (save CPU) @@ -286,8 +285,8 @@ static bool AiBuildBuilding(const CUnitType &type, CUnitType &building, const Ve continue; } // Find a place to build. - if (AiFindBuildingPlace(*unit, building, nearPos, &pos)) { - CommandBuildBuilding(*unit, pos, building, FlushCommands); + if (auto pos = AiFindBuildingPlace(*unit, building, nearPos)) { + CommandBuildBuilding(*unit, *pos, building, FlushCommands); return true; } } diff --git a/src/include/util.h b/src/include/util.h index 13f05e94da..924f01a2c1 100644 --- a/src/include/util.h +++ b/src/include/util.h @@ -345,10 +345,12 @@ namespace ranges } } - template > - void sort(Range &range, Comparer &&comparer = {}) + template , typename Proj = identity> + void sort(Range &range, Comparer &&comparer = {}, Proj &&proj = {}) { - std::sort(std::begin(range), std::end(range), std::forward(comparer)); + std::sort(std::begin(range), std::end(range), [&](const auto &lhs, const auto &rhs) { + return std::invoke(comparer, std::invoke(proj, lhs), std::invoke(proj, rhs)); + }); } template , typename Proj = identity> From 8bc782e960403422e8912890393154372e1e3c67 Mon Sep 17 00:00:00 2001 From: Jarod42 Date: Thu, 2 May 2024 12:50:29 +0200 Subject: [PATCH 2/4] Replace output parameters of `GetMissileMapArea`. --- src/missile/missile.cpp | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/missile/missile.cpp b/src/missile/missile.cpp index 33a8cb7457..cdec53295b 100644 --- a/src/missile/missile.cpp +++ b/src/missile/missile.cpp @@ -393,19 +393,18 @@ void FireMissile(CUnit &unit, CUnit *goal, const Vec2i &goalPos) ** Get area of tiles covered by missile ** ** @param missile Missile to be checked and set. -** @param boxMin OUT: Pointer to top left corner in map tiles. -** @param boxMax OUT: Pointer to bottom right corner in map tiles. ** -** @return sx,sy,ex,ey defining area in Map +** @return tiles area in Map */ -static void GetMissileMapArea(const Missile &missile, Vec2i &boxMin, Vec2i &boxMax) +static std::pair GetMissileMapArea(const Missile &missile) { PixelSize missileSize(missile.Type->Width(), missile.Type->Height()); PixelDiff margin(PixelTileSize.x - 1, PixelTileSize.y - 1); - boxMin = Map.MapPixelPosToTilePos(missile.position); - boxMax = Map.MapPixelPosToTilePos(missile.position + missileSize + margin); + Vec2i boxMin = Map.MapPixelPosToTilePos(missile.position); + Vec2i boxMax = Map.MapPixelPosToTilePos(missile.position + missileSize + margin); Map.Clamp(boxMin); Map.Clamp(boxMax); + return {boxMin, boxMax}; } /** @@ -418,10 +417,7 @@ static void GetMissileMapArea(const Missile &missile, Vec2i &boxMin, Vec2i &boxM */ static bool MissileVisibleInViewport(const CViewport &vp, const Missile &missile) { - Vec2i boxmin; - Vec2i boxmax; - - GetMissileMapArea(missile, boxmin, boxmax); + const auto [boxmin, boxmax] = GetMissileMapArea(missile); if (!vp.AnyMapAreaVisibleInViewport(boxmin, boxmax)) { return false; } From 0a13a78ba23c66024e1184d1aea67853e29635fa Mon Sep 17 00:00:00 2001 From: Jarod42 Date: Thu, 2 May 2024 13:00:39 +0200 Subject: [PATCH 3/4] Replace output parameter of `FindTerrainType`. --- src/action/action_resource.cpp | 42 +++++++++++++++------------------- src/ai/ai_resource.cpp | 6 ++--- src/include/unit_find.h | 4 ++-- src/unit/unit_find.cpp | 27 +++++++++++----------- 4 files changed, 36 insertions(+), 43 deletions(-) diff --git a/src/action/action_resource.cpp b/src/action/action_resource.cpp index 7783154deb..7e26446f88 100644 --- a/src/action/action_resource.cpp +++ b/src/action/action_resource.cpp @@ -389,16 +389,15 @@ bool COrder_Resource::OnAiHitUnit(CUnit &unit, CUnit *attacker, int /* damage*/) */ int COrder_Resource::MoveToResource_Terrain(CUnit &unit) { - Vec2i pos = this->goalPos; - // Wood gone, look somewhere else. - if ((Map.Info.IsPointOnMap(pos) == false || Map.Field(pos)->IsTerrainResourceOnMap(CurrentResource) == false) + if ((Map.Info.IsPointOnMap(this->goalPos) == false + || Map.Field(this->goalPos)->IsTerrainResourceOnMap(CurrentResource) == false) && (!unit.IX) && (!unit.IY)) { - if (!FindTerrainType(unit.Type->MovementMask, MapFieldForest, 16, *unit.Player, this->goalPos, &pos)) { + if (auto pos = FindTerrainType(unit.Type->MovementMask, MapFieldForest, 16, *unit.Player, this->goalPos)) { + this->goalPos = *pos; + } else { // no wood in range return -1; - } else { - this->goalPos = pos; } } switch (DoActionMove(unit)) { @@ -411,12 +410,14 @@ int COrder_Resource::MoveToResource_Terrain(CUnit &unit) AiCanNotMove(unit); } } - if (FindTerrainType(unit.Type->MovementMask, MapFieldForest, 9999, *unit.Player, unit.tilePos, &pos)) { - this->goalPos = pos; - DebugPrint("Found a better place to harvest %d,%d\n", pos.x, pos.y); - // FIXME: can't this overflow? It really shouldn't, since - // x and y are really supossed to be reachable, checked thorugh a flood fill. - // I don't know, sometimes stuff happens. + if (auto pos = FindTerrainType( + unit.Type->MovementMask, MapFieldForest, 9999, *unit.Player, unit.tilePos)) { + this->goalPos = *pos; + DebugPrint("Found a better place to harvest %d,%d\n", pos->x, pos->y); + // FIXME: can't this overflow? It really shouldn't, + // since x and y are really supposed to be reachable, + // checked through a flood fill. + // I don't know, sometime stuff happens. return 0; } return -1; @@ -1091,13 +1092,11 @@ bool COrder_Resource::WaitInDepot(CUnit &unit) // Range hardcoded. don't stray too far though if (resinfo.TerrainHarvester) { - Vec2i pos = this->Resource.Pos; - - if (FindTerrainType(unit.Type->MovementMask, MapFieldForest, 10, *unit.Player, pos, &pos)) { + if (auto pos = FindTerrainType(unit.Type->MovementMask, MapFieldForest, 10, *unit.Player, this->Resource.Pos)) { if (depot) { - DropOutNearest(unit, pos, depot); + DropOutNearest(unit, *pos, depot); } - this->goalPos = pos; + this->goalPos = *pos; } else { if (depot) { DropOutOnSide(unit, LookingW, depot); @@ -1140,8 +1139,6 @@ bool COrder_Resource::WaitInDepot(CUnit &unit) goal = UnitFindResource(unit, (start_unit ? *start_unit : unit), 1000, this->CurrentResource, unit.Player->AiEnabled, (newdepot ? newdepot : depot)); } - - } if (goal) { @@ -1247,11 +1244,10 @@ bool COrder_Resource::FindAnotherResource(CUnit &unit) return true; } } else { - Vec2i resPos; - if (FindTerrainType(unit.Type->MovementMask, MapFieldForest, 8, *unit.Player, unit.tilePos, &resPos)) { - this->goalPos = resPos; + if (auto resPos = FindTerrainType(unit.Type->MovementMask, MapFieldForest, 8, *unit.Player, unit.tilePos)) { + this->goalPos = *resPos; this->State = SUB_MOVE_TO_RESOURCE; - DebugPrint("Found a better place to harvest %d,%d\n", resPos.x, resPos.y); + DebugPrint("Found a better place to harvest %d,%d\n", resPos->x, resPos->y); return true; } } diff --git a/src/ai/ai_resource.cpp b/src/ai/ai_resource.cpp index 780f7ea873..4898f5411d 100644 --- a/src/ai/ai_resource.cpp +++ b/src/ai/ai_resource.cpp @@ -900,11 +900,9 @@ static void AiCheckingWork() static bool AiAssignHarvesterFromTerrain(CUnit &unit, int resource) { // TODO : hardcoded forest - Vec2i forestPos; - // Code for terrain harvesters. Search for piece of terrain to mine. - if (FindTerrainType(unit.Type->MovementMask, MapFieldForest, 1000, *unit.Player, unit.tilePos, &forestPos)) { - CommandResourceLoc(unit, forestPos, FlushCommands); + if (auto forestPos = FindTerrainType(unit.Type->MovementMask, MapFieldForest, 1000, *unit.Player, unit.tilePos)) { + CommandResourceLoc(unit, *forestPos, FlushCommands); return true; } // Ask the AI to explore... diff --git a/src/include/unit_find.h b/src/include/unit_find.h index a06c1e9808..df1192fc13 100644 --- a/src/include/unit_find.h +++ b/src/include/unit_find.h @@ -328,8 +328,8 @@ extern CUnit *FindDeposit(const CUnit &unit, int range, int resource); extern CUnit *FindIdleWorker(const CPlayer &player, const CUnit *last); /// Find the neareast piece of terrain with specific flags. -extern bool FindTerrainType(int movemask, int resmask, int range, - const CPlayer &player, const Vec2i &startPos, Vec2i *pos); +extern std::optional +FindTerrainType(int movemask, int resmask, int range, const CPlayer &player, const Vec2i &startPos); extern std::vector FindUnitsByType(const CUnitType &type, bool everybody = false); diff --git a/src/unit/unit_find.cpp b/src/unit/unit_find.cpp index 7d32590f9a..d476e57d98 100644 --- a/src/unit/unit_find.cpp +++ b/src/unit/unit_find.cpp @@ -101,16 +101,20 @@ VisitResult UnitFinder::Visit(TerrainTraversal &terrainTraversal, const Vec2i &p class TerrainFinder { + friend std::optional + FindTerrainType(int movemask, int resmask, int range, const CPlayer &, const Vec2i &startPos); + public: - TerrainFinder(const CPlayer &player, int maxDist, int movemask, int resmask, Vec2i *resPos) : - player(player), maxDist(maxDist), movemask(movemask), resmask(resmask), resPos(resPos) {} + + TerrainFinder(const CPlayer &player, int maxDist, int movemask, int resmask) : + player(player), maxDist(maxDist), movemask(movemask), resmask(resmask) {} VisitResult Visit(TerrainTraversal &terrainTraversal, const Vec2i &pos, const Vec2i &from); private: const CPlayer &player; int maxDist; int movemask; int resmask; - Vec2i *resPos; + Vec2i resPos; }; VisitResult TerrainFinder::Visit(TerrainTraversal &terrainTraversal, const Vec2i &pos, const Vec2i &from) @@ -120,9 +124,7 @@ VisitResult TerrainFinder::Visit(TerrainTraversal &terrainTraversal, const Vec2i } // Look if found what was required. if (Map.Field(pos)->CheckMask(resmask)) { - if (resPos) { - *resPos = pos; - } + resPos = pos; return VisitResult::Finished; } if (CanMoveToMask(pos, movemask)) { // reachable @@ -145,16 +147,14 @@ VisitResult TerrainFinder::Visit(TerrainTraversal &terrainTraversal, const Vec2i ** @param player Only search fields explored by player ** @param startPos Map start position for the search. ** -** @param terrainPos OUT: Map position of tile. -** ** @note Movement mask can be 0xFFFFFFFF to have no effect ** Range is not circular, but square. ** Player is ignored if nil(search the entire map) ** -** @return True if wood was found. +** @return wood position if found, else std::nullopt. */ -bool FindTerrainType(int movemask, int resmask, int range, - const CPlayer &player, const Vec2i &startPos, Vec2i *terrainPos) +std::optional +FindTerrainType(int movemask, int resmask, int range, const CPlayer &player, const Vec2i &startPos) { TerrainTraversal terrainTraversal; @@ -163,12 +163,11 @@ bool FindTerrainType(int movemask, int resmask, int range, terrainTraversal.PushPos(startPos); - TerrainFinder terrainFinder(player, range, movemask & ~(MapFieldLandUnit | MapFieldAirUnit | MapFieldSeaUnit), resmask, terrainPos); + TerrainFinder terrainFinder(player, range, movemask & ~(MapFieldLandUnit | MapFieldAirUnit | MapFieldSeaUnit), resmask); - return terrainTraversal.Run(terrainFinder); + return terrainTraversal.Run(terrainFinder) ? std::make_optional(terrainFinder.resPos) : std::nullopt; } - template class BestDepotFinder { From f3d3688d8e4d9f57d1113a5fa673a10ad4706ed9 Mon Sep 17 00:00:00 2001 From: Jarod42 Date: Thu, 2 May 2024 13:56:24 +0200 Subject: [PATCH 4/4] Fix comment about out parameter. --- src/action/actions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/action/actions.cpp b/src/action/actions.cpp index c33705bea6..4394eea5a3 100644 --- a/src/action/actions.cpp +++ b/src/action/actions.cpp @@ -212,7 +212,7 @@ void COrder::UpdatePathFinderData_NotCalled(PathFinderInput &input) ** Parse order ** ** @param l Lua state. -** @param order OUT: resulting order. +** @return resulting order. */ std::unique_ptr CclParseOrder(lua_State *l, CUnit &unit) {