Code review: Rename functions, remove auto return type and capture addons checks as bool const, use Gamesettings as parameter instead of player

Author: ottml

libs/s25main/AddonHelperFunctions.cpp

@@ -4,43 +4,53 @@
 
 #include "AddonHelperFunctions.h"
 #include "GlobalGameSettings.h"
-#include "world/GameWorld.h"
 #include <LeatherLoader.h>
 #include <WineLoader.h>
 #include <addons/Addon.h>
 
-auto isUnusedBuilding(GamePlayer const& player) -> std::function<bool(BuildingType const& type)>
+std::function<bool(const BuildingType type)> makeIsUnusedBuilding(const GlobalGameSettings& ggs)
 {
-    return [&player = player](BuildingType const& bld) {
-        if(!wineaddon::isAddonActive(player.GetGameWorld()) && wineaddon::isWineAddonBuildingType(bld))
+    const bool wineAddonActive = ggs.isEnabled(AddonId::WINE);
+    const bool charburnerAddonActive = ggs.isEnabled(AddonId::CHARBURNER);
+    const bool leatherAddonActive = ggs.isEnabled(AddonId::LEATHER);
+
+    return [wineAddonActive, charburnerAddonActive, leatherAddonActive](BuildingType const& bld) {
+        if(!wineAddonActive && wineaddon::isWineAddonBuildingType(bld))
             return true;
-        if(!player.GetGameWorld().GetGGS().isEnabled(AddonId::CHARBURNER) && bld == BuildingType::Charburner)
+        if(!charburnerAddonActive && bld == BuildingType::Charburner)
             return true;
-        if(!leatheraddon::isAddonActive(player.GetGameWorld()) && leatheraddon::isLeatherAddonBuildingType(bld))
+        if(!leatherAddonActive && leatheraddon::isLeatherAddonBuildingType(bld))
             return true;
         return false;
     };
 }
 
-auto isUnusedWare(GamePlayer const& player) -> std::function<bool(GoodType const& type)>
+std::function<bool(const GoodType type)> makeIsUnusedWare(const GlobalGameSettings& ggs)
 {
-    return [&player = player](GoodType const& type) {
-        if(!wineaddon::isAddonActive(player.GetGameWorld()) && wineaddon::isWineAddonGoodType(type))
+    const bool wineAddonActive = ggs.isEnabled(AddonId::WINE);
+    const bool leatherAddonActive = ggs.isEnabled(AddonId::LEATHER);
+
+    return [wineAddonActive, leatherAddonActive](GoodType const& type) {
+        if(!wineAddonActive && wineaddon::isWineAddonGoodType(type))
             return true;
-        if(!leatheraddon::isAddonActive(player.GetGameWorld()) && leatheraddon::isLeatherAddonGoodType(type))
+        if(!leatherAddonActive && leatheraddon::isLeatherAddonGoodType(type))
             return true;
         return false;
     };
 }
 
-auto isUnusedJob(GamePlayer const& player) -> std::function<bool(Job const& job)>
+std::function<bool(const Job job)> makeIsUnusedJob(const GlobalGameSettings& ggs)
 {
-    return [&](Job const& job) {
-        if(!wineaddon::isAddonActive(player.GetGameWorld()) && wineaddon::isWineAddonJobType(job))
+    const bool wineAddonActive = ggs.isEnabled(AddonId::WINE);
+    const bool charburnerAddonActive = ggs.isEnabled(AddonId::CHARBURNER);
+    const bool leatherAddonActive = ggs.isEnabled(AddonId::LEATHER);
+
+    return [wineAddonActive, charburnerAddonActive, leatherAddonActive](Job const& job) {
+        if(!wineAddonActive && wineaddon::isWineAddonJobType(job))
             return true;
-        if(!player.GetGameWorld().GetGGS().isEnabled(AddonId::CHARBURNER) && job == Job::CharBurner)
+        if(!charburnerAddonActive && job == Job::CharBurner)
             return true;
-        if(!leatheraddon::isAddonActive(player.GetGameWorld()) && leatheraddon::isLeatherAddonJobType(job))
+        if(!leatherAddonActive && leatheraddon::isLeatherAddonJobType(job))
             return true;
         return false;
     };

libs/s25main/AddonHelperFunctions.h

@@ -4,8 +4,13 @@
 
 #pragma once
 
-#include "GamePlayer.h"
+#include "gameTypes/BuildingType.h"
+#include "gameTypes/GoodTypes.h"
+#include "gameTypes/JobTypes.h"
+#include <functional>
 
-auto isUnusedBuilding(GamePlayer const& player) -> std::function<bool(BuildingType const& type)>;
-auto isUnusedWare(GamePlayer const& player) -> std::function<bool(GoodType const& type)>;
-auto isUnusedJob(GamePlayer const& player) -> std::function<bool(Job const& job)>;
+class GlobalGameSettings;
+
+std::function<bool(const BuildingType type)> makeIsUnusedBuilding(const GlobalGameSettings& ggs);
+std::function<bool(const GoodType type)> makeIsUnusedWare(const GlobalGameSettings& ggs);
+std::function<bool(const Job job)> makeIsUnusedJob(const GlobalGameSettings& ggs);

libs/s25main/ingameWindows/iwBuildOrder.cpp

@@ -23,7 +23,7 @@
 void iwBuildOrder::fillBuildOrder(const BuildOrders& build_order)
 {
     pendingBuildOrder.assign(build_order.begin(), build_order.end());
-    helpers::erase_if(pendingBuildOrder, isUnusedBuilding(gwv.GetPlayer()));
+    helpers::erase_if(pendingBuildOrder, makeIsUnusedBuilding(gwv.GetWorld().GetGGS()));
 }
 
 iwBuildOrder::iwBuildOrder(const GameWorldViewer& gwv)

libs/s25main/ingameWindows/iwBuildingProductivities.cpp

@@ -40,7 +40,7 @@ const std::array<BuildingType, 30> iwBuildingProductivities::allIcons = {
 void iwBuildingProductivities::setBuildingOrder()
 {
     usedIcons.assign(allIcons.begin(), allIcons.end());
-    helpers::erase_if(usedIcons, isUnusedBuilding(player));
+    helpers::erase_if(usedIcons, makeIsUnusedBuilding(player.GetGameWorld().GetGGS()));
 }
 
 /// Abstand vom linken, oberen Fensterrand

libs/s25main/ingameWindows/iwBuildings.cpp

@@ -51,7 +51,7 @@ void iwBuildings::setBuildingOrder()
       BuildingType::Skinner,        BuildingType::Tannery,    BuildingType::LeatherWorks, // entry 37
     };
 
-    helpers::erase_if(bts, isUnusedBuilding(gwv.GetViewer().GetPlayer()));
+    helpers::erase_if(bts, makeIsUnusedBuilding(gwv.GetWorld().GetGGS()));
 }
 
 // Abstand des ersten Icons vom linken oberen Fensterrand

libs/s25main/ingameWindows/iwDistribution.cpp

@@ -204,7 +204,7 @@ void iwDistribution::CreateGroups()
 
     auto isUnused = [&](std::tuple<std::string, unsigned> const& bts) {
         const BuildingType buildingType = std::get<1>(distributionMap[std::get<1>(bts)]);
-        return isUnusedBuilding(gwv.GetPlayer())(buildingType);
+        return makeIsUnusedBuilding(gwv.GetWorld().GetGGS())(buildingType);
     };
     for(auto& group : groups)
         helpers::erase_if(group.entries, isUnused);

libs/s25main/ingameWindows/iwWares.cpp

@@ -114,8 +114,8 @@ iwWares::iwWares(unsigned id, const DrawPoint& pos, unsigned additionalYSpace, c
                                        Job::Sergeant,      Job::Officer,
                                        Job::General};
 
-    helpers::erase_if(WARE_DISPLAY_ORDER, isUnusedWare(player));
-    helpers::erase_if(JOB_DISPLAY_ORDER, isUnusedJob(player));
+    helpers::erase_if(WARE_DISPLAY_ORDER, makeIsUnusedWare(player.GetGameWorld().GetGGS()));
+    helpers::erase_if(JOB_DISPLAY_ORDER, makeIsUnusedJob(player.GetGameWorld().GetGGS()));
 
     // Warenseite hinzufügen
     ctrlGroup& waresPage = AddPage();