Merge branch 'vfs_normalized_path_3' into 'master'

Use normalized path for correctSoundPath

See merge request OpenMW/openmw!3903

apps/openmw/mwbase/soundmanager.hpp

@@ -6,6 +6,8 @@
 #include <string>
 #include <string_view>
 
+#include <components/vfs/pathutil.hpp>
+
 #include "../mwsound/type.hpp"
 #include "../mwworld/ptr.hpp"
 
@@ -129,11 +131,11 @@ namespace MWBase
         /// \param name of the folder that contains the playlist
         /// Title music playlist is predefined
 
-        virtual void say(const MWWorld::ConstPtr& reference, const std::string& filename) = 0;
+        virtual void say(const MWWorld::ConstPtr& reference, VFS::Path::NormalizedView filename) = 0;
         ///< Make an actor say some text.
         /// \param filename name of a sound file in the VFS
 
-        virtual void say(const std::string& filename) = 0;
+        virtual void say(VFS::Path::NormalizedView filename) = 0;
         ///< Say some text, without an actor ref
         /// \param filename name of a sound file in the VFS
 

apps/openmw/mwdialogue/dialoguemanagerimp.cpp

@@ -653,7 +653,7 @@ namespace MWDialogue
             if (Settings::gui().mSubtitles)
                 winMgr->messageBox(info->mResponse);
             if (!info->mSound.empty())
-                sndMgr->say(actor, Misc::ResourceHelpers::correctSoundPath(info->mSound));
+                sndMgr->say(actor, Misc::ResourceHelpers::correctSoundPath(VFS::Path::Normalized(info->mSound)));
             if (!info->mResultScript.empty())
                 executeScript(info->mResultScript, actor);
         }

apps/openmw/mwgui/charactercreation.cpp

@@ -39,7 +39,7 @@ namespace
     {
         const std::string mText;
         const Response mResponses[3];
-        const std::string mSound;
+        const VFS::Path::Normalized mSound;
     };
 
     Step sGenerateClassSteps(int number)

apps/openmw/mwlua/soundbindings.cpp

@@ -174,12 +174,12 @@ namespace MWLua
         api["say"] = sol::overload(
             [luaManager = context.mLuaManager](
                 std::string_view fileName, const Object& object, sol::optional<std::string_view> text) {
-                MWBase::Environment::get().getSoundManager()->say(object.ptr(), std::string(fileName));
+                MWBase::Environment::get().getSoundManager()->say(object.ptr(), VFS::Path::Normalized(fileName));
                 if (text)
                     luaManager->addUIMessage(*text);
             },
             [luaManager = context.mLuaManager](std::string_view fileName, sol::optional<std::string_view> text) {
-                MWBase::Environment::get().getSoundManager()->say(std::string(fileName));
+                MWBase::Environment::get().getSoundManager()->say(VFS::Path::Normalized(fileName));
                 if (text)
                     luaManager->addUIMessage(*text);
             });
@@ -227,7 +227,7 @@ namespace MWLua
         soundT["maxRange"]
             = sol::readonly_property([](const ESM::Sound& rec) -> unsigned char { return rec.mData.mMaxRange; });
         soundT["fileName"] = sol::readonly_property([](const ESM::Sound& rec) -> std::string {
-            return VFS::Path::normalizeFilename(Misc::ResourceHelpers::correctSoundPath(rec.mSound));
+            return Misc::ResourceHelpers::correctSoundPath(VFS::Path::Normalized(rec.mSound)).value();
         });
 
         return LuaUtil::makeReadOnly(api);

apps/openmw/mwscript/soundextensions.cpp

@@ -33,7 +33,7 @@ namespace MWScript
                 MWScript::InterpreterContext& context
                     = static_cast<MWScript::InterpreterContext&>(runtime.getContext());
 
-                std::string file{ runtime.getStringLiteral(runtime[0].mInteger) };
+                VFS::Path::Normalized file{ runtime.getStringLiteral(runtime[0].mInteger) };
                 runtime.pop();
 
                 std::string_view text = runtime.getStringLiteral(runtime[0].mInteger);

apps/openmw/mwsound/openal_output.cpp

@@ -1034,7 +1034,7 @@ namespace MWSound
         return ret;
     }
 
