From c12a18c9b3a6580cdbcb70c71fc67b838eebf441 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 24 Mar 2026 18:44:13 +0100 Subject: [PATCH 01/19] Ignore pause message in replay mode This is especially important during skipping which expects that each gameframe is actually executed. --- libs/s25main/ingameWindows/iwMissionStatement.cpp | 4 ++-- libs/s25main/network/GameClient.cpp | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/libs/s25main/ingameWindows/iwMissionStatement.cpp b/libs/s25main/ingameWindows/iwMissionStatement.cpp index ada7bcf110..18cb0414a5 100644 --- a/libs/s25main/ingameWindows/iwMissionStatement.cpp +++ b/libs/s25main/ingameWindows/iwMissionStatement.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 @@ -13,7 +13,7 @@ iwMissionStatement::iwMissionStatement(const std::string& title, const std::stri HelpImage image) : IngameWindow(CGI_MISSION_STATEMENT, IngameWindow::posLastOrCenter, Extent(640, 480), title, LOADER.GetImageN("io", 5), true, CloseBehavior::Custom), - pauseGame_(pauseGame) + pauseGame_(pauseGame && !GAMECLIENT.IsReplayModeOn()) { glArchivItem_Bitmap* img = (image == IM_NONE) ? nullptr : LOADER.GetImageN("io", image); const Extent imgSize(img ? img->GetSize() : Extent::all(0)); diff --git a/libs/s25main/network/GameClient.cpp b/libs/s25main/network/GameClient.cpp index da25d038ad..f467577059 100644 --- a/libs/s25main/network/GameClient.cpp +++ b/libs/s25main/network/GameClient.cpp @@ -1223,14 +1223,10 @@ bool GameClient::OnGameMessage(const GameMessage_Server_NWFDone& msg) return true; } -/** - * Pause-Nachricht von Server - * - * @param[in] message Nachricht, welche ausgeführt wird - */ bool GameClient::OnGameMessage(const GameMessage_Pause& msg) { - if(state != ClientState::Game) + // Ignore recorded pause messages in replay mode + if(state != ClientState::Game || replayMode) return true; if(framesinfo.isPaused == msg.paused) return true; @@ -1660,6 +1656,8 @@ void GameClient::SkipGF(unsigned gf, GameWorldView& gwv) } SetPause(false); + if(GetGFNumber() < GetLastReplayGF()) + gf = std::min(gf, GetLastReplayGF() + 1u); skiptogf = gf; // GFs überspringen @@ -1683,6 +1681,8 @@ void GameClient::SkipGF(unsigned gf, GameWorldView& gwv) } ExecuteGameFrame(); } + // Either we force-stopped skipping (skiptogf set to 0) or we reached the target gf + RTTR_Assert(skiptogf == 0u || GetGFNumber() == gf); // Spiel pausieren & text ausgabe wie lang das jetzt gedauert hat unsigned ticks = VIDEODRIVER.GetTickCount() - start_ticks; @@ -1755,11 +1755,11 @@ void GameClient::SetPause(bool pause) framesinfo.frameTime = FramesInfo::milliseconds32_t::zero(); } else if(replayMode) { + // Pause instantly framesinfo.isPaused = pause; framesinfo.frameTime = FramesInfo::milliseconds32_t::zero(); } else if(IsHost()) { - // Pause instantly auto* msg = new GameMessage_Pause(pause); if(pause) OnGameMessage(*msg); From 4d7cf73e37e1b96de2f53b0cb5f238be1e607cc2 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 27 Mar 2026 18:19:35 +0100 Subject: [PATCH 02/19] Don't serialize unused value of nobBaseMilitary --- libs/s25main/SerializedGameData.cpp | 5 +++-- libs/s25main/buildings/nobBaseMilitary.cpp | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/libs/s25main/SerializedGameData.cpp b/libs/s25main/SerializedGameData.cpp index cbfdab34f8..d90258cd09 100644 --- a/libs/s25main/SerializedGameData.cpp +++ b/libs/s25main/SerializedGameData.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2005 - 2024 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 @@ -107,7 +107,8 @@ /// 11: wineaddon added, three new building types and two new goods /// 12: leatheraddon added, three new building types and three new goods /// 13: SeaId & HarborId: World::harborData w/o dummy entry at 0 -static const unsigned currentGameDataVersion = 13; +/// 14: Remove "age" field in nobBaseMilitary +static const unsigned currentGameDataVersion = 14; // clang-format on std::unique_ptr SerializedGameData::Create_GameObject(const GO_Type got, const unsigned obj_id) diff --git a/libs/s25main/buildings/nobBaseMilitary.cpp b/libs/s25main/buildings/nobBaseMilitary.cpp index afc1de22e1..fd280f43c8 100644 --- a/libs/s25main/buildings/nobBaseMilitary.cpp +++ b/libs/s25main/buildings/nobBaseMilitary.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 @@ -96,7 +96,6 @@ void nobBaseMilitary::Serialize(SerializedGameData& sgd) const sgd.PushObjectContainer(leave_house); sgd.PushEvent(leaving_event); sgd.PushBool(go_out); - sgd.PushUnsignedInt(0); // former age, compatibility with 0.7, remove it in furher versions sgd.PushObjectContainer(troops_on_mission); sgd.PushObjectContainer(aggressors, true); sgd.PushObjectContainer(aggressive_defenders, true); @@ -108,7 +107,8 @@ nobBaseMilitary::nobBaseMilitary(SerializedGameData& sgd, const unsigned obj_id) sgd.PopObjectContainer(leave_house); leaving_event = sgd.PopEvent(); go_out = sgd.PopBool(); - sgd.PopUnsignedInt(); // former age, compatibility with 0.7, remove it in furher versions + if(sgd.GetGameDataVersion() < 14) + sgd.PopUnsignedInt(); // former age sgd.PopObjectContainer(troops_on_mission); sgd.PopObjectContainer(aggressors, GO_Type::NofAttacker); sgd.PopObjectContainer(aggressive_defenders, GO_Type::NofAggressivedefender); From ad69449b440db94c0b2a002d07bf49339d71eb22 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 27 Mar 2026 18:19:47 +0100 Subject: [PATCH 03/19] Translate some comments/docstrings --- libs/s25main/buildings/nobBaseMilitary.cpp | 63 +++++++-------------- libs/s25main/buildings/nobBaseMilitary.h | 7 +-- libs/s25main/buildings/nobBaseWarehouse.cpp | 48 +++++----------- libs/s25main/buildings/nobBaseWarehouse.h | 2 +- libs/s25main/buildings/nobMilitary.cpp | 2 +- libs/s25main/figures/noFigure.h | 5 +- libs/s25main/figures/nofActiveSoldier.cpp | 5 +- 7 files changed, 43 insertions(+), 89 deletions(-) diff --git a/libs/s25main/buildings/nobBaseMilitary.cpp b/libs/s25main/buildings/nobBaseMilitary.cpp index fd280f43c8..e79a2a55cc 100644 --- a/libs/s25main/buildings/nobBaseMilitary.cpp +++ b/libs/s25main/buildings/nobBaseMilitary.cpp @@ -30,44 +30,42 @@ nobBaseMilitary::~nobBaseMilitary() = default; void nobBaseMilitary::DestroyBuilding() { - // Soldaten Bescheid sagen, die evtl auf Mission sind + // Notify soldiers on mission. // ATTENTION: iterators can be deleted in HomeDestroyed, -> copy first std::vector tmpTroopsOnMission(troops_on_mission.begin(), troops_on_mission.end()); for(auto* it : tmpTroopsOnMission) it->HomeDestroyed(); troops_on_mission.clear(); - // Und die, die das Gebäude evtl gerade angreifen + // Notify soldiers attacking this building // ATTENTION: iterators can be deleted in AttackedGoalDestroyed, -> copy first std::vector tmpAggressors(aggressors.begin(), aggressors.end()); for(auto* tmpAggressor : tmpAggressors) tmpAggressor->AttackedGoalDestroyed(); aggressors.clear(); - // Aggressiv-Verteidigenden Soldaten Bescheid sagen, dass sie nach Hause gehen können + // Notify soldiers from other buildings defending this std::vector tmpDefenders(aggressive_defenders.begin(), aggressive_defenders.end()); for(auto* tmpDefender : tmpDefenders) tmpDefender->AttackedGoalDestroyed(); aggressive_defenders.clear(); - // Verteidiger Bescheid sagen if(defender_) { defender_->HomeDestroyed(); defender_ = nullptr; } - // Warteschlangenevent vernichten GetEvMgr().RemoveEvent(leaving_event); - // Soldaten, die noch in der Warteschlange hängen, rausschicken + // Put all leaving figures outside for(auto& fig : leave_house) { noFigure& figRef = world->AddFigure(pos, std::move(fig)); - if(figRef.DoJobWorks() && dynamic_cast(&figRef)) - // Wenn er Job-Arbeiten verrichtet, ists ein ActiveSoldier oder TradeDonkey --> dem Soldat muss extra noch - // Bescheid gesagt werden! + // DoJobWorks implies ActiveSoldier or TradeDonkey + // Notify the soldier + if(figRef.IsDoingJobWorks() && dynamic_cast(&figRef)) static_cast(figRef).HomeDestroyedAtBegin(); else { @@ -79,8 +77,7 @@ void nobBaseMilitary::DestroyBuilding() leave_house.clear(); - // Umgebung nach feindlichen Militärgebäuden absuchen und die ihre Grenzflaggen neu berechnen lassen - // da, wir ja nicht mehr existieren + // Let close military buildings recalculate distances to update flags etc. sortedMilitaryBlds buildings = world->LookForMilitaryBuildings(pos, 3); for(auto* building : buildings) { @@ -117,7 +114,6 @@ nobBaseMilitary::nobBaseMilitary(SerializedGameData& sgd, const unsigned obj_id) void nobBaseMilitary::AddLeavingEvent() { - // Wenn gerade keiner rausgeht, muss neues Event angemeldet werden if(!go_out) { leaving_event = GetEvMgr().AddEvent(this, 20 + RANDOM_RAND(10)); @@ -225,27 +221,21 @@ MapPoint nobBaseMilitary::FindAnAttackerPlace(unsigned short& ret_radius, const bool nobBaseMilitary::CallDefender(nofAttacker& attacker) { - // Ist noch ein Verteidiger draußen (der z.B. grad wieder reingeht? + // Use existing defender (e.g. just going back in) if possible if(defender_) { - // Dann nehmen wir den, müssen ihm nur den neuen Angreifer mitteilen defender_->NewAttacker(attacker); - // Leute, die aus diesem Gebäude zum Angriff/aggressiver Verteidigung rauskommen wollen, - // blocken - CancelJobs(); - + // Block soldiers leaving this building e.g. for attacks or aggressive defending + StopLeavingSoldiers(); return true; - } - // ansonsten einen neuen aus dem Gebäude holen - else + } else { auto defender = ProvideDefender(attacker); if(!defender) return false; // Building empty -> Can be conquered - // Leute, die aus diesem Gebäude zum Angriff/aggressiver Verteidigung rauskommen wollen, - // blocken - CancelJobs(); + // Block soldiers leaving this building e.g. for attacks or aggressive defending + StopLeavingSoldiers(); // Soldat muss noch rauskommen defender_ = defender.get(); AddLeavingFigure(std::move(defender)); @@ -286,14 +276,12 @@ void nobBaseMilitary::CheckArrestedAttackers() { for(nofAttacker* aggressor : aggressors) { - // Ist der Soldat überhaupt bereit zum Kämpfen (also wartet er um die Flagge herum)? + // Ready, i.e. waiting around the flag? if(aggressor->IsAttackerReady()) { - // Und kommt er überhaupt zur Flagge (könnte ja in der 2. Reihe stehen, sodass die - // vor ihm ihn den Weg versperren)? + // Can he still reach the flag? Could be blocked by other soldiers if(world->FindHumanPath(aggressor->GetPos(), world->GetNeighbour(pos, Direction::SouthEast), 5, false)) { - // dann kann der zur Flagge gehen aggressor->AttackFlag(); return; } @@ -305,16 +293,13 @@ bool nobBaseMilitary::SendSuccessor(const MapPoint pt, const unsigned short radi { for(nofAttacker* aggressor : aggressors) { - // Wartet der Soldat überhaupt um die Flagge? if(aggressor->IsAttackerReady()) { - // Und steht er auch weiter außen?, sonst machts natürlich keinen Sinn.. + // Only move closer if(aggressor->GetRadius() > radius) { - // Und findet er einen zu diesem Punkt? if(world->FindHumanPath(aggressor->GetPos(), pt, 50, false)) { - // dann soll er dorthin gehen aggressor->StartSucceeding(pt, radius); return true; } @@ -353,28 +338,20 @@ bool nobBaseMilitary::IsOnMission(const nofActiveSoldier& soldier) const return helpers::contains(troops_on_mission, &soldier); } -/// Bricht einen aktuell von diesem Haus gestarteten Angriff/aggressive Verteidigung ab, d.h. setzt die Soldaten -/// aus der Warteschleife wieder in das Haus --> wenn Angreifer an der Fahne ist und Verteidiger rauskommen soll -void nobBaseMilitary::CancelJobs() +void nobBaseMilitary::StopLeavingSoldiers() { - // Soldaten, die noch in der Warteschlange hängen, rausschicken for(auto it = leave_house.begin(); it != leave_house.end();) { - // Nur Soldaten nehmen (Job-Arbeiten) und keine (normalen) Verteidiger, da diese ja rauskommen - // sollen zum Kampf - if((*it)->DoJobWorks() && (*it)->GetGOT() != GO_Type::NofDefender) + // Just take soldiers (in Job state) and no normal defenders which should always come out + if((*it)->IsDoingJobWorks() && (*it)->GetGOT() != GO_Type::NofDefender) { auto soldier = boost::dynamic_pointer_cast(std::move(*it)); RTTR_Assert(soldier); - // Wenn er Job-Arbeiten verrichtet, ists ein ActiveSoldier --> dem muss extra noch Bescheid gesagt werden! soldier->InformTargetsAboutCancelling(); - // Wieder in das Haus verfrachten this->AddActiveSoldier(std::move(soldier)); it = leave_house.erase(it); } else ++it; } - - // leave_house.clear(); } diff --git a/libs/s25main/buildings/nobBaseMilitary.h b/libs/s25main/buildings/nobBaseMilitary.h index 4b7741cf97..a66885e21f 100644 --- a/libs/s25main/buildings/nobBaseMilitary.h +++ b/libs/s25main/buildings/nobBaseMilitary.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 @@ -107,9 +107,8 @@ class nobBaseMilitary : public noBuilding void CheckArrestedAttackers(); /// Der Verteidiger ist entweder tot oder wieder reingegegangen void NoDefender() { defender_ = nullptr; } - /// Bricht einen aktuell von diesem Haus gestarteten Angriff/aggressive Verteidigung ab, d.h. setzt die Soldaten - /// aus der Warteschleife wieder in das Haus --> wenn Angreifer an der Fahne ist und Verteidiger rauskommen soll - void CancelJobs(); + /// Stop all soldiers going for an attack from leaving the house, i.e. put them from the queue back in + void StopLeavingSoldiers(); /// Sind noch Truppen drinne, die dieses Gebäude verteidigen können virtual bool DefendersAvailable() const = 0; diff --git a/libs/s25main/buildings/nobBaseWarehouse.cpp b/libs/s25main/buildings/nobBaseWarehouse.cpp index 1d55c1abf7..ea52a3cb9f 100644 --- a/libs/s25main/buildings/nobBaseWarehouse.cpp +++ b/libs/s25main/buildings/nobBaseWarehouse.cpp @@ -622,20 +622,19 @@ void nobBaseWarehouse::HandleLeaveEvent() const auto it = helpers::find_if(leave_house, [](const auto& sld) { return sld->GetGOT() == GO_Type::NofAggressivedefender || sld->GetGOT() == GO_Type::NofDefender; }); - // no defender found? trigger next leaving event :) + // no defender found? enqueue next leaving event if(it == leave_house.end()) { go_out = false; AddLeavingEvent(); return; } - // and make him leave the house first - // remove defender from list, insert him again in front of all others + // Move defender to front to make him leave first leave_house.push_front(std::move(*it)); leave_house.erase(it); } - // Figuren kommen zuerst raus + // Figures first, then wares if(!leave_house.empty()) { noFigure& fig = world->AddFigure(pos, std::move(leave_house.front())); @@ -672,7 +671,7 @@ void nobBaseWarehouse::HandleLeaveEvent() { if(GetFlag()->HasSpaceForWare()) { - // Dann Ware raustragen lassen + // Send first ware auto ware = std::move(waiting_wares.front()); waiting_wares.pop_front(); inventory.visual.Remove(ConvertShields(ware->type)); @@ -681,13 +680,12 @@ void nobBaseWarehouse::HandleLeaveEvent() .WalkToGoal(); } else { - // Kein Platz mehr für Waren --> keiner brauch mehr rauszukommen, und Figuren gibts ja auch keine mehr + // No space and no figures (checked above) go_out = false; } } - // Wenn keine Figuren und Waren mehr da sind (bzw die Flagge vorm Haus voll ist), brauch auch keiner mehr - // rauszukommen + // Nothing left waiting -> stop if(leave_house.empty() && waiting_wares.empty()) go_out = false; @@ -1084,9 +1082,7 @@ std::unique_ptr nobBaseWarehouse::ProvideDefender(nofAttacker& atta return defender; } ++r; - } - // Reserve - else if(reserve_soldiers_available[i]) + } else if(reserve_soldiers_available[i]) { if(r == rank) { @@ -1137,39 +1133,23 @@ std::unique_ptr nobBaseWarehouse::ProvideDefender(nofAttacker& atta bool nobBaseWarehouse::CanRecruitSoldiers() { - // Mindestanzahl der Gehilfen die vorhanden sein müssen anhand der 1. Militäreinstellung ausrechnen - unsigned needed_helpers = 100 - 10 * world->GetPlayer(player).GetMilitarySetting(0); - - // einer muss natürlich mindestens vorhanden sein! - if(!needed_helpers) - needed_helpers = 1; - - // Wenn alle Bedingungen erfüllt sind, Event anmelden + // Minimal number of helpers required according to current setting, at least 1 + const unsigned needed_helpers = std::max(1, 100 - 10 * world->GetPlayer(player).GetMilitarySetting(0)); return (inventory[Job::Helper] >= needed_helpers && inventory[GoodType::Sword] && inventory[GoodType::ShieldRomans] && inventory[GoodType::Beer]); } void nobBaseWarehouse::TryRecruiting() { - // Wenn noch kein Event angemeldet wurde und alle Bedingungen erfüllt sind, kann ein neues angemeldet werden - if(!recruiting_event) - { - if(CanRecruitSoldiers()) - recruiting_event = GetEvMgr().AddEvent(this, RECRUITE_GF + RANDOM_RAND(RECRUITE_RANDOM_GF), 2); - } + if(!recruiting_event && CanRecruitSoldiers()) + recruiting_event = GetEvMgr().AddEvent(this, RECRUITE_GF + RANDOM_RAND(RECRUITE_RANDOM_GF), 2); } void nobBaseWarehouse::TryStopRecruiting() { - // Wenn ein Event angemeldet wurde und die Bedingungen nicht mehr erfüllt sind, muss es wieder vernichtet werden - if(recruiting_event) - { - if(!CanRecruitSoldiers()) - { - GetEvMgr().RemoveEvent(recruiting_event); - recruiting_event = nullptr; - } - } + // Remove existing event if we have not enough recruits/wares + if(recruiting_event && !CanRecruitSoldiers()) + GetEvMgr().RemoveEvent(recruiting_event); } const Inventory& nobBaseWarehouse::GetInventory() const diff --git a/libs/s25main/buildings/nobBaseWarehouse.h b/libs/s25main/buildings/nobBaseWarehouse.h index 5b9be55d18..4efe0b33bb 100644 --- a/libs/s25main/buildings/nobBaseWarehouse.h +++ b/libs/s25main/buildings/nobBaseWarehouse.h @@ -215,7 +215,7 @@ class nobBaseWarehouse : public nobBaseMilitary, public DataChangedObservable void WareLost(Ware& ware) override; /// Bestellte Ware, die sich noch hier drin befindet, storniert ihre Auslieferung void CancelWare(Ware*& ware); - /// Bestellte Figur, die sich noch inder Warteschlange befindet, kommt nicht mehr und will rausgehauen werden + /// Bestellte Figur, die sich noch ind er Warteschlange befindet, kommt nicht mehr und will rausgehauen werden virtual void CancelFigure(noFigure* figure); /// Wird aufgerufen, wenn eine neue Ware zum dem Gebäude geliefert wird (nicht wenn sie bestellt wurde vom Gebäude!) diff --git a/libs/s25main/buildings/nobMilitary.cpp b/libs/s25main/buildings/nobMilitary.cpp index 2950aae364..e2648fd451 100644 --- a/libs/s25main/buildings/nobMilitary.cpp +++ b/libs/s25main/buildings/nobMilitary.cpp @@ -1015,7 +1015,7 @@ std::unique_ptr nobMilitary::ProvideDefender(nofAttacker& attacker) if(!soldier) { /// Soldaten, die noch auf Mission gehen wollen, canceln und für die Verteidigung mit einziehen - CancelJobs(); + StopLeavingSoldiers(); // Nochmal versuchen soldier = ChooseSoldier(); if(!soldier) diff --git a/libs/s25main/figures/noFigure.h b/libs/s25main/figures/noFigure.h index cb19a64710..ce41045edf 100644 --- a/libs/s25main/figures/noFigure.h +++ b/libs/s25main/figures/noFigure.h @@ -1,4 +1,4 @@ -// Copyright (C) 2005 - 2024 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 @@ -227,8 +227,7 @@ class noFigure : public noMovable unsigned char GetPlayer() const { return player; } - /// Macht die Figur Job-Arbeiten? - bool DoJobWorks() const { return fs == FigureState::Job; } + bool IsDoingJobWorks() const { return fs == FigureState::Job; } void Abrogate(); // beim Arbeitsplatz "kündigen" soll, man das Laufen zum Ziel unterbrechen muss (warum auch immer) diff --git a/libs/s25main/figures/nofActiveSoldier.cpp b/libs/s25main/figures/nofActiveSoldier.cpp index 9bfbe9abdb..9d18b8aa09 100644 --- a/libs/s25main/figures/nofActiveSoldier.cpp +++ b/libs/s25main/figures/nofActiveSoldier.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2005 - 2024 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 @@ -83,8 +83,7 @@ void nofActiveSoldier::WalkingHome() // Walking home to our military building - if(GetPos() == building->GetFlagPos()) // Are we already at the flag? - + if(GetPos() == building->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)); From 96fce760e476b034d65eb4a765fb75870e1784b9 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sat, 28 Mar 2026 19:44:56 +0100 Subject: [PATCH 04/19] Fix wrongly shown soldiers When soldiers in the leave queue get canceled, e.g. because a fight started so we re-add all leaving aggressive defenders, the counts of soldiers gets off: - Adding a soldier to leave queue will remove it from the real count only, not the visual count: He is still in there - When he is removed from the queue after going out the visual count is decreased - When adding it back before he left we must not increase the visual count Currently it checked if the soldier is in the leave queue but at that point he isn't anymore so the check never succeeds. Add new callback to notify when a figure has left the queue. --- libs/s25main/buildings/nobBaseMilitary.cpp | 5 +- libs/s25main/buildings/nobBaseMilitary.h | 2 + libs/s25main/buildings/nobBaseWarehouse.cpp | 63 +++++++--------- libs/s25main/buildings/nobBaseWarehouse.h | 1 + libs/s25main/buildings/nobMilitary.cpp | 2 +- libs/s25main/buildings/nobMilitary.h | 3 +- tests/s25Main/integration/testAttacking.cpp | 83 +++++++++++++++++---- 7 files changed, 107 insertions(+), 52 deletions(-) diff --git a/libs/s25main/buildings/nobBaseMilitary.cpp b/libs/s25main/buildings/nobBaseMilitary.cpp index e79a2a55cc..0c72ba3960 100644 --- a/libs/s25main/buildings/nobBaseMilitary.cpp +++ b/libs/s25main/buildings/nobBaseMilitary.cpp @@ -347,10 +347,11 @@ void nobBaseMilitary::StopLeavingSoldiers() { auto soldier = boost::dynamic_pointer_cast(std::move(*it)); RTTR_Assert(soldier); + it = leave_house.erase(it); + FigureLeft(*soldier); soldier->InformTargetsAboutCancelling(); - this->AddActiveSoldier(std::move(soldier)); - it = leave_house.erase(it); + AddActiveSoldier(std::move(soldier)); } else ++it; } diff --git a/libs/s25main/buildings/nobBaseMilitary.h b/libs/s25main/buildings/nobBaseMilitary.h index a66885e21f..4bd74ad0f2 100644 --- a/libs/s25main/buildings/nobBaseMilitary.h +++ b/libs/s25main/buildings/nobBaseMilitary.h @@ -142,6 +142,8 @@ class nobBaseMilitary : public noBuilding virtual std::unique_ptr ProvideDefender(nofAttacker& attacker) = 0; /// Add a figure that will leave the house void AddLeavingFigure(std::unique_ptr fig); + /// Called when a figure has left the building, i.e. the leave queue + virtual void FigureLeft(const noFigure& fig) = 0; }; class sortedMilitaryBlds : public boost::container::flat_set diff --git a/libs/s25main/buildings/nobBaseWarehouse.cpp b/libs/s25main/buildings/nobBaseWarehouse.cpp index ea52a3cb9f..45d6ea0c38 100644 --- a/libs/s25main/buildings/nobBaseWarehouse.cpp +++ b/libs/s25main/buildings/nobBaseWarehouse.cpp @@ -645,28 +645,7 @@ void nobBaseWarehouse::HandleLeaveEvent() fig.InitializeRoadWalking(GetRoute(Direction::SouthEast), 0, true); fig.ActAtFirst(); - // Bei Lagerhausarbeitern das nicht abziehen! - if(!fig.MemberOfWarehouse()) - { - // War das ein Boot-Träger? - 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); - - if(fig.GetGOT() == GO_Type::NofTradedonkey) - { - // Trade donkey carrying wares? - const auto& carriedWare = static_cast(fig).GetCarriedWare(); - if(carriedWare) - inventory.visual.Remove(*carriedWare); - } - } + FigureLeft(fig); } else { if(GetFlag()->HasSpaceForWare()) @@ -693,6 +672,30 @@ void nobBaseWarehouse::HandleLeaveEvent() leaving_event = GetEvMgr().AddEvent(this, LEAVE_INTERVAL + RANDOM_RAND(LEAVE_INTERVAL_RAND)); } +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); + + if(fig.GetGOT() == GO_Type::NofTradedonkey) + { + // Trade donkey carrying wares? + const auto& carriedWare = static_cast(fig).GetCarriedWare(); + if(carriedWare) + inventory.visual.Remove(*carriedWare); + } +} + Ware* nobBaseWarehouse::OrderWare(const GoodType good, noBaseBuilding& goal) { // Ware überhaupt hier vorhanden (Abfrage eigentlich nicht nötig, aber erstmal zur Sicherheit) @@ -1012,18 +1015,9 @@ void nobBaseWarehouse::SoldierLost(nofSoldier* soldier) void nobBaseWarehouse::AddActiveSoldier(std::unique_ptr soldier) { - // Add soldier. If he is still in the leave-queue, then don't add him to the visual settings again - if(helpers::contains(leave_house, soldier)) - { - inventory.real.Add(soldier->GetJobType()); - if(soldier->HasArmor()) - inventory.real.Add(jobEnumToAmoredSoldierEnum(soldier->GetJobType())); - } else - { - inventory.Add(soldier->GetJobType()); - if(soldier->HasArmor()) - inventory.Add(jobEnumToAmoredSoldierEnum(soldier->GetJobType())); - } + inventory.Add(soldier->GetJobType()); + if(soldier->HasArmor()) + inventory.Add(jobEnumToAmoredSoldierEnum(soldier->GetJobType())); // Evtl. geht der Soldat wieder in die Reserve RefreshReserve(soldier->GetRank()); @@ -1109,7 +1103,6 @@ std::unique_ptr nobBaseWarehouse::ProvideDefender(nofAttacker& atta for(auto it = leave_house.begin(); it != leave_house.end(); ++it) { std::unique_ptr soldier; - // Soldat? if((*it)->GetGOT() == GO_Type::NofAggressivedefender) { soldier = boost::static_pointer_cast(std::move(*it)); diff --git a/libs/s25main/buildings/nobBaseWarehouse.h b/libs/s25main/buildings/nobBaseWarehouse.h index 4efe0b33bb..c2f581de67 100644 --- a/libs/s25main/buildings/nobBaseWarehouse.h +++ b/libs/s25main/buildings/nobBaseWarehouse.h @@ -251,6 +251,7 @@ class nobBaseWarehouse : public nobBaseMilitary, public DataChangedObservable /// Prüft, ob es Waren zum Auslagern gibt bool AreWaresToEmpty() const; + void FigureLeft(const noFigure& fig) override; /// Fügt aktiven Soldaten (der aus von einer Mission) zum Militärgebäude hinzu void AddActiveSoldier(std::unique_ptr soldier) override; /// Gibt Gesamtanzahl aller im Lager befindlichen Soldaten zurück diff --git a/libs/s25main/buildings/nobMilitary.cpp b/libs/s25main/buildings/nobMilitary.cpp index e2648fd451..61e5b6d262 100644 --- a/libs/s25main/buildings/nobMilitary.cpp +++ b/libs/s25main/buildings/nobMilitary.cpp @@ -288,7 +288,7 @@ void nobMilitary::HandleEvent(const unsigned id) // Dann raus mit denen noFigure& soldier = world->AddFigure(pos, std::move(leave_house.front())); leave_house.pop_front(); - + FigureLeft(soldier); soldier.ActAtFirst(); } diff --git a/libs/s25main/buildings/nobMilitary.h b/libs/s25main/buildings/nobMilitary.h index 88075c1d2b..e27bc87543 100644 --- a/libs/s25main/buildings/nobMilitary.h +++ b/libs/s25main/buildings/nobMilitary.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 @@ -84,6 +84,7 @@ class nobMilitary : public nobBaseMilitary nofPassiveSoldier* ChooseSoldier(); /// Stellt Verteidiger zur Verfügung std::unique_ptr ProvideDefender(nofAttacker& attacker) override; + void FigureLeft(const noFigure& /*fig*/) override {} /// Will/kann das Gebäude noch Münzen bekommen? bool WantCoins() const; bool WantArmor() const; diff --git a/tests/s25Main/integration/testAttacking.cpp b/tests/s25Main/integration/testAttacking.cpp index 2b09be7ba1..cf1cfac893 100644 --- a/tests/s25Main/integration/testAttacking.cpp +++ b/tests/s25Main/integration/testAttacking.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2005 - 2024 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 @@ -29,6 +29,7 @@ #include #include #include +#include #include using SoldierState = nofActiveSoldier::SoldierState; @@ -105,6 +106,15 @@ struct AttackFixtureBase : public WorldWithGCExecutionChangeMilitary(MILITARY_SETTINGS_SCALE); } curPlayer = 0; + initGameRNG(); + } + + void FinishRecruiting(nobBaseWarehouse& hq) + { + Inventory goods; + goods.Add(Job::Helper, 100); + hq.AddGoods(goods, true); + RTTR_EXEC_TILL(600, hq.GetNumVisualWares(GoodType::Sword) == 0u); } void MakeVisible(const MapPoint& pt) @@ -242,8 +252,6 @@ struct AttackFixture : public AttackFixtureBase BOOST_FIXTURE_TEST_CASE(NumSoldiersForAttack, NumSoldierTestFixture) { - initGameRNG(); - // Connect buildings curPlayer = 1; BuildRoadForBlds(hqPos[1], milBld1Near->GetPos()); @@ -306,7 +314,6 @@ BOOST_FIXTURE_TEST_CASE(NumSoldiersForAttack, NumSoldierTestFixture) BOOST_FIXTURE_TEST_CASE(StartAttack, AttackFixture<>) { - initGameRNG(); GameWorldViewer gwv(curPlayer, world); // Add soldiers (3 strong, 3 weak) @@ -394,8 +401,6 @@ BOOST_FIXTURE_TEST_CASE(StartAttack, AttackFixture<>) BOOST_FIXTURE_TEST_CASE(ConquerBld, AttackFixture<>) { - initGameRNG(); - AddSoldiers(milBld0Pos, 1, 5); AddSoldiersWithRankAndArmor(milBld1Pos, 1, Job::Private); AddSoldiersWithRankAndArmor(milBld1Pos, 1, Job::PrivateFirstClass); @@ -465,8 +470,6 @@ BOOST_FIXTURE_TEST_CASE(ConquerBld, AttackFixture<>) BOOST_FIXTURE_TEST_CASE(ArmoredSoldierLosesArmorInFight, AttackFixture<>) { - initGameRNG(); - AddSoldiersWithRankAndArmor(milBld0Pos, 3, Job::General, 2); AddSoldiersWithRankAndArmor(milBld1Pos, 1, Job::Private, 1); @@ -527,7 +530,6 @@ BOOST_FIXTURE_TEST_CASE(ConquerBldCoinAddonEnable, AttackFixture<>) { this->ggs.setSelection(AddonId::COINS_CAPTURED_BLD, 1); // addon is active on second run - initGameRNG(); AddSoldiers(milBld0Pos, 1, 5); AddSoldiersWithRankAndArmor(milBld1Pos, 1, Job::Private); // Finish recruiting, carrier outhousing etc. @@ -551,7 +553,6 @@ BOOST_FIXTURE_TEST_CASE(ConquerBldCoinAddonDisable, AttackFixture<>) { this->ggs.setSelection(AddonId::COINS_CAPTURED_BLD, 2); // addon is active on second run - initGameRNG(); AddSoldiers(milBld0Pos, 1, 5); AddSoldiersWithRankAndArmor(milBld1Pos, 1, Job::Private); // Finish recruiting, carrier outhousing etc. @@ -590,7 +591,6 @@ BOOST_DATA_TEST_CASE_F(AttackFixture<>, ConquerBldArmorAddon, else this->ggs.setSelection(AddonId::ARMOR_CAPTURED_BLD, 0); - initGameRNG(); AddSoldiers(milBld0Pos, 1, 5); AddSoldiersWithRankAndArmor(milBld1Pos, 1, Job::Private); @@ -613,7 +613,6 @@ BOOST_DATA_TEST_CASE_F(AttackFixture<>, ConquerBldArmorAddon, using AttackFixture4P = AttackFixture<4, 32, 34>; BOOST_FIXTURE_TEST_CASE(ConquerWithMultipleWalkingIn, AttackFixture4P) { - initGameRNG(); world.GetPlayer(0).team = Team::Team1; //-V525 world.GetPlayer(1).team = Team::None; world.GetPlayer(2).team = Team::Team1; // Allied to 0 @@ -983,7 +982,7 @@ BOOST_FIXTURE_TEST_CASE(FlagBecomesUnreachableForWaitingAttacker, AttackFixture< DestroyRoad(flagPos, Direction::East); BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(defenderBldPos) == 0u); // Fight ended - RTTR_EXEC_TILL(200, !defender.IsFightingAtFlag()); + RTTR_EXEC_TILL(500, !defender.IsFightingAtFlag()); // Attacker2 should go home BOOST_TEST(attacker2.GetState() == SoldierState::WalkingHome); BOOST_TEST(!attacker2.GetAttackedGoal()); @@ -997,6 +996,64 @@ BOOST_FIXTURE_TEST_CASE(FlagBecomesUnreachableForWaitingAttacker, AttackFixture< BOOST_TEST(attacker3.GetState() == SoldierState::AttackingFightingVsDefender); } +BOOST_FIXTURE_TEST_CASE(CancelAgressiveDefenders, AttackFixture<2>) +{ + // Reproduce issue #1907: + // Accounting of soldiers that were about to leave but got canceled was wrong + // Situation: + // - Attack on "military storehouse", i.e. HQ. + // - HQ sends aggressive defender about to leave. + // - Attacker reaches flag and requests defender which cancels any agressive defenders + AddSoldiersWithRankAndArmor(milBld0Pos, 4, Job::General); + auto& attackedHq = ensureNonNull(world.GetSpecObj(hqPos[1])); + FinishRecruiting(attackedHq); + const auto getSoldiers = [&attackedHq]() { + std::array, 2> soldiers{}; + for(const auto job : SOLDIER_JOBS) + { + soldiers[0][getSoldierRank(job)] = attackedHq.GetNumRealFigures(job); + soldiers[1][getSoldierRank(job)] = attackedHq.GetNumVisualFigures(job); + } + return soldiers; + }; + auto soldiersBefore = getSoldiers(); + + this->Attack(attackedHq.GetPos(), 1, true); + BOOST_TEST_REQUIRE(milBld0->GetLeavingFigures().size() == 1u); + // multiple soldiers and last is aggressive + auto& attacker = dynamic_cast(milBld0->GetLeavingFigures().front()); + RTTR_EXEC_TILL(70, milBld0->GetLeavingFigures().empty()); //-V807 + if(!attacker.GetHuntingDefender()) + { + auto* aggDefender = attackedHq.SendAggressiveDefender(attacker); + BOOST_TEST_REQUIRE(aggDefender); + attacker.LetsFight(*aggDefender); + } + BOOST_TEST_REQUIRE(!attackedHq.GetLeavingFigures().empty()); + // Still there visually, but not real + auto soldiersNow = getSoldiers(); + BOOST_TEST(soldiersNow[0] != soldiersBefore[0]); + BOOST_TEST(soldiersNow[1] == soldiersBefore[1]); + moveObjTo(world, attacker, attackedHq.GetFlagPos()); + rescheduleWalkEvent(em, attacker, 1); + RTTR_SKIP_GFS(1); + BOOST_TEST_REQUIRE(attacker.GetState() == SoldierState::AttackingWaitingForDefender); + // All agressive defenders are cancelled + for(const auto& fig : attackedHq.GetLeavingFigures()) + BOOST_TEST(fig.GetGOT() != GO_Type::NofAggressivedefender); + BOOST_TEST_REQUIRE(!attackedHq.GetLeavingFigures().empty()); + // Defender is coming + BOOST_TEST_REQUIRE(attackedHq.GetLeavingFigures().front().GetGOT() == GO_Type::NofDefender); + const auto defenderRank = getSoldierRank(attackedHq.GetLeavingFigures().front().GetJobType()); + // Counts unchanged until defender is out + BOOST_TEST(getSoldiers() == soldiersNow); + RTTR_EXEC_TILL(70, attackedHq.GetLeavingFigures().empty()); + soldiersNow = getSoldiers(); + auto soldiersExpected = soldiersBefore; + soldiersExpected[1][defenderRank] = --soldiersExpected[0][defenderRank]; // Defender is out + BOOST_TEST(soldiersNow == soldiersExpected); +} + using DestroyRoadsOnConquerFixture = AttackFixture<2, 24>; BOOST_FIXTURE_TEST_CASE(DestroyRoadsOnConquer, DestroyRoadsOnConquerFixture) { From 07d4df104c92b7339f2f19df230cc9c4523c0aef Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sun, 29 Mar 2026 11:05:54 +0200 Subject: [PATCH 05/19] Remove useless calls to `AddToInventory` When we clear the inventory or start with an empty one there is no need to call this. --- libs/s25main/buildings/nobHarborBuilding.cpp | 6 ------ libs/s25main/buildings/nobStorehouse.cpp | 8 +------- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/libs/s25main/buildings/nobHarborBuilding.cpp b/libs/s25main/buildings/nobHarborBuilding.cpp index 6fb9d16c76..f35ac54c0f 100644 --- a/libs/s25main/buildings/nobHarborBuilding.cpp +++ b/libs/s25main/buildings/nobHarborBuilding.cpp @@ -58,12 +58,6 @@ nobHarborBuilding::nobHarborBuilding(const MapPoint pos, const unsigned char pla world->GetMilitarySquares().Add(this); world->RecalcTerritory(*this, TerritoryChangeReason::Build); - // Alle Waren 0 - inventory.clear(); - - // Aktuellen Warenbestand zur aktuellen Inventur dazu addieren - AddToInventory(); - // Take 1 as the reserve per rank for(unsigned i = 0; i <= world->GetGGS().GetMaxMilitaryRank(); ++i) { diff --git a/libs/s25main/buildings/nobStorehouse.cpp b/libs/s25main/buildings/nobStorehouse.cpp index 842d86a775..0bf81d8c1e 100644 --- a/libs/s25main/buildings/nobStorehouse.cpp +++ b/libs/s25main/buildings/nobStorehouse.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,12 +9,6 @@ nobStorehouse::nobStorehouse(const MapPoint pos, const unsigned char player, const Nation nation) : nobBaseWarehouse(BuildingType::Storehouse, pos, player, nation) { - // Alle Waren 0, außer 100 Träger. TODO: Really? - inventory.clear(); - - // Aktuellen Warenbestand zur aktuellen Inventur dazu addieren - AddToInventory(); - // Post versenden SendPostMessage(player, std::make_unique( GetEvMgr().GetCurrentGF(), _("New storehouse finished"), PostCategory::Economy, *this)); From d8e512e07d90545f2952c8a25fc519c70b2f1f9e Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sun, 29 Mar 2026 12:22:57 +0200 Subject: [PATCH 06/19] Split out "Count" classes for goods and people `GoodsAndPeopleCounts` is now both a `GoodCounts` and `PeopleCounts` allowing them to be used as required. Refactor the misnamed `AddGoods` to 3 `AddToInventory` methods to avoid parts of the work if only people or goods are added. Ensure visibility of write-accessors for `Inventory` is protecting against mistakes e.g. with armored soldier counts. --- libs/s25main/buildings/BurnedWarehouse.cpp | 6 +-- libs/s25main/buildings/BurnedWarehouse.h | 6 +-- libs/s25main/buildings/nobBaseWarehouse.cpp | 53 +++++++++---------- libs/s25main/buildings/nobBaseWarehouse.h | 8 +-- libs/s25main/buildings/nobHQ.cpp | 11 ++-- libs/s25main/figures/nofTradeDonkey.cpp | 8 +-- libs/s25main/gameTypes/GoodsAndPeopleArray.h | 23 ++++++-- libs/s25main/gameTypes/Inventory.h | 18 ++++++- libs/s25main/gameTypes/VirtualInventory.h | 8 +-- libs/s25main/lua/LuaPlayer.cpp | 21 ++++---- libs/s25main/nodeObjs/noShip.cpp | 12 ++--- tests/s25Main/integration/testArmor.cpp | 10 ++-- tests/s25Main/integration/testAttacking.cpp | 12 ++--- .../s25Main/integration/testBaseWarehouse.cpp | 40 +++++++------- tests/s25Main/integration/testEconomyMode.cpp | 14 ++--- tests/s25Main/integration/testFigures.cpp | 8 +-- .../s25Main/integration/testGameCommands.cpp | 20 +++---- tests/s25Main/integration/testProduction.cpp | 16 +++--- .../s25Main/integration/testSeaAttacking.cpp | 14 ++--- tests/s25Main/integration/testSeafaring.cpp | 22 ++++---- 20 files changed, 177 insertions(+), 153 deletions(-) diff --git a/libs/s25main/buildings/BurnedWarehouse.cpp b/libs/s25main/buildings/BurnedWarehouse.cpp index ce69486dc4..bc0603c29d 100644 --- a/libs/s25main/buildings/BurnedWarehouse.cpp +++ b/libs/s25main/buildings/BurnedWarehouse.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2005 - 2025 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 @@ -19,7 +19,7 @@ constexpr unsigned GO_OUT_PHASES = 10; /// Time between those phases constexpr unsigned PHASE_LENGTH = 2; -BurnedWarehouse::BurnedWarehouse(const MapPoint pos, const unsigned char player, const PeopleArray& people) +BurnedWarehouse::BurnedWarehouse(const MapPoint pos, const unsigned char player, const PeopleCounts& people) : noCoordBase(NodalObjectType::BurnedWarehouse, pos), player(player), go_out_phase(0), people(people) { // First event @@ -115,7 +115,7 @@ void BurnedWarehouse::HandleEvent(const unsigned /*id*/) if(people[armoredSoldier] > 0) { figure.SetArmor(true); - people.armoredSoldiers[armoredSoldier]--; + people[armoredSoldier]--; } } figure.StartWandering(GetObjId()); diff --git a/libs/s25main/buildings/BurnedWarehouse.h b/libs/s25main/buildings/BurnedWarehouse.h index 930cdf7580..f83d93bd53 100644 --- a/libs/s25main/buildings/BurnedWarehouse.h +++ b/libs/s25main/buildings/BurnedWarehouse.h @@ -1,4 +1,4 @@ -// Copyright (C) 2005 - 2025 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 @@ -12,7 +12,7 @@ class SerializedGameData; class BurnedWarehouse : public noCoordBase { public: - BurnedWarehouse(MapPoint pos, unsigned char player, const PeopleArray& people); + BurnedWarehouse(MapPoint pos, unsigned char player, const PeopleCounts& people); BurnedWarehouse(SerializedGameData& sgd, unsigned obj_id); ~BurnedWarehouse() override; @@ -33,5 +33,5 @@ class BurnedWarehouse : public noCoordBase /// Aktuelle Rausgeh-Phase unsigned go_out_phase; // Leute, die noch rauskommen müssen - PeopleArray people; + PeopleCounts people; }; diff --git a/libs/s25main/buildings/nobBaseWarehouse.cpp b/libs/s25main/buildings/nobBaseWarehouse.cpp index 45d6ea0c38..11711cc37d 100644 --- a/libs/s25main/buildings/nobBaseWarehouse.cpp +++ b/libs/s25main/buildings/nobBaseWarehouse.cpp @@ -113,9 +113,8 @@ void nobBaseWarehouse::DestroyBuilding() reserve_soldiers_available_with_armor[rank]); } } - // Objekt, das die flüchtenden Leute nach und nach ausspuckt, erzeugen - world->AddFigure(pos, std::make_unique(pos, player, inventory.real)); + world->AddFigure(pos, std::make_unique(pos, player, inventory.real.peopleOnly())); nobBaseMilitary::DestroyBuilding(); } @@ -188,8 +187,8 @@ nobBaseWarehouse::nobBaseWarehouse(SerializedGameData& sgd, const unsigned obj_i if(sgd.GetGameDataVersion() < 12 && leatheraddon::isLeatherAddonGoodType(i)) continue; - inventory.visual[i] = sgd.PopUnsignedInt(); - inventory.real[i] = sgd.PopUnsignedInt(); + inventory.visual.Add(i, sgd.PopUnsignedInt()); + inventory.real.Add(i, sgd.PopUnsignedInt()); inventorySettings[i] = inventorySettingsVisual[i] = static_cast(sgd.PopUnsignedChar()); } for(const auto i : helpers::enumRange()) @@ -200,8 +199,8 @@ nobBaseWarehouse::nobBaseWarehouse(SerializedGameData& sgd, const unsigned obj_i if(sgd.GetGameDataVersion() < 12 && leatheraddon::isLeatherAddonJobType(i)) continue; - inventory.visual[i] = sgd.PopUnsignedInt(); - inventory.real[i] = sgd.PopUnsignedInt(); + inventory.visual.Add(i, sgd.PopUnsignedInt()); + inventory.real.Add(i, sgd.PopUnsignedInt()); inventorySettings[i] = inventorySettingsVisual[i] = static_cast(sgd.PopUnsignedChar()); } if(sgd.GetGameDataVersion() >= 12) @@ -1150,7 +1149,13 @@ const Inventory& nobBaseWarehouse::GetInventory() const return inventory.visual; } -void nobBaseWarehouse::AddGoods(const Inventory& goods, bool addToPlayer) +void nobBaseWarehouse::AddToInventory(const GoodsAndPeopleCounts& what, bool addToPlayer) +{ + AddToInventory(static_cast(what), addToPlayer); + AddToInventory(static_cast(what), addToPlayer); +} + +void nobBaseWarehouse::AddToInventory(const GoodCounts& goods, bool addToPlayer) { GamePlayer& owner = world->GetPlayer(player); for(const auto i : helpers::enumRange()) @@ -1160,51 +1165,41 @@ void nobBaseWarehouse::AddGoods(const Inventory& goods, bool addToPlayer) // Can only add canonical shields (romans) RTTR_Assert(i == GoodType::ShieldRomans || ConvertShields(i) != GoodType::ShieldRomans); - inventory.Add(i, goods.goods[i]); + inventory.Add(i, goods[i]); if(addToPlayer) - owner.IncreaseInventoryWare(i, goods.goods[i]); + owner.IncreaseInventoryWare(i, goods[i]); CheckUsesForNewWare(i); } +} +void nobBaseWarehouse::AddToInventory(const PeopleCounts& people, bool addToPlayer) +{ + GamePlayer& owner = world->GetPlayer(player); for(const auto i : helpers::enumRange()) { - if(!goods.people[i]) + if(!people[i]) continue; // Boatcarriers are added as carriers and boat individually RTTR_Assert(i != Job::BoatCarrier); - inventory.Add(i, goods.people[i]); + inventory.Add(i, people[i]); if(addToPlayer) - owner.IncreaseInventoryJob(i, goods.people[i]); + owner.IncreaseInventoryJob(i, people[i]); if(isSoldier(i)) { const auto armoredSoldier = jobEnumToAmoredSoldierEnum(i); - if(goods[armoredSoldier]) + if(people[armoredSoldier]) { - inventory.Add(armoredSoldier, goods[armoredSoldier]); + inventory.Add(armoredSoldier, people[armoredSoldier]); if(addToPlayer) - owner.IncreaseInventoryJob(armoredSoldier, goods[armoredSoldier]); + owner.IncreaseInventoryJob(armoredSoldier, people[armoredSoldier]); } } - CheckJobsForNewFigure(i); } } -void nobBaseWarehouse::AddToInventory() -{ - GamePlayer& owner = world->GetPlayer(player); - for(const auto i : helpers::enumRange()) - owner.IncreaseInventoryWare(i, inventory[i]); - - for(const auto i : helpers::enumRange()) - owner.IncreaseInventoryJob(i, inventory[i]); - - for(const auto i : helpers::enumRange()) - owner.IncreaseInventoryJob(i, inventory[i]); -} - bool nobBaseWarehouse::CanRecruit(const Job job) const { if(const GoodType* requiredTool = JOB_CONSTS[job].tool.get_ptr()) diff --git a/libs/s25main/buildings/nobBaseWarehouse.h b/libs/s25main/buildings/nobBaseWarehouse.h index c2f581de67..2cf85eb9f0 100644 --- a/libs/s25main/buildings/nobBaseWarehouse.h +++ b/libs/s25main/buildings/nobBaseWarehouse.h @@ -122,8 +122,6 @@ class nobBaseWarehouse : public nobBaseMilitary, public DataChangedObservable /// Versucht Rekrutierungsevent abzumeldne, falls die Bedingungen nicht mehr erfüllt sind (z.B. wenn Ware /// rausgetragen wurde o.ä.) void TryStopRecruiting(); - /// Aktuellen Warenbestand zur aktuellen Inventur dazu addieren - void AddToInventory(); /// Recruts a worker of the given job if possible bool TryRecruitJob(Job job); @@ -146,10 +144,12 @@ class nobBaseWarehouse : public nobBaseMilitary, public DataChangedObservable const Inventory& GetInventory() const; - /// Adds specified goods. If addToPlayer is true, + /// Adds specified goods and people. If addToPlayer is true, /// then they are also added to the owners inventory (for newly created/arrived goods) /// Use false for goods, that are only moved between players units - void AddGoods(const Inventory& goods, bool addToPlayer); + void AddToInventory(const GoodsAndPeopleCounts& what, bool addToPlayer); + void AddToInventory(const PeopleCounts& people, bool addToPlayer); + void AddToInventory(const GoodCounts& goods, bool addToPlayer); /// Gibt Anzahl der Waren bzw. Figuren zurück unsigned GetNumRealWares(GoodType type) const { return inventory.real[type]; } diff --git a/libs/s25main/buildings/nobHQ.cpp b/libs/s25main/buildings/nobHQ.cpp index c02d5a3eac..4f27d6b91a 100644 --- a/libs/s25main/buildings/nobHQ.cpp +++ b/libs/s25main/buildings/nobHQ.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 @@ -300,8 +300,13 @@ nobHQ::nobHQ(const MapPoint pos, const unsigned char player, const Nation nation inventory.real = inventory.visual; - // Aktuellen Warenbestand zur aktuellen Inventur dazu addieren - AddToInventory(); + GamePlayer& owner = world->GetPlayer(player); + for(const auto i : helpers::enumRange()) + owner.IncreaseInventoryWare(i, inventory[i]); + for(const auto i : helpers::enumRange()) + owner.IncreaseInventoryJob(i, inventory[i]); + for(const auto i : helpers::enumRange()) + owner.IncreaseInventoryJob(i, inventory[i]); // Take 1 as the reserve per rank for(unsigned i = 0; i <= world->GetGGS().GetMaxMilitaryRank(); ++i) diff --git a/libs/s25main/figures/nofTradeDonkey.cpp b/libs/s25main/figures/nofTradeDonkey.cpp index 74336037e7..2cf65861a3 100644 --- a/libs/s25main/figures/nofTradeDonkey.cpp +++ b/libs/s25main/figures/nofTradeDonkey.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2005 - 2024 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 @@ -54,9 +54,9 @@ void nofTradeDonkey::GoalReached() if(gt) { - Inventory goods; - goods.goods[*gt] = 1; - wh->AddGoods(goods, true); + GoodCounts goods; + goods[*gt] = 1; + wh->AddToInventory(goods, true); } whOwner.IncreaseInventoryJob(this->GetJobType(), 1); diff --git a/libs/s25main/gameTypes/GoodsAndPeopleArray.h b/libs/s25main/gameTypes/GoodsAndPeopleArray.h index 4370774238..5562b81a14 100644 --- a/libs/s25main/gameTypes/GoodsAndPeopleArray.h +++ b/libs/s25main/gameTypes/GoodsAndPeopleArray.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 @@ -19,16 +19,29 @@ struct PeopleArray const T& operator[](Job job) const { return people[job]; } T& operator[](Job job) { return people[job]; } + T& operator[](ArmoredSoldier soldier) { return armoredSoldiers[soldier]; } const T& operator[](ArmoredSoldier soldier) const { return armoredSoldiers[soldier]; } }; -/// Combined array for goods and people with typed accessors template -struct GoodsAndPeopleArray : PeopleArray +struct GoodsArray { - using PeopleArray::operator[]; helpers::EnumArray goods = {}; - const T& operator[](GoodType good) const { return goods[good]; } T& operator[](GoodType good) { return goods[good]; } }; + +/// Combined array for goods and people with typed accessors +template +struct GoodsAndPeopleArray : PeopleArray, GoodsArray +{ + using PeopleArray::operator[]; + using GoodsArray::operator[]; +}; + +/// Raw count of goods +using GoodCounts = GoodsArray; +/// Raw count of people +using PeopleCounts = PeopleArray; +/// Raw count of goods and people +using GoodsAndPeopleCounts = GoodsAndPeopleArray; diff --git a/libs/s25main/gameTypes/Inventory.h b/libs/s25main/gameTypes/Inventory.h index aa21c0a5a7..7575fdbac7 100644 --- a/libs/s25main/gameTypes/Inventory.h +++ b/libs/s25main/gameTypes/Inventory.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 @@ -9,8 +9,22 @@ #include "JobTypes.h" /// Struct for wares and people (for HQs, warehouses etc) -struct Inventory : GoodsAndPeopleArray +struct Inventory : private GoodsAndPeopleCounts { + using GoodsAndPeopleCounts::armoredSoldiers; + using GoodsAndPeopleCounts::goods; + using GoodsAndPeopleCounts::people; + // Write access is only allowed via Add and Remove, or explicitely via underlying arrays + // to ensure amounts are non-negative and armored soldier count is valid + auto operator[](GoodType good) const { return goods[good]; } + auto operator[](Job job) const { return people[job]; } + auto operator[](ArmoredSoldier soldier) const { return armoredSoldiers[soldier]; } + + GoodCounts& goodsOnly() { return *this; } + const GoodCounts& goodsOnly() const { return *this; } + PeopleCounts& peopleOnly() { return *this; } + const PeopleCounts& peopleOnly() const { return *this; } + /// Sets everything to 0 void clear(); void Add(const GoodType good, unsigned amount = 1) { goods[good] += amount; } diff --git a/libs/s25main/gameTypes/VirtualInventory.h b/libs/s25main/gameTypes/VirtualInventory.h index dc5ea3d76d..93fa51268b 100644 --- a/libs/s25main/gameTypes/VirtualInventory.h +++ b/libs/s25main/gameTypes/VirtualInventory.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 @@ -59,9 +59,9 @@ struct VirtualInventory } /// Returns the real number of people of the given type - unsigned operator[](Job job) const { return real.people[job]; } + unsigned operator[](Job job) const { return real[job]; } /// Returns the real number of wares of the given type - unsigned operator[](GoodType good) const { return real.goods[good]; } + unsigned operator[](GoodType good) const { return real[good]; } /// Returns the real number of armored soldiers of the given type - unsigned operator[](ArmoredSoldier soldier) const { return real.armoredSoldiers[soldier]; } + unsigned operator[](ArmoredSoldier soldier) const { return real[soldier]; } }; diff --git a/libs/s25main/lua/LuaPlayer.cpp b/libs/s25main/lua/LuaPlayer.cpp index 850678d7eb..b629ead3f0 100644 --- a/libs/s25main/lua/LuaPlayer.cpp +++ b/libs/s25main/lua/LuaPlayer.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 @@ -187,14 +187,11 @@ bool LuaPlayer::AddWares(const std::map, unsigned>& ware if(!warehouse) return false; - Inventory goods; - + GoodCounts counts; for(const auto& ware : wares) - { - goods.Add(ware.first, ware.second); - } + counts[ware.first] += ware.second; - warehouse->AddGoods(goods, true); + warehouse->AddToInventory(counts, true); return true; } @@ -205,20 +202,20 @@ bool LuaPlayer::AddPeople(const std::map, unsigned>& people) if(!warehouse) return false; - Inventory goods; + GoodsAndPeopleCounts counts; for(const auto& it : people) { const Job job = it.first; if(job == Job::BoatCarrier) { - goods.Add(Job::Helper, it.second); - goods.Add(GoodType::Boat, it.second); + counts[Job::Helper] += it.second; + counts[GoodType::Boat] += it.second; } else - goods.Add(job, it.second); + counts[it.first] += it.second; } - warehouse->AddGoods(goods, true); + warehouse->AddToInventory(counts, true); return true; } diff --git a/libs/s25main/nodeObjs/noShip.cpp b/libs/s25main/nodeObjs/noShip.cpp index f10af945ac..95033c7c84 100644 --- a/libs/s25main/nodeObjs/noShip.cpp +++ b/libs/s25main/nodeObjs/noShip.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 @@ -251,11 +251,11 @@ void noShip::HandleEvent(const unsigned id) if(hb && hb->GetGOT() == GO_Type::NobHarborbuilding) { - Inventory goods; + GoodsAndPeopleCounts goods; goods.goods[GoodType::Boards] = BUILDING_COSTS[BuildingType::HarborBuilding].boards; goods.goods[GoodType::Stones] = BUILDING_COSTS[BuildingType::HarborBuilding].stones; goods.people[Job::Builder] = 1; - static_cast(hb)->AddGoods(goods, false); + static_cast(hb)->AddToInventory(goods, false); // Wieder idlen und ggf. neuen Job suchen StartIdling(); world->GetPlayer(ownerId_).GetJobForShip(*this); @@ -277,9 +277,9 @@ void noShip::HandleEvent(const unsigned id) if(hb && hb->GetGOT() == GO_Type::NobHarborbuilding) { // Späher wieder entladen - Inventory goods; - goods.people[Job::Scout] = world->GetGGS().GetNumScoutsExpedition(); - static_cast(hb)->AddGoods(goods, false); + PeopleCounts people; + people[Job::Scout] = world->GetGGS().GetNumScoutsExpedition(); + static_cast(hb)->AddToInventory(people, false); // Wieder idlen und ggf. neuen Job suchen StartIdling(); world->GetPlayer(ownerId_).GetJobForShip(*this); diff --git a/tests/s25Main/integration/testArmor.cpp b/tests/s25Main/integration/testArmor.cpp index 21a21b43d9..403ac796de 100644 --- a/tests/s25Main/integration/testArmor.cpp +++ b/tests/s25Main/integration/testArmor.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2005 - 2025 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 @@ -18,15 +18,15 @@ struct ArmoredSoldierFixture : public WorldWithGCExecution3P { auto* curWh = world.GetSpecObj(world.GetPlayer(i).GetHQPos()); BOOST_TEST_REQUIRE(curWh); - Inventory newGoods; + PeopleCounts soldiers; for(auto soldier : SOLDIER_JOBS) { - newGoods.armoredSoldiers[jobEnumToAmoredSoldierEnum(soldier)] = 2u; - newGoods.people[soldier] = 3u; + soldiers[jobEnumToAmoredSoldierEnum(soldier)] = 2u; + soldiers[soldier] = 3u; curWh->SetRealReserve(getSoldierRank(soldier), 0); curWh->SetReserveVisual(getSoldierRank(soldier), 0); } - curWh->AddGoods(newGoods, true); + curWh->AddToInventory(soldiers, true); } } }; diff --git a/tests/s25Main/integration/testAttacking.cpp b/tests/s25Main/integration/testAttacking.cpp index cf1cfac893..e2e2be32bb 100644 --- a/tests/s25Main/integration/testAttacking.cpp +++ b/tests/s25Main/integration/testAttacking.cpp @@ -95,14 +95,14 @@ struct AttackFixtureBase : public WorldWithGCExecution(hqPos[i])->AddGoods(goods, true); + world.template GetSpecObj(hqPos[i])->AddToInventory(soldiers, true); this->ChangeMilitary(MILITARY_SETTINGS_SCALE); } curPlayer = 0; @@ -111,9 +111,9 @@ struct AttackFixtureBase : public WorldWithGCExecution nothing changed - Inventory newGoods; - hq.AddGoods(newGoods, true); + GoodsAndPeopleCounts newGoods; + hq.AddToInventory(newGoods, true); testNumGoodsHQ(); testNumGoodsPlayer(); @@ -95,17 +95,17 @@ BOOST_FIXTURE_TEST_CASE(AddGoods, AddGoodsFixture) // Boat carrier gets divided upfront if(i == Job::BoatCarrier) continue; - newGoods.Add(i, rttr::enum_cast(i) + 1); + newGoods[i] = rttr::enum_cast(i) + 1; numPeople[i] += rttr::enum_cast(i) + 1; } for(const auto i : helpers::enumRange()) { - newGoods.Add(i, rttr::enum_cast(i) + 1); + newGoods[i] = rttr::enum_cast(i) + 1; numArmoredSoldiers[i] += rttr::enum_cast(i) + 1; } numPeoplePlayer = numPeople; numArmoredSoldiersPlayer = numArmoredSoldiers; - hq.AddGoods(newGoods, true); + hq.AddToInventory(newGoods, true); testNumGoodsHQ(); testNumGoodsPlayer(); @@ -115,39 +115,39 @@ BOOST_FIXTURE_TEST_CASE(AddGoods, AddGoodsFixture) for(const auto i : helpers::enumRange()) numArmoredSoldiers[i] += newGoods[i]; - hq.AddGoods(newGoods, false); + hq.AddToInventory(newGoods, false); testNumGoodsHQ(); testNumGoodsPlayer(); // Add wares - newGoods.clear(); + newGoods = {}; for(const auto i : helpers::enumRange()) { // Only roman shields get added if(ConvertShields(i) == GoodType::ShieldRomans && i != GoodType::ShieldRomans) continue; - newGoods.Add(i, rttr::enum_cast(i) + 2); + newGoods[i] = rttr::enum_cast(i) + 2; numGoods[i] += rttr::enum_cast(i) + 2; } numGoodsPlayer = numGoods; - hq.AddGoods(newGoods, true); + hq.AddToInventory(newGoods, true); testNumGoodsHQ(); testNumGoodsPlayer(); // Add only to hq but not to player for(const auto i : helpers::enumRange()) numGoods[i] += newGoods[i]; - hq.AddGoods(newGoods, false); + hq.AddToInventory(newGoods, false); testNumGoodsHQ(); testNumGoodsPlayer(); #if RTTR_ENABLE_ASSERTS - newGoods.clear(); - newGoods.Add(Job::BoatCarrier); - RTTR_REQUIRE_ASSERT(hq.AddGoods(newGoods, false)); - newGoods.clear(); - newGoods.Add(GoodType::ShieldAfricans); - RTTR_REQUIRE_ASSERT(hq.AddGoods(newGoods, false)); + PeopleCounts invPeople; + invPeople[Job::BoatCarrier] = 1; + RTTR_REQUIRE_ASSERT(hq.AddToInventory(invPeople, false)); + GoodCounts invGoods; + invGoods[GoodType::ShieldAfricans] = 1; + RTTR_REQUIRE_ASSERT(hq.AddToInventory(invGoods, false)); #endif } @@ -221,20 +221,20 @@ BOOST_FIXTURE_TEST_CASE(CollectGoodsAndFigures, WorldWithGCExecution1P) milSettings[0] = 0; // No recruitment this->ChangeMilitary(milSettings); - Inventory inv; + GoodsAndPeopleCounts inv; for(const auto job : helpers::enumRange()) { if(job == Job::BoatCarrier) continue; - inv.Add(job); + inv[job] = 1; } for(const auto good : helpers::enumRange()) { if(ConvertShields(good) != good) continue; - inv.Add(good); + inv[good] = 1; } - wh1->AddGoods(inv, true); + wh1->AddToInventory(inv, true); for(const auto job : helpers::enumRange()) { diff --git a/tests/s25Main/integration/testEconomyMode.cpp b/tests/s25Main/integration/testEconomyMode.cpp index db5bd5cb47..b08130beb5 100644 --- a/tests/s25Main/integration/testEconomyMode.cpp +++ b/tests/s25Main/integration/testEconomyMode.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 @@ -73,9 +73,9 @@ BOOST_FIXTURE_TEST_CASE(EconomyMode3Players, EconModeFixture) for(unsigned playerIdx = 0; playerIdx < 3; ++playerIdx) { - Inventory inv; - inv.Add(goodsToCollect[0], amountsToAdd[playerIdx]); - world.GetSpecObj(hqPos[playerIdx])->AddGoods(inv, true); + GoodCounts inv; + inv[goodsToCollect[0]] = amountsToAdd[playerIdx]; + world.GetSpecObj(hqPos[playerIdx])->AddToInventory(inv, true); } world.GetEvMgr().ExecuteNextGF(); econHandler.UpdateAmounts(); @@ -112,9 +112,9 @@ BOOST_FIXTURE_TEST_CASE(EconomyModeSerialization, EconModeFixture) const auto& goodsToCollect = world.getEconHandler()->GetGoodTypesToCollect(); for(unsigned playerIdx = 0; playerIdx < 3; ++playerIdx) { - Inventory inv; - inv.Add(goodsToCollect[0], rttr::test::randomValue(1u, 50u)); - world.GetSpecObj(hqPos[playerIdx])->AddGoods(inv, true); + GoodCounts inv; + inv[goodsToCollect[0]] = rttr::test::randomValue(1u, 50u); + world.GetSpecObj(hqPos[playerIdx])->AddToInventory(inv, true); } world.GetEvMgr().ExecuteNextGF(); world.getEconHandler()->UpdateAmounts(); diff --git a/tests/s25Main/integration/testFigures.cpp b/tests/s25Main/integration/testFigures.cpp index 6236ca5587..72b71f7fd5 100644 --- a/tests/s25Main/integration/testFigures.cpp +++ b/tests/s25Main/integration/testFigures.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 @@ -27,9 +27,9 @@ BOOST_FIXTURE_TEST_CASE(DestroyWHWithFigure, WorldWithGCExecution2P) MapPoint whPos(flagPos.x + 5, flagPos.y); auto* wh = static_cast( BuildingFactory::CreateBuilding(world, BuildingType::Storehouse, whPos, curPlayer, Nation::Romans)); - Inventory inv; - inv.Add(Job::Helper, 1); - wh->AddGoods(inv, true); + PeopleCounts inv; + inv[Job::Helper] = 1; + wh->AddToInventory(inv, true); const unsigned numHelpers = world.GetPlayer(curPlayer).GetInventory().people[Job::Helper]; //-V807 MapPoint whFlagPos = world.GetNeighbour(whPos, Direction::SouthEast); // Build a road -> Requests a worker diff --git a/tests/s25Main/integration/testGameCommands.cpp b/tests/s25Main/integration/testGameCommands.cpp index 89021d7016..bd959b0c53 100644 --- a/tests/s25Main/integration/testGameCommands.cpp +++ b/tests/s25Main/integration/testGameCommands.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 @@ -434,12 +434,12 @@ BOOST_FIXTURE_TEST_CASE(SendSoldiersHomeTest, WorldWithGCExecution2P) nobBaseWarehouse* wh = player.GetFirstWH(); BOOST_TEST_REQUIRE(wh); BOOST_TEST_REQUIRE(wh->GetInventory().people[Job::General] == 0u); //-V522 - Inventory goods; - goods.Add(Job::PrivateFirstClass, 1); - goods.Add(Job::Sergeant, 1); - goods.Add(Job::Officer, 1); - goods.Add(Job::General, 2); - wh->AddGoods(goods, true); + PeopleCounts soldiers; + soldiers[Job::PrivateFirstClass] = 1; + soldiers[Job::Sergeant] = 1; + soldiers[Job::Officer] = 1; + soldiers[Job::General] = 2; + wh->AddToInventory(soldiers, true); // Don't keep any reserve for(unsigned i = 0; i <= this->ggs.GetMaxMilitaryRank(); ++i) this->ChangeReserve(hqPos, i, 0); @@ -893,12 +893,12 @@ BOOST_FIXTURE_TEST_CASE(ChangeReserveTest, WorldWithGCExecution2P) GamePlayer& player = world.GetPlayer(curPlayer); nobBaseWarehouse* wh = player.GetFirstWH(); BOOST_TEST_REQUIRE(wh); - Inventory goods; + PeopleCounts soldiers; // Add enough soldiers per rank for(auto i : SOLDIER_JOBS) - goods.Add(i, 50); - wh->AddGoods(goods, true); //-V522 + soldiers[i] = 50; + wh->AddToInventory(soldiers, true); //-V522 // Use more for(unsigned i = 0; i < SOLDIER_JOBS.size(); i++) diff --git a/tests/s25Main/integration/testProduction.cpp b/tests/s25Main/integration/testProduction.cpp index 52d24886ec..89685d678b 100644 --- a/tests/s25Main/integration/testProduction.cpp +++ b/tests/s25Main/integration/testProduction.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 @@ -28,9 +28,9 @@ BOOST_FIXTURE_TEST_CASE(MetalWorkerStopped, WorldWithGCExecution1P) rttr::test::LogAccessor logAcc; ggs.setSelection(AddonId::TOOL_ORDERING, 1); ggs.setSelection(AddonId::METALWORKSBEHAVIORONZERO, 1); - Inventory goods; - goods.goods[GoodType::Iron] = 10; - world.GetSpecObj(hqPos)->AddGoods(goods, true); + GoodCounts goods; + goods[GoodType::Iron] = 10; + world.GetSpecObj(hqPos)->AddToInventory(goods, true); MapPoint bldPos = hqPos + MapPoint(2, 0); BuildingFactory::CreateBuilding(world, BuildingType::Metalworks, bldPos, curPlayer, Nation::Africans); this->BuildRoad(world.GetNeighbour(bldPos, Direction::SouthEast), false, @@ -74,10 +74,10 @@ BOOST_FIXTURE_TEST_CASE(MetalWorkerStopped, WorldWithGCExecution1P) BOOST_FIXTURE_TEST_CASE(MetalWorkerOrders, WorldWithGCExecution1P) { - Inventory inv; - inv.Add(GoodType::Boards, 10); - inv.Add(GoodType::Iron, 10); - world.GetSpecObj(hqPos)->AddGoods(inv, true); + GoodCounts goods; + goods[GoodType::Boards] = 10; + goods[GoodType::Iron] = 10; + world.GetSpecObj(hqPos)->AddToInventory(goods, true); ggs.setSelection(AddonId::METALWORKSBEHAVIORONZERO, 1); ggs.setSelection(AddonId::TOOL_ORDERING, 1); ToolSettings settings; diff --git a/tests/s25Main/integration/testSeaAttacking.cpp b/tests/s25Main/integration/testSeaAttacking.cpp index be3e0f6b44..8fa710144b 100644 --- a/tests/s25Main/integration/testSeaAttacking.cpp +++ b/tests/s25Main/integration/testSeaAttacking.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 @@ -78,9 +78,9 @@ struct SeaAttackFixture : public SeaWorldWithGCExecution<3, 62, 64> SetCurPlayer(i); hqPos[i] = world.GetPlayer(i).GetHQPos(); auto* hq = world.GetSpecObj(hqPos[i]); - Inventory goods; - goods.Add(Job::General, 3); - hq->AddGoods(goods, true); + PeopleCounts soldiers; + soldiers[Job::General] = 3; + hq->AddToInventory(soldiers, true); this->ChangeMilitary(MILITARY_SETTINGS_SCALE); } // Assert player positions: 0: Top, 1: Left, 2: Right @@ -425,9 +425,9 @@ BOOST_FIXTURE_TEST_CASE(AttackHarbor, SeaAttackFixture) BuildRoadForBlds(milBld2Pos, hqPos[2]); // Add 1 soldier to dest harbor so we have a defender nobHarborBuilding& hbDest = *world.GetSpecObj(harborPos[1]); - Inventory newGoods; - newGoods.Add(Job::Sergeant, 1); - hbDest.AddGoods(newGoods, true); + PeopleCounts soldiers; + soldiers[Job::Sergeant] = 1; + hbDest.AddToInventory(soldiers, true); // Don't keep him in reserve hbDest.SetRealReserve(getSoldierRank(Job::Sergeant), 0); BOOST_TEST_REQUIRE(hbDest.GetNumVisualFigures(Job::Sergeant) == 1u); diff --git a/tests/s25Main/integration/testSeafaring.cpp b/tests/s25Main/integration/testSeafaring.cpp index 139a080a2c..0484b2ecc4 100644 --- a/tests/s25Main/integration/testSeafaring.cpp +++ b/tests/s25Main/integration/testSeafaring.cpp @@ -467,9 +467,9 @@ BOOST_FIXTURE_TEST_CASE(LongDistanceTravel, ShipReadyFixtureBig) // Make sure that the other harbor is far away BOOST_TEST_REQUIRE(world.CalcHarborDistance(HarborId(2), targetHbId) > 600u); // Add some scouts - Inventory newScouts; + PeopleCounts newScouts; newScouts[Job::Scout] = 20; - harbor.AddGoods(newScouts, true); + harbor.AddToInventory(newScouts, true); // We want the ship to only scout unexplored harbors, so set all but one to visible for(const auto i : helpers::idRange(world.GetNumHarborPoints())) world.GetNodeWriteable(world.GetHarborPoint(i)).fow[curPlayer].visibility = Visibility::Visible; @@ -503,10 +503,10 @@ struct ShipAndHarborsReadyFixture : auto* harbor = static_cast( BuildingFactory::CreateBuilding(world, BuildingType::HarborBuilding, hbPos, curPlayer, Nation::Romans)); BOOST_TEST_REQUIRE(harbor); - Inventory inv; - inv.Add(GoodType::Wood, 10); - inv.Add(Job::Woodcutter, 10); - harbor->AddGoods(inv, true); + GoodsAndPeopleCounts goods; + goods[GoodType::Wood] = 10; + goods[Job::Woodcutter] = 10; + harbor->AddToInventory(goods, true); return *harbor; } @@ -738,11 +738,11 @@ BOOST_FIXTURE_TEST_CASE(GoToNeighborHarbor, ShipReadyFixture<1>) const noShip& ship = ensureNonNull(player.GetShipByID(0)); nobHarborBuilding& homeHarbor = *player.GetBuildingRegister().GetHarbors().front(); const MapPoint homeHbPos = homeHarbor.GetPos(); - Inventory expWares; - expWares.Add(Job::Builder); - expWares.Add(GoodType::Boards, 20); - expWares.Add(GoodType::Stones, 20); - homeHarbor.AddGoods(expWares, true); + GoodsAndPeopleCounts expWares; + expWares[Job::Builder] = 1; + expWares[GoodType::Boards] = 20; + expWares[GoodType::Stones] = 20; + homeHarbor.AddToInventory(expWares, true); /* A ship might get stuck when targeting a harbor (2) SE of the current harbor (1): Issue #1784 Harbors are at NE coast of island, (1) directly at sea, (2) one node inland: From e92e9cacdd8b429ccc50954d88bf392c680eabe3 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sun, 29 Mar 2026 13:46:22 +0200 Subject: [PATCH 07/19] Shuffle HQ positions outside function placing them --- libs/s25main/desktops/dskBenchmark.cpp | 6 +++--- libs/s25main/world/MapLoader.cpp | 20 +++++++++---------- libs/s25main/world/MapLoader.h | 6 +++--- tests/s25Main/lua/GameWithLuaAccess.h | 4 ++-- .../worldFixtures/CreateEmptyWorld.cpp | 4 ++-- .../s25Main/worldFixtures/CreateSeaWorld.cpp | 6 +++--- 6 files changed, 22 insertions(+), 24 deletions(-) diff --git a/libs/s25main/desktops/dskBenchmark.cpp b/libs/s25main/desktops/dskBenchmark.cpp index 26d04d5455..d86faf838f 100644 --- a/libs/s25main/desktops/dskBenchmark.cpp +++ b/libs/s25main/desktops/dskBenchmark.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2005 - 2025 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 @@ -220,7 +220,7 @@ void dskBenchmark::startTest(Benchmark test) return; std::vector hqs(2, MapPoint(0, 0)); hqs[1].x += 30; - MapLoader::PlaceHQs(game_->world_, hqs, false); + MapLoader::PlaceHQs(game_->world_, hqs); break; } case Benchmark::FullGame: @@ -230,7 +230,7 @@ void dskBenchmark::startTest(Benchmark test) return; std::vector hqs(2, MapPoint(0, 0)); hqs[1].x += 30; - MapLoader::PlaceHQs(game_->world_, hqs, false); + MapLoader::PlaceHQs(game_->world_, hqs); for(unsigned i = 0; i < hqs.size(); i++) { std::vector pts = game_->world_.GetPointsInRadius(hqs[i], 15); diff --git a/libs/s25main/world/MapLoader.cpp b/libs/s25main/world/MapLoader.cpp index b6769250bd..793361b1a2 100644 --- a/libs/s25main/world/MapLoader.cpp +++ b/libs/s25main/world/MapLoader.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,6 +9,7 @@ #include "GlobalGameSettings.h" #include "PointOutput.h" #include "RttrForeachPt.h" +#include "buildings/nobHQ.h" #include "factories/BuildingFactory.h" #include "helpers/IdRange.h" #include "lua/GameDataLoader.h" @@ -78,7 +79,10 @@ bool MapLoader::Load(const boost::filesystem::path& mapFilePath) if(!Load(map, world_.GetGGS().exploration)) return false; - if(!PlaceHQs(world_.GetGGS().randomStartPosition)) + std::vector hqPositions = hqPositions_; + if(world_.GetGGS().randomStartPosition) + RANDOM_SHUFFLE2(hqPositions, 0); + if(!PlaceHQs()) return false; world_.CreateTradeGraphs(); @@ -97,9 +101,9 @@ bool MapLoader::LoadLuaScript(Game& game, ILocalGameState& localgameState, const return true; } -bool MapLoader::PlaceHQs(bool randomStartPos) +bool MapLoader::PlaceHQs() { - return PlaceHQs(world_, hqPositions_, randomStartPos); + return PlaceHQs(world_, hqPositions_); } void MapLoader::InitShadows(World& world) @@ -393,14 +397,8 @@ void MapLoader::PlaceAnimals(const libsiedler2::ArchivItem_Map& map) } } -bool MapLoader::PlaceHQs(GameWorldBase& world, std::vector hqPositions, bool randomStartPos) +bool MapLoader::PlaceHQs(GameWorldBase& world, const std::vector& hqPositions) { - // random locations? -> randomize them :) - if(randomStartPos) - { - RANDOM_SHUFFLE2(hqPositions, 0); - } - for(unsigned i = 0; i < world.GetNumPlayers(); ++i) { // Skip unused slots diff --git a/libs/s25main/world/MapLoader.h b/libs/s25main/world/MapLoader.h index f901675afd..39c0d6c233 100644 --- a/libs/s25main/world/MapLoader.h +++ b/libs/s25main/world/MapLoader.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 @@ -47,7 +47,7 @@ class MapLoader bool Load(const boost::filesystem::path& mapFilePath); bool LoadLuaScript(Game& game, ILocalGameState& localgameState, const boost::filesystem::path& luaFilePath); /// Place the HQs on a loaded map (must be loaded first as hqPositions etc. are used) - bool PlaceHQs(bool randomStartPos); + bool PlaceHQs(); /// Return the position of the players HQ (only valid after successful load) MapPoint GetHQPos(unsigned player) const { return hqPositions_[player]; } @@ -56,5 +56,5 @@ class MapLoader static void SetMapExplored(World& world); static bool InitSeasAndHarbors(World& world, const std::vector& additionalHarbors = std::vector()); - static bool PlaceHQs(GameWorldBase& world, std::vector hqPositions, bool randomStartPos); + static bool PlaceHQs(GameWorldBase& world, const std::vector& hqPositions); }; diff --git a/tests/s25Main/lua/GameWithLuaAccess.h b/tests/s25Main/lua/GameWithLuaAccess.h index 1b6e8cd8af..1c26af39f3 100644 --- a/tests/s25Main/lua/GameWithLuaAccess.h +++ b/tests/s25Main/lua/GameWithLuaAccess.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 @@ -96,7 +96,7 @@ struct LuaTestsFixture : public rttr::test::LogAccessor, public LuaBaseFixture, std::vector playerNations; playerNations.push_back(world.GetPlayer(0).nation); playerNations.push_back(world.GetPlayer(1).nation); - BOOST_TEST_REQUIRE(MapLoader::PlaceHQs(world, hqPositions, false)); + BOOST_TEST_REQUIRE(MapLoader::PlaceHQs(world, hqPositions)); for(unsigned id = 0; id < world.GetNumPlayers(); id++) { diff --git a/tests/s25Main/worldFixtures/CreateEmptyWorld.cpp b/tests/s25Main/worldFixtures/CreateEmptyWorld.cpp index e0d603dd31..e74a25f73e 100644 --- a/tests/s25Main/worldFixtures/CreateEmptyWorld.cpp +++ b/tests/s25Main/worldFixtures/CreateEmptyWorld.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 @@ -52,7 +52,7 @@ bool CreateEmptyWorld::operator()(GameWorld& world) const } curPt.y += playerDist.y; } - if(!MapLoader::PlaceHQs(world, hqPositions, false)) + if(!MapLoader::PlaceHQs(world, hqPositions)) return false; // LCOV_EXCL_LINE } world.InitAfterLoad(); diff --git a/tests/s25Main/worldFixtures/CreateSeaWorld.cpp b/tests/s25Main/worldFixtures/CreateSeaWorld.cpp index 95a19a6726..0e5bdedee3 100644 --- a/tests/s25Main/worldFixtures/CreateSeaWorld.cpp +++ b/tests/s25Main/worldFixtures/CreateSeaWorld.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2005 - 2024 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 @@ -141,7 +141,7 @@ bool CreateSeaWorld::operator()(GameWorld& world) const BOOST_TEST_REQUIRE(MapLoader::InitSeasAndHarbors(world, harbors)); - if(!MapLoader::PlaceHQs(world, hqPositions, false)) + if(!MapLoader::PlaceHQs(world, hqPositions)) return false; // LCOV_EXCL_LINE world.InitAfterLoad(); @@ -194,7 +194,7 @@ bool CreateWaterWorld::operator()(GameWorld& world) const node.t1 = node.t2 = t; } } - BOOST_TEST_REQUIRE(MapLoader::PlaceHQs(world, hqPositions, false)); + BOOST_TEST_REQUIRE(MapLoader::PlaceHQs(world, hqPositions)); std::vector harbors; for(MapPoint hqPos : hqPositions) From 1e63f1cff11b9654c3347194220f22a9702ef4d7 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 3 Apr 2026 18:00:29 +0200 Subject: [PATCH 08/19] Add factory functions for Good/PeopleCounts --- libs/s25main/figures/nofTradeDonkey.cpp | 6 +----- libs/s25main/gameTypes/GoodsAndPeopleArray.h | 14 ++++++++++++++ libs/s25main/nodeObjs/noShip.cpp | 3 +-- tests/s25Main/integration/testAttacking.cpp | 7 ++----- tests/s25Main/integration/testBaseWarehouse.cpp | 8 ++------ tests/s25Main/integration/testEconomyMode.cpp | 10 ++++------ tests/s25Main/integration/testFigures.cpp | 4 +--- tests/s25Main/integration/testProduction.cpp | 4 +--- tests/s25Main/integration/testSeaAttacking.cpp | 8 ++------ tests/s25Main/integration/testSeafaring.cpp | 4 +--- 10 files changed, 29 insertions(+), 39 deletions(-) diff --git a/libs/s25main/figures/nofTradeDonkey.cpp b/libs/s25main/figures/nofTradeDonkey.cpp index 2cf65861a3..d9332530ab 100644 --- a/libs/s25main/figures/nofTradeDonkey.cpp +++ b/libs/s25main/figures/nofTradeDonkey.cpp @@ -53,11 +53,7 @@ void nofTradeDonkey::GoalReached() GamePlayer& whOwner = world->GetPlayer(wh->GetPlayer()); if(gt) - { - GoodCounts goods; - goods[*gt] = 1; - wh->AddToInventory(goods, true); - } + wh->AddToInventory(GoodCounts::make(*gt, 1), true); whOwner.IncreaseInventoryJob(this->GetJobType(), 1); if(hasArmor_) diff --git a/libs/s25main/gameTypes/GoodsAndPeopleArray.h b/libs/s25main/gameTypes/GoodsAndPeopleArray.h index 5562b81a14..ead78af52f 100644 --- a/libs/s25main/gameTypes/GoodsAndPeopleArray.h +++ b/libs/s25main/gameTypes/GoodsAndPeopleArray.h @@ -21,6 +21,14 @@ struct PeopleArray T& operator[](Job job) { return people[job]; } T& operator[](ArmoredSoldier soldier) { return armoredSoldiers[soldier]; } const T& operator[](ArmoredSoldier soldier) const { return armoredSoldiers[soldier]; } + + static PeopleArray make(Job job, T value, ArmoredSoldier armoredType = ArmoredSoldier(0), T armoredValue = 0) + { + PeopleArray res; + res[job] = value; + res[armoredType] = armoredValue; + return res; + } }; template @@ -29,6 +37,12 @@ struct GoodsArray helpers::EnumArray goods = {}; const T& operator[](GoodType good) const { return goods[good]; } T& operator[](GoodType good) { return goods[good]; } + static GoodsArray make(GoodType good, T value) + { + GoodsArray res; + res[good] = value; + return res; + } }; /// Combined array for goods and people with typed accessors diff --git a/libs/s25main/nodeObjs/noShip.cpp b/libs/s25main/nodeObjs/noShip.cpp index 95033c7c84..2c71f1719d 100644 --- a/libs/s25main/nodeObjs/noShip.cpp +++ b/libs/s25main/nodeObjs/noShip.cpp @@ -277,8 +277,7 @@ void noShip::HandleEvent(const unsigned id) if(hb && hb->GetGOT() == GO_Type::NobHarborbuilding) { // Späher wieder entladen - PeopleCounts people; - people[Job::Scout] = world->GetGGS().GetNumScoutsExpedition(); + const auto people = PeopleCounts::make(Job::Scout, world->GetGGS().GetNumScoutsExpedition()); static_cast(hb)->AddToInventory(people, false); // Wieder idlen und ggf. neuen Job suchen StartIdling(); diff --git a/tests/s25Main/integration/testAttacking.cpp b/tests/s25Main/integration/testAttacking.cpp index e2e2be32bb..552909beb8 100644 --- a/tests/s25Main/integration/testAttacking.cpp +++ b/tests/s25Main/integration/testAttacking.cpp @@ -95,8 +95,7 @@ struct AttackFixtureBase : public WorldWithGCExecution(hqPos[playerIdx])->AddToInventory(inv, true); + world.GetSpecObj(hqPos[playerIdx]) + ->AddToInventory(GoodCounts::make(goodsToCollect[0], amountsToAdd[playerIdx]), true); } world.GetEvMgr().ExecuteNextGF(); econHandler.UpdateAmounts(); @@ -112,9 +111,8 @@ BOOST_FIXTURE_TEST_CASE(EconomyModeSerialization, EconModeFixture) const auto& goodsToCollect = world.getEconHandler()->GetGoodTypesToCollect(); for(unsigned playerIdx = 0; playerIdx < 3; ++playerIdx) { - GoodCounts inv; - inv[goodsToCollect[0]] = rttr::test::randomValue(1u, 50u); - world.GetSpecObj(hqPos[playerIdx])->AddToInventory(inv, true); + world.GetSpecObj(hqPos[playerIdx]) + ->AddToInventory(GoodCounts::make(goodsToCollect[0], rttr::test::randomValue(1u, 50u)), true); } world.GetEvMgr().ExecuteNextGF(); world.getEconHandler()->UpdateAmounts(); diff --git a/tests/s25Main/integration/testFigures.cpp b/tests/s25Main/integration/testFigures.cpp index 72b71f7fd5..c793c490a9 100644 --- a/tests/s25Main/integration/testFigures.cpp +++ b/tests/s25Main/integration/testFigures.cpp @@ -27,9 +27,7 @@ BOOST_FIXTURE_TEST_CASE(DestroyWHWithFigure, WorldWithGCExecution2P) MapPoint whPos(flagPos.x + 5, flagPos.y); auto* wh = static_cast( BuildingFactory::CreateBuilding(world, BuildingType::Storehouse, whPos, curPlayer, Nation::Romans)); - PeopleCounts inv; - inv[Job::Helper] = 1; - wh->AddToInventory(inv, true); + wh->AddToInventory(PeopleCounts::make(Job::Helper, 1), true); const unsigned numHelpers = world.GetPlayer(curPlayer).GetInventory().people[Job::Helper]; //-V807 MapPoint whFlagPos = world.GetNeighbour(whPos, Direction::SouthEast); // Build a road -> Requests a worker diff --git a/tests/s25Main/integration/testProduction.cpp b/tests/s25Main/integration/testProduction.cpp index 89685d678b..b4d4665ad7 100644 --- a/tests/s25Main/integration/testProduction.cpp +++ b/tests/s25Main/integration/testProduction.cpp @@ -28,9 +28,7 @@ BOOST_FIXTURE_TEST_CASE(MetalWorkerStopped, WorldWithGCExecution1P) rttr::test::LogAccessor logAcc; ggs.setSelection(AddonId::TOOL_ORDERING, 1); ggs.setSelection(AddonId::METALWORKSBEHAVIORONZERO, 1); - GoodCounts goods; - goods[GoodType::Iron] = 10; - world.GetSpecObj(hqPos)->AddToInventory(goods, true); + world.GetSpecObj(hqPos)->AddToInventory(GoodCounts::make(GoodType::Iron, 10), true); MapPoint bldPos = hqPos + MapPoint(2, 0); BuildingFactory::CreateBuilding(world, BuildingType::Metalworks, bldPos, curPlayer, Nation::Africans); this->BuildRoad(world.GetNeighbour(bldPos, Direction::SouthEast), false, diff --git a/tests/s25Main/integration/testSeaAttacking.cpp b/tests/s25Main/integration/testSeaAttacking.cpp index 8fa710144b..632913cb8e 100644 --- a/tests/s25Main/integration/testSeaAttacking.cpp +++ b/tests/s25Main/integration/testSeaAttacking.cpp @@ -78,9 +78,7 @@ struct SeaAttackFixture : public SeaWorldWithGCExecution<3, 62, 64> SetCurPlayer(i); hqPos[i] = world.GetPlayer(i).GetHQPos(); auto* hq = world.GetSpecObj(hqPos[i]); - PeopleCounts soldiers; - soldiers[Job::General] = 3; - hq->AddToInventory(soldiers, true); + hq->AddToInventory(PeopleCounts::make(Job::General, 3), true); this->ChangeMilitary(MILITARY_SETTINGS_SCALE); } // Assert player positions: 0: Top, 1: Left, 2: Right @@ -425,9 +423,7 @@ BOOST_FIXTURE_TEST_CASE(AttackHarbor, SeaAttackFixture) BuildRoadForBlds(milBld2Pos, hqPos[2]); // Add 1 soldier to dest harbor so we have a defender nobHarborBuilding& hbDest = *world.GetSpecObj(harborPos[1]); - PeopleCounts soldiers; - soldiers[Job::Sergeant] = 1; - hbDest.AddToInventory(soldiers, true); + hbDest.AddToInventory(PeopleCounts::make(Job::Sergeant, 1), true); // Don't keep him in reserve hbDest.SetRealReserve(getSoldierRank(Job::Sergeant), 0); BOOST_TEST_REQUIRE(hbDest.GetNumVisualFigures(Job::Sergeant) == 1u); diff --git a/tests/s25Main/integration/testSeafaring.cpp b/tests/s25Main/integration/testSeafaring.cpp index 0484b2ecc4..a4be0232c8 100644 --- a/tests/s25Main/integration/testSeafaring.cpp +++ b/tests/s25Main/integration/testSeafaring.cpp @@ -467,9 +467,7 @@ BOOST_FIXTURE_TEST_CASE(LongDistanceTravel, ShipReadyFixtureBig) // Make sure that the other harbor is far away BOOST_TEST_REQUIRE(world.CalcHarborDistance(HarborId(2), targetHbId) > 600u); // Add some scouts - PeopleCounts newScouts; - newScouts[Job::Scout] = 20; - harbor.AddToInventory(newScouts, true); + harbor.AddToInventory(PeopleCounts::make(Job::Scout, 20), true); // We want the ship to only scout unexplored harbors, so set all but one to visible for(const auto i : helpers::idRange(world.GetNumHarborPoints())) world.GetNodeWriteable(world.GetHarborPoint(i)).fow[curPlayer].visibility = Visibility::Visible; From b677ec2e6181c80323f638e5e08d5192ae5c4c2d Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 3 Apr 2026 18:58:23 +0200 Subject: [PATCH 09/19] Do not add start resources when creating HQ This is not always desired, especially in the tests. So add an extra method for that to allow adding only wares/people when desired. --- libs/s25main/Cheats.cpp | 6 +- libs/s25main/GamePlayer.cpp | 12 +- libs/s25main/GamePlayer.h | 3 +- libs/s25main/buildings/nobHQ.cpp | 569 +++++++++--------- libs/s25main/buildings/nobHQ.h | 11 +- libs/s25main/world/MapLoader.cpp | 12 +- libs/s25main/world/MapLoader.h | 8 +- tests/s25Main/integration/testAI.cpp | 3 +- tests/s25Main/integration/testArmor.cpp | 5 +- tests/s25Main/integration/testAttacking.cpp | 124 ++-- .../s25Main/integration/testBaseWarehouse.cpp | 10 + tests/s25Main/integration/testFarmer.cpp | 3 +- tests/s25Main/integration/testFigures.cpp | 2 + .../s25Main/integration/testGameCommands.cpp | 4 + tests/s25Main/integration/testHarbor.cpp | 4 +- tests/s25Main/integration/testProduction.cpp | 10 +- tests/s25Main/integration/testSeafaring.cpp | 4 + tests/s25Main/integration/testTrading.cpp | 3 +- tests/s25Main/lua/GameWithLuaAccess.h | 2 +- tests/s25Main/lua/testLua.cpp | 20 +- .../worldFixtures/CreateEmptyWorld.cpp | 18 +- .../s25Main/worldFixtures/CreateSeaWorld.cpp | 5 +- tests/s25Main/worldFixtures/WorldFixture.h | 35 +- 23 files changed, 464 insertions(+), 409 deletions(-) diff --git a/libs/s25main/Cheats.cpp b/libs/s25main/Cheats.cpp index d3fa6549bf..a80c7dd502 100644 --- a/libs/s25main/Cheats.cpp +++ b/libs/s25main/Cheats.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2024-2026 Settlers Freaks (sf-team at siedler25.org) +// Copyright (C) 2024 - 2026 Settlers Freaks (sf-team at siedler25.org) // // SPDX-License-Identifier: GPL-2.0-or-later @@ -77,12 +77,10 @@ void Cheats::placeCheatBuilding(const MapPoint& mp, const GamePlayer& player) if(!canPlaceCheatBuilding(mp)) return; - // The new HQ will have default resources. - // In the original game, new HQs created in the Roman campaign had no resources. world_.DestroyNO(mp, false); // if CanPlaceCheatBuilding is true then this must be safe to destroy auto* hq = BuildingFactory::CreateBuilding(world_, BuildingType::Headquarters, mp, player.GetPlayerId(), player.nation); - static_cast(hq)->SetIsTent(player.IsHQTent()); + checkedCast(hq)->SetIsTent(player.IsHQTent()); } void Cheats::toggleHumanAIPlayer() diff --git a/libs/s25main/GamePlayer.cpp b/libs/s25main/GamePlayer.cpp index c669f53ceb..b0cb04c01a 100644 --- a/libs/s25main/GamePlayer.cpp +++ b/libs/s25main/GamePlayer.cpp @@ -507,7 +507,7 @@ void GamePlayer::RemoveBuilding(noBuilding* bld, BuildingType bldType) { // Schiffen Bescheid sagen for(noShip* ship : ships) ship->HarborDestroyed(static_cast(bld)); - } else if(bldType == BuildingType::Headquarters) + } else if(bldType == BuildingType::Headquarters && bld->GetPos() == hqPos) { hqPos = MapPoint::Invalid(); for(const noBaseBuilding* bld : buildings.GetStorehouses()) @@ -1497,10 +1497,14 @@ void GamePlayer::TestDefeat() Surrender(); } -nobHQ* GamePlayer::GetHQ() const +const nobHQ* GamePlayer::GetHQ() const +{ + return hqPos.isValid() ? GetGameWorld().GetSpecObj(hqPos) : nullptr; +} + +nobHQ* GamePlayer::GetHQ() { - const MapPoint& hqPos = GetHQPos(); - return const_cast(hqPos.isValid() ? GetGameWorld().GetSpecObj(hqPos) : nullptr); + return const_cast(std::as_const(*this).GetHQ()); } void GamePlayer::Surrender() diff --git a/libs/s25main/GamePlayer.h b/libs/s25main/GamePlayer.h index acc813e06f..7c9e59f524 100644 --- a/libs/s25main/GamePlayer.h +++ b/libs/s25main/GamePlayer.h @@ -93,6 +93,8 @@ class GamePlayer : public GamePlayerInfo const GameWorld& GetGameWorld() const { return world; } const MapPoint& GetHQPos() const { return hqPos; } + const nobHQ* GetHQ() const; + nobHQ* GetHQ(); bool IsHQTent() const; void SetHQIsTent(bool isTent); @@ -444,7 +446,6 @@ class GamePlayer : public GamePlayerInfo /// Prüft, ob der Spieler besiegt wurde void TestDefeat(); - nobHQ* GetHQ() const; ////////////////////////////////////////////////////////////////////////// /// Unsynchronized state (e.g. lua, gui...) diff --git a/libs/s25main/buildings/nobHQ.cpp b/libs/s25main/buildings/nobHQ.cpp index 4f27d6b91a..ae395d1711 100644 --- a/libs/s25main/buildings/nobHQ.cpp +++ b/libs/s25main/buildings/nobHQ.cpp @@ -16,311 +16,310 @@ nobHQ::nobHQ(const MapPoint pos, const unsigned char player, const Nation nation, const bool isTent) : nobBaseWarehouse(BuildingType::Headquarters, pos, player, nation), isTent_(isTent) { - // StartWaren setzen - switch(world->GetGGS().startWares) + // Take 1 as the reserve per rank + for(unsigned i = 0; i <= world->GetGGS().GetMaxMilitaryRank(); ++i) + { + reserve_soldiers_claimed_visual[i] = reserve_soldiers_claimed_real[i] = 1; + RefreshReserve(i); + } + + // Evtl. liegen am Anfang Waffen im HQ, sodass rekrutiert werden muss + TryRecruiting(); + + // ins Militärquadrat einfügen + world->GetMilitarySquares().Add(this); + world->RecalcTerritory(*this, TerritoryChangeReason::Build); +} + +GoodsAndPeopleCounts nobHQ::getStartInventory(StartWares setting) +{ + GoodsAndPeopleCounts inventory; + switch(setting) { case StartWares::VLow: - inventory.visual.goods[GoodType::Beer] = 0; - inventory.visual.goods[GoodType::Tongs] = 1; - inventory.visual.goods[GoodType::Hammer] = 4; - inventory.visual.goods[GoodType::Axe] = 1; - inventory.visual.goods[GoodType::Saw] = 0; - inventory.visual.goods[GoodType::PickAxe] = 0; - inventory.visual.goods[GoodType::Shovel] = 1; - inventory.visual.goods[GoodType::Crucible] = 1; - inventory.visual.goods[GoodType::RodAndLine] = 1; //?? - inventory.visual.goods[GoodType::Scythe] = 2; //?? - inventory.visual.goods[GoodType::WaterEmpty] = 0; - inventory.visual.goods[GoodType::Water] = 0; - inventory.visual.goods[GoodType::Cleaver] = 0; - inventory.visual.goods[GoodType::Rollingpin] = 1; - inventory.visual.goods[GoodType::Bow] = 0; - inventory.visual.goods[GoodType::Boat] = 0; - inventory.visual.goods[GoodType::Sword] = 0; - inventory.visual.goods[GoodType::Iron] = 0; - inventory.visual.goods[GoodType::Flour] = 0; - inventory.visual.goods[GoodType::Fish] = 1; - inventory.visual.goods[GoodType::Bread] = 2; - inventory.visual.goods[GoodType::ShieldRomans] = 0; - inventory.visual.goods[GoodType::Wood] = 6; - inventory.visual.goods[GoodType::Boards] = 11; - inventory.visual.goods[GoodType::Stones] = 17; - inventory.visual.goods[GoodType::ShieldVikings] = 0; - inventory.visual.goods[GoodType::ShieldAfricans] = 0; - inventory.visual.goods[GoodType::Grain] = 0; - inventory.visual.goods[GoodType::Coins] = 0; - inventory.visual.goods[GoodType::Gold] = 0; - inventory.visual.goods[GoodType::IronOre] = 4; - inventory.visual.goods[GoodType::Coal] = 4; - inventory.visual.goods[GoodType::Meat] = 0; - inventory.visual.goods[GoodType::Ham] = 0; - inventory.visual.goods[GoodType::ShieldJapanese] = 0; + inventory[GoodType::Beer] = 0; + inventory[GoodType::Tongs] = 1; + inventory[GoodType::Hammer] = 4; + inventory[GoodType::Axe] = 1; + inventory[GoodType::Saw] = 0; + inventory[GoodType::PickAxe] = 0; + inventory[GoodType::Shovel] = 1; + inventory[GoodType::Crucible] = 1; + inventory[GoodType::RodAndLine] = 1; //?? + inventory[GoodType::Scythe] = 2; //?? + inventory[GoodType::WaterEmpty] = 0; + inventory[GoodType::Water] = 0; + inventory[GoodType::Cleaver] = 0; + inventory[GoodType::Rollingpin] = 1; + inventory[GoodType::Bow] = 0; + inventory[GoodType::Boat] = 0; + inventory[GoodType::Sword] = 0; + inventory[GoodType::Iron] = 0; + inventory[GoodType::Flour] = 0; + inventory[GoodType::Fish] = 1; + inventory[GoodType::Bread] = 2; + inventory[GoodType::ShieldRomans] = 0; + inventory[GoodType::Wood] = 6; + inventory[GoodType::Boards] = 11; + inventory[GoodType::Stones] = 17; + inventory[GoodType::ShieldVikings] = 0; + inventory[GoodType::ShieldAfricans] = 0; + inventory[GoodType::Grain] = 0; + inventory[GoodType::Coins] = 0; + inventory[GoodType::Gold] = 0; + inventory[GoodType::IronOre] = 4; + inventory[GoodType::Coal] = 4; + inventory[GoodType::Meat] = 0; + inventory[GoodType::Ham] = 0; + inventory[GoodType::ShieldJapanese] = 0; - inventory.visual.people[Job::Helper] = 13; - inventory.visual.people[Job::Woodcutter] = 2; - inventory.visual.people[Job::Fisher] = 0; - inventory.visual.people[Job::Forester] = 1; - inventory.visual.people[Job::Carpenter] = 1; - inventory.visual.people[Job::Stonemason] = 1; - inventory.visual.people[Job::Hunter] = 1; - inventory.visual.people[Job::Farmer] = 0; - inventory.visual.people[Job::Miller] = 0; - inventory.visual.people[Job::Baker] = 0; - inventory.visual.people[Job::Butcher] = 0; - inventory.visual.people[Job::Miner] = 2; - inventory.visual.people[Job::Brewer] = 0; - inventory.visual.people[Job::PigBreeder] = 0; - inventory.visual.people[Job::DonkeyBreeder] = 0; - inventory.visual.people[Job::IronFounder] = 0; - inventory.visual.people[Job::Minter] = 0; - inventory.visual.people[Job::Metalworker] = 0; - inventory.visual.people[Job::Armorer] = 1; - inventory.visual.people[Job::Builder] = 2; - inventory.visual.people[Job::Planer] = 1; - inventory.visual.people[Job::Private] = 13; - inventory.visual.people[Job::PrivateFirstClass] = 0; - inventory.visual.people[Job::Sergeant] = 0; - inventory.visual.people[Job::Officer] = 0; - inventory.visual.people[Job::General] = 0; - inventory.visual.people[Job::Geologist] = 2; - inventory.visual.people[Job::Shipwright] = 0; - inventory.visual.people[Job::Scout] = 1; - inventory.visual.people[Job::PackDonkey] = 2; + inventory[Job::Helper] = 13; + inventory[Job::Woodcutter] = 2; + inventory[Job::Fisher] = 0; + inventory[Job::Forester] = 1; + inventory[Job::Carpenter] = 1; + inventory[Job::Stonemason] = 1; + inventory[Job::Hunter] = 1; + inventory[Job::Farmer] = 0; + inventory[Job::Miller] = 0; + inventory[Job::Baker] = 0; + inventory[Job::Butcher] = 0; + inventory[Job::Miner] = 2; + inventory[Job::Brewer] = 0; + inventory[Job::PigBreeder] = 0; + inventory[Job::DonkeyBreeder] = 0; + inventory[Job::IronFounder] = 0; + inventory[Job::Minter] = 0; + inventory[Job::Metalworker] = 0; + inventory[Job::Armorer] = 1; + inventory[Job::Builder] = 2; + inventory[Job::Planer] = 1; + inventory[Job::Private] = 13; + inventory[Job::PrivateFirstClass] = 0; + inventory[Job::Sergeant] = 0; + inventory[Job::Officer] = 0; + inventory[Job::General] = 0; + inventory[Job::Geologist] = 2; + inventory[Job::Shipwright] = 0; + inventory[Job::Scout] = 1; + inventory[Job::PackDonkey] = 2; break; case StartWares::Low: - inventory.visual.goods[GoodType::Beer] = 0; - inventory.visual.goods[GoodType::Tongs] = 0; - inventory.visual.goods[GoodType::Hammer] = 8; - inventory.visual.goods[GoodType::Axe] = 3; - inventory.visual.goods[GoodType::Saw] = 1; - inventory.visual.goods[GoodType::PickAxe] = 1; - inventory.visual.goods[GoodType::Shovel] = 2; - inventory.visual.goods[GoodType::Crucible] = 2; - inventory.visual.goods[GoodType::RodAndLine] = 3; - inventory.visual.goods[GoodType::Scythe] = 4; - inventory.visual.goods[GoodType::WaterEmpty] = 0; - inventory.visual.goods[GoodType::Water] = 0; - inventory.visual.goods[GoodType::Cleaver] = 1; - inventory.visual.goods[GoodType::Rollingpin] = 1; - inventory.visual.goods[GoodType::Bow] = 1; - inventory.visual.goods[GoodType::Boat] = 6; - inventory.visual.goods[GoodType::Sword] = 0; - inventory.visual.goods[GoodType::Iron] = 0; - inventory.visual.goods[GoodType::Flour] = 0; - inventory.visual.goods[GoodType::Fish] = 2; - inventory.visual.goods[GoodType::Bread] = 4; - inventory.visual.goods[GoodType::ShieldRomans] = 0; - inventory.visual.goods[GoodType::Wood] = 12; - inventory.visual.goods[GoodType::Boards] = 22; - inventory.visual.goods[GoodType::Stones] = 34; - inventory.visual.goods[GoodType::ShieldVikings] = 0; - inventory.visual.goods[GoodType::ShieldAfricans] = 0; - inventory.visual.goods[GoodType::Grain] = 0; - inventory.visual.goods[GoodType::Coins] = 0; - inventory.visual.goods[GoodType::Gold] = 0; - inventory.visual.goods[GoodType::IronOre] = 8; - inventory.visual.goods[GoodType::Coal] = 8; - inventory.visual.goods[GoodType::Meat] = 3; - inventory.visual.goods[GoodType::Ham] = 0; - inventory.visual.goods[GoodType::ShieldJapanese] = 0; + inventory[GoodType::Beer] = 0; + inventory[GoodType::Tongs] = 0; + inventory[GoodType::Hammer] = 8; + inventory[GoodType::Axe] = 3; + inventory[GoodType::Saw] = 1; + inventory[GoodType::PickAxe] = 1; + inventory[GoodType::Shovel] = 2; + inventory[GoodType::Crucible] = 2; + inventory[GoodType::RodAndLine] = 3; + inventory[GoodType::Scythe] = 4; + inventory[GoodType::WaterEmpty] = 0; + inventory[GoodType::Water] = 0; + inventory[GoodType::Cleaver] = 1; + inventory[GoodType::Rollingpin] = 1; + inventory[GoodType::Bow] = 1; + inventory[GoodType::Boat] = 6; + inventory[GoodType::Sword] = 0; + inventory[GoodType::Iron] = 0; + inventory[GoodType::Flour] = 0; + inventory[GoodType::Fish] = 2; + inventory[GoodType::Bread] = 4; + inventory[GoodType::ShieldRomans] = 0; + inventory[GoodType::Wood] = 12; + inventory[GoodType::Boards] = 22; + inventory[GoodType::Stones] = 34; + inventory[GoodType::ShieldVikings] = 0; + inventory[GoodType::ShieldAfricans] = 0; + inventory[GoodType::Grain] = 0; + inventory[GoodType::Coins] = 0; + inventory[GoodType::Gold] = 0; + inventory[GoodType::IronOre] = 8; + inventory[GoodType::Coal] = 8; + inventory[GoodType::Meat] = 3; + inventory[GoodType::Ham] = 0; + inventory[GoodType::ShieldJapanese] = 0; - inventory.visual.people[Job::Helper] = 26; - inventory.visual.people[Job::Woodcutter] = 4; - inventory.visual.people[Job::Fisher] = 0; - inventory.visual.people[Job::Forester] = 2; - inventory.visual.people[Job::Carpenter] = 2; - inventory.visual.people[Job::Stonemason] = 2; - inventory.visual.people[Job::Hunter] = 1; - inventory.visual.people[Job::Farmer] = 0; - inventory.visual.people[Job::Miller] = 0; - inventory.visual.people[Job::Baker] = 0; - inventory.visual.people[Job::Butcher] = 0; - inventory.visual.people[Job::Miner] = 5; - inventory.visual.people[Job::Brewer] = 0; - inventory.visual.people[Job::PigBreeder] = 0; - inventory.visual.people[Job::DonkeyBreeder] = 0; - inventory.visual.people[Job::IronFounder] = 0; - inventory.visual.people[Job::Minter] = 0; - inventory.visual.people[Job::Metalworker] = 1; - inventory.visual.people[Job::Armorer] = 2; - inventory.visual.people[Job::Builder] = 5; - inventory.visual.people[Job::Planer] = 3; - inventory.visual.people[Job::Private] = 26; - inventory.visual.people[Job::PrivateFirstClass] = 0; - inventory.visual.people[Job::Sergeant] = 0; - inventory.visual.people[Job::Officer] = 0; - inventory.visual.people[Job::General] = 0; - inventory.visual.people[Job::Geologist] = 3; - inventory.visual.people[Job::Shipwright] = 0; - inventory.visual.people[Job::Scout] = 1; - inventory.visual.people[Job::PackDonkey] = 4; + inventory[Job::Helper] = 26; + inventory[Job::Woodcutter] = 4; + inventory[Job::Fisher] = 0; + inventory[Job::Forester] = 2; + inventory[Job::Carpenter] = 2; + inventory[Job::Stonemason] = 2; + inventory[Job::Hunter] = 1; + inventory[Job::Farmer] = 0; + inventory[Job::Miller] = 0; + inventory[Job::Baker] = 0; + inventory[Job::Butcher] = 0; + inventory[Job::Miner] = 5; + inventory[Job::Brewer] = 0; + inventory[Job::PigBreeder] = 0; + inventory[Job::DonkeyBreeder] = 0; + inventory[Job::IronFounder] = 0; + inventory[Job::Minter] = 0; + inventory[Job::Metalworker] = 1; + inventory[Job::Armorer] = 2; + inventory[Job::Builder] = 5; + inventory[Job::Planer] = 3; + inventory[Job::Private] = 26; + inventory[Job::PrivateFirstClass] = 0; + inventory[Job::Sergeant] = 0; + inventory[Job::Officer] = 0; + inventory[Job::General] = 0; + inventory[Job::Geologist] = 3; + inventory[Job::Shipwright] = 0; + inventory[Job::Scout] = 1; + inventory[Job::PackDonkey] = 4; break; case StartWares::Normal: - inventory.visual.goods[GoodType::Beer] = 6; - inventory.visual.goods[GoodType::Tongs] = 0; - inventory.visual.goods[GoodType::Hammer] = 16; - inventory.visual.goods[GoodType::Axe] = 6; - inventory.visual.goods[GoodType::Saw] = 2; - inventory.visual.goods[GoodType::PickAxe] = 2; - inventory.visual.goods[GoodType::Shovel] = 4; - inventory.visual.goods[GoodType::Crucible] = 4; - inventory.visual.goods[GoodType::RodAndLine] = 6; - inventory.visual.goods[GoodType::Scythe] = 8; - inventory.visual.goods[GoodType::WaterEmpty] = 0; - inventory.visual.goods[GoodType::Water] = 0; - inventory.visual.goods[GoodType::Cleaver] = 2; - inventory.visual.goods[GoodType::Rollingpin] = 2; - inventory.visual.goods[GoodType::Bow] = 2; - inventory.visual.goods[GoodType::Boat] = 12; - inventory.visual.goods[GoodType::Sword] = 6; - inventory.visual.goods[GoodType::Iron] = 0; - inventory.visual.goods[GoodType::Flour] = 0; - inventory.visual.goods[GoodType::Fish] = 4; - inventory.visual.goods[GoodType::Bread] = 8; - inventory.visual.goods[GoodType::ShieldRomans] = 6; - inventory.visual.goods[GoodType::Wood] = 24; - inventory.visual.goods[GoodType::Boards] = 44; - inventory.visual.goods[GoodType::Stones] = 68; - inventory.visual.goods[GoodType::ShieldVikings] = 0; - inventory.visual.goods[GoodType::ShieldAfricans] = 0; - inventory.visual.goods[GoodType::Grain] = 0; - inventory.visual.goods[GoodType::Coins] = 0; - inventory.visual.goods[GoodType::Gold] = 0; - inventory.visual.goods[GoodType::IronOre] = 16; - inventory.visual.goods[GoodType::Coal] = 16; - inventory.visual.goods[GoodType::Meat] = 6; - inventory.visual.goods[GoodType::Ham] = 0; - inventory.visual.goods[GoodType::ShieldJapanese] = 0; + inventory[GoodType::Beer] = 6; + inventory[GoodType::Tongs] = 0; + inventory[GoodType::Hammer] = 16; + inventory[GoodType::Axe] = 6; + inventory[GoodType::Saw] = 2; + inventory[GoodType::PickAxe] = 2; + inventory[GoodType::Shovel] = 4; + inventory[GoodType::Crucible] = 4; + inventory[GoodType::RodAndLine] = 6; + inventory[GoodType::Scythe] = 8; + inventory[GoodType::WaterEmpty] = 0; + inventory[GoodType::Water] = 0; + inventory[GoodType::Cleaver] = 2; + inventory[GoodType::Rollingpin] = 2; + inventory[GoodType::Bow] = 2; + inventory[GoodType::Boat] = 12; + inventory[GoodType::Sword] = 6; + inventory[GoodType::Iron] = 0; + inventory[GoodType::Flour] = 0; + inventory[GoodType::Fish] = 4; + inventory[GoodType::Bread] = 8; + inventory[GoodType::ShieldRomans] = 6; + inventory[GoodType::Wood] = 24; + inventory[GoodType::Boards] = 44; + inventory[GoodType::Stones] = 68; + inventory[GoodType::ShieldVikings] = 0; + inventory[GoodType::ShieldAfricans] = 0; + inventory[GoodType::Grain] = 0; + inventory[GoodType::Coins] = 0; + inventory[GoodType::Gold] = 0; + inventory[GoodType::IronOre] = 16; + inventory[GoodType::Coal] = 16; + inventory[GoodType::Meat] = 6; + inventory[GoodType::Ham] = 0; + inventory[GoodType::ShieldJapanese] = 0; - inventory.visual.people[Job::Helper] = 52; - inventory.visual.people[Job::Woodcutter] = 8; - inventory.visual.people[Job::Fisher] = 0; - inventory.visual.people[Job::Forester] = 4; - inventory.visual.people[Job::Carpenter] = 4; - inventory.visual.people[Job::Stonemason] = 4; - inventory.visual.people[Job::Hunter] = 2; - inventory.visual.people[Job::Farmer] = 0; - inventory.visual.people[Job::Miller] = 0; - inventory.visual.people[Job::Baker] = 0; - inventory.visual.people[Job::Butcher] = 0; - inventory.visual.people[Job::Miner] = 10; - inventory.visual.people[Job::Brewer] = 0; - inventory.visual.people[Job::PigBreeder] = 0; - inventory.visual.people[Job::DonkeyBreeder] = 0; - inventory.visual.people[Job::IronFounder] = 0; - inventory.visual.people[Job::Minter] = 0; - inventory.visual.people[Job::Metalworker] = 2; - inventory.visual.people[Job::Armorer] = 4; - inventory.visual.people[Job::Builder] = 10; - inventory.visual.people[Job::Planer] = 6; - inventory.visual.people[Job::Private] = 46; - inventory.visual.people[Job::PrivateFirstClass] = 0; - inventory.visual.people[Job::Sergeant] = 0; - inventory.visual.people[Job::Officer] = 0; - inventory.visual.people[Job::General] = 0; - inventory.visual.people[Job::Geologist] = 6; - inventory.visual.people[Job::Shipwright] = 0; - inventory.visual.people[Job::Scout] = 2; - inventory.visual.people[Job::PackDonkey] = 8; + inventory[Job::Helper] = 52; + inventory[Job::Woodcutter] = 8; + inventory[Job::Fisher] = 0; + inventory[Job::Forester] = 4; + inventory[Job::Carpenter] = 4; + inventory[Job::Stonemason] = 4; + inventory[Job::Hunter] = 2; + inventory[Job::Farmer] = 0; + inventory[Job::Miller] = 0; + inventory[Job::Baker] = 0; + inventory[Job::Butcher] = 0; + inventory[Job::Miner] = 10; + inventory[Job::Brewer] = 0; + inventory[Job::PigBreeder] = 0; + inventory[Job::DonkeyBreeder] = 0; + inventory[Job::IronFounder] = 0; + inventory[Job::Minter] = 0; + inventory[Job::Metalworker] = 2; + inventory[Job::Armorer] = 4; + inventory[Job::Builder] = 10; + inventory[Job::Planer] = 6; + inventory[Job::Private] = 46; + inventory[Job::PrivateFirstClass] = 0; + inventory[Job::Sergeant] = 0; + inventory[Job::Officer] = 0; + inventory[Job::General] = 0; + inventory[Job::Geologist] = 6; + inventory[Job::Shipwright] = 0; + inventory[Job::Scout] = 2; + inventory[Job::PackDonkey] = 8; break; case StartWares::ALot: - inventory.visual.goods[GoodType::Beer] = 12; - inventory.visual.goods[GoodType::Tongs] = 0; - inventory.visual.goods[GoodType::Hammer] = 32; - inventory.visual.goods[GoodType::Axe] = 12; - inventory.visual.goods[GoodType::Saw] = 4; - inventory.visual.goods[GoodType::PickAxe] = 4; - inventory.visual.goods[GoodType::Shovel] = 8; - inventory.visual.goods[GoodType::Crucible] = 8; - inventory.visual.goods[GoodType::RodAndLine] = 12; - inventory.visual.goods[GoodType::Scythe] = 16; - inventory.visual.goods[GoodType::WaterEmpty] = 0; - inventory.visual.goods[GoodType::Water] = 0; - inventory.visual.goods[GoodType::Cleaver] = 4; - inventory.visual.goods[GoodType::Rollingpin] = 4; - inventory.visual.goods[GoodType::Bow] = 4; - inventory.visual.goods[GoodType::Boat] = 24; - inventory.visual.goods[GoodType::Sword] = 12; - inventory.visual.goods[GoodType::Iron] = 0; - inventory.visual.goods[GoodType::Flour] = 0; - inventory.visual.goods[GoodType::Fish] = 8; - inventory.visual.goods[GoodType::Bread] = 16; - inventory.visual.goods[GoodType::ShieldRomans] = 12; - inventory.visual.goods[GoodType::Wood] = 48; - inventory.visual.goods[GoodType::Boards] = 88; - inventory.visual.goods[GoodType::Stones] = 136; - inventory.visual.goods[GoodType::ShieldVikings] = 0; - inventory.visual.goods[GoodType::ShieldAfricans] = 0; - inventory.visual.goods[GoodType::Grain] = 0; - inventory.visual.goods[GoodType::Coins] = 0; - inventory.visual.goods[GoodType::Gold] = 0; - inventory.visual.goods[GoodType::IronOre] = 32; - inventory.visual.goods[GoodType::Coal] = 32; - inventory.visual.goods[GoodType::Meat] = 12; - inventory.visual.goods[GoodType::Ham] = 0; - inventory.visual.goods[GoodType::ShieldJapanese] = 0; + inventory[GoodType::Beer] = 12; + inventory[GoodType::Tongs] = 0; + inventory[GoodType::Hammer] = 32; + inventory[GoodType::Axe] = 12; + inventory[GoodType::Saw] = 4; + inventory[GoodType::PickAxe] = 4; + inventory[GoodType::Shovel] = 8; + inventory[GoodType::Crucible] = 8; + inventory[GoodType::RodAndLine] = 12; + inventory[GoodType::Scythe] = 16; + inventory[GoodType::WaterEmpty] = 0; + inventory[GoodType::Water] = 0; + inventory[GoodType::Cleaver] = 4; + inventory[GoodType::Rollingpin] = 4; + inventory[GoodType::Bow] = 4; + inventory[GoodType::Boat] = 24; + inventory[GoodType::Sword] = 12; + inventory[GoodType::Iron] = 0; + inventory[GoodType::Flour] = 0; + inventory[GoodType::Fish] = 8; + inventory[GoodType::Bread] = 16; + inventory[GoodType::ShieldRomans] = 12; + inventory[GoodType::Wood] = 48; + inventory[GoodType::Boards] = 88; + inventory[GoodType::Stones] = 136; + inventory[GoodType::ShieldVikings] = 0; + inventory[GoodType::ShieldAfricans] = 0; + inventory[GoodType::Grain] = 0; + inventory[GoodType::Coins] = 0; + inventory[GoodType::Gold] = 0; + inventory[GoodType::IronOre] = 32; + inventory[GoodType::Coal] = 32; + inventory[GoodType::Meat] = 12; + inventory[GoodType::Ham] = 0; + inventory[GoodType::ShieldJapanese] = 0; - inventory.visual.people[Job::Helper] = 104; - inventory.visual.people[Job::Woodcutter] = 16; - inventory.visual.people[Job::Fisher] = 0; - inventory.visual.people[Job::Forester] = 8; - inventory.visual.people[Job::Carpenter] = 8; - inventory.visual.people[Job::Stonemason] = 8; - inventory.visual.people[Job::Hunter] = 4; - inventory.visual.people[Job::Farmer] = 0; - inventory.visual.people[Job::Miller] = 0; - inventory.visual.people[Job::Baker] = 0; - inventory.visual.people[Job::Butcher] = 0; - inventory.visual.people[Job::Miner] = 20; - inventory.visual.people[Job::Brewer] = 0; - inventory.visual.people[Job::PigBreeder] = 0; - inventory.visual.people[Job::DonkeyBreeder] = 0; - inventory.visual.people[Job::IronFounder] = 0; - inventory.visual.people[Job::Minter] = 0; - inventory.visual.people[Job::Metalworker] = 4; - inventory.visual.people[Job::Armorer] = 8; - inventory.visual.people[Job::Builder] = 20; - inventory.visual.people[Job::Planer] = 12; - inventory.visual.people[Job::Private] = 92; - inventory.visual.people[Job::PrivateFirstClass] = 0; - inventory.visual.people[Job::Sergeant] = 0; - inventory.visual.people[Job::Officer] = 0; - inventory.visual.people[Job::General] = 0; - inventory.visual.people[Job::Geologist] = 12; - inventory.visual.people[Job::Shipwright] = 0; - inventory.visual.people[Job::Scout] = 4; - inventory.visual.people[Job::PackDonkey] = 16; + inventory[Job::Helper] = 104; + inventory[Job::Woodcutter] = 16; + inventory[Job::Fisher] = 0; + inventory[Job::Forester] = 8; + inventory[Job::Carpenter] = 8; + inventory[Job::Stonemason] = 8; + inventory[Job::Hunter] = 4; + inventory[Job::Farmer] = 0; + inventory[Job::Miller] = 0; + inventory[Job::Baker] = 0; + inventory[Job::Butcher] = 0; + inventory[Job::Miner] = 20; + inventory[Job::Brewer] = 0; + inventory[Job::PigBreeder] = 0; + inventory[Job::DonkeyBreeder] = 0; + inventory[Job::IronFounder] = 0; + inventory[Job::Minter] = 0; + inventory[Job::Metalworker] = 4; + inventory[Job::Armorer] = 8; + inventory[Job::Builder] = 20; + inventory[Job::Planer] = 12; + inventory[Job::Private] = 92; + inventory[Job::PrivateFirstClass] = 0; + inventory[Job::Sergeant] = 0; + inventory[Job::Officer] = 0; + inventory[Job::General] = 0; + inventory[Job::Geologist] = 12; + inventory[Job::Shipwright] = 0; + inventory[Job::Scout] = 4; + inventory[Job::PackDonkey] = 16; break; } + return inventory; +} - inventory.real = inventory.visual; - - GamePlayer& owner = world->GetPlayer(player); - for(const auto i : helpers::enumRange()) - owner.IncreaseInventoryWare(i, inventory[i]); - for(const auto i : helpers::enumRange()) - owner.IncreaseInventoryJob(i, inventory[i]); - for(const auto i : helpers::enumRange()) - owner.IncreaseInventoryJob(i, inventory[i]); - - // Take 1 as the reserve per rank - for(unsigned i = 0; i <= world->GetGGS().GetMaxMilitaryRank(); ++i) - { - reserve_soldiers_claimed_visual[i] = reserve_soldiers_claimed_real[i] = 1; - RefreshReserve(i); - } - - // Evtl. liegen am Anfang Waffen im HQ, sodass rekrutiert werden muss - TryRecruiting(); - - // ins Militärquadrat einfügen - world->GetMilitarySquares().Add(this); - world->RecalcTerritory(*this, TerritoryChangeReason::Build); +void nobHQ::addStartWares() +{ + AddToInventory(getStartInventory(world->GetGGS().startWares), true); } void nobHQ::DestroyBuilding() diff --git a/libs/s25main/buildings/nobHQ.h b/libs/s25main/buildings/nobHQ.h index ae35766b99..26a8d7db08 100644 --- a/libs/s25main/buildings/nobHQ.h +++ b/libs/s25main/buildings/nobHQ.h @@ -1,10 +1,11 @@ -// 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 #pragma once #include "nobBaseWarehouse.h" +#include "gameTypes/GameSettingTypes.h" #include "gameData/MilitaryConsts.h" class SerializedGameData; @@ -17,10 +18,9 @@ class nobHQ : public nobBaseWarehouse nobHQ(MapPoint pos, unsigned char player, Nation nation, bool isTent = false); nobHQ(SerializedGameData& sgd, unsigned obj_id); -protected: - void DestroyBuilding() override; + static GoodsAndPeopleCounts getStartInventory(StartWares setting); + void addStartWares(); -public: void Serialize(SerializedGameData& sgd) const override; GO_Type GetGOT() const final { return GO_Type::NobHq; } @@ -32,4 +32,7 @@ class nobHQ : public nobBaseWarehouse void HandleEvent(unsigned id) override; bool IsTent() const { return isTent_; } void SetIsTent(const bool isTent) { isTent_ = isTent; } + +protected: + void DestroyBuilding() override; }; diff --git a/libs/s25main/world/MapLoader.cpp b/libs/s25main/world/MapLoader.cpp index 793361b1a2..49283ab43a 100644 --- a/libs/s25main/world/MapLoader.cpp +++ b/libs/s25main/world/MapLoader.cpp @@ -101,9 +101,9 @@ bool MapLoader::LoadLuaScript(Game& game, ILocalGameState& localgameState, const return true; } -bool MapLoader::PlaceHQs() +bool MapLoader::PlaceHQs(bool addStartWares) { - return PlaceHQs(world_, hqPositions_); + return PlaceHQs(world_, hqPositions_, addStartWares); } void MapLoader::InitShadows(World& world) @@ -397,7 +397,7 @@ void MapLoader::PlaceAnimals(const libsiedler2::ArchivItem_Map& map) } } -bool MapLoader::PlaceHQs(GameWorldBase& world, const std::vector& hqPositions) +bool MapLoader::PlaceHQs(GameWorldBase& world, const std::vector& hqPositions, const bool addStartWares) { for(unsigned i = 0; i < world.GetNumPlayers(); ++i) { @@ -412,8 +412,10 @@ bool MapLoader::PlaceHQs(GameWorldBase& world, const std::vector& hqPo return false; } - BuildingFactory::CreateBuilding(world, BuildingType::Headquarters, hqPositions[i], i, - world.GetPlayer(i).nation); + auto* hq = checkedCast(BuildingFactory::CreateBuilding(world, BuildingType::Headquarters, + hqPositions[i], i, world.GetPlayer(i).nation)); + if(addStartWares) + hq->addStartWares(); } return true; } diff --git a/libs/s25main/world/MapLoader.h b/libs/s25main/world/MapLoader.h index 39c0d6c233..9e9352cb74 100644 --- a/libs/s25main/world/MapLoader.h +++ b/libs/s25main/world/MapLoader.h @@ -47,7 +47,9 @@ class MapLoader bool Load(const boost::filesystem::path& mapFilePath); bool LoadLuaScript(Game& game, ILocalGameState& localgameState, const boost::filesystem::path& luaFilePath); /// Place the HQs on a loaded map (must be loaded first as hqPositions etc. are used) - bool PlaceHQs(); + /// Optionally add the starting wares to the HQs. + /// Return false if there was an error (e.g. invalid start position) + bool PlaceHQs(bool addStartWares = true); /// Return the position of the players HQ (only valid after successful load) MapPoint GetHQPos(unsigned player) const { return hqPositions_[player]; } @@ -56,5 +58,7 @@ class MapLoader static void SetMapExplored(World& world); static bool InitSeasAndHarbors(World& world, const std::vector& additionalHarbors = std::vector()); - static bool PlaceHQs(GameWorldBase& world, const std::vector& hqPositions); + /// Place the HQs on a loaded map and add starting wares if desired. + /// Return false if there was an error. + static bool PlaceHQs(GameWorldBase& world, const std::vector& hqPositions, bool addStartWares = true); }; diff --git a/tests/s25Main/integration/testAI.cpp b/tests/s25Main/integration/testAI.cpp index 8526ec1c7e..3efb3fef97 100644 --- a/tests/s25Main/integration/testAI.cpp +++ b/tests/s25Main/integration/testAI.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2005 - 2024 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 @@ -111,6 +111,7 @@ BOOST_FIXTURE_TEST_CASE(AIChat, EmptyWorldFixture2P) BOOST_FIXTURE_TEST_CASE(KeepBQUpdated, BiggerWorldWithGCExecution) { + addStartResources(); // Place some trees to reduce BQ at some points RTTR_FOREACH_PT(MapPoint, world.GetSize()) { diff --git a/tests/s25Main/integration/testArmor.cpp b/tests/s25Main/integration/testArmor.cpp index 403ac796de..6d0ce4a06a 100644 --- a/tests/s25Main/integration/testArmor.cpp +++ b/tests/s25Main/integration/testArmor.cpp @@ -42,6 +42,8 @@ struct ArmorTradeFixture : public ArmoredSoldierFixture ArmorTradeFixture() { curPlayer = 1; + addStartResources(); + world.GetPlayer(0).team = Team::Team1; //-V525 world.GetPlayer(1).team = Team::Team1; world.GetPlayer(2).team = Team::Team2; @@ -200,8 +202,7 @@ BOOST_AUTO_TEST_CASE(ArmorTradeFail) RTTR_SKIP_GFS(40); // Add a ring of enemy owned land so they cannot pass - std::vector pts = world.GetPointsInRadius(curWh->GetPos(), 10); - for(const MapPoint& pt : pts) + for(const MapPoint& pt : world.GetPointsInRadius(curWh->GetPos(), 10)) { if(world.CalcDistance(pt, curWh->GetPos()) >= 8) world.SetOwner(pt, 2 + 1); // playerID = 2 -> Owner = +1 diff --git a/tests/s25Main/integration/testAttacking.cpp b/tests/s25Main/integration/testAttacking.cpp index 552909beb8..ee3ff83d4d 100644 --- a/tests/s25Main/integration/testAttacking.cpp +++ b/tests/s25Main/integration/testAttacking.cpp @@ -108,20 +108,13 @@ struct AttackFixtureBase : public WorldWithGCExecution BOOST_FIXTURE_TEST_CASE(NumSoldiersForAttack, NumSoldierTestFixture) { - // Connect buildings - curPlayer = 1; - BuildRoadForBlds(hqPos[1], milBld1Near->GetPos()); - curPlayer = 0; - BuildRoadForBlds(hqPos[0], milBld0->GetPos()); - // Let soldiers get into blds. (6 soldiers, 7 fields distance, 20GFs per field, 30GFs for leaving HQ) - unsigned numGFs = 6 * (7 * 20 + 30); - RTTR_SKIP_GFS(numGFs); - // Don't wait for them just add - AddSoldiers(milBld1Far->GetPos(), 6, 0); - BOOST_TEST_REQUIRE(milBld1Near->GetNumTroops() == 6u); - BOOST_TEST_REQUIRE(milBld1Far->GetNumTroops() == 6u); - BOOST_TEST_REQUIRE(milBld0->GetNumTroops() == 6u); + AddSoldiers(milBld0Pos, 5, Job::General); + AddSoldiers(milBld0Pos, 1, Job::Sergeant); + AddSoldiers(milBld1NearPos, 6, Job::General); + AddSoldiers(milBld1FarPos, 6, Job::Private); + + // Functions related to stationed soldiers + BOOST_TEST_REQUIRE(milBld0->HasMaxRankSoldier()); + BOOST_TEST_REQUIRE(milBld1Near->HasMaxRankSoldier()); + BOOST_TEST_REQUIRE(!milBld1Far->HasMaxRankSoldier()); + BOOST_TEST_REQUIRE(milBld1Near->GetSoldiersStrength() > milBld1Far->GetSoldiersStrength()); + BOOST_TEST_REQUIRE(milBld1Near->GetSoldiersStrength() > milBld0->GetSoldiersStrength()); // Player 2 has no military blds -> Can't attack SetCurPlayer(2); BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(hqPos[0]) == 0u); BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(hqPos[1]) == 0u); - BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld0->GetPos()) == 0u); - BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld1Near->GetPos()) == 0u); - BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld1Far->GetPos()) == 0u); + BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld0Pos) == 0u); + BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld1NearPos) == 0u); + BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld1FarPos) == 0u); SetCurPlayer(1); // No self attack BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(hqPos[1]) == 0u); - BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld1Near->GetPos()) == 0u); - BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld1Far->GetPos()) == 0u); + BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld1NearPos) == 0u); + BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld1FarPos) == 0u); // Attack both others BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(hqPos[0]) == 5u); BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(hqPos[2]) == 5u); // This is in the extended range of the far bld -> 2 more (with current range) - BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld0->GetPos()) == 7u); + BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld0Pos) == 7u); SetCurPlayer(0); // No self attack BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(hqPos[0]) == 0u); - BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld0->GetPos()) == 0u); + BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld0Pos) == 0u); // Attack both others BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(hqPos[1]) == 5u); BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(hqPos[2]) == 5u); - BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld1Near->GetPos()) == 5u); + BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld1NearPos) == 5u); // Counterpart: 2 possible for far bld - BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld1Far->GetPos()) == 2u); + BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld1FarPos) == 2u); // Test in peaceful mode -- no attacks should be possible this->ggs.setSelection(AddonId::PEACEFULMODE, 1); SetCurPlayer(1); BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(hqPos[0]) == 0u); BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(hqPos[2]) == 0u); - BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld0->GetPos()) == 0u); + BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld0Pos) == 0u); SetCurPlayer(0); BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(hqPos[1]) == 0u); BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(hqPos[2]) == 0u); - BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld1Near->GetPos()) == 0u); - - // Functions related to stationed soldiers - BOOST_TEST_REQUIRE(milBld0->HasMaxRankSoldier()); - BOOST_TEST_REQUIRE(milBld1Near->HasMaxRankSoldier()); - BOOST_TEST_REQUIRE(!milBld1Far->HasMaxRankSoldier()); - BOOST_TEST_REQUIRE(milBld1Near->GetSoldiersStrength() > milBld1Far->GetSoldiersStrength()); + BOOST_TEST_REQUIRE(gwv.GetNumSoldiersForAttack(milBld1NearPos) == 0u); } BOOST_FIXTURE_TEST_CASE(StartAttack, AttackFixture<>) @@ -399,12 +384,15 @@ BOOST_FIXTURE_TEST_CASE(StartAttack, AttackFixture<>) BOOST_FIXTURE_TEST_CASE(ConquerBld, AttackFixture<>) { AddSoldiers(milBld0Pos, 1, 5); - AddSoldiersWithRankAndArmor(milBld1Pos, 1, Job::Private); - AddSoldiersWithRankAndArmor(milBld1Pos, 1, Job::PrivateFirstClass); + AddSoldiers(milBld1Pos, 1, Job::Private); + AddSoldiers(milBld1Pos, 1, Job::PrivateFirstClass); + + // Add soldiers and build road so that HQ and send additional soldiers to the building starting the attack + PeopleCounts soldiers; + soldiers[Job::Sergeant] = 10; + ensureNonNull(world.GetSpecObj(hqPos[curPlayer])).AddToInventory(soldiers, true); BuildRoadForBlds(milBld0Pos, hqPos[0]); - // Finish recruiting, carrier outhousing etc. - RTTR_SKIP_GFS(400); // Start attack ->1 (weak one first) this->Attack(milBld1Pos, 1, false); this->Attack(milBld1Pos, 5, false); @@ -444,10 +432,8 @@ BOOST_FIXTURE_TEST_CASE(ConquerBld, AttackFixture<>) BOOST_TEST_REQUIRE(milBld1->GetNumTroops() > 1u); // Weak soldier must be dead BOOST_TEST_REQUIRE(attackedPlInventory.people[Job::Private] == oldWeakSoldierCt - 1); - // Src building refill + // Src building gets refilled RTTR_EXEC_TILL(800, milBld0->GetNumTroops() == 6u); - // Src building got refilled - BOOST_TEST_REQUIRE(milBld0->GetNumTroops() == 6u); // We may have lost soldiers BOOST_TEST_REQUIRE(world.GetPlayer(curPlayer).GetInventory().people[Job::General] <= oldAttackerStrongSoldierCt); // The enemy may have lost his stronger soldier @@ -467,11 +453,8 @@ BOOST_FIXTURE_TEST_CASE(ConquerBld, AttackFixture<>) BOOST_FIXTURE_TEST_CASE(ArmoredSoldierLosesArmorInFight, AttackFixture<>) { - AddSoldiersWithRankAndArmor(milBld0Pos, 3, Job::General, 2); - AddSoldiersWithRankAndArmor(milBld1Pos, 1, Job::Private, 1); - - // Finish recruiting, carrier outhousing etc. - RTTR_SKIP_GFS(400); + AddSoldiers(milBld0Pos, 3, Job::General, 2); + AddSoldiers(milBld1Pos, 1, Job::Private, 1); BOOST_TEST_REQUIRE(milBld0->GetNumTroops() == 3u); BOOST_TEST_REQUIRE(milBld1->GetNumTroops() == 1u); @@ -528,9 +511,7 @@ BOOST_FIXTURE_TEST_CASE(ConquerBldCoinAddonEnable, AttackFixture<>) this->ggs.setSelection(AddonId::COINS_CAPTURED_BLD, 1); // addon is active on second run AddSoldiers(milBld0Pos, 1, 5); - AddSoldiersWithRankAndArmor(milBld1Pos, 1, Job::Private); - // Finish recruiting, carrier outhousing etc. - RTTR_SKIP_GFS(400); + AddSoldiers(milBld1Pos, 1, Job::Private); // ensure that coins are disabled milBld1->SetCoinsAllowed(false); @@ -551,9 +532,7 @@ BOOST_FIXTURE_TEST_CASE(ConquerBldCoinAddonDisable, AttackFixture<>) this->ggs.setSelection(AddonId::COINS_CAPTURED_BLD, 2); // addon is active on second run AddSoldiers(milBld0Pos, 1, 5); - AddSoldiersWithRankAndArmor(milBld1Pos, 1, Job::Private); - // Finish recruiting, carrier outhousing etc. - RTTR_SKIP_GFS(400); + AddSoldiers(milBld1Pos, 1, Job::Private); // ensure that coins are enabled milBld1->SetCoinsAllowed(true); @@ -589,7 +568,7 @@ BOOST_DATA_TEST_CASE_F(AttackFixture<>, ConquerBldArmorAddon, this->ggs.setSelection(AddonId::ARMOR_CAPTURED_BLD, 0); AddSoldiers(milBld0Pos, 1, 5); - AddSoldiersWithRankAndArmor(milBld1Pos, 1, Job::Private); + AddSoldiers(milBld1Pos, 1, Job::Private); // ensure that armor are enabled/disabled bool armorAllowedBeforeAttack = (sample == Case::KeepEnabled || sample == Case::DisableAfterEnable); @@ -621,7 +600,7 @@ BOOST_FIXTURE_TEST_CASE(ConquerWithMultipleWalkingIn, AttackFixture4P) this->ChangeMilitary(milSettings); AddSoldiers(milBld0Pos, 0, 6); - AddSoldiersWithRankAndArmor(milBld1Pos, 1, Job::Private); + AddSoldiers(milBld1Pos, 1, Job::Private); MapPoint milBld1FlagPos = world.GetNeighbour(milBld1Pos, Direction::SouthEast); // Scenario 1: Attack with one soldier. @@ -648,7 +627,7 @@ BOOST_FIXTURE_TEST_CASE(ConquerWithMultipleWalkingIn, AttackFixture4P) // Door opened BOOST_TEST_REQUIRE(milBld1->IsDoorOpen()); // New soldiers walked in - AddSoldiersWithRankAndArmor(milBld1Pos, 4, Job::Private); + AddSoldiers(milBld1Pos, 4, Job::Private); // Let attacker walk in (try it at least) RTTR_EXEC_TILL(20, attacker.GetPos() == milBld1Pos); RTTR_EXEC_TILL(20, attacker.GetPos() == milBld1FlagPos); @@ -683,7 +662,7 @@ BOOST_FIXTURE_TEST_CASE(ConquerWithMultipleWalkingIn, AttackFixture4P) MapPoint bldPos = hqPos[curPlayer] + MapPoint(3, 0); auto* alliedBld = static_cast( BuildingFactory::CreateBuilding(world, BuildingType::Guardhouse, bldPos, curPlayer, Nation::Africans)); - AddSoldiersWithRankAndArmor(bldPos, 2, Job::Private); + AddSoldiers(bldPos, 2, Job::Private); this->Attack(milBld1Pos, 1, false); BOOST_TEST_REQUIRE(alliedBld->GetLeavingFigures().size() == 1u); auto& alliedAttacker = dynamic_cast(alliedBld->GetLeavingFigures().front()); @@ -692,7 +671,7 @@ BOOST_FIXTURE_TEST_CASE(ConquerWithMultipleWalkingIn, AttackFixture4P) bldPos = hqPos[curPlayer] + MapPoint(3, 0); auto* hostileBld = static_cast( BuildingFactory::CreateBuilding(world, BuildingType::Guardhouse, bldPos, curPlayer, Nation::Africans)); - AddSoldiersWithRankAndArmor(bldPos, 2, Job::Private); + AddSoldiers(bldPos, 2, Job::Private); this->Attack(milBld1Pos, 1, false); BOOST_TEST_REQUIRE(hostileBld->GetLeavingFigures().size() == 1u); auto& hostileAttacker = dynamic_cast(hostileBld->GetLeavingFigures().front()); @@ -769,7 +748,7 @@ BOOST_FIXTURE_TEST_CASE(ConquerWithCarriersWalkingIn, AttackFixture<2>) // 1. Carrier with coin walking in the building // 2. Carrier with coin walking out of the building AddSoldiers(milBld0Pos, 0, 6); - AddSoldiersWithRankAndArmor(milBld1Pos, 1, Job::Private); + AddSoldiers(milBld1Pos, 1, Job::Private); MapPoint milBld1FlagPos = world.GetNeighbour(milBld1Pos, Direction::SouthEast); curPlayer = 1; @@ -908,7 +887,7 @@ BOOST_FIXTURE_TEST_CASE(FlagBecomesUnreachableForWaitingAttacker, AttackFixture< const auto attackerBldPos = attackerBld->GetPos(); const auto defenderBldPos = defenderBld->GetPos(); AddSoldiers(defenderBldPos, 0, 1); - AddSoldiersWithRankAndArmor(attackerBldPos, 6, Job::Private); + AddSoldiers(attackerBldPos, 6, Job::Private); // Make defender building flag unreachable const auto terrain = this->world.GetDescription().terrain.find( [](const TerrainDesc& t) { return !t.Is(ETerrain::Walkable) && t.GetBQ() != TerrainBQ::Danger; }); @@ -1001,9 +980,8 @@ BOOST_FIXTURE_TEST_CASE(CancelAgressiveDefenders, AttackFixture<2>) // - Attack on "military storehouse", i.e. HQ. // - HQ sends aggressive defender about to leave. // - Attacker reaches flag and requests defender which cancels any agressive defenders - AddSoldiersWithRankAndArmor(milBld0Pos, 4, Job::General); + AddSoldiers(milBld0Pos, 4, Job::General); auto& attackedHq = ensureNonNull(world.GetSpecObj(hqPos[1])); - FinishRecruiting(attackedHq); const auto getSoldiers = [&attackedHq]() { std::array, 2> soldiers{}; for(const auto job : SOLDIER_JOBS) @@ -1063,10 +1041,10 @@ BOOST_FIXTURE_TEST_CASE(DestroyRoadsOnConquer, DestroyRoadsOnConquerFixture) BuildingFactory::CreateBuilding(world, BuildingType::Barracks, rightBldPos, 1, Nation::Babylonians); BOOST_TEST_REQUIRE(rightBld); - AddSoldiersWithRankAndArmor(leftBldPos, 1, Job::Private); - AddSoldiersWithRankAndArmor(rightBldPos, 1, Job::Private); - AddSoldiersWithRankAndArmor(milBld1Pos, 1, Job::Private); - AddSoldiersWithRankAndArmor(milBld0Pos, 6, Job::General); + AddSoldiers(leftBldPos, 1, Job::Private); + AddSoldiers(rightBldPos, 1, Job::Private); + AddSoldiers(milBld1Pos, 1, Job::Private); + AddSoldiers(milBld0Pos, 6, Job::General); curPlayer = 1; // Build 2 roads to attack building to test that destroying them does not cause a bug @@ -1112,8 +1090,8 @@ struct FreeFightFixture : AttackFixture<2> : attackerBld(*milBld0), attackedBld(*milBld1), attackedBldPos(milBld1Pos), fightSpot(attackedBldPos - MapPoint(3, 0)) { - AddSoldiersWithRankAndArmor(milBld0Pos, 6, Job::Private); - AddSoldiersWithRankAndArmor(milBld1Pos, 6, Job::Private); + AddSoldiers(milBld0Pos, 6, Job::Private); + AddSoldiers(milBld1Pos, 6, Job::Private); this->Attack(attackedBldPos, 1, true); auto& attacker = dynamic_cast(attackerBld.GetLeavingFigures().front()); attacker_ = &attacker; diff --git a/tests/s25Main/integration/testBaseWarehouse.cpp b/tests/s25Main/integration/testBaseWarehouse.cpp index b35d4130d7..afdf905bbc 100644 --- a/tests/s25Main/integration/testBaseWarehouse.cpp +++ b/tests/s25Main/integration/testBaseWarehouse.cpp @@ -15,6 +15,7 @@ #include "gameTypes/JobTypes.h" #include "gameData/ShieldConsts.h" #include +#include #include #include #include @@ -155,6 +156,15 @@ BOOST_FIXTURE_TEST_CASE(OrderJob, EmptyWorldFixture1P) world, BuildingType::Storehouse, player.GetHQPos() + MapPoint(4, 0), 0, Nation::Romans)); world.BuildRoad(0, false, hq->GetFlagPos(), {4, Direction::East}); + // Need some builders, hammers and helpers + { + GoodsAndPeopleCounts inv; + inv[Job::Builder] = rttr::test::randomValue(3, 7); + inv[GoodType::Hammer] = rttr::test::randomValue(3, 7); + inv[Job::Helper] = rttr::test::randomValue(7, 23); // At least one per hammer + hq->AddToInventory(inv, true); + } + // Order all existing builders while(hq->GetNumRealFigures(Job::Builder) > 0u) { diff --git a/tests/s25Main/integration/testFarmer.cpp b/tests/s25Main/integration/testFarmer.cpp index 5f01ec486b..3a79e00500 100644 --- a/tests/s25Main/integration/testFarmer.cpp +++ b/tests/s25Main/integration/testFarmer.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 @@ -20,6 +20,7 @@ struct FarmerFixture : public WorldFixture const nofFarmhand* farmer; FarmerFixture() { + addStartResources(); farmPt = world.GetPlayer(0).GetHQPos() + MapPoint(5, 0); farm = dynamic_cast( BuildingFactory::CreateBuilding(world, BuildingType::Farm, farmPt, 0, Nation::Romans)); diff --git a/tests/s25Main/integration/testFigures.cpp b/tests/s25Main/integration/testFigures.cpp index c793c490a9..dcd9514a3c 100644 --- a/tests/s25Main/integration/testFigures.cpp +++ b/tests/s25Main/integration/testFigures.cpp @@ -23,6 +23,7 @@ BOOST_AUTO_TEST_SUITE(FigureTests) BOOST_FIXTURE_TEST_CASE(DestroyWHWithFigure, WorldWithGCExecution2P) { + world.GetPlayer(curPlayer).GetFirstWH()->AddToInventory(PeopleCounts::make(Job::Helper, 10), true); MapPoint flagPos = world.GetNeighbour(hqPos, Direction::SouthEast); MapPoint whPos(flagPos.x + 5, flagPos.y); auto* wh = static_cast( @@ -62,6 +63,7 @@ BOOST_FIXTURE_TEST_CASE(DestroyWHWithFigure, WorldWithGCExecution2P) BOOST_FIXTURE_TEST_CASE(DestroyWHWithWare, WorldWithGCExecution2P) { + addStartResources(); MapPoint flagPos = world.GetNeighbour(hqPos, Direction::SouthEast); MapPoint whFlagPos(flagPos.x + 5, flagPos.y); MapPoint whPos = world.GetNeighbour(whFlagPos, Direction::NorthWest); diff --git a/tests/s25Main/integration/testGameCommands.cpp b/tests/s25Main/integration/testGameCommands.cpp index bd959b0c53..be9876d878 100644 --- a/tests/s25Main/integration/testGameCommands.cpp +++ b/tests/s25Main/integration/testGameCommands.cpp @@ -364,6 +364,7 @@ BOOST_FIXTURE_TEST_CASE(PlayerEconomySettings, WorldWithGCExecution2P) BOOST_FIXTURE_TEST_CASE(BuildBuilding, WorldWithGCExecution2P) { + addStartResources(); initGameRNG(); const MapPoint closePt = hqPos + MapPoint(2, 0); @@ -435,6 +436,7 @@ BOOST_FIXTURE_TEST_CASE(SendSoldiersHomeTest, WorldWithGCExecution2P) BOOST_TEST_REQUIRE(wh); BOOST_TEST_REQUIRE(wh->GetInventory().people[Job::General] == 0u); //-V522 PeopleCounts soldiers; + soldiers[Job::Private] = 10; soldiers[Job::PrivateFirstClass] = 1; soldiers[Job::Sergeant] = 1; soldiers[Job::Officer] = 1; @@ -526,6 +528,7 @@ BOOST_FIXTURE_TEST_CASE(SendSoldiersHomeTest, WorldWithGCExecution2P) namespace { void FlagWorkerTest(WorldWithGCExecution2P& worldFixture, Job workerJob, GoodType toolType) { + worldFixture.addStartResources(); const MapPoint flagPt = worldFixture.world.GetNeighbour(worldFixture.hqPos, Direction::SouthEast) + MapPoint(3, 0); GamePlayer& player = worldFixture.world.GetPlayer(worldFixture.curPlayer); nobBaseWarehouse* wh = player.GetFirstWH(); @@ -814,6 +817,7 @@ auto makeVector(const helpers::EnumArray& srcArray) BOOST_FIXTURE_TEST_CASE(SetInventorySettingTest, WorldWithGCExecution2P) { + addStartResources(); GamePlayer& player = world.GetPlayer(curPlayer); const nobBaseWarehouse* wh = player.GetFirstWH(); BOOST_TEST_REQUIRE(wh); diff --git a/tests/s25Main/integration/testHarbor.cpp b/tests/s25Main/integration/testHarbor.cpp index abf5d1bca6..30d3aff250 100644 --- a/tests/s25Main/integration/testHarbor.cpp +++ b/tests/s25Main/integration/testHarbor.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 @@ -28,6 +28,7 @@ struct HarborFixture : WorldFixture BOOST_FIXTURE_TEST_CASE(StartExpeditionThenCancel, HarborFixture) { + addStartResources(); BOOST_TEST_REQUIRE(hq->GetNumRealFigures(Job::Builder) >= 1u); // Must have a builder const auto numLeavingFigs = hq->GetLeavingFigures().size(); hb->StartExpedition(); @@ -40,6 +41,7 @@ BOOST_FIXTURE_TEST_CASE(StartExpeditionThenCancel, HarborFixture) BOOST_FIXTURE_TEST_CASE(StartExpeditionThenDestroy, HarborFixture) { + addStartResources(); BOOST_TEST_REQUIRE(hq->GetNumRealFigures(Job::Builder) >= 1u); // Must have a builder const auto numLeavingFigs = hq->GetLeavingFigures().size(); hb->StartExpedition(); diff --git a/tests/s25Main/integration/testProduction.cpp b/tests/s25Main/integration/testProduction.cpp index b4d4665ad7..5ce05b76b4 100644 --- a/tests/s25Main/integration/testProduction.cpp +++ b/tests/s25Main/integration/testProduction.cpp @@ -25,6 +25,7 @@ BOOST_AUTO_TEST_SUITE(Production) BOOST_FIXTURE_TEST_CASE(MetalWorkerStopped, WorldWithGCExecution1P) { + addStartResources(); rttr::test::LogAccessor logAcc; ggs.setSelection(AddonId::TOOL_ORDERING, 1); ggs.setSelection(AddonId::METALWORKSBEHAVIORONZERO, 1); @@ -72,10 +73,11 @@ BOOST_FIXTURE_TEST_CASE(MetalWorkerStopped, WorldWithGCExecution1P) BOOST_FIXTURE_TEST_CASE(MetalWorkerOrders, WorldWithGCExecution1P) { - GoodCounts goods; - goods[GoodType::Boards] = 10; - goods[GoodType::Iron] = 10; - world.GetSpecObj(hqPos)->AddToInventory(goods, true); + GoodsAndPeopleCounts inv; + inv[GoodType::Boards] = 10; + inv[GoodType::Iron] = 10; + inv[Job::Metalworker] = 1; + world.GetSpecObj(hqPos)->AddToInventory(inv, true); ggs.setSelection(AddonId::METALWORKSBEHAVIORONZERO, 1); ggs.setSelection(AddonId::TOOL_ORDERING, 1); ToolSettings settings; diff --git a/tests/s25Main/integration/testSeafaring.cpp b/tests/s25Main/integration/testSeafaring.cpp index a4be0232c8..009d28b439 100644 --- a/tests/s25Main/integration/testSeafaring.cpp +++ b/tests/s25Main/integration/testSeafaring.cpp @@ -74,6 +74,7 @@ BOOST_FIXTURE_TEST_CASE(HarborPlacing, SeaWorldWithGCExecution<>) BOOST_FIXTURE_TEST_CASE(ShipBuilding, SeaWorldWithGCExecution<>) { + addStartResources(); initGameRNG(); const GamePlayer& player = world.GetPlayer(curPlayer); @@ -190,6 +191,7 @@ struct ShipReadyFixture : public SeaWorldWithGCExecution) { curPlayer = 0; + addStartResources(); const GamePlayer& player = world.GetPlayer(curPlayer); const noShip& ship = ensureNonNull(player.GetShipByID(0)); const nobHarborBuilding& harbor = *player.GetBuildingRegister().GetHarbors().front(); @@ -286,6 +288,7 @@ BOOST_FIXTURE_TEST_CASE(ExplorationExpedition, ShipReadyFixture<>) BOOST_FIXTURE_TEST_CASE(DestroyHomeOnExplExp, ShipReadyFixture<2>) { + addStartResources(); curPlayer = 0; const GamePlayer& player = world.GetPlayer(curPlayer); const noShip& ship = ensureNonNull(player.GetShipByID(0)); @@ -341,6 +344,7 @@ BOOST_FIXTURE_TEST_CASE(DestroyHomeOnExplExp, ShipReadyFixture<2>) BOOST_FIXTURE_TEST_CASE(Expedition, ShipReadyFixture<>) { + addStartResources(); const GamePlayer& player = world.GetPlayer(curPlayer); const noShip& ship = ensureNonNull(player.GetShipByID(0)); const nobHarborBuilding& harbor = *player.GetBuildingRegister().GetHarbors().front(); diff --git a/tests/s25Main/integration/testTrading.cpp b/tests/s25Main/integration/testTrading.cpp index 483f19f7fa..5d68d0137f 100644 --- a/tests/s25Main/integration/testTrading.cpp +++ b/tests/s25Main/integration/testTrading.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 @@ -24,6 +24,7 @@ struct TradeFixture : public WorldWithGCExecution3P TradeFixture() { curPlayer = 1; + addStartResources(); world.GetPlayer(0).team = Team::Team1; //-V525 world.GetPlayer(1).team = Team::Team1; world.GetPlayer(2).team = Team::Team2; diff --git a/tests/s25Main/lua/GameWithLuaAccess.h b/tests/s25Main/lua/GameWithLuaAccess.h index 1c26af39f3..8f50783a74 100644 --- a/tests/s25Main/lua/GameWithLuaAccess.h +++ b/tests/s25Main/lua/GameWithLuaAccess.h @@ -96,7 +96,7 @@ struct LuaTestsFixture : public rttr::test::LogAccessor, public LuaBaseFixture, std::vector playerNations; playerNations.push_back(world.GetPlayer(0).nation); playerNations.push_back(world.GetPlayer(1).nation); - BOOST_TEST_REQUIRE(MapLoader::PlaceHQs(world, hqPositions)); + BOOST_TEST_REQUIRE(MapLoader::PlaceHQs(world, hqPositions, false)); for(unsigned id = 0; id < world.GetNumPlayers(); id++) { diff --git a/tests/s25Main/lua/testLua.cpp b/tests/s25Main/lua/testLua.cpp index 34fe0d42fb..b8d9c3e29b 100644 --- a/tests/s25Main/lua/testLua.cpp +++ b/tests/s25Main/lua/testLua.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 @@ -10,6 +10,7 @@ #include "buildings/nobHQ.h" #include "enum_cast.hpp" #include "helpers/EnumRange.h" +#include "helpers/Range.h" #include "lua/LuaTraits.h" // IWYU pragma: keep #include "notifications/BuildingNote.h" #include "postSystem/DiplomacyPostQuestion.h" @@ -20,6 +21,8 @@ #include "nodeObjs/noStaticObject.h" #include "gameTypes/GameTypesOutput.h" #include "gameTypes/Resource.h" +#include "gameData/ShieldConsts.h" +#include "rttr/test/random.hpp" #include "s25util/Serializer.h" #include "s25util/StringConversion.h" #include "s25util/tmpFile.h" @@ -166,7 +169,18 @@ BOOST_AUTO_TEST_CASE(GameFunctions) hqs[0] = world.GetSpecObj(world.GetPlayer(0).GetHQPos()); hqs[1] = world.GetSpecObj(world.GetPlayer(1).GetHQPos()); - BOOST_TEST_REQUIRE(hqs[0]->GetNumRealWares(GoodType::Boards) > 0u); + // Add some random resources to HQs + for(auto* hq : hqs) + { + GoodsAndPeopleCounts inv; + for([[maybe_unused]] const auto i : helpers::range(5)) + { + inv[rttr::test::randomEnum()] = rttr::test::randomValue(1u, 10u); + inv[ConvertShields(rttr::test::randomEnum())] = rttr::test::randomValue(1u, 10u); + } + inv[Job::BoatCarrier] = 0; + hq->AddToInventory(inv, true); + } executeLua("rttr:ClearResources()"); for(auto& hq : hqs) @@ -183,7 +197,7 @@ BOOST_AUTO_TEST_CASE(GameFunctions) } } - for(unsigned i = 0; i < 2; i++) + for([[maybe_unused]] const auto i : helpers::range(2)) { BOOST_TEST(isLuaEqual("rttr:GetGF()", s25util::toStringClassic(world.GetEvMgr().GetCurrentGF()))); world.GetEvMgr().ExecuteNextGF(); diff --git a/tests/s25Main/worldFixtures/CreateEmptyWorld.cpp b/tests/s25Main/worldFixtures/CreateEmptyWorld.cpp index e74a25f73e..5e956573e5 100644 --- a/tests/s25Main/worldFixtures/CreateEmptyWorld.cpp +++ b/tests/s25Main/worldFixtures/CreateEmptyWorld.cpp @@ -3,15 +3,30 @@ // SPDX-License-Identifier: GPL-2.0-or-later #include "worldFixtures/CreateEmptyWorld.h" +#include "GamePlayer.h" #include "RttrForeachPt.h" +#include "buildings/nobHQ.h" #include "initGameRNG.hpp" #include "lua/GameDataLoader.h" +#include "worldFixtures/WorldFixture.h" #include "worldFixtures/terrainHelpers.h" #include "world/GameWorld.h" #include "world/MapLoader.h" +#include #include #include +void WorldFixtureBase::addStartResources() +{ + for(unsigned i = 0; i < world.GetNumPlayers(); i++) + addStartResources(i); +} + +void WorldFixtureBase::addStartResources(unsigned playerIdx) +{ + world.GetPlayer(playerIdx).GetHQ()->addStartWares(); +} + CreateEmptyWorld::CreateEmptyWorld(const MapExtent& size) : size_(size) {} bool CreateEmptyWorld::operator()(GameWorld& world) const @@ -52,8 +67,7 @@ bool CreateEmptyWorld::operator()(GameWorld& world) const } curPt.y += playerDist.y; } - if(!MapLoader::PlaceHQs(world, hqPositions)) - return false; // LCOV_EXCL_LINE + BOOST_TEST_REQUIRE(MapLoader::PlaceHQs(world, hqPositions, false)); } world.InitAfterLoad(); return true; diff --git a/tests/s25Main/worldFixtures/CreateSeaWorld.cpp b/tests/s25Main/worldFixtures/CreateSeaWorld.cpp index 0e5bdedee3..01b00df968 100644 --- a/tests/s25Main/worldFixtures/CreateSeaWorld.cpp +++ b/tests/s25Main/worldFixtures/CreateSeaWorld.cpp @@ -141,8 +141,7 @@ bool CreateSeaWorld::operator()(GameWorld& world) const BOOST_TEST_REQUIRE(MapLoader::InitSeasAndHarbors(world, harbors)); - if(!MapLoader::PlaceHQs(world, hqPositions)) - return false; // LCOV_EXCL_LINE + BOOST_TEST_REQUIRE(MapLoader::PlaceHQs(world, hqPositions, false)); world.InitAfterLoad(); /* The HQs and harbor(ids) are here: (H=HQ, 1-8=harbor) @@ -194,7 +193,7 @@ bool CreateWaterWorld::operator()(GameWorld& world) const node.t1 = node.t2 = t; } } - BOOST_TEST_REQUIRE(MapLoader::PlaceHQs(world, hqPositions)); + BOOST_TEST_REQUIRE(MapLoader::PlaceHQs(world, hqPositions, false)); std::vector harbors; for(MapPoint hqPos : hqPositions) diff --git a/tests/s25Main/worldFixtures/WorldFixture.h b/tests/s25Main/worldFixtures/WorldFixture.h index a6c10aab2a..d7b9ae4a6a 100644 --- a/tests/s25Main/worldFixtures/WorldFixture.h +++ b/tests/s25Main/worldFixtures/WorldFixture.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 @@ -86,28 +86,27 @@ struct WorldDefault<2> static constexpr unsigned height = 20; }; -template::width, - unsigned T_height = WorldDefault::height> -struct WorldFixture +struct WorldFixtureBase { std::shared_ptr game; TestEventManager& em; GlobalGameSettings& ggs; GameWorld& world; - T_WorldCreator worldCreator; - WorldFixture() + WorldFixtureBase(unsigned numPlayers) : game(std::make_shared(GlobalGameSettings(), std::make_unique(), - std::vector(T_numPlayers, GetPlayer()))), + std::vector(numPlayers, GetPlayer()))), em(static_cast(*game->em_)), ggs(const_cast(game->ggs_)), - world(game->world_), worldCreator(MapExtent(T_width, T_height)) - { - // Fast moving ships + world(game->world_) + { // Fast moving ships ggs.setSelection(AddonId::SHIP_SPEED, 4); // Explored area stays explored. Avoids fow creation ggs.exploration = Exploration::Classic; - BOOST_TEST_REQUIRE(worldCreator(world)); - BOOST_TEST_REQUIRE(world.GetNumPlayers() == T_numPlayers); } + /// Add start resources to HQs of all players + void addStartResources(); + /// Add start resources to HQ of the given player + void addStartResources(unsigned playerIdx); + static PlayerInfo GetPlayer() { PlayerInfo result; @@ -116,6 +115,18 @@ struct WorldFixture } }; +template::width, + unsigned T_height = WorldDefault::height> +struct WorldFixture : WorldFixtureBase +{ + T_WorldCreator worldCreator; + WorldFixture() : WorldFixtureBase(T_numPlayers), worldCreator(MapExtent(T_width, T_height)) + { + BOOST_TEST_REQUIRE(worldCreator(world)); + BOOST_TEST_REQUIRE(world.GetNumPlayers() == T_numPlayers); + } +}; + class TestWorld : public World { public: From ebb4484169042b23bede0a40a9f1b1ec539c7279 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 6 Apr 2026 13:14:29 +0200 Subject: [PATCH 10/19] Extent test case to handle millitary buildings & HQ w/ full & empty troops When a defender is requested then a) Leaving agg. defenders are stopped b) A defender is chosen from remaining troops This might "convert" and agg. defender to a defender. As this is kind of a sub-case of the existing one, merge them with a data-param. Similar the handling for warehouses and military buildings needs to be checked, so make that another param. --- tests/s25Main/integration/testAttacking.cpp | 236 ++++++++++++++++---- tests/testHelpers/rttr/test/random.hpp | 9 +- 2 files changed, 203 insertions(+), 42 deletions(-) diff --git a/tests/s25Main/integration/testAttacking.cpp b/tests/s25Main/integration/testAttacking.cpp index ee3ff83d4d..19513902c5 100644 --- a/tests/s25Main/integration/testAttacking.cpp +++ b/tests/s25Main/integration/testAttacking.cpp @@ -25,12 +25,14 @@ #include "gameTypes/GameTypesOutput.h" #include "gameData/MilitaryConsts.h" #include "gameData/SettingTypeConv.h" +#include "rttr/test/random.hpp" #include #include #include #include #include #include +#include using SoldierState = nofActiveSoldier::SoldierState; @@ -45,12 +47,20 @@ BOOST_AUTO_TEST_SUITE(AttackSuite) namespace { +namespace dataset = boost::unit_test::data; + struct AttackDefaults { static constexpr unsigned width = 20; static constexpr unsigned height = 12; }; +template +auto calcSum(const T& collection) +{ + return std::accumulate(std::begin(collection), std::end(collection), 0u); +} + /// Reschedule the walk event of the obj to be executed in numGFs GFs void rescheduleWalkEvent(TestEventManager& em, noMovable& obj, unsigned numGFs) { @@ -95,13 +105,11 @@ struct AttackFixtureBase : public WorldWithGCExecution(hqPos[i])->AddToInventory(soldiers, true); this->ChangeMilitary(MILITARY_SETTINGS_SCALE); } curPlayer = 0; @@ -145,6 +153,29 @@ struct AttackFixtureBase : public WorldWithGCExecution, 2> soldiers{}; + for(const auto job : SOLDIER_JOBS) + { + soldiers[real][getSoldierRank(job)] = wh.GetNumRealFigures(job); + soldiers[visual][getSoldierRank(job)] = wh.GetNumVisualFigures(job); + } + return soldiers; + } + // Get counts of soldiers + auto getTotalSoldiers(unsigned player) + { + const auto& inv = world.GetPlayer(player).GetInventory(); + std::array soldiers{}; + for(const auto job : SOLDIER_JOBS) + soldiers[getSoldierRank(job)] = inv[job]; + return soldiers; + } }; // Size is chosen based on current maximum attacking distances! @@ -158,6 +189,12 @@ struct NumSoldierTestFixture : public AttackFixtureBase<3, 56, 38> NumSoldierTestFixture() : gwv(curPlayer, world) { + // Add some soldiers to the HQs + for(const auto pos : hqPos) + { + world.GetSpecObj(pos)->AddToInventory( + PeopleCounts::make(Job::General, rttr::test::randomValue(3, 10)), true); + } // Assert player positions: 0: Top-Left, 1: Top-Right, 2: Bottom-Middle BOOST_TEST_REQUIRE(hqPos[0].x < hqPos[1].x); BOOST_TEST_REQUIRE(hqPos[0].y < hqPos[2].y); @@ -557,8 +594,8 @@ enum Case }; BOOST_DATA_TEST_CASE_F(AttackFixture<>, ConquerBldArmorAddon, - boost::unit_test::data::make(std::array{Case::EnableAfterDisable, Case::DisableAfterEnable, - Case::KeepEnabled, Case::KeepDisabled})) + dataset::make(std::array{Case::EnableAfterDisable, Case::DisableAfterEnable, Case::KeepEnabled, + Case::KeepDisabled})) { if(sample == Case::EnableAfterDisable) this->ggs.setSelection(AddonId::ARMOR_CAPTURED_BLD, 1); @@ -972,61 +1009,178 @@ BOOST_FIXTURE_TEST_CASE(FlagBecomesUnreachableForWaitingAttacker, AttackFixture< BOOST_TEST(attacker3.GetState() == SoldierState::AttackingFightingVsDefender); } -BOOST_FIXTURE_TEST_CASE(CancelAgressiveDefenders, AttackFixture<2>) +enum AttackedBldType +{ + Hq, + MillitaryBld +}; + +#ifndef __INTELLISENSE__ +BOOST_DATA_TEST_CASE_F(AttackFixture<2>, HandleLeavingAggDefendersOnAttack, + dataset::make(std::array{AttackedBldType::Hq, AttackedBldType::MillitaryBld}) + * dataset::make(std::array{false, true}), + test_case, useAllDefenders) +#else +struct HandleLeavingAggDefendersOnAttack : AttackFixture<2> +{ + void test(AttackedBldType test_case, bool useAllDefenders); +}; +void HandleLeavingAggDefendersOnAttack::test(AttackedBldType test_case, bool useAllDefenders) +#endif { + // When defender is called, leaving aggressive defenders should be canceled. + // When there is no other soldier left to send as defender, an agressive defenders should be used as defender + // instead. + // // Reproduce issue #1907: // Accounting of soldiers that were about to leave but got canceled was wrong - // Situation: + // Situation (happens with a single attacker already): // - Attack on "military storehouse", i.e. HQ. // - HQ sends aggressive defender about to leave. // - Attacker reaches flag and requests defender which cancels any agressive defenders - AddSoldiers(milBld0Pos, 4, Job::General); + // - Readding that defender cause error in accounting + // + // Handling for "real" military buildings and attackable warehouses (HQ) is different, so test all combinations + std::array attackers{}; + AddSoldiers(milBld0Pos, attackers.size() + 1, Job::General); auto& attackedHq = ensureNonNull(world.GetSpecObj(hqPos[1])); - const auto getSoldiers = [&attackedHq]() { - std::array, 2> soldiers{}; - for(const auto job : SOLDIER_JOBS) + auto& attackedBld = (test_case == AttackedBldType::Hq) ? attackedHq : static_cast(*milBld1); + const auto attackedPlayer = attackedBld.GetPlayer(); + const auto numGenerals = rttr::test::randomValue(1, attackers.size() / 2); + const auto numPrivates = rttr::test::randomValue(1, attackers.size() / 2); + if(test_case == AttackedBldType::Hq) + { + // Make sure they can be sent out (aggressive defenders are not taken from the reserve) + attackedHq.SetRealReserve(getSoldierRank(Job::General), 0); + attackedHq.SetRealReserve(getSoldierRank(Job::Private), 0); + auto startSoldiers = PeopleCounts::make(Job::General, numGenerals); + startSoldiers[Job::Private] = numPrivates; + attackedHq.AddToInventory(startSoldiers, true); + } else + { + AddSoldiers(attackedBld.GetPos(), numGenerals, Job::General); + AddSoldiers(attackedBld.GetPos(), numPrivates, Job::Private); + } + // Total soldiers should not change because none die in this test case + // This ensures they are not double-accounted during conversion from agressive defender to defender + // and not missed when removed from the leave queue + const auto totalSoldiersBefore = getTotalSoldiers(attackedPlayer); + const auto soldiersBefore = getNumSoldiers(attackedHq); + const auto numAttackers = useAllDefenders ? attackers.size() : 1u; + this->Attack(attackedBld.GetPos(), numAttackers, true); + BOOST_TEST_REQUIRE(milBld0->GetLeavingFigures().size() == numAttackers); + { + int i = 0; + for(auto& fig : milBld0->GetLeavingFigures()) + attackers[i++] = &dynamic_cast(fig); + } + // Let one out then call SendAggressiveDefender for all attackers. + // This is technically wrong but we want to ensure they come from the target bld only and are still in the leave + // queue when the first attacker calls the defender + RTTR_EXEC_TILL(50, milBld0->GetLeavingFigures().size() < numAttackers); + for(unsigned i = 0; i < numAttackers; i++) + { + auto* attacker = attackers[i]; + if(attacker && !attacker->GetHuntingDefender()) { - soldiers[0][getSoldierRank(job)] = attackedHq.GetNumRealFigures(job); - soldiers[1][getSoldierRank(job)] = attackedHq.GetNumVisualFigures(job); + auto* aggDefender = attackedBld.SendAggressiveDefender(*attacker); + if(aggDefender) + attacker->LetsFight(*aggDefender); } - return soldiers; - }; - auto soldiersBefore = getSoldiers(); - - this->Attack(attackedHq.GetPos(), 1, true); - BOOST_TEST_REQUIRE(milBld0->GetLeavingFigures().size() == 1u); - // multiple soldiers and last is aggressive - auto& attacker = dynamic_cast(milBld0->GetLeavingFigures().front()); - RTTR_EXEC_TILL(70, milBld0->GetLeavingFigures().empty()); //-V807 - if(!attacker.GetHuntingDefender()) + } + if(useAllDefenders) { - auto* aggDefender = attackedHq.SendAggressiveDefender(attacker); - BOOST_TEST_REQUIRE(aggDefender); - attacker.LetsFight(*aggDefender); + if(test_case == AttackedBldType::Hq) // HQs will be empty as all are leaving + BOOST_TEST_REQUIRE(attackedBld.GetLeavingFigures().size() == numGenerals + numPrivates); + else // Military buildings keep at least one + BOOST_TEST_REQUIRE(attackedBld.GetLeavingFigures().size() == numGenerals + numPrivates - 1u); + } else + BOOST_TEST_REQUIRE(!attackedBld.GetLeavingFigures().empty()); + if(test_case == AttackedBldType::Hq) + { + auto curSoldiers = getNumSoldiers(attackedHq); + // Still there visually + BOOST_TEST(curSoldiers[visual] == soldiersBefore[visual]); + // but not real + BOOST_TEST(curSoldiers[real] != soldiersBefore[real]); + if(useAllDefenders) + BOOST_TEST(calcSum(curSoldiers[real]) == 0u); + else + BOOST_TEST(calcSum(curSoldiers[real]) == calcSum(soldiersBefore[real]) - numAttackers); } - BOOST_TEST_REQUIRE(!attackedHq.GetLeavingFigures().empty()); - // Still there visually, but not real - auto soldiersNow = getSoldiers(); - BOOST_TEST(soldiersNow[0] != soldiersBefore[0]); - BOOST_TEST(soldiersNow[1] == soldiersBefore[1]); - moveObjTo(world, attacker, attackedHq.GetFlagPos()); + BOOST_TEST(getTotalSoldiers(attackedPlayer) == totalSoldiersBefore); + // Move attacker to flag to trigger defender request + auto& attacker = *attackers.front(); + moveObjTo(world, attacker, attackedBld.GetFlagPos()); rescheduleWalkEvent(em, attacker, 1); RTTR_SKIP_GFS(1); BOOST_TEST_REQUIRE(attacker.GetState() == SoldierState::AttackingWaitingForDefender); // All agressive defenders are cancelled - for(const auto& fig : attackedHq.GetLeavingFigures()) + for(const auto& fig : attackedBld.GetLeavingFigures()) BOOST_TEST(fig.GetGOT() != GO_Type::NofAggressivedefender); - BOOST_TEST_REQUIRE(!attackedHq.GetLeavingFigures().empty()); + BOOST_TEST_REQUIRE(!attackedBld.GetLeavingFigures().empty()); // Defender is coming - BOOST_TEST_REQUIRE(attackedHq.GetLeavingFigures().front().GetGOT() == GO_Type::NofDefender); - const auto defenderRank = getSoldierRank(attackedHq.GetLeavingFigures().front().GetJobType()); - // Counts unchanged until defender is out - BOOST_TEST(getSoldiers() == soldiersNow); + BOOST_TEST_REQUIRE(attackedBld.GetLeavingFigures().front().GetGOT() == GO_Type::NofDefender); + const auto defenderRank = getSoldierRank(attackedBld.GetLeavingFigures().front().GetJobType()); + if(test_case == AttackedBldType::Hq) + { + const auto curSoldiers = getNumSoldiers(attackedHq); + // Only defender is missing, only from real count (until actually left) + BOOST_TEST(calcSum(curSoldiers[real]) == calcSum(soldiersBefore[real]) - 1); + BOOST_TEST(curSoldiers[visual] == soldiersBefore[visual]); + } + BOOST_TEST(getTotalSoldiers(attackedPlayer) == totalSoldiersBefore); + RTTR_EXEC_TILL(70, attackedBld.GetLeavingFigures().empty()); + if(test_case == AttackedBldType::Hq) + { + auto soldiersExpected = soldiersBefore; + soldiersExpected[visual][defenderRank] = --soldiersExpected[real][defenderRank]; // Defender is out + BOOST_TEST(getNumSoldiers(attackedHq) == soldiersExpected); + } + BOOST_TEST(getTotalSoldiers(attackedPlayer) == totalSoldiersBefore); +} + +BOOST_FIXTURE_TEST_CASE(LeavingSoldierUsedAsDefender, AttackFixture<2>) +{ + // When the only soldier in a HQ is currently leaving (to a building) it is used as defender + // Counts need to be correct at all times + AddSoldiers(milBld0Pos, 2, Job::General); + auto& attackedHq = ensureNonNull(world.GetSpecObj(hqPos[1])); + const auto soldierJob = rttr::test::randomElement(SOLDIER_JOBS); + attackedHq.SetRealReserve(getSoldierRank(soldierJob), 0); + attackedHq.AddToInventory(PeopleCounts::make(soldierJob, 1), true); + const auto attackedPlayer = attackedHq.GetPlayer(); + const auto totalSoldiersBefore = getTotalSoldiers(attackedPlayer); + const auto soldiersBefore = getNumSoldiers(attackedHq); + + this->Attack(attackedHq.GetPos(), 1, true); + BOOST_TEST_REQUIRE(milBld0->GetLeavingFigures().size() == 1); + auto& attacker = dynamic_cast(milBld0->GetLeavingFigures().front()); + RTTR_EXEC_TILL(50, milBld0->GetLeavingFigures().empty()); + curPlayer = attackedPlayer; + BuildRoadForBlds(milBld1Pos, hqPos[1]); + curPlayer = 0; + BOOST_TEST_REQUIRE(!attackedHq.GetLeavingFigures().empty()); + { + const auto& leavingSoldier = dynamic_cast(attackedHq.GetLeavingFigures().front()); + BOOST_TEST(leavingSoldier.GetGoal() == milBld1); + } + BOOST_TEST(getTotalSoldiers(attackedPlayer) == totalSoldiersBefore); + const auto soldiersNow = getNumSoldiers(attackedHq); + BOOST_TEST(soldiersNow[visual] == soldiersBefore[visual]); + BOOST_TEST(calcSum(soldiersNow[real]) == 0u); + moveObjTo(world, attacker, attackedHq.GetFlagPos()); + rescheduleWalkEvent(em, attacker, 1); + RTTR_SKIP_GFS(1); + BOOST_TEST_REQUIRE(attackedHq.GetLeavingFigures().size() == 1); + BOOST_TEST(attackedHq.GetLeavingFigures().front().GetGOT() == GO_Type::NofDefender); + BOOST_TEST(getTotalSoldiers(attackedPlayer) == totalSoldiersBefore); + BOOST_TEST(getNumSoldiers(attackedHq) == soldiersNow); RTTR_EXEC_TILL(70, attackedHq.GetLeavingFigures().empty()); - soldiersNow = getSoldiers(); - auto soldiersExpected = soldiersBefore; - soldiersExpected[1][defenderRank] = --soldiersExpected[0][defenderRank]; // Defender is out - BOOST_TEST(soldiersNow == soldiersExpected); + BOOST_TEST(getTotalSoldiers(attackedPlayer) == totalSoldiersBefore); + const auto soldiersAfter = getNumSoldiers(attackedHq); + BOOST_TEST(calcSum(soldiersAfter[visual]) == 0u); + BOOST_TEST(calcSum(soldiersAfter[real]) == 0u); } using DestroyRoadsOnConquerFixture = AttackFixture<2, 24>; diff --git a/tests/testHelpers/rttr/test/random.hpp b/tests/testHelpers/rttr/test/random.hpp index 5911595d16..c3e7202cf5 100644 --- a/tests/testHelpers/rttr/test/random.hpp +++ b/tests/testHelpers/rttr/test/random.hpp @@ -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 @@ -35,4 +35,11 @@ auto randomPoint(typename T::ElementType min = std::numeric_limits +auto randomElement(ContainerT& container) +{ + return helpers::getRandomElement(getRandState(), container); +} + } // namespace rttr::test From 1007adb4c9a9862f0341f113f0cb7dbc3ce437cc Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 6 Apr 2026 13:29:38 +0200 Subject: [PATCH 11/19] Cancel leaving soldiers before requesting defender Soldiers are stopped when a defender is coming out. If no defender is found the leaving soldiers are checked. So it is better to first stop the leaving soldiers and then choose from all of them. Also remove `nofAggressiveDefender::NeedForHomeDefence` which is a wrapper to `InformTargetsAboutCancelling` only used in a single place which is now redundant. --- libs/s25main/buildings/nobBaseMilitary.cpp | 25 +++++++----------- libs/s25main/buildings/nobBaseWarehouse.cpp | 26 +++++++------------ libs/s25main/buildings/nobMilitary.cpp | 9 +------ .../s25main/figures/nofAggressiveDefender.cpp | 7 +---- libs/s25main/figures/nofAggressiveDefender.h | 5 +--- 5 files changed, 23 insertions(+), 49 deletions(-) diff --git a/libs/s25main/buildings/nobBaseMilitary.cpp b/libs/s25main/buildings/nobBaseMilitary.cpp index 0c72ba3960..3d596611a7 100644 --- a/libs/s25main/buildings/nobBaseMilitary.cpp +++ b/libs/s25main/buildings/nobBaseMilitary.cpp @@ -221,27 +221,22 @@ MapPoint nobBaseMilitary::FindAnAttackerPlace(unsigned short& ret_radius, const bool nobBaseMilitary::CallDefender(nofAttacker& attacker) { + // Block soldiers leaving this building e.g. for attacks or aggressive defending + StopLeavingSoldiers(); // Use existing defender (e.g. just going back in) if possible if(defender_) { defender_->NewAttacker(attacker); - // Block soldiers leaving this building e.g. for attacks or aggressive defending - StopLeavingSoldiers(); - return true; - } else - { - auto defender = ProvideDefender(attacker); - if(!defender) - return false; // Building empty -> Can be conquered - - // Block soldiers leaving this building e.g. for attacks or aggressive defending - StopLeavingSoldiers(); - // Soldat muss noch rauskommen - defender_ = defender.get(); - AddLeavingFigure(std::move(defender)); - return true; } + auto defender = ProvideDefender(attacker); + if(!defender) + return false; // Building empty -> Can be conquered + + // Soldat muss noch rauskommen + defender_ = defender.get(); + AddLeavingFigure(std::move(defender)); + return true; } nofAttacker* nobBaseMilitary::FindAttackerNearBuilding() diff --git a/libs/s25main/buildings/nobBaseWarehouse.cpp b/libs/s25main/buildings/nobBaseWarehouse.cpp index 11711cc37d..865dfb4e0f 100644 --- a/libs/s25main/buildings/nobBaseWarehouse.cpp +++ b/libs/s25main/buildings/nobBaseWarehouse.cpp @@ -1098,28 +1098,22 @@ std::unique_ptr nobBaseWarehouse::ProvideDefender(nofAttacker& atta } } - // Kein Soldat gefunden, als letzten Hoffnung die Soldaten nehmen, die ggf in der Warteschlange noch hängen + // If no soldier was found take a soldier that is about to leave (to another building) for(auto it = leave_house.begin(); it != leave_house.end(); ++it) { - std::unique_ptr soldier; - if((*it)->GetGOT() == GO_Type::NofAggressivedefender) + if((*it)->GetGOT() == GO_Type::NofPassivesoldier) { - soldier = boost::static_pointer_cast(std::move(*it)); - static_cast(*soldier).NeedForHomeDefence(); - } else if((*it)->GetGOT() == GO_Type::NofPassivesoldier) - soldier = boost::static_pointer_cast(std::move(*it)); - else - continue; + std::unique_ptr soldier = boost::static_pointer_cast(std::move(*it)); - leave_house.erase(it); // Only allowed in the loop as we return now - soldier->Abrogate(); + leave_house.erase(it); // Only allowed in the loop as we return now + soldier->Abrogate(); - auto defender = std::make_unique(pos, player, *this, soldier->GetRank(), attacker); - defender->SetArmor(soldier->HasArmor()); - soldier->Destroy(); - return defender; + auto defender = std::make_unique(pos, player, *this, soldier->GetRank(), attacker); + defender->SetArmor(soldier->HasArmor()); + soldier->Destroy(); + return defender; + } } - return nullptr; } diff --git a/libs/s25main/buildings/nobMilitary.cpp b/libs/s25main/buildings/nobMilitary.cpp index 61e5b6d262..554840d997 100644 --- a/libs/s25main/buildings/nobMilitary.cpp +++ b/libs/s25main/buildings/nobMilitary.cpp @@ -1013,14 +1013,7 @@ std::unique_ptr nobMilitary::ProvideDefender(nofAttacker& attacker) { nofPassiveSoldier* soldier = ChooseSoldier(); if(!soldier) - { - /// Soldaten, die noch auf Mission gehen wollen, canceln und für die Verteidigung mit einziehen - StopLeavingSoldiers(); - // Nochmal versuchen - soldier = ChooseSoldier(); - if(!soldier) - return nullptr; - } + return nullptr; auto defender = std::make_unique(*soldier, attacker); diff --git a/libs/s25main/figures/nofAggressiveDefender.cpp b/libs/s25main/figures/nofAggressiveDefender.cpp index d658c1a9cf..00948fb785 100644 --- a/libs/s25main/figures/nofAggressiveDefender.cpp +++ b/libs/s25main/figures/nofAggressiveDefender.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2005 - 2024 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 @@ -251,11 +251,6 @@ void nofAggressiveDefender::AttackerLost() // TODO(Replay) When still in leave queue abort to avoid going out and right back } -void nofAggressiveDefender::NeedForHomeDefence() -{ - InformTargetsAboutCancelling(); -} - void nofAggressiveDefender::InformTargetsAboutCancelling() { nofActiveSoldier::InformTargetsAboutCancelling(); diff --git a/libs/s25main/figures/nofAggressiveDefender.h b/libs/s25main/figures/nofAggressiveDefender.h index d96530933d..7db9cec9ce 100644 --- a/libs/s25main/figures/nofAggressiveDefender.h +++ b/libs/s25main/figures/nofAggressiveDefender.h @@ -1,4 +1,4 @@ -// Copyright (C) 2005 - 2024 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 @@ -69,9 +69,6 @@ class nofAggressiveDefender : public nofActiveSoldier void MissAggressiveDefendingContinueWalking(); /// Wenn der jeweils andere Soldat, mit dem man kämpfen wollte, nicht mehr kommen kann void AttackerLost(); - /// Ich befinde mich noch im Lagerhaus in der Warteschlange und muss mein HQ etc. verteidigen - /// Mission muss also abgebrochen werden - void NeedForHomeDefence(); // Debugging const nofAttacker* GetAttacker() const { return attacker; } From 8275e32d4285533189f6ca342786ddcdd3caebf7 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 6 Apr 2026 13:54:57 +0200 Subject: [PATCH 12/19] Fix check when stopping leaving soldiers The check assumes only active soldiers start in "Job-state". This state is set when a figure arrived at its goal, and immediately for those not having a "fixed" goal, like attacking soldiers. But it also applies to e.g. trade-donkeys. So check for soldiers directly. Also rename `NoDefender` to `ResetDefender` to clarify meaning and remove the check for leaving defenders which cannot be met: The method is called before the defender is called. --- libs/s25main/buildings/nobBaseMilitary.cpp | 12 ++++++------ libs/s25main/buildings/nobBaseMilitary.h | 2 +- libs/s25main/buildings/nobBaseWarehouse.cpp | 2 +- libs/s25main/buildings/nobMilitary.cpp | 2 +- libs/s25main/figures/nofDefender.cpp | 4 ++-- libs/s25main/figures/nofDefender.h | 4 ++-- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/libs/s25main/buildings/nobBaseMilitary.cpp b/libs/s25main/buildings/nobBaseMilitary.cpp index 3d596611a7..84806e5d36 100644 --- a/libs/s25main/buildings/nobBaseMilitary.cpp +++ b/libs/s25main/buildings/nobBaseMilitary.cpp @@ -337,17 +337,17 @@ void nobBaseMilitary::StopLeavingSoldiers() { for(auto it = leave_house.begin(); it != leave_house.end();) { - // Just take soldiers (in Job state) and no normal defenders which should always come out - if((*it)->IsDoingJobWorks() && (*it)->GetGOT() != GO_Type::NofDefender) + // Only take active soldiers + if(!dynamic_cast(it->get())) + ++it; + else { - auto soldier = boost::dynamic_pointer_cast(std::move(*it)); - RTTR_Assert(soldier); + auto soldier = boost::static_pointer_cast(std::move(*it)); it = leave_house.erase(it); FigureLeft(*soldier); soldier->InformTargetsAboutCancelling(); AddActiveSoldier(std::move(soldier)); - } else - ++it; + } } } diff --git a/libs/s25main/buildings/nobBaseMilitary.h b/libs/s25main/buildings/nobBaseMilitary.h index 4bd74ad0f2..dbeb9b377c 100644 --- a/libs/s25main/buildings/nobBaseMilitary.h +++ b/libs/s25main/buildings/nobBaseMilitary.h @@ -106,7 +106,7 @@ class nobBaseMilitary : public noBuilding /// zu kommen, diese Funktion sucht nach solchen Soldaten schickt einen ggf. zur Flagge, um anzugreifen void CheckArrestedAttackers(); /// Der Verteidiger ist entweder tot oder wieder reingegegangen - void NoDefender() { defender_ = nullptr; } + void ResetDefender() { defender_ = nullptr; } /// Stop all soldiers going for an attack from leaving the house, i.e. put them from the queue back in void StopLeavingSoldiers(); diff --git a/libs/s25main/buildings/nobBaseWarehouse.cpp b/libs/s25main/buildings/nobBaseWarehouse.cpp index 865dfb4e0f..326893d833 100644 --- a/libs/s25main/buildings/nobBaseWarehouse.cpp +++ b/libs/s25main/buildings/nobBaseWarehouse.cpp @@ -1026,7 +1026,7 @@ void nobBaseWarehouse::AddActiveSoldier(std::unique_ptr soldie // Returned home if(soldier.get() == defender_) - NoDefender(); + ResetDefender(); else { // Ggf. war er auf Mission diff --git a/libs/s25main/buildings/nobMilitary.cpp b/libs/s25main/buildings/nobMilitary.cpp index 554840d997..6ca29ccaac 100644 --- a/libs/s25main/buildings/nobMilitary.cpp +++ b/libs/s25main/buildings/nobMilitary.cpp @@ -785,7 +785,7 @@ void nobMilitary::AddActiveSoldier(std::unique_ptr soldier) // Returned home if(soldier.get() == defender_) - NoDefender(); + ResetDefender(); else if(helpers::contains(troops_on_mission, soldier.get())) { troops_on_mission.remove(soldier.get()); diff --git a/libs/s25main/figures/nofDefender.cpp b/libs/s25main/figures/nofDefender.cpp index b1d2a1943b..d68e9637f6 100644 --- a/libs/s25main/figures/nofDefender.cpp +++ b/libs/s25main/figures/nofDefender.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2005 - 2024 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 @@ -152,7 +152,7 @@ void nofDefender::LostFighting() // Notify building if it still exists if(building) { - building->NoDefender(); + building->ResetDefender(); // A military building potentially needs to get new soldiers if this wasn't the last one if(BuildingProperties::IsMilitary(building->GetBuildingType())) { diff --git a/libs/s25main/figures/nofDefender.h b/libs/s25main/figures/nofDefender.h index 26e901af1c..9ec9fb1c6d 100644 --- a/libs/s25main/figures/nofDefender.h +++ b/libs/s25main/figures/nofDefender.h @@ -1,4 +1,4 @@ -// Copyright (C) 2005 - 2024 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 @@ -43,7 +43,7 @@ class nofDefender : public nofActiveSoldier GO_Type GetGOT() const final { return GO_Type::NofDefender; } - /// Inform that a new attacker arrived at the flag while we are going on such that we can come right back + /// Inform that a new attacker arrived at the flag while we are going in such that we can come right back void NewAttacker(nofAttacker& attacker) { this->attacker = &attacker; } /// The attacker won't come to the flag anymore void AttackerArrested(); From 8d86aacd84b0a09819a23f508554d97698eaca15 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 6 Apr 2026 14:11:05 +0200 Subject: [PATCH 13/19] Inline `StopLeavingFigures` function This is so specific to the call site that it should be inline which explains the reasoning better. --- libs/s25main/buildings/nobBaseMilitary.cpp | 38 ++++++++++------------ libs/s25main/buildings/nobBaseMilitary.h | 2 -- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/libs/s25main/buildings/nobBaseMilitary.cpp b/libs/s25main/buildings/nobBaseMilitary.cpp index 84806e5d36..5c64582413 100644 --- a/libs/s25main/buildings/nobBaseMilitary.cpp +++ b/libs/s25main/buildings/nobBaseMilitary.cpp @@ -221,8 +221,23 @@ MapPoint nobBaseMilitary::FindAnAttackerPlace(unsigned short& ret_radius, const bool nobBaseMilitary::CallDefender(nofAttacker& attacker) { - // Block soldiers leaving this building e.g. for attacks or aggressive defending - StopLeavingSoldiers(); + // 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 + { + 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 + RTTR_Assert(soldier->GetGOT() != GO_Type::NofDefender); + it = leave_house.erase(it); + FigureLeft(*soldier); + + soldier->InformTargetsAboutCancelling(); + AddActiveSoldier(std::move(soldier)); + } + } // Use existing defender (e.g. just going back in) if possible if(defender_) { @@ -332,22 +347,3 @@ bool nobBaseMilitary::IsOnMission(const nofActiveSoldier& soldier) const { return helpers::contains(troops_on_mission, &soldier); } - -void nobBaseMilitary::StopLeavingSoldiers() -{ - for(auto it = leave_house.begin(); it != leave_house.end();) - { - // Only take active soldiers - if(!dynamic_cast(it->get())) - ++it; - else - { - auto soldier = boost::static_pointer_cast(std::move(*it)); - it = leave_house.erase(it); - FigureLeft(*soldier); - - soldier->InformTargetsAboutCancelling(); - AddActiveSoldier(std::move(soldier)); - } - } -} diff --git a/libs/s25main/buildings/nobBaseMilitary.h b/libs/s25main/buildings/nobBaseMilitary.h index dbeb9b377c..071d469ab0 100644 --- a/libs/s25main/buildings/nobBaseMilitary.h +++ b/libs/s25main/buildings/nobBaseMilitary.h @@ -107,8 +107,6 @@ class nobBaseMilitary : public noBuilding void CheckArrestedAttackers(); /// Der Verteidiger ist entweder tot oder wieder reingegegangen void ResetDefender() { defender_ = nullptr; } - /// Stop all soldiers going for an attack from leaving the house, i.e. put them from the queue back in - void StopLeavingSoldiers(); /// Sind noch Truppen drinne, die dieses Gebäude verteidigen können virtual bool DefendersAvailable() const = 0; From 9a7a79f0722019da8c5faf8678694eb65aea36dd Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 6 Apr 2026 19:52:50 +0200 Subject: [PATCH 14/19] Update GitHub Actions configs - Pin used actions - Add explicit permissions - Update actions where required & possible for Node 20 deprecation --- .github/workflows/release.yml | 35 +++++++++------------------ .github/workflows/static-analysis.yml | 12 +++++---- .github/workflows/unit-tests.yml | 18 ++++++++------ 3 files changed, 28 insertions(+), 37 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 825589ed96..82b3cc195b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,8 +1,10 @@ -# Copyright (C) 2005 - 2025 Settlers Freaks +# Copyright (C) 2005 - 2026 Settlers Freaks # # SPDX-License-Identifier: GPL-2.0-or-later name: Create Release +permissions: + contents: write on: push: @@ -18,7 +20,7 @@ jobs: name: Create Release runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - name: Extract tag name id: get_tag run: echo "::set-output name=tag::${GITHUB_REF#refs/tags/}" @@ -60,12 +62,11 @@ jobs: fi - name: Create Release id: create_release - uses: actions/create-release@v1 - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + uses: softprops/action-gh-release@a06a81a03ee405af7f2048a818ed3f03bbf83c7b # v2.5.0 with: + token: ${{ secrets.GITHUB_TOKEN }} tag_name: ${{ github.ref }} - release_name: Release ${{ github.ref }} + name: Release ${{ github.ref }} body: | Return To The Roots (Settlers II(R) Clone) ${{ env.TAG_MSG }} @@ -73,21 +74,7 @@ jobs: - ${{steps.filenames.outputs.devTools}} contains optional binaries for development. Extract over the source folder if required draft: false prerelease: false - - name: Upload source distribution - uses: actions/upload-release-asset@v1 - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - with: - upload_url: ${{ steps.create_release.outputs.upload_url }} - asset_path: ./${{steps.filenames.outputs.src}} - asset_name: ${{steps.filenames.outputs.src}} - asset_content_type: application/tar.gz - - name: Upload Dev Tools - uses: actions/upload-release-asset@v1 - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - with: - upload_url: ${{ steps.create_release.outputs.upload_url }} - asset_path: ./${{steps.filenames.outputs.devTools}} - asset_name: ${{steps.filenames.outputs.devTools}} - asset_content_type: application/tar.gz + files: | + ${{steps.filenames.outputs.src}} + ${{steps.filenames.outputs.devTools}} + fail_on_unmatched_files: true diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index e6554114b1..19dfea80f6 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -1,8 +1,10 @@ -# Copyright (C) 2005 - 2025 Settlers Freaks +# Copyright (C) 2005 - 2026 Settlers Freaks # # SPDX-License-Identifier: GPL-2.0-or-later name: Static analysis +permissions: + contents: write on: push: @@ -16,12 +18,12 @@ jobs: StyleAndFormatting: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - run: git submodule update --init - name: Validation run: tools/ci/staticValidation.sh "$GITHUB_WORKSPACE" - name: Formatting - uses: DoozyX/clang-format-lint-action@v0.18.2 + uses: DoozyX/clang-format-lint-action@c71d0bf4e21876ebec3e5647491186f8797fde31 # v0.18.2 with: source: "extras libs tests external/libendian external/liblobby external/libsiedler2 external/libutil external/mygettext external/s25edit external/s25update" clangFormatVersion: 10 @@ -32,7 +34,7 @@ jobs: -prune -false -o \( -name '*.hpp' -or -name '*.h' \) \ -print0 | xargs -0 -n1 tools/ci/checkIncludeGuards.sh - name: Lint markdown files - uses: avto-dev/markdown-lint@v1 + uses: avto-dev/markdown-lint@04d43ee9191307b50935a753da3b775ab695eceb # v1.5.0 with: ignore: external data/RTTR/MAPS . - name: Check licensing @@ -47,7 +49,7 @@ jobs: ADDITIONAL_CMAKE_FLAGS: "" runs-on: ubuntu-24.04 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - run: git submodule update --init - name: Install dependencies run: | diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 320fc4693b..7df22d94a5 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -1,8 +1,10 @@ -# Copyright (C) 2005 - 2025 Settlers Freaks +# Copyright (C) 2005 - 2026 Settlers Freaks # # SPDX-License-Identifier: GPL-2.0-or-later name: Unit tests +permissions: + contents: write on: push: @@ -37,11 +39,11 @@ jobs: - { os: windows-2022, generator: Visual Studio 17 2022, type: Release, platform: x64} runs-on: ${{matrix.os}} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: submodules: true - name: Install boost - uses: MarkusJx/install-boost@6d8df42f57de83c5b326b5b83e7b35d650030103 + uses: MarkusJx/install-boost@8ba8b2fac59ef3d91a838bb4aa53ceabd33d1aa3 # v2.5.1 id: install-boost with: boost_version: ${{env.BOOST_VERSION}} @@ -139,18 +141,18 @@ jobs: echo "GCOV=$GCOV" >> $GITHUB_ENV # Coverage collection requires access to history to find merge-base - - uses: actions/checkout@v4 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 if: "!matrix.coverage" with: submodules: true - - uses: actions/checkout@v4 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 if: matrix.coverage with: submodules: true fetch-depth: 0 # Full history - name: Cache dependencies - uses: actions/cache@v4 + uses: actions/cache@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4 with: path: ${{env.DEPS_DIR}} key: ${{matrix.os}}-${{env.BOOST_VERSION}} @@ -182,7 +184,7 @@ jobs: fi - name: Setup CCache - uses: hendrikmuhs/ccache-action@v1 + uses: hendrikmuhs/ccache-action@33522472633dbd32578e909b315f5ee43ba878ce # v1.2.22 with: key: ${{matrix.os}}-${{matrix.compiler}}-${{matrix.type}}-${{matrix.boost}} max-size: 200M @@ -218,7 +220,7 @@ jobs: - name: Upload coverage (Coveralls) if: matrix.coverage && success() - uses: coverallsapp/github-action@master + uses: coverallsapp/github-action@5cbfd81b66ca5d10c19b062c04de0199c215fb6e # v2.3.7 with: github-token: ${{secrets.GITHUB_TOKEN}} path-to-lcov: srccov.info From fa2a39fa2c33ac5831a7e17dd65d7a428e4e41f9 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 7 Apr 2026 14:38:22 +0200 Subject: [PATCH 15/19] Fix placing HQs randomly --- libs/s25main/world/MapLoader.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libs/s25main/world/MapLoader.cpp b/libs/s25main/world/MapLoader.cpp index 49283ab43a..da4d04a4a3 100644 --- a/libs/s25main/world/MapLoader.cpp +++ b/libs/s25main/world/MapLoader.cpp @@ -79,9 +79,6 @@ bool MapLoader::Load(const boost::filesystem::path& mapFilePath) if(!Load(map, world_.GetGGS().exploration)) return false; - std::vector hqPositions = hqPositions_; - if(world_.GetGGS().randomStartPosition) - RANDOM_SHUFFLE2(hqPositions, 0); if(!PlaceHQs()) return false; @@ -103,7 +100,10 @@ bool MapLoader::LoadLuaScript(Game& game, ILocalGameState& localgameState, const bool MapLoader::PlaceHQs(bool addStartWares) { - return PlaceHQs(world_, hqPositions_, addStartWares); + std::vector hqPositions = hqPositions_; + if(world_.GetGGS().randomStartPosition) + RANDOM_SHUFFLE2(hqPositions, 0); + return PlaceHQs(world_, hqPositions, addStartWares); } void MapLoader::InitShadows(World& world) From 60a0d21cca21eb8bee4d671e3dc37ac32de14a25 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 7 Apr 2026 15:48:09 +0200 Subject: [PATCH 16/19] Add missing LCOV_EXCL_STOP --- tests/s25Main/integration/testSerialization.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/s25Main/integration/testSerialization.cpp b/tests/s25Main/integration/testSerialization.cpp index 8830b019d1..095212e0c6 100644 --- a/tests/s25Main/integration/testSerialization.cpp +++ b/tests/s25Main/integration/testSerialization.cpp @@ -41,6 +41,7 @@ BOOST_TEST_DONT_PRINT_LOG_VALUE(Resource) BOOST_TEST_DONT_PRINT_LOG_VALUE(AddonId) BOOST_TEST_DONT_PRINT_LOG_VALUE(nofBuildingWorker::State) +// LCOV_EXCL_STOP namespace { using EmptyWorldFixture1P = WorldFixture; From f20cbf041d1d5ebcbda5926db8c78bd1f97ad183 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 7 Apr 2026 15:48:35 +0200 Subject: [PATCH 17/19] GHA: Use CodeCov action for upload --- .github/workflows/unit-tests.yml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 7df22d94a5..799b16d52c 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -212,12 +212,23 @@ jobs: - name: Test run: tools/ci/test.sh - - run: tools/ci/collectCoverageData.sh && external/libutil/tools/ci/uploadCoverageData.sh + - run: tools/ci/collectCoverageData.sh if: matrix.coverage && success() - run: tools/ci/checkTestCoverage.sh if: matrix.coverage && success() + - name: Upload coverage (Codecov) + if: matrix.coverage && success() + uses: codecov/codecov-action@57e3a136b779b570ffcdbf80b3bdc90e7fab3de2 # v6.0.0 + with: + fail_ci_if_error: true + disable_search: true + files: coverage.info + name: Github Actions + token: ${{secrets.CODECOV_TOKEN}} + verbose: true + - name: Upload coverage (Coveralls) if: matrix.coverage && success() uses: coverallsapp/github-action@5cbfd81b66ca5d10c19b062c04de0199c215fb6e # v2.3.7 From 20c1c6c24615b6f237ddd7fbf0e36ed2926b1763 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 7 Apr 2026 18:59:14 +0200 Subject: [PATCH 18/19] Add test for shuffling HQ positions --- libs/common/include/helpers/containerUtils.h | 11 +++- libs/s25main/languages.cpp | 1 + libs/s25main/world/MapLoader.h | 4 +- tests/s25Main/integration/testWorld.cpp | 65 ++++++++++++++------ 4 files changed, 57 insertions(+), 24 deletions(-) diff --git a/libs/common/include/helpers/containerUtils.h b/libs/common/include/helpers/containerUtils.h index 2bf3a318da..7519aa0163 100644 --- a/libs/common/include/helpers/containerUtils.h +++ b/libs/common/include/helpers/containerUtils.h @@ -128,6 +128,13 @@ size_t count_if(const T& container, T_Predicate&& predicate) return static_cast>(result); } +/// Sort collection with optional predicate +template> +void sort(T& container, T_Predicate&& predicate = T_Predicate{}) +{ + std::sort(begin(container), end(container), std::forward(predicate)); +} + /// Remove duplicate values from the given sorted container template void makeUniqueSorted(T& container) @@ -139,13 +146,13 @@ void makeUniqueSorted(T& container) template void makeUnique(T& container) { - std::sort(begin(container), end(container)); + sort(container); makeUniqueSorted(container); } template void makeUnique(T& container, T_Predicate&& predicate) { - std::sort(begin(container), end(container), std::forward(predicate)); + sort(container, std::forward(predicate)); makeUniqueSorted(container); } /// Remove duplicate values from the given container without changing the order diff --git a/libs/s25main/languages.cpp b/libs/s25main/languages.cpp index df01ae7765..b9cd9cde02 100644 --- a/libs/s25main/languages.cpp +++ b/libs/s25main/languages.cpp @@ -6,6 +6,7 @@ #include "Loader.h" #include "RttrConfig.h" #include "files.h" +#include "helpers/containerUtils.h" #include "mygettext/mygettext.h" #include "libsiedler2/ArchivItem_Ini.h" #include "libsiedler2/ArchivItem_Text.h" diff --git a/libs/s25main/world/MapLoader.h b/libs/s25main/world/MapLoader.h index 9e9352cb74..4dd35e9a36 100644 --- a/libs/s25main/world/MapLoader.h +++ b/libs/s25main/world/MapLoader.h @@ -51,8 +51,8 @@ class MapLoader /// Return false if there was an error (e.g. invalid start position) bool PlaceHQs(bool addStartWares = true); - /// Return the position of the players HQ (only valid after successful load) - MapPoint GetHQPos(unsigned player) const { return hqPositions_[player]; } + /// Return the (original/unshuffled) position of the players HQ (only valid after successful load) + MapPoint GetOriginalHQPos(unsigned player) const { return hqPositions_[player]; } static void InitShadows(World& world); static void SetMapExplored(World& world); diff --git a/tests/s25Main/integration/testWorld.cpp b/tests/s25Main/integration/testWorld.cpp index dfd4f1f0a2..950a6f6ba4 100644 --- a/tests/s25Main/integration/testWorld.cpp +++ b/tests/s25Main/integration/testWorld.cpp @@ -40,20 +40,6 @@ struct UninitializedWorldCreator bool operator()(GameWorldBase&) { return true; } }; -struct LoadWorldFromFileCreator : MapTestFixture -{ - std::vector hqs; - - explicit LoadWorldFromFileCreator(MapExtent) {} - bool operator()(GameWorldBase& world) - { - MapLoader loader(world); - BOOST_TEST_REQUIRE(loader.Load(testMapPath)); - for(unsigned i = 0; i < world.GetNumPlayers(); i++) - hqs.push_back(loader.GetHQPos(i)); - return true; - } -}; struct LoadWorldAndS2MapCreator : MapTestFixture { libsiedler2::ArchivItem_Map map; @@ -72,7 +58,6 @@ struct LoadWorldAndS2MapCreator : MapTestFixture }; using WorldLoadedWithS2MapFixture = WorldFixture; -using WorldLoaded1PFixture = WorldFixture; using WorldFixtureEmpty1P = WorldFixture; } // namespace @@ -133,12 +118,52 @@ BOOST_FIXTURE_TEST_CASE(SameBQasInS2, WorldLoadedWithS2MapFixture) } } -BOOST_FIXTURE_TEST_CASE(HQPlacement, WorldLoaded1PFixture) +BOOST_AUTO_TEST_CASE(HQPlacement) { - GamePlayer& player = world.GetPlayer(0); - BOOST_TEST_REQUIRE(player.isUsed()); - BOOST_TEST_REQUIRE(worldCreator.hqs[0].isValid()); - BOOST_TEST_REQUIRE(world.GetNO(worldCreator.hqs[0])->GetGOT() == GO_Type::NobHq); + constexpr auto numPlayers = 4u; + + std::vector hqsOriginalMap, hqsShuffledMap, hqsOriginalWorld, hqsShuffledWorld; + for(const bool randStartPos : {false, true}) + { + BOOST_TEST_INFO_SCOPE("Random: " << randStartPos); + WorldFixtureBase fixture(numPlayers); + fixture.ggs.randomStartPosition = randStartPos; + auto& world = fixture.world; + MapLoader loader(world); + BOOST_TEST_REQUIRE(loader.Load(testMapPath)); + auto& hqsMap = randStartPos ? hqsShuffledMap : hqsOriginalMap; + auto& hqsWorld = randStartPos ? hqsShuffledWorld : hqsOriginalWorld; + hqsWorld.resize(world.GetNumPlayers()); + for(unsigned i = 0; i < world.GetNumPlayers(); i++) + { + BOOST_TEST_INFO_SCOPE("Player: " << i); + GamePlayer& player = world.GetPlayer(i); + BOOST_TEST_REQUIRE(player.isUsed()); + + auto hqPos = loader.GetOriginalHQPos(i); + BOOST_TEST_REQUIRE(hqPos.isValid()); + BOOST_TEST(!helpers::contains(hqsMap, hqPos)); // Unique position + hqsMap.push_back(hqPos); + BOOST_TEST(world.GetNO(hqPos)->GetGOT() == GO_Type::NobHq); + + hqPos = world.GetPlayer(i).GetHQPos(); + BOOST_TEST_REQUIRE(hqPos.isValid()); + BOOST_TEST(!helpers::contains(hqsWorld, hqPos)); // Unique position + hqsWorld[i] = hqPos; + } + } + BOOST_TEST(hqsOriginalMap.size() == numPlayers); + BOOST_TEST(hqsShuffledMap.size() == numPlayers); + // The loader stores the HQ positions read from the map + BOOST_TEST(hqsShuffledMap == hqsOriginalMap); + // When shuffled the positions should have changed + BOOST_TEST(hqsShuffledWorld != hqsOriginalWorld); + helpers::sort(hqsOriginalMap, MapPointLess{}); + helpers::sort(hqsShuffledWorld, MapPointLess{}); + helpers::sort(hqsOriginalWorld, MapPointLess{}); + // But the same positions should be used just in different order + BOOST_TEST(hqsOriginalMap == hqsOriginalWorld); + BOOST_TEST(hqsShuffledWorld == hqsOriginalWorld); } BOOST_FIXTURE_TEST_CASE(CloseHarborSpots, WorldFixture) From a411a93982a5da771ee647b7e2cad45c0cf234e0 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 7 Apr 2026 19:04:56 +0200 Subject: [PATCH 19/19] Use `helpers::sort` consistently --- libs/common/include/helpers/containerUtils.h | 2 ++ libs/libGamedata/lua/CheckedLuaTable.cpp | 3 ++- libs/s25main/GamePlayer.cpp | 10 +++++----- libs/s25main/ListDir.cpp | 3 ++- libs/s25main/buildings/nobBaseMilitary.h | 2 +- libs/s25main/controls/ctrlTable.cpp | 3 ++- libs/s25main/desktops/dskOptions.cpp | 2 +- libs/s25main/gameTypes/HarborPos.h | 2 +- libs/s25main/languages.cpp | 4 ++-- libs/s25main/mapGenerator/HeadQuarters.h | 2 +- libs/s25main/mapGenerator/Textures.h | 2 +- libs/s25main/network/GameServer.cpp | 2 +- libs/s25main/ogl/glTexturePacker.cpp | 3 ++- libs/s25main/pathfinding/PathfindingPoint.h | 2 +- libs/s25main/world/GameWorld.cpp | 4 ++-- tests/s25Main/integration/testWorld.cpp | 8 ++++---- tests/s25Main/lua/testLua.cpp | 8 ++++---- tests/s25Main/simple/testMapBase.cpp | 5 +++-- 18 files changed, 37 insertions(+), 30 deletions(-) diff --git a/libs/common/include/helpers/containerUtils.h b/libs/common/include/helpers/containerUtils.h index 7519aa0163..7604e111f0 100644 --- a/libs/common/include/helpers/containerUtils.h +++ b/libs/common/include/helpers/containerUtils.h @@ -132,6 +132,8 @@ size_t count_if(const T& container, T_Predicate&& predicate) template> void sort(T& container, T_Predicate&& predicate = T_Predicate{}) { + using std::begin; + using std::end; std::sort(begin(container), end(container), std::forward(predicate)); } diff --git a/libs/libGamedata/lua/CheckedLuaTable.cpp b/libs/libGamedata/lua/CheckedLuaTable.cpp index 05b93e45f0..074778bb66 100644 --- a/libs/libGamedata/lua/CheckedLuaTable.cpp +++ b/libs/libGamedata/lua/CheckedLuaTable.cpp @@ -3,6 +3,7 @@ // SPDX-License-Identifier: GPL-2.0-or-later #include "CheckedLuaTable.h" +#include "helpers/containerUtils.h" #include "s25util/Log.h" #include #include @@ -22,7 +23,7 @@ void CheckedLuaTable::checkUnused() checkEnabled = false; std::vector tableKeys = table.keys(); - std::sort(tableKeys.begin(), tableKeys.end()); + helpers::sort(tableKeys); std::vector unusedKeys; std::set_difference(tableKeys.begin(), tableKeys.end(), accessedKeys_.begin(), accessedKeys_.end(), std::back_inserter(unusedKeys)); diff --git a/libs/s25main/GamePlayer.cpp b/libs/s25main/GamePlayer.cpp index b0cb04c01a..2230e3ccce 100644 --- a/libs/s25main/GamePlayer.cpp +++ b/libs/s25main/GamePlayer.cpp @@ -1022,7 +1022,7 @@ struct ClientForWare : bld(bld), estimate(estimate), points(points) {} - bool operator<(const ClientForWare& b) const + bool operator<(const ClientForWare& b) const noexcept { // use estimate, points and object id (as tie breaker) for sorting if(estimate != b.estimate) @@ -1118,7 +1118,7 @@ noBaseBuilding* GamePlayer::FindClientForWare(const Ware& ware) } // sort our clients, highest score first - std::sort(possibleClients.begin(), possibleClients.end()); + helpers::sort(possibleClients); noBaseBuilding* lastBld = nullptr; noBaseBuilding* bestBld = nullptr; @@ -1903,7 +1903,7 @@ struct ShipForHarbor ShipForHarbor(noShip* ship, uint32_t estimate) : ship(ship), estimate(estimate) {} - bool operator<(const ShipForHarbor& b) const + bool operator<(const ShipForHarbor& b) const noexcept { return (estimate < b.estimate) || (estimate == b.estimate && ship->GetObjId() < b.ship->GetObjId()); } @@ -1934,7 +1934,7 @@ bool GamePlayer::OrderShip(nobHarborBuilding& hb) } } - std::sort(sfh.begin(), sfh.end()); + helpers::sort(sfh); noShip* best_ship = nullptr; uint32_t best_distance = std::numeric_limits::max(); @@ -2313,7 +2313,7 @@ void GamePlayer::Trade(nobBaseWarehouse* goalWh, const boost_variant2GetFlagPos(); std::vector whs(buildings.GetStorehouses().begin(), buildings.GetStorehouses().end()); - std::sort(whs.begin(), whs.end(), WarehouseDistanceComparator(*goalWh, world)); + helpers::sort(whs, WarehouseDistanceComparator(*goalWh, world)); TradePathCache& tradePathCache = world.GetTradePathCache(); for(nobBaseWarehouse* wh : whs) { diff --git a/libs/s25main/ListDir.cpp b/libs/s25main/ListDir.cpp index 7d11d18a6d..0baf77495f 100644 --- a/libs/s25main/ListDir.cpp +++ b/libs/s25main/ListDir.cpp @@ -3,6 +3,7 @@ // SPDX-License-Identifier: GPL-2.0-or-later #include "ListDir.h" +#include "helpers/containerUtils.h" #include "s25util/strAlgos.h" #include #include @@ -41,6 +42,6 @@ std::vector ListDir(const bfs::path& path, std::string extension, boo result.emplace_back(std::move(curPath)); } - std::sort(result.begin(), result.end()); + helpers::sort(result); return result; } diff --git a/libs/s25main/buildings/nobBaseMilitary.h b/libs/s25main/buildings/nobBaseMilitary.h index 071d469ab0..0d4d6eb5e7 100644 --- a/libs/s25main/buildings/nobBaseMilitary.h +++ b/libs/s25main/buildings/nobBaseMilitary.h @@ -56,7 +56,7 @@ class nobBaseMilitary : public noBuilding const nofDefender* GetDefender() const { return defender_; } /// Compares according to build time (Age): Bigger objIds are "younger" - bool operator<(const nobBaseMilitary& other) const { return GetObjId() > other.GetObjId(); } + bool operator<(const nobBaseMilitary& other) const noexcept { return GetObjId() > other.GetObjId(); } /// Meldet ein neues "Rausgeh"-Event an, falls gerade keiner rausgeht /// (damit nicht alle auf einmal rauskommen), für Lager- und Militärhäuser) diff --git a/libs/s25main/controls/ctrlTable.cpp b/libs/s25main/controls/ctrlTable.cpp index ba9a843e77..48fb3d314d 100644 --- a/libs/s25main/controls/ctrlTable.cpp +++ b/libs/s25main/controls/ctrlTable.cpp @@ -8,6 +8,7 @@ #include "ctrlScrollBar.h" #include "driver/KeyEvent.h" #include "driver/MouseCoords.h" +#include "helpers/containerUtils.h" #include "helpers/mathFuncs.h" #include "ogl/glFont.h" #include "s25util/StringConversion.h" @@ -295,7 +296,7 @@ void ctrlTable::SortRows(unsigned column, const TableSortDir sortDir) const int cmpResult = Compare(a, b, sortType); return (sortDir == TableSortDir::Ascending) ? cmpResult < 0 : cmpResult > 0; }; - std::sort(rows_.begin(), rows_.end(), compareFunc); + helpers::sort(rows_, compareFunc); } /** diff --git a/libs/s25main/desktops/dskOptions.cpp b/libs/s25main/desktops/dskOptions.cpp index 1048729aa5..bd3145808e 100644 --- a/libs/s25main/desktops/dskOptions.cpp +++ b/libs/s25main/desktops/dskOptions.cpp @@ -743,7 +743,7 @@ void dskOptions::loadVideoModes() // Remove everything below 800x600 helpers::erase_if(video_modes, [](const auto& it) { return it.width < 800 && it.height < 600; }); // Sort by aspect ratio - std::sort(video_modes.begin(), video_modes.end(), cmpVideoModes); + helpers::sort(video_modes, cmpVideoModes); } void dskOptions::Msg_ScreenResize(const ScreenResizeEvent& sr) diff --git a/libs/s25main/gameTypes/HarborPos.h b/libs/s25main/gameTypes/HarborPos.h index 0e147649ea..bff3f4347a 100644 --- a/libs/s25main/gameTypes/HarborPos.h +++ b/libs/s25main/gameTypes/HarborPos.h @@ -19,7 +19,7 @@ struct HarborPos Neighbor(HarborId id, unsigned distance) noexcept : id(id), distance(distance) {} - bool operator<(const Neighbor& two) const + bool operator<(const Neighbor& two) const noexcept { return (distance < two.distance) || (distance == two.distance && id.value() < two.id.value()); } diff --git a/libs/s25main/languages.cpp b/libs/s25main/languages.cpp index b9cd9cde02..9d056dfae3 100644 --- a/libs/s25main/languages.cpp +++ b/libs/s25main/languages.cpp @@ -12,7 +12,7 @@ #include "libsiedler2/ArchivItem_Text.h" #include -static bool operator<(const Language& o1, const Language& o2) +static bool operator<(const Language& o1, const Language& o2) noexcept { if(o1.name < o2.name) return true; @@ -45,7 +45,7 @@ void Languages::loadLanguages() } // Sprachen sortieren - std::sort(languages.begin(), languages.end()); + helpers::sort(languages); // Systemsprache hinzufügen Language l(gettext_noop("System language"), ""); diff --git a/libs/s25main/mapGenerator/HeadQuarters.h b/libs/s25main/mapGenerator/HeadQuarters.h index 56f00d4b64..cbfbfe8c8f 100644 --- a/libs/s25main/mapGenerator/HeadQuarters.h +++ b/libs/s25main/mapGenerator/HeadQuarters.h @@ -66,7 +66,7 @@ std::vector FindHqPositions(const Map& map, const T_Container& area, M const auto byDistanceToOtherHqs = [&distanceToOtherHqs](MapPoint p1, MapPoint p2) { return distanceToOtherHqs[p1] > distanceToOtherHqs[p2]; }; - std::sort(possiblePositions.begin(), possiblePositions.end(), byDistanceToOtherHqs); + helpers::sort(possiblePositions, byDistanceToOtherHqs); // 3. filter out points within minimum distance to existing HQs const unsigned minHqDistance = (map.size.x + map.size.y) / 16; diff --git a/libs/s25main/mapGenerator/Textures.h b/libs/s25main/mapGenerator/Textures.h index 1d2ca10509..71719c0590 100644 --- a/libs/s25main/mapGenerator/Textures.h +++ b/libs/s25main/mapGenerator/Textures.h @@ -71,7 +71,7 @@ class TextureOperator return sortBy(worldDesc_.get(t1)) < sortBy(worldDesc_.get(t2)); }; - std::sort(textures.begin(), textures.end(), lessThan); + helpers::sort(textures, lessThan); } template diff --git a/libs/s25main/network/GameServer.cpp b/libs/s25main/network/GameServer.cpp index 1eb58d7bd3..d0bfa26f6f 100644 --- a/libs/s25main/network/GameServer.cpp +++ b/libs/s25main/network/GameServer.cpp @@ -476,7 +476,7 @@ bool GameServer::assignPlayersOfRandomTeams(std::vector& playerI } // To make the teams as even as possible we start to assign the most constrained players first - std::sort(playersToAssign.begin(), playersToAssign.end(), [](const AssignPlayer& lhs, const AssignPlayer& rhs) { + helpers::sort(playersToAssign, [](const AssignPlayer& lhs, const AssignPlayer& rhs) { return lhs.possibleTeams.size() < rhs.possibleTeams.size(); }); diff --git a/libs/s25main/ogl/glTexturePacker.cpp b/libs/s25main/ogl/glTexturePacker.cpp index 2641a2dfa1..978d6a4043 100644 --- a/libs/s25main/ogl/glTexturePacker.cpp +++ b/libs/s25main/ogl/glTexturePacker.cpp @@ -4,6 +4,7 @@ #include "glTexturePacker.h" #include "drivers/VideoDriverWrapper.h" +#include "helpers/containerUtils.h" #include "ogl/glSmartBitmap.h" #include "ogl/glTexturePackerNode.h" #include "ogl/saveBitmap.h" @@ -117,7 +118,7 @@ bool glTexturePacker::packHelper(std::vector& list) bool glTexturePacker::pack() { - std::sort(items.begin(), items.end(), isSizeGreater); + helpers::sort(items, isSizeGreater); if(packHelper(items)) return true; diff --git a/libs/s25main/pathfinding/PathfindingPoint.h b/libs/s25main/pathfinding/PathfindingPoint.h index fee8765d60..b52c249d45 100644 --- a/libs/s25main/pathfinding/PathfindingPoint.h +++ b/libs/s25main/pathfinding/PathfindingPoint.h @@ -17,7 +17,7 @@ struct PathfindingPoint {} /// Operator für den Vergleich - bool operator<(const PathfindingPoint& rhs) const + bool operator<(const PathfindingPoint& rhs) const noexcept { // Wenn die Wegkosten gleich sind, vergleichen wir die Koordinaten, da wir für std::set eine streng monoton // steigende Folge brauchen diff --git a/libs/s25main/world/GameWorld.cpp b/libs/s25main/world/GameWorld.cpp index ed09640eb4..c3f99881ac 100644 --- a/libs/s25main/world/GameWorld.cpp +++ b/libs/s25main/world/GameWorld.cpp @@ -842,9 +842,9 @@ void GameWorld::AttackViaSea(const unsigned char player_attacker, const MapPoint // Sort them if(strong_soldiers) - std::sort(attackers.begin(), attackers.end(), CmpSeaAttacker>()); + helpers::sort(attackers, CmpSeaAttacker>()); else - std::sort(attackers.begin(), attackers.end(), CmpSeaAttacker>()); + helpers::sort(attackers, CmpSeaAttacker>()); auto& attacked_building = *GetSpecObj(pt); unsigned counter = 0; diff --git a/tests/s25Main/integration/testWorld.cpp b/tests/s25Main/integration/testWorld.cpp index 950a6f6ba4..963efabb4b 100644 --- a/tests/s25Main/integration/testWorld.cpp +++ b/tests/s25Main/integration/testWorld.cpp @@ -155,15 +155,15 @@ BOOST_AUTO_TEST_CASE(HQPlacement) BOOST_TEST(hqsOriginalMap.size() == numPlayers); BOOST_TEST(hqsShuffledMap.size() == numPlayers); // The loader stores the HQ positions read from the map - BOOST_TEST(hqsShuffledMap == hqsOriginalMap); + BOOST_TEST(hqsShuffledMap == hqsOriginalMap, boost::test_tools::per_element()); // When shuffled the positions should have changed - BOOST_TEST(hqsShuffledWorld != hqsOriginalWorld); + BOOST_TEST(hqsShuffledWorld != hqsOriginalWorld, boost::test_tools::per_element()); helpers::sort(hqsOriginalMap, MapPointLess{}); helpers::sort(hqsShuffledWorld, MapPointLess{}); helpers::sort(hqsOriginalWorld, MapPointLess{}); // But the same positions should be used just in different order - BOOST_TEST(hqsOriginalMap == hqsOriginalWorld); - BOOST_TEST(hqsShuffledWorld == hqsOriginalWorld); + BOOST_TEST(hqsOriginalMap == hqsOriginalWorld, boost::test_tools::per_element()); + BOOST_TEST(hqsShuffledWorld == hqsOriginalWorld, boost::test_tools::per_element()); } BOOST_FIXTURE_TEST_CASE(CloseHarborSpots, WorldFixture) diff --git a/tests/s25Main/lua/testLua.cpp b/tests/s25Main/lua/testLua.cpp index b8d9c3e29b..944174a12d 100644 --- a/tests/s25Main/lua/testLua.cpp +++ b/tests/s25Main/lua/testLua.cpp @@ -819,8 +819,8 @@ BOOST_AUTO_TEST_CASE(onOccupied) { Points& gamePts = gamePtsPerPlayer[i]; Points& luaPts = luaPtsPerPlayer[i]; - std::sort(gamePts.begin(), gamePts.end()); - std::sort(luaPts.begin(), luaPts.end()); + helpers::sort(gamePts); + helpers::sort(luaPts); BOOST_TEST_REQUIRE(luaPts == gamePts, boost::test_tools::per_element()); } } @@ -851,8 +851,8 @@ BOOST_AUTO_TEST_CASE(onExplored) { Points& gamePts = gamePtsPerPlayer[i]; Points& luaPts = luaPtsPerPlayer[i]; - std::sort(gamePts.begin(), gamePts.end()); - std::sort(luaPts.begin(), luaPts.end()); + helpers::sort(gamePts); + helpers::sort(luaPts); BOOST_TEST_REQUIRE(luaPts == gamePts, boost::test_tools::per_element()); } } diff --git a/tests/s25Main/simple/testMapBase.cpp b/tests/s25Main/simple/testMapBase.cpp index 37587184d8..c4e7722a9f 100644 --- a/tests/s25Main/simple/testMapBase.cpp +++ b/tests/s25Main/simple/testMapBase.cpp @@ -5,6 +5,7 @@ #include "PointOutput.h" #include "enum_cast.hpp" #include "helpers/EnumArray.h" +#include "helpers/containerUtils.h" #include "world/MapBase.h" #include "world/MapGeometry.h" #include "gameData/MapConsts.h" @@ -39,8 +40,8 @@ BOOST_AUTO_TEST_CASE(GetAllNeighboursUnion) auto resultPoints = world.GetAllNeighboursUnion(testPoints); - std::sort(resultPoints.begin(), resultPoints.end(), MapPointLess()); - std::sort(expectedResultPoints.begin(), expectedResultPoints.end(), MapPointLess()); + helpers::sort(resultPoints, MapPointLess()); + helpers::sort(expectedResultPoints, MapPointLess()); BOOST_TEST(resultPoints == expectedResultPoints, boost::test_tools::per_element()); }