Merge pull request #125 from DFHack/desmell

Remove code smells from `stonesense`, pt. 1
DFHack · Jan 12, 2025 · 8f40e92 · 8f40e92
2 parents be9fe60 + daa7b8a
commit 8f40e92
Show file tree

Hide file tree

Showing 34 changed files with 1,693 additions and 1,903 deletions.
diff --git a/BuildingConfiguration.cpp b/BuildingConfiguration.cpp
diff --git a/BuildingConfiguration.h b/BuildingConfiguration.h
@@ -15,7 +15,22 @@ class BuildingConfiguration
     bool canBeAnySize;
     std::unique_ptr<SpriteNode> sprites;
 
-    BuildingConfiguration(std::string name, int game_type, int game_subtype, int32_t game_custom);
-    BuildingConfiguration();
-    ~BuildingConfiguration(void);
+    BuildingConfiguration(std::string name, int game_type, int game_subtype, int32_t custom) :
+        game_type{ game_type },
+        game_subtype{ game_subtype },
+        game_custom{ custom },
+        width{ 1 },
+        height{ 1 },
+        name{ name },
+        canBeFloating{ false },
+        canBeAnySize{ false },
+        sprites{ nullptr }
+    { }
+    BuildingConfiguration() :
+        BuildingConfiguration{ "", -1, -1, -1 }
+    { }
+    ~BuildingConfiguration()
+    {
+        //cant delete bc.sprites here- screws up BCs copy semantics
+    }
 };
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -11,7 +11,6 @@ SET(PROJECT_SRCS
     Tile.cpp
     TileCondition.cpp
     TileTree.cpp
-    BuildingConfiguration.cpp
     ColorConfiguration.cpp
     ConditionalSprite.cpp
     Config.cpp
@@ -39,8 +38,6 @@ SET(PROJECT_SRCS
     Keybinds.cpp
     EnumToString.cpp
     MaterialMatcher.cpp
-    TiletypeMatcher.cpp
-    ConnectionState.cpp
     main.cpp
 )
 
@@ -52,7 +49,6 @@ SET(PROJECT_HDRS
     commonTypes.h
     ConditionalSprite.h
     Config.h
-    ConnectionState.h
     Constructions.h
     ContentBuildingReader.h
     ContentLoader.h
@@ -77,7 +73,6 @@ SET(PROJECT_HDRS
     TileCondition.h
     TileDirection.h
     TileTree.h
-    TiletypeMatcher.h
     TrackingModes.h
     TreeGrowthConfiguration.h
     UserInput.h

diff --git a/ColorConfiguration.cpp b/ColorConfiguration.cpp
@@ -1,118 +1,106 @@
 #include "ColorConfiguration.h"
 #include "ContentLoader.h"
-#include <set>
+#include <vector>
 
-using namespace std;
 using namespace DFHack;
 using namespace df::enums;
 
-ColorMaterialConfiguration::ColorMaterialConfiguration()
-{
-    color = al_map_rgb(255,255,255);
-    colorSet = 0;
-}
-
-ColorConfiguration::ColorConfiguration()
-{
-    color = al_map_rgb(255,255,255);
-    colorSet = 0;
-}
-
-ColorConfiguration::~ColorConfiguration()
-{
-    colorMaterials.clear();
-}
-
-void parseColorElement(TiXmlElement* elemColor, vector<ColorConfiguration> & configTable, MaterialMatcher<ALLEGRO_COLOR> & materialConfig)
-{
-    const char* colorRedStr = elemColor->Attribute("red");
-    if(colorRedStr == NULL || colorRedStr[0] == 0) {
-        contentError("Invalid or missing color attribute",elemColor);
-        return; //nothing to work with
-    }
-    const char* colorGreenStr = elemColor->Attribute("green");
-    if(colorGreenStr == NULL || colorGreenStr[0] == 0) {
-        contentError("Invalid or missing color attribute",elemColor);
-        return; //nothing to work with
-    }
-    const char* colorBlueStr = elemColor->Attribute("blue");
-    if (colorBlueStr == NULL || colorBlueStr[0] == 0) {
-        contentError("Invalid or missing color attribute", elemColor);
-        return; //nothing to work with
-    }
-    int alpha = 255;
-    const char* colorAlphaStr = elemColor->Attribute("alpha");
-    if (colorAlphaStr != NULL && colorAlphaStr[0] != 0) {
-        alpha = atoi(colorAlphaStr);
-    }
+using std::string;
+using std::vector;
 
-    int red = atoi(colorRedStr);
-    int green = atoi(colorGreenStr);
-    int blue = atoi(colorBlueStr);
-    ALLEGRO_COLOR color = al_map_rgba(red, green, blue, alpha);
-
-
-
-    //parse material elements
-    TiXmlElement* elemMaterial = elemColor->FirstChildElement("material");
-    if(elemMaterial == NULL) {
-        //if none, there's nothing to be done with this color.
-        contentError("Invalid or missing material attribute",elemColor);
-        return;
-    }
-    for( ; elemMaterial; elemMaterial = elemMaterial->NextSiblingElement("material")) {
-        //first try to match with a material token
-        const char* elemToken = elemMaterial->Attribute("token");
-        if (elemToken && elemToken[0])
-        {
-            materialConfig.set_material(color, elemToken);
-            continue;
+namespace {
+    void parseColorElement(TiXmlElement* elemColor, vector<ColorConfiguration> & configTable, MaterialMatcher<ALLEGRO_COLOR> & materialConfig)
+    {
+        const char* colorRedStr = elemColor->Attribute("red");
+        if(colorRedStr == NULL || colorRedStr[0] == 0) {
+            contentError("Invalid or missing color attribute",elemColor);
+            return; //nothing to work with
         }
-        // get material type
-        int elemIndex = lookupMaterialType(elemMaterial->Attribute("value"));
-        if (elemIndex == INVALID_INDEX) {
-            contentError("Invalid or missing value or token attribute",elemMaterial);
-            continue;
+        const char* colorGreenStr = elemColor->Attribute("green");
+        if(colorGreenStr == NULL || colorGreenStr[0] == 0) {
+            contentError("Invalid or missing color attribute",elemColor);
+            return; //nothing to work with
+        }
+        const char* colorBlueStr = elemColor->Attribute("blue");
+        if (colorBlueStr == NULL || colorBlueStr[0] == 0) {
+            contentError("Invalid or missing color attribute", elemColor);
+            return; //nothing to work with
+        }
+        int alpha = 255;
+        const char* colorAlphaStr = elemColor->Attribute("alpha");
+        if (colorAlphaStr != NULL && colorAlphaStr[0] != 0) {
+            alpha = atoi(colorAlphaStr);
         }
 
-        // parse subtype elements
-        TiXmlElement* elemSubtype = elemMaterial->FirstChildElement("subtype");
-        if (elemSubtype == NULL) {
-            // add the configurations
-            if (configTable.size() <= (uint32_t)elemIndex) {
-                //increase size if needed
-                configTable.resize(elemIndex+1);
-            }
-            if(configTable.at(elemIndex).colorSet == false) {
-                configTable.at(elemIndex).color = color;
-                configTable.at(elemIndex).colorSet = true;
-            }
+        int red = atoi(colorRedStr);
+        int green = atoi(colorGreenStr);
+        int blue = atoi(colorBlueStr);
+        ALLEGRO_COLOR color = al_map_rgba(red, green, blue, alpha);
+
+        //parse material elements
+        TiXmlElement* elemMaterial = elemColor->FirstChildElement("material");
+        if(elemMaterial == NULL) {
+            //if none, there's nothing to be done with this color.
+            contentError("Invalid or missing material attribute",elemColor);
             return;
         }
-        for ( ; elemSubtype; elemSubtype = elemSubtype ->NextSiblingElement("subtype")) {
-            // get subtype
-            int subtypeId = lookupMaterialIndex( elemIndex,elemSubtype->Attribute("value"));
-            if (subtypeId == INVALID_INDEX) {
-                contentError("Invalid or missing value attribute",elemSubtype);
+        for( ; elemMaterial; elemMaterial = elemMaterial->NextSiblingElement("material")) {
+            //first try to match with a material token
+            const char* elemToken = elemMaterial->Attribute("token");
+            if (elemToken && elemToken[0])
+            {
+                materialConfig.set_material(color, elemToken);
                 continue;
             }
-
-            // add the configurations
-            if (configTable.size() <= (uint32_t)elemIndex) {
-                //increase size if needed
-                configTable.resize(elemIndex+1);
+            // get material type
+            int elemIndex = lookupMaterialType(elemMaterial->Attribute("value"));
+            if (elemIndex == INVALID_INDEX) {
+                contentError("Invalid or missing value or token attribute",elemMaterial);
+                continue;
             }
 
-            if (configTable.at(elemIndex).colorMaterials.size() <= (uint32_t)subtypeId) {
-                //increase size if needed
-                configTable.at(elemIndex).colorMaterials.resize(subtypeId+1);
+            // parse subtype elements
+            size_t newIndex{ size_t(elemIndex) };
+            TiXmlElement* elemSubtype = elemMaterial->FirstChildElement("subtype");
+            if (elemSubtype == NULL) {
+                // add the configurations
+                if (configTable.size() <= newIndex) {
+                    //increase size if needed
+                    configTable.resize(newIndex+1);
+                }
+                if(configTable.at(newIndex).colorSet == false) {
+                    configTable.at(newIndex).color = color;
+                    configTable.at(newIndex).colorSet = true;
+                }
+                return;
             }
-            if (configTable.at(elemIndex).colorMaterials.at(subtypeId).colorSet == false) {
-                configTable.at(elemIndex).colorMaterials.at(subtypeId).color = color;
-                configTable.at(elemIndex).colorMaterials.at(subtypeId).colorSet = true;
+            for ( ; elemSubtype; elemSubtype = elemSubtype ->NextSiblingElement("subtype")) {
+                // get subtype
+                int subtypeId = lookupMaterialIndex( newIndex,elemSubtype->Attribute("value"));
+                if (subtypeId == INVALID_INDEX) {
+                    contentError("Invalid or missing value attribute",elemSubtype);
+                    continue;
+                }
+
+                size_t newSubtypeIndex{ size_t(subtypeId) };
+                // add the configurations
+                if (configTable.size() <= newSubtypeIndex) {
+                    //increase size if needed
+                    configTable.resize(newIndex+1);
+                }
+
+                if (configTable.at(newIndex).colorMaterials.size() <= newSubtypeIndex) {
+                    //increase size if needed
+                    configTable.at(newIndex).colorMaterials.resize(newSubtypeIndex+1);
+                }
+                if (configTable.at(newIndex).colorMaterials.at(newSubtypeIndex).colorSet == false) {
+                    configTable.at(newIndex).colorMaterials.at(newSubtypeIndex).color = color;
+                    configTable.at(newIndex).colorMaterials.at(newSubtypeIndex).colorSet = true;
+                }
             }
         }
     }
+
 }
 
 bool addSingleColorConfig( TiXmlElement* elemRoot)

diff --git a/ColorConfiguration.h b/ColorConfiguration.h
@@ -1,15 +1,16 @@
 #pragma once
 #include "common.h"
 #include "tinyxml.h"
-#include <map>
 
 class ColorMaterialConfiguration
 {
 public:
     ALLEGRO_COLOR color;
     bool colorSet;
-    ColorMaterialConfiguration();
-} ;
+    ColorMaterialConfiguration() :
+        color{ al_map_rgb(255,255,255) }, colorSet{ false }
+    { };
+};
 
 
 class ColorConfiguration
@@ -18,10 +19,11 @@ class ColorConfiguration
     std::vector<ColorMaterialConfiguration> colorMaterials;
     ALLEGRO_COLOR color;
     bool colorSet;
-    ColorConfiguration();
-    ~ColorConfiguration();
-} ;
+    ColorConfiguration() :
+        color(al_map_rgb(255, 255, 255)), colorSet{ false }
+    { };
 
-bool addSingleColorConfig( TiXmlElement* elemRoot);
+    ~ColorConfiguration() = default;
+};
 
-void flushColorConfig(std::vector<ColorConfiguration>& config);
+bool addSingleColorConfig( TiXmlElement* elemRoot);