Indicate moved cell refs explicitly

This is less error prone approach than use of MovedCellRef fields.

Also make separate functions for skipping and reading moved cell refs to avoid
passing special flags  logic and null pointers for unused arguments.

apps/opencs/model/world/refcollection.cpp

@@ -19,8 +19,9 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool
     ESM::MovedCellRef mref;
     mref.mRefNum.mIndex = 0;
     bool isDeleted = false;
+    bool isMoved = false;
 
-    while (ESM::Cell::getNextRef(reader, ref, isDeleted, true, &mref))
+    while (ESM::Cell::getNextRef(reader, ref, isDeleted, mref, isMoved))
     {
         // Keep mOriginalCell empty when in modified (as an indicator that the
         // original cell will always be equal the current cell).
@@ -34,7 +35,7 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool
             ref.mCell = "#" + std::to_string(index.first) + " " + std::to_string(index.second);
 
             // Handle non-base moved references
-            if (!base && mref.mRefNum.mIndex != 0)
+            if (!base && !isMoved)
             {
                 // Moved references must have a link back to their original cell
                 // See discussion: https://forum.openmw.org/viewtopic.php?f=6&t=577&start=30
@@ -59,8 +60,6 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool
         else
             ref.mCell = cell2.mId;
 
-        mref.mRefNum.mIndex = 0;
-
         // ignore content file number
         std::map<ESM::RefNum, std::string>::iterator iter = cache.begin();
         unsigned int thisIndex = ref.mRefNum.mIndex & 0x00ffffff;

apps/openmw/mwrender/objectpaging.cpp

@@ -427,13 +427,11 @@ namespace MWRender
                         ESM::MovedCellRef cMRef;
                         cMRef.mRefNum.mIndex = 0;
                         bool deleted = false;
-                        while(cell->getNextRef(esm[index], ref, deleted, /*ignoreMoves*/true, &cMRef))
+                        bool moved = false;
+                        while(cell->getNextRef(esm[index], ref, deleted, cMRef, moved))
                         {
-                            if (cMRef.mRefNum.mIndex)
+                            if (moved)
-                            {
+                                continue;
-                                cMRef.mRefNum.mIndex = 0;
-                                continue; // ignore refs that are moved
-                            }
 
                             if (std::find(cell->mMovedRefs.begin(), cell->mMovedRefs.end(), ref.mRefNum) != cell->mMovedRefs.end()) continue;
                             Misc::StringUtils::lowerCaseInPlace(ref.mRefID);

apps/openmw/mwworld/cellstore.cpp

@@ -553,17 +553,12 @@ namespace MWWorld
                 ESM::MovedCellRef cMRef;
                 cMRef.mRefNum.mIndex = 0;
                 bool deleted = false;
-                while(mCell->getNextRef(esm[index], ref, deleted, /*ignoreMoves*/true, &cMRef))
+                bool moved = false;
+                while(mCell->getNextRef(esm[index], ref, deleted, cMRef, moved))
                 {
-                    if (deleted)
+                    if (deleted || moved)
                         continue;
 
-                    if (cMRef.mRefNum.mIndex)
-                    {
-                        cMRef.mRefNum.mIndex = 0;
-                        continue; // ignore refs that are moved
-                    }
-
                     // Don't list reference if it was moved to a different cell.
                     ESM::MovedCellRefTracker::const_iterator iter =
                         std::find(mCell->mMovedRefs.begin(), mCell->mMovedRefs.end(), ref.mRefNum);
@@ -618,13 +613,11 @@ namespace MWWorld
                 ESM::MovedCellRef cMRef;
                 cMRef.mRefNum.mIndex = 0;
                 bool deleted = false;
-                while(mCell->getNextRef(esm[index], ref, deleted, /*ignoreMoves*/true, &cMRef))
+                bool moved = false;
+                while(mCell->getNextRef(esm[index], ref, deleted, cMRef, moved))
                 {
-                    if (cMRef.mRefNum.mIndex)
+                    if (moved)
-                    {
+                        continue;
-                        cMRef.mRefNum.mIndex = 0;
-                        continue; // ignore refs that are moved
-                    }
 
                     // Don't load reference if it was moved to a different cell.
                     ESM::MovedCellRefTracker::const_iterator iter =

apps/openmw/mwworld/store.cpp

@@ -502,8 +502,8 @@ namespace MWWorld
     {
         ESM::CellRef ref;
         ESM::MovedCellRef cMRef;
-        cMRef.mRefNum.mIndex = 0;
         bool deleted = false;
+        bool moved = false;
 
         ESM::ESM_Context ctx = esm.getContext();
 
@@ -512,10 +512,10 @@ namespace MWWorld
         //
         // Get regular moved reference data. Adapted from CellStore::loadRefs. Maybe we can optimize the following
         //  implementation when the oher implementation works as well.
-        while (cell->getNextRef(esm, ref, deleted, /*ignoreMoves*/true, &cMRef))
+        while (cell->getNextRef(esm, ref, deleted, cMRef, moved))
         {
-            if (!cMRef.mRefNum.mIndex)
+            if (!moved)
-                continue; // ignore refs that are not moved
+                continue;
 
             ESM::Cell *cellAlt = const_cast<ESM::Cell*>(searchOrCreate(cMRef.mTarget[0], cMRef.mTarget[1]));
 

components/esm/loadcell.cpp

@@ -224,7 +224,7 @@ namespace ESM
         return region + ' ' + cellGrid;
     }
 
-    bool Cell::getNextRef(ESMReader &esm, CellRef &ref, bool &isDeleted, bool ignoreMoves, MovedCellRef *mref)
+    bool Cell::getNextRef(ESMReader& esm, CellRef& ref, bool& isDeleted)
     {
         isDeleted = false;
 
@@ -235,20 +235,11 @@ namespace ESM
         // NOTE: We should not need this check. It is a safety check until we have checked
         // more plugins, and how they treat these moved references.
         if (esm.isNextSub("MVRF"))
-        {
-            if (ignoreMoves)
-            {
-                esm.getHT (mref->mRefNum.mIndex);
-                esm.getHNOT (mref->mTarget, "CNDT");
-                adjustRefNum (mref->mRefNum, esm);
-            }
-            else
         {
             // skip rest of cell record (moved references), they are handled elsewhere
             esm.skipRecord(); // skip MVRF, CNDT
             return false;
         }
-        }
 
         if (esm.peekNextSub("FRMR"))
         {
@@ -263,6 +254,29 @@ namespace ESM
         return false;
     }
 
+    bool Cell::getNextRef(ESMReader& esm, CellRef& cellRef, bool& deleted, MovedCellRef& movedCellRef, bool& moved)
+    {
+        deleted = false;
+        moved = false;
+
+        if (!esm.hasMoreSubs())
+            return false;
+
+        if (esm.isNextSub("MVRF"))
+        {
+            moved = true;
+            getNextMVRF(esm, movedCellRef);
+        }
+
+        if (!esm.peekNextSub("FRMR"))
+            return false;
+
+        cellRef.load(esm, deleted);
+        adjustRefNum(cellRef.mRefNum, esm);
+
+        return true;
+    }
+
     bool Cell::getNextMVRF(ESMReader &esm, MovedCellRef &mref)
     {
         esm.getHT(mref.mRefNum.mIndex);

components/esm/loadcell.hpp

@@ -181,12 +181,9 @@ struct Cell
      All fields of the CellRef struct are overwritten. You can safely
      reuse one memory location without blanking it between calls.
   */
-  /// \param ignoreMoves ignore MVRF record and read reference like a regular CellRef.
+    static bool getNextRef(ESMReader& esm, CellRef& ref, bool& deleted);
-  static bool getNextRef(ESMReader &esm, 
+
-                         CellRef &ref, 
+    static bool getNextRef(ESMReader& esm, CellRef& cellRef, bool& deleted, MovedCellRef& movedCellRef, bool& moved);
-                         bool &isDeleted,
-                         bool ignoreMoves = false, 
-                         MovedCellRef *mref = nullptr);
 
   /* This fetches an MVRF record, which is used to track moved references.
    * Since they are comparably rare, we use a separate method for this.