-    std::pair<Sound_Handle, size_t> OpenAL_Output::loadSound(const std::string& fname)
+    std::pair<Sound_Handle, size_t> OpenAL_Output::loadSound(VFS::Path::NormalizedView fname)
     {
         getALError();
 
@@ -1045,7 +1045,7 @@ namespace MWSound
         try
         {
             DecoderPtr decoder = mManager.getDecoder();
-            decoder->open(Misc::ResourceHelpers::correctSoundPath(fname, decoder->mResourceMgr));
+            decoder->open(Misc::ResourceHelpers::correctSoundPath(fname, *decoder->mResourceMgr));
 
             ChannelConfig chans;
             SampleType type;

apps/openmw/mwsound/openal_output.hpp

@@ -7,6 +7,8 @@
 #include <string>
 #include <vector>
 
+#include <components/vfs/pathutil.hpp>
+
 #include "al.h"
 #include "alc.h"
 #include "alext.h"
@@ -85,7 +87,7 @@ namespace MWSound
 
         std::vector<std::string> enumerateHrtf() override;
 
-        std::pair<Sound_Handle, size_t> loadSound(const std::string& fname) override;
+        std::pair<Sound_Handle, size_t> loadSound(VFS::Path::NormalizedView fname) override;
         size_t unloadSound(Sound_Handle data) override;
 
         bool playSound(Sound* sound, Sound_Handle data, float offset) override;

apps/openmw/mwsound/sound_buffer.cpp

@@ -183,9 +183,8 @@ namespace MWSound
         min = std::max(min, 1.0f);
         max = std::max(min, max);
 
-        Sound_Buffer& sfx
+        Sound_Buffer& sfx = mSoundBuffers.emplace_back(
-            = mSoundBuffers.emplace_back(Misc::ResourceHelpers::correctSoundPath(sound.mSound), volume, min, max);
+            Misc::ResourceHelpers::correctSoundPath(VFS::Path::Normalized(sound.mSound)), volume, min, max);
-        VFS::Path::normalizeFilenameInPlace(sfx.mResourceName);
 
         mBufferNameMap.emplace(soundId, &sfx);
         return &sfx;

apps/openmw/mwsound/sound_buffer.hpp

@@ -35,7 +35,7 @@ namespace MWSound
         {
         }
 
-        const std::string& getResourceName() const noexcept { return mResourceName; }
+        const VFS::Path::Normalized& getResourceName() const noexcept { return mResourceName; }
 
         Sound_Handle getHandle() const noexcept { return mHandle; }
 
@@ -46,7 +46,7 @@ namespace MWSound
         float getMaxDist() const noexcept { return mMaxDist; }
 
     private:
-        std::string mResourceName;
+        VFS::Path::Normalized mResourceName;
         float mVolume;
         float mMinDist;
         float mMaxDist;

apps/openmw/mwsound/sound_output.hpp

@@ -6,6 +6,7 @@
 #include <vector>
 
 #include <components/settings/hrtfmode.hpp>
+#include <components/vfs/pathutil.hpp>
 
 #include "../mwbase/soundmanager.hpp"
 
@@ -39,7 +40,7 @@ namespace MWSound
 
         virtual std::vector<std::string> enumerateHrtf() = 0;
 
-        virtual std::pair<Sound_Handle, size_t> loadSound(const std::string& fname) = 0;
+        virtual std::pair<Sound_Handle, size_t> loadSound(VFS::Path::NormalizedView fname) = 0;
         virtual size_t unloadSound(Sound_Handle data) = 0;
 
         virtual bool playSound(Sound* sound, Sound_Handle data, float offset) = 0;

apps/openmw/mwsound/soundmanagerimp.cpp

@@ -172,12 +172,12 @@ namespace MWSound
         return std::make_shared<FFmpeg_Decoder>(mVFS);
     }
 
-    DecoderPtr SoundManager::loadVoice(const std::string& voicefile)
+    DecoderPtr SoundManager::loadVoice(VFS::Path::NormalizedView voicefile)
     {
         try
         {
             DecoderPtr decoder = getDecoder();
-            decoder->open(Misc::ResourceHelpers::correctSoundPath(voicefile, decoder->mResourceMgr));
+            decoder->open(Misc::ResourceHelpers::correctSoundPath(voicefile, *decoder->mResourceMgr));
             return decoder;
         }
         catch (std::exception& e)
@@ -380,7 +380,7 @@ namespace MWSound
         startRandomTitle();
     }
 
-    void SoundManager::say(const MWWorld::ConstPtr& ptr, const std::string& filename)
+    void SoundManager::say(const MWWorld::ConstPtr& ptr, VFS::Path::NormalizedView filename)
     {
         if (!mOutput->isInitialized())
             return;
@@ -412,7 +412,7 @@ namespace MWSound
         return 0.0f;
     }
 
-    void SoundManager::say(const std::string& filename)
+    void SoundManager::say(VFS::Path::NormalizedView filename)
     {
         if (!mOutput->isInitialized())
             return;

apps/openmw/mwsound/soundmanagerimp.hpp

@@ -116,7 +116,7 @@ namespace MWSound
         Sound_Buffer* insertSound(const std::string& soundId, const ESM::Sound* sound);
 
         // returns a decoder to start streaming, or nullptr if the sound was not found
-        DecoderPtr loadVoice(const std::string& voicefile);
+        DecoderPtr loadVoice(VFS::Path::NormalizedView voicefile);
 
         SoundPtr getSoundRef();
         StreamPtr getStreamRef();
@@ -188,11 +188,11 @@ namespace MWSound
         /// \param name of the folder that contains the playlist
         /// Title music playlist is predefined
 
-        void say(const MWWorld::ConstPtr& reference, const std::string& filename) override;
+        void say(const MWWorld::ConstPtr& reference, VFS::Path::NormalizedView filename) override;
         ///< Make an actor say some text.
         /// \param filename name of a sound file in the VFS
 
-        void say(const std::string& filename) override;
+        void say(VFS::Path::NormalizedView filename) override;
         ///< Say some text, without an actor ref
         /// \param filename name of a sound file in the VFS
 

apps/openmw_test_suite/misc/test_resourcehelpers.cpp

@@ -8,26 +8,19 @@ namespace
     TEST(CorrectSoundPath, wav_files_not_overridden_with_mp3_in_vfs_are_not_corrected)
     {
         std::unique_ptr<VFS::Manager> mVFS = TestingOpenMW::createTestVFS({ { "sound/bar.wav", nullptr } });
-        EXPECT_EQ(correctSoundPath("sound/bar.wav", mVFS.get()), "sound/bar.wav");
+        EXPECT_EQ(correctSoundPath("sound/bar.wav", *mVFS), "sound/bar.wav");
     }
 
     TEST(CorrectSoundPath, wav_files_overridden_with_mp3_in_vfs_are_corrected)
     {
         std::unique_ptr<VFS::Manager> mVFS = TestingOpenMW::createTestVFS({ { "sound/foo.mp3", nullptr } });
-        EXPECT_EQ(correctSoundPath("sound/foo.wav", mVFS.get()), "sound/foo.mp3");
+        EXPECT_EQ(correctSoundPath("sound/foo.wav", *mVFS), "sound/foo.mp3");
     }
 
     TEST(CorrectSoundPath, corrected_path_does_not_check_existence_in_vfs)
     {
         std::unique_ptr<VFS::Manager> mVFS = TestingOpenMW::createTestVFS({});
-        EXPECT_EQ(correctSoundPath("sound/foo.wav", mVFS.get()), "sound/foo.mp3");
+        EXPECT_EQ(correctSoundPath("sound/foo.wav", *mVFS), "sound/foo.mp3");
-    }
-
-    TEST(CorrectSoundPath, correct_path_normalize_paths)
-    {
-        std::unique_ptr<VFS::Manager> mVFS = TestingOpenMW::createTestVFS({});
-        EXPECT_EQ(correctSoundPath("sound\\foo.wav", mVFS.get()), "sound/foo.mp3");
-        EXPECT_EQ(correctSoundPath("SOUND\\foo.WAV", mVFS.get()), "sound/foo.mp3");
     }
 
     namespace

apps/openmw_test_suite/testing_util.hpp

@@ -2,6 +2,7 @@
 #define TESTING_UTIL_H
 
 #include <filesystem>
+#include <initializer_list>
 #include <sstream>
 
 #include <components/misc/strings/conversion.hpp>
@@ -73,6 +74,12 @@ namespace TestingOpenMW
         return vfs;
     }
 
+    inline std::unique_ptr<VFS::Manager> createTestVFS(
+        std::initializer_list<std::pair<std::string_view, VFS::File*>> files)
+    {
+        return createTestVFS(VFS::FileMap(files.begin(), files.end()));
+    }
+
 #define EXPECT_ERROR(X, ERR_SUBSTR)                                                                                    \
     try                                                                                                                \
     {                                                                                                                  \

apps/openmw_test_suite/vfs/testpathutil.cpp

@@ -65,6 +65,53 @@ namespace VFS::Path
             EXPECT_EQ(stream.str(), "foo/bar/baz");
         }
 
+        TEST(NormalizedTest, shouldSupportOperatorDivEqual)
+        {
+            Normalized value("foo/bar");
+            value /= NormalizedView("baz");
+            EXPECT_EQ(value.value(), "foo/bar/baz");
+        }
+
+        TEST(NormalizedTest, changeExtensionShouldReplaceAfterLastDot)
+        {
+            Normalized value("foo/bar.a");
+            ASSERT_TRUE(value.changeExtension("so"));
+            EXPECT_EQ(value.value(), "foo/bar.so");
+        }
+
+        TEST(NormalizedTest, changeExtensionShouldNormalizeExtension)
+        {
+            Normalized value("foo/bar.a");
+            ASSERT_TRUE(value.changeExtension("SO"));
+            EXPECT_EQ(value.value(), "foo/bar.so");
+        }
+
+        TEST(NormalizedTest, changeExtensionShouldIgnorePathWithoutADot)
+        {
+            Normalized value("foo/bar");
+            ASSERT_FALSE(value.changeExtension("so"));
+            EXPECT_EQ(value.value(), "foo/bar");
+        }
+
+        TEST(NormalizedTest, changeExtensionShouldIgnorePathWithDotBeforeSeparator)
+        {
+            Normalized value("foo.bar/baz");
+            ASSERT_FALSE(value.changeExtension("so"));
+            EXPECT_EQ(value.value(), "foo.bar/baz");
+        }
+
+        TEST(NormalizedTest, changeExtensionShouldThrowExceptionOnExtensionWithDot)
+        {
+            Normalized value("foo.a");
+            EXPECT_THROW(value.changeExtension(".so"), std::invalid_argument);
+        }
+
+        TEST(NormalizedTest, changeExtensionShouldThrowExceptionOnExtensionWithSeparator)
+        {
+            Normalized value("foo.a");
+            EXPECT_THROW(value.changeExtension("/so"), std::invalid_argument);
+        }
+
         template <class T>
         struct NormalizedOperatorsTest : Test
         {
@@ -135,5 +182,13 @@ namespace VFS::Path
         {
             EXPECT_THROW([] { NormalizedView("Foo\\Bar/baz"); }(), std::invalid_argument);
         }
+
+        TEST(NormalizedView, shouldSupportOperatorDiv)
+        {
+            const NormalizedView a("foo/bar");
+            const NormalizedView b("baz");
+            const Normalized result = a / b;
+            EXPECT_EQ(result.value(), "foo/bar/baz");
+        }
     }
 }

