Skip to content

Commit c67fb2f

Browse files
committed
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.
1 parent f32807f commit c67fb2f

12 files changed

Lines changed: 117 additions & 49 deletions

File tree

libs/s25main/Pathfinding.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ bool GameWorldBase::FindShipPath(const MapPoint start, const MapPoint dest, unsi
9090

9191
/// Prüft, ob eine Schiffsroute noch Gültigkeit hat
9292
bool GameWorld::CheckShipRoute(const MapPoint start, const std::vector<Direction>& route, const unsigned pos,
93-
MapPoint* dest)
93+
MapPoint* dest) const
9494
{
9595
return GetFreePathFinder().CheckRoute(start, route, pos, PathConditionShip(*this), dest);
9696
}

libs/s25main/SerializedGameData.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,8 @@
108108
/// 12: leatheraddon added, three new building types and three new goods
109109
/// 13: SeaId & HarborId: World::harborData w/o dummy entry at 0
110110
/// 14: Remove "age" field in nobBaseMilitary
111-
static const unsigned currentGameDataVersion = 14;
111+
/// 15: Add sea to HarborPos::Neighbor
112+
static const unsigned currentGameDataVersion = 15;
112113
// clang-format on
113114

114115
std::unique_ptr<GameObject> SerializedGameData::Create_GameObject(const GO_Type got, const unsigned obj_id)

libs/s25main/buildings/nobHarborBuilding.cpp

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -835,23 +835,27 @@ std::vector<nobHarborBuilding::ShipConnection> nobHarborBuilding::GetShipConnect
835835
if(world->GetGOT(pos) != GO_Type::NobHarborbuilding)
836836
return connections;
837837

