Check warehouse acceptance before cancelling ordered troops

Author: DevOpsOfChaos

libs/s25main/buildings/nobMilitary.cpp

@@ -514,32 +514,38 @@ void nobMilitary::RegulateTroops()
     for(unsigned rank = 0; rank < NUM_SOLDIER_RANKS; rank++)
     {
         int excess = counts[rank] - troop_limits[rank];
+        const bool canReturnHome =
+          excess > 0
+          && world->GetPlayer(player).FindWarehouse(*this, FW::AcceptsFigure(SOLDIER_JOBS[rank]), true, false);
+
         if(excess > 0)
         {
             lack[rank] = 0;
 
-            std::vector<nofPassiveSoldier*> notNeededSoldiers;
-            for(auto it = ordered_troops.begin(); excess && it != ordered_troops.end();)
+            if(canReturnHome)
             {
-                if((*it)->GetRank() == rank)
-                {
-                    notNeededSoldiers.push_back(*it);
-                    it = ordered_troops.erase(it);
-                    --excess;
-                } else
+                std::vector<nofPassiveSoldier*> notNeededSoldiers;
+                for(auto it = ordered_troops.begin(); excess && it != ordered_troops.end();)
                 {
-                    ++it;
+                    if((*it)->GetRank() == rank)
+                    {
+                        notNeededSoldiers.push_back(*it);
+                        it = ordered_troops.erase(it);
+                        --excess;
+                    } else
+                    {
+                        ++it;
+                    }
                 }
+                for(auto* notNeededSoldier : notNeededSoldiers)
+                    notNeededSoldier->NotNeeded();
             }
-            for(auto* notNeededSoldier : notNeededSoldiers)
-                notNeededSoldier->NotNeeded();
         } else
         {
             // This bit is for ordering troops later
             lack[rank] = troop_limits[rank] - counts[rank];
         }
-        if(excess > 0
-           && world->GetPlayer(player).FindWarehouse(*this, FW::AcceptsFigure(SOLDIER_JOBS[rank]), true, false))
+        if(excess > 0 && canReturnHome)
         {
             for(auto it = troops.begin(); excess && it != troops.end() && troops.size() > 1;)
             {

tests/s25Main/integration/testGameCommands.cpp

@@ -525,6 +525,41 @@ BOOST_FIXTURE_TEST_CASE(SendSoldiersHomeTest, WorldWithGCExecution2P)
         BOOST_TEST_REQUIRE(itTroops->GetRank() == i);
 }
 
+BOOST_FIXTURE_TEST_CASE(OrderedSoldierKeepsMilitaryGoalWhenReturnWarehouseRejectsRank, WorldWithGCExecution2P)
+{
+    initGameRNG();
+
+    const MapPoint milPt = hqPos + MapPoint(4, 0);
+    GamePlayer& player = world.GetPlayer(curPlayer);
+    nobBaseWarehouse* wh = player.GetFirstWH();
+    BOOST_TEST_REQUIRE(wh);
+    BOOST_TEST_REQUIRE(wh->GetInventory().people[Job::General] == 0u);
+
+    wh->AddToInventory(PeopleCounts::make(Job::General, 1), true);
+
+    for(unsigned i = 0; i <= this->ggs.GetMaxMilitaryRank(); ++i)
+        this->ChangeReserve(hqPos, i, 0);
+
+    this->ChangeMilitary(MILITARY_SETTINGS_SCALE);
+
+    auto* bld = dynamic_cast<nobMilitary*>(
+      BuildingFactory::CreateBuilding(world, BuildingType::Watchtower, milPt, curPlayer, player.nation));
+    BOOST_TEST_REQUIRE(bld);
+
+    this->BuildRoad(world.GetNeighbour(hqPos, Direction::SouthEast), false,
+                    std::vector<Direction>((milPt.x - hqPos.x), Direction::East));
+
+    RTTR_SKIP_GFS(40);
+    BOOST_TEST_REQUIRE(wh->GetNumRealFigures(Job::General) == 0u);
+
+    this->SetInventorySetting(hqPos, Job::General, EInventorySetting::Stop);
+    this->SetTroopLimit(milPt, getSoldierRank(Job::General), 0);
+
+    RTTR_EXEC_TILL(1000, bld->GetNumTroops() == 1u);
+    BOOST_TEST_REQUIRE(bld->GetNumTroops() == 1u);
+    BOOST_TEST_REQUIRE(bld->GetTroops().front().GetRank() == getSoldierRank(Job::General));
+    BOOST_TEST_REQUIRE(bld->GetLeavingFigures().empty());
+}
 namespace {
 void FlagWorkerTest(WorldWithGCExecution2P& worldFixture, Job workerJob, GoodType toolType)
 {