diff --git a/libs/s25main/Ware.cpp b/libs/s25main/Ware.cpp index af310ceda1..160da9c6d4 100644 --- a/libs/s25main/Ware.cpp +++ b/libs/s25main/Ware.cpp @@ -338,7 +338,7 @@ unsigned Ware::CheckNewGoalForLostWare(const noBaseBuilding& newgoal) const Ware::RouteParams Ware::CalcPathToGoal(const noBaseBuilding& newgoal) const { RTTR_Assert(location); - unsigned length = 0xFFFFFFFF; + unsigned length; RoadPathDirection possibledir = world->FindPathForWareOnRoads(*location, newgoal, &length); if(possibledir != RoadPathDirection::None) // there is a valid path to the goal? -> ordered! { @@ -346,19 +346,17 @@ Ware::RouteParams Ware::CalcPathToGoal(const noBaseBuilding& newgoal) const // because non-warehouses cannot just carry in new wares they need a helper to do this if(possibledir == RoadPathDirection::NorthWest && newgoal.GetFlagPos() == location->GetPos()) { + // Not executed for road from flag to the warehouse as that is handled directly by the warehouse + RTTR_Assert(!BuildingProperties::IsWareHouse(newgoal.GetBuildingType())); for(const auto dir : helpers::EnumRange{}) { + // Bounce of in this direction if(dir != Direction::NorthWest && location->GetRoute(dir)) - { - possibledir = toRoadPathDirection(dir); - break; - } + return {1, toRoadPathDirection(dir)}; } - if(possibledir == RoadPathDirection::NorthWest) // got no other route from the flag -> impossible - return {0xFFFFFFFF, RoadPathDirection::None}; + // got no other route from the flag -> impossible + return {0xFFFFFFFF, RoadPathDirection::None}; } - // at this point there either is a road to the goal - // or we are at the flag of the goal and have a road to a different flag to bounce off of to get to the goal return {length, possibledir}; } return {0xFFFFFFFF, RoadPathDirection::None}; diff --git a/libs/s25main/buildings/nobHarborBuilding.cpp b/libs/s25main/buildings/nobHarborBuilding.cpp index 8bd0c4c2ac..f321f16e8f 100644 --- a/libs/s25main/buildings/nobHarborBuilding.cpp +++ b/libs/s25main/buildings/nobHarborBuilding.cpp @@ -839,8 +839,8 @@ std::vector nobHarborBuilding::GetShipConnect { ShipConnection sc; sc.dest = harbor_building; - // Als Kantengewicht nehmen wir die doppelte Entfernung (evtl muss ja das Schiff erst kommen) - // plus einer Kopfpauschale (Ein/Ausladen usw. dauert ja alles) + // Use twice the distance as cost (ship might need to arrive first) and a fixed value to represent + // loading&unloading sc.way_costs = 2 * world->CalcHarborDistance(GetHarborPosID(), harbor_building->GetHarborPosID()) + 10; connections.push_back(sc); } diff --git a/libs/s25main/nodeObjs/noFlag.cpp b/libs/s25main/nodeObjs/noFlag.cpp index 9e31b95aaa..3f0911801b 100644 --- a/libs/s25main/nodeObjs/noFlag.cpp +++ b/libs/s25main/nodeObjs/noFlag.cpp @@ -228,13 +228,14 @@ unsigned noFlag::GetNumWaresForRoad(const Direction dir) const return helpers::count_if(wares, [roadDir](const auto& ware) { return ware->GetNextDir() == roadDir; }); } -/** - * Gibt Wegstrafpunkte für das Pathfinden für Waren, die in eine bestimmte - * Richtung noch transportiert werden müssen. - */ unsigned noFlag::GetPunishmentPoints(const Direction dir) const { - // Waren zählen, die in diese Richtung transportiert werden müssen + constexpr auto PATHFINDING_PENALTY_CARRIER_ARRIVING = 50; + constexpr auto PATHFINDING_PENALTY_NO_CARRIER = 500; + // This must be the same as "NO_CARRIER" for replay compatibility + // TODO(Replay): Move to `way_costs` in nobHarborBuilding::GetShipConnections + constexpr auto PATHFINDING_PENALTY_START_SHIPPING = 500; + // 2 Points per ware as carrier has to walk to other point and back for each ware unsigned points = GetNumWaresForRoad(dir) * 2; const RoadSegment* routeInDir = GetRoute(dir); @@ -243,9 +244,20 @@ unsigned noFlag::GetPunishmentPoints(const Direction dir) const { // normal carrier has been ordered from the warehouse but has not yet arrived and no donkey if(humanCarrier->GetCarrierState() == CarrierState::FigureWork && !routeInDir->hasCarrier(1)) - points += 50; + points += PATHFINDING_PENALTY_CARRIER_ARRIVING; } else if(!routeInDir->hasCarrier(1)) - points += 500; // No carrier at all -> Large penalty + { + // Routes are either between flags or from a flag to a building, see the ctor of noBaseBuilding + const bool isBuildingEntry = (dir == Direction::NorthWest) && (routeInDir->GetF2()->GetGOT() != GO_Type::Flag); + // For entering a building no carrier is required + // Only roads to buildings considered by path finding are those to harbors + if(isBuildingEntry) + { + RTTR_Assert(routeInDir->GetF2()->GetGOT() == GO_Type::NobHarborbuilding); + points += PATHFINDING_PENALTY_START_SHIPPING; + } else + points += PATHFINDING_PENALTY_NO_CARRIER; + } return points; } diff --git a/libs/s25main/nodeObjs/noFlag.h b/libs/s25main/nodeObjs/noFlag.h index 4e5e5beee5..bccc6c3a17 100644 --- a/libs/s25main/nodeObjs/noFlag.h +++ b/libs/s25main/nodeObjs/noFlag.h @@ -46,8 +46,7 @@ class noFlag : public noRoadNode std::unique_ptr SelectWare(Direction roadDir, bool swap_wares, const noFigure* carrier); /// Prüft, ob es Waren gibt, die auf den Weg in Richtung dir getragen werden müssen. unsigned GetNumWaresForRoad(Direction dir) const; - /// Gibt Wegstrafpunkte für das Pathfinden für Waren, die in eine bestimmte Richtung noch transportiert werden - /// müssen. + /// Penalty for transporting a ware in a specific direction unsigned GetPunishmentPoints(Direction dir) const override; /// Zerstört evtl. vorhandenes Gebäude bzw. Baustelle vor der Flagge. void DestroyAttachedBuilding(); diff --git a/libs/s25main/pathfinding/RoadPathFinder.cpp b/libs/s25main/pathfinding/RoadPathFinder.cpp index 9a4a9b066c..681b0c44d3 100644 --- a/libs/s25main/pathfinding/RoadPathFinder.cpp +++ b/libs/s25main/pathfinding/RoadPathFinder.cpp @@ -20,8 +20,7 @@ struct RoadNodeComperatorGreater { if(lhs->estimate == rhs->estimate) { - // Wenn die Wegkosten gleich sind, vergleichen wir die Koordinaten, da wir für std::set eine streng - // monoton steigende Folge brauchen + // Use a tie-breaker to get strict ordering return (lhs->GetObjId() > rhs->GetObjId()); } @@ -100,7 +99,9 @@ struct And : private T_Func1, private T_Func2 }; } // namespace SegmentConstraints -/// Wegfinden ( A* ), O(v lg v) --> Wegfindung auf Stra�en +/// Path finding on roads using A* O(n lg n) +/// \tparam T_AdditionalCosts Cost for each road segment but the one to the goal building +/// \tparam T_SegmentConstraints Predicate whether a road is allowed template bool RoadPathFinder::FindPathImpl(const noRoadNode& start, const noRoadNode& goal, const unsigned max, const T_AdditionalCosts addCosts, const T_SegmentConstraints isSegmentAllowed, @@ -124,9 +125,12 @@ bool RoadPathFinder::FindPathImpl(const noRoadNode& start, const noRoadNode& goa return true; } - // increase current_visit_on_roads, so we don't have to clear the visited-states at every run - currentVisit++; + // If the goal is a flag (unlikely) we have no goal building + // TODO(Replay): Change RoadPathFinder::FindPath to target flag instead of building for wares + const noRoadNode* goalBld = (goal.GetGOT() == GO_Type::Flag) ? nullptr : &goal; + // Use a counter for the visited-states so we don't have to reset them on every invocation + currentVisit++; // if the counter reaches its maximum, tidy up if(currentVisit == std::numeric_limits::max()) { @@ -163,19 +167,20 @@ bool RoadPathFinder::FindPathImpl(const noRoadNode& start, const noRoadNode& goa if(length) *length = best.cost; - // Backtrace to get the last node that is not the start node (has a prev node) --> Next node from start on - // path - const noRoadNode* firstNode = &best; - while(firstNode->prev != &start) + // Backtrack to get the last node that is not the start node (has a prev node) + // --> Next node from start on path + if(firstDir || firstNodePos) { - firstNode = firstNode->prev; - } + const noRoadNode* firstNode = &best; + while(firstNode->prev != &start) + firstNode = firstNode->prev; - if(firstDir) - *firstDir = firstNode->dir_; + if(firstDir) + *firstDir = firstNode->dir_; - if(firstNodePos) - *firstNodePos = firstNode->GetPos(); + if(firstNodePos) + *firstNodePos = firstNode->GetPos(); + } // Done, path found return true; @@ -184,7 +189,7 @@ bool RoadPathFinder::FindPathImpl(const noRoadNode& start, const noRoadNode& goa const helpers::EnumArray routes = best.getRoutes(); const noRoadNode* prevNode = best.prev; - // Nachbarflagge bzw. Wege in allen 6 Richtungen verfolgen + // Check paths in all directions for(const auto dir : helpers::EnumRange{}) { const auto* route = routes[dir]; @@ -209,12 +214,11 @@ bool RoadPathFinder::FindPathImpl(const noRoadNode& start, const noRoadNode& goa continue; } - // evtl verboten? + // Check if route is forbidden if(!isSegmentAllowed(*route)) continue; - unsigned cost = best.cost + route->GetLength(); - cost += addCosts(best, dir); + const unsigned cost = best.cost + route->GetLength() + (neighbour != goalBld ? addCosts(best, dir) : 0); if(cost > max) continue; @@ -283,7 +287,7 @@ bool RoadPathFinder::FindPathImpl(const noRoadNode& start, const noRoadNode& goa } } - // Liste leer und kein Ziel erreicht --> kein Weg + // Queue is empty without reaching the goal --> No allowed, existing path return false; } @@ -291,10 +295,11 @@ bool RoadPathFinder::FindPath(const noRoadNode& start, const noRoadNode& goal, c const RoadSegment* const forbidden, unsigned* const length, RoadPathDirection* const firstDir, MapPoint* const firstNodePos) { - RTTR_Assert(length || firstDir || firstNodePos); // If none of them is set use the \ref PathExist function! + RTTR_Assert_Msg(length || firstDir || firstNodePos, "Use PathExists instead!"); if(wareMode) { + // TODO(Replay): Change to target flag instead of its attached building if(forbidden) return FindPathImpl(start, goal, max, AdditonalCosts::Carrier(), SegmentConstraints::AvoidSegment(forbidden), length, firstDir, firstNodePos); @@ -319,6 +324,8 @@ bool RoadPathFinder::PathExists(const noRoadNode& start, const noRoadNode& goal, { if(allowWaterRoads) { + // TODO(Replay): Change to target flag instead of its attached building. + // Likely combine with RoadPathFinder::FindPath if(forbidden) return FindPathImpl(start, goal, max, AdditonalCosts::None(), SegmentConstraints::AvoidSegment(forbidden)); else