From 4c36059d1e8598e7a1c18ac8192ccc804793212e Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sat, 25 Apr 2026 18:40:40 +0200 Subject: [PATCH 01/11] Remove `GameWorld(Base)::CreateTradeGraphs` Not required (anymore), trade path cache can be created on demand. --- libs/s25main/world/GameWorld.cpp | 10 ++-------- libs/s25main/world/GameWorld.h | 6 +++--- libs/s25main/world/GameWorldBase.h | 4 ---- libs/s25main/world/MapLoader.cpp | 2 -- libs/s25main/world/MapSerializer.cpp | 1 - tests/s25Main/integration/testArmor.cpp | 1 - tests/s25Main/integration/testTrading.cpp | 1 - 7 files changed, 5 insertions(+), 20 deletions(-) diff --git a/libs/s25main/world/GameWorld.cpp b/libs/s25main/world/GameWorld.cpp index 919c7b4765..062f8a9442 100644 --- a/libs/s25main/world/GameWorld.cpp +++ b/libs/s25main/world/GameWorld.cpp @@ -656,13 +656,6 @@ void GameWorld::CleanTerritoryRegion(TerritoryRegion& region, TerritoryChangeRea } } -void GameWorld::CreateTradeGraphs() -{ - // Only if trade is enabled - if(GetGGS().isEnabled(AddonId::TRADE)) - tradePathCache = std::make_unique(*this); -} - void GameWorld::DestroyPlayerRests(const MapPoint pt, unsigned char newOwner, const noBaseBuilding* exception) { noBase* no = GetNode(pt).obj; @@ -864,7 +857,8 @@ void GameWorld::AttackViaSea(const unsigned char player_attacker, const MapPoint TradePathCache& GameWorld::GetTradePathCache() { - RTTR_Assert(tradePathCache); + if(!tradePathCache) + tradePathCache = std::make_unique(*this); return *tradePathCache; } diff --git a/libs/s25main/world/GameWorld.h b/libs/s25main/world/GameWorld.h index 8e7bbcaf43..38e6b2804d 100644 --- a/libs/s25main/world/GameWorld.h +++ b/libs/s25main/world/GameWorld.h @@ -20,6 +20,7 @@ class nofAttacker; struct PlayerInfo; class RoadSegment; class TerritoryRegion; +class TradePathCache; enum class TerritoryChangeReason { @@ -31,6 +32,8 @@ enum class TerritoryChangeReason /// "Interface-Klasse" für das Spiel class GameWorld : public GameWorldBase { + std::unique_ptr tradePathCache; + /// Destroys player belongings if that pint does not belong to the player anymore void DestroyPlayerRests(MapPoint pt, unsigned char newOwner, const noBaseBuilding* exception); @@ -164,9 +167,6 @@ class GameWorld : public GameWorldBase /// Recalculates where border stones should be done after a change in the given region void RecalcBorderStones(Position startPt, Extent areaSize); - /// Create Trade graphs - void CreateTradeGraphs() final; - protected: void VisibilityChanged(MapPoint pt, unsigned player, Visibility oldVis, Visibility newVis) override; }; diff --git a/libs/s25main/world/GameWorldBase.h b/libs/s25main/world/GameWorldBase.h index 13494c1c8d..8d01f57b21 100644 --- a/libs/s25main/world/GameWorldBase.h +++ b/libs/s25main/world/GameWorldBase.h @@ -27,7 +27,6 @@ class noFlag; class nofPassiveSoldier; class RoadPathFinder; class SoundManager; -class TradePathCache; constexpr Direction getOppositeDir(const RoadDir roadDir) noexcept { @@ -66,7 +65,6 @@ class GameWorldBase : public World /// Interface zum GUI GameInterface* gi; std::unique_ptr econHandler; - std::unique_ptr tradePathCache; public: GameWorldBase(std::vector players, const GlobalGameSettings& gameSettings, EventManager& em); @@ -74,8 +72,6 @@ class GameWorldBase : public World // Grundlegende Initialisierungen void Init(const MapExtent& mapSize, DescIdx lt = DescIdx(0)) override; - /// Create Trade graphs - virtual void CreateTradeGraphs() = 0; // Remaining initialization after loading (BQ...) void InitAfterLoad(); diff --git a/libs/s25main/world/MapLoader.cpp b/libs/s25main/world/MapLoader.cpp index 5fd781ce56..47b0683766 100644 --- a/libs/s25main/world/MapLoader.cpp +++ b/libs/s25main/world/MapLoader.cpp @@ -86,8 +86,6 @@ bool MapLoader::Load(const boost::filesystem::path& mapFilePath) if(!PlaceHQs()) return false; - world_.CreateTradeGraphs(); - return true; } diff --git a/libs/s25main/world/MapSerializer.cpp b/libs/s25main/world/MapSerializer.cpp index 85fe82a23c..78e84b5cbd 100644 --- a/libs/s25main/world/MapSerializer.cpp +++ b/libs/s25main/world/MapSerializer.cpp @@ -188,5 +188,4 @@ void MapSerializer::Deserialize(GameWorldBase& world, SerializedGameData& sgd, G } game.SetLua(std::move(lua)); } - world.CreateTradeGraphs(); } diff --git a/tests/s25Main/integration/testArmor.cpp b/tests/s25Main/integration/testArmor.cpp index 6d0ce4a06a..e18c13a3fa 100644 --- a/tests/s25Main/integration/testArmor.cpp +++ b/tests/s25Main/integration/testArmor.cpp @@ -79,7 +79,6 @@ struct ArmorTradeFixture : public ArmoredSoldierFixture // Enable trading this->ggs.setSelection(AddonId::TRADE, 1); - world.CreateTradeGraphs(); } void testExpectedFiguresInGlobalInventoryMatchWithHQInventory() const diff --git a/tests/s25Main/integration/testTrading.cpp b/tests/s25Main/integration/testTrading.cpp index 5d68d0137f..06f2117ead 100644 --- a/tests/s25Main/integration/testTrading.cpp +++ b/tests/s25Main/integration/testTrading.cpp @@ -52,7 +52,6 @@ struct TradeFixture : public WorldWithGCExecution3P // Enable trading this->ggs.setSelection(AddonId::TRADE, 1); - world.CreateTradeGraphs(); } void testExpectedWares() const From 70f5ecf148655d9098cb59852c9930b1af64512f Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sun, 19 Jan 2025 17:59:16 +0100 Subject: [PATCH 02/11] Add test for ship connections --- tests/s25Main/integration/testSeafaring.cpp | 65 +++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/tests/s25Main/integration/testSeafaring.cpp b/tests/s25Main/integration/testSeafaring.cpp index 009d28b439..cd81a56bf1 100644 --- a/tests/s25Main/integration/testSeafaring.cpp +++ b/tests/s25Main/integration/testSeafaring.cpp @@ -22,6 +22,7 @@ #include "gameTypes/GameTypesOutput.h" #include "rttr/test/testHelpers.hpp" #include +#include // LCOV_EXCL_START #define RTTR_ENUM_OUTPUT(EnumName) \ @@ -34,10 +35,21 @@ RTTR_ENUM_OUTPUT(nobShipYard::Mode) // LCOV_EXCL_STOP namespace { + +using SmallSeaWorld = WorldFixture::width, SmallSeaWorldDefault<2>::height>; + std::vector FindRoadPath(const MapPoint fromPt, const MapPoint toPt, const GameWorldBase& world) { return FindPathForRoad(world, fromPt, toPt, false); } +/// Helper to extract only the destinations(harbors) from the connections list +std::set toHarbors(const std::vector& connections) +{ + std::set res; + std::transform(connections.begin(), connections.end(), std::inserter(res, res.begin()), + [](const auto& con) { return con.dest; }); + return res; +} } // namespace BOOST_AUTO_TEST_SUITE(SeafaringTestSuite) @@ -72,6 +84,59 @@ BOOST_FIXTURE_TEST_CASE(HarborPlacing, SeaWorldWithGCExecution<>) BOOST_TEST_REQUIRE(!road.empty()); } +BOOST_FIXTURE_TEST_CASE(ShipConnections, SmallSeaWorld) +{ + unsigned curPlayer = 0; + const auto createHarbor = [this, &curPlayer](HarborId hbId) -> nobHarborBuilding& { + MapPoint hbPos = world.GetHarborPoint(hbId); + auto* harbor = static_cast( + BuildingFactory::CreateBuilding(world, BuildingType::HarborBuilding, hbPos, curPlayer, Nation::Romans)); + BOOST_TEST_REQUIRE(harbor); + return *harbor; + }; + /* Layout of harbors (1 inner + 1 outer sea) + * 1 + * 2 + * 3 4 5 6 + * 7 + * 8 + */ + // I.e.: + constexpr SeaId sea1(1); + constexpr SeaId sea2(2); + BOOST_TEST(world.IsHarborAtSea(HarborId(1), sea1)); + BOOST_TEST(world.IsHarborAtSea(HarborId(3), sea1)); + BOOST_TEST(world.IsHarborAtSea(HarborId(6), sea1)); + BOOST_TEST(world.IsHarborAtSea(HarborId(8), sea1)); + + BOOST_TEST(world.IsHarborAtSea(HarborId(2), sea2)); + BOOST_TEST(world.IsHarborAtSea(HarborId(4), sea2)); + BOOST_TEST(world.IsHarborAtSea(HarborId(5), sea2)); + BOOST_TEST(world.IsHarborAtSea(HarborId(7), sea2)); + + const auto& hb1 = createHarbor(HarborId(1)); + // No other harbor + BOOST_TEST(hb1.GetShipConnections().empty()); + const auto& hb2 = createHarbor(HarborId(2)); + // Different seas + BOOST_TEST(hb1.GetShipConnections().empty()); + BOOST_TEST(hb2.GetShipConnections().empty()); + using harbors_t = std::set; + // 3 and 1 share sea + const auto& hb3 = createHarbor(HarborId(3)); + BOOST_TEST(toHarbors(hb1.GetShipConnections()) == harbors_t{&hb3}); + BOOST_TEST(hb2.GetShipConnections().empty()); + BOOST_TEST(toHarbors(hb3.GetShipConnections()) == harbors_t{&hb1}); + // 2,4,5 share sea + const auto& hb4 = createHarbor(HarborId(4)); + const auto& hb5 = createHarbor(HarborId(5)); + BOOST_TEST(toHarbors(hb1.GetShipConnections()) == harbors_t{&hb3}); + BOOST_TEST(toHarbors(hb2.GetShipConnections()) == (harbors_t{&hb4, &hb5})); + BOOST_TEST(toHarbors(hb3.GetShipConnections()) == harbors_t{&hb1}); + BOOST_TEST(toHarbors(hb4.GetShipConnections()) == (harbors_t{&hb2, &hb5})); + BOOST_TEST(toHarbors(hb5.GetShipConnections()) == (harbors_t{&hb2, &hb4})); +} + BOOST_FIXTURE_TEST_CASE(ShipBuilding, SeaWorldWithGCExecution<>) { addStartResources(); From d2fdcbded19c5317d6c49923c76044e84d78ba75 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sat, 25 Apr 2026 19:01:25 +0200 Subject: [PATCH 03/11] Do not return self in `nobHarbor::GetShipConnections` --- libs/s25main/GamePlayer.cpp | 12 -------- libs/s25main/GamePlayer.h | 2 -- libs/s25main/buildings/nobHarborBuilding.cpp | 32 +++++++++++--------- tests/s25Main/integration/testSeafaring.cpp | 5 +-- 4 files changed, 19 insertions(+), 32 deletions(-) diff --git a/libs/s25main/GamePlayer.cpp b/libs/s25main/GamePlayer.cpp index 2230e3ccce..b655e38c53 100644 --- a/libs/s25main/GamePlayer.cpp +++ b/libs/s25main/GamePlayer.cpp @@ -2058,18 +2058,6 @@ noShip* GamePlayer::GetShipByID(const unsigned ship_id) const return ships[ship_id]; } -void GamePlayer::AddHarborsAtSea(std::vector& harborBuildings, const SeaId seaId) const -{ - for(nobHarborBuilding* harbor : buildings.GetHarbors()) - { - if(helpers::contains(harborBuildings, harbor)) - continue; - - if(world.IsHarborAtSea(harbor->GetHarborPosID(), seaId)) - harborBuildings.push_back(harbor); - } -} - /// Gibt die Anzahl der Schiffe, die einen bestimmten Hafen ansteuern, zurück unsigned GamePlayer::GetShipsToHarbor(const nobHarborBuilding& hb) const { diff --git a/libs/s25main/GamePlayer.h b/libs/s25main/GamePlayer.h index 7c9e59f524..ada0ba552a 100644 --- a/libs/s25main/GamePlayer.h +++ b/libs/s25main/GamePlayer.h @@ -253,8 +253,6 @@ class GamePlayer : public GamePlayerInfo unsigned GetNumShips() const { return ships.size(); } /// Gibt liste der Schiffe zurück const std::vector& GetShips() const { return ships; } - /// Add all unique harbor buildings at the given sea to the given vector - void AddHarborsAtSea(std::vector& harborBuildings, SeaId seaId) const; /// Gibt die Anzahl der Schiffe, die einen bestimmten Hafen ansteuern, zurück unsigned GetShipsToHarbor(const nobHarborBuilding& hb) const; /// Sucht einen Hafen in der Nähe, wo dieses Schiff seine Waren abladen kann diff --git a/libs/s25main/buildings/nobHarborBuilding.cpp b/libs/s25main/buildings/nobHarborBuilding.cpp index ccbd238984..bac29022b2 100644 --- a/libs/s25main/buildings/nobHarborBuilding.cpp +++ b/libs/s25main/buildings/nobHarborBuilding.cpp @@ -835,21 +835,25 @@ std::vector nobHarborBuilding::GetShipConnect if(world->GetGOT(pos) != GO_Type::NobHarborbuilding) return connections; - std::vector harborBuildings; - for(SeaId seaId : seaIds) - { - if(seaId) - world->GetPlayer(player).AddHarborsAtSea(harborBuildings, seaId); - } - - for(auto* harbor_building : harborBuildings) + const auto sharesSeaWith = [&seaIds = this->seaIds, world = this->world](const nobHarborBuilding& harbor) { + for(SeaId seaId : seaIds) + { + if(seaId && world->IsHarborAtSea(harbor.GetHarborPosID(), seaId)) + return true; + } + return false; + }; + for(auto* harbor_building : world->GetPlayer(player).GetBuildingRegister().GetHarbors()) { - ShipConnection sc; - sc.dest = harbor_building; - // 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); + if(harbor_building != this && sharesSeaWith(*harbor_building)) + { + ShipConnection sc; + sc.dest = harbor_building; + // 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); + } } return connections; } diff --git a/tests/s25Main/integration/testSeafaring.cpp b/tests/s25Main/integration/testSeafaring.cpp index cd81a56bf1..8e14d6ad8f 100644 --- a/tests/s25Main/integration/testSeafaring.cpp +++ b/tests/s25Main/integration/testSeafaring.cpp @@ -67,17 +67,14 @@ BOOST_FIXTURE_TEST_CASE(HarborPlacing, SeaWorldWithGCExecution<>) auto* harbor = dynamic_cast( BuildingFactory::CreateBuilding(world, BuildingType::HarborBuilding, hbPos, curPlayer, Nation::Romans)); BOOST_TEST_REQUIRE(harbor); + BOOST_TEST_REQUIRE(world.IsHarborAtSea(hbId, seaId)); BOOST_TEST_REQUIRE(buildings.GetHarbors().size() == 1u); //-V807 BOOST_TEST_REQUIRE(buildings.GetHarbors().front() == harbor); // A harbor is also a storehouse BOOST_TEST_REQUIRE(buildings.GetStorehouses().size() == 2u); BOOST_TEST_REQUIRE(buildings.GetHarbors().back() == harbor); - std::vector harbors; BOOST_TEST_REQUIRE(world.GetNode(MapPoint(0, 0)).seaId == seaId); BOOST_TEST_REQUIRE(world.GetSeaId(hbId, Direction::NorthEast) == seaId); - player.AddHarborsAtSea(harbors, seaId); - BOOST_TEST_REQUIRE(harbors.size() == 1u); - BOOST_TEST_REQUIRE(harbors.front() == harbor); const std::vector road = FindRoadPath(world.GetNeighbour(hqPos, Direction::SouthEast), world.GetNeighbour(hbPos, Direction::SouthEast), world); From 9a29a2fe576d8432dc13159b11c0be71cc242ed3 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sat, 25 Apr 2026 19:08:07 +0200 Subject: [PATCH 04/11] Remove superflous docstrings from GameClient.cpp --- libs/s25main/GamePlayer.cpp | 30 ------------------------------ 1 file changed, 30 deletions(-) diff --git a/libs/s25main/GamePlayer.cpp b/libs/s25main/GamePlayer.cpp index b655e38c53..c444ebc90e 100644 --- a/libs/s25main/GamePlayer.cpp +++ b/libs/s25main/GamePlayer.cpp @@ -1332,15 +1332,12 @@ void GamePlayer::RegulateAllTroops() milBld->RegulateTroops(); } -/// Prüft von allen Militärgebäuden die Fahnen neu void GamePlayer::RecalcMilitaryFlags() { for(nobMilitary* milBld : buildings.GetMilitaryBuildings()) milBld->LookForEnemyBuildings(nullptr); } -/// Sucht für Soldaten ein neues Militärgebäude, als Argument wird Referenz auf die -/// entsprechende Soldatenanzahl im Lagerhaus verlangt void GamePlayer::NewSoldiersAvailable(const unsigned& soldier_count) { RTTR_Assert(soldier_count > 0); @@ -1436,7 +1433,6 @@ void GamePlayer::ChangeMilitarySettings(const MilitarySettings& military_setting RefreshDefenderList(); } -/// Setzt neue Werkzeugeinstellungen void GamePlayer::ChangeToolsSettings(const ToolSettings& tools_settings, const helpers::EnumArray& orderChanges) { @@ -1459,7 +1455,6 @@ void GamePlayer::ChangeToolsSettings(const ToolSettings& tools_settings, } } -/// Setzt neue Verteilungseinstellungen void GamePlayer::ChangeDistribution(const Distributions& distribution_settings) { unsigned idx = 0; @@ -1471,7 +1466,6 @@ void GamePlayer::ChangeDistribution(const Distributions& distribution_settings) RecalcDistribution(); } -/// Setzt neue Baureihenfolge-Einstellungen void GamePlayer::ChangeBuildOrder(bool useCustomBuildOrder, const BuildOrders& order_data) { this->useCustomBuildOrder_ = useCustomBuildOrder; @@ -1570,7 +1564,6 @@ void GamePlayer::IncreaseMerchandiseStatistic(GoodType type) } } -/// Calculates current statistics void GamePlayer::CalcStatistics() { // Waren aus der Inventur zählen @@ -1710,7 +1703,6 @@ void GamePlayer::AcceptPact(const unsigned id, const PactType pt, const unsigned } } -/// Bündnis (real, d.h. spielentscheidend) abschließen void GamePlayer::MakePact(const PactType pt, const unsigned char other_player, const unsigned duration) { pacts[other_player][pt].accepted = true; @@ -1722,7 +1714,6 @@ void GamePlayer::MakePact(const PactType pt, const unsigned char other_player, c std::make_unique(world.GetEvMgr().GetCurrentGF(), pt, world.GetPlayer(other_player), true)); } -/// Zeigt an, ob ein Pakt besteht PactState GamePlayer::GetPactState(const PactType pt, const unsigned char other_player) const { // Prüfen, ob Bündnis in Kraft ist @@ -1739,7 +1730,6 @@ PactState GamePlayer::GetPactState(const PactType pt, const unsigned char other_ return PactState::None; } -/// all allied players get a letter with the location void GamePlayer::NotifyAlliesOfLocation(const MapPoint pt) { for(unsigned i = 0; i < world.GetNumPlayers(); ++i) @@ -1751,7 +1741,6 @@ void GamePlayer::NotifyAlliesOfLocation(const MapPoint pt) } } -/// Gibt die verbleibende Dauer zurück, die ein Bündnis noch laufen wird (DURATION_INFINITE = für immer) unsigned GamePlayer::GetRemainingPactTime(const PactType pt, const unsigned char other_player) const { if(pacts[other_player][pt].duration) @@ -1769,8 +1758,6 @@ unsigned GamePlayer::GetRemainingPactTime(const PactType pt, const unsigned char return 0; } -/// Gibt Einverständnis, dass dieser Spieler den Pakt auflösen will -/// Falls dieser Spieler einen Bündnisvorschlag gemacht hat, wird dieser dagegen zurückgenommen void GamePlayer::CancelPact(const PactType pt, const unsigned char otherPlayerIdx) { // Don't try to cancel pact with self @@ -1888,11 +1875,9 @@ void GamePlayer::DecreaseInventoryWare(const GoodType ware, const unsigned count global_inventory.Remove(ConvertShields(ware), count); } -/// Registriert ein Schiff beim Einwohnermeldeamt void GamePlayer::RegisterShip(noShip& ship) { ships.push_back(&ship); - // Evtl bekommt das Schiffchen gleich was zu tun? GetJobForShip(ship); } @@ -1909,7 +1894,6 @@ struct ShipForHarbor } }; -/// Schiff für Hafen bestellen bool GamePlayer::OrderShip(nobHarborBuilding& hb) { std::vector sfh; @@ -1981,7 +1965,6 @@ bool GamePlayer::OrderShip(nobHarborBuilding& hb) return false; } -/// Meldet das Schiff wieder ab void GamePlayer::RemoveShip(noShip* ship) { auto it = helpers::find(ships, ship); @@ -1989,10 +1972,8 @@ void GamePlayer::RemoveShip(noShip* ship) ships.erase(it); } -/// Versucht, für ein untätiges Schiff eine Arbeit zu suchen void GamePlayer::GetJobForShip(noShip& ship) { - // Evtl. steht irgendwo eine Expedition an und das Schiff kann diese übernehmen nobHarborBuilding* best = nullptr; int best_points = 0; std::vector best_route; @@ -2040,7 +2021,6 @@ void GamePlayer::GetJobForShip(noShip& ship) // Einen Hafen gefunden? if(best) - // Dann bekommt das gleich der Hafen ship.GoToHarbor(*best, best_route); } @@ -2049,7 +2029,6 @@ unsigned GamePlayer::GetShipID(const noShip& ship) const return static_cast(helpers::indexOf(ships, &ship)); } -/// Gibt ein Schiff anhand der ID zurück bzw. nullptr, wenn keines mit der ID existiert noShip* GamePlayer::GetShipByID(const unsigned ship_id) const { if(ship_id >= ships.size()) @@ -2058,7 +2037,6 @@ noShip* GamePlayer::GetShipByID(const unsigned ship_id) const return ships[ship_id]; } -/// Gibt die Anzahl der Schiffe, die einen bestimmten Hafen ansteuern, zurück unsigned GamePlayer::GetShipsToHarbor(const nobHarborBuilding& hb) const { unsigned count = 0; @@ -2071,8 +2049,6 @@ unsigned GamePlayer::GetShipsToHarbor(const nobHarborBuilding& hb) const return count; } -/// Sucht einen Hafen in der Nähe, wo dieses Schiff seine Waren abladen kann -/// gibt true zurück, falls erfolgreich bool GamePlayer::FindHarborForUnloading(noShip* ship, const MapPoint start, HarborId* goalHarborId, std::vector* route, nobHarborBuilding* exception) { @@ -2157,7 +2133,6 @@ void GamePlayer::TestForEmergencyProgramm() } } -/// Testet die Bündnisse, ob sie nicht schon abgelaufen sind void GamePlayer::TestPacts() { for(unsigned i = 0; i < world.GetNumPlayers(); ++i) @@ -2212,9 +2187,6 @@ bool GamePlayer::CanBuildCatapult() const return bc.buildings[BuildingType::Catapult] + bc.buildingSites[BuildingType::Catapult] < max; } -/// A ship has discovered new hostile territory --> determines if this is new -/// i.e. there is a sufficient distance to older locations -/// Returns true if yes and false if not bool GamePlayer::ShipDiscoveredHostileTerritory(const MapPoint location) { // Prüfen, ob Abstand zu bisherigen Punkten nicht zu klein @@ -2230,7 +2202,6 @@ bool GamePlayer::ShipDiscoveredHostileTerritory(const MapPoint location) return true; } -/// For debug only bool GamePlayer::IsDependentFigure(const noFigure& fig) { for(const nobBaseWarehouse* wh : buildings.GetStorehouses()) @@ -2281,7 +2252,6 @@ struct WarehouseDistanceComparator } }; -/// Send wares to warehouse wh void GamePlayer::Trade(nobBaseWarehouse* goalWh, const boost_variant2& what, unsigned count) const { if(!world.GetGGS().isEnabled(AddonId::TRADE)) From b8b9a7029a10ec3b397330866522798f47aba65a Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sat, 25 Apr 2026 19:39:38 +0200 Subject: [PATCH 05/11] Remove hardcoded harbor id in test --- tests/s25Main/integration/testSeafaring.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/s25Main/integration/testSeafaring.cpp b/tests/s25Main/integration/testSeafaring.cpp index 8e14d6ad8f..393c01c73b 100644 --- a/tests/s25Main/integration/testSeafaring.cpp +++ b/tests/s25Main/integration/testSeafaring.cpp @@ -531,7 +531,7 @@ BOOST_FIXTURE_TEST_CASE(LongDistanceTravel, ShipReadyFixtureBig) // Go to opposite one const HarborId targetHbId(7); // Make sure that the other harbor is far away - BOOST_TEST_REQUIRE(world.CalcHarborDistance(HarborId(2), targetHbId) > 600u); + BOOST_TEST_REQUIRE(world.CalcHarborDistance(harbor.GetHarborPosID(), targetHbId) > 600u); // Add some scouts harbor.AddToInventory(PeopleCounts::make(Job::Scout, 20), true); // We want the ship to only scout unexplored harbors, so set all but one to visible From cbf3f05af43e179560581d265237fb991954fabd Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sun, 26 Apr 2026 20:38:10 +0200 Subject: [PATCH 06/11] Add reproducer for crash due to too far distance to harbor --- libs/s25main/world/World.cpp | 2 +- tests/s25Main/integration/testSeafaring.cpp | 98 +++++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/libs/s25main/world/World.cpp b/libs/s25main/world/World.cpp index d33a37ad54..5399c40ca2 100644 --- a/libs/s25main/world/World.cpp +++ b/libs/s25main/world/World.cpp @@ -425,7 +425,7 @@ unsigned World::CalcHarborDistance(HarborId haborId1, HarborId harborId2) const } } - return 0xffffffff; + return std::numeric_limits::max(); } SeaId World::GetSeaFromCoastalPoint(const MapPoint pt) const diff --git a/tests/s25Main/integration/testSeafaring.cpp b/tests/s25Main/integration/testSeafaring.cpp index 393c01c73b..f64d86d8b0 100644 --- a/tests/s25Main/integration/testSeafaring.cpp +++ b/tests/s25Main/integration/testSeafaring.cpp @@ -712,6 +712,104 @@ BOOST_FIXTURE_TEST_CASE(HarborDestroyed, ShipAndHarborsReadyFixture<1>) BOOST_TEST_REQUIRE(ship.GetTargetHarbor() == hb1Id); } +// Dimensions must be large enough so path tolerances get too small +using ShipReadyFixtureLarger = ShipReadyFixture<1, 2, 90, 260>; +BOOST_FIXTURE_TEST_CASE(IncreasedPathLengthWhenHarborDestroyed, ShipReadyFixtureLarger) +{ + // Edge case: Path to harbor longer than any direct connection + /* + * 2 + * W + * WLW + * WLW + * WWLWW + * 4WLLLLLW5 + * WWLWW + * WWLWW + * 7 + */ + // Shortest path 4->5 is over south, ship might be between 2 & 4 => still finds path to 5 + constexpr HarborId hbIdTop(2); + constexpr HarborId hbIdLeft(4); + constexpr HarborId hbIdRight(5); + constexpr HarborId hbIdBottom(7); + const MapPoint hbTopPos = world.GetHarborPoint(hbIdTop); + const MapPoint hbLeftPos = world.GetHarborPoint(hbIdLeft); + const MapPoint hbRightPos = world.GetHarborPoint(hbIdRight); + const MapPoint hbBottomPos = world.GetHarborPoint(hbIdBottom); + // Sanity check expected geometry, ensured by fixture + BOOST_TEST_REQUIRE(hbLeftPos.x < hbRightPos.x); + BOOST_TEST_REQUIRE(hbLeftPos.y == hbRightPos.y); + BOOST_TEST_REQUIRE(hbTopPos.y < hbLeftPos.y); + BOOST_TEST_REQUIRE(hbLeftPos.y < hbBottomPos.y); + const unsigned yDiff = hbLeftPos.y - hbTopPos.y; + BOOST_TEST_REQUIRE(yDiff >= 4u * 3u); // So divisions below yield noticeable difference + const auto t = GetLandTerrain(world.GetDescription(), ETerrain::Buildable); + for(const auto y : helpers::range(hbLeftPos.y - yDiff * 3u / 4u, hbLeftPos.y + yDiff / 2u)) + { + auto& node = world.GetNodeWriteable(MapPoint(hbTopPos.x, y)); + node.t1 = node.t2 = t; + } + constexpr auto horStripHeight = 2u; + // Horizontal land strip to avoid short cuts + for(const auto y : helpers::range(hbLeftPos.y - horStripHeight, hbLeftPos.y + horStripHeight)) + { + for(const auto x : helpers::range(hbLeftPos.x + 4, hbRightPos.x - 4)) + { + auto& node = world.GetNodeWriteable(MapPoint(x, y)); + node.t1 = node.t2 = t; + } + } + MapLoader::InitSeasAndHarbors(world); + // Harbors unchanged + BOOST_TEST_REQUIRE(world.GetHarborPointID(hbTopPos) == hbIdTop); + BOOST_TEST_REQUIRE(world.GetHarborPointID(hbLeftPos) == hbIdLeft); + BOOST_TEST_REQUIRE(world.GetHarborPointID(hbRightPos) == hbIdRight); + BOOST_TEST_REQUIRE(world.GetHarborPointID(hbBottomPos) == hbIdBottom); + + const GamePlayer& player = world.GetPlayer(curPlayer); + const noShip& ship = ensureNonNull(player.GetShipByID(0)); + const SeaId seaId = ship.GetSeaID(); + + // Start shipment to left harbor + nobHarborBuilding& topHarbor = *player.GetBuildingRegister().GetHarbors().front(); + BuildingFactory::CreateBuilding(world, BuildingType::HarborBuilding, hbLeftPos, curPlayer, Nation::Romans); + + topHarbor.AddToInventory(GoodCounts::make(GoodType::Wood, 10), true); + SetInventorySetting(hbLeftPos, GoodType::Wood, EInventorySetting::Collect); + + RTTR_EXEC_TILL(90, ship.IsLoading()); + BOOST_TEST_REQUIRE(ship.GetPos() == world.GetCoastalPoint(hbIdTop, seaId)); + RTTR_EXEC_TILL(400, ship.IsMoving()); + BOOST_TEST_REQUIRE(ship.GetTargetHarbor() == hbIdLeft); + const auto leftCoastPt = world.GetCoastalPoint(hbIdLeft, seaId); + + // Wait till ship is close to left harbor + constexpr auto extraDist = 16u; // Choosen to be larger than a magic number used as tolerance + RTTR_EXEC_TILL(2000, world.CalcDistance(ship.GetPos(), leftCoastPt) < extraDist * 2); + + // Destroy home so it cannot go back, then target harbor + destroyBldAndFire(world, hbTopPos); + destroyBldAndFire(world, hbLeftPos); + RTTR_EXEC_TILL(10, ship.IsLost()); + BOOST_TEST_REQUIRE(!ship.GetHomeHarbor()); + BOOST_TEST_REQUIRE(!ship.GetTargetHarbor()); + + // Building harbor at right makes ship go there even though distance is (too) long + unsigned len; + BOOST_TEST_REQUIRE( + world.FindShipPath(ship.GetPos(), world.GetCoastalPoint(hbIdRight, seaId), unsigned(-1), nullptr, &len)); + const auto hbDistance = world.CalcHarborDistance(hbIdLeft, hbIdRight); + // Reproducer requires the path to be longer than the direct distance and other connections + BOOST_TEST_REQUIRE(len > hbDistance + extraDist); + BOOST_TEST_REQUIRE(hbDistance > world.CalcHarborDistance(hbIdRight, hbIdTop)); + BOOST_TEST_REQUIRE(hbDistance > world.CalcHarborDistance(hbIdRight, hbIdBottom)); + BuildingFactory::CreateBuilding(world, BuildingType::HarborBuilding, hbRightPos, curPlayer, Nation::Romans); + BOOST_TEST_REQUIRE(ship.IsMoving()); + BOOST_TEST_REQUIRE(ship.GetHomeHarbor() == hbIdRight); + BOOST_TEST_REQUIRE(ship.GetTargetHarbor() == hbIdRight); +} + struct MockWare : Ware { static bool destroyed; From d89de7a9b2cec4f3ded8f483a095e33216690017 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sat, 2 May 2026 12:11:51 +0200 Subject: [PATCH 07/11] Use PlayerIdx type in TradePathCache.cpp --- libs/s25main/TradePathCache.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libs/s25main/TradePathCache.cpp b/libs/s25main/TradePathCache.cpp index bfa05d329c..d8cb75f509 100644 --- a/libs/s25main/TradePathCache.cpp +++ b/libs/s25main/TradePathCache.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2005 - 2021 Settlers Freaks (sf-team at siedler25.org) +// Copyright (C) 2005 - 2026 Settlers Freaks (sf-team at siedler25.org) // // SPDX-License-Identifier: GPL-2.0-or-later @@ -8,7 +8,7 @@ #include "world/GameWorld.h" #include "gameData/GameConsts.h" -bool TradePathCache::pathExists(const MapPoint start, const MapPoint goal, const unsigned char player) +bool TradePathCache::pathExists(const MapPoint start, const MapPoint goal, const PlayerIdx player) { RTTR_Assert(start != goal); @@ -57,7 +57,7 @@ int TradePathCache::findEntry(const MapPoint start, const MapPoint goal, const P return -1; } -void TradePathCache::addEntry(TradePath path, const unsigned char player) +void TradePathCache::addEntry(TradePath path, const PlayerIdx player) { Entry entry{player, world.GetEvMgr().GetCurrentGF(), std::move(path)}; From 6b6f529f19f3b263b411a9625c65de7deae252bb Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sun, 3 May 2026 18:38:59 +0200 Subject: [PATCH 08/11] Fix handling of harbors connected by multiple seas It could be possible that 2 harbors are at the same 2 seas. The current harbor connections do not honor this and only store a single distance which would hence be wrong for one of those 2. Store the seaId for each connection. --- libs/s25main/Pathfinding.cpp | 2 +- libs/s25main/SerializedGameData.cpp | 3 +- libs/s25main/buildings/nobHarborBuilding.cpp | 20 ++++--- libs/s25main/gameTypes/HarborPos.h | 10 +++- libs/s25main/nodeObjs/noShip.cpp | 2 +- libs/s25main/world/GameWorld.h | 2 +- libs/s25main/world/MapLoader.cpp | 30 ++-------- libs/s25main/world/MapSerializer.cpp | 60 +++++++++++++++++++- libs/s25main/world/World.cpp | 20 ++++++- libs/s25main/world/World.h | 7 ++- tests/s25Main/integration/testSeafaring.cpp | 8 +-- tests/s25Main/integration/testWorld.cpp | 2 +- 12 files changed, 117 insertions(+), 49 deletions(-) diff --git a/libs/s25main/Pathfinding.cpp b/libs/s25main/Pathfinding.cpp index bf4eb96cb2..3f82aef330 100644 --- a/libs/s25main/Pathfinding.cpp +++ b/libs/s25main/Pathfinding.cpp @@ -90,7 +90,7 @@ bool GameWorldBase::FindShipPath(const MapPoint start, const MapPoint dest, unsi /// Prüft, ob eine Schiffsroute noch Gültigkeit hat bool GameWorld::CheckShipRoute(const MapPoint start, const std::vector& route, const unsigned pos, - MapPoint* dest) + MapPoint* dest) const { return GetFreePathFinder().CheckRoute(start, route, pos, PathConditionShip(*this), dest); } diff --git a/libs/s25main/SerializedGameData.cpp b/libs/s25main/SerializedGameData.cpp index d90258cd09..1b0997c910 100644 --- a/libs/s25main/SerializedGameData.cpp +++ b/libs/s25main/SerializedGameData.cpp @@ -108,7 +108,8 @@ /// 12: leatheraddon added, three new building types and three new goods /// 13: SeaId & HarborId: World::harborData w/o dummy entry at 0 /// 14: Remove "age" field in nobBaseMilitary -static const unsigned currentGameDataVersion = 14; +/// 15: Add sea to HarborPos::Neighbor +static const unsigned currentGameDataVersion = 15; // clang-format on std::unique_ptr SerializedGameData::Create_GameObject(const GO_Type got, const unsigned obj_id) diff --git a/libs/s25main/buildings/nobHarborBuilding.cpp b/libs/s25main/buildings/nobHarborBuilding.cpp index bac29022b2..e157270971 100644 --- a/libs/s25main/buildings/nobHarborBuilding.cpp +++ b/libs/s25main/buildings/nobHarborBuilding.cpp @@ -835,23 +835,27 @@ std::vector nobHarborBuilding::GetShipConnect if(world->GetGOT(pos) != GO_Type::NobHarborbuilding) return connections; - const auto sharesSeaWith = [&seaIds = this->seaIds, world = this->world](const nobHarborBuilding& harbor) { + const auto getSharedSea = [&seaIds = this->seaIds, world = this->world](const nobHarborBuilding& harbor) { for(SeaId seaId : seaIds) { if(seaId && world->IsHarborAtSea(harbor.GetHarborPosID(), seaId)) - return true; + return seaId; } - return false; + return SeaId::invalidValue(); }; for(auto* harbor_building : world->GetPlayer(player).GetBuildingRegister().GetHarbors()) { - if(harbor_building != this && sharesSeaWith(*harbor_building)) + if(harbor_building == this) + continue; + SeaId seaId = getSharedSea(*harbor_building); + if(seaId) { ShipConnection sc; sc.dest = harbor_building; // 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; + sc.way_costs = + 2 * world->GetHarborDistance(GetHarborPosID(), harbor_building->GetHarborPosID(), seaId) + 10; connections.push_back(sc); } } @@ -1166,10 +1170,10 @@ nobHarborBuilding::GetAttackerBuildingsForSeaAttack(const std::vector& continue; // Entfernung zwischen Hafen und möglichen Zielhafenpunkt ausrechnen - unsigned min_distance = 0xffffffff; + auto min_distance = std::numeric_limits::max(); for(HarborId defender_harbor : defender_harbors) { - min_distance = std::min(min_distance, world->CalcHarborDistance(GetHarborPosID(), defender_harbor)); + min_distance = std::min(min_distance, world->GetMinHarborDistance(GetHarborPosID(), defender_harbor)); } // Gebäude suchen, vielleicht schon vorhanden? @@ -1202,7 +1206,7 @@ void nobHarborBuilding::AddSeaAttacker(std::unique_ptr attacker) world->GetHarborPointsAroundMilitaryBuilding(attacker->GetAttackedGoal()->GetPos()); for(HarborId harbor_point : harbor_points) { - unsigned tmp_distance = world->CalcHarborDistance(this->GetHarborPosID(), harbor_point); + unsigned tmp_distance = world->GetMinHarborDistance(this->GetHarborPosID(), harbor_point); if(tmp_distance < best_distance) { best_distance = tmp_distance; diff --git a/libs/s25main/gameTypes/HarborPos.h b/libs/s25main/gameTypes/HarborPos.h index bff3f4347a..698b86b566 100644 --- a/libs/s25main/gameTypes/HarborPos.h +++ b/libs/s25main/gameTypes/HarborPos.h @@ -15,13 +15,19 @@ struct HarborPos struct Neighbor { HarborId id; + SeaId sea; unsigned distance; - Neighbor(HarborId id, unsigned distance) noexcept : id(id), distance(distance) {} + Neighbor(HarborId id, SeaId sea, unsigned distance) noexcept : id(id), sea(sea), distance(distance) {} bool operator<(const Neighbor& two) const noexcept { - return (distance < two.distance) || (distance == two.distance && id.value() < two.id.value()); + if(distance == two.distance) + { + if(sea != two.sea) + return sea.value() < two.sea.value(); + } + return id.value() < two.id.value(); } }; diff --git a/libs/s25main/nodeObjs/noShip.cpp b/libs/s25main/nodeObjs/noShip.cpp index 2c71f1719d..ffd683f9aa 100644 --- a/libs/s25main/nodeObjs/noShip.cpp +++ b/libs/s25main/nodeObjs/noShip.cpp @@ -973,7 +973,7 @@ void noShip::StartDrivingToHarborPlace() if(homeHarbor && pos == world->GetCoastalPoint(homeHarbor, seaId_)) { // Use the maximum distance between the harbors plus 6 fields - unsigned maxDistance = world->CalcHarborDistance(homeHarbor, goalHarbor) + 6; + unsigned maxDistance = world->GetMinHarborDistance(homeHarbor, goalHarbor) + 6; routeFound = world->FindShipPath(pos, coastalPos, maxDistance, &route_, nullptr); } else routeFound = world->FindShipPathToHarbor(pos, goalHarbor, seaId_, &route_, nullptr); diff --git a/libs/s25main/world/GameWorld.h b/libs/s25main/world/GameWorld.h index 38e6b2804d..11eead0cd7 100644 --- a/libs/s25main/world/GameWorld.h +++ b/libs/s25main/world/GameWorld.h @@ -94,7 +94,7 @@ class GameWorld : public GameWorldBase unsigned* length = nullptr, MapPoint* firstPt = nullptr, unsigned max = std::numeric_limits::max()); /// Prüft, ob eine Schiffsroute noch Gültigkeit hat - bool CheckShipRoute(MapPoint start, const std::vector& route, unsigned pos, MapPoint* dest); + bool CheckShipRoute(MapPoint start, const std::vector& route, unsigned pos, MapPoint* dest) const; /// Find a route for trade caravanes helpers::OptionalEnum FindTradePath(MapPoint start, MapPoint dest, unsigned char player, unsigned max_route = 0xffffffff, bool random_route = false, diff --git a/libs/s25main/world/MapLoader.cpp b/libs/s25main/world/MapLoader.cpp index 47b0683766..b1c8572ea3 100644 --- a/libs/s25main/world/MapLoader.cpp +++ b/libs/s25main/world/MapLoader.cpp @@ -586,24 +586,6 @@ bool MapLoader::InitSeasAndHarbors(World& world, const std::vector& ad // Calculate the neighbors and distances CalcHarborPosNeighbors(world); - - // Validate - for(const auto startHbId : helpers::idRange(world.harborData.size())) - { - const HarborPos& startHbPos = world.harborData[startHbId]; - for(const std::vector& neighbors : startHbPos.neighbors) - { - for(const HarborPos::Neighbor& neighbor : neighbors) - { - if(world.CalcHarborDistance(neighbor.id, startHbId) != neighbor.distance) - { - LOG.write("Bug: Harbor distance mismatch for harbors %1%->%2%: %3% != %4%\n") % startHbId - % neighbor.id % world.CalcHarborDistance(neighbor.id, startHbId) % neighbor.distance; - return false; - } - } - } - } return true; } @@ -682,12 +664,12 @@ void MapLoader::CalcHarborPosNeighbors(World& world) for(const MapPoint& ownCoastPt : ownCoastalPoints) { // Special case: Get all harbors that share the coast point with us - SeaId seaId = world.GetSeaFromCoastalPoint(ownCoastPt); + const SeaId seaId = world.GetSeaFromCoastalPoint(ownCoastPt); auto const coastToHbs = coastToHarborPerSea[seaId].equal_range(world.GetIdx(ownCoastPt)); for(auto it = coastToHbs.first; it != coastToHbs.second; ++it) { ShipDirection shipDir = world.GetShipDir(ownCoastPt, ownCoastPt); - world.harborData[startHbId].neighbors[shipDir].push_back(HarborPos::Neighbor(it->second, 0)); + world.harborData[startHbId].neighbors[shipDir].push_back(HarborPos::Neighbor(it->second, seaId, 0)); hbFound[it->second] = true; } todo_list.push(CalcHarborPosNeighborsNode(ownCoastPt, 0)); @@ -713,8 +695,9 @@ void MapLoader::CalcHarborPosNeighbors(World& world) if(ptValue > 0) // found harbor(s) { - ShipDirection shipDir = world.GetShipDir(world.harborData[startHbId].pos, curPt); - SeaId seaId = world.GetSeaFromCoastalPoint(curPt); + const ShipDirection shipDir = world.GetShipDir(world.harborData[startHbId].pos, curPt); + const SeaId seaId = world.GetSeaFromCoastalPoint(curPt); + RTTR_Assert(seaId); auto const coastToHbs = coastToHarborPerSea[seaId].equal_range(idx); for(auto it = coastToHbs.first; it != coastToHbs.second; ++it) { @@ -724,11 +707,10 @@ void MapLoader::CalcHarborPosNeighbors(World& world) hbFound[otherHbId] = true; world.harborData[startHbId].neighbors[shipDir].push_back( - HarborPos::Neighbor(otherHbId, curNode.distance + 1)); + HarborPos::Neighbor(otherHbId, seaId, curNode.distance + 1)); // Make this the only coastal point of this harbor for this sea HarborPos& otherHb = world.harborData[otherHbId]; - RTTR_Assert(seaId); for(const auto hbDir : helpers::EnumRange{}) { if(otherHb.seaIds[hbDir] == seaId && world.GetNeighbour(otherHb.pos, hbDir) != curPt) diff --git a/libs/s25main/world/MapSerializer.cpp b/libs/s25main/world/MapSerializer.cpp index 78e84b5cbd..fa6847fc63 100644 --- a/libs/s25main/world/MapSerializer.cpp +++ b/libs/s25main/world/MapSerializer.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2005 - 2021 Settlers Freaks (sf-team at siedler25.org) +// Copyright (C) 2005 - 2026 Settlers Freaks (sf-team at siedler25.org) // // SPDX-License-Identifier: GPL-2.0-or-later @@ -7,10 +7,12 @@ #include "Game.h" #include "SerializedGameData.h" #include "buildings/noBuildingSite.h" +#include "helpers/IdRange.h" #include "helpers/Range.h" #include "lua/GameDataLoader.h" #include "world/GameWorldBase.h" #include "s25util/warningSuppression.h" +#include #include void MapSerializer::Serialize(const GameWorldBase& world, SerializedGameData& sgd) @@ -49,6 +51,7 @@ void MapSerializer::Serialize(const GameWorldBase& world, SerializedGameData& sg for(const auto& c : curNeighbors) { sgd.PushUnsignedInt(c.id.value()); + sgd.PushUnsignedInt(c.sea.value()); sgd.PushUnsignedInt(c.distance); } } @@ -147,8 +150,11 @@ void MapSerializer::Deserialize(GameWorldBase& world, SerializedGameData& sgd, G { RTTR_UNUSED(j); const auto id = HarborId(sgd.PopUnsignedInt()); + SeaId sea; + if(sgd.GetGameDataVersion() >= 15) + sea = SeaId(sgd.PopUnsignedInt()); const auto distance = sgd.PopUnsignedInt(); - neighbor.emplace_back(id, distance); + neighbor.emplace_back(id, sea, distance); } } } @@ -158,6 +164,56 @@ void MapSerializer::Deserialize(GameWorldBase& world, SerializedGameData& sgd, G if(!world.harborData.front().pos.isValid()) world.harborData.erase(world.harborData.begin()); } + if(sgd.GetGameDataVersion() < 15) + { + const auto getSeas = [&world](const HarborId& hbId) { + boost::container::static_vector> seas; + for(const auto sea : world.harborData[hbId].seaIds) + { + if(sea) + seas.push_back(sea); + } + return seas; + }; + // Determine seas for neighbors + for(const auto startHbId : helpers::idRange(world.GetNumHarborPoints())) + { + const auto mySeas = getSeas(startHbId); + for(auto& neighborsPerDir : world.harborData[startHbId].neighbors) + { + for(auto& neighbor : neighborsPerDir) + { + // Easy case: Either harbor is only at a single sea, so that must be the one + if(mySeas.size() == 1u) + { + neighbor.sea = mySeas.front(); + continue; + } + const auto otherSeas = getSeas(neighbor.id); + if(otherSeas.size() == 1u) + { + neighbor.sea = otherSeas.front(); + continue; + } + // Find the sea where this distance matches + for(const auto sea : mySeas) + { + if(!helpers::contains(otherSeas, sea)) + continue; + unsigned len; + if(world.FindShipPath(world.GetCoastalPoint(startHbId, sea), + world.GetCoastalPoint(neighbor.id, sea), neighbor.distance, nullptr, &len) + && len == neighbor.distance) + { + neighbor.sea = sea; + break; + } + } + RTTR_Assert(neighbor.sea); + } + } + } + } sgd.PopObjectContainer(world.harbor_building_sites_from_sea, GO_Type::Buildingsite); diff --git a/libs/s25main/world/World.cpp b/libs/s25main/world/World.cpp index 5399c40ca2..f5af3e96ce 100644 --- a/libs/s25main/world/World.cpp +++ b/libs/s25main/world/World.cpp @@ -412,19 +412,35 @@ const std::vector& World::GetHarborNeighbors(const HarborId return harborData[harborId].neighbors[dir]; } -unsigned World::CalcHarborDistance(HarborId haborId1, HarborId harborId2) const +unsigned World::GetMinHarborDistance(HarborId haborId1, HarborId harborId2) const { if(haborId1 == harborId2) // special case: distance to self return 0; + unsigned minDistance = std::numeric_limits::max(); for(const auto dir : helpers::EnumRange{}) { for(const HarborPos::Neighbor& n : harborData[haborId1].neighbors[dir]) { if(n.id == harborId2) - return n.distance; + minDistance = std::min(minDistance, n.distance); } } + return minDistance; +} +unsigned World::GetHarborDistance(HarborId haborId1, HarborId harborId2, SeaId sea) const +{ + if(haborId1 == harborId2) // special case: distance to self + return 0; + for(const auto dir : helpers::EnumRange{}) + { + for(const HarborPos::Neighbor& n : harborData[haborId1].neighbors[dir]) + { + if(n.id == harborId2 && n.sea == sea) + return n.distance; + } + } + RTTR_Assert(!"Should only query actual neighbors"); return std::numeric_limits::max(); } diff --git a/libs/s25main/world/World.h b/libs/s25main/world/World.h index 6d5041eda8..1fc4532338 100644 --- a/libs/s25main/world/World.h +++ b/libs/s25main/world/World.h @@ -192,8 +192,11 @@ class World : public MapBase /// Return the ID of the harbor point on that node or 0 if there is none HarborId GetHarborPointID(const MapPoint pt) const { return GetNode(pt).harborId; } const std::vector& GetHarborNeighbors(HarborId harborId, const ShipDirection& dir) const; - /// Berechnet die Entfernung zwischen 2 Hafenpunkten - unsigned CalcHarborDistance(HarborId haborId1, HarborId harborId2) const; + /// Return the minimum distance between 2 harbors + /// Note: Harbors could be at 2 seas, so there might be multiple, different distances + unsigned GetMinHarborDistance(HarborId haborId1, HarborId harborId2) const; + /// Return distance between harbors at given sea + unsigned GetHarborDistance(HarborId haborId1, HarborId harborId2, SeaId sea) const; /// Return the sea id if this is a point at a coast to a sea where ships can go. Else returns 0 SeaId GetSeaFromCoastalPoint(MapPoint pt) const; diff --git a/tests/s25Main/integration/testSeafaring.cpp b/tests/s25Main/integration/testSeafaring.cpp index f64d86d8b0..6d33f477a9 100644 --- a/tests/s25Main/integration/testSeafaring.cpp +++ b/tests/s25Main/integration/testSeafaring.cpp @@ -531,7 +531,7 @@ BOOST_FIXTURE_TEST_CASE(LongDistanceTravel, ShipReadyFixtureBig) // Go to opposite one const HarborId targetHbId(7); // Make sure that the other harbor is far away - BOOST_TEST_REQUIRE(world.CalcHarborDistance(harbor.GetHarborPosID(), targetHbId) > 600u); + BOOST_TEST_REQUIRE(world.GetMinHarborDistance(harbor.GetHarborPosID(), targetHbId) > 600u); // Add some scouts harbor.AddToInventory(PeopleCounts::make(Job::Scout, 20), true); // We want the ship to only scout unexplored harbors, so set all but one to visible @@ -799,11 +799,11 @@ BOOST_FIXTURE_TEST_CASE(IncreasedPathLengthWhenHarborDestroyed, ShipReadyFixture unsigned len; BOOST_TEST_REQUIRE( world.FindShipPath(ship.GetPos(), world.GetCoastalPoint(hbIdRight, seaId), unsigned(-1), nullptr, &len)); - const auto hbDistance = world.CalcHarborDistance(hbIdLeft, hbIdRight); + const auto hbDistance = world.GetMinHarborDistance(hbIdLeft, hbIdRight); // Reproducer requires the path to be longer than the direct distance and other connections BOOST_TEST_REQUIRE(len > hbDistance + extraDist); - BOOST_TEST_REQUIRE(hbDistance > world.CalcHarborDistance(hbIdRight, hbIdTop)); - BOOST_TEST_REQUIRE(hbDistance > world.CalcHarborDistance(hbIdRight, hbIdBottom)); + BOOST_TEST_REQUIRE(hbDistance > world.GetMinHarborDistance(hbIdRight, hbIdTop)); + BOOST_TEST_REQUIRE(hbDistance > world.GetMinHarborDistance(hbIdRight, hbIdBottom)); BuildingFactory::CreateBuilding(world, BuildingType::HarborBuilding, hbRightPos, curPlayer, Nation::Romans); BOOST_TEST_REQUIRE(ship.IsMoving()); BOOST_TEST_REQUIRE(ship.GetHomeHarbor() == hbIdRight); diff --git a/tests/s25Main/integration/testWorld.cpp b/tests/s25Main/integration/testWorld.cpp index 963efabb4b..dc25e23549 100644 --- a/tests/s25Main/integration/testWorld.cpp +++ b/tests/s25Main/integration/testWorld.cpp @@ -241,7 +241,7 @@ BOOST_FIXTURE_TEST_CASE(CloseHarborSpots, WorldFixture route; BOOST_TEST_REQUIRE((startPt == destPt || world.FindShipPath(startPt, destPt, 10000, &route, nullptr))); - BOOST_TEST_REQUIRE(route.size() == world.CalcHarborDistance(startHb, targetHb)); + BOOST_TEST_REQUIRE(route.size() == world.GetMinHarborDistance(startHb, targetHb)); } } } From a682974574f409cc8378363fc7938c7dde776207 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sun, 3 May 2026 20:35:46 +0200 Subject: [PATCH 09/11] Fix ship path finding for long distances Do not try to limit pathfinding for ships. Instead store the most common routes: Those between harbors in a small, fast lookup table. For other routes, e.g. from shipyards or lost ships, use a LRU cache and fall back to unlimited search. --- libs/s25main/Pathfinding.cpp | 54 ++++++++++--- libs/s25main/nodeObjs/noShip.cpp | 38 ++++----- libs/s25main/pathfinding/ShipPathData.cpp | 95 +++++++++++++++++++++++ libs/s25main/pathfinding/ShipPathData.h | 58 ++++++++++++++ libs/s25main/world/GameWorldBase.cpp | 8 ++ libs/s25main/world/GameWorldBase.h | 7 +- 6 files changed, 227 insertions(+), 33 deletions(-) create mode 100644 libs/s25main/pathfinding/ShipPathData.cpp create mode 100644 libs/s25main/pathfinding/ShipPathData.h diff --git a/libs/s25main/Pathfinding.cpp b/libs/s25main/Pathfinding.cpp index 3f82aef330..76cc75327c 100644 --- a/libs/s25main/Pathfinding.cpp +++ b/libs/s25main/Pathfinding.cpp @@ -4,12 +4,14 @@ #include "GamePlayer.h" #include "helpers/EnumRange.h" +#include "helpers/IdRange.h" #include "pathfinding/FreePathFinder.h" #include "pathfinding/FreePathFinderImpl.h" #include "pathfinding/PathConditionHuman.h" #include "pathfinding/PathConditionShip.h" #include "pathfinding/PathConditionTrade.h" #include "pathfinding/RoadPathFinder.h" +#include "pathfinding/ShipPathData.h" #include "world/GameWorld.h" #include "gameTypes/ShipDirection.h" #include "gameData/GameConsts.h" @@ -51,11 +53,10 @@ RoadPathDirection GameWorld::FindPathForWareOnRoads(const noRoadNode& start, con } bool GameWorldBase::FindShipPathToHarbor(const MapPoint start, HarborId harborId, SeaId seaId, - std::vector* route, unsigned* length) + std::vector* route, unsigned* length) const { - // Find the distance to the furthest harbor from the target harbor and take that as maximum - unsigned maxDistance = 0; const MapPoint coastalPoint = GetCoastalPoint(harborId, seaId); + RTTR_Assert(coastalPoint.isValid()); // already arrived? if(start == coastalPoint) @@ -66,23 +67,52 @@ bool GameWorldBase::FindShipPathToHarbor(const MapPoint start, HarborId harborId route->clear(); return true; } + auto& shipPathData = GetShipPathData(); + // If we start from a harbor we can get the route directly w/o the costly pathfinding + for(const auto startHbId : helpers::idRange(GetNumHarborPoints())) + { + if(GetCoastalPoint(startHbId, seaId) == start) + { + auto hbRoute = shipPathData.getHarborConnection(startHbId, harborId, seaId); + if(length) + *length = hbRoute.size(); + if(route) + *route = std::move(hbRoute); + } + } - for(const auto dir : helpers::EnumRange{}) + // Try cache first + bool reversed = false; + if(const auto* cachedPath = shipPathData.findCachedPath(start, coastalPoint, reversed)) { - const std::vector& neighbors = GetHarborNeighbors(harborId, dir); - for(const HarborPos::Neighbor& neighbor : neighbors) + if(route) { - if(IsHarborAtSea(neighbor.id, seaId) && neighbor.distance > maxDistance) - maxDistance = neighbor.distance; + *route = *cachedPath; + if(reversed) + { + std::reverse(route->begin(), route->end()); + for(auto& d : *route) + d = d + 3u; // reverse direction + } } + if(length) + *length = cachedPath->size(); + return true; } - // Add a few fields reserve - maxDistance += 6; - return FindShipPath(start, coastalPoint, maxDistance, route, length); + + // Not in cache -> compute full path and add + std::vector newRoute; + if(!FindShipPath(start, coastalPoint, std::numeric_limits::max(), &newRoute, length)) + return false; + shipPathData.addToCache(start, coastalPoint, newRoute); + if(route) + *route = std::move(newRoute); + + return true; } bool GameWorldBase::FindShipPath(const MapPoint start, const MapPoint dest, unsigned maxDistance, - std::vector* route, unsigned* length) + std::vector* route, unsigned* length) const { return GetFreePathFinder().FindPath(start, dest, true, maxDistance, route, length, nullptr, PathConditionShip(*this)); diff --git a/libs/s25main/nodeObjs/noShip.cpp b/libs/s25main/nodeObjs/noShip.cpp index ffd683f9aa..96de3f1c70 100644 --- a/libs/s25main/nodeObjs/noShip.cpp +++ b/libs/s25main/nodeObjs/noShip.cpp @@ -22,6 +22,7 @@ #include "notifications/ShipNote.h" #include "ogl/glArchivItem_Bitmap.h" #include "ogl/glArchivItem_Bitmap_Player.h" +#include "pathfinding/ShipPathData.h" #include "postSystem/ShipPostMsg.h" #include "random/Random.h" #include "world/GameWorld.h" @@ -952,7 +953,6 @@ void noShip::AbortSeaAttack() } } -/// Fängt an zu einem Hafen zu fahren (berechnet Route usw.) void noShip::StartDrivingToHarborPlace() { if(!goalHarbor) @@ -967,28 +967,28 @@ void noShip::StartDrivingToHarborPlace() route_.clear(); else { - bool routeFound; - // Use upper bound to distance by checking the distance between the harbors if we still have and are at the home - // harbor + // if we still have and are at the home harbor get route directly if(homeHarbor && pos == world->GetCoastalPoint(homeHarbor, seaId_)) { - // Use the maximum distance between the harbors plus 6 fields - unsigned maxDistance = world->GetMinHarborDistance(homeHarbor, goalHarbor) + 6; - routeFound = world->FindShipPath(pos, coastalPos, maxDistance, &route_, nullptr); + route_ = world->GetShipPathData().getHarborConnection(homeHarbor, goalHarbor, seaId_); + RTTR_Assert(!route_.empty()); } else - routeFound = world->FindShipPathToHarbor(pos, goalHarbor, seaId_, &route_, nullptr); - if(!routeFound) { - // todo - RTTR_Assert(false); - LOG.write("WARNING: Bug detected (GF: %u). Please report this with the savegame and " - "replay.\nnoShip::StartDrivingToHarborPlace: Schiff hat keinen Weg gefunden!\nplayer %i state %i " - "pos %u,%u goal " - "coastal %u,%u goal-id %i goalpos %u,%u \n") - % GetEvMgr().GetCurrentGF() % unsigned(ownerId_) % unsigned(state) % pos.x % pos.y % coastalPos.x - % coastalPos.y % goalHarbor % world->GetHarborPoint(goalHarbor).x % world->GetHarborPoint(goalHarbor).y; - goalHarbor.reset(); - return; + if(!world->FindShipPathToHarbor(pos, goalHarbor, seaId_, &route_, nullptr)) + { + // todo + RTTR_Assert(false); + LOG.write( + "WARNING: Bug detected (GF: %u). Please report this with the savegame and " + "replay.\nnoShip::StartDrivingToHarborPlace: Schiff hat keinen Weg gefunden!\nplayer %i state %i " + "pos %u,%u goal " + "coastal %u,%u goal-id %i goalpos %u,%u \n") + % GetEvMgr().GetCurrentGF() % unsigned(ownerId_) % unsigned(state) % pos.x % pos.y % coastalPos.x + % coastalPos.y % goalHarbor % world->GetHarborPoint(goalHarbor).x + % world->GetHarborPoint(goalHarbor).y; + goalHarbor.reset(); + return; + } } } curRouteIdx = 0; diff --git a/libs/s25main/pathfinding/ShipPathData.cpp b/libs/s25main/pathfinding/ShipPathData.cpp new file mode 100644 index 0000000000..7b9a776668 --- /dev/null +++ b/libs/s25main/pathfinding/ShipPathData.cpp @@ -0,0 +1,95 @@ +// Copyright (C) 2005 - 2026 Settlers Freaks (sf-team at siedler25.org) +// +// SPDX-License-Identifier: GPL-2.0-or-later + +#include "ShipPathData.h" +#include "EventManager.h" +#include "helpers/IdRange.h" +#include "helpers/containerUtils.h" +#include "pathfinding/FreePathFinder.h" +#include "pathfinding/PathConditionShip.h" +#include "world/GameWorldBase.h" +#include +#include + +struct ShipPathData::HarborConnection::Matcher +{ + HarborId harbor; + SeaId sea; + constexpr bool operator()(const ShipPathData::HarborConnection& con) const + { + return con.destHarbor == harbor && con.sea == sea; + } +}; + +std::vector ShipPathData::getHarborConnection(HarborId startHarbor, HarborId destHarbor, SeaId seaId) +{ + if(harborConnections_.empty()) + initHarborConnections(); + const auto it = helpers::find_if(harborConnections_[startHarbor], HarborConnection::Matcher{destHarbor, seaId}); + RTTR_Assert(it != harborConnections_[startHarbor].end()); + return it->route; +} + +const std::vector* ShipPathData::findCachedPath(MapPoint start, MapPoint dest, bool& reversed) const +{ + for(const PathEntry& e : cachedPaths_) + { + if(e.start == start && e.dest == dest) + { + const_cast(e).lastUse = world_.GetEvMgr().GetCurrentGF(); + reversed = false; + return &e.route; + } + if(e.start == dest && e.dest == start) + { + const_cast(e).lastUse = world_.GetEvMgr().GetCurrentGF(); + reversed = true; + return &e.route; + } + } + return nullptr; +} + +void ShipPathData::addToCache(MapPoint start, MapPoint dest, std::vector route) +{ + PathEntry entry{start, dest, world_.GetEvMgr().GetCurrentGF(), std::move(route)}; + if(cachedPaths_.size() < cachedPaths_.max_size()) + cachedPaths_.emplace_back(std::move(entry)); + else + { + const auto itOldest = + std::min_element(cachedPaths_.begin(), cachedPaths_.end(), + [](const PathEntry& a, const PathEntry& b) { return a.lastUse < b.lastUse; }); + *itOldest = std::move(entry); + } +} + +void ShipPathData::initHarborConnections() +{ + harborConnections_.resize(world_.GetNumHarborPoints()); + for(const auto startHbId : helpers::idRange(world_.GetNumHarborPoints())) + { + auto& curConnections = harborConnections_[startHbId]; + std::vector neighbors; + for(const auto dir : helpers::EnumRange{}) + { + const auto& curNbs = world_.GetHarborNeighbors(startHbId, dir); + neighbors.insert(neighbors.end(), curNbs.begin(), curNbs.end()); + } + for(const auto& nb : neighbors) + { + RTTR_Assert(startHbId != nb.id); + RTTR_Assert(nb.sea); + // Connections are unique + RTTR_Assert(!helpers::contains_if(curConnections, HarborConnection::Matcher{nb.id, nb.sea})); + curConnections.push_back(HarborConnection{nb.id, nb.sea, {}}); + unsigned len; + const bool found = + world_.FindShipPath(world_.GetCoastalPoint(startHbId, nb.sea), world_.GetCoastalPoint(nb.id, nb.sea), + nb.distance, &curConnections.back().route, &len); + RTTR_Assert(found); + RTTR_Assert(len == nb.distance); + } + } +} diff --git a/libs/s25main/pathfinding/ShipPathData.h b/libs/s25main/pathfinding/ShipPathData.h new file mode 100644 index 0000000000..fd1fdb8d95 --- /dev/null +++ b/libs/s25main/pathfinding/ShipPathData.h @@ -0,0 +1,58 @@ +// Copyright (C) 2005 - 2026 Settlers Freaks (sf-team at siedler25.org) +// +// SPDX-License-Identifier: GPL-2.0-or-later + +#pragma once +#include +#include +#include +#include +#include +#include +#include + +class GameWorldBase; + +/// Implements data for finding ship paths. +/// Assumes seas and land don't change so paths can be heavily cached. +/// Most paths are between harbor points and can be queried directly, for others a LRU cache is provided +class ShipPathData +{ +public: + ShipPathData(const GameWorldBase& world) : world_(world) {} + /// Get route from start to destination harbor on the given sea. + /// Assumes it exists, i.e. they both have a coast at that sea. + std::vector getHarborConnection(HarborId startHarbor, HarborId destHarbor, SeaId seaId); + /// Find a route in the cache + /// Return reference to it if found, else nullptr. + /// Sets `reversed` to true if start and dest of the route are swapped + const std::vector* findCachedPath(MapPoint start, MapPoint dest, bool& reversed) const; + void addToCache(MapPoint start, MapPoint dest, std::vector route); + +private: + const GameWorldBase& world_; + struct HarborConnection + { + HarborId destHarbor; + SeaId sea; + std::vector route; + struct Matcher; + }; + + /// Connections from a harbor to others. + /// Usually only a small number of harbors are reachable, so inline some. + using HarborConnectionVector = boost::container::small_vector; + helpers::StrongIdVector harborConnections_; + + struct PathEntry + { + MapPoint start; + MapPoint dest; + unsigned lastUse; + std::vector route; + }; + // LRU cache for arbitrary ship paths (start,dest). + boost::container::static_vector cachedPaths_; + + void initHarborConnections(); +}; diff --git a/libs/s25main/world/GameWorldBase.cpp b/libs/s25main/world/GameWorldBase.cpp index 9e7c1d1971..38363427fc 100644 --- a/libs/s25main/world/GameWorldBase.cpp +++ b/libs/s25main/world/GameWorldBase.cpp @@ -22,6 +22,7 @@ #include "notifications/PlayerNodeNote.h" #include "pathfinding/FreePathFinder.h" #include "pathfinding/RoadPathFinder.h" +#include "pathfinding/ShipPathData.h" #include "nodeObjs/noFlag.h" #include "gameData/BuildingProperties.h" #include "gameData/GameConsts.h" @@ -81,6 +82,13 @@ bool GameWorldBase::IsSinglePlayer() const return true; } +ShipPathData& GameWorldBase::GetShipPathData() const +{ + if(!shipPathData) + shipPathData = std::make_unique(*this); + return *shipPathData; +} + bool GameWorldBase::IsRoadAvailable(const bool boat_road, const MapPoint pt) const { // Hindernisse diff --git a/libs/s25main/world/GameWorldBase.h b/libs/s25main/world/GameWorldBase.h index 8d01f57b21..3d5ff465d6 100644 --- a/libs/s25main/world/GameWorldBase.h +++ b/libs/s25main/world/GameWorldBase.h @@ -16,6 +16,7 @@ #include #include +class ShipPathData; class EventManager; class FreePathFinder; class GameInterface; @@ -51,6 +52,7 @@ class GameWorldBase : public World { std::unique_ptr roadPathFinder; std::unique_ptr freePathFinder; + mutable std::unique_ptr shipPathData; PostManager postManager; mutable NotificationManager notifications; @@ -114,12 +116,13 @@ class GameWorldBase : public World /// Find path for ships to a specific harbor and see. Return true on success /// If starting point equals the coastal point of target harbor, set length 0, clear route and return true. bool FindShipPathToHarbor(MapPoint start, HarborId harborId, SeaId seaId, std::vector* route, - unsigned* length); + unsigned* length) const; /// Find path for ships with a limited distance. Return true on success bool FindShipPath(MapPoint start, MapPoint dest, unsigned maxDistance, std::vector* route, - unsigned* length); + unsigned* length) const; RoadPathFinder& GetRoadPathFinder() const { return *roadPathFinder; } FreePathFinder& GetFreePathFinder() const { return *freePathFinder; } + ShipPathData& GetShipPathData() const; /// Return flag that is on road at given point. dir will be set to the direction of the road from the returned flag /// prevDir (if set) will be skipped when searching for the road points From 20adc09a3434d06742f8fa55ca24b61c38aa1b91 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 7 May 2026 20:13:54 +0200 Subject: [PATCH 10/11] Don't look for ship path when the start and goal are the same. There is no meaningful result for all out-parameters in the base pathfinding implementation. --- libs/s25main/figures/nofCarrier.cpp | 5 +++-- libs/s25main/pathfinding/ShipPathData.cpp | 8 +++++--- libs/s25main/world/MapSerializer.cpp | 9 ++++++++- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/libs/s25main/figures/nofCarrier.cpp b/libs/s25main/figures/nofCarrier.cpp index 8c5862afca..a9f5047501 100644 --- a/libs/s25main/figures/nofCarrier.cpp +++ b/libs/s25main/figures/nofCarrier.cpp @@ -630,13 +630,14 @@ void nofCarrier::LostWork() // Look for the shore const unsigned maxNodeDistance = 5; - std::vector coastPoints = + const auto coastPoints = world->GetMatchingPointsInRadius(tmpPos, maxNodeDistance, IsCoastalAndForFigs(*world)); + shore_path.clear(); for(const auto& it : coastPoints) { // 10x the node distance should be enough, otherwise it would be to far to paddle const unsigned maxDistance = maxNodeDistance * 10; - if(world->FindShipPath(tmpPos, it, maxDistance, &shore_path, nullptr)) + if(it == tmpPos || world->FindShipPath(tmpPos, it, maxDistance, &shore_path, nullptr)) { // Ok let's paddle to the coast rs_pos = 0; diff --git a/libs/s25main/pathfinding/ShipPathData.cpp b/libs/s25main/pathfinding/ShipPathData.cpp index 7b9a776668..a4f8f40a8f 100644 --- a/libs/s25main/pathfinding/ShipPathData.cpp +++ b/libs/s25main/pathfinding/ShipPathData.cpp @@ -84,10 +84,12 @@ void ShipPathData::initHarborConnections() // Connections are unique RTTR_Assert(!helpers::contains_if(curConnections, HarborConnection::Matcher{nb.id, nb.sea})); curConnections.push_back(HarborConnection{nb.id, nb.sea, {}}); - unsigned len; + unsigned len = 0; + const auto startPoint = world_.GetCoastalPoint(startHbId, nb.sea); + const auto nbPoint = world_.GetCoastalPoint(nb.id, nb.sea); const bool found = - world_.FindShipPath(world_.GetCoastalPoint(startHbId, nb.sea), world_.GetCoastalPoint(nb.id, nb.sea), - nb.distance, &curConnections.back().route, &len); + (startPoint == nbPoint) + || world_.FindShipPath(startPoint, nbPoint, nb.distance, &curConnections.back().route, &len); RTTR_Assert(found); RTTR_Assert(len == nb.distance); } diff --git a/libs/s25main/world/MapSerializer.cpp b/libs/s25main/world/MapSerializer.cpp index fa6847fc63..32b2c53a3a 100644 --- a/libs/s25main/world/MapSerializer.cpp +++ b/libs/s25main/world/MapSerializer.cpp @@ -200,7 +200,14 @@ void MapSerializer::Deserialize(GameWorldBase& world, SerializedGameData& sgd, G { if(!helpers::contains(otherSeas, sea)) continue; - unsigned len; + unsigned len = 0; + const auto startPoint = world.GetCoastalPoint(startHbId, sea); + const auto nbPoint = world.GetCoastalPoint(neighbor.id, sea); + if(neighbor.distance == 0 && startPoint == nbPoint) + { + neighbor.sea = sea; + break; + } if(world.FindShipPath(world.GetCoastalPoint(startHbId, sea), world.GetCoastalPoint(neighbor.id, sea), neighbor.distance, nullptr, &len) && len == neighbor.distance) From ef760c2f13a7a35038d7aba044c2a7dfa615accc Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 7 May 2026 20:19:13 +0200 Subject: [PATCH 11/11] Fix corner case for wandering boat carrier When we are already at the goal `WanderOnWater` changes the state to reflect it. So we must ensure to change the state only before, not after the call to this function. --- libs/s25main/figures/nofCarrier.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libs/s25main/figures/nofCarrier.cpp b/libs/s25main/figures/nofCarrier.cpp index a9f5047501..08aa754514 100644 --- a/libs/s25main/figures/nofCarrier.cpp +++ b/libs/s25main/figures/nofCarrier.cpp @@ -642,9 +642,10 @@ void nofCarrier::LostWork() // Ok let's paddle to the coast rs_pos = 0; cur_rs = nullptr; - if(state == CarrierState::WaitForWare || state == CarrierState::WaitForWareSpace) - WanderOnWater(); + const auto oldState = state; state = CarrierState::BoatcarrierWanderOnWater; + if(oldState == CarrierState::WaitForWare || oldState == CarrierState::WaitForWareSpace) + WanderOnWater(); return; } }