diff --git a/CHANGELOG.md b/CHANGELOG.md index c57854409c..4e40974322 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -168,6 +168,7 @@ Bug #7872: Region sounds use wrong odds Bug #7886: Equip and unequip animations can't share the animation track section Bug #7887: Editor: Mismatched reported script data size and actual data size causes a crash during save + Bug #7896: Editor: Loading cellrefs incorrectly transforms Refnums, causing load failures Bug #7898: Editor: Invalid reference scales are allowed Bug #7899: Editor: Doors can't be unlocked Bug #7901: Editor: Teleport-related fields shouldn't be editable if a ref does not teleport diff --git a/apps/opencs/model/doc/loader.cpp b/apps/opencs/model/doc/loader.cpp index 46dea447fe..5be733e0d1 100644 --- a/apps/opencs/model/doc/loader.cpp +++ b/apps/opencs/model/doc/loader.cpp @@ -17,13 +17,6 @@ #include "document.hpp" -CSMDoc::Loader::Stage::Stage() - : mFile(0) - , mRecordsLoaded(0) - , mRecordsLeft(false) -{ -} - CSMDoc::Loader::Loader() : mShouldStop(false) { @@ -105,7 +98,7 @@ void CSMDoc::Loader::load() if (iter->second.mFile < size) // start loading the files { - std::filesystem::path path = document->getContentFiles()[iter->second.mFile]; + const std::filesystem::path& path = document->getContentFiles()[iter->second.mFile]; int steps = document->getData().startLoading(path, iter->second.mFile != editedIndex, /*project*/ false); iter->second.mRecordsLeft = true; diff --git a/apps/opencs/model/doc/loader.hpp b/apps/opencs/model/doc/loader.hpp index ccf493d19e..79b4524f4f 100644 --- a/apps/opencs/model/doc/loader.hpp +++ b/apps/opencs/model/doc/loader.hpp @@ -23,11 +23,9 @@ namespace CSMDoc struct Stage { - int mFile; - int mRecordsLoaded; - bool mRecordsLeft; - - Stage(); + int mFile = 0; + int mRecordsLoaded = 0; + bool mRecordsLeft = false; }; QMutex mMutex; diff --git a/apps/opencs/model/doc/savingstages.cpp b/apps/opencs/model/doc/savingstages.cpp index 77effc3a5c..5a1666da17 100644 --- a/apps/opencs/model/doc/savingstages.cpp +++ b/apps/opencs/model/doc/savingstages.cpp @@ -305,9 +305,8 @@ void CSMDoc::WriteCellCollectionStage::writeReferences( { CSMWorld::CellRef refRecord = ref.get(); - // Check for uninitialized content file - if (!refRecord.mRefNum.hasContentFile()) - refRecord.mRefNum.mContentFile = 0; + // -1 is the current file, saved indices are 1-based + refRecord.mRefNum.mContentFile++; // recalculate the ref's cell location std::ostringstream stream; diff --git a/apps/opencs/model/tools/mergestages.cpp b/apps/opencs/model/tools/mergestages.cpp index c2ab74bcca..8af612573a 100644 --- a/apps/opencs/model/tools/mergestages.cpp +++ b/apps/opencs/model/tools/mergestages.cpp @@ -117,7 +117,7 @@ void CSMTools::MergeReferencesStage::perform(int stage, CSMDoc::Messages& messag ref.mOriginalCell = ref.mCell; ref.mRefNum.mIndex = mIndex[ref.mCell]++; - ref.mRefNum.mContentFile = 0; + ref.mRefNum.mContentFile = -1; ref.mNew = false; mState.mTarget->getData().getReferences().appendRecord(std::make_unique>( diff --git a/apps/opencs/model/world/data.cpp b/apps/opencs/model/world/data.cpp index 470ce04131..dddbb8a850 100644 --- a/apps/opencs/model/world/data.cpp +++ b/apps/opencs/model/world/data.cpp @@ -137,9 +137,8 @@ CSMWorld::Data::Data(ToUTF8::FromType encoding, const Files::PathContainer& data : mEncoder(encoding) , mPathgrids(mCells) , mRefs(mCells) - , mReader(nullptr) , mDialogue(nullptr) - , mReaderIndex(1) + , mReaderIndex(0) , mDataPaths(dataPaths) , mArchives(archives) , mVFS(std::make_unique()) @@ -688,8 +687,6 @@ CSMWorld::Data::~Data() { for (std::vector::iterator iter(mModels.begin()); iter != mModels.end(); ++iter) delete *iter; - - delete mReader; } std::shared_ptr CSMWorld::Data::getResourceSystem() @@ -1068,17 +1065,11 @@ int CSMWorld::Data::startLoading(const std::filesystem::path& path, bool base, b { Log(Debug::Info) << "Loading content file " << path; - // Don't delete the Reader yet. Some record types store a reference to the Reader to handle on-demand loading - std::shared_ptr ptr(mReader); - mReaders.push_back(ptr); - mReader = nullptr; - mDialogue = nullptr; - mReader = new ESM::ESMReader; - mReader->setEncoder(&mEncoder); - mReader->setIndex((project || !base) ? 0 : mReaderIndex++); - mReader->open(path); + ESM::ReadersCache::BusyItem reader = mReaders.get(mReaderIndex++); + reader->setEncoder(&mEncoder); + reader->open(path); mBase = base; mProject = project; @@ -1087,13 +1078,13 @@ int CSMWorld::Data::startLoading(const std::filesystem::path& path, bool base, b { MetaData metaData; metaData.mId = ESM::RefId::stringRefId("sys::meta"); - metaData.load(*mReader); + metaData.load(*reader); mMetaData.setRecord(0, std::make_unique>(Record(RecordBase::State_ModifiedOnly, nullptr, &metaData))); } - return mReader->getRecordCount(); + return reader->getRecordCount(); } void CSMWorld::Data::loadFallbackEntries() @@ -1140,24 +1131,17 @@ void CSMWorld::Data::loadFallbackEntries() bool CSMWorld::Data::continueLoading(CSMDoc::Messages& messages) { - if (!mReader) + if (mReaderIndex == 0) throw std::logic_error("can't continue loading, because no load has been started"); + ESM::ReadersCache::BusyItem reader = mReaders.get(mReaderIndex - 1); + if (!reader->isOpen()) + throw std::logic_error("can't continue loading, because no load has been started"); + reader->setEncoder(&mEncoder); + reader->setIndex(static_cast(mReaderIndex - 1)); + reader->resolveParentFileIndices(mReaders); - if (!mReader->hasMoreRecs()) + if (!reader->hasMoreRecs()) { - if (mBase) - { - // Don't delete the Reader yet. Some record types store a reference to the Reader to handle on-demand - // loading. We don't store non-base reader, because everything going into modified will be fully loaded - // during the initial loading process. - std::shared_ptr ptr(mReader); - mReaders.push_back(ptr); - } - else - delete mReader; - - mReader = nullptr; - mDialogue = nullptr; loadFallbackEntries(); @@ -1165,76 +1149,76 @@ bool CSMWorld::Data::continueLoading(CSMDoc::Messages& messages) return true; } - ESM::NAME n = mReader->getRecName(); - mReader->getRecHeader(); + ESM::NAME n = reader->getRecName(); + reader->getRecHeader(); bool unhandledRecord = false; switch (n.toInt()) { case ESM::REC_GLOB: - mGlobals.load(*mReader, mBase); + mGlobals.load(*reader, mBase); break; case ESM::REC_GMST: - mGmsts.load(*mReader, mBase); + mGmsts.load(*reader, mBase); break; case ESM::REC_SKIL: - mSkills.load(*mReader, mBase); + mSkills.load(*reader, mBase); break; case ESM::REC_CLAS: - mClasses.load(*mReader, mBase); + mClasses.load(*reader, mBase); break; case ESM::REC_FACT: - mFactions.load(*mReader, mBase); + mFactions.load(*reader, mBase); break; case ESM::REC_RACE: - mRaces.load(*mReader, mBase); + mRaces.load(*reader, mBase); break; case ESM::REC_SOUN: - mSounds.load(*mReader, mBase); + mSounds.load(*reader, mBase); break; case ESM::REC_SCPT: - mScripts.load(*mReader, mBase); + mScripts.load(*reader, mBase); break; case ESM::REC_REGN: - mRegions.load(*mReader, mBase); + mRegions.load(*reader, mBase); break; case ESM::REC_BSGN: - mBirthsigns.load(*mReader, mBase); + mBirthsigns.load(*reader, mBase); break; case ESM::REC_SPEL: - mSpells.load(*mReader, mBase); + mSpells.load(*reader, mBase); break; case ESM::REC_ENCH: - mEnchantments.load(*mReader, mBase); + mEnchantments.load(*reader, mBase); break; case ESM::REC_BODY: - mBodyParts.load(*mReader, mBase); + mBodyParts.load(*reader, mBase); break; case ESM::REC_SNDG: - mSoundGens.load(*mReader, mBase); + mSoundGens.load(*reader, mBase); break; case ESM::REC_MGEF: - mMagicEffects.load(*mReader, mBase); + mMagicEffects.load(*reader, mBase); break; case ESM::REC_PGRD: - mPathgrids.load(*mReader, mBase); + mPathgrids.load(*reader, mBase); break; case ESM::REC_SSCR: - mStartScripts.load(*mReader, mBase); + mStartScripts.load(*reader, mBase); break; case ESM::REC_LTEX: - mLandTextures.load(*mReader, mBase); + mLandTextures.load(*reader, mBase); break; case ESM::REC_LAND: - mLand.load(*mReader, mBase); + mLand.load(*reader, mBase); break; case ESM::REC_CELL: { - int index = mCells.load(*mReader, mBase); + int index = mCells.load(*reader, mBase); if (index < 0 || index >= mCells.getSize()) { // log an error and continue loading the refs to the last loaded cell @@ -1243,69 +1227,69 @@ bool CSMWorld::Data::continueLoading(CSMDoc::Messages& messages) index = mCells.getSize() - 1; } - mRefs.load(*mReader, index, mBase, mRefLoadCache[mCells.getId(index)], messages); + mRefs.load(*reader, index, mBase, mRefLoadCache[mCells.getId(index)], messages); break; } case ESM::REC_ACTI: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Activator); + mReferenceables.load(*reader, mBase, UniversalId::Type_Activator); break; case ESM::REC_ALCH: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Potion); + mReferenceables.load(*reader, mBase, UniversalId::Type_Potion); break; case ESM::REC_APPA: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Apparatus); + mReferenceables.load(*reader, mBase, UniversalId::Type_Apparatus); break; case ESM::REC_ARMO: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Armor); + mReferenceables.load(*reader, mBase, UniversalId::Type_Armor); break; case ESM::REC_BOOK: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Book); + mReferenceables.load(*reader, mBase, UniversalId::Type_Book); break; case ESM::REC_CLOT: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Clothing); + mReferenceables.load(*reader, mBase, UniversalId::Type_Clothing); break; case ESM::REC_CONT: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Container); + mReferenceables.load(*reader, mBase, UniversalId::Type_Container); break; case ESM::REC_CREA: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Creature); + mReferenceables.load(*reader, mBase, UniversalId::Type_Creature); break; case ESM::REC_DOOR: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Door); + mReferenceables.load(*reader, mBase, UniversalId::Type_Door); break; case ESM::REC_INGR: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Ingredient); + mReferenceables.load(*reader, mBase, UniversalId::Type_Ingredient); break; case ESM::REC_LEVC: - mReferenceables.load(*mReader, mBase, UniversalId::Type_CreatureLevelledList); + mReferenceables.load(*reader, mBase, UniversalId::Type_CreatureLevelledList); break; case ESM::REC_LEVI: - mReferenceables.load(*mReader, mBase, UniversalId::Type_ItemLevelledList); + mReferenceables.load(*reader, mBase, UniversalId::Type_ItemLevelledList); break; case ESM::REC_LIGH: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Light); + mReferenceables.load(*reader, mBase, UniversalId::Type_Light); break; case ESM::REC_LOCK: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Lockpick); + mReferenceables.load(*reader, mBase, UniversalId::Type_Lockpick); break; case ESM::REC_MISC: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Miscellaneous); + mReferenceables.load(*reader, mBase, UniversalId::Type_Miscellaneous); break; case ESM::REC_NPC_: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Npc); + mReferenceables.load(*reader, mBase, UniversalId::Type_Npc); break; case ESM::REC_PROB: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Probe); + mReferenceables.load(*reader, mBase, UniversalId::Type_Probe); break; case ESM::REC_REPA: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Repair); + mReferenceables.load(*reader, mBase, UniversalId::Type_Repair); break; case ESM::REC_STAT: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Static); + mReferenceables.load(*reader, mBase, UniversalId::Type_Static); break; case ESM::REC_WEAP: - mReferenceables.load(*mReader, mBase, UniversalId::Type_Weapon); + mReferenceables.load(*reader, mBase, UniversalId::Type_Weapon); break; case ESM::REC_DIAL: @@ -1313,7 +1297,7 @@ bool CSMWorld::Data::continueLoading(CSMDoc::Messages& messages) ESM::Dialogue record; bool isDeleted = false; - record.load(*mReader, isDeleted); + record.load(*reader, isDeleted); if (isDeleted) { @@ -1359,14 +1343,14 @@ bool CSMWorld::Data::continueLoading(CSMDoc::Messages& messages) messages.add(UniversalId::Type_None, "Found info record not following a dialogue record", "", CSMDoc::Message::Severity_Error); - mReader->skipRecord(); + reader->skipRecord(); break; } if (mDialogue->mType == ESM::Dialogue::Journal) - mJournalInfos.load(*mReader, mBase, *mDialogue, mJournalInfoOrder); + mJournalInfos.load(*reader, mBase, *mDialogue, mJournalInfoOrder); else - mTopicInfos.load(*mReader, mBase, *mDialogue, mTopicInfoOrder); + mTopicInfos.load(*reader, mBase, *mDialogue, mTopicInfoOrder); break; } @@ -1379,7 +1363,7 @@ bool CSMWorld::Data::continueLoading(CSMDoc::Messages& messages) break; } - mFilters.load(*mReader, mBase); + mFilters.load(*reader, mBase); break; case ESM::REC_DBGP: @@ -1390,7 +1374,7 @@ bool CSMWorld::Data::continueLoading(CSMDoc::Messages& messages) break; } - mDebugProfiles.load(*mReader, mBase); + mDebugProfiles.load(*reader, mBase); break; case ESM::REC_SELG: @@ -1401,7 +1385,7 @@ bool CSMWorld::Data::continueLoading(CSMDoc::Messages& messages) break; } - mSelectionGroups.load(*mReader, mBase); + mSelectionGroups.load(*reader, mBase); break; default: @@ -1414,7 +1398,7 @@ bool CSMWorld::Data::continueLoading(CSMDoc::Messages& messages) messages.add( UniversalId::Type_None, "Unsupported record type: " + n.toString(), "", CSMDoc::Message::Severity_Error); - mReader->skipRecord(); + reader->skipRecord(); } return false; @@ -1424,6 +1408,8 @@ void CSMWorld::Data::finishLoading() { mTopicInfos.sort(mTopicInfoOrder); mJournalInfos.sort(mJournalInfoOrder); + // Release file locks so we can actually write to the file we're editing + mReaders.clear(); } bool CSMWorld::Data::hasId(const std::string& id) const diff --git a/apps/opencs/model/world/data.hpp b/apps/opencs/model/world/data.hpp index 8e01452f3b..0cf25883af 100644 --- a/apps/opencs/model/world/data.hpp +++ b/apps/opencs/model/world/data.hpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -122,12 +123,12 @@ namespace CSMWorld std::unique_ptr mActorAdapter; std::vector mModels; std::map mModelIndex; - ESM::ESMReader* mReader; + ESM::ReadersCache mReaders; const ESM::Dialogue* mDialogue; // last loaded dialogue bool mBase; bool mProject; - std::map> mRefLoadCache; - int mReaderIndex; + std::map> mRefLoadCache; + std::size_t mReaderIndex; Files::PathContainer mDataPaths; std::vector mArchives; @@ -135,14 +136,11 @@ namespace CSMWorld ResourcesManager mResourcesManager; std::shared_ptr mResourceSystem; - std::vector> mReaders; - InfoOrderByTopic mJournalInfoOrder; InfoOrderByTopic mTopicInfoOrder; - // not implemented - Data(const Data&); - Data& operator=(const Data&); + Data(const Data&) = delete; + Data& operator=(const Data&) = delete; void addModel(QAbstractItemModel* model, UniversalId::Type type, bool update = true); diff --git a/apps/opencs/model/world/ref.cpp b/apps/opencs/model/world/ref.cpp index 328d205106..e28ef64fe7 100644 --- a/apps/opencs/model/world/ref.cpp +++ b/apps/opencs/model/world/ref.cpp @@ -8,8 +8,6 @@ CSMWorld::CellRef::CellRef() : mNew(true) , mIdNum(0) { - mRefNum.mIndex = 0; - mRefNum.mContentFile = 0; } std::pair CSMWorld::CellRef::getCellIndex() const diff --git a/apps/opencs/model/world/refcollection.cpp b/apps/opencs/model/world/refcollection.cpp index 642edcfb64..124e697de8 100644 --- a/apps/opencs/model/world/refcollection.cpp +++ b/apps/opencs/model/world/refcollection.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -47,7 +48,7 @@ namespace CSMWorld } void CSMWorld::RefCollection::load(ESM::ESMReader& reader, int cellIndex, bool base, - std::map& cache, CSMDoc::Messages& messages) + std::map& cache, CSMDoc::Messages& messages) { Record cell = mCells.getRecord(cellIndex); @@ -64,6 +65,8 @@ void CSMWorld::RefCollection::load(ESM::ESMReader& reader, int cellIndex, bool b if (!ESM::Cell::getNextRef(reader, ref, isDeleted, mref, isMoved)) break; + if (!base && reader.getIndex() == ref.mRefNum.mContentFile) + ref.mRefNum.mContentFile = -1; // Keep mOriginalCell empty when in modified (as an indicator that the // original cell will always be equal the current cell). ref.mOriginalCell = base ? cell2.mId : ESM::RefId(); @@ -102,16 +105,7 @@ void CSMWorld::RefCollection::load(ESM::ESMReader& reader, int cellIndex, bool b else ref.mCell = cell2.mId; - if (ref.mRefNum.mContentFile != -1 && !base) - { - ref.mRefNum.mContentFile = ref.mRefNum.mIndex >> 24; - ref.mRefNum.mIndex &= 0x00ffffff; - } - - unsigned int refNum = (ref.mRefNum.mIndex & 0x00ffffff) - | (ref.mRefNum.hasContentFile() ? ref.mRefNum.mContentFile : 0xff) << 24; - - std::map::iterator iter = cache.find(refNum); + auto iter = cache.find(ref.mRefNum); if (isMoved) { @@ -181,7 +175,7 @@ void CSMWorld::RefCollection::load(ESM::ESMReader& reader, int cellIndex, bool b ref.mIdNum = mNextId; // FIXME: fragile ref.mId = ESM::RefId::stringRefId(getNewId()); - cache.emplace(refNum, ref.mIdNum); + cache.emplace(ref.mRefNum, ref.mIdNum); auto record = std::make_unique>(); record->mState = base ? RecordBase::State_BaseOnly : RecordBase::State_ModifiedOnly; @@ -305,10 +299,10 @@ void CSMWorld::RefCollection::cloneRecord( copy->get().mId = destination; copy->get().mIdNum = extractIdNum(destination.getRefIdString()); - if (copy->get().mRefNum.mContentFile != 0) + if (copy->get().mRefNum.hasContentFile()) { mRefIndex.insert(std::make_pair(static_cast*>(copy.get())->get().mIdNum, index)); - copy->get().mRefNum.mContentFile = 0; + copy->get().mRefNum.mContentFile = -1; copy->get().mRefNum.mIndex = index; } else diff --git a/apps/opencs/model/world/refcollection.hpp b/apps/opencs/model/world/refcollection.hpp index a5e5fd3b6f..d3d200e6c2 100644 --- a/apps/opencs/model/world/refcollection.hpp +++ b/apps/opencs/model/world/refcollection.hpp @@ -55,7 +55,7 @@ namespace CSMWorld { } - void load(ESM::ESMReader& reader, int cellIndex, bool base, std::map& cache, + void load(ESM::ESMReader& reader, int cellIndex, bool base, std::map& cache, CSMDoc::Messages& messages); ///< Load a sequence of references. diff --git a/components/esm3/readerscache.cpp b/components/esm3/readerscache.cpp index deae2a3dc7..732a2a22ba 100644 --- a/components/esm3/readerscache.cpp +++ b/components/esm3/readerscache.cpp @@ -86,4 +86,12 @@ namespace ESM it->mState = State::Closed; } } + + void ReadersCache::clear() + { + mIndex.clear(); + mBusyItems.clear(); + mFreeItems.clear(); + mClosedItems.clear(); + } } diff --git a/components/esm3/readerscache.hpp b/components/esm3/readerscache.hpp index 5d8c2afcbd..80ca5d1ef3 100644 --- a/components/esm3/readerscache.hpp +++ b/components/esm3/readerscache.hpp @@ -55,6 +55,8 @@ namespace ESM BusyItem get(std::size_t index); + void clear(); + private: const std::size_t mCapacity; std::map::iterator> mIndex;