Merge branch 'string_to_ref_id' into 'master'

Replace std::string and std::string_view by ESM::RefId to avoid getRefIdString call

See merge request OpenMW/openmw!2790

apps/opencs/model/doc/savingstages.cpp

@@ -236,7 +236,7 @@ CSMDoc::CollectionReferencesStage::CollectionReferencesStage(Document& document,
 
 int CSMDoc::CollectionReferencesStage::setup()
 {
-    mState.getSubRecords().clear();
+    mState.clearSubRecords();
 
     int size = mDocument.getData().getReferences().getSize();
 
@@ -260,7 +260,7 @@ void CSMDoc::CollectionReferencesStage::perform(int stage, Messages& messages)
             const ESM::RefId& cellId
                 = record.get().mOriginalCell.empty() ? record.get().mCell : record.get().mOriginalCell;
 
-            std::deque<int>& indices = mState.getSubRecords()[cellId.getRefIdString()];
+            std::deque<int>& indices = mState.getOrInsertSubRecord(cellId);
 
             // collect moved references at the end of the container
             const bool interior = !cellId.startsWith("#");
@@ -360,11 +360,9 @@ void CSMDoc::WriteCellCollectionStage::perform(int stage, Messages& messages)
     std::deque<int> tempRefs;
     std::deque<int> persistentRefs;
 
-    std::map<std::string, std::deque<int>>::const_iterator references
+    const std::deque<int>* references = mState.findSubRecord(cell.get().mId);
-        = mState.getSubRecords().find(cell.get().mId.getRefIdString());
 
-    if (cell.isModified() || cell.mState == CSMWorld::RecordBase::State_Deleted
+    if (cell.isModified() || cell.mState == CSMWorld::RecordBase::State_Deleted || references != nullptr)
-        || references != mState.getSubRecords().end())
     {
         CSMWorld::Cell cellRecord = cell.get();
         const bool interior = !cellRecord.mId.startsWith("#");
@@ -372,10 +370,9 @@ void CSMDoc::WriteCellCollectionStage::perform(int stage, Messages& messages)
         // count new references and adjust RefNumCount accordingsly
         unsigned int newRefNum = cellRecord.mRefNumCounter;
 
-        if (references != mState.getSubRecords().end())
+        if (references != nullptr)
         {
-            for (std::deque<int>::const_iterator iter(references->second.begin()); iter != references->second.end();
+            for (std::deque<int>::const_iterator iter(references->begin()); iter != references->end(); ++iter)
-                 ++iter)
             {
                 const CSMWorld::Record<CSMWorld::CellRef>& ref = mDocument.getData().getReferences().getRecord(*iter);
 
@@ -421,10 +418,10 @@ void CSMDoc::WriteCellCollectionStage::perform(int stage, Messages& messages)
         cellRecord.save(writer, cell.mState == CSMWorld::RecordBase::State_Deleted);
 
         // write references
-        if (references != mState.getSubRecords().end())
+        if (references != nullptr)
         {
             writeReferences(persistentRefs, interior, newRefNum);
-            cellRecord.saveTempMarker(writer, int(references->second.size()) - persistentRefs.size());
+            cellRecord.saveTempMarker(writer, static_cast<int>(references->size()) - persistentRefs.size());
             writeReferences(tempRefs, interior, newRefNum);
         }
 

apps/opencs/model/doc/savingstate.cpp

@@ -68,7 +68,20 @@ bool CSMDoc::SavingState::isProjectFile() const
     return mProjectFile;
 }
 
-std::map<std::string, std::deque<int>, Misc::StringUtils::CiComp>& CSMDoc::SavingState::getSubRecords()
+const std::deque<int>* CSMDoc::SavingState::findSubRecord(const ESM::RefId& refId) const
 {
-    return mSubRecords;
+    const auto it = mSubRecords.find(refId);
+    if (it == mSubRecords.end())
+        return nullptr;
+    return &it->second;
+}
+
+std::deque<int>& CSMDoc::SavingState::getOrInsertSubRecord(const ESM::RefId& refId)
+{
+    return mSubRecords[refId];
+}
+
+void CSMDoc::SavingState::clearSubRecords()
+{
+    mSubRecords.clear();
 }

apps/opencs/model/doc/savingstate.hpp

@@ -8,9 +8,9 @@
 #include <string>
 
 #include <components/esm3/esmwriter.hpp>
-
 #include <components/misc/algorithm.hpp>
 #include <components/to_utf8/to_utf8.hpp>
+
 namespace CSMDoc
 {
     class Operation;
@@ -26,7 +26,7 @@ namespace CSMDoc
         ESM::ESMWriter mWriter;
         std::filesystem::path mProjectPath;
         bool mProjectFile;
-        std::map<std::string, std::deque<int>, Misc::StringUtils::CiComp> mSubRecords; // record ID, list of subrecords
+        std::map<ESM::RefId, std::deque<int>> mSubRecords; // record ID, list of subrecords
 
     public:
         SavingState(Operation& operation, std::filesystem::path projectPath, ToUTF8::FromType encoding);
@@ -47,7 +47,11 @@ namespace CSMDoc
         bool isProjectFile() const;
         ///< Currently saving project file? (instead of content file)
 
-        std::map<std::string, std::deque<int>, Misc::StringUtils::CiComp>& getSubRecords();
+        const std::deque<int>* findSubRecord(const ESM::RefId& refId) const;
+
+        std::deque<int>& getOrInsertSubRecord(const ESM::RefId& refId);
+
+        void clearSubRecords();
     };
 
 }

apps/opencs/model/tools/mergestages.cpp

@@ -15,7 +15,6 @@
 
 #include <components/esm3/cellref.hpp>
 #include <components/esm3/esmreader.hpp>
-#include <components/misc/strings/lower.hpp>
 
 #include "mergestate.hpp"
 
@@ -117,7 +116,7 @@ void CSMTools::MergeReferencesStage::perform(int stage, CSMDoc::Messages& messag
 
         ref.mOriginalCell = ref.mCell;
 
-        ref.mRefNum.mIndex = mIndex[Misc::StringUtils::lowerCase(ref.mCell.getRefIdString())]++;
+        ref.mRefNum.mIndex = mIndex[ref.mCell]++;
         ref.mRefNum.mContentFile = 0;
         ref.mNew = false;
 

apps/opencs/model/tools/mergestages.hpp

@@ -113,7 +113,7 @@ namespace CSMTools
     class MergeReferencesStage : public CSMDoc::Stage
     {
         MergeState& mState;
-        std::map<std::string, int> mIndex;
+        std::map<ESM::RefId, int> mIndex;
 
     public:
         MergeReferencesStage(MergeState& state);

apps/opencs/model/world/actoradapter.cpp

@@ -136,7 +136,7 @@ namespace CSMWorld
         return SceneUtil::getActorSkeleton(firstPerson, mFemale, beast, werewolf);
     }
 
-    std::string_view ActorAdapter::ActorData::getPart(ESM::PartReferenceType index) const
+    ESM::RefId ActorAdapter::ActorData::getPart(ESM::PartReferenceType index) const
     {
         auto it = mParts.find(index);
         if (it == mParts.end())
@@ -146,12 +146,11 @@ namespace CSMWorld
                 if (mFemale)
                 {
                     // Note: we should use male parts for females as fallback
-                    const std::string& femalePart = mRaceData->getFemalePart(index).getRefIdString();
+                    if (const ESM::RefId femalePart = mRaceData->getFemalePart(index); !femalePart.empty())
-                    if (!femalePart.empty())
                         return femalePart;
                 }
 
-                return mRaceData->getMalePart(index).getRefIdString();
+                return mRaceData->getMalePart(index);
             }
 
             return {};
@@ -174,7 +173,7 @@ namespace CSMWorld
                 return;
         }
 
-        mParts[index] = std::make_pair(partId.getRefIdString(), priority);
+        mParts[index] = std::make_pair(partId, priority);
         addOtherDependency(partId);
     }
 

apps/opencs/model/world/actoradapter.hpp

@@ -35,7 +35,7 @@ namespace CSMWorld
         Q_OBJECT
     public:
         /// A list indexed by ESM::PartReferenceType
-        using ActorPartList = std::map<ESM::PartReferenceType, std::pair<std::string, int>>;
+        using ActorPartList = std::map<ESM::PartReferenceType, std::pair<ESM::RefId, int>>;
         /// A list indexed by ESM::BodyPart::MeshPart
         using RacePartList = std::array<ESM::RefId, ESM::BodyPart::MP_Count>;
         /// Tracks unique strings
@@ -95,7 +95,7 @@ namespace CSMWorld
             /// Returns the skeleton the actor should use for attaching parts to
             std::string getSkeleton() const;
             /// Retrieves the associated actor part
-            std::string_view getPart(ESM::PartReferenceType index) const;
+            ESM::RefId getPart(ESM::PartReferenceType index) const;
             /// Checks if the actor has a data dependency
             bool hasDependency(const ESM::RefId& id) const;
 

apps/opencs/model/world/cellcoordinates.cpp

@@ -6,6 +6,7 @@
 
 #include <osg/Vec3d>
 
+#include <components/esm/refid.hpp>
 #include <components/esm3/loadland.hpp>
 #include <components/misc/constants.hpp>
 
@@ -59,6 +60,11 @@ bool CSMWorld::CellCoordinates::isExteriorCell(const std::string& id)
     return (!id.empty() && id[0] == '#');
 }
 
+bool CSMWorld::CellCoordinates::isExteriorCell(const ESM::RefId& id)
+{
+    return id.startsWith("#");
+}
+
 std::pair<CSMWorld::CellCoordinates, bool> CSMWorld::CellCoordinates::fromId(const std::string& id)
 {
     // no worldspace for now, needs to be changed for 1.1

apps/opencs/model/world/cellcoordinates.hpp

@@ -12,6 +12,11 @@ namespace osg
     class Vec3d;
 }
 
+namespace ESM
+{
+    class RefId;
+}
+
 namespace CSMWorld
 {
     class CellCoordinates
@@ -41,6 +46,8 @@ namespace CSMWorld
 
         static bool isExteriorCell(const std::string& id);
 
+        static bool isExteriorCell(const ESM::RefId& id);
+
         /// \return first: CellCoordinates (or 0, 0 if cell does not have coordinates),
         /// second: is cell paged?
         ///

apps/opencs/model/world/collection.hpp

@@ -91,10 +91,10 @@ namespace CSMWorld
         ///
         /// \return Success?
 
-        int cloneRecordImp(const std::string& origin, const std::string& dest, UniversalId::Type type);
+        int cloneRecordImp(const ESM::RefId& origin, const ESM::RefId& dest, UniversalId::Type type);
         ///< Returns the index of the clone.
 
-        int touchRecordImp(const std::string& id);
+        int touchRecordImp(const ESM::RefId& id);
         ///< Returns the index of the record on success, -1 on failure.
 
     public:
@@ -227,29 +227,28 @@ namespace CSMWorld
 
     template <typename ESXRecordT>
     int Collection<ESXRecordT>::cloneRecordImp(
-        const std::string& origin, const std::string& destination, UniversalId::Type type)
+        const ESM::RefId& origin, const ESM::RefId& destination, UniversalId::Type type)
     {
         auto copy = std::make_unique<Record<ESXRecordT>>();
-        copy->mModified = getRecord(ESM::RefId::stringRefId(origin)).get();
+        copy->mModified = getRecord(origin).get();
         copy->mState = RecordBase::State_ModifiedOnly;
-        setRecordId(ESM::RefId::stringRefId(destination), copy->get());
+        setRecordId(destination, copy->get());
 
         if (type == UniversalId::Type_Reference)
         {
             CSMWorld::CellRef* ptr = (CSMWorld::CellRef*)&copy->mModified;
             ptr->mRefNum.mIndex = 0;
         }
-        ESM::RefId destinationRefId = ESM::RefId::stringRefId(destination);
+        const int index = getAppendIndex(destination, type);
-        int index = getAppendIndex(destinationRefId, type);
+        insertRecord(std::move(copy), getAppendIndex(destination, type));
-        insertRecord(std::move(copy), getAppendIndex(destinationRefId, type));
 
         return index;
     }
 
     template <typename ESXRecordT>
-    int Collection<ESXRecordT>::touchRecordImp(const std::string& id)
+    int Collection<ESXRecordT>::touchRecordImp(const ESM::RefId& id)
     {
-        int index = getIndex(ESM::RefId::stringRefId(id));
+        const int index = getIndex(id);
         Record<ESXRecordT>& record = *mRecords.at(index);
         if (record.isDeleted())
         {
@@ -269,27 +268,27 @@ namespace CSMWorld
     void Collection<ESXRecordT>::cloneRecord(
         const ESM::RefId& origin, const ESM::RefId& destination, const UniversalId::Type type)
     {
-        cloneRecordImp(origin.getRefIdString(), destination.getRefIdString(), type);
+        cloneRecordImp(origin, destination, type);
     }
 
     template <>
     inline void Collection<Land>::cloneRecord(
         const ESM::RefId& origin, const ESM::RefId& destination, const UniversalId::Type type)
     {
-        int index = cloneRecordImp(origin.getRefIdString(), destination.getRefIdString(), type);
+        const int index = cloneRecordImp(origin, destination, type);
         mRecords.at(index)->get().setPlugin(0);
     }
 
     template <typename ESXRecordT>
     bool Collection<ESXRecordT>::touchRecord(const ESM::RefId& id)
     {
-        return touchRecordImp(id.getRefIdString()) != -1;
+        return touchRecordImp(id) != -1;
     }
 
     template <>
     inline bool Collection<Land>::touchRecord(const ESM::RefId& id)
     {
-        int index = touchRecordImp(id.getRefIdString());
+        const int index = touchRecordImp(id);
         if (index >= 0)
         {
             mRecords.at(index)->get().setPlugin(0);

apps/opencs/model/world/commanddispatcher.cpp

@@ -113,11 +113,7 @@ void CSMWorld::CommandDispatcher::setEditLock(bool locked)
 void CSMWorld::CommandDispatcher::setSelection(const std::vector<std::string>& selection)
 {
     mSelection = selection;
-    for (auto& sel : mSelection)
+    std::sort(mSelection.begin(), mSelection.end(), Misc::StringUtils::CiComp{});
-    {
-        Misc::StringUtils::lowerCaseInPlace(sel);
-    }
-    std::sort(mSelection.begin(), mSelection.end());
 }
 
 void CSMWorld::CommandDispatcher::setExtendedTypes(const std::vector<UniversalId>& types)
@@ -290,8 +286,7 @@ void CSMWorld::CommandDispatcher::executeExtendedDelete()
                 if (record.mState == RecordBase::State_Deleted)
                     continue;
 
-                if (!std::binary_search(mSelection.begin(), mSelection.end(),
+                if (!std::binary_search(mSelection.begin(), mSelection.end(), record.get().mCell))
-                        Misc::StringUtils::lowerCase(record.get().mCell.getRefIdString())))
                     continue;
 
                 macro.push(new CSMWorld::DeleteCommand(model, record.get().mId.getRefIdString()));
@@ -321,8 +316,7 @@ void CSMWorld::CommandDispatcher::executeExtendedRevert()
             {
                 const Record<CellRef>& record = collection.getRecord(i);
 
-                if (!std::binary_search(mSelection.begin(), mSelection.end(),
+                if (!std::binary_search(mSelection.begin(), mSelection.end(), record.get().mCell))
-                        Misc::StringUtils::lowerCase(record.get().mCell.getRefIdString())))
                     continue;
 
                 macro.push(new CSMWorld::RevertCommand(model, record.get().mId.getRefIdString()));

apps/opencs/model/world/data.cpp

@@ -1209,9 +1209,8 @@ bool CSMWorld::Data::continueLoading(CSMDoc::Messages& messages)
                 messages.add(id, "Logic error: cell index out of bounds", "", CSMDoc::Message::Severity_Error);
                 index = mCells.getSize() - 1;
             }
-            const std::string cellId = mCells.getId(index).getRefIdString();
 
-            mRefs.load(*mReader, index, mBase, mRefLoadCache[cellId], messages);
+            mRefs.load(*mReader, index, mBase, mRefLoadCache[mCells.getId(index)], messages);
             break;
         }
 

apps/opencs/model/world/data.hpp

@@ -123,7 +123,7 @@ namespace CSMWorld
         const ESM::Dialogue* mDialogue; // last loaded dialogue
         bool mBase;
         bool mProject;
-        std::map<std::string, std::map<unsigned int, unsigned int>, Misc::StringUtils::CiComp> mRefLoadCache;
+        std::map<ESM::RefId, std::map<unsigned int, unsigned int>> mRefLoadCache;
         int mReaderIndex;
 
         bool mFsStrict;

apps/opencs/model/world/scriptcontext.cpp

@@ -85,7 +85,7 @@ std::pair<char, bool> CSMWorld::ScriptContext::getMemberType(const std::string&
     if (index == -1)
         return std::make_pair(' ', false);
 
-    std::map<std::string, Compiler::Locals>::iterator iter = mLocals.find(id2.getRefIdString());
+    auto iter = mLocals.find(id2);
 
     if (iter == mLocals.end())
     {
@@ -97,7 +97,7 @@ std::pair<char, bool> CSMWorld::ScriptContext::getMemberType(const std::string&
         Compiler::Scanner scanner(errorHandler, stream, getExtensions());
         scanner.scan(parser);
 
-        iter = mLocals.insert(std::make_pair(id2.getRefIdString(), locals)).first;
+        iter = mLocals.emplace(id2, std::move(locals)).first;
     }
 
     return std::make_pair(iter->second.getType(Misc::StringUtils::lowerCase(name)), reference);
@@ -131,7 +131,7 @@ void CSMWorld::ScriptContext::clear()
 
 bool CSMWorld::ScriptContext::clearLocals(const std::string& script)
 {
-    std::map<std::string, Compiler::Locals>::iterator iter = mLocals.find(script);
+    const auto iter = mLocals.find(script);
 
     if (iter != mLocals.end())
     {

apps/opencs/model/world/scriptcontext.hpp

@@ -19,7 +19,7 @@ namespace CSMWorld
         const Data& mData;
         mutable std::vector<ESM::RefId> mIds;
         mutable bool mIdsUpdated;
-        mutable std::map<std::string, Compiler::Locals, Misc::StringUtils::CiComp> mLocals;
+        mutable std::map<ESM::RefId, Compiler::Locals, std::less<>> mLocals;
 
     public:
         ScriptContext(const Data& data);

apps/opencs/view/render/actor.cpp

@@ -104,9 +104,8 @@ namespace CSVRender
     {
         for (int i = 0; i < ESM::PRT_Count; ++i)
         {
-            auto type = (ESM::PartReferenceType)i;
+            const auto type = static_cast<ESM::PartReferenceType>(i);
-            const std::string_view partId = mActorData->getPart(type);
+            attachBodyPart(type, getBodyPartMesh(mActorData->getPart(type)));
-            attachBodyPart(type, getBodyPartMesh(partId));
         }
     }
 
@@ -124,11 +123,11 @@ namespace CSVRender
         }
     }
 
-    std::string Actor::getBodyPartMesh(std::string_view bodyPartId)
+    std::string Actor::getBodyPartMesh(const ESM::RefId& bodyPartId)
     {
         const auto& bodyParts = mData.getBodyParts();
 
-        const int index = bodyParts.searchId(ESM::RefId::stringRefId(bodyPartId));
+        const int index = bodyParts.searchId(bodyPartId);
         if (index != -1 && !bodyParts.getRecord(index).isDeleted())
             return MeshPrefix + bodyParts.getRecord(index).get().mModel;
         else

apps/opencs/view/render/actor.hpp

@@ -51,7 +51,7 @@ namespace CSVRender
         void loadBodyParts();
         void attachBodyPart(ESM::PartReferenceType, const std::string& mesh);
 
-        std::string getBodyPartMesh(std::string_view bodyPartId);
+        std::string getBodyPartMesh(const ESM::RefId& bodyPartId);
 
         static const std::string MeshPrefix;
 

apps/opencs/view/render/object.cpp

@@ -737,7 +737,7 @@ void CSVRender::Object::apply(CSMWorld::CommandMacro& commands)
         // Do cell check first so positions can be compared
         const CSMWorld::CellRef& ref = collection.getRecord(recordIndex).get();
 
-        if (CSMWorld::CellCoordinates::isExteriorCell(ref.mCell.getRefIdString()))
+        if (CSMWorld::CellCoordinates::isExteriorCell(ref.mCell))
         {
             // Find cell index at new position
             std::pair<int, int> cellIndex

apps/openmw/mwworld/globals.cpp

@@ -37,7 +37,7 @@ namespace MWWorld
 
         for (const ESM::Global& esmGlobal : globals)
         {
-            mVariables.emplace(esmGlobal.mId.getRefIdString(), esmGlobal);
+            mVariables.emplace(esmGlobal.mId, esmGlobal);
         }
     }
 
@@ -98,8 +98,7 @@ namespace MWWorld
             // Deleted globals can't appear there, so isDeleted will be ignored here.
             global.load(reader, isDeleted);
 
-            Collection::iterator iter = mVariables.find(global.mId.getRefIdString());
+            if (const auto iter = mVariables.find(global.mId); iter != mVariables.end())
-            if (iter != mVariables.end())
                 iter->second = global;
 
             return true;

apps/openmw/mwworld/globals.hpp

@@ -2,8 +2,8 @@
 #define GAME_MWWORLD_GLOBALS_H
 
 #include <cstdint>
+#include <map>
 #include <string>
-#include <unordered_map>
 #include <vector>
 
 #include <components/esm3/loadglob.hpp>
@@ -29,8 +29,7 @@ namespace MWWorld
     class Globals
     {
     private:
-        using Collection
+        using Collection = std::map<ESM::RefId, ESM::Global, std::less<>>;
-            = std::unordered_map<std::string, ESM::Global, Misc::StringUtils::CiHash, Misc::StringUtils::CiEqual>;
 
         Collection mVariables; // type, value
 

apps/openmw_test_suite/CMakeLists.txt

@@ -23,6 +23,7 @@ file(GLOB UNITTEST_SRC_FILES
 
     esm/test_fixed_string.cpp
     esm/variant.cpp
+    esm/testrefid.cpp
 
     lua/test_lua.cpp
     lua/test_scriptscontainer.cpp

apps/openmw_test_suite/esm/testrefid.cpp

@@ -0,0 +1,148 @@
+#include <components/esm/refid.hpp>
+
+#include <gtest/gtest.h>
+
+#include <functional>
+#include <map>
+#include <string>
+
+namespace ESM
+{
+    namespace
+    {
+        using namespace ::testing;
+
+        TEST(ESMRefIdTest, defaultConstructedIsEmpty)
+        {
+            const RefId refId;
+            EXPECT_TRUE(refId.empty());
+        }
+
+        TEST(ESMRefIdTest, stringRefIdIsNotEmpty)
+        {
+            const RefId refId = RefId::stringRefId("ref_id");
+            EXPECT_FALSE(refId.empty());
+        }
+
+        TEST(ESMRefIdTest, formIdRefIdIsNotEmpty)
+        {
+            const RefId refId = RefId::formIdRefId(42);
+            EXPECT_FALSE(refId.empty());
+        }
+
+        TEST(ESMRefIdTest, defaultConstructedIsEqualToItself)
+        {
+            const RefId refId;
+            EXPECT_EQ(refId, refId);
+        }
+
+        TEST(ESMRefIdTest, defaultConstructedIsEqualToDefaultConstructed)
+        {
+            const RefId a;
+            const RefId b;
+            EXPECT_EQ(a, b);
+        }
+
+        TEST(ESMRefIdTest, defaultConstructedIsNotEqualToDebugStringRefId)
+        {
+            const RefId a;
+            const RefId b = RefId::stringRefId("b");
+            EXPECT_NE(a, b);
+        }
+
+        TEST(ESMRefIdTest, defaultConstructedIsNotEqualToFormIdRefId)
+        {
+            const RefId a;
+            const RefId b = RefId::formIdRefId(42);
+            EXPECT_NE(a, b);
+        }
+
+        TEST(ESMRefIdTest, defaultConstructedIsNotEqualToDebugStringLiteral)
+        {
+            const RefId a;
+            EXPECT_NE(a, "foo");
+        }
+
+        TEST(ESMRefIdTest, stringRefIdIsEqualToTheSameStringLiteralValue)
+        {
+            const RefId refId = RefId::stringRefId("ref_id");
+            EXPECT_EQ(refId, "ref_id");
+        }
+
+        TEST(ESMRefIdTest, stringRefIdIsCaseInsensitiveEqualToTheSameStringLiteralValue)
+        {
+            const RefId refId = RefId::stringRefId("ref_id");
+            EXPECT_EQ(refId, "REF_ID");
+        }
+
+        TEST(ESMRefIdTest, stringRefIdIsEqualToTheSameStringRefId)
+        {
+            const RefId a = RefId::stringRefId("ref_id");
+            const RefId b = RefId::stringRefId("ref_id");
+            EXPECT_EQ(a, b);
+        }
+
+        TEST(ESMRefIdTest, stringRefIdIsCaseInsensitiveEqualToTheSameStringRefId)
+        {
+            const RefId lower = RefId::stringRefId("ref_id");
+            const RefId upper = RefId::stringRefId("REF_ID");
+            EXPECT_EQ(lower, upper);
+        }
+
+        TEST(ESMRefIdTest, stringRefIdIsEqualToItself)
+        {
+            const RefId refId = RefId::stringRefId("ref_id");
+            EXPECT_EQ(refId, refId);
+        }
+
+        TEST(ESMRefIdTest, stringRefIdIsCaseInsensitiveLessByContent)
+        {
+            const RefId a = RefId::stringRefId("a");
+            const RefId b = RefId::stringRefId("B");
+            EXPECT_LT(a, b);
+        }
+
+        TEST(ESMRefIdTest, stringRefIdHasCaseInsensitiveHash)
+        {
+            const RefId lower = RefId::stringRefId("a");
+            const RefId upper = RefId::stringRefId("A");
+            const std::hash<RefId> hash;
+            EXPECT_EQ(hash(lower), hash(upper));
+        }
+
+        TEST(ESMRefIdTest, hasCaseInsensitiveEqualityWithStringView)
+        {
+            const RefId a = RefId::stringRefId("a");
+            const std::string_view b = "A";
+            EXPECT_EQ(a, b);
+        }
+
+        TEST(ESMRefIdTest, hasCaseInsensitiveLessWithStringView)
+        {
+            const RefId a = RefId::stringRefId("a");
+            const std::string_view b = "B";
+            EXPECT_LT(a, b);
+        }
+
+        TEST(ESMRefIdTest, hasCaseInsensitiveStrongOrderWithStringView)
+        {
+            const RefId a = RefId::stringRefId("a");
+            const std::string_view b = "B";
+            const RefId c = RefId::stringRefId("c");
+            EXPECT_LT(a, b);
+            EXPECT_LT(b, c);
+        }
+
+        TEST(ESMRefIdTest, canBeUsedAsMapKeyWithLookupByStringView)
+        {
+            const std::map<ESM::RefId, int, std::less<>> map({ { ESM::RefId::stringRefId("a"), 42 } });
+            EXPECT_EQ(map.count("A"), 1);
+        }
+
+        TEST(ESMRefIdTest, canBeUsedAsLookupKeyForMapWithStringKey)
+        {
+            const std::map<std::string, int, std::less<>> map({ { "a", 42 } });
+            EXPECT_EQ(map.count(ESM::RefId::stringRefId("A")), 1);
+        }
+    }
+}

apps/openmw_test_suite/mwscript/test_utils.hpp

@@ -144,7 +144,7 @@ namespace
     class TestInterpreterContext : public Interpreter::Context
     {
         LocalVariables mLocals;
-        std::map<std::string, GlobalVariables, Misc::StringUtils::CiComp> mMembers;
+        std::map<ESM::RefId, GlobalVariables> mMembers;
 
     public:
         const ESM::RefId& getTarget() const override { return ESM::RefId::sEmpty; }
@@ -209,7 +209,7 @@ namespace
 
         int getMemberShort(const ESM::RefId& id, std::string_view name, bool global) const override
         {
-            auto it = mMembers.find(id.getRefIdString());
+            auto it = mMembers.find(id);
             if (it != mMembers.end())
                 return it->second.getShort(name);
             return {};
@@ -217,7 +217,7 @@ namespace
 
         int getMemberLong(const ESM::RefId& id, std::string_view name, bool global) const override
         {
-            auto it = mMembers.find(id.getRefIdString());
+            auto it = mMembers.find(id);
             if (it != mMembers.end())
                 return it->second.getLong(name);
             return {};
@@ -225,7 +225,7 @@ namespace
 
         float getMemberFloat(const ESM::RefId& id, std::string_view name, bool global) const override
         {
-            auto it = mMembers.find(id.getRefIdString());
+            auto it = mMembers.find(id);
             if (it != mMembers.end())
                 return it->second.getFloat(name);
             return {};
@@ -233,17 +233,17 @@ namespace
 
         void setMemberShort(const ESM::RefId& id, std::string_view name, int value, bool global) override
         {
-            mMembers[id.getRefIdString()].setShort(name, value);
+            mMembers[id].setShort(name, value);
         }
 
         void setMemberLong(const ESM::RefId& id, std::string_view name, int value, bool global) override
         {
-            mMembers[id.getRefIdString()].setLong(name, value);
+            mMembers[id].setLong(name, value);
         }
 
         void setMemberFloat(const ESM::RefId& id, std::string_view name, float value, bool global) override
         {
-            mMembers[id.getRefIdString()].setFloat(name, value);
+            mMembers[id].setFloat(name, value);
         }
     };
 

components/esm/refid.cpp

@@ -16,9 +16,14 @@ namespace ESM
         return Misc::StringUtils::ciLess(mId, rhs.mId);
     }
 
-    bool RefId::operator<(std::string_view rhs) const
+    bool operator<(const RefId& lhs, std::string_view rhs)
     {
-        return Misc::StringUtils::ciLess(mId, rhs);
+        return Misc::StringUtils::ciLess(lhs.mId, rhs);
+    }
+
+    bool operator<(std::string_view lhs, const RefId& rhs)
+    {
+        return Misc::StringUtils::ciLess(lhs, rhs.mId);
     }
 
     std::ostream& operator<<(std::ostream& os, const RefId& refId)

components/esm/refid.hpp

@@ -39,7 +39,9 @@ namespace ESM
 
         bool operator<(const RefId& rhs) const;
 
-        bool operator<(std::string_view rhs) const;
+        friend bool operator<(const RefId& lhs, std::string_view rhs);
+
+        friend bool operator<(std::string_view lhs, const RefId& rhs);
 
         friend std::ostream& operator<<(std::ostream& os, const RefId& dt);