Core/Vmaps: Minor cleanup in vmap4assembler - use unique_ptr to manage file handles

1 files changed, 76 insertions, 105 deletions

diff --git a/src/tools/vmap4_assembler/TileAssembler.cpp b/src/tools/vmap4_assembler/TileAssembler.cpp
index 888d483466f..5f61fb31de8 100644
--- a/src/tools/vmap4_assembler/TileAssembler.cpp
+++ b/src/tools/vmap4_assembler/TileAssembler.cpp
@@ -18,12 +18,11 @@
 #include "TileAssembler.h"
 #include "BoundingIntervalHierarchy.h"
 #include "MapTree.h"
+#include "Memory.h"
 #include "StringFormat.h"
 #include "VMapDefinitions.h"
-#include <boost/filesystem.hpp>
-#include <iomanip>
+#include <boost/filesystem/operations.hpp>
 #include <set>
-#include <sstream>
 
 template<> struct BoundsTrait<VMAP::ModelSpawn*>
 {
@@ -41,16 +40,12 @@ namespace VMAP
 
     //=================================================================
 
-    TileAssembler::TileAssembler(const std::string& pSrcDirName, const std::string& pDestDirName)
-        : iDestDir(pDestDirName), iSrcDir(pSrcDirName)
+    TileAssembler::TileAssembler(std::string pSrcDirName, std::string pDestDirName)
+        : iDestDir(std::move(pDestDirName)), iSrcDir(std::move(pSrcDirName))
     {
         boost::filesystem::create_directories(iDestDir);
     }
 
-    TileAssembler::~TileAssembler()
-    {
-    }
-
     bool TileAssembler::convertWorld2()
     {
         bool success = readMapSpawns();
@@ -98,66 +93,63 @@ namespace VMAP
             }
             catch (std::exception& e)
             {
-                printf("Exception ""%s"" when calling pTree.build", e.what());
+                printf(R"(Exception "%s" when calling pTree.build)", e.what());
                 return false;
             }
 
-            // ===> possibly move this code to StaticMapTree class
 
             // write map tree file
-            std::stringstream mapfilename;
-            mapfilename << iDestDir << '/' << std::setfill('0') << std::setw(4) << data.MapId << ".vmtree";
-            FILE* mapfile = fopen(mapfilename.str().c_str(), "wb");
+            std::string mapfilename = Trinity::StringFormat("{}/{:04}.vmtree", iDestDir, data.MapId);
+            auto mapfile = Trinity::make_unique_ptr_with_deleter(fopen(mapfilename.c_str(), "wb"), &::fclose);
             if (!mapfile)
             {
                 success = false;
-                printf("Cannot open %s\n", mapfilename.str().c_str());
+                printf("Cannot open %s\n", mapfilename.c_str());
                 break;
             }
 
             //general info
-            if (success && fwrite(VMAP_MAGIC, 1, 8, mapfile) != 8) success = false;
+            if (success && fwrite(VMAP_MAGIC, 1, 8, mapfile.get()) != 8) success = false;
             // Nodes
-            if (success && fwrite("NODE", 4, 1, mapfile) != 1) success = false;
-            if (success) success = pTree.writeToFile(mapfile);
+            if (success && fwrite("NODE", 4, 1, mapfile.get()) != 1) success = false;
+            if (success) success = pTree.writeToFile(mapfile.get());
 
             // spawn id to index map
             uint32 mapSpawnsSize = mapSpawns.size();
-            if (success && fwrite("SIDX", 4, 1, mapfile) != 1) success = false;
-            if (success && fwrite(&mapSpawnsSize, sizeof(uint32), 1, mapfile) != 1) success = false;
+            if (success && fwrite("SIDX", 4, 1, mapfile.get()) != 1) success = false;
+            if (success && fwrite(&mapSpawnsSize, sizeof(uint32), 1, mapfile.get()) != 1) success = false;
             for (uint32 i = 0; i < mapSpawnsSize; ++i)
             {
-                if (success && fwrite(&mapSpawns[i]->ID, sizeof(uint32), 1, mapfile) != 1) success = false;
+                if (success && fwrite(&mapSpawns[i]->ID, sizeof(uint32), 1, mapfile.get()) != 1) success = false;
             }
 
-            fclose(mapfile);
+            mapfile = nullptr;
 
             // <====
 
             // write map tile files, similar to ADT files, only with extra BIH tree node info
-            for (auto tileItr = data.TileEntries.begin(); tileItr != data.TileEntries.end(); ++tileItr)
+            for (auto const& [tileId, spawns] : data.TileEntries)
             {
                 uint32 x, y;
-                StaticMapTree::unpackTileID(tileItr->first, x, y);
+                StaticMapTree::unpackTileID(tileId, x, y);
                 std::string tileFileName = Trinity::StringFormat("{}/{:04}_{:02}_{:02}.vmtile", iDestDir, data.MapId, y, x);
-                if (FILE* tileFile = fopen(tileFileName.c_str(), "wb"))
+                auto tileFile = Trinity::make_unique_ptr_with_deleter(fopen(tileFileName.c_str(), "wb"), &::fclose);
+                if (tileFile)
                 {
-                    std::set<uint32> const& parentTileEntries = data.ParentTileEntries[tileItr->first];
+                    std::set<uint32> const& parentTileEntries = data.ParentTileEntries[tileId];
 
-                    uint32 nSpawns = tileItr->second.size() + parentTileEntries.size();
+                    uint32 nSpawns = spawns.size() + parentTileEntries.size();
 
                     // file header
-                    if (success && fwrite(VMAP_MAGIC, 1, 8, tileFile) != 8) success = false;
+                    if (success && fwrite(VMAP_MAGIC, 1, 8, tileFile.get()) != 8) success = false;
                     // write number of tile spawns
-                    if (success && fwrite(&nSpawns, sizeof(uint32), 1, tileFile) != 1) success = false;
+                    if (success && fwrite(&nSpawns, sizeof(uint32), 1, tileFile.get()) != 1) success = false;
                     // write tile spawns
-                    for (auto spawnItr = tileItr->second.begin(); spawnItr != tileItr->second.end() && success; ++spawnItr)
-                        success = ModelSpawn::writeToFile(tileFile, data.UniqueEntries[*spawnItr]);
+                    for (auto spawnItr = spawns.begin(); spawnItr != spawns.end() && success; ++spawnItr)
+                        success = ModelSpawn::writeToFile(tileFile.get(), data.UniqueEntries[*spawnItr]);
 
                     for (auto spawnItr = parentTileEntries.begin(); spawnItr != parentTileEntries.end() && success; ++spawnItr)
-                        success = ModelSpawn::writeToFile(tileFile, data.UniqueEntries[*spawnItr]);
-
-                    fclose(tileFile);
+                        success = ModelSpawn::writeToFile(tileFile.get(), data.UniqueEntries[*spawnItr]);
                 }
             }
         }
