Greatly improved how the variant on MWWorld::CellRef works

apps/openmw/mwworld/cellref.cpp

@@ -7,28 +7,17 @@
 
 namespace MWWorld
 {
+    // makes it easier to use std visit with a variant
+    template <class... Ts>
+    struct RefVisit : Ts...
+    {
+        using Ts::operator()...;
+    };
+
     CellRef::CellRef(const ESM::CellRef& ref)
         : mCellRef(ESM::ReferenceVariant(ref))
     {
         mChanged = false;
-        mSoul = ref.mSoul;
-        mTrap = ref.mTrap;
-        mKey = ref.mKey;
-        mFaction = ref.mFaction;
-        mOwner = ref.mOwner;
-        mReferenceType = ref.mRefID;
-        mPos = ref.mPos;
-        mDoorDest = ref.mDoorDest;
-        mRefNum = ref.mRefNum;
-        mGlobalVariable = ref.mGlobalVariable;
-        mDestCell = ref.mDestCell;
-
-        mLockLevel = ref.mLockLevel;
-        mGoldValue = ref.mGoldValue;
-        mFactionRank = ref.mFactionRank;
-        mEnchantmentCharge = ref.mEnchantmentCharge;
-
-        mScale = ref.mScale;
     }
 
     CellRef::CellRef(const ESM4::Reference& ref)
@@ -36,25 +25,23 @@ namespace MWWorld
     {
 
         mChanged = false;
+    }
 
-        mReferenceType = ref.mBaseObj;
+    static const ESM::RefNum emptyRefNum = {};
-        mPos = { { ref.mPlacement.pos.x, ref.mPlacement.pos.y, ref.mPlacement.pos.z },
-            { ref.mPlacement.rot.x, ref.mPlacement.rot.y, ref.mPlacement.rot.z } };
 
-        mRefNum = {};
+    const ESM::RefNum& CellRef::getRefNum() const
-        mDoorDest = {};
+    {
-
+        return std::visit(RefVisit{ [&](const ESM4::Reference& ref) -> const ESM::RefNum& { return emptyRefNum; },
-        mLockLevel = ref.mLockLevel;
+                              [&](const ESM::CellRef& ref) -> const ESM::RefNum& { return ref.mRefNum; } },
-        mFactionRank = ref.mFactionRank;
+            mCellRef.mVariant);
-        mGoldValue = 0;
-        mEnchantmentCharge = 0;
-
-        mScale = ref.mScale;
     }
 
     const ESM::RefNum& CellRef::getOrAssignRefNum(ESM::RefNum& lastAssignedRefNum)
     {
-        if (!mRefNum.isSet())
+        return std::visit(
+            RefVisit{ [&](ESM4::Reference& ref) -> const ESM::RefNum& { return emptyRefNum; },
+                [&](ESM::CellRef& ref) -> const ESM::RefNum& {
+                    if (!ref.mRefNum.isSet())
                     {
                         // Generated RefNums have negative mContentFile
                         assert(lastAssignedRefNum.mContentFile < 0);
@@ -66,38 +53,47 @@ namespace MWWorld
                             else
                                 Log(Debug::Error) << "RefNum counter overflow in CellRef::getOrAssignRefNum";
                         }
-            mRefNum = lastAssignedRefNum;
+                        ref.mRefNum = lastAssignedRefNum;
                         mChanged = true;
                     }
-        if (!mCellRef.isESM4())
+                    return ref.mRefNum;
-            mCellRef.getEsm3().mRefNum = mRefNum;
+                } },
-        return mRefNum;
+            mCellRef.mVariant);
     }
 
     void CellRef::unsetRefNum()
     {
-        mRefNum = ESM::RefNum{};
+        std::visit(RefVisit{ [&](ESM4::Reference& ref) {}, [&](ESM::CellRef& ref) { ref.mRefNum = emptyRefNum; } },
-        if (!mCellRef.isESM4())
+            mCellRef.mVariant);
-            mCellRef.getEsm3().mRefNum = mRefNum;
+    }
+
+    static const std::string emptyString = "";
+
+    const std::string& CellRef::getDestCell() const
+    {
+        return mCellRef.isESM4() ? emptyString : mCellRef.getEsm3().mDestCell;
     }
 
     void CellRef::setScale(float scale)
     {
-        if (scale != mScale)
+        if (scale != getScale())
         {
             mChanged = true;
-            mScale = scale;
+            std::visit([scale](auto&& ref) { ref.mScale = scale; }, mCellRef.mVariant);
         }
-        if (!mCellRef.isESM4())
-            mCellRef.getEsm3().mScale = Scale;
     }
 
     void CellRef::setPosition(const ESM::Position& position)
     {
         mChanged = true;
-        mPos = position;
+        std::visit([&position](auto&& ref) { ref.mPos = position; }, mCellRef.mVariant);
-        if (!mCellRef.isESM4())
+    }
-            mCellRef.getEsm3().mPos = position;
+
+    float CellRef::getEnchantmentCharge() const
+    {
+        return std::visit(RefVisit{ [&](const ESM4::Reference& ref) { return 0.f; },
+                              [&](const ESM::CellRef& ref) { return ref.mEnchantmentCharge; } },
+            mCellRef.mVariant);
     }
 
     float CellRef::getNormalizedEnchantmentCharge(int maxCharge) const
