From 26fc8d359a7a4746d5341ae29f107bee40c6e3ef Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 2 Feb 2026 11:11:05 +0100 Subject: [PATCH 1/7] Run `DestroyCurrentObjs` in Test-EventManager This is run for every gameframe in "real mode" and required for some tests simulating e.g. a single step. --- .../worldFixtures/TestEventManager.cpp | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/tests/s25Main/worldFixtures/TestEventManager.cpp b/tests/s25Main/worldFixtures/TestEventManager.cpp index 49a02e6766..e12213abb5 100644 --- a/tests/s25Main/worldFixtures/TestEventManager.cpp +++ b/tests/s25Main/worldFixtures/TestEventManager.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 @@ -9,22 +9,18 @@ unsigned TestEventManager::ExecuteNextEvent(unsigned maxGF) { if(GetCurrentGF() >= maxGF) return 0; - if(events.empty()) + unsigned numGFs; + if(events.empty() || events.begin()->first > maxGF) { - unsigned numGFs = maxGF - GetCurrentGF(); + numGFs = maxGF - GetCurrentGF(); currentGF = maxGF; - return numGFs; - } - auto itEvents = events.begin(); - if(itEvents->first > maxGF) + } else { - unsigned numGFs = maxGF - GetCurrentGF(); - currentGF = maxGF; - return numGFs; + auto itEvents = events.begin(); + numGFs = itEvents->first - GetCurrentGF(); + currentGF = itEvents->first; + ExecuteEvents(itEvents); } - unsigned numGFs = itEvents->first - GetCurrentGF(); - currentGF = itEvents->first; - ExecuteEvents(itEvents); DestroyCurrentObjects(); return numGFs; } From 709a81ec8ee2800bf6fd53a6ecbd8e48dd0e6e72 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 2 Feb 2026 11:15:27 +0100 Subject: [PATCH 2/7] Const-correct `GamePlayer::GetFirstWH` --- libs/s25main/GamePlayer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/s25main/GamePlayer.h b/libs/s25main/GamePlayer.h index 7c9e59f524..558c67ca7d 100644 --- a/libs/s25main/GamePlayer.h +++ b/libs/s25main/GamePlayer.h @@ -117,7 +117,7 @@ class GamePlayer : public GamePlayerInfo /// Returns true if the given wh does still exist and hence the ptr is valid bool IsWarehouseValid(nobBaseWarehouse* wh) const; /// Gibt erstes Lagerhaus zurück - nobBaseWarehouse* GetFirstWH() + nobBaseWarehouse* GetFirstWH() const { return buildings.GetStorehouses().empty() ? nullptr : buildings.GetStorehouses().front(); } From 7280303f7f193301347abd4598bc27cbf61744b2 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 2 Feb 2026 11:16:12 +0100 Subject: [PATCH 3/7] Take non-NULL ship param by reference --- libs/s25main/GamePlayer.cpp | 8 +++----- libs/s25main/GamePlayer.h | 2 +- libs/s25main/nodeObjs/noShip.cpp | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/libs/s25main/GamePlayer.cpp b/libs/s25main/GamePlayer.cpp index 2230e3ccce..ced06946c5 100644 --- a/libs/s25main/GamePlayer.cpp +++ b/libs/s25main/GamePlayer.cpp @@ -1981,12 +1981,10 @@ bool GamePlayer::OrderShip(nobHarborBuilding& hb) return false; } -/// Meldet das Schiff wieder ab -void GamePlayer::RemoveShip(noShip* ship) +void GamePlayer::RemoveShip(noShip& ship) { - auto it = helpers::find(ships, ship); - RTTR_Assert(it != ships.end()); - ships.erase(it); + RTTR_Assert(helpers::contains(ships, &ship)); + helpers::erase(ships, &ship); } /// Versucht, für ein untätiges Schiff eine Arbeit zu suchen diff --git a/libs/s25main/GamePlayer.h b/libs/s25main/GamePlayer.h index 558c67ca7d..383f76f25c 100644 --- a/libs/s25main/GamePlayer.h +++ b/libs/s25main/GamePlayer.h @@ -240,7 +240,7 @@ class GamePlayer : public GamePlayerInfo /// Registriert ein Schiff beim Einwohnermeldeamt void RegisterShip(noShip& ship); /// Meldet das Schiff wieder ab - void RemoveShip(noShip* ship); + void RemoveShip(noShip& ship); /// Versucht, für ein untätiges Schiff eine Arbeit zu suchen void GetJobForShip(noShip& ship); /// Schiff für Hafen bestellen. Wenn ein Schiff kommt, true. diff --git a/libs/s25main/nodeObjs/noShip.cpp b/libs/s25main/nodeObjs/noShip.cpp index 2c71f1719d..4c2854283d 100644 --- a/libs/s25main/nodeObjs/noShip.cpp +++ b/libs/s25main/nodeObjs/noShip.cpp @@ -104,7 +104,7 @@ void noShip::Destroy() RTTR_Assert(wares.empty()); world->GetNotifications().publish(ShipNote(ShipNote::Destroyed, ownerId_, pos)); // Schiff wieder abmelden - world->GetPlayer(ownerId_).RemoveShip(this); + world->GetPlayer(ownerId_).RemoveShip(*this); } void noShip::Draw(DrawPoint drawPt) From 5a46da3e6365dc2eb9117ebc4d93018da67608ac Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 2 Feb 2026 11:18:32 +0100 Subject: [PATCH 4/7] Add `noShip::Sink` --- libs/s25main/nodeObjs/noShip.cpp | 22 +++++++++++++ libs/s25main/nodeObjs/noShip.h | 5 ++- tests/s25Main/integration/testSeafaring.cpp | 35 +++++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/libs/s25main/nodeObjs/noShip.cpp b/libs/s25main/nodeObjs/noShip.cpp index 4c2854283d..66d4057b2e 100644 --- a/libs/s25main/nodeObjs/noShip.cpp +++ b/libs/s25main/nodeObjs/noShip.cpp @@ -1223,3 +1223,25 @@ void noShip::NewHarborBuilt(nobHarborBuilding* hb) break; } } + +void noShip::Sink() +{ + for(auto& figure : figures) + { + figure->Abrogate(); + figure->SetGoalTonullptr(); + figure->RemoveFromInventory(); + } + figures.clear(); + + for(auto& ware : wares) + { + ware->WareLost(ownerId_); + ware->Destroy(); + } + wares.clear(); + + GetEvMgr().RemoveEvent(current_ev); + GetEvMgr().AddToKillList(world->RemoveFigure(pos, *this)); + world->RecalcVisibilitiesAroundPoint(pos, GetVisualRange(), ownerId_, nullptr); +} diff --git a/libs/s25main/nodeObjs/noShip.h b/libs/s25main/nodeObjs/noShip.h index fd5072dd00..27ef2313cb 100644 --- a/libs/s25main/nodeObjs/noShip.h +++ b/libs/s25main/nodeObjs/noShip.h @@ -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 @@ -224,4 +224,7 @@ class noShip : public noMovable void HarborDestroyed(nobHarborBuilding* hb); /// Sagt dem Schiff, dass ein neuer Hafen erbaut wurde void NewHarborBuilt(nobHarborBuilding* hb); + + /// Destroy the ship immediately + void Sink(); }; diff --git a/tests/s25Main/integration/testSeafaring.cpp b/tests/s25Main/integration/testSeafaring.cpp index 009d28b439..391a29592a 100644 --- a/tests/s25Main/integration/testSeafaring.cpp +++ b/tests/s25Main/integration/testSeafaring.cpp @@ -825,4 +825,39 @@ BOOST_FIXTURE_TEST_CASE(GoToNeighborHarbor, ShipReadyFixture<1>) BOOST_TEST_REQUIRE(ship.IsMoving()); } +BOOST_FIXTURE_TEST_CASE(SinkShipLoosesCargo, ShipAndHarborsReadyFixture<1>) +{ + const GamePlayer& player = world.GetPlayer(curPlayer); + noShip& ship = *player.GetShipByID(0); + + const auto& harbors = player.GetBuildingRegister().GetHarbors(); + BOOST_TEST_REQUIRE(harbors.size() >= 2u); + nobHarborBuilding& harbor1 = *harbors.front(); + nobHarborBuilding& harbor2 = **(++harbors.begin()); + + // Transport something + BOOST_TEST_REQUIRE(harbor1.OrderJob(Job::Woodcutter, harbor2, false)); + BOOST_TEST_REQUIRE(harbor1.OrderWare(GoodType::Wood, harbor2)); + BOOST_TEST_REQUIRE(harbor1.OrderWare(GoodType::Wood, harbor2)); + + RTTR_EXEC_TILL(90, ship.IsLoading()); + RTTR_EXEC_TILL(200, ship.IsMoving()); + BOOST_TEST(ship.GetWares().size() == 2u); + BOOST_TEST(ship.GetFigures().size() == 1u); + + const auto shipPos = ship.GetPos(); + BOOST_TEST_REQUIRE(player.GetNumShips() == 1u); + BOOST_TEST_REQUIRE(world.GetFigures(shipPos).size() == 1u); + BOOST_TEST_REQUIRE(dynamic_cast(&world.GetFigures(shipPos).front())); + + const auto numWood = player.GetInventory()[GoodType::Wood]; + const auto numWoodcutters = player.GetInventory()[Job::Woodcutter]; + ship.Sink(); + RTTR_SKIP_GFS(1); // Handle delayed destruction + BOOST_TEST(player.GetNumShips() == 0u); + BOOST_TEST(world.GetFigures(shipPos).size() == 0u); + BOOST_TEST(player.GetInventory()[GoodType::Wood] == numWood - 2); + BOOST_TEST(player.GetInventory()[Job::Woodcutter] == numWoodcutters - 1); +} + BOOST_AUTO_TEST_SUITE_END() From 4fc51db2345d7a549e913e84913d70767f9a26fe Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 2 Feb 2026 11:19:00 +0100 Subject: [PATCH 5/7] Destroy ships when player is defeated See #183 --- libs/s25main/GamePlayer.cpp | 4 +++ tests/s25Main/integration/testSeafaring.cpp | 34 +++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/libs/s25main/GamePlayer.cpp b/libs/s25main/GamePlayer.cpp index ced06946c5..da200901c3 100644 --- a/libs/s25main/GamePlayer.cpp +++ b/libs/s25main/GamePlayer.cpp @@ -1512,6 +1512,10 @@ void GamePlayer::Surrender() if(isDefeated) return; + const auto shipsCopy = ships; // copy to avoid modification during iteration + for(auto* ship : shipsCopy) + ship->Sink(); + isDefeated = true; // GUI Bescheid sagen diff --git a/tests/s25Main/integration/testSeafaring.cpp b/tests/s25Main/integration/testSeafaring.cpp index 391a29592a..dfc3b55403 100644 --- a/tests/s25Main/integration/testSeafaring.cpp +++ b/tests/s25Main/integration/testSeafaring.cpp @@ -860,4 +860,38 @@ BOOST_FIXTURE_TEST_CASE(SinkShipLoosesCargo, ShipAndHarborsReadyFixture<1>) BOOST_TEST(player.GetInventory()[Job::Woodcutter] == numWoodcutters - 1); } +BOOST_FIXTURE_TEST_CASE(RemoveShipsOnDefeat, ShipAndHarborsReadyFixture<2>) +{ + const GamePlayer& player = world.GetPlayer(curPlayer); + const noShip& ship = *player.GetShipByID(0); + + const auto& harbors = player.GetBuildingRegister().GetHarbors(); + BOOST_TEST_REQUIRE(harbors.size() >= 2u); + nobHarborBuilding& harbor1 = *harbors.front(); + nobHarborBuilding& harbor2 = **(++harbors.begin()); + + // Transport something + BOOST_TEST_REQUIRE(harbor1.OrderJob(Job::Woodcutter, harbor2, false)); + BOOST_TEST_REQUIRE(harbor1.OrderWare(GoodType::Wood, harbor2)); + + RTTR_EXEC_TILL(90, ship.IsLoading()); + RTTR_EXEC_TILL(200, ship.IsMoving()); + const auto numWood = ship.GetWares().size(); + const auto numWoodcutters = ship.GetFigures().size(); + BOOST_TEST(numWood == 1u); + BOOST_TEST(numWoodcutters == 1u); + + const auto shipPos = ship.GetPos(); + BOOST_TEST_REQUIRE(player.GetNumShips() == 1u); + BOOST_TEST_REQUIRE(world.GetFigures(shipPos).size() == 1u); + BOOST_TEST_REQUIRE(dynamic_cast(&world.GetFigures(shipPos).front())); + const auto warehouses = player.GetBuildingRegister().GetStorehouses(); // Copy for iteration + for(const auto* bld : warehouses) + world.DestroyBuilding(bld->GetPos(), curPlayer); + RTTR_SKIP_GFS(1); // Handle delayed destruction + BOOST_TEST(player.IsDefeated()); + BOOST_TEST(player.GetNumShips() == 0u); + BOOST_TEST(world.GetFigures(shipPos).size() == 0u); +} + BOOST_AUTO_TEST_SUITE_END() From 9abcba1f098eb3272cb0fbd7a1c34ef82d8f6684 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 2 Feb 2026 11:20:55 +0100 Subject: [PATCH 6/7] Allow `nobHarborBuilding::WareLost` during destruction This is a corner case when destroying the harbor leads to defeat which leads to sinking ships potentially with wares to this building. --- libs/s25main/buildings/nobHarborBuilding.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/libs/s25main/buildings/nobHarborBuilding.cpp b/libs/s25main/buildings/nobHarborBuilding.cpp index f35ac54c0f..eeff5db7d2 100644 --- a/libs/s25main/buildings/nobHarborBuilding.cpp +++ b/libs/s25main/buildings/nobHarborBuilding.cpp @@ -435,8 +435,7 @@ void nobHarborBuilding::StopExplorationExpedition() /// Bestellt die zusätzlichen erforderlichen Waren für eine Expedition void nobHarborBuilding::OrderExpeditionWares() { - RTTR_Assert(!IsBeingDestroyedNow()); // Wares should already be canceled! - if(this->IsBeingDestroyedNow()) // don't order new stuff if we are about to be destroyed + if(IsBeingDestroyedNow()) // don't order new stuff if we are about to be destroyed return; if(!expedition.active) // expedition no longer active? @@ -490,11 +489,8 @@ void nobHarborBuilding::OrderExpeditionWares() orderware_ev = GetEvMgr().AddEvent(this, 210, 10); } -/// Eine bestellte Ware konnte doch nicht kommen void nobHarborBuilding::WareLost(Ware& ware) { - RTTR_Assert(!IsBeingDestroyedNow()); - // ggf. neue Waren für Expedition bestellen if(expedition.active && (ware.type == GoodType::Boards || ware.type == GoodType::Stones)) OrderExpeditionWares(); nobBaseWarehouse::WareLost(ware); From 098628406e5c23c148e8dbd51a3b0349d4af0ad8 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sun, 12 Apr 2026 14:05:47 +0200 Subject: [PATCH 7/7] Update docstring Co-authored-by: Flow86 <656249+Flow86@users.noreply.github.com> --- libs/s25main/nodeObjs/noShip.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/s25main/nodeObjs/noShip.h b/libs/s25main/nodeObjs/noShip.h index 27ef2313cb..f382d5fb0b 100644 --- a/libs/s25main/nodeObjs/noShip.h +++ b/libs/s25main/nodeObjs/noShip.h @@ -225,6 +225,6 @@ class noShip : public noMovable /// Sagt dem Schiff, dass ein neuer Hafen erbaut wurde void NewHarborBuilt(nobHarborBuilding* hb); - /// Destroy the ship immediately + /// Remove the ship and its contents void Sink(); };