@@ -165,13 +157,13 @@ namespace VMAP
         // add an object models, listed in temp_gameobject_models file
         exportGameobjectModels();
         // export objects
-        std::cout << "\nConverting Model Files" << std::endl;
+        printf("\nConverting Model Files\n");
         for (std::string const& spawnedModelFile : spawnedModelFiles)
         {
-            std::cout << "Converting " << spawnedModelFile << std::endl;
+            printf("Converting %s\n", spawnedModelFile.c_str());
             if (!convertRawFile(spawnedModelFile))
             {
-                std::cout << "error converting " << spawnedModelFile << std::endl;
+                printf("error converting %s\n", spawnedModelFile.c_str());
                 success = false;
                 break;
             }
@@ -183,7 +175,7 @@ namespace VMAP
     bool TileAssembler::readMapSpawns()
     {
         std::string fname = iSrcDir + "/dir_bin";
-        FILE* dirf = fopen(fname.c_str(), "rb");
+        auto dirf = Trinity::make_unique_ptr_with_deleter(fopen(fname.c_str(), "rb"), &::fclose);
         if (!dirf)
         {
             printf("Could not read dir_bin file!\n");
@@ -192,25 +184,25 @@ namespace VMAP
         printf("Read coordinate mapping...\n");
         uint32 mapID, check;
         std::map<uint32, MapSpawns> data;
-        while (!feof(dirf))
+        while (!feof(dirf.get()))
         {
             // read mapID, Flags, NameSet, UniqueId, Pos, Rot, Scale, Bound_lo, Bound_hi, name
-            check = fread(&mapID, sizeof(uint32), 1, dirf);
+            check = fread(&mapID, sizeof(uint32), 1, dirf.get());
             if (check == 0) // EoF...
                 break;
 
             ModelSpawn spawn;
-            if (!ModelSpawn::readFromFile(dirf, spawn))
+            if (!ModelSpawn::readFromFile(dirf.get(), spawn))
                 break;
 
-            auto map_iter = data.emplace(std::piecewise_construct, std::forward_as_tuple(mapID), std::forward_as_tuple());
-            if (map_iter.second)
+            auto [itr, isNew] = data.try_emplace(mapID);
+            if (isNew)
             {
-                map_iter.first->second.MapId = mapID;
+                itr->second.MapId = mapID;
                 printf("spawning Map %u\n", mapID);
             }
 
-            map_iter.first->second.UniqueEntries.emplace(spawn.ID, spawn);
+            itr->second.UniqueEntries.emplace(spawn.ID, spawn);
         }
 
         mapData.resize(data.size());
@@ -218,8 +210,7 @@ namespace VMAP
         for (auto src = data.begin(); src != data.end(); ++src, ++dst)
             *dst = std::move(src->second);
 
-        bool success = (ferror(dirf) == 0);
-        fclose(dirf);
+        bool success = (ferror(dirf.get()) == 0);
         return success;
     }
 
@@ -287,8 +278,8 @@ namespace VMAP
             {
                 GroupModel_Raw& raw_group = raw_model.groupsArray[g];
                 groupsArray.push_back(GroupModel(raw_group.mogpflags, raw_group.GroupWMOID, raw_group.bounds));
-                groupsArray.back().setMeshData(raw_group.vertexArray, raw_group.triangles);
-                groupsArray.back().setLiquidData(raw_group.liquid);
+                groupsArray.back().setMeshData(std::move(raw_group.vertexArray), std::move(raw_group.triangles));
+                groupsArray.back().setLiquidData(raw_group.liquid.release());
             }
 
             model.setGroupModels(groupsArray);
@@ -301,39 +292,33 @@ namespace VMAP
 
     void TileAssembler::exportGameobjectModels()
     {
-        FILE* model_list = fopen((iSrcDir + "/" + "temp_gameobject_models").c_str(), "rb");
+        auto model_list = Trinity::make_unique_ptr_with_deleter(fopen((iSrcDir + "/" + "temp_gameobject_models").c_str(), "rb"), &::fclose);
         if (!model_list)
             return;
 
         char ident[8];
-        if (fread(ident, 1, 8, model_list) != 8 || memcmp(ident, VMAP::RAW_VMAP_MAGIC, 8) != 0)
-        {
-            fclose(model_list);
+        if (fread(ident, 1, 8, model_list.get()) != 8 || memcmp(ident, VMAP::RAW_VMAP_MAGIC, 8) != 0)
             return;
-        }
 
-        FILE* model_list_copy = fopen((iDestDir + "/" + GAMEOBJECT_MODELS).c_str(), "wb");
+        auto model_list_copy = Trinity::make_unique_ptr_with_deleter(fopen((iDestDir + "/" + GAMEOBJECT_MODELS).c_str(), "wb"), &::fclose);
         if (!model_list_copy)
-        {
-            fclose(model_list);
             return;
-        }
 
-        fwrite(VMAP::VMAP_MAGIC, 1, 8, model_list_copy);
+        fwrite(VMAP::VMAP_MAGIC, 1, 8, model_list_copy.get());
 
         uint32 name_length, displayId;
         char buff[500];
         while (true)
         {
-            if (fread(&displayId, sizeof(uint32), 1, model_list) != 1)
-                if (feof(model_list))   // EOF flag is only set after failed reading attempt
+            if (fread(&displayId, sizeof(uint32), 1, model_list.get()) != 1)
+                if (feof(model_list.get()))   // EOF flag is only set after failed reading attempt
                     break;
 
-            if (fread(&name_length, sizeof(uint32), 1, model_list) != 1
+            if (fread(&name_length, sizeof(uint32), 1, model_list.get()) != 1
                 || name_length >= sizeof(buff)
-                || fread(&buff, sizeof(char), name_length, model_list) != name_length)
+                || fread(&buff, sizeof(char), name_length, model_list.get()) != name_length)
             {
-                std::cout << "\nFile 'temp_gameobject_models' seems to be corrupted" << std::endl;
+                printf("\nFile 'temp_gameobject_models' seems to be corrupted\n");
                 break;
             }
 
@@ -351,34 +336,29 @@ namespace VMAP
 
             if (bounds.isEmpty())
             {
-                std::cout << "\nModel " << std::string(buff, name_length) << " has empty bounding box" << std::endl;
+                printf("\nModel %s has empty bounding box\n", model_name.c_str());
                 continue;
             }
 
             if (!bounds.isFinite())
             {
-                std::cout << "\nModel " << std::string(buff, name_length) << " has invalid bounding box" << std::endl;
+                printf("\nModel %s has invalid bounding box\n", model_name.c_str());
                 continue;
             }
 
-            fwrite(&displayId, sizeof(uint32), 1, model_list_copy);
-            fwrite(&name_length, sizeof(uint32), 1, model_list_copy);
-            fwrite(&buff, sizeof(char), name_length, model_list_copy);
-            fwrite(&bounds.low(), sizeof(G3D::Vector3), 1, model_list_copy);
-            fwrite(&bounds.high(), sizeof(G3D::Vector3), 1, model_list_copy);
+            fwrite(&displayId, sizeof(uint32), 1, model_list_copy.get());
+            fwrite(&name_length, sizeof(uint32), 1, model_list_copy.get());
+            fwrite(&buff, sizeof(char), name_length, model_list_copy.get());
+            fwrite(&bounds.low(), sizeof(G3D::Vector3), 1, model_list_copy.get());
+            fwrite(&bounds.high(), sizeof(G3D::Vector3), 1, model_list_copy.get());
         }
-
-        fclose(model_list);
-        fclose(model_list_copy);
     }
 
 // temporary use defines to simplify read/check code (close file and return at fail)
-#define READ_OR_RETURN(V, S) if (fread((V), (S), 1, rf) != 1) { \
-                                fclose(rf); printf("%s readfail, op = %s\n", __FUNCTION__, #V); return(false); }
-#define READ_OR_RETURN_WITH_DELETE(V, S) if (fread((V), (S), 1, rf) != 1) { \
-                                fclose(rf); printf("%s readfail, op = %s\n", __FUNCTION__, #V); delete[] V; return(false); };
-#define CMP_OR_RETURN(V, S)  if (strcmp((V), (S)) != 0)        { \
-                                fclose(rf); printf("%s cmpfail, %s!=%s\n", __FUNCTION__, V, S);return(false); }
+#define READ_OR_RETURN(V, S) if (fread((V), (S), 1, rf) != 1) do { \
+                                printf("%s readfail, op = %s\n", __FUNCTION__, #V); return false; } while(false)
+#define CMP_OR_RETURN(V, S)  if (strcmp((V), (S)) != 0)       do { \
+                                printf("%s cmpfail, %s!=%s\n", __FUNCTION__, V, S);return false; } while(false)
 
     bool GroupModel_Raw::Read(FILE* rf)
     {
@@ -403,7 +383,7 @@ namespace VMAP
         CMP_OR_RETURN(blockId, "GRP ");
         READ_OR_RETURN(&blocksize, sizeof(int));
         READ_OR_RETURN(&branches, sizeof(uint32));
-        for (uint32 b=0; b<branches; ++b)
+        for (uint32 b = 0; b < branches; ++b)
         {
             uint32 indexes;
             // indexes for each branch (not used jet)
@@ -416,15 +396,13 @@ namespace VMAP
         READ_OR_RETURN(&blocksize, sizeof(int));
         uint32 nindexes;
         READ_OR_RETURN(&nindexes, sizeof(uint32));
-        if (nindexes >0)
+        if (nindexes > 0)
         {
-            uint32 *indexarray = new uint32[nindexes];
-            READ_OR_RETURN_WITH_DELETE(indexarray, nindexes*sizeof(uint32));
+            std::unique_ptr<uint32[]> indexarray = std::make_unique<uint32[]>(nindexes);
+            READ_OR_RETURN(indexarray.get(), nindexes * sizeof(uint32));
             triangles.reserve(nindexes / 3);
-            for (uint32 i=0; i<nindexes; i+=3)
+            for (uint32 i = 0; i < nindexes; i += 3)
                 triangles.push_back({ .idx0 = indexarray[i], .idx1 = indexarray[i + 1], .idx2 = indexarray[i + 2] });
-
-            delete[] indexarray;
         }
 
         // ---- vectors
@@ -434,14 +412,12 @@ namespace VMAP
         uint32 nvectors;
         READ_OR_RETURN(&nvectors, sizeof(uint32));
 
-        if (nvectors >0)
+        if (nvectors > 0)
         {
-            float *vectorarray = new float[nvectors*3];
-            READ_OR_RETURN_WITH_DELETE(vectorarray, nvectors*sizeof(float)*3);
-            for (uint32 i=0; i<nvectors; ++i)
-                vertexArray.push_back(G3D::Vector3(vectorarray + 3*i) );
-
-            delete[] vectorarray;
+            std::unique_ptr<float[]> vectorarray = std::make_unique<float[]>(nvectors * 3);
+            READ_OR_RETURN(vectorarray.get(), nvectors * sizeof(float) * 3);
+            for (uint32 i = 0; i < nvectors; ++i)
+                vertexArray.push_back(G3D::Vector3(&vectorarray[3 * i]));
         }
         // ----- liquid
         liquid = nullptr;
@@ -456,7 +432,7 @@ namespace VMAP
             {
                 WMOLiquidHeader hlq;
                 READ_OR_RETURN(&hlq, sizeof(WMOLiquidHeader));
-                liquid = new WmoLiquid(hlq.xtiles, hlq.ytiles, G3D::Vector3(hlq.pos_x, hlq.pos_y, hlq.pos_z), liquidType);
+                liquid.reset(new WmoLiquid(hlq.xtiles, hlq.ytiles, G3D::Vector3(hlq.pos_x, hlq.pos_y, hlq.pos_z), liquidType));
                 uint32 size = hlq.xverts * hlq.yverts;
                 READ_OR_RETURN(liquid->GetHeightStorage(), size * sizeof(float));
                 size = hlq.xtiles * hlq.ytiles;
@@ -464,7 +440,7 @@ namespace VMAP
             }
             else
             {
-                liquid = new WmoLiquid(0, 0, G3D::Vector3::zero(), liquidType);
+                liquid.reset(new WmoLiquid(0, 0, G3D::Vector3::zero(), liquidType));
                 liquid->GetHeightStorage()[0] = bounds.high().z;
             }
         }
@@ -472,20 +448,17 @@ namespace VMAP
         return true;
     }
 
-    GroupModel_Raw::~GroupModel_Raw()
-    {
-        delete liquid;
-    }
-
     bool WorldModel_Raw::Read(const char * path)
     {
-        FILE* rf = fopen(path, "rb");
-        if (!rf)
+        auto file = Trinity::make_unique_ptr_with_deleter(fopen(path, "rb"), &::fclose);
+        if (!file)
         {
             printf("ERROR: Can't open raw model file: %s\n", path);
             return false;
         }
 
+        FILE* rf = file.get();
+
         char ident[9];
         ident[8] = '\0';
 
@@ -506,8 +479,6 @@ namespace VMAP
         for (uint32 g = 0; g < groups && succeed; ++g)
             succeed = groupsArray[g].Read(rf);
 
-        if (succeed) /// rf will be freed inside Read if the function had any errors.
-            fclose(rf);
         return succeed;
     }