Use normalized path to implement correctResourcePath

apps/components_tests/misc/testresourcehelpers.cpp

@@ -37,7 +37,7 @@ namespace Misc::ResourceHelpers
         TEST(MiscResourceHelpersCorrectResourcePath, shouldFallbackToGivenExtentionIfDoesNotExistInVfs)
         {
             const std::unique_ptr<const VFS::Manager> vfs = TestingOpenMW::createTestVFS({});
-            EXPECT_EQ(correctResourcePath({ { "sound" } }, "sound/foo.wav", vfs.get(), ".mp3"), "sound/foo.mp3");
+            EXPECT_EQ(correctResourcePath({ { "sound" } }, "sound/foo.wav", vfs.get(), "mp3"), "sound/foo.mp3");
         }
 
         TEST(MiscResourceHelpersCorrectResourcePath, shouldFallbackToGivenExtentionIfBothExistInVfs)
@@ -48,7 +48,7 @@ namespace Misc::ResourceHelpers
                 { wav, nullptr },
                 { mp3, nullptr },
             });
-            EXPECT_EQ(correctResourcePath({ { "sound" } }, wav.value(), vfs.get(), ".mp3"), "sound/foo.mp3");
+            EXPECT_EQ(correctResourcePath({ { "sound" } }, wav.value(), vfs.get(), "mp3"), "sound/foo.mp3");
         }
 
         TEST(MiscResourceHelpersCorrectResourcePath, shouldKeepExtentionIfExistInVfs)
@@ -57,13 +57,13 @@ namespace Misc::ResourceHelpers
             const std::unique_ptr<const VFS::Manager> vfs = TestingOpenMW::createTestVFS({
                 { wav, nullptr },
             });
-            EXPECT_EQ(correctResourcePath({ { "sound" } }, wav.value(), vfs.get(), ".mp3"), "sound/foo.wav");
+            EXPECT_EQ(correctResourcePath({ { "sound" } }, wav.value(), vfs.get(), "mp3"), "sound/foo.wav");
         }
 
         TEST(MiscResourceHelpersCorrectResourcePath, shouldPrefixWithGivenTopDirectory)
         {
             const std::unique_ptr<const VFS::Manager> vfs = TestingOpenMW::createTestVFS({});
-            EXPECT_EQ(correctResourcePath({ { "sound" } }, "foo.mp3", vfs.get(), ".mp3"), "sound/foo.mp3");
+            EXPECT_EQ(correctResourcePath({ { "sound" } }, "foo.mp3", vfs.get(), "mp3"), "sound/foo.mp3");
         }
 
         TEST(MiscResourceHelpersCorrectResourcePath, shouldChangeTopDirectoryAndKeepExtensionIfOriginalExistInVfs)
@@ -73,7 +73,7 @@ namespace Misc::ResourceHelpers
                 { a, nullptr },
             });
             EXPECT_EQ(
-                correctResourcePath({ { "textures", "bookart" } }, "bookart/foo.a", vfs.get(), ".b"), "textures/foo.a");
+                correctResourcePath({ { "textures", "bookart" } }, "bookart/foo.a", vfs.get(), "b"), "textures/foo.a");
         }
 
         TEST(MiscResourceHelpersCorrectResourcePath, shouldChangeTopDirectoryAndChangeExtensionIfFallbackExistInVfs)