838-
const auto sharesSeaWith = [&seaIds = this->seaIds, world = this->world](const nobHarborBuilding& harbor) {
838+
const auto getSharedSea = [&seaIds = this->seaIds, world = this->world](const nobHarborBuilding& harbor) {
839839
for(SeaId seaId : seaIds)
840840
{
841841
if(seaId && world->IsHarborAtSea(harbor.GetHarborPosID(), seaId))
842-
return true;
842+
return seaId;
843843
}
844-
return false;
844+
return SeaId::invalidValue();
845845
};
846846
for(auto* harbor_building : world->GetPlayer(player).GetBuildingRegister().GetHarbors())
847847
{
848-
if(harbor_building != this && sharesSeaWith(*harbor_building))
848+
if(harbor_building == this)
849+
continue;
850+
SeaId seaId = getSharedSea(*harbor_building);
851+
if(seaId)
849852
{
850853
ShipConnection sc;
851854
sc.dest = harbor_building;
852855
// Use twice the distance as cost (ship might need to arrive first) and a fixed value to represent
853856
// loading&unloading
854-
sc.way_costs = 2 * world->CalcHarborDistance(GetHarborPosID(), harbor_building->GetHarborPosID()) + 10;
857+
sc.way_costs =
858+
2 * world->GetHarborDistance(GetHarborPosID(), harbor_building->GetHarborPosID(), seaId) + 10;
855859
connections.push_back(sc);
856860
}
857861
}
@@ -1166,10 +1170,10 @@ nobHarborBuilding::GetAttackerBuildingsForSeaAttack(const std::vector<HarborId>&
11661170
continue;
11671171

11681172
// Entfernung zwischen Hafen und möglichen Zielhafenpunkt ausrechnen
1169-
unsigned min_distance = 0xffffffff;
1173+
auto min_distance = std::numeric_limits<unsigned>::max();
11701174
for(HarborId defender_harbor : defender_harbors)
11711175
{
1172-
min_distance = std::min(min_distance, world->CalcHarborDistance(GetHarborPosID(), defender_harbor));
1176+
min_distance = std::min(min_distance, world->GetMinHarborDistance(GetHarborPosID(), defender_harbor));
11731177
}
11741178

11751179
// Gebäude suchen, vielleicht schon vorhanden?
@@ -1202,7 +1206,7 @@ void nobHarborBuilding::AddSeaAttacker(std::unique_ptr<nofAttacker> attacker)
12021206
world->GetHarborPointsAroundMilitaryBuilding(attacker->GetAttackedGoal()->GetPos());
12031207
for(HarborId harbor_point : harbor_points)
12041208
{
1205-
unsigned tmp_distance = world->CalcHarborDistance(this->GetHarborPosID(), harbor_point);
1209+
unsigned tmp_distance = world->GetMinHarborDistance(this->GetHarborPosID(), harbor_point);
12061210
if(tmp_distance < best_distance)
12071211
{
12081212
best_distance = tmp_distance;

libs/s25main/gameTypes/HarborPos.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,19 @@ struct HarborPos
1515
struct Neighbor
1616
{
1717
HarborId id;
18+
SeaId sea;
1819
unsigned distance;
1920

20-
Neighbor(HarborId id, unsigned distance) noexcept : id(id), distance(distance) {}
21+
Neighbor(HarborId id, SeaId sea, unsigned distance) noexcept : id(id), sea(sea), distance(distance) {}
2122

2223
bool operator<(const Neighbor& two) const noexcept
2324
{
24-
return (distance < two.distance) || (distance == two.distance && id.value() < two.id.value());
25+
if(distance == two.distance)
26+
{
27+
if(sea != two.sea)
28+
return sea.value() < two.sea.value();
29+
}
30+
return id.value() < two.id.value();
2531
}
2632
};
2733

libs/s25main/nodeObjs/noShip.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -973,7 +973,7 @@ void noShip::StartDrivingToHarborPlace()
973973
if(homeHarbor && pos == world->GetCoastalPoint(homeHarbor, seaId_))
974974
{
975975
// Use the maximum distance between the harbors plus 6 fields
976-
unsigned maxDistance = world->CalcHarborDistance(homeHarbor, goalHarbor) + 6;
976+
unsigned maxDistance = world->GetMinHarborDistance(homeHarbor, goalHarbor) + 6;
977977
routeFound = world->FindShipPath(pos, coastalPos, maxDistance, &route_, nullptr);
978978
} else
979979
routeFound = world->FindShipPathToHarbor(pos, goalHarbor, seaId_, &route_, nullptr);

libs/s25main/world/GameWorld.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ class GameWorld : public GameWorldBase
9494
unsigned* length = nullptr, MapPoint* firstPt = nullptr,
9595
unsigned max = std::numeric_limits<unsigned>::max());
9696
/// Prüft, ob eine Schiffsroute noch Gültigkeit hat
97-
bool CheckShipRoute(MapPoint start, const std::vector<Direction>& route, unsigned pos, MapPoint* dest);
97+
bool CheckShipRoute(MapPoint start, const std::vector<Direction>& route, unsigned pos, MapPoint* dest) const;
9898
/// Find a route for trade caravanes
9999
helpers::OptionalEnum<Direction> FindTradePath(MapPoint start, MapPoint dest, unsigned char player,
100100
unsigned max_route = 0xffffffff, bool random_route = false,

libs/s25main/world/MapLoader.cpp

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -483,24 +483,6 @@ bool MapLoader::InitSeasAndHarbors(World& world, const std::vector<MapPoint>& ad
483483

484484
// Calculate the neighbors and distances
485485
CalcHarborPosNeighbors(world);
486-
487-
// Validate
488-
for(const auto startHbId : helpers::idRange<HarborId>(world.harborData.size()))
489-
{
490-
const HarborPos& startHbPos = world.harborData[startHbId];
491-
for(const std::vector<HarborPos::Neighbor>& neighbors : startHbPos.neighbors)
492-
{
493-
for(const HarborPos::Neighbor& neighbor : neighbors)
494-
{
495-
if(world.CalcHarborDistance(neighbor.id, startHbId) != neighbor.distance)
496-
{
497-
LOG.write("Bug: Harbor distance mismatch for harbors %1%->%2%: %3% != %4%\n") % startHbId
498-
% neighbor.id % world.CalcHarborDistance(neighbor.id, startHbId) % neighbor.distance;
499-
return false;
500-
}
501-
}
502-
}
503-
}
504486
return true;
505487
}
506488

@@ -579,12 +561,12 @@ void MapLoader::CalcHarborPosNeighbors(World& world)
579561
for(const MapPoint& ownCoastPt : ownCoastalPoints)
580562
{
581563
// Special case: Get all harbors that share the coast point with us
582-
SeaId seaId = world.GetSeaFromCoastalPoint(ownCoastPt);
564+
const SeaId seaId = world.GetSeaFromCoastalPoint(ownCoastPt);
583565
auto const coastToHbs = coastToHarborPerSea[seaId].equal_range(world.GetIdx(ownCoastPt));
584566
for(auto it = coastToHbs.first; it != coastToHbs.second; ++it)
585567
{
586568
ShipDirection shipDir = world.GetShipDir(ownCoastPt, ownCoastPt);
587-
world.harborData[startHbId].neighbors[shipDir].push_back(HarborPos::Neighbor(it->second, 0));
569+
world.harborData[startHbId].neighbors[shipDir].push_back(HarborPos::Neighbor(it->second, seaId, 0));
588570
hbFound[it->second] = true;
589571
}
590572
todo_list.push(CalcHarborPosNeighborsNode(ownCoastPt, 0));
@@ -610,8 +592,9 @@ void MapLoader::CalcHarborPosNeighbors(World& world)
610592

611593
if(ptValue > 0) // found harbor(s)
612594
{
613-
ShipDirection shipDir = world.GetShipDir(world.harborData[startHbId].pos, curPt);
614-
SeaId seaId = world.GetSeaFromCoastalPoint(curPt);
595+
const ShipDirection shipDir = world.GetShipDir(world.harborData[startHbId].pos, curPt);
596+
const SeaId seaId = world.GetSeaFromCoastalPoint(curPt);
597+
RTTR_Assert(seaId);
615598
auto const coastToHbs = coastToHarborPerSea[seaId].equal_range(idx);
616599
for(auto it = coastToHbs.first; it != coastToHbs.second; ++it)
617600
{
@@ -621,11 +604,10 @@ void MapLoader::CalcHarborPosNeighbors(World& world)
621604

622605
hbFound[otherHbId] = true;
623606
world.harborData[startHbId].neighbors[shipDir].push_back(
624-
HarborPos::Neighbor(otherHbId, curNode.distance + 1));
607+
HarborPos::Neighbor(otherHbId, seaId, curNode.distance + 1));
625608

626609
// Make this the only coastal point of this harbor for this sea
627610
HarborPos& otherHb = world.harborData[otherHbId];
628-
RTTR_Assert(seaId);
629611
for(const auto hbDir : helpers::EnumRange<Direction>{})
630612
{
631613
if(otherHb.seaIds[hbDir] == seaId && world.GetNeighbour(otherHb.pos, hbDir) != curPt)

libs/s25main/world/MapSerializer.cpp

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (C) 2005 - 2021 Settlers Freaks (sf-team at siedler25.org)
1+
// Copyright (C) 2005 - 2026 Settlers Freaks (sf-team at siedler25.org)
22
//
33
// SPDX-License-Identifier: GPL-2.0-or-later
44

@@ -7,10 +7,12 @@
77
#include "Game.h"
88
#include "SerializedGameData.h"
99
#include "buildings/noBuildingSite.h"
10+
#include "helpers/IdRange.h"
1011
#include "helpers/Range.h"
1112
#include "lua/GameDataLoader.h"
1213
#include "world/GameWorldBase.h"
1314
#include "s25util/warningSuppression.h"
15+
#include <boost/container/static_vector.hpp>
1416
#include <mygettext/mygettext.h>
1517

1618
void MapSerializer::Serialize(const GameWorldBase& world, SerializedGameData& sgd)
@@ -49,6 +51,7 @@ void MapSerializer::Serialize(const GameWorldBase& world, SerializedGameData& sg
4951
for(const auto& c : curNeighbors)
5052
{
5153
sgd.PushUnsignedInt(c.id.value());
54+
sgd.PushUnsignedInt(c.sea.value());
5255
sgd.PushUnsignedInt(c.distance);
5356
}
5457
}
@@ -147,8 +150,11 @@ void MapSerializer::Deserialize(GameWorldBase& world, SerializedGameData& sgd, G
147150
{
148151
RTTR_UNUSED(j);
149152
const auto id = HarborId(sgd.PopUnsignedInt());
153+
SeaId sea;
154+
if(sgd.GetGameDataVersion() >= 15)
155+
sea = SeaId(sgd.PopUnsignedInt());
150156
const auto distance = sgd.PopUnsignedInt();
151-
neighbor.emplace_back(id, distance);
157+
neighbor.emplace_back(id, sea, distance);
152158
}
153159
}
154160
}
@@ -158,6 +164,56 @@ void MapSerializer::Deserialize(GameWorldBase& world, SerializedGameData& sgd, G
158164
if(!world.harborData.front().pos.isValid())
159165
world.harborData.erase(world.harborData.begin());
160166
}
167+
if(sgd.GetGameDataVersion() < 15)
168+
{
169+
const auto getSeas = [&world](const HarborId& hbId) {
170+
boost::container::static_vector<SeaId, helpers::NumEnumValues_v<Direction>> seas;
171+
for(const auto sea : world.harborData[hbId].seaIds)
172+
{
173+
if(sea)
174+
seas.push_back(sea);
175+
}
176+
return seas;
177+
};
178+
// Determine seas for neighbors
179+
for(const auto startHbId : helpers::idRange<HarborId>(world.GetNumHarborPoints()))
180+
{
181+
const auto mySeas = getSeas(startHbId);
182+
for(auto& neighborsPerDir : world.harborData[startHbId].neighbors)
183+
{
184+
for(auto& neighbor : neighborsPerDir)
185+
{
186+
// Easy case: Either harbor is only at a single sea, so that must be the one
187+
if(mySeas.size() == 1u)
188+
{
189+
neighbor.sea = mySeas.front();
190+
continue;
191+
}
192+
const auto otherSeas = getSeas(neighbor.id);
193+
if(otherSeas.size() == 1u)
194+
{
195+
neighbor.sea = otherSeas.front();
196+
continue;
197+
}
198+
// Find the sea where this distance matches
199+
for(const auto sea : mySeas)
200+
{
201+
if(!helpers::contains(otherSeas, sea))
202+
continue;
203+
unsigned len;
204+
if(world.FindShipPath(world.GetCoastalPoint(startHbId, sea),
205+
world.GetCoastalPoint(neighbor.id, sea), neighbor.distance, nullptr, &len)
206+
&& len == neighbor.distance)
207+
{
208+
neighbor.sea = sea;
209+
break;
210+
}
211+
}
212+
RTTR_Assert(neighbor.sea);
213+
}
214+
}
215+
}
216+
}
161217

162218
sgd.PopObjectContainer(world.harbor_building_sites_from_sea, GO_Type::Buildingsite);
163219

libs/s25main/world/World.cpp

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -412,19 +412,35 @@ const std::vector<HarborPos::Neighbor>& World::GetHarborNeighbors(const HarborId
412412
return harborData[harborId].neighbors[dir];
413413
}
414414

415-
unsigned World::CalcHarborDistance(HarborId haborId1, HarborId harborId2) const
415+
unsigned World::GetMinHarborDistance(HarborId haborId1, HarborId harborId2) const
416416
{
417417
if(haborId1 == harborId2) // special case: distance to self
418418
return 0;
419+
unsigned minDistance = std::numeric_limits<unsigned>::max();
419420
for(const auto dir : helpers::EnumRange<ShipDirection>{})
420421
{
421422
for(const HarborPos::Neighbor& n : harborData[haborId1].neighbors[dir])
422423
{
423424
if(n.id == harborId2)
424-
return n.distance;
425+
minDistance = std::min(minDistance, n.distance);
425426
}
426427
}
428+
return minDistance;
429+
}
427430

431+
unsigned World::GetHarborDistance(HarborId haborId1, HarborId harborId2, SeaId sea) const
432+
{
433+
if(haborId1 == harborId2) // special case: distance to self
434+
return 0;
435+
for(const auto dir : helpers::EnumRange<ShipDirection>{})
436+
{
437+
for(const HarborPos::Neighbor& n : harborData[haborId1].neighbors[dir])
438+
{
439+
if(n.id == harborId2 && n.sea == sea)
440+
return n.distance;
441+
}
442+
}
443+
RTTR_Assert(!"Should only query actual neighbors");
428444
return std::numeric_limits<unsigned>::max();
429445
}
430446

libs/s25main/world/World.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,8 +192,11 @@ class World : public MapBase
192192
/// Return the ID of the harbor point on that node or 0 if there is none
193193
HarborId GetHarborPointID(const MapPoint pt) const { return GetNode(pt).harborId; }
194194
const std::vector<HarborPos::Neighbor>& GetHarborNeighbors(HarborId harborId, const ShipDirection& dir) const;
195-
/// Berechnet die Entfernung zwischen 2 Hafenpunkten
196-
unsigned CalcHarborDistance(HarborId haborId1, HarborId harborId2) const;
195+
/// Return the minimum distance between 2 harbors
196+
/// Note: Harbors could be at 2 seas, so there might be multiple, different distances
197+
unsigned GetMinHarborDistance(HarborId haborId1, HarborId harborId2) const;
198+
/// Return distance between harbors at given sea
199+
unsigned GetHarborDistance(HarborId haborId1, HarborId harborId2, SeaId sea) const;
197200
/// Return the sea id if this is a point at a coast to a sea where ships can go. Else returns 0
198201
SeaId GetSeaFromCoastalPoint(MapPoint pt) const;
199202

0 commit comments

Comments
 (0)