@@ -106,50 +102,43 @@ namespace MWWorld
         {
             return 0;
         }
-        else if (mEnchantmentCharge == -1)
+        else if (getEnchantmentCharge() == -1)
         {
             return 1;
         }
         else
         {
-            return mEnchantmentCharge / static_cast<float>(maxCharge);
+            return getEnchantmentCharge() / static_cast<float>(maxCharge);
         }
     }
 
     void CellRef::setEnchantmentCharge(float charge)
     {
-        if (charge != mEnchantmentCharge)
+        if (charge != getEnchantmentCharge())
         {
             mChanged = true;
-            mEnchantmentCharge = charge;
+
+            std::visit(
+                RefVisit{ [&](ESM4::Reference& ref) {}, [&](ESM::CellRef& ref) { ref.mEnchantmentCharge = charge; } },
+                mCellRef.mVariant);
         }
-        if (!mCellRef.isESM4())
-            mCellRef.getEsm3().mEnchantmentCharge = mEnchantmentCharge;
     }
 
     void CellRef::setCharge(int charge)
     {
-        if (mCellRef.isESM4())
+        std::visit(RefVisit{ [&](ESM4::Reference& ref) {}, [&](ESM::CellRef& ref) { ref.mChargeInt = charge; } },
-            return;
+            mCellRef.mVariant);
-
-        auto& cellRef3 = mCellRef.getEsm3();
-        if (charge != cellRef3.mChargeInt)
-        {
-            mChanged = true;
-            cellRef3.mChargeInt = charge;
-        }
     }
 
     void CellRef::applyChargeRemainderToBeSubtracted(float chargeRemainder)
     {
-        if (mCellRef.isESM4())
+        std::visit(RefVisit{ [&](ESM4::Reference& ref) {},
-            return;
+                       [&](ESM::CellRef& cellRef3) {
-
-        auto& cellRef3 = mCellRef.getEsm3();
                            cellRef3.mChargeIntRemainder += std::abs(chargeRemainder);
                            if (cellRef3.mChargeIntRemainder > 1.0f)
                            {
-            float newChargeRemainder = (cellRef3.mChargeIntRemainder - std::floor(cellRef3.mChargeIntRemainder));
+                               float newChargeRemainder
+                                   = (cellRef3.mChargeIntRemainder - std::floor(cellRef3.mChargeIntRemainder));
                                if (cellRef3.mChargeInt <= static_cast<int>(cellRef3.mChargeIntRemainder))
                                {
                                    cellRef3.mChargeInt = 0;
@@ -160,86 +149,80 @@ namespace MWWorld
                                }
                                cellRef3.mChargeIntRemainder = newChargeRemainder;
                            }
+                       } },
+            mCellRef.mVariant);
     }
 
     void CellRef::setChargeFloat(float charge)
     {
-        if (mCellRef.isESM4())
+        std::visit(RefVisit{ [&](ESM4::Reference& ref) {}, [&](ESM::CellRef& ref) { ref.mChargeFloat = charge; } },
-            return;
+            mCellRef.mVariant);
-
-        auto& cellRef3 = mCellRef.getEsm3();
-        if (charge != cellRef3.mChargeFloat)
-        {
-            mChanged = true;
-            cellRef3.mChargeFloat = charge;
     }
+
+    const std::string& CellRef::getGlobalVariable() const
+    {
+
+        return std::visit(RefVisit{ [&](const ESM4::Reference& ref) -> const std::string& { return emptyString; },
+                              [&](const ESM::CellRef& ref) -> const std::string& { return ref.mGlobalVariable; } },
+            mCellRef.mVariant);
     }
 
     void CellRef::resetGlobalVariable()
     {
-        if (!mGlobalVariable.empty())
+        if (!getGlobalVariable().empty())
         {
             mChanged = true;
-            mGlobalVariable.erase();
+            std::visit(
+                RefVisit{ [&](ESM4::Reference& ref) {}, [&](ESM::CellRef& ref) { ref.mGlobalVariable.erase(); } },
+                mCellRef.mVariant);
         }
-
-        if (!mCellRef.isESM4())
-            mCellRef.getEsm3().mGlobalVariable = mGlobalVariable;
     }
 
     void CellRef::setFactionRank(int factionRank)
     {
-        if (factionRank != mFactionRank)
+        if (factionRank != getFactionRank())
         {
             mChanged = true;
-            mFactionRank = factionRank;
+            std::visit([&](auto&& ref) { ref.mFactionRank = factionRank; }, mCellRef.mVariant);
         }
-        if (!mCellRef.isESM4())
-            mCellRef.getEsm3().mFactionRank = mFactionRank;
     }
 
     void CellRef::setOwner(const ESM::RefId& owner)
     {
-        if (owner != mOwner)
+        if (owner != getOwner())
         {
-            mChanged = true;
+            std::visit(RefVisit{ [&](ESM4::Reference& ref) {}, [&](ESM::CellRef& ref) { ref.mOwner = owner; } },
-            mOwner = owner;
+                mCellRef.mVariant);
         }
-        if (!mCellRef.isESM4())
-            mCellRef.getEsm3().mOwner = mOwner;
     }
 
     void CellRef::setSoul(const ESM::RefId& soul)
     {
-        if (soul != mSoul)
+        if (soul != getSoul())
         {
             mChanged = true;
-            mSoul = soul;
+            std::visit(RefVisit{ [&](ESM4::Reference& ref) {}, [&](ESM::CellRef& ref) { ref.mSoul = soul; } },
+                mCellRef.mVariant);
         }
-        if (!mCellRef.isESM4())
-            mCellRef.getEsm3().mSoul = mSoul;
     }
 
     void CellRef::setFaction(const ESM::RefId& faction)
     {
-        if (faction != mFaction)
+        if (faction != getFaction())
         {
             mChanged = true;
-            mFaction = faction;
+            std::visit(RefVisit{ [&](ESM4::Reference& ref) {}, [&](ESM::CellRef& ref) { ref.mFaction = faction; } },
+                mCellRef.mVariant);
         }
-        if (!mCellRef.isESM4())
-            mCellRef.getEsm3().mFaction = mFaction;
     }
 
     void CellRef::setLockLevel(int lockLevel)
     {
-        if (lockLevel != mLockLevel)
+        if (lockLevel != getLockLevel())
         {
             mChanged = true;
-            mLockLevel = lockLevel;
+            std::visit([&](auto&& ref) { ref.mLockLevel = lockLevel; }, mCellRef.mVariant);
         }
-        if (!mCellRef.isESM4())
-            mCellRef.getEsm3().mLockLevel = mLockLevel;
     }
 
     void CellRef::lock(int lockLevel)
@@ -252,29 +235,27 @@ namespace MWWorld
 
     void CellRef::unlock()
     {
-        setLockLevel(-abs(mLockLevel)); // Makes lockLevel negative
+        setLockLevel(-abs(getLockLevel())); // Makes lockLevel negative
     }
 
     void CellRef::setTrap(const ESM::RefId& trap)
     {
-        if (trap != mTrap)
+        if (trap != getTrap())
         {
             mChanged = true;
-            mTrap = trap;
+            std::visit(RefVisit{ [&](ESM4::Reference& ref) {}, [&](ESM::CellRef& ref) { ref.mTrap = trap; } },
+                mCellRef.mVariant);
         }
-        if (!mCellRef.isESM4())
-            mCellRef.getEsm3().mTrap = mTrap;
     }
 
     void CellRef::setGoldValue(int value)
     {
-        if (value != mGoldValue)
+        if (value != getGoldValue())
         {
             mChanged = true;
-            mGoldValue = value;
+            std::visit(RefVisit{ [&](ESM4::Reference& ref) {}, [&](ESM::CellRef& ref) { ref.mGoldValue = value; } },
+                mCellRef.mVariant);
         }
-        if (!mCellRef.isESM4())
-            mCellRef.getEsm3().mGoldValue = mGoldValue;
     }
 
     void CellRef::writeState(ESM::ObjectState& state) const
@@ -285,5 +266,4 @@ namespace MWWorld
             state.mRef = cellRef3;
         }
     }
