Close #13637: Refactor sprite compiler for filesystem efficiency

- sprite building would save a file with just the sprite file header
   and then immediately open it again at the beginning of compilation
 - sprite file generation is now entirely in memory until the final
   output file is saved on success
 - added validation of no file activity in the failed build test case;
   failed builds will not generate a file or edit an existing one

src/openrct2/CmdlineSprite.cpp

@@ -589,10 +589,11 @@ int32_t cmdline_for_sprite(const char** argv, int32_t argc)
 
         bool silent = (argc >= 4 && strcmp(argv[3], "silent") == 0);
 
-        SpriteFile baseFile;
-        baseFile.Header.num_entries = 0;
-        baseFile.Header.total_size = 0;
-        baseFile.Save(spriteFilePath);
+        // keep sprite file entirely in memory until ready to write out a complete,
+        // correct file
+        SpriteFile spriteFile;
+        spriteFile.Header.num_entries = 0;
+        spriteFile.Header.total_size = 0;
 
         fprintf(stdout, "Building: %s\n", spriteFilePath);
 
@@ -632,25 +633,18 @@ int32_t cmdline_for_sprite(const char** argv, int32_t argc)
                 return -1;
             }
 
-            auto spriteFile = SpriteFile::Open(spriteFilePath);
-            if (!spriteFile.has_value())
-            {
-                fprintf(stderr, "Unable to open sprite file: %s\nCanceling\n", spriteFilePath);
-                return -1;
-            }
-
-            spriteFile->AddImage(importResult.value());
-
-            if (!spriteFile->Save(spriteFilePath))
-            {
-                fprintf(stderr, "Could not save sprite file: %s\nCanceling\n", imagePath.c_str());
-                return -1;
-            }
+            spriteFile.AddImage(importResult.value());
 
             if (!silent)
                 fprintf(stdout, "Added: %s\n", imagePath.c_str());
         }
 
+        if (!spriteFile.Save(spriteFilePath))
+        {
+            log_error("Could not save sprite file, cancelling.");
+            return -1;
+        }
+
         free(directoryPath);
 
         fprintf(stdout, "Finished\n");

test/tests/CLITests.cpp

@@ -0,0 +1,98 @@
+
+#include "TestData.h"
+
+#include <algorithm>
+#include <fstream>
+#include <gtest/gtest.h>
+#include <iterator>
+#include <openrct2/CmdlineSprite.h>
+#include <openrct2/core/Path.hpp>
+
+class CommandLineTests : public testing::Test
+{
+public:
+    static std::string SpriteTestDataPath()
+    {
+        return Path::Combine(TestData::GetBasePath(), "sprites");
+    }
+
+    static std::string ManifestFilePath()
+    {
+        return Path::Combine(SpriteTestDataPath(), "manifest.json");
+    }
+
+    static std::string BadManifestFilePath()
+    {
+        return Path::Combine(SpriteTestDataPath(), "badManifest.json");
+    }
+
+    static std::string ExampleSpriteFilePath()
+    {
+        return Path::Combine(SpriteTestDataPath(), "example.dat");
+    }
+
+    static std::string BuildOutputfilePath()
+    {
+        return Path::Combine(SpriteTestDataPath(), "result.dat");
+    }
+
+    static bool CompareSpriteFiles(std::string original, std::string generated)
+    {
+        std::ifstream originalFile(original, std::ios::binary | std::ifstream::in);
+        std::ifstream generatedFile(generated, std::ios::binary | std::ifstream::in);
+        if (!(originalFile.is_open() && generatedFile.is_open()))
+        {
+            return false;
+        }
+        if (originalFile.tellg() != generatedFile.tellg())
+        {
+            return false;
+        }
+        return std::equal(
+            std::istreambuf_iterator<char>(originalFile.rdbuf()), std::istreambuf_iterator<char>(),
+            std::istreambuf_iterator<char>(generatedFile.rdbuf()));
+    }
+};
+
+TEST_F(CommandLineTests, cmdline_cmdline_for_sprite_details)
+{
+    std::string exampleFilePath = ExampleSpriteFilePath();
+    const char* detailsCmd[3] = { "details", exampleFilePath.c_str() };
+
+    int32_t result = cmdline_for_sprite(detailsCmd, 2);
+    // need to come up with some way to extract stdout/stderr stream if we want to
+    // fully test this module
+    ASSERT_EQ(result, 1);
+}
+
+TEST_F(CommandLineTests, cmdline_cmdline_for_sprite_build)
+{
+    std::string manifestFilePath = ManifestFilePath();
+    std::string outputfilePath = BuildOutputfilePath();
+    const char* detailsCmd[3] = { "build", outputfilePath.c_str(), manifestFilePath.c_str() };
+
+    int32_t result = cmdline_for_sprite(detailsCmd, 3);
+    ASSERT_EQ(result, 1);
+    // compare the resulting output file and assert its identical to expected
+    ASSERT_TRUE(CompareSpriteFiles(ExampleSpriteFilePath(), outputfilePath));
+}
+
+TEST_F(CommandLineTests, cmdline_cmdline_for_sprite_failed_build)
+{
+    // run on correct manifest file
+    std::string manifestFilePath = ManifestFilePath();
+    std::string outputfilePath = BuildOutputfilePath();
+    const char* detailsCmd[3] = { "build", outputfilePath.c_str(), manifestFilePath.c_str() };
+    int32_t result = cmdline_for_sprite(detailsCmd, 3);
+    ASSERT_EQ(result, 1);
+    ASSERT_TRUE(CompareSpriteFiles(ExampleSpriteFilePath(), outputfilePath));
+
+    // now use bad manifest and make sure output file is not edited
+    std::string badManifestFilePath = BadManifestFilePath();
+    detailsCmd[2] = badManifestFilePath.c_str();
+    result = cmdline_for_sprite(detailsCmd, 3);
+    // check the command failed
+    ASSERT_EQ(result, -1);
+    // validate the target file was unchanged
+    ASSERT_TRUE(CompareSpriteFiles(ExampleSpriteFilePath(), outputfilePath));
+}

test/tests/testdata/sprites/badManifest.json

@@ -0,0 +1,8 @@
+[
+    {
+        "path": "../images/logo.png"
+    },
+    {
+        "path": "../images/FAKEFILE.png"
+    }
+]

test/tests/testdata/sprites/manifest.json

@@ -0,0 +1,5 @@
+[
+    {
+        "path": "../images/logo.png"
+    }
+]

test/tests/tests.vcxproj

@@ -57,6 +57,7 @@
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="CircularBuffer.cpp" />
+    <ClCompile Include="CLITests.cpp" />
     <ClCompile Include="CryptTests.cpp" />
     <ClCompile Include="Endianness.cpp" />
     <ClCompile Include="FormattingTests.cpp" />
@@ -78,5 +79,10 @@
     <ClCompile Include="StringTest.cpp" />
     <ClCompile Include="TileElements.cpp" />
   </ItemGroup>
+  <ItemGroup>
+    <None Include="testdata\sprites\badManifest.json" />
+    <None Include="testdata\sprites\example.dat" />
+    <None Include="testdata\sprites\manifest.json" />
+  </ItemGroup>
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
 </Project>