From 7c824c25a18cdb0b81b1ba2aab6c89b40b2b7700 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Fri, 10 Jan 2025 03:24:43 -0700 Subject: [PATCH 01/10] Move `CrimeExecution` header data to unnamed namespace This data doesn't need to be part of the class, nor does it need to be in the header where changes would cause a cascade of recompiles. --- OPHD/States/CrimeExecution.cpp | 22 ++++++++++++++++++++++ OPHD/States/CrimeExecution.h | 18 ------------------ 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/OPHD/States/CrimeExecution.cpp b/OPHD/States/CrimeExecution.cpp index d669b2a33..43f0af517 100644 --- a/OPHD/States/CrimeExecution.cpp +++ b/OPHD/States/CrimeExecution.cpp @@ -9,6 +9,28 @@ #include +namespace +{ + const static inline std::map stealingMultipliers + { + {Difficulty::Beginner, 0.5f}, + {Difficulty::Easy, 0.75f}, + {Difficulty::Medium, 1.0f}, + {Difficulty::Hard, 1.5f} + }; + + const static inline std::vector stealingResoureReasons + { + "There are no identified suspects", + "An investigation has been opened", + "A local crime syndicate is under investigation", + "A suspect was aprehended but the goods remain unaccounted for", + "A separatist political movement has claimed responsibility", + "The rebel faction is suspected in preparation for a splinter colony" + }; +} + + CrimeExecution::CrimeExecution(NotificationArea& notificationArea) : mNotificationArea(notificationArea) {} diff --git a/OPHD/States/CrimeExecution.h b/OPHD/States/CrimeExecution.h index 29a947184..c997abe2c 100644 --- a/OPHD/States/CrimeExecution.h +++ b/OPHD/States/CrimeExecution.h @@ -30,24 +30,6 @@ class CrimeExecution std::vector> moraleChanges() const { return mMoraleChanges; } private: - const static inline std::map stealingMultipliers - { - {Difficulty::Beginner, 0.5f}, - {Difficulty::Easy, 0.75f}, - {Difficulty::Medium, 1.0f}, - {Difficulty::Hard, 1.5f} - }; - - const static inline std::vector stealingResoureReasons - { - "There are no identified suspects", - "An investigation has been opened", - "A local crime syndicate is under investigation", - "A suspect was aprehended but the goods remain unaccounted for", - "A separatist political movement has claimed responsibility", - "The rebel faction is suspected in preparation for a splinter colony" - }; - Difficulty mDifficulty{Difficulty::Medium}; NotificationArea& mNotificationArea; std::vector> mMoraleChanges; From de6d6d331e8169555ddcaf6edc18de271a63d453 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Fri, 10 Jan 2025 03:27:09 -0700 Subject: [PATCH 02/10] Move `getReasonForStealing` from header to unnamed namespace This method doesn't use any member field, so doesn't need to be part of the class. --- OPHD/States/CrimeExecution.cpp | 12 ++++++------ OPHD/States/CrimeExecution.h | 1 - 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/OPHD/States/CrimeExecution.cpp b/OPHD/States/CrimeExecution.cpp index 43f0af517..1d400371c 100644 --- a/OPHD/States/CrimeExecution.cpp +++ b/OPHD/States/CrimeExecution.cpp @@ -28,6 +28,12 @@ namespace "A separatist political movement has claimed responsibility", "The rebel faction is suspected in preparation for a splinter colony" }; + + + std::string getReasonForStealing() + { + return stealingResoureReasons[randomNumber.generate(0, stealingResoureReasons.size() - 1)]; + } } @@ -150,9 +156,3 @@ int CrimeExecution::calcAmountForStealing(int unadjustedMin, int unadjustedMax) return static_cast(stealingMultipliers.at(mDifficulty) * amountToSteal); } - - -std::string CrimeExecution::getReasonForStealing() -{ - return stealingResoureReasons[randomNumber.generate(0, stealingResoureReasons.size() - 1)]; -} diff --git a/OPHD/States/CrimeExecution.h b/OPHD/States/CrimeExecution.h index c997abe2c..367de2e27 100644 --- a/OPHD/States/CrimeExecution.h +++ b/OPHD/States/CrimeExecution.h @@ -36,5 +36,4 @@ class CrimeExecution void stealResources(Structure& structure, const std::array& resourceNames); int calcAmountForStealing(int unadjustedMin, int unadjustedMax); - std::string getReasonForStealing(); }; From c85007f5078bfd79b5e8611b4fcac1d5583e36e1 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Fri, 10 Jan 2025 04:15:53 -0700 Subject: [PATCH 03/10] Move `calcAmountForStealing` to unnamed namespace Pass `Difficulty` parameter so we don't need to access any class members, and can remove this method from the class. --- OPHD/States/CrimeExecution.cpp | 20 ++++++++++---------- OPHD/States/CrimeExecution.h | 1 - 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/OPHD/States/CrimeExecution.cpp b/OPHD/States/CrimeExecution.cpp index 1d400371c..14a7914f6 100644 --- a/OPHD/States/CrimeExecution.cpp +++ b/OPHD/States/CrimeExecution.cpp @@ -34,6 +34,14 @@ namespace { return stealingResoureReasons[randomNumber.generate(0, stealingResoureReasons.size() - 1)]; } + + + int calcAmountForStealing(Difficulty difficulty, int unadjustedMin, int unadjustedMax) + { + auto amountToSteal = randomNumber.generate(unadjustedMin, unadjustedMax); + + return static_cast(stealingMultipliers.at(difficulty) * amountToSteal); + } } @@ -76,7 +84,7 @@ void CrimeExecution::stealFood(FoodProduction& structure) { if (structure.foodLevel() > 0) { - int foodStolen = calcAmountForStealing(5, 15); + int foodStolen = calcAmountForStealing(mDifficulty, 5, 15); if (foodStolen > structure.foodLevel()) { foodStolen = structure.foodLevel(); @@ -118,7 +126,7 @@ void CrimeExecution::stealResources(Structure& structure, const std::array(0, resourceIndicesWithStock.size() - 1); - int amountStolen = calcAmountForStealing(2, 5); + int amountStolen = calcAmountForStealing(mDifficulty, 2, 5); if (amountStolen > structure.storage().resources[indexToStealFrom]) { amountStolen = structure.storage().resources[indexToStealFrom]; @@ -148,11 +156,3 @@ void CrimeExecution::vandalize(Structure& structure) structureTile.xyz(), NotificationArea::NotificationType::Warning}); } - - -int CrimeExecution::calcAmountForStealing(int unadjustedMin, int unadjustedMax) -{ - auto amountToSteal = randomNumber.generate(unadjustedMin, unadjustedMax); - - return static_cast(stealingMultipliers.at(mDifficulty) * amountToSteal); -} diff --git a/OPHD/States/CrimeExecution.h b/OPHD/States/CrimeExecution.h index 367de2e27..4447c9b4c 100644 --- a/OPHD/States/CrimeExecution.h +++ b/OPHD/States/CrimeExecution.h @@ -35,5 +35,4 @@ class CrimeExecution std::vector> mMoraleChanges; void stealResources(Structure& structure, const std::array& resourceNames); - int calcAmountForStealing(int unadjustedMin, int unadjustedMax); }; From 18beccca11c6c879f30ef514c58878d17e972811 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Fri, 10 Jan 2025 04:17:08 -0700 Subject: [PATCH 04/10] Integrate clipping into `calcAmountForStealing` This reveals an obvious bug in the `stealFood` method. --- OPHD/States/CrimeExecution.cpp | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/OPHD/States/CrimeExecution.cpp b/OPHD/States/CrimeExecution.cpp index 14a7914f6..a03cc9648 100644 --- a/OPHD/States/CrimeExecution.cpp +++ b/OPHD/States/CrimeExecution.cpp @@ -36,11 +36,12 @@ namespace } - int calcAmountForStealing(Difficulty difficulty, int unadjustedMin, int unadjustedMax) + int calcAmountForStealing(Difficulty difficulty, int low, int high, int max) { - auto amountToSteal = randomNumber.generate(unadjustedMin, unadjustedMax); - - return static_cast(stealingMultipliers.at(difficulty) * amountToSteal); + auto stealRandom = randomNumber.generate(low, high); + auto stealModified = static_cast(stealingMultipliers.at(difficulty) * stealRandom); + auto stealClipped = std::min(stealModified, max); + return stealClipped; } } @@ -84,12 +85,7 @@ void CrimeExecution::stealFood(FoodProduction& structure) { if (structure.foodLevel() > 0) { - int foodStolen = calcAmountForStealing(mDifficulty, 5, 15); - if (foodStolen > structure.foodLevel()) - { - foodStolen = structure.foodLevel(); - } - + int foodStolen = calcAmountForStealing(mDifficulty, 5, 15, structure.foodLevel()); structure.foodLevel(-foodStolen); const auto& structureTile = NAS2D::Utility::get().tileFromStructure(&structure); @@ -126,12 +122,7 @@ void CrimeExecution::stealResources(Structure& structure, const std::array(0, resourceIndicesWithStock.size() - 1); - int amountStolen = calcAmountForStealing(mDifficulty, 2, 5); - if (amountStolen > structure.storage().resources[indexToStealFrom]) - { - amountStolen = structure.storage().resources[indexToStealFrom]; - } - + int amountStolen = calcAmountForStealing(mDifficulty, 2, 5, structure.storage().resources[indexToStealFrom]); structure.storage().resources[indexToStealFrom] -= amountStolen; const auto& structureTile = NAS2D::Utility::get().tileFromStructure(&structure); From 876694dfa9a69747d166365094074f5b2174d0ae Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Fri, 10 Jan 2025 04:18:44 -0700 Subject: [PATCH 05/10] Fix bug in `stealFood` --- OPHD/States/CrimeExecution.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OPHD/States/CrimeExecution.cpp b/OPHD/States/CrimeExecution.cpp index a03cc9648..deb4014d0 100644 --- a/OPHD/States/CrimeExecution.cpp +++ b/OPHD/States/CrimeExecution.cpp @@ -86,7 +86,7 @@ void CrimeExecution::stealFood(FoodProduction& structure) if (structure.foodLevel() > 0) { int foodStolen = calcAmountForStealing(mDifficulty, 5, 15, structure.foodLevel()); - structure.foodLevel(-foodStolen); + structure.foodLevel(structure.foodLevel() - foodStolen); const auto& structureTile = NAS2D::Utility::get().tileFromStructure(&structure); From 52d3e64a346415f1112b2617b3c5595bbf6e6b61 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Fri, 10 Jan 2025 04:33:34 -0700 Subject: [PATCH 06/10] Remove need to cast from `double` to `int` --- OPHD/States/CrimeExecution.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/OPHD/States/CrimeExecution.cpp b/OPHD/States/CrimeExecution.cpp index deb4014d0..20604b02b 100644 --- a/OPHD/States/CrimeExecution.cpp +++ b/OPHD/States/CrimeExecution.cpp @@ -11,12 +11,13 @@ namespace { - const static inline std::map stealingMultipliers + const static inline int stealingScale = 4; + const static inline std::map stealingMultipliers { - {Difficulty::Beginner, 0.5f}, - {Difficulty::Easy, 0.75f}, - {Difficulty::Medium, 1.0f}, - {Difficulty::Hard, 1.5f} + {Difficulty::Beginner, 2}, + {Difficulty::Easy, 3}, + {Difficulty::Medium, 4}, + {Difficulty::Hard, 6} }; const static inline std::vector stealingResoureReasons @@ -39,7 +40,7 @@ namespace int calcAmountForStealing(Difficulty difficulty, int low, int high, int max) { auto stealRandom = randomNumber.generate(low, high); - auto stealModified = static_cast(stealingMultipliers.at(difficulty) * stealRandom); + auto stealModified = stealRandom * stealingMultipliers.at(difficulty) / stealingScale; auto stealClipped = std::min(stealModified, max); return stealClipped; } From 7e7b0d3cbc8af937c049695280ef6a86e8ca96c5 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Fri, 10 Jan 2025 04:44:32 -0700 Subject: [PATCH 07/10] Remove unnecessary `nullptr` check Before any `structure` is added to the list, it is already dereferenced to determine if it should be on the list. That means any undefined behavior from dereferencing a `nullptr` would have already happened earlier. --- OPHD/States/CrimeExecution.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/OPHD/States/CrimeExecution.cpp b/OPHD/States/CrimeExecution.cpp index 20604b02b..97465ced0 100644 --- a/OPHD/States/CrimeExecution.cpp +++ b/OPHD/States/CrimeExecution.cpp @@ -56,11 +56,6 @@ void CrimeExecution::executeCrimes(const std::vector& structuresComm for (auto& structure : structuresCommittingCrime) { - if (structure == nullptr) - { - continue; - } - switch (structure->structureId()) { case StructureID::SID_AGRIDOME: From 0c7bc3ee4b02bab92562291919a615c6f7f27e2c Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Fri, 10 Jan 2025 05:16:42 -0700 Subject: [PATCH 08/10] Adjust formatting of `CrimeExecution` constructor --- OPHD/States/CrimeExecution.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/OPHD/States/CrimeExecution.cpp b/OPHD/States/CrimeExecution.cpp index 97465ced0..20cbf84e9 100644 --- a/OPHD/States/CrimeExecution.cpp +++ b/OPHD/States/CrimeExecution.cpp @@ -47,7 +47,10 @@ namespace } -CrimeExecution::CrimeExecution(NotificationArea& notificationArea) : mNotificationArea(notificationArea) {} +CrimeExecution::CrimeExecution(NotificationArea& notificationArea) : + mNotificationArea{notificationArea} +{ +} void CrimeExecution::executeCrimes(const std::vector& structuresCommittingCrime) From 0eed46327e64667028c854fd845129612c7090cd Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Fri, 10 Jan 2025 13:19:10 -0700 Subject: [PATCH 09/10] Remove `static inline` from variables in unnamed namespace --- OPHD/States/CrimeExecution.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/OPHD/States/CrimeExecution.cpp b/OPHD/States/CrimeExecution.cpp index 20cbf84e9..01dc6be7a 100644 --- a/OPHD/States/CrimeExecution.cpp +++ b/OPHD/States/CrimeExecution.cpp @@ -11,8 +11,8 @@ namespace { - const static inline int stealingScale = 4; - const static inline std::map stealingMultipliers + const int stealingScale = 4; + const std::map stealingMultipliers { {Difficulty::Beginner, 2}, {Difficulty::Easy, 3}, @@ -20,7 +20,7 @@ namespace {Difficulty::Hard, 6} }; - const static inline std::vector stealingResoureReasons + const std::vector stealingResoureReasons { "There are no identified suspects", "An investigation has been opened", From 7245e80ed97d8b4c5d263bd7dce5f8ae6867b6b3 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Fri, 10 Jan 2025 13:20:12 -0700 Subject: [PATCH 10/10] Use `constexpr` rather than `const` for unnamed namespace variable --- OPHD/States/CrimeExecution.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OPHD/States/CrimeExecution.cpp b/OPHD/States/CrimeExecution.cpp index 01dc6be7a..a0c077375 100644 --- a/OPHD/States/CrimeExecution.cpp +++ b/OPHD/States/CrimeExecution.cpp @@ -11,7 +11,7 @@ namespace { - const int stealingScale = 4; + constexpr int stealingScale = 4; const std::map stealingMultipliers { {Difficulty::Beginner, 2},