-
 }

apps/openmw/mwworld/cellref.hpp

@@ -18,13 +18,14 @@ namespace MWWorld
     /// \brief Encapsulated variant of ESM::CellRef with change tracking
     class CellRef
     {
+    protected:
     public:
         CellRef(const ESM::CellRef& ref);
 
         CellRef(const ESM4::Reference& ref);
 
         // Note: Currently unused for items in containers
-        const ESM::RefNum& getRefNum() const { return mRefNum; }
+        const ESM::RefNum& getRefNum() const;
 
         // Returns RefNum.
         // If RefNum is not set, assigns a generated one and changes the "lastAssignedRefNum" counter.
@@ -34,32 +35,44 @@ namespace MWWorld
         void unsetRefNum();
 
         /// Does the RefNum have a content file?
-        bool hasContentFile() const { return mRefNum.hasContentFile(); }
+        bool hasContentFile() const { return getRefNum().hasContentFile(); }
 
         // Id of object being referenced
-        const ESM::RefId& getRefId() const { return mReferenceType; }
+        const ESM::RefId& getRefId() const
+        {
+            return mCellRef.isESM4() ? mCellRef.getEsm4().mBaseObj : mCellRef.getEsm3().mRefID;
+        }
 
         // For doors - true if this door teleports to somewhere else, false
         // if it should open through animation.
         bool getTeleport() const { return mCellRef.isESM4() ? false : mCellRef.getEsm3().mTeleport; }
 
         // Teleport location for the door, if this is a teleporting door.
-        const ESM::Position& getDoorDest() const { return mDoorDest; }
+        const ESM::Position& getDoorDest() const
+        {
+            return mCellRef.isESM4() ? mCellRef.getEsm4().mDoor.destPos : mCellRef.getEsm3().mDoorDest;
+        }
 
         // Destination cell for doors (optional)
-        const std::string& getDestCell() const { return mDestCell; }
+        const std::string& getDestCell() const;
 
         // Scale applied to mesh
-        float getScale() const { return mScale; }
+        float getScale() const
+        {
+            return std::visit([&](auto&& ref) { return ref.mScale; }, mCellRef.mVariant);
+        }
         void setScale(float scale);
 
         // The *original* position and rotation as it was given in the Construction Set.
         // Current position and rotation of the object is stored in RefData.
-        const ESM::Position& getPosition() const { return mPos; }
+        const ESM::Position& getPosition() const
+        {
+            return std::visit([](auto&& ref) -> const ESM::Position& { return ref.mPos; }, mCellRef.mVariant);
+        }
         void setPosition(const ESM::Position& position);
 
         // Remaining enchantment charge. This could be -1 if the charge was not touched yet (i.e. full).
-        float getEnchantmentCharge() const { return mEnchantmentCharge; }
+        float getEnchantmentCharge() const;
 
         // Remaining enchantment charge rescaled to the supplied maximum charge (such as one of the enchantment).
         float getNormalizedEnchantmentCharge(int maxCharge) const;
@@ -79,43 +92,58 @@ namespace MWWorld
         void applyChargeRemainderToBeSubtracted(float chargeRemainder); // Stores remainders and applies if > 1
 
         // The NPC that owns this object (and will get angry if you steal it)
-        const ESM::RefId& getOwner() const { return mOwner; }
+        const ESM::RefId& getOwner() const
+        {
+            return mCellRef.isESM4() ? ESM::RefId::sEmpty : mCellRef.getEsm3().mOwner;
+        }
         void setOwner(const ESM::RefId& owner);
 
         // Name of a global variable. If the global variable is set to '1', using the object is temporarily allowed
         // even if it has an Owner field.
         // Used by bed rent scripts to allow the player to use the bed for the duration of the rent.
-        const std::string& getGlobalVariable() const { return mGlobalVariable; }
+        const std::string& getGlobalVariable() const;
 
         void resetGlobalVariable();
 
         // ID of creature trapped in this soul gem
-        const ESM::RefId& getSoul() const { return mSoul; }
+        const ESM::RefId& getSoul() const { return mCellRef.isESM4() ? ESM::RefId::sEmpty : mCellRef.getEsm3().mSoul; }
         void setSoul(const ESM::RefId& soul);
 
         // The faction that owns this object (and will get angry if
         // you take it and are not a faction member)
-        const ESM::RefId& getFaction() const { return mFaction; }
+        const ESM::RefId& getFaction() const
+        {
+            return mCellRef.isESM4() ? ESM::RefId::sEmpty : mCellRef.getEsm3().mFaction;
+        }
         void setFaction(const ESM::RefId& faction);
 
         // PC faction rank required to use the item. Sometimes is -1, which means "any rank".
         void setFactionRank(int factionRank);
-        int getFactionRank() const { return mFactionRank; }
+        int getFactionRank() const
+        {
+            return std::visit([&](auto&& ref) { return ref.mFactionRank; }, mCellRef.mVariant);
+        }
 
         // Lock level for doors and containers
         // Positive for a locked door. 0 for a door that was never locked.
         // For an unlocked door, it is set to -(previous locklevel)
-        int getLockLevel() const { return mLockLevel; }
+        int getLockLevel() const
+        {
+            return std::visit([](auto&& ref) { return static_cast<int>(ref.mLockLevel); }, mCellRef.mVariant);
+        }
         void setLockLevel(int lockLevel);
         void lock(int lockLevel);
         void unlock();
         // Key and trap ID names, if any
-        const ESM::RefId& getKey() const { return mKey; }
+        const ESM::RefId& getKey() const
-        const ESM::RefId& getTrap() const { return mTrap; }
+        {
+            return std::visit([](auto&& ref) -> const ESM::RefId& { return ref.mKey; }, mCellRef.mVariant);
+        }
+        const ESM::RefId& getTrap() const { return mCellRef.isESM4() ? ESM::RefId::sEmpty : mCellRef.getEsm3().mTrap; }
         void setTrap(const ESM::RefId& trap);
 
         // This is 5 for Gold_005 references, 100 for Gold_100 and so on.
-        int getGoldValue() const { return mGoldValue; }
+        int getGoldValue() const { return mCellRef.isESM4() ? 0 : mCellRef.getEsm3().mGoldValue; }
         void setGoldValue(int value);
 
         // Write the content of this CellRef into the given ObjectState
@@ -127,14 +155,6 @@ namespace MWWorld
     private:
         bool mChanged;
         ESM::ReferenceVariant mCellRef;
-
-        ESM::RefId mSoul, mFaction, mKey, mTrap, mOwner, mReferenceType;
-        float Scale;
-        ESM::Position mPos, mDoorDest;
-        ESM::RefNum mRefNum;
-        std::string mGlobalVariable, mDestCell;
-        int mLockLevel, mGoldValue, mFactionRank, mEnchantmentCharge;
-        float mScale;
     };
 
 }

