Merge branch 'correct-sound' into 'master'

Add correct sound path in resource helpers

See merge request OpenMW/openmw!1381

apps/openmw/mwsound/openal_output.cpp

@@ -13,6 +13,7 @@
 
 #include <components/debug/debuglog.hpp>
 #include <components/misc/constants.hpp>
+#include <components/misc/resourcehelpers.hpp>
 #include <components/vfs/manager.hpp>
 
 #include "openal_output.hpp"
@@ -954,17 +955,7 @@ std::pair<Sound_Handle,size_t> OpenAL_Output::loadSound(const std::string &fname
     try
     {
         DecoderPtr decoder = mManager.getDecoder();
-        // Workaround: Bethesda at some point converted some of the files to mp3, but the references were kept as .wav.
+        decoder->open(Misc::ResourceHelpers::correctSoundPath(fname, decoder->mResourceMgr));
-        if(decoder->mResourceMgr->exists(fname))
-            decoder->open(fname);
-        else
-        {
-            std::string file = fname;
-            std::string::size_type pos = file.rfind('.');
-            if(pos != std::string::npos)
-                file = file.substr(0, pos)+".mp3";
-            decoder->open(file);
-        }
 
         ChannelConfig chans;
         SampleType type;

apps/openmw/mwsound/soundmanagerimp.cpp

@@ -6,6 +6,7 @@
 
 #include <osg/Matrixf>
 
+#include <components/misc/resourcehelpers.hpp>
 #include <components/misc/rng.hpp>
 #include <components/debug/debuglog.hpp>
 #include <components/vfs/manager.hpp>
@@ -145,19 +146,7 @@ namespace MWSound
         try
         {
             DecoderPtr decoder = getDecoder();
-
+            decoder->open(Misc::ResourceHelpers::correctSoundPath(voicefile, decoder->mResourceMgr));
-            // Workaround: Bethesda at some point converted some of the files to mp3, but the references were kept as .wav.
-            if(mVFS->exists(voicefile))
-                decoder->open(voicefile);
-            else
-            {
-                std::string file = voicefile;
-                std::string::size_type pos = file.rfind('.');
-                if(pos != std::string::npos)
-                    file = file.substr(0, pos)+".mp3";
-                decoder->open(file);
-            }
-
             return decoder;
         }
         catch(std::exception &e)

apps/openmw_test_suite/CMakeLists.txt

@@ -26,6 +26,7 @@ if (GTEST_FOUND AND GMOCK_FOUND)
 
         misc/test_stringops.cpp
         misc/test_endianness.cpp
+        misc/test_resourcehelpers.cpp
         misc/progressreporter.cpp
         misc/compression.cpp
 

apps/openmw_test_suite/misc/test_resourcehelpers.cpp

@@ -0,0 +1,80 @@
+#include <gtest/gtest.h>
+#include "components/misc/resourcehelpers.hpp"
+#include "../lua/testing_util.hpp"
+
+namespace
+{
+    using namespace Misc::ResourceHelpers;
+    TEST(CorrectSoundPath, wav_files_not_overridden_with_mp3_in_vfs_are_not_corrected)
+    {
+        std::unique_ptr<VFS::Manager> mVFS = createTestVFS({
+            {"sound/bar.wav", nullptr}
+        });
+        EXPECT_EQ(correctSoundPath("sound/bar.wav", mVFS.get()), "sound/bar.wav");
+    }
+
+    TEST(CorrectSoundPath, wav_files_overridden_with_mp3_in_vfs_are_corrected)
+    {
+        std::unique_ptr<VFS::Manager> mVFS = createTestVFS({
+            {"sound/foo.mp3", nullptr}
+        });
+        EXPECT_EQ(correctSoundPath("sound/foo.wav", mVFS.get()), "sound/foo.mp3");
+    }
+
+    TEST(CorrectSoundPath, corrected_path_does_not_check_existence_in_vfs)
+    {
+        std::unique_ptr<VFS::Manager> mVFS = createTestVFS({
+        });
+        EXPECT_EQ(correctSoundPath("sound/foo.wav", mVFS.get()), "sound/foo.mp3");
+    }
+
+    TEST(CorrectSoundPath, correct_path_normalize_paths)
+    {
+        std::unique_ptr<VFS::Manager> mVFS = createTestVFS({
+        });
+        EXPECT_EQ(correctSoundPath("sound\\foo.wav", mVFS.get()), "sound/foo.mp3");
+        EXPECT_EQ(correctSoundPath("SOUND\\foo.WAV", mVFS.get()), "SOUND/foo.mp3");
+    }
+
+    namespace
+    {
+        std::string checkChangeExtensionToDds(std::string path)
+        {
+            changeExtensionToDds(path);
+            return path;
+        }
+    }
+
+    TEST(ChangeExtensionToDds, original_extension_with_same_size_as_dds)
+    {
+        EXPECT_EQ(checkChangeExtensionToDds("texture/bar.tga"), "texture/bar.dds");
+    }
+
+    TEST(ChangeExtensionToDds, original_extension_greater_than_dds)
+    {
+        EXPECT_EQ(checkChangeExtensionToDds("texture/bar.jpeg"), "texture/bar.dds");
+    }
+
+    TEST(ChangeExtensionToDds, original_extension_smaller_than_dds)
+    {
+        EXPECT_EQ(checkChangeExtensionToDds("texture/bar.xx"), "texture/bar.dds");
+    }
+
+    TEST(ChangeExtensionToDds, does_not_change_dds_extension)
+    {
+        std::string path = "texture/bar.dds";
+        EXPECT_FALSE(changeExtensionToDds(path));
+    }
+
+    TEST(ChangeExtensionToDds, does_not_change_when_no_extension)
+    {
+        std::string path = "texture/bar";
+        EXPECT_FALSE(changeExtensionToDds(path));
+    }
+
+    TEST(ChangeExtensionToDds, change_when_there_is_an_extension)
+    {
+        std::string path = "texture/bar.jpeg";
+        EXPECT_TRUE(changeExtensionToDds(path));
+    }
+}

components/misc/resourcehelpers.cpp

@@ -30,17 +30,22 @@ namespace
 
 }
 
-bool Misc::ResourceHelpers::changeExtensionToDds(std::string &path)
+bool changeExtension(std::string &path, std::string_view ext)
 {
     std::string::size_type pos = path.rfind('.');
-    if(pos != std::string::npos && path.compare(pos, path.length() - pos, ".dds") != 0)
+    if(pos != std::string::npos && path.compare(pos, path.length() - pos, ext) != 0)
     {
-        path.replace(pos, path.length(), ".dds");
+        path.replace(pos, path.length(), ext);
         return true;
     }
     return false;
 }
 
+bool Misc::ResourceHelpers::changeExtensionToDds(std::string &path)
+{
+    return changeExtension(path, ".dds");
+}
+
 std::string Misc::ResourceHelpers::correctResourcePath(const std::string &topLevelDirectory, const std::string &resPath, const VFS::Manager* vfs)
 {
     /* Bethesda at some point converted all their BSA
@@ -140,6 +145,17 @@ std::string Misc::ResourceHelpers::correctActorModelPath(const std::string &resP
     return mdlname;
 }
 
+std::string Misc::ResourceHelpers::correctSoundPath(const std::string& resPath, const VFS::Manager* vfs)
+{
+    std::string sound = resPath;
+    // Workaround: Bethesda at some point converted some of the files to mp3, but the references were kept as .wav.
+    if (!vfs->exists(sound))
+        changeExtension(sound, ".mp3");
+
+    return vfs->normalizeFilename(sound);
+
+}
+
 bool Misc::ResourceHelpers::isHiddenMarker(std::string_view id)
 {
     return Misc::StringUtils::ciEqual(id, "prisonmarker") || Misc::StringUtils::ciEqual(id, "divinemarker") || Misc::StringUtils::ciEqual(id, "templemarker") || Misc::StringUtils::ciEqual(id, "northmarker");

components/misc/resourcehelpers.hpp

@@ -25,6 +25,8 @@ namespace Misc
         /// Use "xfoo.nif" instead of "foo.nif" if available
         std::string correctActorModelPath(const std::string &resPath, const VFS::Manager* vfs);
 
+        std::string correctSoundPath(const std::string& resPath, const VFS::Manager* vfs);
+
         /// marker objects that have a hardcoded function in the game logic, should be hidden from the player
         bool isHiddenMarker(std::string_view id);
     }