components/misc/resourcehelpers.cpp

@@ -180,9 +180,10 @@ std::string Misc::ResourceHelpers::correctMeshPath(std::string_view resPath)
     return res;
 }
 
-std::string Misc::ResourceHelpers::correctSoundPath(const std::string& resPath)
+VFS::Path::Normalized Misc::ResourceHelpers::correctSoundPath(VFS::Path::NormalizedView resPath)
 {
-    return "sound\\" + resPath;
+    static constexpr VFS::Path::NormalizedView prefix("sound");
+    return prefix / resPath;
 }
 
 std::string Misc::ResourceHelpers::correctMusicPath(const std::string& resPath)
@@ -201,17 +202,17 @@ std::string_view Misc::ResourceHelpers::meshPathForESM3(std::string_view resPath
     return resPath.substr(prefix.size() + 1);
 }
 
-std::string Misc::ResourceHelpers::correctSoundPath(std::string_view resPath, const VFS::Manager* vfs)
+VFS::Path::Normalized Misc::ResourceHelpers::correctSoundPath(
+    VFS::Path::NormalizedView resPath, const VFS::Manager& vfs)
 {
     // Workaround: Bethesda at some point converted some of the files to mp3, but the references were kept as .wav.
-    if (!vfs->exists(resPath))
+    if (!vfs.exists(resPath))
     {
-        std::string sound{ resPath };
+        VFS::Path::Normalized sound(resPath);
-        changeExtension(sound, ".mp3");
+        sound.changeExtension("mp3");
-        VFS::Path::normalizeFilenameInPlace(sound);
         return sound;
     }
-    return VFS::Path::normalizeFilename(resPath);
+    return VFS::Path::Normalized(resPath);
 }
 
 bool Misc::ResourceHelpers::isHiddenMarker(const ESM::RefId& id)