apps/openmw/mwworld/cellstore.cpp

@@ -840,7 +840,6 @@ namespace MWWorld
 
     void CellStore::loadRefs()
     {
-        assert(mCellVariant.isValid());
         std::map<ESM::RefNum, ESM::RefId> refNumToID; // used to detect refID modifications
 
         std::visit([&refNumToID, this](auto&& cell) { this->loadRefs(*cell, refNumToID); }, mCellVariant.mVariant);

apps/openmw/mwworld/refdata.cpp

@@ -91,8 +91,7 @@ namespace MWWorld
         , mEnabled(true)
         , mPhysicsPostponed(false)
         , mCount(1)
-        , mPosition{ { cellRef.mPlacement.pos.x, cellRef.mPlacement.pos.y, cellRef.mPlacement.pos.z },
+        , mPosition(cellRef.mPos)
-            { cellRef.mPlacement.rot.x, cellRef.mPlacement.rot.y, cellRef.mPlacement.rot.z } }
         , mCustomData(nullptr)
         , mChanged(false)
         , mFlags(0)

components/esm4/loadrefr.cpp

@@ -76,7 +76,7 @@ void ESM4::Reference::load(ESM4::Reader& reader)
                 break;
             }
             case ESM4::SUB_DATA:
-                reader.get(mPlacement);
+                reader.get(mPos);
                 break;
             case ESM4::SUB_XSCL:
                 reader.get(mScale);
