Fix crash when asking for defenders of harbor with waiting attackers

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<nofActiveSoldier*>(it->get()))
-            ++it;
-        else
+        const auto* sldPtr = dynamic_cast<nofActiveSoldier*>(it->get());
+        if(sldPtr && helpers::contains(troops_on_mission, sldPtr))
         {
             auto soldier = boost::static_pointer_cast<nofActiveSoldier>(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

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<noFigure> 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<noFigure> 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()

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);

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));
     }
@@ -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::ShipConnection> nobHarborBuilding::GetShipConnect
 void nobHarborBuilding::AddFigureForShip(std::unique_ptr<noFigure> 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<noFigure>& 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;
@@ -1058,7 +1032,7 @@ void nobHarborBuilding::ReceiveGoodsFromShip(std::list<std::unique_ptr<noFigure>
         {
             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);
@@ -1069,16 +1043,7 @@ void nobHarborBuilding::ReceiveGoodsFromShip(std::list<std::unique_ptr<noFigure>
 
             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)
             {
@@ -1247,8 +1212,8 @@ void nobHarborBuilding::AddSeaAttacker(std::unique_ptr<nofAttacker> 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;
     }
@@ -1267,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);
@@ -1339,6 +1304,7 @@ std::unique_ptr<nofDefender> nobHarborBuilding::ProvideDefender(nofAttacker& att
         std::unique_ptr<nofAttacker> defender_attacker = std::move(soldiers_for_ships.begin()->attacker);
         soldiers_for_ships.pop_front();
         defender = std::make_unique<nofDefender>(pos, player, *this, defender_attacker->GetRank(), attacker);
+        defender->SetArmor(defender_attacker->HasArmor());
         defender_attacker->CancelSeaAttack();
         defender_attacker->Destroy();
     }

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;

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
     {

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<nobMilitary>(this->GetPos());
-        if(building)
+        homeBld = world->GetSpecObj<nobMilitary>(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)
         {

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