components/misc/resourcehelpers.hpp

@@ -1,6 +1,8 @@
 #ifndef MISC_RESOURCEHELPERS_H
 #define MISC_RESOURCEHELPERS_H
 
+#include <components/vfs/pathutil.hpp>
+
 #include <span>
 #include <string>
 #include <string_view>
@@ -37,7 +39,7 @@ namespace Misc
         std::string correctMeshPath(std::string_view resPath);
 
         // Adds "sound\\".
-        std::string correctSoundPath(const std::string& resPath);
+        VFS::Path::Normalized correctSoundPath(VFS::Path::NormalizedView resPath);
 
         // Adds "music\\".
         std::string correctMusicPath(const std::string& resPath);
@@ -45,7 +47,7 @@ namespace Misc
         // Removes "meshes\\".
         std::string_view meshPathForESM3(std::string_view resPath);
 
-        std::string correctSoundPath(std::string_view resPath, const VFS::Manager* vfs);
+        VFS::Path::Normalized correctSoundPath(VFS::Path::NormalizedView resPath, const VFS::Manager& vfs);
 
         /// marker objects that have a hardcoded function in the game logic, should be hidden from the player
         bool isHiddenMarker(const ESM::RefId& id);

components/vfs/manager.cpp

@@ -57,6 +57,11 @@ namespace VFS
         return mIndex.find(name) != mIndex.end();
     }
 
