Applies some review comments.

Proper visit for ESM::CellVariant

Fixed MWWorldCell constructor

apps/openmw/mwbase/world.hpp

@@ -57,8 +57,6 @@ namespace ESM
     struct ItemLevList;
     struct TimeStamp;
     struct RefId;
-    class CellVariant;
-
 }
 
 namespace MWPhysics
@@ -184,6 +182,8 @@ namespace MWBase
         /// generate a name.
         virtual std::string_view getCellName(const MWWorld::Cell& cell) const = 0;
 
+        virtual std::string_view getCellName(const ESM::Cell* cell) const = 0;
+
         virtual void removeRefScript(MWWorld::RefData* ref) = 0;
         //< Remove the script attached to ref from mLocalScripts
 

apps/openmw/mwclass/door.cpp

@@ -307,7 +307,7 @@ namespace MWClass
             const osg::Vec2i index
                 = MWWorld::positionToCellIndex(door.mRef.getDoorDest().pos[0], door.mRef.getDoorDest().pos[1]);
             const ESM::Cell* cell = world->getStore().get<ESM::Cell>().search(index.x(), index.y());
-            dest = world->getCellName(MWWorld::Cell(*cell));
+            dest = world->getCellName(cell);
         }
 
         return "#{sCell=" + std::string{ dest } + "}";

apps/openmw/mwmechanics/aiwander.hpp

