Merge branch 'exteriorcoc' into 'master'

Improve COC exterior destination choice consistency

See merge request OpenMW/openmw!5026

Author: Capostrophic

apps/openmw/mwworld/store.cpp

@@ -639,20 +639,9 @@ namespace MWWorld
         for (const auto& [_, cell] : mDynamicInt)
             mCells.erase(cell->mId);
         mDynamicInt.clear();
-        setUp();
-    }
-
-    void Store<ESM::Cell>::setUp()
-    {
-        mSharedInt.clear();
-        mSharedInt.reserve(mInt.size());
-        for (auto& [_, cell] : mInt)
-            mSharedInt.push_back(cell);
 
-        mSharedExt.clear();
-        mSharedExt.reserve(mExt.size());
-        for (auto& [_, cell] : mExt)
-            mSharedExt.push_back(cell);
+        mSharedInt.erase(mSharedInt.begin() + mInt.size(), mSharedInt.end());
+        mSharedExt.erase(mSharedExt.begin() + mExt.size(), mSharedExt.end());
     }
     RecordId Store<ESM::Cell>::load(ESM::ESMReader& esm)
     {
@@ -685,7 +674,10 @@ namespace MWWorld
         {
             cell.loadCell(esm, true);
             if (newCell)
+            {
                 mInt[cell.mName] = &cell;
+                mSharedInt.push_back(&cell);
+            }
         }
         else
         {
@@ -698,7 +690,10 @@ namespace MWWorld
             // push the new references on the list of references to manage
             cell.postLoad(esm);
             if (newCell)
+            {
                 mExt[std::make_pair(cell.mData.mX, cell.mData.mY)] = &cell;
+                mSharedExt.push_back(&cell);
+            }
             else
             {
                 // merge lists of leased references, use newer data in case of conflict
@@ -749,38 +744,6 @@ namespace MWWorld
     {
         return iterator(mSharedExt.end());
     }
-    const ESM::Cell* Store<ESM::Cell>::searchExtByName(std::string_view name) const
-    {
-        const ESM::Cell* cell = nullptr;
-        for (const ESM::Cell* sharedCell : mSharedExt)
-        {
-            if (Misc::StringUtils::ciEqual(sharedCell->mName, name))
-            {
-                if (cell == nullptr || (sharedCell->mData.mX > cell->mData.mX)
-                    || (sharedCell->mData.mX == cell->mData.mX && sharedCell->mData.mY > cell->mData.mY))
-                {
-                    cell = sharedCell;
-                }
-            }
-        }
-        return cell;
-    }
-    const ESM::Cell* Store<ESM::Cell>::searchExtByRegion(const ESM::RefId& id) const
-    {
-        const ESM::Cell* cell = nullptr;
-        for (const ESM::Cell* sharedCell : mSharedExt)
-        {
-            if (sharedCell->mRegion == id)
-            {
-                if (cell == nullptr || (sharedCell->mData.mX > cell->mData.mX)
-                    || (sharedCell->mData.mX == cell->mData.mX && sharedCell->mData.mY > cell->mData.mY))
-                {
-                    cell = sharedCell;
-                }
-            }
-        }
-        return cell;
-    }
     size_t Store<ESM::Cell>::getSize() const
     {
         return mSharedInt.size() + mSharedExt.size();

apps/openmw/mwworld/store.hpp

@@ -360,27 +360,10 @@ namespace MWWorld
     template <>
     class Store<ESM::Cell> : public DynamicStore
     {
-        struct DynamicExtCmp
-        {
-            bool operator()(const std::pair<int, int>& left, const std::pair<int, int>& right) const
-            {
-                if (left.first == right.first && left.second == right.second)
-                    return false;
-
-                if (left.first == right.first)
-                    return left.second > right.second;
-
-                // Exterior cells are listed in descending, row-major order,
-                // this is a workaround for an ambiguous chargen_plank reference in the vanilla game.
-                // there is one at -22,16 and one at -2,-9, the latter should be used.
-                return left.first > right.first;
-            }
-        };
-
         typedef std::unordered_map<std::string, ESM::Cell*, Misc::StringUtils::CiHash, Misc::StringUtils::CiEqual>
             DynamicInt;
 
-        typedef std::map<std::pair<int, int>, ESM::Cell*, DynamicExtCmp> DynamicExt;
+        typedef std::map<std::pair<int, int>, ESM::Cell*> DynamicExt;
 
         std::unordered_map<ESM::RefId, ESM::Cell> mCells;
 
@@ -410,7 +393,6 @@ namespace MWWorld
         const ESM::Cell* find(int x, int y) const;
 
         void clearDynamic() override;
-        void setUp() override;
 
         RecordId load(ESM::ESMReader& esm) override;
 
@@ -419,12 +401,6 @@ namespace MWWorld
         iterator extBegin() const;
         iterator extEnd() const;
 
-        // Return the northernmost cell in the easternmost column.
-        const ESM::Cell* searchExtByName(std::string_view id) const;
-
-        // Return the northernmost cell in the easternmost column.
-        const ESM::Cell* searchExtByRegion(const ESM::RefId& id) const;
-
         size_t getSize() const override;
         size_t getExtSize() const;
         size_t getIntSize() const;

apps/openmw/mwworld/worldmodel.cpp

@@ -298,25 +298,29 @@ namespace MWWorld
             return cellStore;
 
         // try named exteriors
-        const ESM::Cell* cell = mStore.get<ESM::Cell>().searchExtByName(name);
+        const ESM::Cell* cell = nullptr;
+        const Store<ESM::Cell>& cells = mStore.get<ESM::Cell>();
+        const Store<ESM::GameSetting>& gmsts = mStore.get<ESM::GameSetting>();
+        const Store<ESM::Region>& regions = mStore.get<ESM::Region>();
+        static const std::string& defaultName = gmsts.find("sDefaultCellname")->mValue.getString();
 
-        if (cell == nullptr)
+        for (auto it = cells.extBegin(); it != cells.extEnd(); ++it)
         {
-            // treat "Wilderness" like an empty string
-            static const std::string& defaultName
-                = mStore.get<ESM::GameSetting>().find("sDefaultCellname")->mValue.getString();
-            if (Misc::StringUtils::ciEqual(name, defaultName))
-                cell = mStore.get<ESM::Cell>().searchExtByName({});
-        }
+            std::string_view resolvedName = defaultName;
+            if (!it->mName.empty())
+                resolvedName = it->mName;
+            else if (!it->mRegion.empty())
+            {
+                const ESM::Region* region = regions.search(it->mRegion);
+                if (region != nullptr)
+                    resolvedName = !region->mName.empty() ? region->mName : region->mId.getRefIdString();
+            }
 
-        if (cell == nullptr)
-        {
-            // now check for regions
-            const Store<ESM::Region>& regions = mStore.get<ESM::Region>();
-            const auto region = std::find_if(regions.begin(), regions.end(),
-                [&](const ESM::Region& v) { return Misc::StringUtils::ciEqual(name, v.mName); });
-            if (region != regions.end())
-                cell = mStore.get<ESM::Cell>().searchExtByRegion(region->mId);
+            if (Misc::StringUtils::ciEqual(resolvedName, name))
+            {
+                cell = &(*it);
+                break;
+            }
         }
 
         if (cell != nullptr)