Skip to content

Destroy ships when player is defeated #1883

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Flamefire wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Destroy ships when player is defeated #1883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
26fc8d3
Run `DestroyCurrentObjs` in Test-EventManager
Flamefire Feb 2, 2026
709a81e
Const-correct `GamePlayer::GetFirstWH`
Flamefire Feb 2, 2026
7280303
Take non-NULL ship param by reference
Flamefire Feb 2, 2026
5a46da3
Add `noShip::Sink`
Flamefire Feb 2, 2026
4fc51db
Destroy ships when player is defeated
Flamefire Feb 2, 2026
9abcba1
Allow `nobHarborBuilding::WareLost` during destruction
Flamefire Feb 2, 2026
0986284
Update docstring
Flamefire Apr 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions libs/s25main/GamePlayer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Comment on lines +1515 to +1518
Copy link
Copy Markdown
Member

@Flow86 Flow86 Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be an addon? in the original RttR, defeated ships were not destroyed

Copy link
Copy Markdown
Member

@Spikeone Spikeone Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depends on - in the original only the player ever had ships, so it's a quite RttRish problem. Although I think just removing them is somewhat funny, since there is no burning/sinking animation. Maybe we should add some kind of animation (even the fire used for buildings does make sense I think).

Copy link
Copy Markdown
Member

@Flow86 Flow86 Apr 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep some burning/oil spill/whatever would be cool

Copy link
Copy Markdown
Member Author

@Flamefire Flamefire Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the small building fire as a vanishing, non-blocking object would work and not cause much trouble. Or do you want to make a new animation?

Copy link
Copy Markdown
Member

@Flow86 Flow86 Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if that works, I think thats a good thing.

isDefeated = true;

// GUI Bescheid sagen
Expand Down Expand Up @@ -1981,12 +1985,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
Expand Down
4 changes: 2 additions & 2 deletions libs/s25main/GamePlayer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 1 addition & 5 deletions libs/s25main/buildings/nobHarborBuilding.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down Expand Up @@ -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);
Expand Down
24 changes: 23 additions & 1 deletion libs/s25main/nodeObjs/noShip.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);
}
5 changes: 4 additions & 1 deletion libs/s25main/nodeObjs/noShip.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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);

/// Remove the ship and its contents
void Sink();
};
69 changes: 69 additions & 0 deletions tests/s25Main/integration/testSeafaring.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,4 +825,73 @@ 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<noShip*>(&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_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<noShip*>(&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()
22 changes: 9 additions & 13 deletions tests/s25Main/worldFixtures/TestEventManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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;
}
Expand Down
Loading