From 4f8e2b6a086ab44f395dd447b198ee1655c764f4 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sun, 19 Apr 2026 19:43:58 +0200 Subject: [PATCH 1/6] Update translations --- external/languages | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/languages b/external/languages index 4a65484a04..e212f6a2ad 160000 --- a/external/languages +++ b/external/languages @@ -1 +1 @@ -Subproject commit 4a65484a04a19ab8be071f86c1a6b54c8a3fc139 +Subproject commit e212f6a2ada902eeddc7fab4c4a8f32d3bcfa319 From 60542a487290741aa19cf92be167e7cbe120d068 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sun, 19 Apr 2026 19:44:34 +0200 Subject: [PATCH 2/6] Rename `Soldier::building` to `Soldier::homeBld` --- libs/s25main/figures/nofActiveSoldier.cpp | 18 +++++----- libs/s25main/figures/nofActiveSoldier.h | 2 +- .../s25main/figures/nofAggressiveDefender.cpp | 8 ++--- libs/s25main/figures/nofAttacker.cpp | 34 +++++++++---------- libs/s25main/figures/nofDefender.cpp | 28 +++++++-------- libs/s25main/figures/nofPassiveSoldier.cpp | 12 +++---- libs/s25main/figures/nofPassiveSoldier.h | 2 +- libs/s25main/figures/nofSoldier.cpp | 17 +++++----- libs/s25main/figures/nofSoldier.h | 16 ++++----- 9 files changed, 67 insertions(+), 70 deletions(-) diff --git a/libs/s25main/figures/nofActiveSoldier.cpp b/libs/s25main/figures/nofActiveSoldier.cpp index 9d18b8aa09..bcfed86d3f 100644 --- a/libs/s25main/figures/nofActiveSoldier.cpp +++ b/libs/s25main/figures/nofActiveSoldier.cpp @@ -43,11 +43,11 @@ void nofActiveSoldier::GoalReached() { // We reached the military building // Add myself to the building - if(!building) + if(!homeBld) { RTTR_Assert(false); - building = world->GetSpecObj(this->GetPos()); - if(building) + homeBld = world->GetSpecObj(this->GetPos()); + if(homeBld) LOG.write("nofActiveSoldier::GoalRoached() - no valid 'building' but found one at soldier's position " "(%i,%i) (gf: %u)\n") % pos.x % pos.y % GetEvMgr().GetCurrentGF(); @@ -59,7 +59,7 @@ void nofActiveSoldier::GoalReached() throw std::runtime_error("No building found for soldier"); } } - building->AddActiveSoldier(world->RemoveFigure(pos, *this)); + homeBld->AddActiveSoldier(world->RemoveFigure(pos, *this)); } void nofActiveSoldier::ReturnHome() @@ -71,7 +71,7 @@ void nofActiveSoldier::ReturnHome() void nofActiveSoldier::WalkingHome() { // Is our home military building destroyed? - if(!building) + if(!homeBld) { // Start wandering around state = SoldierState::FigureWork; @@ -83,13 +83,13 @@ void nofActiveSoldier::WalkingHome() // Walking home to our military building - if(GetPos() == building->GetFlagPos()) // Are we already at the flag? + if(GetPos() == homeBld->GetFlagPos()) // Are we already at the flag? StartWalking(Direction::NorthWest); // Enter via the door - else if(GetPos() == building->GetPos()) // or are we at the building - building->AddActiveSoldier(world->RemoveFigure(pos, *this)); + else if(GetPos() == homeBld->GetPos()) // or are we at the building + homeBld->AddActiveSoldier(world->RemoveFigure(pos, *this)); else { - const auto dir = world->FindHumanPath(pos, building->GetFlagPos(), 100); + const auto dir = world->FindHumanPath(pos, homeBld->GetFlagPos(), 100); if(dir) { diff --git a/libs/s25main/figures/nofActiveSoldier.h b/libs/s25main/figures/nofActiveSoldier.h index 0c6ac27169..469826d7ee 100644 --- a/libs/s25main/figures/nofActiveSoldier.h +++ b/libs/s25main/figures/nofActiveSoldier.h @@ -133,7 +133,7 @@ class nofActiveSoldier : public nofSoldier SoldierState GetState() const { return state; } /// Set the home (building) to nullptr /// e.g. after the soldier was removed from the homes list but it was not destroyed - void ResetHome() { building = nullptr; } + void ResetHome() { homeBld = nullptr; } void FightVsDefenderStarted() { state = SoldierState::AttackingFightingVsDefender; } // For debugging diff --git a/libs/s25main/figures/nofAggressiveDefender.cpp b/libs/s25main/figures/nofAggressiveDefender.cpp index 00948fb785..ddffc771f3 100644 --- a/libs/s25main/figures/nofAggressiveDefender.cpp +++ b/libs/s25main/figures/nofAggressiveDefender.cpp @@ -72,12 +72,12 @@ void nofAggressiveDefender::Walked() void nofAggressiveDefender::HomeDestroyed() { - building = nullptr; + homeBld = nullptr; } void nofAggressiveDefender::HomeDestroyedAtBegin() { - building = nullptr; + homeBld = nullptr; // angegriffenem Gebäude Bescheid sagen, dass wir doch nicht mehr kommen InformTargetsAboutCancelling(); @@ -105,7 +105,7 @@ void nofAggressiveDefender::WonFighting() IncreaseRank(); // Ist evtl. unser Heimatgebäude zerstört? - if(!building) + if(!homeBld) { // Ziel Bescheid sagen InformTargetsAboutCancelling(); @@ -177,7 +177,7 @@ void nofAggressiveDefender::MissAggressiveDefendingContinueWalking() void nofAggressiveDefender::MissAggressiveDefendingWalk() { - if(!building) + if(!homeBld) { // Home destroyed so abort and wander around InformTargetsAboutCancelling(); diff --git a/libs/s25main/figures/nofAttacker.cpp b/libs/s25main/figures/nofAttacker.cpp index 4baf7b0504..987b13bc17 100644 --- a/libs/s25main/figures/nofAttacker.cpp +++ b/libs/s25main/figures/nofAttacker.cpp @@ -202,15 +202,15 @@ void nofAttacker::Walked() { RTTR_Assert(dynamic_cast(attacked_goal)); // We will now have this building as the new home, so inform old home, hunting soldier and ship - if(building) - building->SoldierLost(this); + if(homeBld) + homeBld->SoldierLost(this); CancelAtHuntingDefender(); if(ship_obj_id) CancelAtShip(); // Store the goal in a temporary as attacked_goal will be reset auto* goal = static_cast(attacked_goal); goal->Capture(player); - building = attacked_goal; + homeBld = attacked_goal; attacked_goal->AddActiveSoldier(world->RemoveFigure(pos, *this)); RemoveFromAttackedGoal(); // This might call other capturers @@ -292,7 +292,7 @@ void nofAttacker::HomeDestroyed() if(ship_obj_id) CancelAtShip(); - building = nullptr; + homeBld = nullptr; state = SoldierState::FigureWork; StartWandering(); Wander(); @@ -302,15 +302,15 @@ void nofAttacker::HomeDestroyed() } else { // If we were going back home, reset that goal - if(goal_ == building) + if(goal_ == homeBld) goal_ = nullptr; - building = nullptr; + homeBld = nullptr; } } void nofAttacker::HomeDestroyedAtBegin() { - building = nullptr; + homeBld = nullptr; // We are lost now and hence not targetable InformTargetsAboutCancelling(); @@ -327,7 +327,7 @@ void nofAttacker::WonFighting() IncreaseRank(); // If our home was destroyed then we are lost // unless we are currently fighting at the flag so that building can become our new home - if(!building && state != SoldierState::AttackingFightingVsDefender) + if(!homeBld && state != SoldierState::AttackingFightingVsDefender) { // Lost -> Tell all dependents InformTargetsAboutCancelling(); @@ -397,7 +397,7 @@ void nofAttacker::ReturnHomeMissionAttacking() void nofAttacker::MissAttackingWalk() { // If our home is destroyed we are lost - if(!building) + if(!homeBld) { InformTargetsAboutCancelling(); if(ship_obj_id) @@ -581,7 +581,7 @@ void nofAttacker::AttackedGoalDestroyed() auto* harbor = world->GetSpecObj(harborPos); RTTR_Assert(harbor); // go home - goal_ = building; + goal_ = homeBld; state = SoldierState::FigureWork; fs = FigureState::GotToGoal; harbor->CancelSeaAttacker(this); @@ -655,12 +655,12 @@ void nofAttacker::CapturingWalking() if(pos == attacked_goal->GetPos()) { // We switch buildings - if(building) - building->SoldierLost(this); + if(homeBld) + homeBld->SoldierLost(this); CancelAtHuntingDefender(); if(ship_obj_id) CancelAtShip(); - building = attacked_goal; + homeBld = attacked_goal; attacked_goal->AddActiveSoldier(world->RemoveFigure(pos, *this)); // No longer attacking @@ -687,7 +687,7 @@ void nofAttacker::CapturingWalking() StartWalking(Direction::NorthWest); RTTR_Assert(attacked_goal->GetPlayer() == player); // Assumed by the call below static_cast(attacked_goal)->NeedOccupyingTroops(); - } else if(!building) + } else if(!homeBld) { // If our home is destroyed we are lost and don't walk to the target (our new home if we were at least at the // flag already) Notify it, if it still exists (could be destroyed in the meantime too) @@ -845,7 +845,7 @@ void nofAttacker::StartReturnViaShip(noShip& ship) InformTargetsAboutCancelling(); } - goal_ = building; + goal_ = homeBld; state = SoldierState::FigureWork; fs = FigureState::GotToGoal; on_ship = true; @@ -883,7 +883,7 @@ void nofAttacker::CancelAtHuntingDefender() void nofAttacker::HandleState_SeaAttack_ReturnToShip() { - if(!building) + if(!homeBld) { // Home destroyed -> start wandering state = SoldierState::FigureWork; @@ -921,7 +921,7 @@ void nofAttacker::HandleState_SeaAttack_ReturnToShip() Wander(); // Notify home and ship - building->SoldierLost(this); + homeBld->SoldierLost(this); CancelAtShip(); } } diff --git a/libs/s25main/figures/nofDefender.cpp b/libs/s25main/figures/nofDefender.cpp index d68e9637f6..cdb67e0647 100644 --- a/libs/s25main/figures/nofDefender.cpp +++ b/libs/s25main/figures/nofDefender.cpp @@ -51,7 +51,7 @@ void nofDefender::Walked() attacker->FightVsDefenderStarted(); break; case SoldierState::DefendingWalkingFrom: - if(!building) + if(!homeBld) { // Home destroyed -> Start wandering around attacker = nullptr; @@ -68,7 +68,7 @@ void nofDefender::Walked() StartWalking(Direction::SouthEast); } else { - nobBaseMilitary* bld = building; + nobBaseMilitary* bld = homeBld; RTTR_Assert(bld->GetDefender() == this); // I should be the defender bld->AddActiveSoldier(world->RemoveFigure(pos, *this)); RTTR_Assert(!bld->GetDefender()); // No defender anymore @@ -81,7 +81,7 @@ void nofDefender::Walked() void nofDefender::HomeDestroyed() { - building = nullptr; + homeBld = nullptr; switch(state) { @@ -106,7 +106,7 @@ void nofDefender::HomeDestroyed() void nofDefender::HomeDestroyedAtBegin() { - building = nullptr; + homeBld = nullptr; state = SoldierState::FigureWork; @@ -122,7 +122,7 @@ void nofDefender::WonFighting() // Attacker is dead attacker = nullptr; - if(!building) + if(!homeBld) { // Home destroyed so abort and wander around state = SoldierState::FigureWork; @@ -132,7 +132,7 @@ void nofDefender::WonFighting() return; } - attacker = building->FindAttackerNearBuilding(); + attacker = homeBld->FindAttackerNearBuilding(); if(attacker) { // New attacker found so wait for him @@ -150,23 +150,23 @@ void nofDefender::LostFighting() attacker = nullptr; // Notify building if it still exists - if(building) + if(homeBld) { - building->ResetDefender(); + homeBld->ResetDefender(); // A military building potentially needs to get new soldiers if this wasn't the last one - if(BuildingProperties::IsMilitary(building->GetBuildingType())) + if(BuildingProperties::IsMilitary(homeBld->GetBuildingType())) { - RTTR_Assert(dynamic_cast(building)); - if(static_cast(building)->GetNumTroops()) - static_cast(building)->RegulateTroops(); + RTTR_Assert(dynamic_cast(homeBld)); + if(static_cast(homeBld)->GetNumTroops()) + static_cast(homeBld)->RegulateTroops(); } - building = nullptr; + homeBld = nullptr; } } void nofDefender::AttackerArrested() { - attacker = building->FindAttackerNearBuilding(); + attacker = homeBld->FindAttackerNearBuilding(); if(!attacker) { // Go back into the building diff --git a/libs/s25main/figures/nofPassiveSoldier.cpp b/libs/s25main/figures/nofPassiveSoldier.cpp index 4817c6aecd..fff9cb9d4d 100644 --- a/libs/s25main/figures/nofPassiveSoldier.cpp +++ b/libs/s25main/figures/nofPassiveSoldier.cpp @@ -97,7 +97,7 @@ void nofPassiveSoldier::Heal() void nofPassiveSoldier::GoalReached() { - static_cast(building)->AddPassiveSoldier(world->RemoveFigure(pos, *this)); + static_cast(homeBld)->AddPassiveSoldier(world->RemoveFigure(pos, *this)); } void nofPassiveSoldier::LeaveBuilding() @@ -105,16 +105,16 @@ void nofPassiveSoldier::LeaveBuilding() // Nach Hause in ein Lagerhaus gehen rs_dir = true; rs_pos = 1; - cur_rs = building->GetRoute(Direction::SouthEast); + cur_rs = homeBld->GetRoute(Direction::SouthEast); GoHome(); - building = nullptr; + homeBld = nullptr; } void nofPassiveSoldier::Upgrade() { // We must not be in the buildings list while upgrading. This would destroy the ordered list - RTTR_Assert(!building || !static_cast(building)->IsInTroops(*this)); + RTTR_Assert(!homeBld || !static_cast(homeBld)->IsInTroops(*this)); // Einen Rang höher job_ = Job(unsigned(job_) + 1); @@ -139,11 +139,11 @@ void nofPassiveSoldier::Walked() void nofPassiveSoldier::NotNeeded() { - building = nullptr; + homeBld = nullptr; GoHome(); } nobMilitary* nofPassiveSoldier::getHome() const { - return checkedCast(building); + return checkedCast(homeBld); } diff --git a/libs/s25main/figures/nofPassiveSoldier.h b/libs/s25main/figures/nofPassiveSoldier.h index 97a314ee97..4239c012f5 100644 --- a/libs/s25main/figures/nofPassiveSoldier.h +++ b/libs/s25main/figures/nofPassiveSoldier.h @@ -53,6 +53,6 @@ class nofPassiveSoldier : public nofSoldier /// Soldat befindet sich auf dem Hinweg zum Militärgebäude und wird nich länger gebraucht void NotNeeded(); /// Tells the soldier it is not in its home building anymore (e.g. died, or converted to attacker) - void LeftBuilding() { building = nullptr; } + void LeftBuilding() { homeBld = nullptr; } nobMilitary* getHome() const; }; diff --git a/libs/s25main/figures/nofSoldier.cpp b/libs/s25main/figures/nofSoldier.cpp index f98357ec2d..65866f08b5 100644 --- a/libs/s25main/figures/nofSoldier.cpp +++ b/libs/s25main/figures/nofSoldier.cpp @@ -12,7 +12,7 @@ nofSoldier::nofSoldier(const MapPoint pos, const unsigned char player, nobBaseMilitary* const goal, nobBaseMilitary* const home, const unsigned char rank, bool armor) - : noFigure(SOLDIER_JOBS[rank], pos, player, goal), building(home), hitpoints(HITPOINTS[rank]) + : noFigure(SOLDIER_JOBS[rank], pos, player, goal), homeBld(home), hitpoints(HITPOINTS[rank]) { hasArmor_ = armor; RTTR_Assert(IsSoldier()); @@ -20,7 +20,7 @@ nofSoldier::nofSoldier(const MapPoint pos, const unsigned char player, nobBaseMi nofSoldier::nofSoldier(const MapPoint pos, const unsigned char player, nobBaseMilitary& home, const unsigned char rank, bool armor) - : noFigure(SOLDIER_JOBS[rank], pos, player), building(&home), hitpoints(HITPOINTS[rank]) + : noFigure(SOLDIER_JOBS[rank], pos, player), homeBld(&home), hitpoints(HITPOINTS[rank]) { hasArmor_ = armor; RTTR_Assert(IsSoldier()); @@ -31,7 +31,7 @@ void nofSoldier::Serialize(SerializedGameData& sgd) const noFigure::Serialize(sgd); if(fs != FigureState::Wander && fs != FigureState::GoHome) - sgd.PushObject(building); + sgd.PushObject(homeBld); sgd.PushUnsignedChar(hitpoints); } @@ -41,9 +41,9 @@ nofSoldier::nofSoldier(SerializedGameData& sgd, const unsigned obj_id) : noFigur RTTR_Assert(IsSoldier()); if(fs != FigureState::Wander && fs != FigureState::GoHome) - building = sgd.PopObject(); + homeBld = sgd.PopObject(); else - building = nullptr; + homeBld = nullptr; hitpoints = sgd.PopUnsignedChar(); } @@ -58,11 +58,10 @@ void nofSoldier::DrawSoldierWaiting(DrawPoint drawPt) void nofSoldier::AbrogateWorkplace() { - // Militärgebäude Bescheid sagen, dass ich nicht kommen kann - if(building) + if(homeBld) { - building->SoldierLost(this); - building = nullptr; + homeBld->SoldierLost(this); + homeBld = nullptr; } } diff --git a/libs/s25main/figures/nofSoldier.h b/libs/s25main/figures/nofSoldier.h index 38e804ca0d..d897a6a989 100644 --- a/libs/s25main/figures/nofSoldier.h +++ b/libs/s25main/figures/nofSoldier.h @@ -9,19 +9,18 @@ class nobBaseMilitary; class SerializedGameData; -/// Basisklasse für alle Soldatentypen +/// Base class for all solider class nofSoldier : public noFigure { protected: - /// Heimatgebäude, ist bei Soldaten aus HQs das HQ! - nobBaseMilitary* building; - /// Hitpoints - unsigned char hitpoints; + /// Build to which this soldier belongs + nobBaseMilitary* homeBld; + uint8_t hitpoints; - /// Zeichnet den Soldaten beim ganz normalen Laufen + /// Draw during regular walking void DrawSoldierWaiting(DrawPoint drawPt); - /// wenn man beim Arbeitsplatz "kündigen" soll, man das Laufen zum Ziel unterbrechen muss (warum auch immer) + /// Tell workplace (i.e. home bld) that we are not coming back void AbrogateWorkplace() override; explicit nofSoldier(const nofSoldier&) = default; @@ -39,10 +38,9 @@ class nofSoldier : public noFigure } void Serialize(SerializedGameData& sgd) const override; - /// Liefert Rang des Soldaten unsigned char GetRank() const; unsigned char GetHitpoints() const; - bool HasNoHome() const { return building == nullptr; } + bool HasNoHome() const { return homeBld == nullptr; } }; /// Comparator to sort soldiers by rank and armor (and ID for ties), weak ones first From e12ce7b1dd6ea0d7062c229e4d23838ef6744489 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sun, 19 Apr 2026 20:36:21 +0200 Subject: [PATCH 3/6] Only stop own soldiers from leaving attacked building The idea is to potentially use those for defense. However e.g. harbors might contain soldiers that belong to another building and are about to leave. Don't consider those as this complicates the logic too much. Currently it crashed because adding "active" soldiers is assumed to be either the defender or a soldier on a mission from this building. Other soldiers will get lost in the accounting of their home building. Fixes #1912 --- libs/s25main/buildings/nobBaseMilitary.cpp | 11 ++++++++--- libs/s25main/figures/nofSoldier.h | 1 + 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/libs/s25main/buildings/nobBaseMilitary.cpp b/libs/s25main/buildings/nobBaseMilitary.cpp index 5c64582413..2ee6539d48 100644 --- a/libs/s25main/buildings/nobBaseMilitary.cpp +++ b/libs/s25main/buildings/nobBaseMilitary.cpp @@ -224,9 +224,8 @@ bool nobBaseMilitary::CallDefender(nofAttacker& attacker) // Stop attacking soldiers (including aggressive defenders) from leaving this building for(auto it = leave_house.begin(); it != leave_house.end();) { - if(!dynamic_cast(it->get())) - ++it; - else + const auto* sldPtr = dynamic_cast(it->get()); + if(sldPtr && helpers::contains(troops_on_mission, sldPtr)) { auto soldier = boost::static_pointer_cast(std::move(*it)); // At this point there shouldn't be a defender leaving as we are just requesting one @@ -236,6 +235,12 @@ bool nobBaseMilitary::CallDefender(nofAttacker& attacker) soldier->InformTargetsAboutCancelling(); AddActiveSoldier(std::move(soldier)); + } else + { + // If a leaving active soldier is not on a mission he doesn't belong to this building. + // E.g. soldiers in harbors could be about to go home. + RTTR_Assert(!sldPtr || sldPtr->GetHomeBld() != this); + ++it; } } // Use existing defender (e.g. just going back in) if possible diff --git a/libs/s25main/figures/nofSoldier.h b/libs/s25main/figures/nofSoldier.h index d897a6a989..123c41da2d 100644 --- a/libs/s25main/figures/nofSoldier.h +++ b/libs/s25main/figures/nofSoldier.h @@ -41,6 +41,7 @@ class nofSoldier : public noFigure unsigned char GetRank() const; unsigned char GetHitpoints() const; bool HasNoHome() const { return homeBld == nullptr; } + const nobBaseMilitary* GetHomeBld() const { return homeBld; } }; /// Comparator to sort soldiers by rank and armor (and ID for ties), weak ones first From d1e895d088669c20890be6c7105fca2de342d54d Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sun, 19 Apr 2026 20:46:52 +0200 Subject: [PATCH 4/6] Fix missed armor setting when providing defender out of sea attackers --- libs/s25main/buildings/nobHarborBuilding.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/s25main/buildings/nobHarborBuilding.cpp b/libs/s25main/buildings/nobHarborBuilding.cpp index f35ac54c0f..51c4b8f56f 100644 --- a/libs/s25main/buildings/nobHarborBuilding.cpp +++ b/libs/s25main/buildings/nobHarborBuilding.cpp @@ -1339,6 +1339,7 @@ std::unique_ptr nobHarborBuilding::ProvideDefender(nofAttacker& att std::unique_ptr defender_attacker = std::move(soldiers_for_ships.begin()->attacker); soldiers_for_ships.pop_front(); defender = std::make_unique(pos, player, *this, defender_attacker->GetRank(), attacker); + defender->SetArmor(defender_attacker->HasArmor()); defender_attacker->CancelSeaAttack(); defender_attacker->Destroy(); } From 913f4ffb8b48fb15c91b524ce6c72b6014b16752 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sun, 19 Apr 2026 21:04:15 +0200 Subject: [PATCH 5/6] Consolidate handling of adding/removing people from warehouse inventory It combines handling of armor and boat carriers. --- libs/s25main/buildings/nobBaseWarehouse.cpp | 85 +++++++++----------- libs/s25main/buildings/nobBaseWarehouse.h | 5 +- libs/s25main/buildings/nobHarborBuilding.cpp | 45 ++--------- 3 files changed, 45 insertions(+), 90 deletions(-) diff --git a/libs/s25main/buildings/nobBaseWarehouse.cpp b/libs/s25main/buildings/nobBaseWarehouse.cpp index 326893d833..bb5068110a 100644 --- a/libs/s25main/buildings/nobBaseWarehouse.cpp +++ b/libs/s25main/buildings/nobBaseWarehouse.cpp @@ -676,15 +676,7 @@ void nobBaseWarehouse::FigureLeft(const noFigure& fig) // Adapt inventory for non-warehouse workers if(fig.MemberOfWarehouse()) return; - if(fig.GetJobType() == Job::BoatCarrier) - { - // Remove helper and boat separately - inventory.visual.Remove(Job::Helper); - inventory.visual.Remove(GoodType::Boat); - } else - inventory.visual.Remove(fig.GetJobType()); - - RemoveArmoredFigurFromVisualInventory(fig); + RemoveFromVisualInventory(fig); if(fig.GetGOT() == GO_Type::NofTradedonkey) { @@ -839,38 +831,9 @@ void nobBaseWarehouse::AddFigure(std::unique_ptr figure, const bool in // Warenhausarbeiter werden nicht gezählt! if(!figure->MemberOfWarehouse()) { - // War das ein Boot-Träger? - if(figure->GetJobType() == Job::BoatCarrier) - { - if(increase_visual_counts) - { - inventory.Add(Job::Helper); - inventory.Add(GoodType::Boat); - } else - { - inventory.real.Add(Job::Helper); - inventory.real.Add(GoodType::Boat); - } - } else if(isSoldier(figure->GetJobType())) - { - if(increase_visual_counts) - { - inventory.Add(figure->GetJobType()); - if(figure->HasArmor()) - inventory.Add(jobEnumToAmoredSoldierEnum(figure->GetJobType())); - } else - { - inventory.real.Add(figure->GetJobType()); - if(figure->HasArmor()) - inventory.real.Add(jobEnumToAmoredSoldierEnum(figure->GetJobType())); - } - } else - { - if(increase_visual_counts) - inventory.Add(figure->GetJobType()); - else - inventory.real.Add(figure->GetJobType()); - } + AddToRealInventory(*figure); + if(increase_visual_counts) + AddToVisualInventory(*figure); } // Check if we were actually waiting for this figure or if it was just added (e.g. builder that constructed it) to @@ -882,19 +845,45 @@ void nobBaseWarehouse::AddFigure(std::unique_ptr figure, const bool in GetEvMgr().AddToKillList(std::move(figure)); } -void nobBaseWarehouse::RemoveArmoredFigurFromVisualInventory(const noFigure& figure) +void nobBaseWarehouse::RemoveFromVisualInventory(const noFigure& figure) { - if(isSoldier(figure.GetJobType()) && figure.HasArmor()) + const auto job = figure.GetJobType(); + if(job == Job::BoatCarrier) { - RTTR_Assert(inventory.visual[jobEnumToAmoredSoldierEnum(figure.GetJobType())] > 0); - inventory.visual.Remove(jobEnumToAmoredSoldierEnum(figure.GetJobType())); + // Remove helper and boat separately + inventory.visual.Remove(Job::Helper); + inventory.visual.Remove(GoodType::Boat); + } else + { + inventory.visual.Remove(job); + if(isSoldier(job) && figure.HasArmor()) + inventory.visual.Remove(jobEnumToAmoredSoldierEnum(job)); } } -void nobBaseWarehouse::AddArmoredFigurToVisualInventory(const noFigure& figure) +static void addToInventory(Inventory& inventory, const noFigure& figure) +{ + const auto job = figure.GetJobType(); + if(job == Job::BoatCarrier) + { + inventory.Add(Job::Helper); + inventory.Add(GoodType::Boat); + } else + { + inventory.Add(job); + if(isSoldier(job) && figure.HasArmor()) + inventory.Add(jobEnumToAmoredSoldierEnum(job)); + } +} + +void nobBaseWarehouse::AddToVisualInventory(const noFigure& figure) +{ + addToInventory(inventory.visual, figure); +} + +void nobBaseWarehouse::AddToRealInventory(const noFigure& figure) { - if(isSoldier(figure.GetJobType()) && figure.HasArmor()) - inventory.visual.Add(jobEnumToAmoredSoldierEnum(figure.GetJobType())); + addToInventory(inventory.real, figure); } void nobBaseWarehouse::FetchWare() diff --git a/libs/s25main/buildings/nobBaseWarehouse.h b/libs/s25main/buildings/nobBaseWarehouse.h index 2cf85eb9f0..75cda8f81f 100644 --- a/libs/s25main/buildings/nobBaseWarehouse.h +++ b/libs/s25main/buildings/nobBaseWarehouse.h @@ -125,8 +125,9 @@ class nobBaseWarehouse : public nobBaseMilitary, public DataChangedObservable /// Recruts a worker of the given job if possible bool TryRecruitJob(Job job); - void RemoveArmoredFigurFromVisualInventory(const noFigure& figure); - void AddArmoredFigurToVisualInventory(const noFigure& figure); + void RemoveFromVisualInventory(const noFigure& figure); + void AddToVisualInventory(const noFigure& figure); + void AddToRealInventory(const noFigure& figure); nobBaseWarehouse(BuildingType type, MapPoint pos, unsigned char player, Nation nation); nobBaseWarehouse(SerializedGameData& sgd, unsigned obj_id); diff --git a/libs/s25main/buildings/nobHarborBuilding.cpp b/libs/s25main/buildings/nobHarborBuilding.cpp index 51c4b8f56f..83020f9ddd 100644 --- a/libs/s25main/buildings/nobHarborBuilding.cpp +++ b/libs/s25main/buildings/nobHarborBuilding.cpp @@ -515,8 +515,7 @@ void nobHarborBuilding::ShipArrived(noShip& ship) { if(it->dest == ship_dest) { - inventory.visual.Remove(it->attacker->GetJobType()); - RemoveArmoredFigurFromVisualInventory(*it->attacker); + RemoveFromVisualInventory(*it->attacker); attackers.push_back(std::move(it->attacker)); it = soldiers_for_ships.erase(it); } else @@ -589,15 +588,7 @@ void nobHarborBuilding::ShipArrived(noShip& ship) if(it->dest == dest) { it->fig->StartShipJourney(); - if(it->fig->GetJobType() != Job::BoatCarrier) - { - inventory.visual.Remove(it->fig->GetJobType()); - RemoveArmoredFigurFromVisualInventory(*it->fig); - } else - { - inventory.visual.Remove(Job::Helper); - inventory.visual.Remove(GoodType::Boat); - } + RemoveFromVisualInventory(*it->fig); figures.push_back(std::move(it->fig)); it = figures_for_ships.erase(it); } else @@ -867,16 +858,7 @@ std::vector nobHarborBuilding::GetShipConnect void nobHarborBuilding::AddFigureForShip(std::unique_ptr fig, MapPoint dest) { RTTR_Assert(!world->HasFigureAt(fig->GetPos(), *fig)); // Figure is in the harbor, so it cannot be outside - // Anzahl visuell erhöhen - if(fig->GetJobType() != Job::BoatCarrier) - { - inventory.visual.Add(fig->GetJobType()); - AddArmoredFigurToVisualInventory(*fig); - } else - { - inventory.visual.Add(Job::Helper); - inventory.visual.Add(GoodType::Boat); - } + AddToVisualInventory(*fig); figures_for_ships.emplace_back(FigureForShip{std::move(fig), dest}); OrderShip(); } @@ -1027,15 +1009,7 @@ bool nobHarborBuilding::UseFigureAtOnce(std::unique_ptr& fig, noRoadNo { // Reduce figure count because figures don't go through the house leaving process // And therefore the visual count reducement - if(fig->GetJobType() != Job::BoatCarrier) - { - inventory.visual.Remove(fig->GetJobType()); - RemoveArmoredFigurFromVisualInventory(*fig); - } else - { - inventory.visual.Remove(Job::Helper); - inventory.visual.Remove(GoodType::Boat); - } + RemoveFromVisualInventory(*fig); // Dann fügen wir die mal bei uns hinzu AddFigureForShip(std::move(fig), next_harbor); return true; @@ -1069,16 +1043,7 @@ void nobHarborBuilding::ReceiveGoodsFromShip(std::list if(nextDir == RoadPathDirection::SouthEast) { - // Increase visual count - if(figure->GetJobType() == Job::BoatCarrier) - { - inventory.visual.Add(Job::Helper); - inventory.visual.Add(GoodType::Boat); - } else - { - inventory.visual.Add(figure->GetJobType()); - AddArmoredFigurToVisualInventory(*figure); - } + AddToVisualInventory(*figure); AddLeavingFigure(std::move(figure)); } else if(nextDir == RoadPathDirection::Ship) { From 956319737fa0c714805ebd8e80da151e75be2ef2 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 20 Apr 2026 10:32:36 +0200 Subject: [PATCH 6/6] Remove `HasNoHome` and `HasNoGoal` methods Replaced by `GetHome` & `GetGoal` --- libs/s25main/buildings/nobHarborBuilding.cpp | 12 ++++++------ libs/s25main/figures/noFigure.cpp | 4 ++-- libs/s25main/figures/noFigure.h | 3 --- libs/s25main/figures/nofSoldier.h | 3 +-- tests/s25Main/integration/testAttacking.cpp | 4 ++-- tests/s25Main/integration/testSeaAttacking.cpp | 2 +- 6 files changed, 12 insertions(+), 16 deletions(-) diff --git a/libs/s25main/buildings/nobHarborBuilding.cpp b/libs/s25main/buildings/nobHarborBuilding.cpp index 83020f9ddd..ccbd238984 100644 --- a/libs/s25main/buildings/nobHarborBuilding.cpp +++ b/libs/s25main/buildings/nobHarborBuilding.cpp @@ -128,8 +128,8 @@ void nobHarborBuilding::DestroyBuilding() soldier.CancelSeaAttack(); RTTR_Assert(!soldier.GetAttackedGoal()); - RTTR_Assert(soldier.HasNoHome()); - RTTR_Assert(soldier.HasNoGoal()); + RTTR_Assert(!soldier.GetHomeBld()); + RTTR_Assert(!soldier.GetGoal()); soldier.StartWandering(); soldier.StartWalking(RANDOM_ENUM(Direction)); } @@ -1032,7 +1032,7 @@ void nobHarborBuilding::ReceiveGoodsFromShip(std::list { figure->SetGoalTonullptr(); AddFigure(std::move(figure), true); - } else if(figure->HasNoGoal()) + } else if(!figure->GetGoal()) { AddDependentFigure(*figure); // No goal? We take it AddFigure(std::move(figure), true); @@ -1212,8 +1212,8 @@ void nobHarborBuilding::AddSeaAttacker(std::unique_ptr attacker) // notify target about noShow, notify home that soldier wont return, add to inventory attacker->SeaAttackFailedBeforeLaunch(); // set state, remove target & home RTTR_Assert(!attacker->GetAttackedGoal()); - RTTR_Assert(attacker->HasNoHome()); - RTTR_Assert(attacker->HasNoGoal()); + RTTR_Assert(!attacker->GetHomeBld()); + RTTR_Assert(!attacker->GetGoal()); AddFigure(std::move(attacker), true); return; } @@ -1232,7 +1232,7 @@ void nobHarborBuilding::CancelSeaAttacker(nofAttacker* attacker) helpers::find_if(soldiers_for_ships, [attacker](const auto& it) { return it.attacker.get() == attacker; }); RTTR_Assert(it != soldiers_for_ships.end()); - if(attacker->HasNoGoal()) + if(!attacker->GetGoal()) { // No goal? We take it AddDependentFigure(*attacker); diff --git a/libs/s25main/figures/noFigure.cpp b/libs/s25main/figures/noFigure.cpp index 3d3dee69bd..f4df49958e 100644 --- a/libs/s25main/figures/noFigure.cpp +++ b/libs/s25main/figures/noFigure.cpp @@ -67,7 +67,7 @@ noFigure::noFigure(const Job job, const MapPoint pos, const unsigned char player void noFigure::Destroy() { - RTTR_Assert(HasNoGoal()); + RTTR_Assert(!goal_); RTTR_Assert(!cur_rs); noMovable::Destroy(); @@ -455,7 +455,7 @@ void noFigure::GoHome(noRoadNode* goal) void noFigure::StartWandering(const unsigned burned_wh_id) { - RTTR_Assert(HasNoGoal()); + RTTR_Assert(!goal_); fs = FigureState::Wander; cur_rs = nullptr; rs_pos = 0; diff --git a/libs/s25main/figures/noFigure.h b/libs/s25main/figures/noFigure.h index ce41045edf..b52a20c630 100644 --- a/libs/s25main/figures/noFigure.h +++ b/libs/s25main/figures/noFigure.h @@ -235,9 +235,6 @@ class noFigure : public noMovable void StartShipJourney(); /// Tells the figure it arrived at a harbor at the given position void ArrivedByShip(MapPoint harborPos); - /// Gibt zurück, ob die Figur kein Ziel mehr hat und damit nach einer Schifffahrt im - /// Lagerhaus interniert werden muss - bool HasNoGoal() const { return (goal_ == nullptr); } /// Gibt zurück, ob die Figur auf Straßen läuft zu ihrem Arbeitsplatz o.Ä. bool IsWalkingOnRoad() const { diff --git a/libs/s25main/figures/nofSoldier.h b/libs/s25main/figures/nofSoldier.h index 123c41da2d..de77dc405e 100644 --- a/libs/s25main/figures/nofSoldier.h +++ b/libs/s25main/figures/nofSoldier.h @@ -33,14 +33,13 @@ class nofSoldier : public noFigure void Destroy() override { - RTTR_Assert(HasNoHome()); + RTTR_Assert(!GetHomeBld()); noFigure::Destroy(); } void Serialize(SerializedGameData& sgd) const override; unsigned char GetRank() const; unsigned char GetHitpoints() const; - bool HasNoHome() const { return homeBld == nullptr; } const nobBaseMilitary* GetHomeBld() const { return homeBld; } }; diff --git a/tests/s25Main/integration/testAttacking.cpp b/tests/s25Main/integration/testAttacking.cpp index 19513902c5..975d9c4cc1 100644 --- a/tests/s25Main/integration/testAttacking.cpp +++ b/tests/s25Main/integration/testAttacking.cpp @@ -143,7 +143,7 @@ struct AttackFixtureBase : public WorldWithGCExecution Already reached -> Added and all internal states set correctly soldier.WalkToGoal(); - BOOST_TEST_REQUIRE(soldier.HasNoGoal()); + BOOST_TEST_REQUIRE(!soldier.GetGoal()); } BOOST_TEST_REQUIRE(bld->GetNumTroops() == oldNumSoldiers + numSoldiers); } @@ -754,7 +754,7 @@ BOOST_FIXTURE_TEST_CASE(ConquerWithMultipleWalkingIn, AttackFixture4P) // 1. Attackers from this building // No home -> Wander - BOOST_TEST_REQUIRE(attackerFromPl0.HasNoHome()); + BOOST_TEST_REQUIRE(!attackerFromPl0.GetHomeBld()); rescheduleWalkEvent(em, attackerFromPl0, 1); RTTR_EXEC_TILL(2, attackerFromPl0.IsWandering()); // 2. Aggressive defenders from this building diff --git a/tests/s25Main/integration/testSeaAttacking.cpp b/tests/s25Main/integration/testSeaAttacking.cpp index 632913cb8e..0d2ff3c5df 100644 --- a/tests/s25Main/integration/testSeaAttacking.cpp +++ b/tests/s25Main/integration/testSeaAttacking.cpp @@ -180,7 +180,7 @@ struct SeaAttackFixture : public SeaWorldWithGCExecution<3, 62, 64> world.GetPlayer(bld->GetPlayer()).IncreaseInventoryJob(soldier.GetJobType(), 1); // Let him "walk" to goal -> Already reached -> Added and all internal states set correctly soldier.WalkToGoal(); - BOOST_TEST_REQUIRE(soldier.HasNoGoal()); + BOOST_TEST_REQUIRE(!soldier.GetGoal()); } BOOST_TEST_REQUIRE(bld->GetNumTroops() == oldNumSoldiers + numSoldiers); }