From d7907473891b02b7bd6577336a4422bd0e5ecbd7 Mon Sep 17 00:00:00 2001 From: MiroslavR Date: Fri, 22 Jul 2016 01:59:02 +0200 Subject: [PATCH 1/3] Implement deletion of moved references (Bug #3471) --- apps/openmw/mwworld/cellstore.cpp | 11 +++++++---- apps/openmw/mwworld/store.cpp | 23 ++++++++++------------- components/esm/loadcell.hpp | 10 +++++++++- 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/apps/openmw/mwworld/cellstore.cpp b/apps/openmw/mwworld/cellstore.cpp index 1ca452efe..31b8a1515 100644 --- a/apps/openmw/mwworld/cellstore.cpp +++ b/apps/openmw/mwworld/cellstore.cpp @@ -497,9 +497,11 @@ namespace MWWorld // List moved references, from separately tracked list. for (ESM::CellRefTracker::const_iterator it = mCell->mLeasedRefs.begin(); it != mCell->mLeasedRefs.end(); ++it) { - const ESM::CellRef &ref = *it; + const ESM::CellRef &ref = it->first; + bool deleted = it->second; - mIds.push_back(Misc::StringUtils::lowerCase(ref.mRefID)); + if (!deleted) + mIds.push_back(Misc::StringUtils::lowerCase(ref.mRefID)); } std::sort (mIds.begin(), mIds.end()); @@ -549,9 +551,10 @@ namespace MWWorld // Load moved references, from separately tracked list. for (ESM::CellRefTracker::const_iterator it = mCell->mLeasedRefs.begin(); it != mCell->mLeasedRefs.end(); ++it) { - ESM::CellRef &ref = const_cast(*it); + ESM::CellRef &ref = const_cast(it->first); + bool deleted = it->second; - loadRef (ref, false); + loadRef (ref, deleted); } updateMergedRefs(); diff --git a/apps/openmw/mwworld/store.cpp b/apps/openmw/mwworld/store.cpp index 4cddbcdfb..fbe94b7d0 100644 --- a/apps/openmw/mwworld/store.cpp +++ b/apps/openmw/mwworld/store.cpp @@ -513,19 +513,16 @@ namespace MWWorld bool deleted = false; cell->getNextRef(esm, ref, deleted); - if (!deleted) - { - // Add data required to make reference appear in the correct cell. - // We should not need to test for duplicates, as this part of the code is pre-cell merge. - cell->mMovedRefs.push_back(cMRef); + // Add data required to make reference appear in the correct cell. + // We should not need to test for duplicates, as this part of the code is pre-cell merge. + cell->mMovedRefs.push_back(cMRef); - // But there may be duplicates here! - ESM::CellRefTracker::iterator iter = std::find(cellAlt->mLeasedRefs.begin(), cellAlt->mLeasedRefs.end(), ref.mRefNum); - if (iter == cellAlt->mLeasedRefs.end()) - cellAlt->mLeasedRefs.push_back(ref); - else - *iter = ref; - } + // But there may be duplicates here! + ESM::CellRefTracker::iterator iter = std::find_if(cellAlt->mLeasedRefs.begin(), cellAlt->mLeasedRefs.end(), ESM::CellRefTrackerPredicate(ref.mRefNum)); + if (iter == cellAlt->mLeasedRefs.end()) + cellAlt->mLeasedRefs.push_back(std::make_pair(ref, deleted)); + else + *iter = std::make_pair(ref, deleted); } } const ESM::Cell *Store::search(const std::string &id) const @@ -681,7 +678,7 @@ namespace MWWorld if (itold != oldcell->mMovedRefs.end()) { ESM::MovedCellRef target0 = *itold; ESM::Cell *wipecell = const_cast(search(target0.mTarget[0], target0.mTarget[1])); - ESM::CellRefTracker::iterator it_lease = std::find(wipecell->mLeasedRefs.begin(), wipecell->mLeasedRefs.end(), it->mRefNum); + ESM::CellRefTracker::iterator it_lease = std::find_if(wipecell->mLeasedRefs.begin(), wipecell->mLeasedRefs.end(), ESM::CellRefTrackerPredicate(it->mRefNum)); if (it_lease != wipecell->mLeasedRefs.end()) wipecell->mLeasedRefs.erase(it_lease); else diff --git a/components/esm/loadcell.hpp b/components/esm/loadcell.hpp index 549ed7309..249d812b1 100644 --- a/components/esm/loadcell.hpp +++ b/components/esm/loadcell.hpp @@ -43,7 +43,15 @@ bool operator==(const MovedCellRef& ref, const RefNum& refNum); bool operator==(const CellRef& ref, const RefNum& refNum); typedef std::list MovedCellRefTracker; -typedef std::list CellRefTracker; +typedef std::list > CellRefTracker; + +struct CellRefTrackerPredicate +{ + RefNum mRefNum; + + CellRefTrackerPredicate(const RefNum& refNum) : mRefNum(refNum) {} + bool operator() (const std::pair& refdelPair) { return refdelPair.first == mRefNum; } +}; /* Cells hold data about objects, creatures, statics (rocks, walls, buildings) and landscape (for exterior cells). Cells frequently From dafe184220dc2790435c212e7bbe43fe9ccc1ebb Mon Sep 17 00:00:00 2001 From: MiroslavR Date: Fri, 22 Jul 2016 02:12:03 +0200 Subject: [PATCH 2/3] Fix moved references disappearing when modified by a plugin --- apps/openmw/mwworld/store.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/apps/openmw/mwworld/store.cpp b/apps/openmw/mwworld/store.cpp index fbe94b7d0..c13f10d9a 100644 --- a/apps/openmw/mwworld/store.cpp +++ b/apps/openmw/mwworld/store.cpp @@ -675,14 +675,17 @@ namespace MWWorld for (ESM::MovedCellRefTracker::const_iterator it = cell.mMovedRefs.begin(); it != cell.mMovedRefs.end(); ++it) { // remove reference from current leased ref tracker and add it to new cell ESM::MovedCellRefTracker::iterator itold = std::find(oldcell->mMovedRefs.begin(), oldcell->mMovedRefs.end(), it->mRefNum); - if (itold != oldcell->mMovedRefs.end()) { - ESM::MovedCellRef target0 = *itold; - ESM::Cell *wipecell = const_cast(search(target0.mTarget[0], target0.mTarget[1])); - ESM::CellRefTracker::iterator it_lease = std::find_if(wipecell->mLeasedRefs.begin(), wipecell->mLeasedRefs.end(), ESM::CellRefTrackerPredicate(it->mRefNum)); - if (it_lease != wipecell->mLeasedRefs.end()) - wipecell->mLeasedRefs.erase(it_lease); - else - std::cerr << "can't find " << it->mRefNum.mIndex << " " << it->mRefNum.mContentFile << " in leasedRefs " << std::endl; + if (itold != oldcell->mMovedRefs.end()) + { + if (it->mTarget[0] != itold->mTarget[0] || it->mTarget[1] != itold->mTarget[1]) + { + ESM::Cell *wipecell = const_cast(search(itold->mTarget[0], itold->mTarget[1])); + ESM::CellRefTracker::iterator it_lease = std::find_if(wipecell->mLeasedRefs.begin(), wipecell->mLeasedRefs.end(), ESM::CellRefTrackerPredicate(it->mRefNum)); + if (it_lease != wipecell->mLeasedRefs.end()) + wipecell->mLeasedRefs.erase(it_lease); + else + std::cerr << "can't find " << it->mRefNum.mIndex << " " << it->mRefNum.mContentFile << " in leasedRefs " << std::endl; + } *itold = *it; } else From 4a3529488b45e0020d1ae35400f49210bbb18760 Mon Sep 17 00:00:00 2001 From: MiroslavR Date: Fri, 22 Jul 2016 03:58:23 +0200 Subject: [PATCH 3/3] Fix possible reference duplication when the refID is modified by a plugin (Bug #3471) --- apps/openmw/mwworld/cellreflist.hpp | 12 ++++++++ apps/openmw/mwworld/cellstore.cpp | 48 ++++++++++++++++++++++++++--- apps/openmw/mwworld/cellstore.hpp | 2 +- 3 files changed, 57 insertions(+), 5 deletions(-) diff --git a/apps/openmw/mwworld/cellreflist.hpp b/apps/openmw/mwworld/cellreflist.hpp index 4d503dcc8..30be4a661 100644 --- a/apps/openmw/mwworld/cellreflist.hpp +++ b/apps/openmw/mwworld/cellreflist.hpp @@ -29,6 +29,18 @@ namespace MWWorld mList.push_back(item); return mList.back(); } + + /// Remove all references with the given refNum from this list. + void remove (const ESM::RefNum &refNum) + { + for (typename List::iterator it = mList.begin(); it != mList.end();) + { + if (*it == refNum) + mList.erase(it++); + else + ++it; + } + } }; } diff --git a/apps/openmw/mwworld/cellstore.cpp b/apps/openmw/mwworld/cellstore.cpp index 31b8a1515..db3707441 100644 --- a/apps/openmw/mwworld/cellstore.cpp +++ b/apps/openmw/mwworld/cellstore.cpp @@ -516,6 +516,8 @@ namespace MWWorld if (mCell->mContextList.empty()) return; // this is a dynamically generated cell -> skipping. + std::map refNumToID; // used to detect refID modifications + // Load references from all plugins that do something with this cell. for (size_t i = 0; i < mCell->mContextList.size(); i++) { @@ -539,7 +541,7 @@ namespace MWWorld continue; } - loadRef (ref, deleted); + loadRef (ref, deleted, refNumToID); } } catch (std::exception& e) @@ -554,7 +556,7 @@ namespace MWWorld ESM::CellRef &ref = const_cast(it->first); bool deleted = it->second; - loadRef (ref, deleted); + loadRef (ref, deleted, refNumToID); } updateMergedRefs(); @@ -585,12 +587,47 @@ namespace MWWorld return Ptr(); } - void CellStore::loadRef (ESM::CellRef& ref, bool deleted) + void CellStore::loadRef (ESM::CellRef& ref, bool deleted, std::map& refNumToID) { Misc::StringUtils::lowerCaseInPlace (ref.mRefID); const MWWorld::ESMStore& store = mStore; + std::map::iterator it = refNumToID.find(ref.mRefNum); + if (it != refNumToID.end()) + { + if (it->second != ref.mRefID) + { + // refID was modified, make sure we don't end up with duplicated refs + switch (store.find(it->second)) + { + case ESM::REC_ACTI: mActivators.remove(ref.mRefNum); break; + case ESM::REC_ALCH: mPotions.remove(ref.mRefNum); break; + case ESM::REC_APPA: mAppas.remove(ref.mRefNum); break; + case ESM::REC_ARMO: mArmors.remove(ref.mRefNum); break; + case ESM::REC_BOOK: mBooks.remove(ref.mRefNum); break; + case ESM::REC_CLOT: mClothes.remove(ref.mRefNum); break; + case ESM::REC_CONT: mContainers.remove(ref.mRefNum); break; + case ESM::REC_CREA: mCreatures.remove(ref.mRefNum); break; + case ESM::REC_DOOR: mDoors.remove(ref.mRefNum); break; + case ESM::REC_INGR: mIngreds.remove(ref.mRefNum); break; + case ESM::REC_LEVC: mCreatureLists.remove(ref.mRefNum); break; + case ESM::REC_LEVI: mItemLists.remove(ref.mRefNum); break; + case ESM::REC_LIGH: mLights.remove(ref.mRefNum); break; + case ESM::REC_LOCK: mLockpicks.remove(ref.mRefNum); break; + case ESM::REC_MISC: mMiscItems.remove(ref.mRefNum); break; + case ESM::REC_NPC_: mNpcs.remove(ref.mRefNum); break; + case ESM::REC_PROB: mProbes.remove(ref.mRefNum); break; + case ESM::REC_REPA: mRepairs.remove(ref.mRefNum); break; + case ESM::REC_STAT: mStatics.remove(ref.mRefNum); break; + case ESM::REC_WEAP: mWeapons.remove(ref.mRefNum); break; + case ESM::REC_BODY: mBodyParts.remove(ref.mRefNum); break; + default: + break; + } + } + } + switch (store.find (ref.mRefID)) { case ESM::REC_ACTI: mActivators.load(ref, deleted, store); break; @@ -615,12 +652,15 @@ namespace MWWorld case ESM::REC_WEAP: mWeapons.load(ref, deleted, store); break; case ESM::REC_BODY: mBodyParts.load(ref, deleted, store); break; - case 0: std::cerr << "Cell reference '" + ref.mRefID + "' not found!\n"; break; + case 0: std::cerr << "Cell reference '" + ref.mRefID + "' not found!\n"; return; default: std::cerr << "WARNING: Ignoring reference '" << ref.mRefID << "' of unhandled type\n"; + return; } + + refNumToID[ref.mRefNum] = ref.mRefID; } void CellStore::loadState (const ESM::CellState& state) diff --git a/apps/openmw/mwworld/cellstore.hpp b/apps/openmw/mwworld/cellstore.hpp index 36cc7f5eb..1aee13132 100644 --- a/apps/openmw/mwworld/cellstore.hpp +++ b/apps/openmw/mwworld/cellstore.hpp @@ -385,7 +385,7 @@ namespace MWWorld void loadRefs(); - void loadRef (ESM::CellRef& ref, bool deleted); + void loadRef (ESM::CellRef& ref, bool deleted, std::map& refNumToID); ///< Make case-adjustments to \a ref and insert it into the respective container. /// /// Invalid \a ref objects are silently dropped.