Revert rebasing the moved refs to the new cell, because the original ref may still be referred by a mod.

apps/opencs/model/doc/savingstages.cpp

@@ -303,13 +303,6 @@ void CSMDoc::WriteCellCollectionStage::perform (int stage, Messages& messages)
                 if (ref.mState==CSMWorld::RecordBase::State_Modified ||
                     ref.mState==CSMWorld::RecordBase::State_ModifiedOnly)
                 {
-                    // To get an MVRF tag, the ref's mOriginalCell needs to be non-empty (empty
-                    // is meant to indicate that it is the same as the current cell) and
-                    // different to mCell (its current cell) TODO: the second check seems redundant?
-                    //
-                    // To have mOriginalCell be non-empty, it needs to be loaded as 'base' in
-                    // RefCollection::load()
-                    //
                     // recalculate the ref's cell location
                     std::ostringstream stream;
                     if (!interior)
@@ -318,8 +311,10 @@ void CSMDoc::WriteCellCollectionStage::perform (int stage, Messages& messages)
                         stream << "#" << index.first << " " << index.second;
                     }
 
-                    if (!ref.get().mOriginalCell.empty() &&
+                    // An empty mOriginalCell is meant to indicate that it is the same as
-                        ref.get().mOriginalCell!=stream.str())
+                    // the current cell.  It is possible that a moved ref is moved again.
+                    if ((ref.get().mOriginalCell.empty() ? ref.get().mCell : ref.get().mOriginalCell)
+                            != stream.str())
                     {
                         ESM::MovedCellRef moved;
                         moved.mRefNum = ref.get().mRefNum;

apps/opencs/model/world/data.cpp

@@ -870,7 +870,7 @@ bool CSMWorld::Data::continueLoading (CSMDoc::Messages& messages)
                 index = mCells.getSize()-1;
             }
             std::string cellId = Misc::StringUtils::lowerCase (mCells.getId (index));
-            mRefs.load (*mReader, index, mBase, mRefLoadCache, cellId, messages);
+            mRefs.load (*mReader, index, mBase, mRefLoadCache[cellId], messages);
             break;
         }
 

apps/opencs/model/world/refcollection.cpp

@@ -13,10 +13,8 @@
 #include "record.hpp"
 
 void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool base,
-    std::map<std::string, std::map<ESM::RefNum, std::string> >& cache, const std::string& origCellId,
+    std::map<ESM::RefNum, std::string>& cache, CSMDoc::Messages& messages)
-    CSMDoc::Messages& messages)
 {
-    std::string cellid = origCellId;
     Record<Cell> cell = mCells.getRecord (cellIndex);
 
     Cell& cell2 = base ? cell.mBase : cell.mModified;
@@ -43,18 +41,13 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool
 
             ref.mCell = stream.str();
 
-            // It is not always possibe to ignore moved references sub-record and calculate from
-            // coordinates. Some mods may place the ref in positions outside normal bounds,
-            // resulting in non sensical cell id's.
-            //
-            // Use the target cell from the MVRF tag but if different output an error message
             if (!base &&                  // don't try to update base records
                 mref.mRefNum.mIndex != 0) // MVRF tag found
             {
-                std::ostringstream stream;
+                // there is a requirement for a placeholder where the original object was
-                stream << "#" << mref.mTarget[0] << " " << mref.mTarget[1];
+                //
-                ref.mCell = stream.str(); // overwrite
+                // see the forum discussions here for more details:
-
+                // https://forum.openmw.org/viewtopic.php?f=6&t=577&start=30
                 ref.mOriginalCell = cell2.mId;
 
                 if (deleted)
@@ -67,29 +60,35 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool
 
                     continue;
                 }
-                else
+
+                // It is not always possibe to ignore moved references sub-record and
+                // calculate from coordinates. Some mods may place the ref in positions
+                // outside normal bounds, resulting in non sensical cell id's.  This often
+                // happens if the moved ref was deleted.
+                //
+                // Use the target cell from the MVRF tag but if different output an error
+                // message
+                if (index.first != mref.mTarget[0] || index.second != mref.mTarget[1])
                 {
-                    if (index.first != mref.mTarget[0] || index.second != mref.mTarget[1])
+                    std::cerr << "The Position of moved ref "
-                    {
+                        << ref.mRefID << " does not match the target cell" << std::endl;
-                        std::cerr << "The Position of moved ref "
+                    std::cerr << "Position: #" << index.first << " " << index.second
-                            << ref.mRefID << " does not match the target cell" << std::endl;
+                        <<", Target #"<< mref.mTarget[0] << " " << mref.mTarget[1] << std::endl;
-                        std::cerr << "Position: #" << index.first << " " << index.second
+
-                            <<", Target #"<< mref.mTarget[0] << " " << mref.mTarget[1] << std::endl;
+                    std::ostringstream stream;
-                    }
+                    stream << "#" << mref.mTarget[0] << " " << mref.mTarget[1];
-
+                    ref.mCell = stream.str(); // overwrite
-                    // transfer the ref to the new cell
-                    cellid = ref.mCell;
                 }
             }
         }
         else
             ref.mCell = cell2.mId;
 
-        std::map<ESM::RefNum, std::string>::iterator iter = cache[cellid].find (ref.mRefNum);
+        std::map<ESM::RefNum, std::string>::iterator iter = cache.find (ref.mRefNum);
 
         if (deleted)
         {
-            if (iter==cache[cellid].end())
+            if (iter==cache.end())
             {
                 CSMWorld::UniversalId id (CSMWorld::UniversalId::Type_Cell,
                     mCells.getId (cellIndex));
@@ -106,7 +105,7 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool
             if (record.mState==RecordBase::State_BaseOnly)
             {
                 removeRows (index, 1);
-                cache[cellid].erase (iter);
+                cache.erase (iter);
             }
             else
             {
@@ -117,7 +116,7 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool
             continue;
         }
 
-        if (iter==cache[cellid].end())
+        if (iter==cache.end())
         {
             // new reference
             ref.mId = getNewId();
@@ -128,7 +127,7 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool
 
             appendRecord (record);
 
-            cache[cellid].insert (std::make_pair (ref.mRefNum, ref.mId));
+            cache.insert (std::make_pair (ref.mRefNum, ref.mId));
         }
         else
         {

apps/opencs/model/world/refcollection.hpp

@@ -27,8 +27,7 @@ namespace CSMWorld
             {}
 
             void load (ESM::ESMReader& reader, int cellIndex, bool base,
-                std::map<std::string, std::map<ESM::RefNum, std::string> >& cache, const std::string& cellid,
+                std::map<ESM::RefNum, std::string>& cache, CSMDoc::Messages& messages);
-                CSMDoc::Messages& messages);
             ///< Load a sequence of references.
 
             std::string getNewId();