From f90c4ae22f02e0285181fe95bf223c1813622313 Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 12 Jul 2021 18:26:27 +0200 Subject: [PATCH 1/4] Add yaml-like separator between cell refs To be able to separate records visually. --- apps/esmtool/esmtool.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/esmtool/esmtool.cpp b/apps/esmtool/esmtool.cpp index 6b6e428e62..ee2223b6a8 100644 --- a/apps/esmtool/esmtool.cpp +++ b/apps/esmtool/esmtool.cpp @@ -244,7 +244,7 @@ void loadCell(ESM::Cell &cell, ESM::ESMReader &esm, Arguments& info) if(quiet) continue; - std::cout << " Refnum: " << ref.mRefNum.mIndex << '\n'; + std::cout << " - Refnum: " << ref.mRefNum.mIndex << '\n'; std::cout << " ID: " << ref.mRefID << '\n'; std::cout << " Position: (" << ref.mPos.pos[0] << ", " << ref.mPos.pos[1] << ", " << ref.mPos.pos[2] << ")\n"; if (ref.mScale != 1.f) From cfdbd0d471c6bac72f904f86a1bec486ca2a3822 Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 12 Jul 2021 17:30:39 +0200 Subject: [PATCH 2/4] 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 | 7 ++-- apps/openmw/mwrender/objectpaging.cpp | 10 +++--- apps/openmw/mwworld/cellstore.cpp | 21 ++++-------- apps/openmw/mwworld/store.cpp | 8 ++--- components/esm/loadcell.cpp | 40 +++++++++++++++-------- components/esm/loadcell.hpp | 9 ++--- 6 files changed, 48 insertions(+), 47 deletions(-) diff --git a/apps/opencs/model/world/refcollection.cpp b/apps/opencs/model/world/refcollection.cpp index dfdb8e73bf..003a300c06 100644 --- a/apps/opencs/model/world/refcollection.cpp +++ b/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::iterator iter = cache.begin(); unsigned int thisIndex = ref.mRefNum.mIndex & 0x00ffffff; diff --git a/apps/openmw/mwrender/objectpaging.cpp b/apps/openmw/mwrender/objectpaging.cpp index d91f4efaaa..2cbffbc6eb 100644 --- a/apps/openmw/mwrender/objectpaging.cpp +++ b/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) - { - cMRef.mRefNum.mIndex = 0; - continue; // ignore refs that are moved - } + if (moved) + continue; if (std::find(cell->mMovedRefs.begin(), cell->mMovedRefs.end(), ref.mRefNum) != cell->mMovedRefs.end()) continue; Misc::StringUtils::lowerCaseInPlace(ref.mRefID); diff --git a/apps/openmw/mwworld/cellstore.cpp b/apps/openmw/mwworld/cellstore.cpp index 53245be247..2b73a3362b 100644 --- a/apps/openmw/mwworld/cellstore.cpp +++ b/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) - { - cMRef.mRefNum.mIndex = 0; - continue; // ignore refs that are moved - } + if (moved) + continue; // Don't load reference if it was moved to a different cell. ESM::MovedCellRefTracker::const_iterator iter = diff --git a/apps/openmw/mwworld/store.cpp b/apps/openmw/mwworld/store.cpp index 2ed051a141..1614e07461 100644 --- a/apps/openmw/mwworld/store.cpp +++ b/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) - continue; // ignore refs that are not moved + if (!moved) + continue; ESM::Cell *cellAlt = const_cast(searchOrCreate(cMRef.mTarget[0], cMRef.mTarget[1])); diff --git a/components/esm/loadcell.cpp b/components/esm/loadcell.cpp index d43911135a..e90af7b2f6 100644 --- a/components/esm/loadcell.cpp +++ b/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; @@ -236,18 +236,9 @@ namespace ESM // 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; - } + // 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); diff --git a/components/esm/loadcell.hpp b/components/esm/loadcell.hpp index c49dc20c59..3a8133d881 100644 --- a/components/esm/loadcell.hpp +++ b/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 &isDeleted, - bool ignoreMoves = false, - MovedCellRef *mref = nullptr); + static bool getNextRef(ESMReader& esm, CellRef& ref, bool& deleted); + + static bool getNextRef(ESMReader& esm, CellRef& cellRef, bool& deleted, MovedCellRef& movedCellRef, bool& moved); /* This fetches an MVRF record, which is used to track moved references. * Since they are comparably rare, we use a separate method for this. From aec4e02417cef6106b78ffa5978c6279c32abdd0 Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 12 Jul 2021 18:20:51 +0200 Subject: [PATCH 3/4] Ignore only CellRefs with preceding MVRF subrecord MVRF subrecord means that only single following FRMR subrecord is moved not the rest of subrecords. --- components/esm/loadcell.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/components/esm/loadcell.cpp b/components/esm/loadcell.cpp index e90af7b2f6..562e8f643a 100644 --- a/components/esm/loadcell.cpp +++ b/components/esm/loadcell.cpp @@ -232,13 +232,18 @@ namespace ESM if (!esm.hasMoreSubs()) return false; - // 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")) + // MVRF are FRMR are present in pairs. MVRF indicates that following FRMR describes moved CellRef. + // This function has to skip all moved CellRefs therefore read all such pairs to ignored values. + while (esm.isNextSub("MVRF")) { - // skip rest of cell record (moved references), they are handled elsewhere - esm.skipRecord(); // skip MVRF, CNDT - return false; + MovedCellRef movedCellRef; + esm.getHT(movedCellRef.mRefNum.mIndex); + esm.getHNOT(movedCellRef.mTarget, "CNDT"); + CellRef skippedCellRef; + if (!esm.peekNextSub("FRMR")) + return false; + bool skippedDeleted; + skippedCellRef.load(esm, skippedDeleted); } if (esm.peekNextSub("FRMR")) From 5c9af1742a65fc70565725ffb7d18fbb21ea6d02 Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 12 Jul 2021 18:24:51 +0200 Subject: [PATCH 4/4] Dump moved cell refs in esmtool --- apps/esmtool/esmtool.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/apps/esmtool/esmtool.cpp b/apps/esmtool/esmtool.cpp index ee2223b6a8..dc893061a7 100644 --- a/apps/esmtool/esmtool.cpp +++ b/apps/esmtool/esmtool.cpp @@ -236,7 +236,9 @@ void loadCell(ESM::Cell &cell, ESM::ESMReader &esm, Arguments& info) if(!quiet) std::cout << " References:\n"; bool deleted = false; - while(cell.getNextRef(esm, ref, deleted)) + ESM::MovedCellRef movedCellRef; + bool moved = false; + while(cell.getNextRef(esm, ref, deleted, movedCellRef, moved)) { if (save) { info.data.mCellRefs[&cell].push_back(std::make_pair(ref, deleted)); @@ -276,6 +278,13 @@ void loadCell(ESM::Cell &cell, ESM::ESMReader &esm, Arguments& info) if (!ref.mDestCell.empty()) std::cout << " Destination cell: " << ref.mDestCell << '\n'; } + std::cout << " Moved: " << std::boolalpha << moved << '\n'; + if (moved) + { + std::cout << " Moved refnum: " << movedCellRef.mRefNum.mIndex << '\n'; + std::cout << " Moved content file: " << movedCellRef.mRefNum.mContentFile << '\n'; + std::cout << " Target: " << movedCellRef.mTarget[0] << ", " << movedCellRef.mTarget[1] << '\n'; + } } }