@@ -235,7 +235,9 @@ void ESM4::Reference::load(ESM4::Reader& reader)
                 reader.get(dummy);
                 reader.get(dummy);
                 reader.get(dummy);
-                reader.getFormId(mKey);
+                FormId keyForm;
+                reader.getFormId(keyForm);
+                mKey = ESM::RefId::formIdRefId(keyForm);
                 reader.get(dummy); // flag?
                 reader.get(dummy);
                 reader.get(dummy);

components/esm4/loadrefr.hpp

@@ -59,7 +59,7 @@ namespace ESM4
     struct TeleportDest
     {
         FormId destDoor;
-        Placement destPos;
+        ESM::Position destPos;
         std::uint32_t flags; // 0x01 no alarm (only in TES5)
     };
 
@@ -84,7 +84,7 @@ namespace ESM4
         std::string mFullName;
         ESM::RefId mBaseObj;
 
-        Placement mPlacement;
+        ESM::Position mPos;
         float mScale = 1.0f;
         FormId mOwner;
         FormId mGlobal;
@@ -105,7 +105,7 @@ namespace ESM4
         TeleportDest mDoor;
         bool mIsLocked;
         std::int8_t mLockLevel;
-        FormId mKey;
+        ESM::RefId mKey;
 
         FormId mTargetRef;