+    bool Manager::exists(Path::NormalizedView name) const
+    {
+        return mIndex.find(name) != mIndex.end();
+    }
+
     std::string Manager::getArchive(const Path::Normalized& name) const
     {
         for (auto it = mArchives.rbegin(); it != mArchives.rend(); ++it)

components/vfs/manager.hpp

@@ -43,6 +43,8 @@ namespace VFS
         /// @note May be called from any thread once the index has been built.
         bool exists(const Path::Normalized& name) const;
 
+        bool exists(Path::NormalizedView name) const;
+
         /// Retrieve a file by name.
         /// @note Throws an exception if the file can not be found.
         /// @note May be called from any thread once the index has been built.

components/vfs/pathutil.hpp

@@ -11,9 +11,12 @@
 
 namespace VFS::Path
 {
+    inline constexpr char separator = '/';
+    inline constexpr char extensionSeparator = '.';
+
     inline constexpr char normalize(char c)
     {
-        return c == '\\' ? '/' : Misc::StringUtils::toLower(c);
+        return c == '\\' ? separator : Misc::StringUtils::toLower(c);
     }
 
     inline constexpr bool isNormalized(std::string_view name)
@@ -21,9 +24,14 @@ namespace VFS::Path
         return std::all_of(name.begin(), name.end(), [](char v) { return v == normalize(v); });
     }
 
+    inline void normalizeFilenameInPlace(auto begin, auto end)
+    {
+        std::transform(begin, end, begin, normalize);
+    }
+
     inline void normalizeFilenameInPlace(std::string& name)
     {
-        std::transform(name.begin(), name.end(), name.begin(), normalize);
+        normalizeFilenameInPlace(name.begin(), name.end());
     }
 
     /// Normalize the given filename, making slashes/backslashes consistent, and lower-casing.
@@ -59,6 +67,11 @@ namespace VFS::Path
         bool operator()(std::string_view left, std::string_view right) const { return pathLess(left, right); }
     };
 
+    inline constexpr auto findSeparatorOrExtensionSeparator(auto begin, auto end)
+    {
+        return std::find_if(begin, end, [](char v) { return v == extensionSeparator || v == separator; });
+    }
+
     class Normalized;
 
     class NormalizedView
@@ -122,7 +135,7 @@ namespace VFS::Path
         {
         }
 
-        Normalized(const char* value)
+        explicit Normalized(const char* value)
             : Normalized(std::string_view(value))
         {
         }
@@ -153,6 +166,27 @@ namespace VFS::Path
 
         operator const std::string&() const { return mValue; }
 
+        bool changeExtension(std::string_view extension)
+        {
+            if (findSeparatorOrExtensionSeparator(extension.begin(), extension.end()) != extension.end())
+                throw std::invalid_argument("Invalid extension: " + std::string(extension));
+            const auto it = findSeparatorOrExtensionSeparator(mValue.rbegin(), mValue.rend());
+            if (it == mValue.rend() || *it == separator)
+                return false;
+            const std::string::difference_type pos = mValue.rend() - it;
+            mValue.replace(pos, mValue.size(), extension);
+            normalizeFilenameInPlace(mValue.begin() + pos, mValue.end());
+            return true;
+        }
+
+        Normalized& operator/=(NormalizedView value)
+        {
+            mValue.reserve(mValue.size() + value.value().size() + 1);
+            mValue += separator;
+            mValue += value.value();
+            return *this;
+        }
+
         friend bool operator==(const Normalized& lhs, const Normalized& rhs) = default;
 
         friend bool operator==(const Normalized& lhs, const auto& rhs) { return lhs.mValue == rhs; }
@@ -207,6 +241,13 @@ namespace VFS::Path
         : mValue(value.view())
     {
     }
+
+    inline Normalized operator/(NormalizedView lhs, NormalizedView rhs)
+    {
+        Normalized result(lhs);
+        result /= rhs;
+        return result;
+    }
 }
 
 #endif