@@ -83,31 +83,31 @@ namespace Misc::ResourceHelpers
                 { b, nullptr },
             });
             EXPECT_EQ(
-                correctResourcePath({ { "textures", "bookart" } }, "bookart/foo.a", vfs.get(), ".b"), "textures/foo.b");
+                correctResourcePath({ { "textures", "bookart" } }, "bookart/foo.a", vfs.get(), "b"), "textures/foo.b");
         }
 
         TEST(MiscResourceHelpersCorrectResourcePath, shouldLowerCase)
         {
             const std::unique_ptr<const VFS::Manager> vfs = TestingOpenMW::createTestVFS({});
-            EXPECT_EQ(correctResourcePath({ { "sound" } }, "SOUND\\Foo.MP3", vfs.get(), ".mp3"), "sound/foo.mp3");
+            EXPECT_EQ(correctResourcePath({ { "sound" } }, "SOUND\\Foo.MP3", vfs.get(), "mp3"), "sound/foo.mp3");
         }
 
         TEST(MiscResourceHelpersCorrectResourcePath, shouldRemoveLeadingSlash)
         {
             const std::unique_ptr<const VFS::Manager> vfs = TestingOpenMW::createTestVFS({});
-            EXPECT_EQ(correctResourcePath({ { "sound" } }, "\\SOUND\\Foo.MP3", vfs.get(), ".mp3"), "sound/foo.mp3");
+            EXPECT_EQ(correctResourcePath({ { "sound" } }, "\\SOUND\\Foo.MP3", vfs.get(), "mp3"), "sound/foo.mp3");
         }
 
         TEST(MiscResourceHelpersCorrectResourcePath, shouldRemoveDuplicateSlashes)
         {
             const std::unique_ptr<const VFS::Manager> vfs = TestingOpenMW::createTestVFS({});
-            EXPECT_EQ(correctResourcePath({ { "sound" } }, "\\\\SOUND\\\\Foo.MP3", vfs.get(), ".mp3"), "sound/foo.mp3");
+            EXPECT_EQ(correctResourcePath({ { "sound" } }, "\\\\SOUND\\\\Foo.MP3", vfs.get(), "mp3"), "sound/foo.mp3");
         }
 
         TEST(MiscResourceHelpersCorrectResourcePath, shouldConvertToForwardSlash)
         {
             const std::unique_ptr<const VFS::Manager> vfs = TestingOpenMW::createTestVFS({});
-            EXPECT_EQ(correctResourcePath({ { "sound" } }, "SOUND/Foo.MP3", vfs.get(), ".mp3"), "sound/foo.mp3");
+            EXPECT_EQ(correctResourcePath({ { "sound" } }, "SOUND/Foo.MP3", vfs.get(), "mp3"), "sound/foo.mp3");
         }
 
         struct MiscResourceHelpersCorrectResourcePathShouldRemoveExtraPrefix : TestWithParam<std::string>
@@ -117,7 +117,7 @@ namespace Misc::ResourceHelpers
         TEST_P(MiscResourceHelpersCorrectResourcePathShouldRemoveExtraPrefix, shouldMatchExpected)
         {
             const std::unique_ptr<const VFS::Manager> vfs = TestingOpenMW::createTestVFS({});
-            EXPECT_EQ(correctResourcePath({ { "sound" } }, GetParam(), vfs.get(), ".mp3"), "sound/foo.mp3");
+            EXPECT_EQ(correctResourcePath({ { "sound" } }, GetParam(), vfs.get(), "mp3"), "sound/foo.mp3");
         }
 
         const std::vector<std::string> pathsWithPrefix = {

apps/components_tests/vfs/testpathutil.cpp

@@ -168,6 +168,18 @@ namespace VFS::Path
             EXPECT_THROW(value.changeExtension("so/"), std::invalid_argument);
         }
 
+        TEST(VFSPathNormalizedTest, filenameShouldReturnLastComponentOfThePath)
+        {
+            const Normalized value("foo/bar");
+            EXPECT_EQ(value.filename(), "bar");
+        }
+
+        TEST(VFSPathNormalizedTest, filenameShouldReturnSameValueForPathWithSingleComponent)
+        {
+            const Normalized value("foo");
+            EXPECT_EQ(value.filename(), "foo");
+        }
+
         template <class T>
         struct VFSPathNormalizedOperatorsTest : Test
         {
@@ -246,5 +258,17 @@ namespace VFS::Path
             const Normalized result = a / b;
             EXPECT_EQ(result.value(), "foo/bar/baz");
         }
+
+        TEST(VFSPathNormalizedViewTest, filenameShouldReturnLastComponentOfThePath)
+        {
+            const NormalizedView value("foo/bar");
+            EXPECT_EQ(value.filename(), "bar");
+        }
+
+        TEST(VFSPathNormalizedViewTest, filenameShouldReturnSameValueForPathWithSingleComponent)
+        {
+            const NormalizedView value("foo");
+            EXPECT_EQ(value.filename(), "foo");
+        }
     }
 }

apps/openmw/mwsound/soundbuffer.cpp