@@ -12,7 +12,6 @@
 namespace ESM
 {
     struct Cell;
-    struct CellCommon;
     namespace AiSequence
     {
         struct AiWander;
@@ -159,7 +158,6 @@ namespace MWMechanics
             GroupIndex_MaxIdle = 9
         };
 
-        /// convert point from local (i.e. cell) to world coordinates
         void setCurrentNodeToClosestAllowedNode(AiWanderStorage& storage);
 
         void addNonPathGridAllowedPoints(const ESM::Pathgrid* pathGrid, int pointIndex, AiWanderStorage& storage,

apps/openmw/mwrender/renderingmanager.hpp

@@ -43,7 +43,6 @@ namespace ESM
 {
     struct Cell;
     struct RefNum;
-    class CellVariant;
 }
 
 namespace Terrain

apps/openmw/mwworld/cell.cpp

@@ -8,52 +8,44 @@ namespace MWWorld
 {
     Cell::Cell(const ESM4::Cell& cell)
         : ESM::CellVariant(cell)
+        , mIsExterior(!(cell.mCellFlags & ESM4::CELL_Interior))
+        , mIsQuasiExterior(cell.mCellFlags & ESM4::CELL_QuasiExt)
+        , mHasWater(cell.mCellFlags & ESM4::CELL_HasWater)
+        , mNoSleep(false) // No such notion in ESM4
+        , mGridPos(cell.mX, cell.mY)
+        , mDisplayname(cell.mFullName)
+        , mNameID(cell.mEditorId)
+        , mRegion(ESM::RefId::sEmpty) // Unimplemented for now
+        , mCellId{
+            .mWorldspace{ Misc::StringUtils::lowerCase(cell.mEditorId) },
+            .mIndex{ cell.getGridX(), cell.getGridY() },
+            .mPaged = isExterior(),}
+        ,mMood{
+            .mAmbiantColor = cell.mLighting.ambient,
+            .mDirectionalColor = cell.mLighting.directional,
+            .mFogColor = cell.mLighting.fogColor,
+            .mFogDensity = cell.mLighting.fogPower,}
     {
-
-        mNameID = cell.mEditorId;
-        mDisplayname = cell.mFullName;
-        mGridPos.x() = cell.mX;
-        mGridPos.y() = cell.mY;
-
-        mRegion = ESM::RefId::sEmpty; // Unimplemented for now
-
-        mFlags.mHasWater = cell.mCellFlags & ESM4::CELL_HasWater;
-        mFlags.mIsExterior = !(cell.mCellFlags & ESM4::CELL_Interior);
-        mFlags.mIsQuasiExterior = cell.mCellFlags & ESM4::CELL_QuasiExt;
-        mFlags.mNoSleep = false; // No such notion in ESM4
-
-        mCellId.mWorldspace = Misc::StringUtils::lowerCase(cell.mEditorId);
-        mCellId.mIndex.mX = cell.getGridX();
-        mCellId.mIndex.mX = cell.getGridY();
-        mCellId.mPaged = isExterior();
-
-        mMood.mAmbiantColor = cell.mLighting.ambient;
-        mMood.mFogColor = cell.mLighting.fogColor;
-        mMood.mDirectionalColor = cell.mLighting.directional;
-        mMood.mFogDensity = cell.mLighting.fogPower;
     }
 
     Cell::Cell(const ESM::Cell& cell)
         : ESM::CellVariant(cell)
+        , mIsExterior(!(cell.mData.mFlags & ESM::Cell::Interior))
+        , mIsQuasiExterior(cell.mData.mFlags & ESM::Cell::QuasiEx)
+        , mHasWater(cell.mData.mFlags & ESM::Cell::HasWater)
+        , mNoSleep(cell.mData.mFlags & ESM::Cell::NoSleep)
+        , mGridPos(cell.getGridX(), cell.getGridY())
+        , mDisplayname(cell.mName)
+        , mNameID(cell.mName)
+        , mRegion(ESM::RefId::sEmpty) // Unimplemented for now
+        , mCellId(cell.getCellId())
+        , mMood{
+            .mAmbiantColor = cell.mAmbi.mAmbient,
+            .mDirectionalColor = cell.mAmbi.mSunlight,
+            .mFogColor = cell.mAmbi.mFog,
+            .mFogDensity = cell.mAmbi.mFogDensity,
+        }
     {
-        mNameID = cell.mName;
-        mDisplayname = cell.mName;
-        mGridPos.x() = cell.getGridX();
-        mGridPos.y() = cell.getGridY();
-
-        mRegion = ESM::RefId::sEmpty; // Unimplemented for now
-
-        mFlags.mHasWater = cell.mData.mFlags & ESM::Cell::HasWater;
-        mFlags.mIsExterior = !(cell.mData.mFlags & ESM::Cell::Interior);
-        mFlags.mIsQuasiExterior = cell.mData.mFlags & ESM::Cell::QuasiEx;
-        mFlags.mNoSleep = cell.mData.mFlags & ESM::Cell::NoSleep;
-
-        mCellId = cell.getCellId();
-
-        mMood.mAmbiantColor = cell.mAmbi.mAmbient;
-        mMood.mFogColor = cell.mAmbi.mFog;
-        mMood.mDirectionalColor = cell.mAmbi.mSunlight;
-        mMood.mFogDensity = cell.mAmbi.mFogDensity;
     }
 
     std::string Cell::getDescription() const

apps/openmw/mwworld/cell.hpp

@@ -38,10 +38,10 @@ namespace MWWorld
 
         int getGridX() const { return mGridPos.x(); }
         int getGridY() const { return mGridPos.y(); }
-        bool isExterior() const { return mFlags.mIsExterior; }
+        bool isExterior() const { return mIsExterior; }
-        bool isQuasiExterior() const { return mFlags.mIsQuasiExterior; }
+        bool isQuasiExterior() const { return mIsQuasiExterior; }
-        bool hasWater() const { return mFlags.mHasWater; }
+        bool hasWater() const { return mHasWater; }
-        bool noSleep() const { return mFlags.mNoSleep; }
+        bool noSleep() const { return mNoSleep; }
         const ESM::CellId& getCellId() const { return mCellId; }
         const ESM::RefId& getRegion() const { return mRegion; }
         std::string_view getNameId() const { return mNameID; }
@@ -50,13 +50,10 @@ namespace MWWorld
         const MoodData& getMood() const { return mMood; }
 
     private:
-        struct
+        bool mIsExterior;
-        {
+        bool mIsQuasiExterior;
-            bool mIsExterior;
+        bool mHasWater;
-            bool mIsQuasiExterior;
+        bool mNoSleep;
-            bool mHasWater;
-            bool mNoSleep;
-        } mFlags;
 
         osg::Vec2i mGridPos;
         std::string mDisplayname; // How the game displays it

apps/openmw/mwworld/cellstore.cpp

@@ -709,7 +709,7 @@ namespace MWWorld
         if (mCellVariant.isEsm4())
             return;
 
-        auto cell3 = mCellVariant.getEsm3();
+        const ESM::Cell& cell3 = mCellVariant.getEsm3();
 
         if (cell3.mContextList.empty())
             return; // this is a dynamically generated cell -> skipping.
@@ -834,7 +834,7 @@ namespace MWWorld
     {
         std::map<ESM::RefNum, ESM::RefId> refNumToID; // used to detect refID modifications
 
-        std::visit([&refNumToID, this](auto&& cell) { this->loadRefs(*cell, refNumToID); }, mCellVariant);
+        ESM::visit([&refNumToID, this](auto&& cell) { this->loadRefs(cell, refNumToID); }, mCellVariant);
 
         updateMergedRefs();
     }
@@ -1077,13 +1077,13 @@ namespace MWWorld
         }
     }
 
-    struct Visitor
+    struct IsEqualVisitor
     {
-        bool operator()(const ESM::Cell* a, const ESM::Cell* b) const { return a->getCellId() == b->getCellId(); }
+        bool operator()(const ESM::Cell& a, const ESM::Cell& b) const { return a.getCellId() == b.getCellId(); }
-        bool operator()(const ESM4::Cell* a, const ESM4::Cell* b) const { return a->mId == b->mId; }
+        bool operator()(const ESM4::Cell& a, const ESM4::Cell& b) const { return a.mId == b.mId; }
 
         template <class L, class R>
-        bool operator()(L, R) const
+        bool operator()(const L&, const R&) const
         {
             return false;
         }
@@ -1091,7 +1091,7 @@ namespace MWWorld
 
     bool CellStore::operator==(const CellStore& right) const
     {
-        return std::visit(Visitor{}, this->mCellVariant, right.mCellVariant);
+        return ESM::visit(IsEqualVisitor(), this->mCellVariant, right.mCellVariant);
     }
 
     void CellStore::setFog(std::unique_ptr<ESM::FogState>&& fog)

apps/openmw/mwworld/cellstore.hpp

@@ -9,7 +9,6 @@
 #include <string_view>
 #include <tuple>
 #include <typeinfo>
-#include <variant>
 #include <vector>
 
 #include "cell.hpp"

apps/openmw/mwworld/scene.cpp

@@ -432,7 +432,7 @@ namespace MWWorld
 
         if (!cellVariant.isEsm4())
         {
-            auto cell3 = cellVariant.getEsm3();
+            const ESM::Cell& cell3 = cellVariant.getEsm3();
             if (const auto pathgrid = mWorld.getStore().get<ESM::Pathgrid>().search(cell3))
                 mNavigator.addPathgrid(cell3, *pathgrid);
         }

apps/openmw/mwworld/worldimp.cpp

@@ -644,11 +644,21 @@ namespace MWWorld
 
         if (!cell.isEsm4())
         {
-            const ESM::Region* region = mStore.get<ESM::Region>().search(cell.getEsm3().mRegion);
+            return getCellName(&cell.getEsm3());
-            if (region)
+        }
+        return mStore.get<ESM::GameSetting>().find("sDefaultCellname")->mValue.getString();
+    }
+
+    std::string_view World::getCellName(const ESM::Cell* cell) const
+    {
+        if (cell)
+        {
+            if (!cell->isExterior() || !cell->mName.empty())
+                return cell->mName;
+
+            if (const ESM::Region* region = mStore.get<ESM::Region>().search(cell->mRegion))
                 return region->mName;
         }
-
         return mStore.get<ESM::GameSetting>().find("sDefaultCellname")->mValue.getString();
     }
 
@@ -3244,7 +3254,7 @@ namespace MWWorld
         }
         else
         {
-            auto cellVariant = *cell->getCell();
+            const MWWorld::Cell& cellVariant = *cell->getCell();
             uint32_t ambient = cellVariant.getMood().mAmbiantColor;
             int ambientTotal = (ambient & 0xff) + ((ambient >> 8) & 0xff) + ((ambient >> 16) & 0xff);
             return !cell->getCell()->noSleep() && ambientTotal <= 201;

apps/openmw/mwworld/worldimp.hpp

@@ -270,6 +270,7 @@ namespace MWWorld
         /// \note If cell==0, the cell the player is currently in will be used instead to
         /// generate a name.
         std::string_view getCellName(const MWWorld::Cell& cell) const override;
+        std::string_view getCellName(const ESM::Cell* cell) const override;
 
         void removeRefScript(MWWorld::RefData* ref) override;
         //< Remove the script attached to ref from mLocalScripts

components/esm/esmbridge.hpp

@@ -38,21 +38,8 @@ namespace ESM
         const ESM4::Cell& getEsm4() const;
         const ESM::Cell& getEsm3() const;
 
-        template <class F>
+        template <class F, class... T>
-        auto visit(F&& f) const
+        friend auto visit(F&& f, T&&... v);
-        {
-            return std::visit(f, mVariant);
-        }
-        template <class F>
-        auto visit(F&& f)
-        {
-            return std::visit(f, mVariant);
-        }
-        template <class F>
-        auto visit(F&& f, const CellVariant& v2) const
-        {
-            return std::visit(f, mVariant, v2.mVariant);
-        }
     };
 
     struct ReferenceVariant
@@ -77,24 +64,11 @@ namespace ESM
         ESM::CellRef& getEsm3() { return std::get<ESM::CellRef>(mVariant); }
         ESM4::Reference& getEsm4() { return std::get<ESM4::Reference>(mVariant); }
     };
-}
 
-namespace std
+    template <class F, class... T>
-{
+    auto visit(F&& f, T&&... v)
-    template <class F>
-    auto visit(F&& f, const ESM::CellVariant& v)
     {
-        return v.visit(f);
+        return std::visit([&](auto*... ptr) { return std::forward<F>(f)(*ptr...); }, std::forward<T>(v).mVariant...);
-    }
-    template <class F>
-    auto visit(F&& f, ESM::CellVariant& v)
-    {
-        return v.visit(f);
-    }
-    template <class F>
-    auto visit(F&& f, const ESM::CellVariant& v1, const ESM::CellVariant& v2)
-    {
-        return v1.visit(f, v2);
     }
 }
 #endif

components/esm4/loadcell.cpp

@@ -39,7 +39,6 @@
 // #include "writer.hpp"
 
 #include <components/esm/refid.hpp>
-#include <components/misc/algorithm.hpp>
 
 // TODO: Try loading only EDID and XCLC (along with mFormId, mFlags and mParent)
 //