@@ -201,7 +201,7 @@ namespace MWSound
     SoundBuffer* SoundBufferPool::insertSound(const ESM::RefId& soundId, const ESM4::Sound& sound)
     {
         std::string path = Misc::ResourceHelpers::correctResourcePath(
-            { { "sound" } }, sound.mSoundFile, MWBase::Environment::get().getResourceSystem()->getVFS(), ".mp3");
+            { { "sound" } }, sound.mSoundFile, MWBase::Environment::get().getResourceSystem()->getVFS(), "mp3");
         float volume = 1, min = 1, max = 255; // TODO: needs research
         SoundBuffer& sfx = mSoundBuffers.emplace_back(VFS::Path::Normalized(std::move(path)), volume, min, max);
         mBufferNameMap.emplace(soundId, &sfx);
@@ -211,7 +211,7 @@ namespace MWSound
     SoundBuffer* SoundBufferPool::insertSound(const ESM::RefId& soundId, const ESM4::SoundReference& sound)
     {
         std::string path = Misc::ResourceHelpers::correctResourcePath(
-            { { "sound" } }, sound.mSoundFile, MWBase::Environment::get().getResourceSystem()->getVFS(), ".mp3");
+            { { "sound" } }, sound.mSoundFile, MWBase::Environment::get().getResourceSystem()->getVFS(), "mp3");
         float volume = 1, min = 1, max = 255; // TODO: needs research
         // TODO: sound.mSoundId can link to another SoundReference, probably we will need to add additional lookups to
         // ESMStore.

components/misc/resourcehelpers.cpp

@@ -25,6 +25,38 @@ namespace
         }
         return false;
     }
+
+    std::size_t findDirectory(VFS::Path::NormalizedView path, std::string_view directory)
+    {
+        const std::string_view pathValue = path.value();
+        const std::size_t directorySize = directory.size();
+
+        for (std::size_t offset = 0, pathSize = pathValue.size(); offset < pathSize;)
+        {
+            const std::size_t position = pathValue.find(directory, offset);
+
+            if (position == std::string_view::npos)
+                return std::string_view::npos;
+
+            if (position + directorySize > pathSize)
+                return std::string_view::npos;
+
+            if ((position == 0 || pathValue[position - 1] == VFS::Path::separator)
+                && pathValue[position + directorySize] == VFS::Path::separator)
+                return position;
+
+            offset = position + directorySize;
+        }
+
+        return std::string_view::npos;
+    }
+
+    VFS::Path::Normalized withPrefix(VFS::Path::NormalizedView path, std::string_view prefix)
+    {
+        VFS::Path::Normalized prefixed(prefix);
+        prefixed /= path;
+        return prefixed;
+    }
 }
 
 bool Misc::ResourceHelpers::changeExtensionToDds(std::string& path)
@@ -36,38 +68,29 @@ bool Misc::ResourceHelpers::changeExtensionToDds(std::string& path)
 std::string Misc::ResourceHelpers::correctResourcePath(std::span<const std::string_view> topLevelDirectories,
     std::string_view resPath, const VFS::Manager* vfs, std::string_view ext)
 {
-    std::string correctedPath = VFS::Path::normalizeFilename(resPath);
+    VFS::Path::Normalized correctedPath(resPath);
 
     // Handle top level directory
     bool needsPrefix = true;
-    for (std::string_view potentialTopLevelDirectory : topLevelDirectories)
-    {
-        if (correctedPath.starts_with(potentialTopLevelDirectory)
-            && correctedPath.size() > potentialTopLevelDirectory.size()
-            && correctedPath[potentialTopLevelDirectory.size()] == VFS::Path::separator)
-        {
-            needsPrefix = false;
-            break;
-        }
-        else
-        {
-            std::string topLevelPrefix = std::string{ potentialTopLevelDirectory } + VFS::Path::separator;
-            size_t topLevelPos = correctedPath.find(VFS::Path::separator + topLevelPrefix);
-            if (topLevelPos != std::string::npos)
-            {
-                correctedPath.erase(0, topLevelPos + 1);
-                needsPrefix = false;
-                break;
-            }
-        }
-    }
-    if (needsPrefix)
-        correctedPath = std::string{ topLevelDirectories.front() } + VFS::Path::separator + correctedPath;
 
-    std::string origExt = correctedPath;
+    for (const std::string_view potentialTopLevelDirectory : topLevelDirectories)
+    {
+        if (const std::size_t topLevelPos = findDirectory(correctedPath, potentialTopLevelDirectory);
+            topLevelPos != std::string::npos)
+        {
+            correctedPath = VFS::Path::Normalized(correctedPath.value().substr(topLevelPos));
+            needsPrefix = false;
+            break;
+        }
+    }
+
+    if (needsPrefix)
+        correctedPath = withPrefix(correctedPath, topLevelDirectories.front());
+
+    const VFS::Path::Normalized origExt = correctedPath;
 
     // replace extension if `ext` is specified (used for .tga -> .dds, .wav -> .mp3)
-    bool isExtChanged = !ext.empty() && changeExtension(correctedPath, ext);
+    const bool isExtChanged = !ext.empty() && correctedPath.changeExtension(ext);
 
     if (vfs->exists(correctedPath))
         return correctedPath;
@@ -77,18 +100,15 @@ std::string Misc::ResourceHelpers::correctResourcePath(std::span<const std::stri
         return origExt;
 
     // fall back to a resource in the top level directory if it exists
-    std::string fallback{ topLevelDirectories.front() };
+    {
-    fallback += VFS::Path::separator;
+        const VFS::Path::Normalized fallback = withPrefix(correctedPath.filename(), topLevelDirectories.front());
-    fallback += Misc::getFileName(correctedPath);
-
         if (vfs->exists(fallback))
             return fallback;
+    }
 
     if (isExtChanged)
     {
-        fallback = topLevelDirectories.front();
+        const VFS::Path::Normalized fallback = withPrefix(origExt.filename(), topLevelDirectories.front());
-        fallback += VFS::Path::separator;
-        fallback += Misc::getFileName(origExt);
         if (vfs->exists(fallback))
             return fallback;
     }
@@ -102,17 +122,17 @@ std::string Misc::ResourceHelpers::correctResourcePath(std::span<const std::stri
 
 std::string Misc::ResourceHelpers::correctTexturePath(std::string_view resPath, const VFS::Manager* vfs)
 {
-    return correctResourcePath({ { "textures", "bookart" } }, resPath, vfs, ".dds");
+    return correctResourcePath({ { "textures", "bookart" } }, resPath, vfs, "dds");
 }
 
 std::string Misc::ResourceHelpers::correctIconPath(std::string_view resPath, const VFS::Manager* vfs)
 {
-    return correctResourcePath({ { "icons" } }, resPath, vfs, ".dds");
+    return correctResourcePath({ { "icons" } }, resPath, vfs, "dds");
 }
 
 std::string Misc::ResourceHelpers::correctBookartPath(std::string_view resPath, const VFS::Manager* vfs)
 {
-    return correctResourcePath({ { "bookart", "textures" } }, resPath, vfs, ".dds");
+    return correctResourcePath({ { "bookart", "textures" } }, resPath, vfs, "dds");
 }
 
 std::string Misc::ResourceHelpers::correctBookartPath(
@@ -190,9 +210,9 @@ VFS::Path::Normalized Misc::ResourceHelpers::correctSoundPath(
     VFS::Path::NormalizedView resPath, const VFS::Manager& vfs)
 {
     // Note: likely should be replaced with
-    //     return correctResourcePath({ { "sound" } }, resPath, vfs, ".mp3");
+    //     return correctResourcePath({ { "sound" } }, resPath, vfs, "mp3");
     // but there is a slight difference in behaviour:
-    // - `correctResourcePath(..., ".mp3")` first checks `.mp3`, then tries the original extension
+    // - `correctResourcePath(..., "mp3")` first checks `.mp3`, then tries the original extension
     // - the implementation below first tries the original extension, then falls back to `.mp3`.
 
     // Workaround: Bethesda at some point converted some of the files to mp3, but the references were kept as .wav.

components/vfs/pathutil.hpp

@@ -183,6 +183,14 @@ namespace VFS::Path
             return stem;
         }
 
+        NormalizedView filename() const
+        {
+            NormalizedView result(*this);
+            if (const std::size_t position = mValue.find_last_of(separator); position != std::string_view::npos)
+                result.mValue.remove_prefix(position + 1);
+            return result;
+        }
+
     private:
         std::string_view mValue;
     };
@@ -329,6 +337,11 @@ namespace VFS::Path
             return NormalizedView(*this).stem();
         }
 
+        NormalizedView filename() const
+        {
+            return NormalizedView(*this).filename();
+        }
+
     private:
         std::string mValue;
     };