From be45092e55bcbcba314905054e00e947f40862ab Mon Sep 17 00:00:00 2001 From: cc9cii Date: Fri, 23 Jul 2021 14:21:21 +1000 Subject: [PATCH 01/11] Use std::unique_ptr to store records in collections, RefidCollection and RefIdData. (copied the changes from commit 23e7e3c165bb2631f9d8eb298f86da862e91cefa) --- apps/opencs/model/doc/document.cpp | 25 ++-- apps/opencs/model/doc/savingstages.cpp | 12 +- apps/opencs/model/tools/journalcheck.cpp | 2 +- apps/opencs/model/tools/mergestages.cpp | 22 ++-- apps/opencs/model/tools/mergestages.hpp | 4 +- apps/opencs/model/world/collection.hpp | 111 +++++++++--------- apps/opencs/model/world/collectionbase.hpp | 5 +- apps/opencs/model/world/commands.cpp | 35 +++--- apps/opencs/model/world/commands.hpp | 4 +- apps/opencs/model/world/data.cpp | 23 ++-- apps/opencs/model/world/idcollection.hpp | 33 +++--- apps/opencs/model/world/idtable.cpp | 11 +- apps/opencs/model/world/idtable.hpp | 3 +- apps/opencs/model/world/infocollection.cpp | 45 +++---- apps/opencs/model/world/infocollection.hpp | 2 +- .../opencs/model/world/nestedidcollection.hpp | 32 ++--- .../model/world/nestedinfocollection.cpp | 32 ++--- apps/opencs/model/world/record.hpp | 18 +-- apps/opencs/model/world/refcollection.cpp | 23 ++-- apps/opencs/model/world/refidcollection.cpp | 15 +-- apps/opencs/model/world/refidcollection.hpp | 4 +- apps/opencs/model/world/refiddata.cpp | 12 +- apps/opencs/model/world/refiddata.hpp | 59 ++++++---- 23 files changed, 278 insertions(+), 254 deletions(-) diff --git a/apps/opencs/model/doc/document.cpp b/apps/opencs/model/doc/document.cpp index 3a20555d1e..f4071c525d 100644 --- a/apps/opencs/model/doc/document.cpp +++ b/apps/opencs/model/doc/document.cpp @@ -1,6 +1,7 @@ #include "document.hpp" #include +#include #include #include @@ -115,10 +116,10 @@ void CSMDoc::Document::addOptionalGmst (const ESM::GameSetting& gmst) { if (getData().getGmsts().searchId (gmst.mId)==-1) { - CSMWorld::Record record; - record.mBase = gmst; - record.mState = CSMWorld::RecordBase::State_BaseOnly; - getData().getGmsts().appendRecord (record); + std::unique_ptr > record(new CSMWorld::Record); + record->mBase = gmst; + record->mState = CSMWorld::RecordBase::State_BaseOnly; + getData().getGmsts().appendRecord (std::move(record)); } } @@ -126,10 +127,10 @@ void CSMDoc::Document::addOptionalGlobal (const ESM::Global& global) { if (getData().getGlobals().searchId (global.mId)==-1) { - CSMWorld::Record record; - record.mBase = global; - record.mState = CSMWorld::RecordBase::State_BaseOnly; - getData().getGlobals().appendRecord (record); + std::unique_ptr > record(new CSMWorld::Record); + record->mBase = global; + record->mState = CSMWorld::RecordBase::State_BaseOnly; + getData().getGlobals().appendRecord (std::move(record)); } } @@ -137,10 +138,10 @@ void CSMDoc::Document::addOptionalMagicEffect (const ESM::MagicEffect& magicEffe { if (getData().getMagicEffects().searchId (magicEffect.mId)==-1) { - CSMWorld::Record record; - record.mBase = magicEffect; - record.mState = CSMWorld::RecordBase::State_BaseOnly; - getData().getMagicEffects().appendRecord (record); + std::unique_ptr > record(new CSMWorld::Record); + record->mBase = magicEffect; + record->mState = CSMWorld::RecordBase::State_BaseOnly; + getData().getMagicEffects().appendRecord (std::move(record)); } } diff --git a/apps/opencs/model/doc/savingstages.cpp b/apps/opencs/model/doc/savingstages.cpp index 8c17a3b8c1..a410d34b2a 100644 --- a/apps/opencs/model/doc/savingstages.cpp +++ b/apps/opencs/model/doc/savingstages.cpp @@ -114,7 +114,7 @@ void CSMDoc::WriteDialogueCollectionStage::perform (int stage, Messages& message for (CSMWorld::InfoCollection::RecordConstIterator iter (range.first); iter!=range.second; ++iter) { - if (iter->isModified() || iter->mState == CSMWorld::RecordBase::State_Deleted) + if ((*iter)->isModified() || (*iter)->mState == CSMWorld::RecordBase::State_Deleted) { infoModified = true; break; @@ -140,9 +140,9 @@ void CSMDoc::WriteDialogueCollectionStage::perform (int stage, Messages& message // write modified selected info records for (CSMWorld::InfoCollection::RecordConstIterator iter (range.first); iter!=range.second; ++iter) { - if (iter->isModified() || iter->mState == CSMWorld::RecordBase::State_Deleted) + if ((*iter)->isModified() || (*iter)->mState == CSMWorld::RecordBase::State_Deleted) { - ESM::DialInfo info = iter->get(); + ESM::DialInfo info = (*iter)->get(); info.mId = info.mId.substr (info.mId.find_last_of ('#')+1); info.mPrev = ""; @@ -151,7 +151,7 @@ void CSMDoc::WriteDialogueCollectionStage::perform (int stage, Messages& message CSMWorld::InfoCollection::RecordConstIterator prev = iter; --prev; - info.mPrev = prev->get().mId.substr (prev->get().mId.find_last_of ('#')+1); + info.mPrev = (*prev)->get().mId.substr ((*prev)->get().mId.find_last_of ('#')+1); } CSMWorld::InfoCollection::RecordConstIterator next = iter; @@ -160,11 +160,11 @@ void CSMDoc::WriteDialogueCollectionStage::perform (int stage, Messages& message info.mNext = ""; if (next!=range.second) { - info.mNext = next->get().mId.substr (next->get().mId.find_last_of ('#')+1); + info.mNext = (*next)->get().mId.substr ((*next)->get().mId.find_last_of ('#')+1); } writer.startRecord (info.sRecordId); - info.save (writer, iter->mState == CSMWorld::RecordBase::State_Deleted); + info.save (writer, (*iter)->mState == CSMWorld::RecordBase::State_Deleted); writer.endRecord (info.sRecordId); } } diff --git a/apps/opencs/model/tools/journalcheck.cpp b/apps/opencs/model/tools/journalcheck.cpp index ae83abfa01..5959cdf54b 100644 --- a/apps/opencs/model/tools/journalcheck.cpp +++ b/apps/opencs/model/tools/journalcheck.cpp @@ -35,7 +35,7 @@ void CSMTools::JournalCheckStage::perform(int stage, CSMDoc::Messages& messages) for (CSMWorld::InfoCollection::RecordConstIterator it = range.first; it != range.second; ++it) { - const CSMWorld::Record infoRecord = (*it); + const CSMWorld::Record infoRecord = (*it->get()); if (infoRecord.isDeleted()) continue; diff --git a/apps/opencs/model/tools/mergestages.cpp b/apps/opencs/model/tools/mergestages.cpp index 016e2da392..021cf61d81 100644 --- a/apps/opencs/model/tools/mergestages.cpp +++ b/apps/opencs/model/tools/mergestages.cpp @@ -103,10 +103,9 @@ void CSMTools::MergeReferencesStage::perform (int stage, CSMDoc::Messages& messa ref.mRefNum.mContentFile = 0; ref.mNew = false; - CSMWorld::Record newRecord ( - CSMWorld::RecordBase::State_ModifiedOnly, nullptr, &ref); - - mState.mTarget->getData().getReferences().appendRecord (newRecord); + mState.mTarget->getData().getReferences().appendRecord ( + std::make_unique >( + CSMWorld::Record(CSMWorld::RecordBase::State_ModifiedOnly, nullptr, &ref))); } } @@ -128,7 +127,9 @@ void CSMTools::PopulateLandTexturesMergeStage::perform (int stage, CSMDoc::Messa if (!record.isDeleted()) { - mState.mTarget->getData().getLandTextures().appendRecord(record); + mState.mTarget->getData().getLandTextures().appendRecord( + std::make_unique >( + CSMWorld::Record(CSMWorld::RecordBase::State_ModifiedOnly, nullptr, &record.get()))); } } @@ -150,7 +151,9 @@ void CSMTools::MergeLandStage::perform (int stage, CSMDoc::Messages& messages) if (!record.isDeleted()) { - mState.mTarget->getData().getLand().appendRecord (record); + mState.mTarget->getData().getLand().appendRecord ( + std::make_unique >( + CSMWorld::Record(CSMWorld::RecordBase::State_ModifiedOnly, nullptr, &record.get()))); } } @@ -185,10 +188,9 @@ void CSMTools::FixLandsAndLandTexturesMergeStage::perform (int stage, CSMDoc::Me const CSMWorld::Record& oldRecord = mState.mTarget->getData().getLand().getRecord (stage); - CSMWorld::Record newRecord(CSMWorld::RecordBase::State_ModifiedOnly, - nullptr, &oldRecord.get()); - - mState.mTarget->getData().getLand().setRecord(stage, newRecord); + mState.mTarget->getData().getLand().setRecord (stage, + std::make_unique >( + CSMWorld::Record(CSMWorld::RecordBase::State_ModifiedOnly, nullptr, &oldRecord.get()))); } } diff --git a/apps/opencs/model/tools/mergestages.hpp b/apps/opencs/model/tools/mergestages.hpp index a6b6de58f8..778bea7c68 100644 --- a/apps/opencs/model/tools/mergestages.hpp +++ b/apps/opencs/model/tools/mergestages.hpp @@ -3,6 +3,7 @@ #include #include +#include #include @@ -82,7 +83,8 @@ namespace CSMTools const CSMWorld::Record& record = source.getRecord (stage); if (!record.isDeleted()) - target.appendRecord (CSMWorld::Record (CSMWorld::RecordBase::State_ModifiedOnly, nullptr, &record.get())); + target.appendRecord (std::make_unique >( + CSMWorld::Record(CSMWorld::RecordBase::State_ModifiedOnly, nullptr, &record.get()))); } class MergeRefIdsStage : public CSMDoc::Stage diff --git a/apps/opencs/model/world/collection.hpp b/apps/opencs/model/world/collection.hpp index 451ef9d0e1..8fc5d076ce 100644 --- a/apps/opencs/model/world/collection.hpp +++ b/apps/opencs/model/world/collection.hpp @@ -8,6 +8,7 @@ #include #include #include +#include #include @@ -84,7 +85,7 @@ namespace CSMWorld private: - std::vector > mRecords; + std::vector > > mRecords; std::map mIndex; std::vector *> mColumns; @@ -96,7 +97,7 @@ namespace CSMWorld const std::map& getIdMap() const; - const std::vector >& getRecords() const; + const std::vector > >& getRecords() const; bool reorderRowsImp (int baseIndex, const std::vector& newOrder); ///< Reorder the rows [baseIndex, baseIndex+newOrder.size()) according to the indices @@ -158,12 +159,12 @@ namespace CSMWorld ////< Search record with \a id. /// \return index of record (if found) or -1 (not found) - void replace (int index, const RecordBase& record) override; + void replace (int index, std::unique_ptr record) override; ///< If the record type does not match, an exception is thrown. /// /// \attention \a record must not change the ID. - void appendRecord (const RecordBase& record, + void appendRecord (std::unique_ptr record, UniversalId::Type type = UniversalId::Type_None) override; ///< If the record type does not match, an exception is thrown. ///< \param type Will be ignored, unless the collection supports multiple record types @@ -181,7 +182,7 @@ namespace CSMWorld /// /// \param listDeleted include deleted record in the list - virtual void insertRecord (const RecordBase& record, int index, + virtual void insertRecord (std::unique_ptr record, int index, UniversalId::Type type = UniversalId::Type_None); ///< Insert record before index. /// @@ -198,7 +199,7 @@ namespace CSMWorld void addColumn (Column *column); - void setRecord (int index, const Record& record); + void setRecord (int index, std::unique_ptr > record); ///< \attention This function must not change the ID. NestableColumn *getNestableColumn (int column) const; @@ -211,7 +212,7 @@ namespace CSMWorld } template - const std::vector >& Collection::getRecords() const + const std::vector > >& Collection::getRecords() const { return mRecords; } @@ -231,15 +232,15 @@ namespace CSMWorld return false; // reorder records - std::vector > buffer (size); + std::vector > > buffer (size); for (int i=0; isetModified (buffer[newOrder[i]]->get()); } - std::copy (buffer.begin(), buffer.end(), mRecords.begin()+baseIndex); + std::move (buffer.begin(), buffer.end(), mRecords.begin()+baseIndex); // adjust index for (std::map::iterator iter (mIndex.begin()); iter!=mIndex.end(); @@ -255,18 +256,18 @@ namespace CSMWorld int Collection::cloneRecordImp(const std::string& origin, const std::string& destination, UniversalId::Type type) { - Record copy; - copy.mModified = getRecord(origin).get(); - copy.mState = RecordBase::State_ModifiedOnly; - IdAccessorT().setId(copy.get(), destination); + std::unique_ptr > copy(new Record); + copy->mModified = getRecord(origin).get(); + copy->mState = RecordBase::State_ModifiedOnly; + IdAccessorT().setId(copy->get(), destination); if (type == UniversalId::Type_Reference) { - CSMWorld::CellRef* ptr = (CSMWorld::CellRef*) ©.mModified; + CSMWorld::CellRef* ptr = (CSMWorld::CellRef*) ©->mModified; ptr->mRefNum.mIndex = 0; } int index = getAppendIndex(destination, type); - insertRecord(copy, getAppendIndex(destination, type)); + insertRecord(std::move(copy), getAppendIndex(destination, type)); return index; } @@ -275,7 +276,7 @@ namespace CSMWorld int Collection::touchRecordImp(const std::string& id) { int index = getIndex(id); - Record& record = mRecords.at(index); + Record& record = *mRecords.at(index); if (record.isDeleted()) { throw std::runtime_error("attempt to touch deleted record"); @@ -302,7 +303,7 @@ namespace CSMWorld const std::string& destination, const UniversalId::Type type) { int index = cloneRecordImp(origin, destination, type); - mRecords.at(index).get().mPlugin = 0; + mRecords.at(index)->get().mPlugin = 0; } template @@ -317,7 +318,7 @@ namespace CSMWorld int index = touchRecordImp(id); if (index >= 0) { - mRecords.at(index).get().mPlugin = 0; + mRecords.at(index)->get().mPlugin = 0; return true; } @@ -344,15 +345,15 @@ namespace CSMWorld if (iter==mIndex.end()) { - Record record2; - record2.mState = Record::State_ModifiedOnly; - record2.mModified = record; + std::unique_ptr > record2(new Record); + record2->mState = Record::State_ModifiedOnly; + record2->mModified = record; - insertRecord (record2, getAppendIndex (id)); + insertRecord (std::move(record2), getAppendIndex (id)); } else { - mRecords[iter->second].setModified (record); + mRecords[iter->second]->setModified (record); } } @@ -365,7 +366,7 @@ namespace CSMWorld template std::string Collection::getId (int index) const { - return IdAccessorT().getId (mRecords.at (index).get()); + return IdAccessorT().getId (mRecords.at (index)->get()); } template @@ -388,13 +389,13 @@ namespace CSMWorld template QVariant Collection::getData (int index, int column) const { - return mColumns.at (column)->get (mRecords.at (index)); + return mColumns.at (column)->get (*mRecords.at (index)); } template void Collection::setData (int index, int column, const QVariant& data) { - return mColumns.at (column)->set (mRecords.at (index), data); + return mColumns.at (column)->set (*mRecords.at (index), data); } template @@ -421,8 +422,8 @@ namespace CSMWorld template void Collection::merge() { - for (typename std::vector >::iterator iter (mRecords.begin()); iter!=mRecords.end(); ++iter) - iter->merge(); + for (typename std::vector > >::iterator iter (mRecords.begin()); iter!=mRecords.end(); ++iter) + (*iter)->merge(); purge(); } @@ -434,7 +435,7 @@ namespace CSMWorld while (i (mRecords.size())) { - if (mRecords[i].isErased()) + if (mRecords[i]->isErased()) removeRows (i, 1); else ++i; @@ -475,11 +476,11 @@ namespace CSMWorld IdAccessorT().setId(record, id); record.blank(); - Record record2; - record2.mState = Record::State_ModifiedOnly; - record2.mModified = record; + std::unique_ptr > record2(new Record); + record2->mState = Record::State_ModifiedOnly; + record2->mModified = record; - insertRecord (record2, getAppendIndex (id, type), type); + insertRecord (std::move(record2), getAppendIndex (id, type), type); } template @@ -496,18 +497,19 @@ namespace CSMWorld } template - void Collection::replace (int index, const RecordBase& record) + void Collection::replace (int index, std::unique_ptr record) { - mRecords.at (index) = dynamic_cast&> (record); + std::unique_ptr > tmp(static_cast*>(record.release())); + mRecords.at (index) = std::move(tmp); } template - void Collection::appendRecord (const RecordBase& record, + void Collection::appendRecord (std::unique_ptr record, UniversalId::Type type) { - insertRecord (record, - getAppendIndex (IdAccessorT().getId ( - dynamic_cast&> (record).get()), type), type); + int index = + getAppendIndex(IdAccessorT().getId(static_cast*>(record.get())->get()), type); + insertRecord (std::move(record), index, type); } template @@ -525,8 +527,8 @@ namespace CSMWorld for (typename std::map::const_iterator iter = mIndex.begin(); iter!=mIndex.end(); ++iter) { - if (listDeleted || !mRecords[iter->second].isDeleted()) - ids.push_back (IdAccessorT().getId (mRecords[iter->second].get())); + if (listDeleted || !mRecords[iter->second]->isDeleted()) + ids.push_back (IdAccessorT().getId (mRecords[iter->second]->get())); } return ids; @@ -536,25 +538,26 @@ namespace CSMWorld const Record& Collection::getRecord (const std::string& id) const { int index = getIndex (id); - return mRecords.at (index); + return *mRecords.at (index); } template const Record& Collection::getRecord (int index) const { - return mRecords.at (index); + return *mRecords.at (index); } template - void Collection::insertRecord (const RecordBase& record, int index, + void Collection::insertRecord (std::unique_ptr record, int index, UniversalId::Type type) { if (index<0 || index>static_cast (mRecords.size())) throw std::runtime_error ("index out of range"); - const Record& record2 = dynamic_cast&> (record); + std::unique_ptr > record2(static_cast*>(record.release())); + std::string lowerId = Misc::StringUtils::lowerCase(IdAccessorT().getId(record2->get())); - mRecords.insert (mRecords.begin()+index, record2); + mRecords.insert (mRecords.begin()+index, std::move(record2)); if (index (mRecords.size())-1) { @@ -564,18 +567,18 @@ namespace CSMWorld ++(iter->second); } - mIndex.insert (std::make_pair (Misc::StringUtils::lowerCase (IdAccessorT().getId ( - record2.get())), index)); + mIndex.insert (std::make_pair (lowerId, index)); } template - void Collection::setRecord (int index, const Record& record) + void Collection::setRecord (int index, + std::unique_ptr > record) { - if (Misc::StringUtils::lowerCase (IdAccessorT().getId (mRecords.at (index).get()))!= - Misc::StringUtils::lowerCase (IdAccessorT().getId (record.get()))) + if (Misc::StringUtils::lowerCase (IdAccessorT().getId (mRecords.at (index)->get())) != + Misc::StringUtils::lowerCase (IdAccessorT().getId (record->get()))) throw std::runtime_error ("attempt to change the ID of a record"); - mRecords.at (index) = record; + mRecords.at (index) = std::move(record); } template diff --git a/apps/opencs/model/world/collectionbase.hpp b/apps/opencs/model/world/collectionbase.hpp index bac790c5d0..ae06d892f5 100644 --- a/apps/opencs/model/world/collectionbase.hpp +++ b/apps/opencs/model/world/collectionbase.hpp @@ -3,6 +3,7 @@ #include #include +#include #include "universalid.hpp" #include "columns.hpp" @@ -64,13 +65,13 @@ namespace CSMWorld ////< Search record with \a id. /// \return index of record (if found) or -1 (not found) - virtual void replace (int index, const RecordBase& record) = 0; + virtual void replace (int index, std::unique_ptr record) = 0; ///< If the record type does not match, an exception is thrown. /// /// \attention \a record must not change the ID. ///< \param type Will be ignored, unless the collection supports multiple record types - virtual void appendRecord (const RecordBase& record, + virtual void appendRecord (std::unique_ptr record, UniversalId::Type type = UniversalId::Type_None) = 0; ///< If the record type does not match, an exception is thrown. diff --git a/apps/opencs/model/world/commands.cpp b/apps/opencs/model/world/commands.cpp index 5ec7401dc9..e69600eff4 100644 --- a/apps/opencs/model/world/commands.cpp +++ b/apps/opencs/model/world/commands.cpp @@ -24,7 +24,7 @@ CSMWorld::TouchCommand::TouchCommand(IdTable& table, const std::string& id, QUnd , mChanged(false) { setText(("Touch " + mId).c_str()); - mOld.reset(mTable.getRecord(mId).clone()); + mOld.reset(mTable.getRecord(mId).clone().get()); } void CSMWorld::TouchCommand::redo() @@ -36,7 +36,7 @@ void CSMWorld::TouchCommand::undo() { if (mChanged) { - mTable.setRecord(mId, *mOld); + mTable.setRecord(mId, std::move(mOld)); mChanged = false; } } @@ -159,7 +159,7 @@ CSMWorld::TouchLandCommand::TouchLandCommand(IdTable& landTable, IdTable& ltexTa , mChanged(false) { setText(("Touch " + mId).c_str()); - mOld.reset(mLands.getRecord(mId).clone()); + mOld.reset(mLands.getRecord(mId).clone().get()); } const std::string& CSMWorld::TouchLandCommand::getOriginId() const @@ -181,7 +181,7 @@ void CSMWorld::TouchLandCommand::onUndo() { if (mChanged) { - mLands.setRecord(mId, *mOld); + mLands.setRecord(mId, std::move(mOld)); mChanged = false; } } @@ -291,16 +291,15 @@ void CSMWorld::CreateCommand::undo() } CSMWorld::RevertCommand::RevertCommand (IdTable& model, const std::string& id, QUndoCommand* parent) -: QUndoCommand (parent), mModel (model), mId (id), mOld (nullptr) +: QUndoCommand (parent), mModel (model), mId (id) { setText (("Revert record " + id).c_str()); - mOld = model.getRecord (id).clone(); + mOld = std::move(model.getRecord (id).clone()); } CSMWorld::RevertCommand::~RevertCommand() { - delete mOld; } void CSMWorld::RevertCommand::redo() @@ -322,21 +321,20 @@ void CSMWorld::RevertCommand::redo() void CSMWorld::RevertCommand::undo() { - mModel.setRecord (mId, *mOld); + mModel.setRecord (mId, std::move(mOld)); } CSMWorld::DeleteCommand::DeleteCommand (IdTable& model, const std::string& id, CSMWorld::UniversalId::Type type, QUndoCommand* parent) -: QUndoCommand (parent), mModel (model), mId (id), mOld (nullptr), mType(type) +: QUndoCommand (parent), mModel (model), mId (id), mType(type) { setText (("Delete record " + id).c_str()); - mOld = model.getRecord (id).clone(); + mOld = std::move(model.getRecord (id).clone()); } CSMWorld::DeleteCommand::~DeleteCommand() { - delete mOld; } void CSMWorld::DeleteCommand::redo() @@ -358,7 +356,7 @@ void CSMWorld::DeleteCommand::redo() void CSMWorld::DeleteCommand::undo() { - mModel.setRecord (mId, *mOld, mType); + mModel.setRecord (mId, std::move(mOld), mType); } @@ -424,18 +422,19 @@ void CSMWorld::CreatePathgridCommand::redo() { CreateCommand::redo(); - Record record = static_cast& >(mModel.getRecord(mId)); - record.get().blank(); - record.get().mCell = mId; + std::unique_ptr > record + = std::make_unique >(static_cast& >(mModel.getRecord(mId))); + record->get().blank(); + record->get().mCell = mId; std::pair coords = CellCoordinates::fromId(mId); if (coords.second) { - record.get().mData.mX = coords.first.getX(); - record.get().mData.mY = coords.first.getY(); + record->get().mData.mX = coords.first.getX(); + record->get().mData.mY = coords.first.getY(); } - mModel.setRecord(mId, record, mType); + mModel.setRecord(mId, std::move(record), mType); } CSMWorld::UpdateCellCommand::UpdateCellCommand (IdTable& model, int row, QUndoCommand *parent) diff --git a/apps/opencs/model/world/commands.hpp b/apps/opencs/model/world/commands.hpp index 33608304f4..2ed629ef5c 100644 --- a/apps/opencs/model/world/commands.hpp +++ b/apps/opencs/model/world/commands.hpp @@ -203,7 +203,7 @@ namespace CSMWorld { IdTable& mModel; std::string mId; - RecordBase *mOld; + std::unique_ptr mOld; // not implemented RevertCommand (const RevertCommand&); @@ -224,7 +224,7 @@ namespace CSMWorld { IdTable& mModel; std::string mId; - RecordBase *mOld; + std::unique_ptr mOld; UniversalId::Type mType; // not implemented diff --git a/apps/opencs/model/world/data.cpp b/apps/opencs/model/world/data.cpp index 4ccd2a06db..7d6f0f37cb 100644 --- a/apps/opencs/model/world/data.cpp +++ b/apps/opencs/model/world/data.cpp @@ -917,8 +917,8 @@ const CSMWorld::MetaData& CSMWorld::Data::getMetaData() const void CSMWorld::Data::setMetaData (const MetaData& metaData) { - Record record (RecordBase::State_ModifiedOnly, nullptr, &metaData); - mMetaData.setRecord (0, record); + mMetaData.setRecord (0, std::make_unique >( + Record(RecordBase::State_ModifiedOnly, nullptr, &metaData))); } QAbstractItemModel *CSMWorld::Data::getTableModel (const CSMWorld::UniversalId& id) @@ -983,7 +983,8 @@ int CSMWorld::Data::startLoading (const boost::filesystem::path& path, bool base metaData.mId = "sys::meta"; metaData.load (*mReader); - mMetaData.setRecord (0, Record (RecordBase::State_ModifiedOnly, nullptr, &metaData)); + mMetaData.setRecord (0, std::make_unique >( + Record (RecordBase::State_ModifiedOnly, nullptr, &metaData))); } return mReader->getRecordCount(); @@ -1011,10 +1012,10 @@ void CSMWorld::Data::loadFallbackEntries() ESM::Static newMarker; newMarker.mId = marker.first; newMarker.mModel = marker.second; - CSMWorld::Record record; - record.mBase = newMarker; - record.mState = CSMWorld::RecordBase::State_BaseOnly; - mReferenceables.appendRecord (record, CSMWorld::UniversalId::Type_Static); + std::unique_ptr > record(new CSMWorld::Record); + record->mBase = newMarker; + record->mState = CSMWorld::RecordBase::State_BaseOnly; + mReferenceables.appendRecord (std::move(record), CSMWorld::UniversalId::Type_Static); } } @@ -1025,10 +1026,10 @@ void CSMWorld::Data::loadFallbackEntries() ESM::Door newMarker; newMarker.mId = marker.first; newMarker.mModel = marker.second; - CSMWorld::Record record; - record.mBase = newMarker; - record.mState = CSMWorld::RecordBase::State_BaseOnly; - mReferenceables.appendRecord (record, CSMWorld::UniversalId::Type_Door); + std::unique_ptr > record(new CSMWorld::Record); + record->mBase = newMarker; + record->mState = CSMWorld::RecordBase::State_BaseOnly; + mReferenceables.appendRecord (std::move(record), CSMWorld::UniversalId::Type_Door); } } } diff --git a/apps/opencs/model/world/idcollection.hpp b/apps/opencs/model/world/idcollection.hpp index 7849aab926..9f834d8fae 100644 --- a/apps/opencs/model/world/idcollection.hpp +++ b/apps/opencs/model/world/idcollection.hpp @@ -83,9 +83,9 @@ namespace CSMWorld return -1; } - Record baseRecord = this->getRecord (index); - baseRecord.mState = RecordBase::State_Deleted; - this->setRecord (index, baseRecord); + std::unique_ptr > baseRecord(new Record(this->getRecord(index))); + baseRecord->mState = RecordBase::State_Deleted; + this->setRecord(index, std::move(baseRecord)); return index; } @@ -96,30 +96,31 @@ namespace CSMWorld int IdCollection::load (const ESXRecordT& record, bool base, int index) { - if (index==-2) + if (index==-2) // index unknown index = this->searchId (IdAccessorT().getId (record)); if (index==-1) { // new record - Record record2; - record2.mState = base ? RecordBase::State_BaseOnly : RecordBase::State_ModifiedOnly; - (base ? record2.mBase : record2.mModified) = record; + std::unique_ptr > record2(new Record); + record2->mState = base ? RecordBase::State_BaseOnly : RecordBase::State_ModifiedOnly; + (base ? record2->mBase : record2->mModified) = record; index = this->getSize(); - this->appendRecord (record2); + this->appendRecord(std::move(record2)); } else { // old record - Record record2 = Collection::getRecord (index); + std::unique_ptr > record2( + new Record(Collection::getRecord(index))); if (base) - record2.mBase = record; + record2->mBase = record; else - record2.setModified (record); + record2->setModified(record); - this->setRecord (index, record2); + this->setRecord(index, std::move(record2)); } return index; @@ -133,7 +134,7 @@ namespace CSMWorld if (index==-1) return false; - Record record = Collection::getRecord (index); + const Record& record = Collection::getRecord (index); if (record.isDeleted()) return false; @@ -144,8 +145,10 @@ namespace CSMWorld } else { - record.mState = RecordBase::State_Deleted; - this->setRecord (index, record); + std::unique_ptr > record2( + new Record(Collection::getRecord(index))); + record2->mState = RecordBase::State_Deleted; + this->setRecord(index, std::move(record2)); } return true; diff --git a/apps/opencs/model/world/idtable.cpp b/apps/opencs/model/world/idtable.cpp index 30fe6f639a..bce934bd9f 100644 --- a/apps/opencs/model/world/idtable.cpp +++ b/apps/opencs/model/world/idtable.cpp @@ -228,23 +228,24 @@ QModelIndex CSMWorld::IdTable::getModelIndex (const std::string& id, int column) return QModelIndex(); } -void CSMWorld::IdTable::setRecord (const std::string& id, const RecordBase& record, CSMWorld::UniversalId::Type type) +void CSMWorld::IdTable::setRecord (const std::string& id, + std::unique_ptr record, CSMWorld::UniversalId::Type type) { int index = mIdCollection->searchId (id); if (index==-1) { - index = mIdCollection->getAppendIndex (id, type); + int index2 = mIdCollection->getAppendIndex (id, type); - beginInsertRows (QModelIndex(), index, index); + beginInsertRows (QModelIndex(), index2, index2); - mIdCollection->appendRecord (record, type); + mIdCollection->appendRecord (std::move(record), type); endInsertRows(); } else { - mIdCollection->replace (index, record); + mIdCollection->replace (index, std::move(record)); emit dataChanged (CSMWorld::IdTable::index (index, 0), CSMWorld::IdTable::index (index, mIdCollection->getColumns()-1)); } diff --git a/apps/opencs/model/world/idtable.hpp b/apps/opencs/model/world/idtable.hpp index 6b7b8d3182..c38e1b5f9c 100644 --- a/apps/opencs/model/world/idtable.hpp +++ b/apps/opencs/model/world/idtable.hpp @@ -2,6 +2,7 @@ #define CSM_WOLRD_IDTABLE_H #include +#include #include "idtablebase.hpp" #include "universalid.hpp" @@ -67,7 +68,7 @@ namespace CSMWorld QModelIndex getModelIndex (const std::string& id, int column) const override; - void setRecord (const std::string& id, const RecordBase& record, + void setRecord (const std::string& id, std::unique_ptr record, UniversalId::Type type = UniversalId::Type_None); ///< Add record or overwrite existing record. diff --git a/apps/opencs/model/world/infocollection.cpp b/apps/opencs/model/world/infocollection.cpp index be73aece6f..c64e21d3c7 100644 --- a/apps/opencs/model/world/infocollection.cpp +++ b/apps/opencs/model/world/infocollection.cpp @@ -15,23 +15,23 @@ void CSMWorld::InfoCollection::load (const Info& record, bool base) if (index==-1) { // new record - Record record2; - record2.mState = base ? RecordBase::State_BaseOnly : RecordBase::State_ModifiedOnly; - (base ? record2.mBase : record2.mModified) = record; + std::unique_ptr > record2(new Record); + record2->mState = base ? RecordBase::State_BaseOnly : RecordBase::State_ModifiedOnly; + (base ? record2->mBase : record2->mModified) = record; - std::string topic = Misc::StringUtils::lowerCase (record2.get().mTopicId); + std::string topic = Misc::StringUtils::lowerCase (record2->get().mTopicId); - if (!record2.get().mPrev.empty()) + if (!record2->get().mPrev.empty()) { - index = getInfoIndex (record2.get().mPrev, topic); + index = getInfoIndex (record2->get().mPrev, topic); // WARN: index repurposed if (index!=-1) ++index; } - if (index==-1 && !record2.get().mNext.empty()) + if (index==-1 && !record2->get().mNext.empty()) { - index = getInfoIndex (record2.get().mNext, topic); + index = getInfoIndex (record2->get().mNext, topic); } if (index==-1) @@ -41,19 +41,19 @@ void CSMWorld::InfoCollection::load (const Info& record, bool base) index = std::distance (getRecords().begin(), range.second); } - insertRecord (record2, index); + insertRecord (std::move(record2), index); } else { // old record - Record record2 = getRecord (index); + std::unique_ptr > record2(new Record(getRecord(index))); if (base) - record2.mBase = record; + record2->mBase = record; else - record2.setModified (record); + record2->setModified (record); - setRecord (index, record2); + setRecord (index, std::move(record2)); } } @@ -64,7 +64,7 @@ int CSMWorld::InfoCollection::getInfoIndex (const std::string& id, const std::st std::pair range = getTopicRange (topic); for (; range.first!=range.second; ++range.first) - if (Misc::StringUtils::ciEqual(range.first->get().mId, fullId)) + if (Misc::StringUtils::ciEqual((*range.first).get()->get().mId, fullId)) return std::distance (getRecords().begin(), range.first); return -1; @@ -126,9 +126,9 @@ void CSMWorld::InfoCollection::load (ESM::ESMReader& reader, bool base, const ES } else { - Record record = getRecord (index); - record.mState = RecordBase::State_Deleted; - setRecord (index, record); + std::unique_ptr > record(new Record(getRecord(index))); + record->mState = RecordBase::State_Deleted; + setRecord (index, std::move(record)); } } else @@ -169,7 +169,7 @@ CSMWorld::InfoCollection::Range CSMWorld::InfoCollection::getTopicRange (const s while (begin != getRecords().begin()) { - if (!Misc::StringUtils::ciEqual(begin->get().mTopicId, topic2)) + if (!Misc::StringUtils::ciEqual((*begin)->get().mTopicId, topic2)) { // we've gone one too far, go back ++begin; @@ -182,7 +182,7 @@ CSMWorld::InfoCollection::Range CSMWorld::InfoCollection::getTopicRange (const s RecordConstIterator end = begin; for (; end!=getRecords().end(); ++end) - if (!Misc::StringUtils::ciEqual(end->get().mTopicId, topic2)) + if (!Misc::StringUtils::ciEqual((*end)->get().mTopicId, topic2)) break; return Range (begin, end); @@ -197,7 +197,7 @@ void CSMWorld::InfoCollection::removeDialogueInfos(const std::string& dialogueId std::map::const_iterator end = getIdMap().end(); for (; current != end; ++current) { - Record record = getRecord(current->second); + const Record& record = getRecord(current->second); if (Misc::StringUtils::ciEqual(dialogueId, record.get().mTopicId)) { @@ -207,8 +207,9 @@ void CSMWorld::InfoCollection::removeDialogueInfos(const std::string& dialogueId } else { - record.mState = RecordBase::State_Deleted; - setRecord(current->second, record); + std::unique_ptr > record2(new Record(record)); + record2->mState = RecordBase::State_Deleted; + setRecord(current->second, std::move(record2)); } } else diff --git a/apps/opencs/model/world/infocollection.hpp b/apps/opencs/model/world/infocollection.hpp index 8f5aea6012..f0379b3c14 100644 --- a/apps/opencs/model/world/infocollection.hpp +++ b/apps/opencs/model/world/infocollection.hpp @@ -15,7 +15,7 @@ namespace CSMWorld { public: - typedef std::vector >::const_iterator RecordConstIterator; + typedef std::vector > >::const_iterator RecordConstIterator; typedef std::pair Range; private: diff --git a/apps/opencs/model/world/nestedidcollection.hpp b/apps/opencs/model/world/nestedidcollection.hpp index 1bd20aeebf..4af864eb42 100644 --- a/apps/opencs/model/world/nestedidcollection.hpp +++ b/apps/opencs/model/world/nestedidcollection.hpp @@ -90,23 +90,23 @@ namespace CSMWorld template void NestedIdCollection::addNestedRow(int row, int column, int position) { - Record record; - record.assign(Collection::getRecord(row)); + std::unique_ptr > record(new Record); + record->assign(Collection::getRecord(row)); - getAdapter(Collection::getColumn(column)).addRow(record, position); + getAdapter(Collection::getColumn(column)).addRow(*record, position); - Collection::setRecord(row, record); + Collection::setRecord(row, std::move(record)); } template void NestedIdCollection::removeNestedRows(int row, int column, int subRow) { - Record record; - record.assign(Collection::getRecord(row)); + std::unique_ptr > record(new Record); + record->assign(Collection::getRecord(row)); - getAdapter(Collection::getColumn(column)).removeRow(record, subRow); + getAdapter(Collection::getColumn(column)).removeRow(*record, subRow); - Collection::setRecord(row, record); + Collection::setRecord(row, std::move(record)); } template @@ -121,13 +121,13 @@ namespace CSMWorld void NestedIdCollection::setNestedData(int row, int column, const QVariant& data, int subRow, int subColumn) { - Record record; - record.assign(Collection::getRecord(row)); + std::unique_ptr > record(new Record); + record->assign(Collection::getRecord(row)); getAdapter(Collection::getColumn(column)).setData( - record, data, subRow, subColumn); + *record, data, subRow, subColumn); - Collection::setRecord(row, record); + Collection::setRecord(row, std::move(record)); } template @@ -142,13 +142,13 @@ namespace CSMWorld void NestedIdCollection::setNestedTable(int row, int column, const CSMWorld::NestedTableWrapperBase& nestedTable) { - Record record; - record.assign(Collection::getRecord(row)); + std::unique_ptr > record(new Record); + record->assign(Collection::getRecord(row)); getAdapter(Collection::getColumn(column)).setTable( - record, nestedTable); + *record, nestedTable); - Collection::setRecord(row, record); + Collection::setRecord(row, std::move(record)); } template diff --git a/apps/opencs/model/world/nestedinfocollection.cpp b/apps/opencs/model/world/nestedinfocollection.cpp index 4abaaf9c02..d404bb9a63 100644 --- a/apps/opencs/model/world/nestedinfocollection.cpp +++ b/apps/opencs/model/world/nestedinfocollection.cpp @@ -35,22 +35,22 @@ namespace CSMWorld void NestedInfoCollection::addNestedRow(int row, int column, int position) { - Record record; - record.assign(Collection >::getRecord(row)); + std::unique_ptr > record(new Record); + record->assign(Collection >::getRecord(row)); - getAdapter(Collection >::getColumn(column)).addRow(record, position); + getAdapter(Collection >::getColumn(column)).addRow(*record, position); - Collection >::setRecord(row, record); + Collection >::setRecord(row, std::move(record)); } void NestedInfoCollection::removeNestedRows(int row, int column, int subRow) { - Record record; - record.assign(Collection >::getRecord(row)); + std::unique_ptr > record(new Record); + record->assign(Collection >::getRecord(row)); - getAdapter(Collection >::getColumn(column)).removeRow(record, subRow); + getAdapter(Collection >::getColumn(column)).removeRow(*record, subRow); - Collection >::setRecord(row, record); + Collection >::setRecord(row, std::move(record)); } QVariant NestedInfoCollection::getNestedData (int row, @@ -63,13 +63,13 @@ namespace CSMWorld void NestedInfoCollection::setNestedData(int row, int column, const QVariant& data, int subRow, int subColumn) { - Record record; - record.assign(Collection >::getRecord(row)); + std::unique_ptr > record(new Record); + record->assign(Collection >::getRecord(row)); getAdapter(Collection >::getColumn(column)).setData( - record, data, subRow, subColumn); + *record, data, subRow, subColumn); - Collection >::setRecord(row, record); + Collection >::setRecord(row, std::move(record)); } CSMWorld::NestedTableWrapperBase* NestedInfoCollection::nestedTable(int row, @@ -82,13 +82,13 @@ namespace CSMWorld void NestedInfoCollection::setNestedTable(int row, int column, const CSMWorld::NestedTableWrapperBase& nestedTable) { - Record record; - record.assign(Collection >::getRecord(row)); + std::unique_ptr > record(new Record); + record->assign(Collection >::getRecord(row)); getAdapter(Collection >::getColumn(column)).setTable( - record, nestedTable); + *record, nestedTable); - Collection >::setRecord(row, record); + Collection >::setRecord(row, std::move(record)); } int NestedInfoCollection::getNestedRowsCount(int row, int column) const diff --git a/apps/opencs/model/world/record.hpp b/apps/opencs/model/world/record.hpp index 5f67a93b12..eb4a74c343 100644 --- a/apps/opencs/model/world/record.hpp +++ b/apps/opencs/model/world/record.hpp @@ -1,6 +1,7 @@ #ifndef CSM_WOLRD_RECORD_H #define CSM_WOLRD_RECORD_H +#include #include namespace CSMWorld @@ -20,9 +21,9 @@ namespace CSMWorld virtual ~RecordBase(); - virtual RecordBase *clone() const = 0; + virtual std::unique_ptr clone() const = 0; - virtual RecordBase *modifiedCopy() const = 0; + virtual std::unique_ptr modifiedCopy() const = 0; virtual void assign (const RecordBase& record) = 0; ///< Will throw an exception if the types don't match. @@ -45,9 +46,9 @@ namespace CSMWorld Record(State state, const ESXRecordT *base = 0, const ESXRecordT *modified = 0); - RecordBase *clone() const override; + std::unique_ptr clone() const; - RecordBase *modifiedCopy() const override; + std::unique_ptr modifiedCopy() const; void assign (const RecordBase& record) override; @@ -85,15 +86,16 @@ namespace CSMWorld } template - RecordBase *Record::modifiedCopy() const + std::unique_ptr Record::modifiedCopy() const { - return new Record (State_ModifiedOnly, nullptr, &(this->get())); + return std::make_unique >( + Record(State_ModifiedOnly, nullptr, &(this->get()))); } template - RecordBase *Record::clone() const + std::unique_ptr Record::clone() const { - return new Record (*this); + return std::make_unique >(Record(*this)); } template diff --git a/apps/opencs/model/world/refcollection.cpp b/apps/opencs/model/world/refcollection.cpp index 003a300c06..f5210ca3e4 100644 --- a/apps/opencs/model/world/refcollection.cpp +++ b/apps/opencs/model/world/refcollection.cpp @@ -84,8 +84,6 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool int index = getIndex (iter->second); - Record record = getRecord (index); - if (base) { removeRows (index, 1); @@ -93,8 +91,9 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool } else { - record.mState = RecordBase::State_Deleted; - setRecord (index, record); + std::unique_ptr > record2(new Record(getRecord(index))); + record2->mState = RecordBase::State_Deleted; + setRecord(index, std::move(record2)); } continue; @@ -105,13 +104,13 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool // new reference ref.mId = getNewId(); - Record record; - record.mState = base ? RecordBase::State_BaseOnly : RecordBase::State_ModifiedOnly; + std::unique_ptr > record(new Record); + record->mState = base ? RecordBase::State_BaseOnly : RecordBase::State_ModifiedOnly; const ESM::RefNum refNum = ref.mRefNum; std::string refId = ref.mId; - (base ? record.mBase : record.mModified) = std::move(ref); + (base ? record->mBase : record->mModified) = std::move(ref); - appendRecord (record); + appendRecord(std::move(record)); cache.emplace(refNum, std::move(refId)); } @@ -122,11 +121,11 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool int index = getIndex (ref.mId); - Record record = getRecord (index); - record.mState = base ? RecordBase::State_BaseOnly : RecordBase::State_Modified; - (base ? record.mBase : record.mModified) = std::move(ref); + std::unique_ptr > record(new Record(getRecord(index))); + record->mState = base ? RecordBase::State_BaseOnly : RecordBase::State_Modified; + (base ? record->mBase : record->mModified) = std::move(ref); - setRecord (index, record); + setRecord(index, std::move(record)); } } } diff --git a/apps/opencs/model/world/refidcollection.cpp b/apps/opencs/model/world/refidcollection.cpp index 0cf9a4e6dc..7f3e71106c 100644 --- a/apps/opencs/model/world/refidcollection.cpp +++ b/apps/opencs/model/world/refidcollection.cpp @@ -796,18 +796,19 @@ int CSMWorld::RefIdCollection::searchId (const std::string& id) const return mData.localToGlobalIndex (localIndex); } -void CSMWorld::RefIdCollection::replace (int index, const RecordBase& record) +void CSMWorld::RefIdCollection::replace (int index, std::unique_ptr record) { - mData.getRecord (mData.globalToLocalIndex (index)).assign (record); + mData.getRecord (mData.globalToLocalIndex (index)).assign (*record.release()); } void CSMWorld::RefIdCollection::cloneRecord(const std::string& origin, const std::string& destination, const CSMWorld::UniversalId::Type type) { - std::unique_ptr newRecord(mData.getRecord(mData.searchId(origin)).modifiedCopy()); + std::unique_ptr newRecord = + std::move(mData.getRecord(mData.searchId(origin)).modifiedCopy()); mAdapters.find(type)->second->setId(*newRecord, destination); - mData.insertRecord(*newRecord, type, destination); + mData.insertRecord(std::move(newRecord), type, destination); } bool CSMWorld::RefIdCollection::touchRecord(const std::string& id) @@ -816,16 +817,16 @@ bool CSMWorld::RefIdCollection::touchRecord(const std::string& id) return false; } -void CSMWorld::RefIdCollection::appendRecord (const RecordBase& record, +void CSMWorld::RefIdCollection::appendRecord (std::unique_ptr record, UniversalId::Type type) { - std::string id = findAdapter (type).getId (record); + std::string id = findAdapter (type).getId (*record.get()); int index = mData.getAppendIndex (type); mData.appendRecord (type, id, false); - mData.getRecord (mData.globalToLocalIndex (index)).assign (record); + mData.getRecord (mData.globalToLocalIndex (index)).assign (*record.release()); } const CSMWorld::RecordBase& CSMWorld::RefIdCollection::getRecord (const std::string& id) const diff --git a/apps/opencs/model/world/refidcollection.hpp b/apps/opencs/model/world/refidcollection.hpp index 5b38e80da5..dc722055ff 100644 --- a/apps/opencs/model/world/refidcollection.hpp +++ b/apps/opencs/model/world/refidcollection.hpp @@ -89,12 +89,12 @@ namespace CSMWorld ////< Search record with \a id. /// \return index of record (if found) or -1 (not found) - void replace (int index, const RecordBase& record) override; + void replace (int index, std::unique_ptr record) override; ///< If the record type does not match, an exception is thrown. /// /// \attention \a record must not change the ID. - void appendRecord (const RecordBase& record, UniversalId::Type type) override; + void appendRecord (std::unique_ptr record, UniversalId::Type type) override; ///< If the record type does not match, an exception is thrown. /// ///< \param type Will be ignored, unless the collection supports multiple record types diff --git a/apps/opencs/model/world/refiddata.cpp b/apps/opencs/model/world/refiddata.cpp index 79a2efdcc8..df8e06c251 100644 --- a/apps/opencs/model/world/refiddata.cpp +++ b/apps/opencs/model/world/refiddata.cpp @@ -400,7 +400,7 @@ const CSMWorld::RefIdDataContainer< ESM::Static >& CSMWorld::RefIdData::getStati return mStatics; } -void CSMWorld::RefIdData::insertRecord (CSMWorld::RecordBase& record, CSMWorld::UniversalId::Type type, const std::string& id) +void CSMWorld::RefIdData::insertRecord (std::unique_ptr record, CSMWorld::UniversalId::Type type, const std::string& id) { std::map::iterator iter = mRecordContainers.find (type); @@ -408,7 +408,7 @@ void CSMWorld::RefIdData::insertRecord (CSMWorld::RecordBase& record, CSMWorld:: if (iter==mRecordContainers.end()) throw std::logic_error ("invalid local index type"); - iter->second->insertRecord(record); + iter->second->insertRecord(std::move(record)); mIndex.insert (std::make_pair (Misc::StringUtils::lowerCase (id), LocalIndex (iter->second->getSize()-1, type))); @@ -420,9 +420,7 @@ void CSMWorld::RefIdData::copyTo (int index, RefIdData& target) const RefIdDataContainerBase *source = mRecordContainers.find (localIndex.second)->second; - std::string id = source->getId (localIndex.first); - - std::unique_ptr newRecord (source->getRecord (localIndex.first).modifiedCopy()); - - target.insertRecord (*newRecord, localIndex.second, id); + target.insertRecord(source->getRecord(localIndex.first).modifiedCopy(), + localIndex.second, + source->getId(localIndex.first)); } diff --git a/apps/opencs/model/world/refiddata.hpp b/apps/opencs/model/world/refiddata.hpp index ab5269b390..7eeb936985 100644 --- a/apps/opencs/model/world/refiddata.hpp +++ b/apps/opencs/model/world/refiddata.hpp @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -51,7 +52,7 @@ namespace CSMWorld virtual void appendRecord (const std::string& id, bool base) = 0; - virtual void insertRecord (RecordBase& record) = 0; + virtual void insertRecord (std::unique_ptr record) = 0; virtual int load (ESM::ESMReader& reader, bool base) = 0; ///< \return index of a loaded record or -1 if no record was loaded @@ -66,7 +67,7 @@ namespace CSMWorld template struct RefIdDataContainer : public RefIdDataContainerBase { - std::vector > mContainer; + std::vector > > mContainer; int getSize() const override; @@ -78,7 +79,7 @@ namespace CSMWorld void appendRecord (const std::string& id, bool base) override; - void insertRecord (RecordBase& record) override; + void insertRecord (std::unique_ptr record) override; int load (ESM::ESMReader& reader, bool base) override; ///< \return index of a loaded record or -1 if no record was loaded @@ -91,10 +92,18 @@ namespace CSMWorld }; template - void RefIdDataContainer::insertRecord(RecordBase& record) + void RefIdDataContainer::insertRecord(std::unique_ptr record) { - Record& newRecord = dynamic_cast& >(record); - mContainer.push_back(newRecord); + Record *tmp = dynamic_cast*>(record.get()); + if(tmp != nullptr) + { + record.release(); + std::unique_ptr > newRecord; + newRecord.reset(tmp); + mContainer.push_back(std::move(newRecord)); + } + else + throw std::runtime_error ("invalid record for RefIdDataContainer"); } template @@ -106,33 +115,33 @@ namespace CSMWorld template const RecordBase& RefIdDataContainer::getRecord (int index) const { - return mContainer.at (index); + return *mContainer.at (index); } template RecordBase& RefIdDataContainer::getRecord (int index) { - return mContainer.at (index); + return *mContainer.at (index); } template unsigned int RefIdDataContainer::getRecordFlags (int index) const { - return mContainer.at (index).get().mRecordFlags; + return mContainer.at (index)->get().mRecordFlags; } template void RefIdDataContainer::appendRecord (const std::string& id, bool base) { - Record record; + std::unique_ptr > record(new Record); - record.mState = base ? RecordBase::State_BaseOnly : RecordBase::State_ModifiedOnly; + record->mState = base ? RecordBase::State_BaseOnly : RecordBase::State_ModifiedOnly; - record.mBase.mId = id; - record.mModified.mId = id; - (base ? record.mBase : record.mModified).blank(); + record->mBase.mId = id; + record->mModified.mId = id; + (base ? record->mBase : record->mModified).blank(); - mContainer.push_back (record); + mContainer.push_back (std::move(record)); } template @@ -147,7 +156,7 @@ namespace CSMWorld int numRecords = static_cast(mContainer.size()); for (; index < numRecords; ++index) { - if (Misc::StringUtils::ciEqual(mContainer[index].get().mId, record.mId)) + if (Misc::StringUtils::ciEqual(mContainer[index]->get().mId, record.mId)) { break; } @@ -165,7 +174,7 @@ namespace CSMWorld // Flag the record as Deleted even for a base content file. // RefIdData is responsible for its erasure. - mContainer[index].mState = RecordBase::State_Deleted; + mContainer[index]->mState = RecordBase::State_Deleted; } else { @@ -174,22 +183,22 @@ namespace CSMWorld appendRecord(record.mId, base); if (base) { - mContainer.back().mBase = record; + mContainer.back()->mBase = record; } else { - mContainer.back().mModified = record; + mContainer.back()->mModified = record; } } else if (!base) { - mContainer[index].setModified(record); + mContainer[index]->setModified(record); } else { // Overwrite - mContainer[index].setModified(record); - mContainer[index].merge(); + mContainer[index]->setModified(record); + mContainer[index]->merge(); } } @@ -208,13 +217,13 @@ namespace CSMWorld template std::string RefIdDataContainer::getId (int index) const { - return mContainer.at (index).get().mId; + return mContainer.at (index)->get().mId; } template void RefIdDataContainer::save (int index, ESM::ESMWriter& writer) const { - Record record = mContainer.at(index); + const Record& record = *mContainer.at(index); if (record.isModified() || record.mState == RecordBase::State_Deleted) { @@ -276,7 +285,7 @@ namespace CSMWorld void erase (int index, int count); - void insertRecord (CSMWorld::RecordBase& record, CSMWorld::UniversalId::Type type, + void insertRecord (std::unique_ptr record, CSMWorld::UniversalId::Type type, const std::string& id); const RecordBase& getRecord (const LocalIndex& index) const; From 66bda842405b431c50d781db036b6a51128a99b5 Mon Sep 17 00:00:00 2001 From: cc9cii Date: Fri, 23 Jul 2021 15:30:33 +1000 Subject: [PATCH 02/11] Convert RefNum index map to use find(). (copied the changes from commits 68e16b6cee0f533027e1d8f1293b8a5582ac5a68 and 0de223c637d0820a97417d84796c1bfeb1836fb8) NOTE: it is unclear how this change affects commit 61a4a0807b4ed2398b8169b477132b12a1a3a106 --- apps/opencs/model/world/data.hpp | 2 +- apps/opencs/model/world/refcollection.cpp | 22 ++++++++-------------- apps/opencs/model/world/refcollection.hpp | 2 +- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/apps/opencs/model/world/data.hpp b/apps/opencs/model/world/data.hpp index 5e2054b712..f7a51b289f 100644 --- a/apps/opencs/model/world/data.hpp +++ b/apps/opencs/model/world/data.hpp @@ -118,7 +118,7 @@ namespace CSMWorld const ESM::Dialogue *mDialogue; // last loaded dialogue bool mBase; bool mProject; - std::map > mRefLoadCache; + std::map > mRefLoadCache; int mReaderIndex; bool mFsStrict; diff --git a/apps/opencs/model/world/refcollection.cpp b/apps/opencs/model/world/refcollection.cpp index f5210ca3e4..a26bf1d2f4 100644 --- a/apps/opencs/model/world/refcollection.cpp +++ b/apps/opencs/model/world/refcollection.cpp @@ -8,7 +8,7 @@ #include "record.hpp" 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); @@ -60,16 +60,12 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool else ref.mCell = cell2.mId; - // ignore content file number - std::map::iterator iter = cache.begin(); - unsigned int thisIndex = ref.mRefNum.mIndex & 0x00ffffff; - if (ref.mRefNum.mContentFile != -1 && !base) ref.mRefNum.mContentFile = ref.mRefNum.mIndex >> 24; + unsigned int refNum = (ref.mRefNum.mIndex & 0x00ffffff) | + (ref.mRefNum.hasContentFile() ? ref.mRefNum.mContentFile : 0xff) << 24; - for (; iter != cache.end(); ++iter) - { - if (thisIndex == iter->first.mIndex) - break; - } + std::map::iterator iter = cache.find(refNum); + + if (ref.mRefNum.mContentFile != -1 && !base) ref.mRefNum.mContentFile = ref.mRefNum.mIndex >> 24; if (isDeleted) { @@ -104,15 +100,13 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool // new reference ref.mId = getNewId(); + cache.emplace(refNum, ref.mId); + std::unique_ptr > record(new Record); record->mState = base ? RecordBase::State_BaseOnly : RecordBase::State_ModifiedOnly; - const ESM::RefNum refNum = ref.mRefNum; - std::string refId = ref.mId; (base ? record->mBase : record->mModified) = std::move(ref); appendRecord(std::move(record)); - - cache.emplace(refNum, std::move(refId)); } else { diff --git a/apps/opencs/model/world/refcollection.hpp b/apps/opencs/model/world/refcollection.hpp index d031398d3f..42bb363570 100644 --- a/apps/opencs/model/world/refcollection.hpp +++ b/apps/opencs/model/world/refcollection.hpp @@ -27,7 +27,7 @@ namespace CSMWorld {} void load (ESM::ESMReader& reader, int cellIndex, bool base, - std::map& cache, CSMDoc::Messages& messages); + std::map& cache, CSMDoc::Messages& messages); ///< Load a sequence of references. std::string getNewId(); From 5fffcab94f57c8f9f20404d795ea3cd1de89fe9b Mon Sep 17 00:00:00 2001 From: cc9cii Date: Fri, 23 Jul 2021 16:05:58 +1000 Subject: [PATCH 03/11] Performance improvements for loading Info records. - The order of info records with the same topic are maintained in Collection::mRecords - The index lookup data structure are not ordered. The topic string is hashed. The infos for the topic are simply placed in a vector. - The index values for appending or inserting a record takes prev/next values (if exist) - FIXME: prev/next values are not adjusted for adding or removing records - FIXME: undo after reordering does not reset the modified flag (copied the changes from commit SHA-1: 06f9922822bf5a076894bce44bde37234d7ccee1) --- CMakeLists.txt | 1 + apps/opencs/CMakeLists.txt | 1 + apps/opencs/model/world/collection.hpp | 8 - apps/opencs/model/world/collectionbase.cpp | 5 + apps/opencs/model/world/collectionbase.hpp | 6 + apps/opencs/model/world/idtable.cpp | 8 +- apps/opencs/model/world/infocollection.cpp | 335 ++++++++++--- apps/opencs/model/world/infocollection.hpp | 76 ++- extern/murmurhash/CMakeLists.txt | 17 + extern/murmurhash/MurmurHash2.cpp | 522 +++++++++++++++++++++ extern/murmurhash/MurmurHash2.h | 38 ++ 11 files changed, 929 insertions(+), 88 deletions(-) create mode 100644 extern/murmurhash/CMakeLists.txt create mode 100644 extern/murmurhash/MurmurHash2.cpp create mode 100644 extern/murmurhash/MurmurHash2.h diff --git a/CMakeLists.txt b/CMakeLists.txt index b69cb5b71f..f40d77c1dc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -536,6 +536,7 @@ endif (CMAKE_CXX_COMPILER_ID STREQUAL GNU OR CMAKE_CXX_COMPILER_ID STREQUAL Clan add_subdirectory (extern/osg-ffmpeg-videoplayer) add_subdirectory (extern/oics) add_subdirectory (extern/Base64) +add_subdirectory (extern/murmurhash) if (BUILD_OPENCS) add_subdirectory (extern/osgQt) endif() diff --git a/apps/opencs/CMakeLists.txt b/apps/opencs/CMakeLists.txt index 88c4233c9c..9a70950879 100644 --- a/apps/opencs/CMakeLists.txt +++ b/apps/opencs/CMakeLists.txt @@ -226,6 +226,7 @@ target_link_libraries(openmw-cs ${OSGTEXT_LIBRARIES} ${OSG_LIBRARIES} ${EXTERN_OSGQT_LIBRARY} + ${MURMURHASH_LIBRARIES} ${Boost_SYSTEM_LIBRARY} ${Boost_FILESYSTEM_LIBRARY} ${Boost_PROGRAM_OPTIONS_LIBRARY} diff --git a/apps/opencs/model/world/collection.hpp b/apps/opencs/model/world/collection.hpp index 8fc5d076ce..89430f1c58 100644 --- a/apps/opencs/model/world/collection.hpp +++ b/apps/opencs/model/world/collection.hpp @@ -95,8 +95,6 @@ namespace CSMWorld protected: - const std::map& getIdMap() const; - const std::vector > >& getRecords() const; bool reorderRowsImp (int baseIndex, const std::vector& newOrder); @@ -205,12 +203,6 @@ namespace CSMWorld NestableColumn *getNestableColumn (int column) const; }; - template - const std::map& Collection::getIdMap() const - { - return mIndex; - } - template const std::vector > >& Collection::getRecords() const { diff --git a/apps/opencs/model/world/collectionbase.cpp b/apps/opencs/model/world/collectionbase.cpp index 6134dc1727..f20fc643e2 100644 --- a/apps/opencs/model/world/collectionbase.cpp +++ b/apps/opencs/model/world/collectionbase.cpp @@ -8,6 +8,11 @@ CSMWorld::CollectionBase::CollectionBase() {} CSMWorld::CollectionBase::~CollectionBase() {} +int CSMWorld::CollectionBase::getInsertIndex (const std::string& id, UniversalId::Type type, RecordBase *record) const +{ + return getAppendIndex(id, type); +} + int CSMWorld::CollectionBase::searchColumnIndex (Columns::ColumnId id) const { int columns = getColumns(); diff --git a/apps/opencs/model/world/collectionbase.hpp b/apps/opencs/model/world/collectionbase.hpp index ae06d892f5..13471b9886 100644 --- a/apps/opencs/model/world/collectionbase.hpp +++ b/apps/opencs/model/world/collectionbase.hpp @@ -100,6 +100,12 @@ namespace CSMWorld /// /// \return Success? + virtual int getInsertIndex (const std::string& id, + UniversalId::Type type = UniversalId::Type_None, + RecordBase *record = nullptr) const; + ///< Works like getAppendIndex unless an overloaded method uses the record pointer + /// to get additional info about the record that results in an alternative index. + int searchColumnIndex (Columns::ColumnId id) const; ///< Return index of column with the given \a id. If no such column exists, -1 is returned. diff --git a/apps/opencs/model/world/idtable.cpp b/apps/opencs/model/world/idtable.cpp index bce934bd9f..8fb3cd6593 100644 --- a/apps/opencs/model/world/idtable.cpp +++ b/apps/opencs/model/world/idtable.cpp @@ -235,7 +235,13 @@ void CSMWorld::IdTable::setRecord (const std::string& id, if (index==-1) { - int index2 = mIdCollection->getAppendIndex (id, type); + // For info records, appendRecord may use a different index than the one returned by + // getAppendIndex (because of prev/next links). This can result in the display not + // updating correctly after an undo + // + // Use an alternative method to get the correct index. For non-Info records the + // record pointer is ignored and internally calls getAppendIndex. + int index2 = mIdCollection->getInsertIndex (id, type, record.get()); beginInsertRows (QModelIndex(), index2, index2); diff --git a/apps/opencs/model/world/infocollection.cpp b/apps/opencs/model/world/infocollection.cpp index c64e21d3c7..d229ddacc2 100644 --- a/apps/opencs/model/world/infocollection.cpp +++ b/apps/opencs/model/world/infocollection.cpp @@ -8,40 +8,79 @@ #include -void CSMWorld::InfoCollection::load (const Info& record, bool base) +namespace CSMWorld { - int index = searchId (record.mId); + template<> + void Collection >::removeRows (int index, int count) + { + mRecords.erase(mRecords.begin()+index, mRecords.begin()+index+count); - if (index==-1) + // index map is updated in InfoCollection::removeRows() + } + + template<> + void Collection >::insertRecord (std::unique_ptr record, + int index, UniversalId::Type type) { - // new record - std::unique_ptr > record2(new Record); - record2->mState = base ? RecordBase::State_BaseOnly : RecordBase::State_ModifiedOnly; - (base ? record2->mBase : record2->mModified) = record; + int size = static_cast(mRecords.size()); + if (index < 0 || index > size) + throw std::runtime_error("index out of range"); - std::string topic = Misc::StringUtils::lowerCase (record2->get().mTopicId); + std::unique_ptr > record2(static_cast*>(record.release())); - if (!record2->get().mPrev.empty()) - { - index = getInfoIndex (record2->get().mPrev, topic); // WARN: index repurposed + if (index == size) + mRecords.push_back(std::move(record2)); + else + mRecords.insert(mRecords.begin()+index, std::move(record2)); - if (index!=-1) - ++index; - } + // index map is updated in InfoCollection::insertRecord() + } - if (index==-1 && !record2->get().mNext.empty()) + template<> + bool Collection >::reorderRowsImp (int baseIndex, + const std::vector& newOrder) + { + if (!newOrder.empty()) { - index = getInfoIndex (record2->get().mNext, topic); - } + int size = static_cast(newOrder.size()); - if (index==-1) - { - Range range = getTopicRange (topic); + // check that all indices are present + std::vector test(newOrder); + std::sort(test.begin(), test.end()); + if (*test.begin() != 0 || *--test.end() != size-1) + return false; + + // reorder records + std::vector > > buffer(size); + + // FIXME: BUG: undo does not remove modified flag + for (int i = 0; i < size; ++i) + { + buffer[newOrder[i]] = std::move(mRecords[baseIndex+i]); + buffer[newOrder[i]]->setModified(buffer[newOrder[i]]->get()); + } + + std::move(buffer.begin(), buffer.end(), mRecords.begin()+baseIndex); - index = std::distance (getRecords().begin(), range.second); + // index map is updated in InfoCollection::reorderRows() } - insertRecord (std::move(record2), index); + return true; + } +} + +void CSMWorld::InfoCollection::load (const Info& record, bool base) +{ + int index = searchId (record.mId); + + if (index==-1) + { + // new record + std::unique_ptr > record2(new Record); + record2->mState = base ? RecordBase::State_BaseOnly : RecordBase::State_ModifiedOnly; + (base ? record2->mBase : record2->mModified) = record; + + appendRecord(std::move(record2)); } else { @@ -59,30 +98,74 @@ void CSMWorld::InfoCollection::load (const Info& record, bool base) int CSMWorld::InfoCollection::getInfoIndex (const std::string& id, const std::string& topic) const { - std::string fullId = Misc::StringUtils::lowerCase (topic) + "#" + id; + // find the topic first + std::map > >::const_iterator iter + = mInfoIndex.find(StringHash(std::make_shared(Misc::StringUtils::lowerCase(topic)))); - std::pair range = getTopicRange (topic); + if (iter == mInfoIndex.end()) + return -1; - for (; range.first!=range.second; ++range.first) - if (Misc::StringUtils::ciEqual((*range.first).get()->get().mId, fullId)) - return std::distance (getRecords().begin(), range.first); + // brute force loop + for (std::vector >::const_iterator it = iter->second.begin(); + it != iter->second.end(); ++it) + { + if (Misc::StringUtils::ciEqual(it->first, id)) + return it->second; + } return -1; } -int CSMWorld::InfoCollection::getAppendIndex (const std::string& id, UniversalId::Type type) const +// Calling insertRecord() using index from getInsertIndex() needs to take into account of +// prev/next records; an example is deleting a record then undo +int CSMWorld::InfoCollection::getInsertIndex (const std::string& id, + UniversalId::Type type, RecordBase *record) const { - std::string::size_type separator = id.find_last_of ('#'); + if (record == nullptr) + { + std::string::size_type separator = id.find_last_of('#'); - if (separator==std::string::npos) - throw std::runtime_error ("invalid info ID: " + id); + if (separator == std::string::npos) + throw std::runtime_error("invalid info ID: " + id); - std::pair range = getTopicRange (id.substr (0, separator)); + std::pair range = getTopicRange(id.substr(0, separator)); - if (range.first==range.second) - return Collection >::getAppendIndex (id, type); + if (range.first == range.second) + return Collection >::getAppendIndex(id, type); + + return std::distance(getRecords().begin(), range.second); + } - return std::distance (getRecords().begin(), range.second); + int index = -1; + + const Info& info = static_cast*>(record)->get(); + std::string topic = info.mTopicId; + + // if the record has a prev, find its index value + if (!info.mPrev.empty()) + { + index = getInfoIndex(info.mPrev, topic); + + if (index != -1) + ++index; // if prev exists, set current index to one above prev + } + + // if prev doesn't exist or not found and the record has a next, find its index value + if (index == -1 && !info.mNext.empty()) + { + // if next exists, use its index as the current index + index = getInfoIndex(info.mNext, topic); + } + + // if next doesn't exist or not found (i.e. neither exist yet) then start a new one + if (index == -1) + { + Range range = getTopicRange(topic); // getTopicRange converts topic to lower case first + + index = std::distance(getRecords().begin(), range.second); + } + + return index; } bool CSMWorld::InfoCollection::reorderRows (int baseIndex, const std::vector& newOrder) @@ -99,7 +182,23 @@ bool CSMWorld::InfoCollection::reorderRows (int baseIndex, const std::vector >::reorderRowsImp(baseIndex, newOrder)) + return false; + + // adjust index + int size = static_cast(newOrder.size()); + for (std::map > >::iterator iter + = mInfoIndex.begin(); iter != mInfoIndex.end(); ++iter) + { + for (std::vector >::iterator it = iter->second.begin(); + it != iter->second.end(); ++it) + { + if (it->second >= baseIndex && it->second < baseIndex+size) + it->second = newOrder.at(it->second-baseIndex)+baseIndex; + } + } + + return true; } void CSMWorld::InfoCollection::load (ESM::ESMReader& reader, bool base, const ESM::Dialogue& dialogue) @@ -142,74 +241,54 @@ void CSMWorld::InfoCollection::load (ESM::ESMReader& reader, bool base, const ES CSMWorld::InfoCollection::Range CSMWorld::InfoCollection::getTopicRange (const std::string& topic) const { - std::string topic2 = Misc::StringUtils::lowerCase (topic); - - std::map::const_iterator iter = getIdMap().lower_bound (topic2); - - // Skip invalid records: The beginning of a topic string could be identical to another topic - // string. - for (; iter!=getIdMap().end(); ++iter) - { - std::string testTopicId = - Misc::StringUtils::lowerCase (getRecord (iter->second).get().mTopicId); - - if (testTopicId==topic2) - break; + std::string lowerTopic = Misc::StringUtils::lowerCase (topic); - std::size_t size = topic2.size(); + // find the topic + std::map > >::const_iterator iter + = mInfoIndex.find(StringHash(std::make_shared(lowerTopic))); - if (testTopicId.size()second; - - while (begin != getRecords().begin()) + // topic found, find the starting index + int low = INT_MAX; + for (std::vector >::const_iterator it = iter->second.begin(); + it != iter->second.end(); ++it) { - if (!Misc::StringUtils::ciEqual((*begin)->get().mTopicId, topic2)) - { - // we've gone one too far, go back - ++begin; - break; - } - --begin; + low = std::min(low, it->second); } - // Find end - RecordConstIterator end = begin; + RecordConstIterator begin = getRecords().begin() + low; - for (; end!=getRecords().end(); ++end) - if (!Misc::StringUtils::ciEqual((*end)->get().mTopicId, topic2)) - break; + // Find end (one past the range) + RecordConstIterator end = begin + iter->second.size(); + if (end != getRecords().end()) + ++end; return Range (begin, end); } void CSMWorld::InfoCollection::removeDialogueInfos(const std::string& dialogueId) { - std::string id = Misc::StringUtils::lowerCase(dialogueId); std::vector erasedRecords; - std::map::const_iterator current = getIdMap().lower_bound(id); - std::map::const_iterator end = getIdMap().end(); - for (; current != end; ++current) + Range range = getTopicRange(dialogueId); // getTopicRange converts dialogueId to lower case first + + for (; range.first != range.second; ++range.first) { - const Record& record = getRecord(current->second); + const Record& record = **range.first; if (Misc::StringUtils::ciEqual(dialogueId, record.get().mTopicId)) { if (record.mState == RecordBase::State_ModifiedOnly) { - erasedRecords.push_back(current->second); + erasedRecords.push_back(range.first - getRecords().begin()); } else { std::unique_ptr > record2(new Record(record)); record2->mState = RecordBase::State_Deleted; - setRecord(current->second, std::move(record2)); + setRecord(range.first - getRecords().begin(), std::move(record2)); } } else @@ -224,3 +303,105 @@ void CSMWorld::InfoCollection::removeDialogueInfos(const std::string& dialogueId erasedRecords.pop_back(); } } + +// FIXME: removing a record should adjust prev/next and mark those records as modified +// accordingly (also consider undo) +void CSMWorld::InfoCollection::removeRows (int index, int count) +{ + Collection >::removeRows(index, count); // erase records only + + for (std::map > >::iterator iter + = mInfoIndex.begin(); iter != mInfoIndex.end();) + { + for (std::vector >::iterator it = iter->second.begin(); + it != iter->second.end();) + { + if (it->second >= index) + { + if (it->second >= index+count) + { + it->second -= count; + ++it; + } + else + iter->second.erase(it); + } + else + ++it; + } + + // check for an empty vector + if (iter->second.empty()) + mInfoIndex.erase(iter++); + else + ++iter; + } +} + +void CSMWorld::InfoCollection::appendBlankRecord (const std::string& id, UniversalId::Type type) +{ + std::unique_ptr > record2(new Record); + + record2->mState = Record::State_ModifiedOnly; + record2->mModified.blank(); + + record2->get().mId = id; + + insertRecord(std::move(record2), getInsertIndex(id, type, nullptr), type); // call InfoCollection::insertRecord() +} + +int CSMWorld::InfoCollection::searchId (const std::string& id) const +{ + std::string::size_type separator = id.find_last_of('#'); + + if (separator == std::string::npos) + throw std::runtime_error("invalid info ID: " + id); + + return getInfoIndex(id.substr(separator+1), id.substr(0, separator)); +} + +void CSMWorld::InfoCollection::appendRecord (std::unique_ptr record, UniversalId::Type type) +{ + int index = getInsertIndex(static_cast*>(record.get())->get().mId, type, record.get()); + + insertRecord(std::move(record), index, type); +} + +void CSMWorld::InfoCollection::insertRecord (std::unique_ptr record, int index, + UniversalId::Type type) +{ + int size = static_cast(getRecords().size()); + + std::string id = static_cast*>(record.get())->get().mId; + std::string::size_type separator = id.find_last_of('#'); + + if (separator == std::string::npos) + throw std::runtime_error("invalid info ID: " + id); + + Collection >::insertRecord(std::move(record), index, type); // add records only + + // adjust index + if (index < size-1) + { + for (std::map > >::iterator iter + = mInfoIndex.begin(); iter != mInfoIndex.end(); ++iter) + { + for (std::vector >::iterator it = iter->second.begin(); + it != iter->second.end(); ++it) + { + if (it->second >= index) + ++(it->second); + } + } + } + + // get iterator for existing topic or a new topic + std::string lowerId = Misc::StringUtils::lowerCase(id); + std::pair > >::iterator, bool> res + = mInfoIndex.insert( + std::make_pair(StringHash(std::make_shared(lowerId.substr(0, separator))), + std::vector >())); // empty vector + + // insert info and index + res.first->second.push_back(std::make_pair(lowerId.substr(separator+1), index)); +} diff --git a/apps/opencs/model/world/infocollection.hpp b/apps/opencs/model/world/infocollection.hpp index f0379b3c14..1417af6659 100644 --- a/apps/opencs/model/world/infocollection.hpp +++ b/apps/opencs/model/world/infocollection.hpp @@ -1,6 +1,8 @@ #ifndef CSM_WOLRD_INFOCOLLECTION_H #define CSM_WOLRD_INFOCOLLECTION_H +#include + #include "collection.hpp" #include "info.hpp" @@ -11,6 +13,48 @@ namespace ESM namespace CSMWorld { + struct StringHash + { + uint64_t mHash; + std::shared_ptr mString; + + StringHash (std::shared_ptr str) : mString(str) + { + mHash = MurmurHash64A(str->c_str(), str->size(), /*seed*/1); + } + }; +} + +namespace std +{ + template<> struct less + { + bool operator() (const CSMWorld::StringHash& lhs, const CSMWorld::StringHash& rhs) const + { + if (lhs.mHash < rhs.mHash) + return true; + + if (lhs.mHash > rhs.mHash) + return false; + + return *lhs.mString < *rhs.mString; + } + }; +} + +namespace CSMWorld +{ + template<> + void Collection >::removeRows (int index, int count); + + template<> + void Collection >::insertRecord (std::unique_ptr record, + int index, UniversalId::Type type); + + template<> + bool Collection >::reorderRowsImp (int baseIndex, + const std::vector& newOrder); + class InfoCollection : public Collection > { public: @@ -20,18 +64,32 @@ namespace CSMWorld private: + // The general strategy is to keep the records in Collection kept in order (within + // a topic group) while the index lookup maps are not ordered. It is assumed that + // each topic has a small number of infos, which allows the use of vectors for + // iterating through them without too much penalty. + // + // NOTE: hashed topic string as well as id string are stored in lower case. + std::map > > mInfoIndex; + void load (const Info& record, bool base); int getInfoIndex (const std::string& id, const std::string& topic) const; ///< Return index for record \a id or -1 (if not present; deleted records are considered) /// /// \param id info ID without topic prefix + // + /// \attention id and topic are assumed to be in lower case public: - int getAppendIndex (const std::string& id, - UniversalId::Type type = UniversalId::Type_None) const override; + int getInsertIndex (const std::string& id, + UniversalId::Type type = UniversalId::Type_None, + RecordBase *record = nullptr) const override; ///< \param type Will be ignored, unless the collection supports multiple record types + /// + /// Works like getAppendIndex unless an overloaded method uses the record pointer + /// to get additional info about the record that results in an alternative index. bool reorderRows (int baseIndex, const std::vector& newOrder) override; ///< Reorder the rows [baseIndex, baseIndex+newOrder.size()) according to the indices @@ -46,6 +104,20 @@ namespace CSMWorld /// the given topic. void removeDialogueInfos(const std::string& dialogueId); + + void removeRows (int index, int count); + + void appendBlankRecord (const std::string& id, + UniversalId::Type type = UniversalId::Type_None); + + int searchId (const std::string& id) const; + + void appendRecord (std::unique_ptr record, + UniversalId::Type type = UniversalId::Type_None); + + void insertRecord (std::unique_ptr record, + int index, + UniversalId::Type type = UniversalId::Type_None); }; } diff --git a/extern/murmurhash/CMakeLists.txt b/extern/murmurhash/CMakeLists.txt new file mode 100644 index 0000000000..cd8199ff06 --- /dev/null +++ b/extern/murmurhash/CMakeLists.txt @@ -0,0 +1,17 @@ +cmake_minimum_required(VERSION 2.8) + +# This is NOT intended as a stand-alone build system! Instead, you should include this from the main CMakeLists of your project. + +set(MURMURHASH_LIBRARY "murmurhash") + +# Sources +set(SOURCE_FILES + MurmurHash2.cpp +) + +add_library(${MURMURHASH_LIBRARY} STATIC ${SOURCE_FILES}) + +set(MURMURHASH_LIBRARIES ${MURMURHASH_LIBRARY}) + +link_directories(${CMAKE_CURRENT_BINARY_DIR}) +set(MURMURHASH_LIBRARIES ${MURMURHASH_LIBRARIES} PARENT_SCOPE) diff --git a/extern/murmurhash/MurmurHash2.cpp b/extern/murmurhash/MurmurHash2.cpp new file mode 100644 index 0000000000..d1b6f476e8 --- /dev/null +++ b/extern/murmurhash/MurmurHash2.cpp @@ -0,0 +1,522 @@ +//----------------------------------------------------------------------------- +// MurmurHash2 was written by Austin Appleby, and is placed in the public +// domain. The author hereby disclaims copyright to this source code. + +// Note - This code makes a few assumptions about how your machine behaves - + +// 1. We can read a 4-byte value from any address without crashing +// 2. sizeof(int) == 4 + +// And it has a few limitations - + +// 1. It will not work incrementally. +// 2. It will not produce the same results on little-endian and big-endian +// machines. + +#include "MurmurHash2.h" + +//----------------------------------------------------------------------------- +// Platform-specific functions and macros + +// Microsoft Visual Studio + +#if defined(_MSC_VER) + +#define BIG_CONSTANT(x) (x) + +// Other compilers + +#else // defined(_MSC_VER) + +#define BIG_CONSTANT(x) (x##LLU) + +#endif // !defined(_MSC_VER) + +//----------------------------------------------------------------------------- + +uint32_t MurmurHash2 ( const void * key, int len, uint32_t seed ) +{ + // 'm' and 'r' are mixing constants generated offline. + // They're not really 'magic', they just happen to work well. + + const uint32_t m = 0x5bd1e995; + const int r = 24; + + // Initialize the hash to a 'random' value + + uint32_t h = seed ^ len; + + // Mix 4 bytes at a time into the hash + + const unsigned char * data = (const unsigned char *)key; + + while(len >= 4) + { + uint32_t k = *(uint32_t*)data; + + k *= m; + k ^= k >> r; + k *= m; + + h *= m; + h ^= k; + + data += 4; + len -= 4; + } + + // Handle the last few bytes of the input array + + switch(len) + { + case 3: h ^= data[2] << 16; + case 2: h ^= data[1] << 8; + case 1: h ^= data[0]; + h *= m; + }; + + // Do a few final mixes of the hash to ensure the last few + // bytes are well-incorporated. + + h ^= h >> 13; + h *= m; + h ^= h >> 15; + + return h; +} + +//----------------------------------------------------------------------------- +// MurmurHash2, 64-bit versions, by Austin Appleby + +// The same caveats as 32-bit MurmurHash2 apply here - beware of alignment +// and endian-ness issues if used across multiple platforms. + +// 64-bit hash for 64-bit platforms + +uint64_t MurmurHash64A ( const void * key, int len, uint64_t seed ) +{ + const uint64_t m = BIG_CONSTANT(0xc6a4a7935bd1e995); + const int r = 47; + + uint64_t h = seed ^ (len * m); + + const uint64_t * data = (const uint64_t *)key; + const uint64_t * end = data + (len/8); + + while(data != end) + { + uint64_t k = *data++; + + k *= m; + k ^= k >> r; + k *= m; + + h ^= k; + h *= m; + } + + const unsigned char * data2 = (const unsigned char*)data; + + switch(len & 7) + { + case 7: h ^= uint64_t(data2[6]) << 48; + case 6: h ^= uint64_t(data2[5]) << 40; + case 5: h ^= uint64_t(data2[4]) << 32; + case 4: h ^= uint64_t(data2[3]) << 24; + case 3: h ^= uint64_t(data2[2]) << 16; + case 2: h ^= uint64_t(data2[1]) << 8; + case 1: h ^= uint64_t(data2[0]); + h *= m; + }; + + h ^= h >> r; + h *= m; + h ^= h >> r; + + return h; +} + + +// 64-bit hash for 32-bit platforms + +uint64_t MurmurHash64B ( const void * key, int len, uint64_t seed ) +{ + const uint32_t m = 0x5bd1e995; + const int r = 24; + + uint32_t h1 = uint32_t(seed) ^ len; + uint32_t h2 = uint32_t(seed >> 32); + + const uint32_t * data = (const uint32_t *)key; + + while(len >= 8) + { + uint32_t k1 = *data++; + k1 *= m; k1 ^= k1 >> r; k1 *= m; + h1 *= m; h1 ^= k1; + len -= 4; + + uint32_t k2 = *data++; + k2 *= m; k2 ^= k2 >> r; k2 *= m; + h2 *= m; h2 ^= k2; + len -= 4; + } + + if(len >= 4) + { + uint32_t k1 = *data++; + k1 *= m; k1 ^= k1 >> r; k1 *= m; + h1 *= m; h1 ^= k1; + len -= 4; + } + + switch(len) + { + case 3: h2 ^= ((unsigned char*)data)[2] << 16; + case 2: h2 ^= ((unsigned char*)data)[1] << 8; + case 1: h2 ^= ((unsigned char*)data)[0]; + h2 *= m; + }; + + h1 ^= h2 >> 18; h1 *= m; + h2 ^= h1 >> 22; h2 *= m; + h1 ^= h2 >> 17; h1 *= m; + h2 ^= h1 >> 19; h2 *= m; + + uint64_t h = h1; + + h = (h << 32) | h2; + + return h; +} + +//----------------------------------------------------------------------------- +// MurmurHash2A, by Austin Appleby + +// This is a variant of MurmurHash2 modified to use the Merkle-Damgard +// construction. Bulk speed should be identical to Murmur2, small-key speed +// will be 10%-20% slower due to the added overhead at the end of the hash. + +// This variant fixes a minor issue where null keys were more likely to +// collide with each other than expected, and also makes the function +// more amenable to incremental implementations. + +#define mmix(h,k) { k *= m; k ^= k >> r; k *= m; h *= m; h ^= k; } + +uint32_t MurmurHash2A ( const void * key, int len, uint32_t seed ) +{ + const uint32_t m = 0x5bd1e995; + const int r = 24; + uint32_t l = len; + + const unsigned char * data = (const unsigned char *)key; + + uint32_t h = seed; + + while(len >= 4) + { + uint32_t k = *(uint32_t*)data; + + mmix(h,k); + + data += 4; + len -= 4; + } + + uint32_t t = 0; + + switch(len) + { + case 3: t ^= data[2] << 16; + case 2: t ^= data[1] << 8; + case 1: t ^= data[0]; + }; + + mmix(h,t); + mmix(h,l); + + h ^= h >> 13; + h *= m; + h ^= h >> 15; + + return h; +} + +//----------------------------------------------------------------------------- +// CMurmurHash2A, by Austin Appleby + +// This is a sample implementation of MurmurHash2A designed to work +// incrementally. + +// Usage - + +// CMurmurHash2A hasher +// hasher.Begin(seed); +// hasher.Add(data1,size1); +// hasher.Add(data2,size2); +// ... +// hasher.Add(dataN,sizeN); +// uint32_t hash = hasher.End() + +class CMurmurHash2A +{ +public: + + void Begin ( uint32_t seed = 0 ) + { + m_hash = seed; + m_tail = 0; + m_count = 0; + m_size = 0; + } + + void Add ( const unsigned char * data, int len ) + { + m_size += len; + + MixTail(data,len); + + while(len >= 4) + { + uint32_t k = *(uint32_t*)data; + + mmix(m_hash,k); + + data += 4; + len -= 4; + } + + MixTail(data,len); + } + + uint32_t End ( void ) + { + mmix(m_hash,m_tail); + mmix(m_hash,m_size); + + m_hash ^= m_hash >> 13; + m_hash *= m; + m_hash ^= m_hash >> 15; + + return m_hash; + } + +private: + + static const uint32_t m = 0x5bd1e995; + static const int r = 24; + + void MixTail ( const unsigned char * & data, int & len ) + { + while( len && ((len<4) || m_count) ) + { + m_tail |= (*data++) << (m_count * 8); + + m_count++; + len--; + + if(m_count == 4) + { + mmix(m_hash,m_tail); + m_tail = 0; + m_count = 0; + } + } + } + + uint32_t m_hash; + uint32_t m_tail; + uint32_t m_count; + uint32_t m_size; +}; + +//----------------------------------------------------------------------------- +// MurmurHashNeutral2, by Austin Appleby + +// Same as MurmurHash2, but endian- and alignment-neutral. +// Half the speed though, alas. + +uint32_t MurmurHashNeutral2 ( const void * key, int len, uint32_t seed ) +{ + const uint32_t m = 0x5bd1e995; + const int r = 24; + + uint32_t h = seed ^ len; + + const unsigned char * data = (const unsigned char *)key; + + while(len >= 4) + { + uint32_t k; + + k = data[0]; + k |= data[1] << 8; + k |= data[2] << 16; + k |= data[3] << 24; + + k *= m; + k ^= k >> r; + k *= m; + + h *= m; + h ^= k; + + data += 4; + len -= 4; + } + + switch(len) + { + case 3: h ^= data[2] << 16; + case 2: h ^= data[1] << 8; + case 1: h ^= data[0]; + h *= m; + }; + + h ^= h >> 13; + h *= m; + h ^= h >> 15; + + return h; +} + +//----------------------------------------------------------------------------- +// MurmurHashAligned2, by Austin Appleby + +// Same algorithm as MurmurHash2, but only does aligned reads - should be safer +// on certain platforms. + +// Performance will be lower than MurmurHash2 + +#define MIX(h,k,m) { k *= m; k ^= k >> r; k *= m; h *= m; h ^= k; } + + +uint32_t MurmurHashAligned2 ( const void * key, int len, uint32_t seed ) +{ + const uint32_t m = 0x5bd1e995; + const int r = 24; + + const unsigned char * data = (const unsigned char *)key; + + uint32_t h = seed ^ len; + + int align = (uint64_t)data & 3; + + if(align && (len >= 4)) + { + // Pre-load the temp registers + + uint32_t t = 0, d = 0; + + switch(align) + { + case 1: t |= data[2] << 16; + case 2: t |= data[1] << 8; + case 3: t |= data[0]; + } + + t <<= (8 * align); + + data += 4-align; + len -= 4-align; + + int sl = 8 * (4-align); + int sr = 8 * align; + + // Mix + + while(len >= 4) + { + d = *(uint32_t *)data; + t = (t >> sr) | (d << sl); + + uint32_t k = t; + + MIX(h,k,m); + + t = d; + + data += 4; + len -= 4; + } + + // Handle leftover data in temp registers + + d = 0; + + if(len >= align) + { + switch(align) + { + case 3: d |= data[2] << 16; + case 2: d |= data[1] << 8; + case 1: d |= data[0]; + } + + uint32_t k = (t >> sr) | (d << sl); + MIX(h,k,m); + + data += align; + len -= align; + + //---------- + // Handle tail bytes + + switch(len) + { + case 3: h ^= data[2] << 16; + case 2: h ^= data[1] << 8; + case 1: h ^= data[0]; + h *= m; + }; + } + else + { + switch(len) + { + case 3: d |= data[2] << 16; + case 2: d |= data[1] << 8; + case 1: d |= data[0]; + case 0: h ^= (t >> sr) | (d << sl); + h *= m; + } + } + + h ^= h >> 13; + h *= m; + h ^= h >> 15; + + return h; + } + else + { + while(len >= 4) + { + uint32_t k = *(uint32_t *)data; + + MIX(h,k,m); + + data += 4; + len -= 4; + } + + //---------- + // Handle tail bytes + + switch(len) + { + case 3: h ^= data[2] << 16; + case 2: h ^= data[1] << 8; + case 1: h ^= data[0]; + h *= m; + }; + + h ^= h >> 13; + h *= m; + h ^= h >> 15; + + return h; + } +} + +//----------------------------------------------------------------------------- diff --git a/extern/murmurhash/MurmurHash2.h b/extern/murmurhash/MurmurHash2.h new file mode 100644 index 0000000000..e6d0c36924 --- /dev/null +++ b/extern/murmurhash/MurmurHash2.h @@ -0,0 +1,38 @@ +//----------------------------------------------------------------------------- +// MurmurHash2 was written by Austin Appleby, and is placed in the public +// domain. The author hereby disclaims copyright to this source code. + +#ifndef _MURMURHASH2_H_ +#define _MURMURHASH2_H_ + +//----------------------------------------------------------------------------- +// Platform-specific functions and macros + +// Microsoft Visual Studio + +#if defined(_MSC_VER) && (_MSC_VER < 1600) + +typedef unsigned char uint8_t; +typedef unsigned int uint32_t; +typedef unsigned __int64 uint64_t; + +// Other compilers + +#else // defined(_MSC_VER) + +#include + +#endif // !defined(_MSC_VER) + +//----------------------------------------------------------------------------- + +uint32_t MurmurHash2 ( const void * key, int len, uint32_t seed ); +uint64_t MurmurHash64A ( const void * key, int len, uint64_t seed ); +uint64_t MurmurHash64B ( const void * key, int len, uint64_t seed ); +uint32_t MurmurHash2A ( const void * key, int len, uint32_t seed ); +uint32_t MurmurHashNeutral2 ( const void * key, int len, uint32_t seed ); +uint32_t MurmurHashAligned2 ( const void * key, int len, uint32_t seed ); + +//----------------------------------------------------------------------------- + +#endif // _MURMURHASH2_H_ From fc2f68a4650f4205ebef80c6d678b66508b12612 Mon Sep 17 00:00:00 2001 From: cc9cii Date: Fri, 23 Jul 2021 17:34:27 +1000 Subject: [PATCH 04/11] Change the loader's top progress bar to show total number of records processed rather than the number of files. (copied the changes from commit SHA-1: ff072441fde05d189cb06cb44cc9c360690c2f09) --- apps/opencs/model/doc/loader.cpp | 8 ++--- apps/opencs/model/world/data.cpp | 16 +++++++++ apps/opencs/model/world/data.hpp | 2 ++ apps/opencs/view/doc/loader.cpp | 56 +++++++++++++++++--------------- apps/opencs/view/doc/loader.hpp | 8 +++-- 5 files changed, 56 insertions(+), 34 deletions(-) diff --git a/apps/opencs/model/doc/loader.cpp b/apps/opencs/model/doc/loader.cpp index 1c5a7348c8..9d2f89b8e9 100644 --- a/apps/opencs/model/doc/loader.cpp +++ b/apps/opencs/model/doc/loader.cpp @@ -85,19 +85,19 @@ void CSMDoc::Loader::load() return; } - if (iter->second.mFilesecond.mFilegetContentFiles()[iter->second.mFile]; - int steps = document->getData().startLoading (path, iter->second.mFile!=editedIndex, false); + int steps = document->getData().startLoading (path, iter->second.mFile!=editedIndex, /*project*/false); iter->second.mRecordsLeft = true; iter->second.mRecordsLoaded = 0; emit nextStage (document, path.filename().string(), steps); } - else if (iter->second.mFile==size) + else if (iter->second.mFile==size) // start loading the last (project) file { - int steps = document->getData().startLoading (document->getProjectPath(), false, true); + int steps = document->getData().startLoading (document->getProjectPath(), /*base*/false, true); iter->second.mRecordsLeft = true; iter->second.mRecordsLoaded = 0; diff --git a/apps/opencs/model/world/data.cpp b/apps/opencs/model/world/data.cpp index 7d6f0f37cb..5db052bcbf 100644 --- a/apps/opencs/model/world/data.cpp +++ b/apps/opencs/model/world/data.cpp @@ -958,6 +958,22 @@ void CSMWorld::Data::merge() mGlobals.merge(); } +int CSMWorld::Data::getTotalRecords (const std::vector& files) +{ + int records = 0; + + std::unique_ptr reader = std::unique_ptr(new ESM::ESMReader); + + for (unsigned int i = 0; i < files.size(); ++i) + { + reader->open(files[i].string()); + records += reader->getRecordCount(); + reader->close(); + } + + return records; +} + int CSMWorld::Data::startLoading (const boost::filesystem::path& path, bool base, bool project) { // Don't delete the Reader yet. Some record types store a reference to the Reader to handle on-demand loading diff --git a/apps/opencs/model/world/data.hpp b/apps/opencs/model/world/data.hpp index f7a51b289f..d38d40c456 100644 --- a/apps/opencs/model/world/data.hpp +++ b/apps/opencs/model/world/data.hpp @@ -292,6 +292,8 @@ namespace CSMWorld void merge(); ///< Merge modified into base. + int getTotalRecords (const std::vector& files); // for better loading bar + int startLoading (const boost::filesystem::path& path, bool base, bool project); ///< Begin merging content of a file into base or modified. /// diff --git a/apps/opencs/view/doc/loader.cpp b/apps/opencs/view/doc/loader.cpp index 1cdfc01733..7929b13f6c 100644 --- a/apps/opencs/view/doc/loader.cpp +++ b/apps/opencs/view/doc/loader.cpp @@ -17,7 +17,7 @@ void CSVDoc::LoadingDocument::closeEvent (QCloseEvent *event) } CSVDoc::LoadingDocument::LoadingDocument (CSMDoc::Document *document) -: mDocument (document), mAborted (false), mMessages (nullptr), mTotalRecords (0) +: mDocument (document), mAborted (false), mMessages (nullptr), mRecordsLabel (0), mTotalRecordsLabel (0) { setWindowTitle (QString::fromUtf8((std::string("Opening ") + document->getSavePath().filename().string()).c_str())); @@ -25,26 +25,25 @@ CSVDoc::LoadingDocument::LoadingDocument (CSMDoc::Document *document) mLayout = new QVBoxLayout (this); - // file progress - mFile = new QLabel (this); + // total progress + mTotalRecordsLabel = new QLabel (this); - mLayout->addWidget (mFile); + mLayout->addWidget (mTotalRecordsLabel); - mFileProgress = new QProgressBar (this); + mTotalProgress = new QProgressBar (this); - mLayout->addWidget (mFileProgress); + mLayout->addWidget (mTotalProgress); - int size = static_cast (document->getContentFiles().size())+1; - if (document->isNew()) - --size; + mTotalProgress->setMinimum (0); + mTotalProgress->setMaximum (document->getData().getTotalRecords(document->getContentFiles())); + mTotalProgress->setTextVisible (true); + mTotalProgress->setValue (0); + mTotalRecords = 0; - mFileProgress->setMinimum (0); - mFileProgress->setMaximum (size); - mFileProgress->setTextVisible (true); - mFileProgress->setValue (0); + mFilesLoaded = 0; // record progress - mLayout->addWidget (mRecords = new QLabel ("Records", this)); + mLayout->addWidget (mRecordsLabel = new QLabel ("Records", this)); mRecordProgress = new QProgressBar (this); @@ -74,29 +73,32 @@ CSVDoc::LoadingDocument::LoadingDocument (CSMDoc::Document *document) connect (mButtons, SIGNAL (rejected()), this, SLOT (cancel())); } -void CSVDoc::LoadingDocument::nextStage (const std::string& name, int totalRecords) +void CSVDoc::LoadingDocument::nextStage (const std::string& name, int fileRecords) { - mFile->setText (QString::fromUtf8 (("Loading: " + name).c_str())); + ++mFilesLoaded; + size_t numFiles = mDocument->getContentFiles().size(); - mFileProgress->setValue (mFileProgress->value()+1); + mTotalRecordsLabel->setText (QString::fromUtf8 (("Loading: "+name + +" ("+std::to_string(mFilesLoaded)+" of "+std::to_string((numFiles))+")").c_str())); + + mTotalRecords = mTotalProgress->value(); mRecordProgress->setValue (0); - mRecordProgress->setMaximum (totalRecords>0 ? totalRecords : 1); + mRecordProgress->setMaximum (fileRecords>0 ? fileRecords : 1); - mTotalRecords = totalRecords; + mRecords = fileRecords; } void CSVDoc::LoadingDocument::nextRecord (int records) { - if (records<=mTotalRecords) + if (records <= mRecords) { - mRecordProgress->setValue (records); - - std::ostringstream stream; + mTotalProgress->setValue (mTotalRecords+records); - stream << "Records: " << records << " of " << mTotalRecords; + mRecordProgress->setValue(records); - mRecords->setText (QString::fromUtf8 (stream.str().c_str())); + mRecordsLabel->setText(QString::fromStdString( + "Records: "+std::to_string(records)+" of "+std::to_string(mRecords))); } } @@ -176,12 +178,12 @@ void CSVDoc::Loader::loadingStopped (CSMDoc::Document *document, bool completed, } void CSVDoc::Loader::nextStage (CSMDoc::Document *document, const std::string& name, - int totalRecords) + int fileRecords) { std::map::iterator iter = mDocuments.find (document); if (iter!=mDocuments.end()) - iter->second->nextStage (name, totalRecords); + iter->second->nextStage (name, fileRecords); } void CSVDoc::Loader::nextRecord (CSMDoc::Document *document, int records) diff --git a/apps/opencs/view/doc/loader.hpp b/apps/opencs/view/doc/loader.hpp index 63b03ec8d2..1d21d1e02e 100644 --- a/apps/opencs/view/doc/loader.hpp +++ b/apps/opencs/view/doc/loader.hpp @@ -25,16 +25,18 @@ namespace CSVDoc Q_OBJECT CSMDoc::Document *mDocument; - QLabel *mFile; - QLabel *mRecords; - QProgressBar *mFileProgress; + QLabel *mTotalRecordsLabel; + QLabel *mRecordsLabel; + QProgressBar *mTotalProgress; QProgressBar *mRecordProgress; bool mAborted; QDialogButtonBox *mButtons; QLabel *mError; QListWidget *mMessages; QVBoxLayout *mLayout; + int mRecords; int mTotalRecords; + int mFilesLoaded; private: From 10c6304a1fbf374861dc3fef865ab08577945d4b Mon Sep 17 00:00:00 2001 From: cc9cii Date: Fri, 23 Jul 2021 18:12:09 +1000 Subject: [PATCH 05/11] Fix typo from commit cfdbd0d471c6bac72f904f86a1bec486ca2a3822. --- apps/opencs/model/world/refcollection.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/opencs/model/world/refcollection.cpp b/apps/opencs/model/world/refcollection.cpp index a26bf1d2f4..f4dc0bcb97 100644 --- a/apps/opencs/model/world/refcollection.cpp +++ b/apps/opencs/model/world/refcollection.cpp @@ -35,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 && !isMoved) + 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 From 5c504e4d22d4d4f5eb0982e1c66a64257d22b1da Mon Sep 17 00:00:00 2001 From: cc9cii Date: Fri, 23 Jul 2021 19:07:56 +1000 Subject: [PATCH 06/11] Convert the CellRef record index lookup maps to use integer keys rather than strings. - Morrowind load over 300,000 references, so even small inefficiencies add up to longer loading times. - std::map is used, but should try others, std::unordered_map or even std::vector (copied the changes from commit SHA-1: 86945d19128468ad987b5bf4332602d613d25d9f) --- apps/opencs/model/world/data.hpp | 2 +- apps/opencs/model/world/ref.cpp | 2 +- apps/opencs/model/world/ref.hpp | 1 + apps/opencs/model/world/refcollection.cpp | 180 +++++++++++++++++++++- apps/opencs/model/world/refcollection.hpp | 35 ++++- 5 files changed, 210 insertions(+), 10 deletions(-) diff --git a/apps/opencs/model/world/data.hpp b/apps/opencs/model/world/data.hpp index d38d40c456..b1c20b8629 100644 --- a/apps/opencs/model/world/data.hpp +++ b/apps/opencs/model/world/data.hpp @@ -118,7 +118,7 @@ namespace CSMWorld const ESM::Dialogue *mDialogue; // last loaded dialogue bool mBase; bool mProject; - std::map > mRefLoadCache; + std::map > mRefLoadCache; int mReaderIndex; bool mFsStrict; diff --git a/apps/opencs/model/world/ref.cpp b/apps/opencs/model/world/ref.cpp index b336235909..0b07b484ca 100644 --- a/apps/opencs/model/world/ref.cpp +++ b/apps/opencs/model/world/ref.cpp @@ -2,7 +2,7 @@ #include "cellcoordinates.hpp" -CSMWorld::CellRef::CellRef() : mNew (true) +CSMWorld::CellRef::CellRef() : mNew (true), mIdNum(0) { mRefNum.mIndex = 0; mRefNum.mContentFile = 0; diff --git a/apps/opencs/model/world/ref.hpp b/apps/opencs/model/world/ref.hpp index 5d10a3a1b3..23b4ad1b5c 100644 --- a/apps/opencs/model/world/ref.hpp +++ b/apps/opencs/model/world/ref.hpp @@ -14,6 +14,7 @@ namespace CSMWorld std::string mCell; std::string mOriginalCell; bool mNew; // new reference, not counted yet, ref num not assigned yet + unsigned int mIdNum; CellRef(); diff --git a/apps/opencs/model/world/refcollection.cpp b/apps/opencs/model/world/refcollection.cpp index f4dc0bcb97..61cdd8205a 100644 --- a/apps/opencs/model/world/refcollection.cpp +++ b/apps/opencs/model/world/refcollection.cpp @@ -7,8 +7,37 @@ #include "universalid.hpp" #include "record.hpp" +namespace CSMWorld +{ + template<> + void Collection >::removeRows (int index, int count) + { + mRecords.erase(mRecords.begin()+index, mRecords.begin()+index+count); + + // index map is updated in RefCollection::removeRows() + } + + template<> + void Collection >::insertRecord (std::unique_ptr record, int index, + UniversalId::Type type) + { + int size = static_cast(mRecords.size()); + if (index < 0 || index > size) + throw std::runtime_error("index out of range"); + + std::unique_ptr > record2(static_cast*>(record.release())); + + if (index == size) + mRecords.push_back(std::move(record2)); + else + mRecords.insert(mRecords.begin()+index, std::move(record2)); + + // index map is updated in RefCollection::insertRecord() + } +} + 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); @@ -63,7 +92,7 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool unsigned int refNum = (ref.mRefNum.mIndex & 0x00ffffff) | (ref.mRefNum.hasContentFile() ? ref.mRefNum.mContentFile : 0xff) << 24; - std::map::iterator iter = cache.find(refNum); + std::map::iterator iter = cache.find(refNum); if (ref.mRefNum.mContentFile != -1 && !base) ref.mRefNum.mContentFile = ref.mRefNum.mIndex >> 24; @@ -74,11 +103,15 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool CSMWorld::UniversalId id (CSMWorld::UniversalId::Type_Cell, mCells.getId (cellIndex)); - messages.add (id, "Attempt to delete a non-existent reference"); + messages.add (id, "Attempt to delete a non-existent reference - RefNum index " + + std::to_string(ref.mRefNum.mIndex) + ", refID " + ref.mRefID + ", content file index " + + std::to_string(ref.mRefNum.mContentFile), + /*hint*/"", + CSMDoc::Message::Severity_Warning); continue; } - int index = getIndex (iter->second); + int index = getIntIndex (iter->second); if (base) { @@ -98,9 +131,10 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool if (iter==cache.end()) { // new reference + ref.mIdNum = mNextId; // FIXME: fragile ref.mId = getNewId(); - cache.emplace(refNum, ref.mId); + cache.emplace(refNum, ref.mIdNum); std::unique_ptr > record(new Record); record->mState = base ? RecordBase::State_BaseOnly : RecordBase::State_ModifiedOnly; @@ -111,9 +145,27 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool else { // old reference -> merge - ref.mId = iter->second; + int index = getIntIndex(iter->second); +#if 0 + // ref.mRefNum.mIndex : the key + // iter->second : previously cached idNum for the key + // index : position of the record for that idNum + // getRecord(index).get() : record in the index position + assert(iter->second != getRecord(index).get().mIdNum); // sanity check - int index = getIndex (ref.mId); + // check if the plugin used the same RefNum index for a different record + if (ref.mRefID != getRecord(index).get().mRefID) + { + CSMWorld::UniversalId id(CSMWorld::UniversalId::Type_Cell, mCells.getId(cellIndex)); + messages.add(id, + "RefNum renamed from RefID \"" + getRecord(index).get().mRefID + "\" to \"" + + ref.mRefID + "\" (RefNum index " + std::to_string(ref.mRefNum.mIndex) + ")", + /*hint*/"", + CSMDoc::Message::Severity_Info); + } +#endif + ref.mId = getRecord(index).get().mId; + ref.mIdNum = extractIdNum(ref.mId); std::unique_ptr > record(new Record(getRecord(index))); record->mState = base ? RecordBase::State_BaseOnly : RecordBase::State_Modified; @@ -128,3 +180,117 @@ std::string CSMWorld::RefCollection::getNewId() { return "ref#" + std::to_string(mNextId++); } + +unsigned int CSMWorld::RefCollection::extractIdNum (const std::string& id) const +{ + std::string::size_type separator = id.find_last_of('#'); + + if (separator == std::string::npos) + throw std::runtime_error("invalid ref ID: " + id); + + return static_cast(std::stoi(id.substr(separator+1))); +} + +int CSMWorld::RefCollection::getIntIndex (unsigned int id) const +{ + int index = searchId(id); + + if (index == -1) + throw std::runtime_error("invalid RefNum: " + std::to_string(id)); + + return index; +} + +int CSMWorld::RefCollection::searchId (unsigned int id) const +{ + std::map::const_iterator iter = mRefIndex.find(id); + + if (iter == mRefIndex.end()) + return -1; + + return iter->second; +} + +void CSMWorld::RefCollection::removeRows (int index, int count) +{ + Collection >::removeRows(index, count); // erase records only + + std::map::iterator iter = mRefIndex.begin(); + while (iter != mRefIndex.end()) + { + if (iter->second>=index) + { + if (iter->second >= index+count) + { + iter->second -= count; + ++iter; + } + else + mRefIndex.erase(iter++); + } + else + ++iter; + } +} + +void CSMWorld::RefCollection::appendBlankRecord (const std::string& id, UniversalId::Type type) +{ + std::unique_ptr > record2(new Record); + + record2->mState = Record::State_ModifiedOnly; + record2->mModified.blank(); + + record2->get().mId = id; + record2->get().mIdNum = extractIdNum(id); + + Collection >::appendRecord(std::move(record2)); +} + +void CSMWorld::RefCollection::cloneRecord (const std::string& origin, + const std::string& destination, + const UniversalId::Type type) +{ + std::unique_ptr > copy(new Record); + + copy->mModified = getRecord(origin).get(); + copy->mState = RecordBase::State_ModifiedOnly; + + copy->get().mId = destination; + copy->get().mIdNum = extractIdNum(destination); + + insertRecord(std::move(copy), getAppendIndex(destination, type)); // call RefCollection::insertRecord() +} + +int CSMWorld::RefCollection::searchId (const std::string& id) const +{ + return searchId(extractIdNum(id)); +} + +void CSMWorld::RefCollection::appendRecord (std::unique_ptr record, UniversalId::Type type) +{ + int index = getAppendIndex(/*id*/"", type); // for CellRef records id is ignored + + mRefIndex.insert(std::make_pair(static_cast*>(record.get())->get().mIdNum, index)); + + Collection >::insertRecord(std::move(record), index, type); // add records only +} + +void CSMWorld::RefCollection::insertRecord (std::unique_ptr record, int index, + UniversalId::Type type) +{ + int size = getAppendIndex(/*id*/"", type); // for CellRef records id is ignored + unsigned int idNum = static_cast*>(record.get())->get().mIdNum; + + Collection >::insertRecord(std::move(record), index, type); // add records only + + if (index < size-1) + { + for (std::map::iterator iter(mRefIndex.begin()); iter != mRefIndex.end(); ++iter) + { + if (iter->second >= index) + ++(iter->second); + } + } + + mRefIndex.insert(std::make_pair(idNum, index)); +} diff --git a/apps/opencs/model/world/refcollection.hpp b/apps/opencs/model/world/refcollection.hpp index 42bb363570..1bdfda3925 100644 --- a/apps/opencs/model/world/refcollection.hpp +++ b/apps/opencs/model/world/refcollection.hpp @@ -14,12 +14,27 @@ namespace CSMWorld struct Cell; class UniversalId; + template<> + void Collection >::removeRows (int index, int count); + + template<> + void Collection >::insertRecord (std::unique_ptr record, int index, + UniversalId::Type type); + /// \brief References in cells class RefCollection : public Collection { Collection& mCells; + std::map mRefIndex; + int mNextId; + unsigned int extractIdNum(const std::string& id) const; + + int getIntIndex (unsigned int id) const; + + int searchId (unsigned int id) const; + public: // MSVC needs the constructor for a class inheriting a template to be defined in header RefCollection (Collection& cells) @@ -27,10 +42,28 @@ namespace CSMWorld {} void load (ESM::ESMReader& reader, int cellIndex, bool base, - std::map& cache, CSMDoc::Messages& messages); + std::map& cache, CSMDoc::Messages& messages); ///< Load a sequence of references. std::string getNewId(); + + virtual void removeRows (int index, int count); + + virtual void appendBlankRecord (const std::string& id, + UniversalId::Type type = UniversalId::Type_None); + + virtual void cloneRecord (const std::string& origin, + const std::string& destination, + const UniversalId::Type type); + + virtual int searchId (const std::string& id) const; + + virtual void appendRecord (std::unique_ptr record, + UniversalId::Type type = UniversalId::Type_None); + + virtual void insertRecord (std::unique_ptr record, + int index, + UniversalId::Type type = UniversalId::Type_None); }; } From 44a333b6db8690d984aa1375d7962ad49f8f4613 Mon Sep 17 00:00:00 2001 From: cc9cii Date: Fri, 23 Jul 2021 21:25:35 +1000 Subject: [PATCH 07/11] Don't attempt to open files yet to be created. --- apps/opencs/model/world/data.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/opencs/model/world/data.cpp b/apps/opencs/model/world/data.cpp index 5db052bcbf..192602290d 100644 --- a/apps/opencs/model/world/data.cpp +++ b/apps/opencs/model/world/data.cpp @@ -966,6 +966,9 @@ int CSMWorld::Data::getTotalRecords (const std::vector& for (unsigned int i = 0; i < files.size(); ++i) { + if (!boost::filesystem::exists(files[i])) + continue; + reader->open(files[i].string()); records += reader->getRecordCount(); reader->close(); From 725d689e8aa95c245ef625d36011afb9a95de509 Mon Sep 17 00:00:00 2001 From: cc9cii Date: Fri, 23 Jul 2021 23:17:16 +1000 Subject: [PATCH 08/11] Call push_back() if inserting to the end of the vector. It seems MSVC may be generating different code compared to insert(). (copied the changes from commit SHA-1: 257126ed69a5f6f964ba771766de061e81f87433) --- apps/opencs/model/world/collection.hpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/apps/opencs/model/world/collection.hpp b/apps/opencs/model/world/collection.hpp index 89430f1c58..9d9c69a5da 100644 --- a/apps/opencs/model/world/collection.hpp +++ b/apps/opencs/model/world/collection.hpp @@ -543,20 +543,25 @@ namespace CSMWorld void Collection::insertRecord (std::unique_ptr record, int index, UniversalId::Type type) { - if (index<0 || index>static_cast (mRecords.size())) + int size = static_cast(mRecords.size()); + if (index < 0 || index > size) throw std::runtime_error ("index out of range"); std::unique_ptr > record2(static_cast*>(record.release())); std::string lowerId = Misc::StringUtils::lowerCase(IdAccessorT().getId(record2->get())); - mRecords.insert (mRecords.begin()+index, std::move(record2)); + if (index == size) + mRecords.push_back (std::move(record2)); + else + mRecords.insert (mRecords.begin()+index, std::move(record2)); - if (index (mRecords.size())-1) + if (index < size-1) { - for (std::map::iterator iter (mIndex.begin()); iter!=mIndex.end(); - ++iter) - if (iter->second>=index) - ++(iter->second); + for (std::map::iterator iter (mIndex.begin()); iter!=mIndex.end(); ++iter) + { + if (iter->second >= index) + ++(iter->second); + } } mIndex.insert (std::make_pair (lowerId, index)); From ee3361a118d51850f99935cd393edcae7186f078 Mon Sep 17 00:00:00 2001 From: cc9cii Date: Fri, 23 Jul 2021 23:18:11 +1000 Subject: [PATCH 09/11] Fix table being sorted twice (at least it appeared that way according to the sample profiler) - Quoting Qt-4.8: "Note: . Setting the property to true with setSortingEnabled() immediately triggers a call to sortByColumn() with the current sort section and order." (copied the changes from commit SHA-1: 77394fce99281cea65f30c67dbcd69a2d3eeba88) --- apps/opencs/view/world/table.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/apps/opencs/view/world/table.cpp b/apps/opencs/view/world/table.cpp index c676a5ecc0..5c54bb9344 100644 --- a/apps/opencs/view/world/table.cpp +++ b/apps/opencs/view/world/table.cpp @@ -269,12 +269,6 @@ CSVWorld::Table::Table (const CSMWorld::UniversalId& id, setSelectionBehavior (QAbstractItemView::SelectRows); setSelectionMode (QAbstractItemView::ExtendedSelection); - setSortingEnabled (sorting); - if (sorting) - { - sortByColumn (mModel->findColumnIndex(CSMWorld::Columns::ColumnId_Id), Qt::AscendingOrder); - } - int columns = mModel->columnCount(); for (int i=0; ifindColumnIndex(CSMWorld::Columns::ColumnId_Id), Qt::AscendingOrder); + } + setSortingEnabled (sorting); + mEditAction = new QAction (tr ("Edit Record"), this); connect (mEditAction, SIGNAL (triggered()), this, SLOT (editRecord())); mEditAction->setIcon(QIcon(":edit-edit")); From fd67ebde2549bf5eb626c8c4ea76536c1f003a56 Mon Sep 17 00:00:00 2001 From: cc9cii Date: Sat, 24 Jul 2021 09:17:48 +1000 Subject: [PATCH 10/11] Changes based on review comments, including: * replace murmurhash with std::unordered_map * plug potential leak from unique_ptr release * replacing some sections with cleaner code --- CMakeLists.txt | 1 - apps/opencs/CMakeLists.txt | 1 - apps/opencs/model/world/infocollection.cpp | 30 +- apps/opencs/model/world/infocollection.hpp | 37 +- apps/opencs/model/world/refcollection.cpp | 35 +- apps/opencs/model/world/refcollection.hpp | 2 +- apps/opencs/model/world/refiddata.hpp | 6 +- extern/murmurhash/CMakeLists.txt | 17 - extern/murmurhash/MurmurHash2.cpp | 522 --------------------- extern/murmurhash/MurmurHash2.h | 38 -- 10 files changed, 39 insertions(+), 650 deletions(-) delete mode 100644 extern/murmurhash/CMakeLists.txt delete mode 100644 extern/murmurhash/MurmurHash2.cpp delete mode 100644 extern/murmurhash/MurmurHash2.h diff --git a/CMakeLists.txt b/CMakeLists.txt index f40d77c1dc..b69cb5b71f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -536,7 +536,6 @@ endif (CMAKE_CXX_COMPILER_ID STREQUAL GNU OR CMAKE_CXX_COMPILER_ID STREQUAL Clan add_subdirectory (extern/osg-ffmpeg-videoplayer) add_subdirectory (extern/oics) add_subdirectory (extern/Base64) -add_subdirectory (extern/murmurhash) if (BUILD_OPENCS) add_subdirectory (extern/osgQt) endif() diff --git a/apps/opencs/CMakeLists.txt b/apps/opencs/CMakeLists.txt index 9a70950879..88c4233c9c 100644 --- a/apps/opencs/CMakeLists.txt +++ b/apps/opencs/CMakeLists.txt @@ -226,7 +226,6 @@ target_link_libraries(openmw-cs ${OSGTEXT_LIBRARIES} ${OSG_LIBRARIES} ${EXTERN_OSGQT_LIBRARY} - ${MURMURHASH_LIBRARIES} ${Boost_SYSTEM_LIBRARY} ${Boost_FILESYSTEM_LIBRARY} ${Boost_PROGRAM_OPTIONS_LIBRARY} diff --git a/apps/opencs/model/world/infocollection.cpp b/apps/opencs/model/world/infocollection.cpp index d229ddacc2..b4223d03ab 100644 --- a/apps/opencs/model/world/infocollection.cpp +++ b/apps/opencs/model/world/infocollection.cpp @@ -99,8 +99,8 @@ void CSMWorld::InfoCollection::load (const Info& record, bool base) int CSMWorld::InfoCollection::getInfoIndex (const std::string& id, const std::string& topic) const { // find the topic first - std::map > >::const_iterator iter - = mInfoIndex.find(StringHash(std::make_shared(Misc::StringUtils::lowerCase(topic)))); + std::unordered_map > >::const_iterator iter + = mInfoIndex.find(Misc::StringUtils::lowerCase(topic)); if (iter == mInfoIndex.end()) return -1; @@ -187,16 +187,10 @@ bool CSMWorld::InfoCollection::reorderRows (int baseIndex, const std::vector(newOrder.size()); - for (std::map > >::iterator iter - = mInfoIndex.begin(); iter != mInfoIndex.end(); ++iter) - { - for (std::vector >::iterator it = iter->second.begin(); - it != iter->second.end(); ++it) - { - if (it->second >= baseIndex && it->second < baseIndex+size) - it->second = newOrder.at(it->second-baseIndex)+baseIndex; - } - } + for (auto& [hash, infos] : mInfoIndex) + for (auto& [a, b] : infos) + if (b >= baseIndex && b < baseIndex + size) + b = newOrder.at(b - baseIndex) + baseIndex; return true; } @@ -244,8 +238,8 @@ CSMWorld::InfoCollection::Range CSMWorld::InfoCollection::getTopicRange (const s std::string lowerTopic = Misc::StringUtils::lowerCase (topic); // find the topic - std::map > >::const_iterator iter - = mInfoIndex.find(StringHash(std::make_shared(lowerTopic))); + std::unordered_map > >::const_iterator iter + = mInfoIndex.find(lowerTopic); if (iter == mInfoIndex.end()) return Range (getRecords().end(), getRecords().end()); @@ -310,7 +304,7 @@ void CSMWorld::InfoCollection::removeRows (int index, int count) { Collection >::removeRows(index, count); // erase records only - for (std::map > >::iterator iter + for (std::unordered_map > >::iterator iter = mInfoIndex.begin(); iter != mInfoIndex.end();) { for (std::vector >::iterator it = iter->second.begin(); @@ -383,7 +377,7 @@ void CSMWorld::InfoCollection::insertRecord (std::unique_ptr record, // adjust index if (index < size-1) { - for (std::map > >::iterator iter + for (std::unordered_map > >::iterator iter = mInfoIndex.begin(); iter != mInfoIndex.end(); ++iter) { for (std::vector >::iterator it = iter->second.begin(); @@ -397,9 +391,9 @@ void CSMWorld::InfoCollection::insertRecord (std::unique_ptr record, // get iterator for existing topic or a new topic std::string lowerId = Misc::StringUtils::lowerCase(id); - std::pair > >::iterator, bool> res + std::pair > >::iterator, bool> res = mInfoIndex.insert( - std::make_pair(StringHash(std::make_shared(lowerId.substr(0, separator))), + std::make_pair(lowerId.substr(0, separator), std::vector >())); // empty vector // insert info and index diff --git a/apps/opencs/model/world/infocollection.hpp b/apps/opencs/model/world/infocollection.hpp index 1417af6659..8974cbb8fb 100644 --- a/apps/opencs/model/world/infocollection.hpp +++ b/apps/opencs/model/world/infocollection.hpp @@ -1,8 +1,6 @@ #ifndef CSM_WOLRD_INFOCOLLECTION_H #define CSM_WOLRD_INFOCOLLECTION_H -#include - #include "collection.hpp" #include "info.hpp" @@ -11,37 +9,6 @@ namespace ESM struct Dialogue; } -namespace CSMWorld -{ - struct StringHash - { - uint64_t mHash; - std::shared_ptr mString; - - StringHash (std::shared_ptr str) : mString(str) - { - mHash = MurmurHash64A(str->c_str(), str->size(), /*seed*/1); - } - }; -} - -namespace std -{ - template<> struct less - { - bool operator() (const CSMWorld::StringHash& lhs, const CSMWorld::StringHash& rhs) const - { - if (lhs.mHash < rhs.mHash) - return true; - - if (lhs.mHash > rhs.mHash) - return false; - - return *lhs.mString < *rhs.mString; - } - }; -} - namespace CSMWorld { template<> @@ -69,8 +36,8 @@ namespace CSMWorld // each topic has a small number of infos, which allows the use of vectors for // iterating through them without too much penalty. // - // NOTE: hashed topic string as well as id string are stored in lower case. - std::map > > mInfoIndex; + // NOTE: topic string as well as id string are stored in lower case. + std::unordered_map > > mInfoIndex; void load (const Info& record, bool base); diff --git a/apps/opencs/model/world/refcollection.cpp b/apps/opencs/model/world/refcollection.cpp index 61cdd8205a..1f6fc76c4d 100644 --- a/apps/opencs/model/world/refcollection.cpp +++ b/apps/opencs/model/world/refcollection.cpp @@ -1,5 +1,7 @@ #include "refcollection.hpp" +#include + #include #include "ref.hpp" @@ -120,9 +122,9 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool } else { - std::unique_ptr > record2(new Record(getRecord(index))); - record2->mState = RecordBase::State_Deleted; - setRecord(index, std::move(record2)); + std::unique_ptr > record(new Record(getRecord(index))); + record->mState = RecordBase::State_Deleted; + setRecord(index, std::move(record)); } continue; @@ -181,14 +183,19 @@ std::string CSMWorld::RefCollection::getNewId() return "ref#" + std::to_string(mNextId++); } -unsigned int CSMWorld::RefCollection::extractIdNum (const std::string& id) const +unsigned int CSMWorld::RefCollection::extractIdNum(std::string_view id) const { - std::string::size_type separator = id.find_last_of('#'); + const auto separator = id.find_last_of('#'); + + if (separator == std::string_view::npos) + throw std::runtime_error("invalid ref ID: " + std::string(id)); - if (separator == std::string::npos) - throw std::runtime_error("invalid ref ID: " + id); + const std::string_view number = id.substr(separator + 1); + unsigned int result; + if (std::from_chars(number.data(), number.data() + number.size(), result).ec != std::errc()) + throw std::runtime_error("invalid ref ID number: " + std::string(number)); - return static_cast(std::stoi(id.substr(separator+1))); + return result; } int CSMWorld::RefCollection::getIntIndex (unsigned int id) const @@ -235,15 +242,15 @@ void CSMWorld::RefCollection::removeRows (int index, int count) void CSMWorld::RefCollection::appendBlankRecord (const std::string& id, UniversalId::Type type) { - std::unique_ptr > record2(new Record); + std::unique_ptr > record(new Record); - record2->mState = Record::State_ModifiedOnly; - record2->mModified.blank(); + record->mState = Record::State_ModifiedOnly; + record->mModified.blank(); - record2->get().mId = id; - record2->get().mIdNum = extractIdNum(id); + record->get().mId = id; + record->get().mIdNum = extractIdNum(id); - Collection >::appendRecord(std::move(record2)); + Collection >::appendRecord(std::move(record)); } void CSMWorld::RefCollection::cloneRecord (const std::string& origin, diff --git a/apps/opencs/model/world/refcollection.hpp b/apps/opencs/model/world/refcollection.hpp index 1bdfda3925..e0e88d721f 100644 --- a/apps/opencs/model/world/refcollection.hpp +++ b/apps/opencs/model/world/refcollection.hpp @@ -29,7 +29,7 @@ namespace CSMWorld int mNextId; - unsigned int extractIdNum(const std::string& id) const; + unsigned int extractIdNum(std::string_view id) const; int getIntIndex (unsigned int id) const; diff --git a/apps/opencs/model/world/refiddata.hpp b/apps/opencs/model/world/refiddata.hpp index 7eeb936985..c921ab7244 100644 --- a/apps/opencs/model/world/refiddata.hpp +++ b/apps/opencs/model/world/refiddata.hpp @@ -94,13 +94,13 @@ namespace CSMWorld template void RefIdDataContainer::insertRecord(std::unique_ptr record) { - Record *tmp = dynamic_cast*>(record.get()); - if(tmp != nullptr) + // convert base pointer to record type pointer first + if(Record *tmp = dynamic_cast*>(record.get())) { - record.release(); std::unique_ptr > newRecord; newRecord.reset(tmp); mContainer.push_back(std::move(newRecord)); + record.release(); } else throw std::runtime_error ("invalid record for RefIdDataContainer"); diff --git a/extern/murmurhash/CMakeLists.txt b/extern/murmurhash/CMakeLists.txt deleted file mode 100644 index cd8199ff06..0000000000 --- a/extern/murmurhash/CMakeLists.txt +++ /dev/null @@ -1,17 +0,0 @@ -cmake_minimum_required(VERSION 2.8) - -# This is NOT intended as a stand-alone build system! Instead, you should include this from the main CMakeLists of your project. - -set(MURMURHASH_LIBRARY "murmurhash") - -# Sources -set(SOURCE_FILES - MurmurHash2.cpp -) - -add_library(${MURMURHASH_LIBRARY} STATIC ${SOURCE_FILES}) - -set(MURMURHASH_LIBRARIES ${MURMURHASH_LIBRARY}) - -link_directories(${CMAKE_CURRENT_BINARY_DIR}) -set(MURMURHASH_LIBRARIES ${MURMURHASH_LIBRARIES} PARENT_SCOPE) diff --git a/extern/murmurhash/MurmurHash2.cpp b/extern/murmurhash/MurmurHash2.cpp deleted file mode 100644 index d1b6f476e8..0000000000 --- a/extern/murmurhash/MurmurHash2.cpp +++ /dev/null @@ -1,522 +0,0 @@ -//----------------------------------------------------------------------------- -// MurmurHash2 was written by Austin Appleby, and is placed in the public -// domain. The author hereby disclaims copyright to this source code. - -// Note - This code makes a few assumptions about how your machine behaves - - -// 1. We can read a 4-byte value from any address without crashing -// 2. sizeof(int) == 4 - -// And it has a few limitations - - -// 1. It will not work incrementally. -// 2. It will not produce the same results on little-endian and big-endian -// machines. - -#include "MurmurHash2.h" - -//----------------------------------------------------------------------------- -// Platform-specific functions and macros - -// Microsoft Visual Studio - -#if defined(_MSC_VER) - -#define BIG_CONSTANT(x) (x) - -// Other compilers - -#else // defined(_MSC_VER) - -#define BIG_CONSTANT(x) (x##LLU) - -#endif // !defined(_MSC_VER) - -//----------------------------------------------------------------------------- - -uint32_t MurmurHash2 ( const void * key, int len, uint32_t seed ) -{ - // 'm' and 'r' are mixing constants generated offline. - // They're not really 'magic', they just happen to work well. - - const uint32_t m = 0x5bd1e995; - const int r = 24; - - // Initialize the hash to a 'random' value - - uint32_t h = seed ^ len; - - // Mix 4 bytes at a time into the hash - - const unsigned char * data = (const unsigned char *)key; - - while(len >= 4) - { - uint32_t k = *(uint32_t*)data; - - k *= m; - k ^= k >> r; - k *= m; - - h *= m; - h ^= k; - - data += 4; - len -= 4; - } - - // Handle the last few bytes of the input array - - switch(len) - { - case 3: h ^= data[2] << 16; - case 2: h ^= data[1] << 8; - case 1: h ^= data[0]; - h *= m; - }; - - // Do a few final mixes of the hash to ensure the last few - // bytes are well-incorporated. - - h ^= h >> 13; - h *= m; - h ^= h >> 15; - - return h; -} - -//----------------------------------------------------------------------------- -// MurmurHash2, 64-bit versions, by Austin Appleby - -// The same caveats as 32-bit MurmurHash2 apply here - beware of alignment -// and endian-ness issues if used across multiple platforms. - -// 64-bit hash for 64-bit platforms - -uint64_t MurmurHash64A ( const void * key, int len, uint64_t seed ) -{ - const uint64_t m = BIG_CONSTANT(0xc6a4a7935bd1e995); - const int r = 47; - - uint64_t h = seed ^ (len * m); - - const uint64_t * data = (const uint64_t *)key; - const uint64_t * end = data + (len/8); - - while(data != end) - { - uint64_t k = *data++; - - k *= m; - k ^= k >> r; - k *= m; - - h ^= k; - h *= m; - } - - const unsigned char * data2 = (const unsigned char*)data; - - switch(len & 7) - { - case 7: h ^= uint64_t(data2[6]) << 48; - case 6: h ^= uint64_t(data2[5]) << 40; - case 5: h ^= uint64_t(data2[4]) << 32; - case 4: h ^= uint64_t(data2[3]) << 24; - case 3: h ^= uint64_t(data2[2]) << 16; - case 2: h ^= uint64_t(data2[1]) << 8; - case 1: h ^= uint64_t(data2[0]); - h *= m; - }; - - h ^= h >> r; - h *= m; - h ^= h >> r; - - return h; -} - - -// 64-bit hash for 32-bit platforms - -uint64_t MurmurHash64B ( const void * key, int len, uint64_t seed ) -{ - const uint32_t m = 0x5bd1e995; - const int r = 24; - - uint32_t h1 = uint32_t(seed) ^ len; - uint32_t h2 = uint32_t(seed >> 32); - - const uint32_t * data = (const uint32_t *)key; - - while(len >= 8) - { - uint32_t k1 = *data++; - k1 *= m; k1 ^= k1 >> r; k1 *= m; - h1 *= m; h1 ^= k1; - len -= 4; - - uint32_t k2 = *data++; - k2 *= m; k2 ^= k2 >> r; k2 *= m; - h2 *= m; h2 ^= k2; - len -= 4; - } - - if(len >= 4) - { - uint32_t k1 = *data++; - k1 *= m; k1 ^= k1 >> r; k1 *= m; - h1 *= m; h1 ^= k1; - len -= 4; - } - - switch(len) - { - case 3: h2 ^= ((unsigned char*)data)[2] << 16; - case 2: h2 ^= ((unsigned char*)data)[1] << 8; - case 1: h2 ^= ((unsigned char*)data)[0]; - h2 *= m; - }; - - h1 ^= h2 >> 18; h1 *= m; - h2 ^= h1 >> 22; h2 *= m; - h1 ^= h2 >> 17; h1 *= m; - h2 ^= h1 >> 19; h2 *= m; - - uint64_t h = h1; - - h = (h << 32) | h2; - - return h; -} - -//----------------------------------------------------------------------------- -// MurmurHash2A, by Austin Appleby - -// This is a variant of MurmurHash2 modified to use the Merkle-Damgard -// construction. Bulk speed should be identical to Murmur2, small-key speed -// will be 10%-20% slower due to the added overhead at the end of the hash. - -// This variant fixes a minor issue where null keys were more likely to -// collide with each other than expected, and also makes the function -// more amenable to incremental implementations. - -#define mmix(h,k) { k *= m; k ^= k >> r; k *= m; h *= m; h ^= k; } - -uint32_t MurmurHash2A ( const void * key, int len, uint32_t seed ) -{ - const uint32_t m = 0x5bd1e995; - const int r = 24; - uint32_t l = len; - - const unsigned char * data = (const unsigned char *)key; - - uint32_t h = seed; - - while(len >= 4) - { - uint32_t k = *(uint32_t*)data; - - mmix(h,k); - - data += 4; - len -= 4; - } - - uint32_t t = 0; - - switch(len) - { - case 3: t ^= data[2] << 16; - case 2: t ^= data[1] << 8; - case 1: t ^= data[0]; - }; - - mmix(h,t); - mmix(h,l); - - h ^= h >> 13; - h *= m; - h ^= h >> 15; - - return h; -} - -//----------------------------------------------------------------------------- -// CMurmurHash2A, by Austin Appleby - -// This is a sample implementation of MurmurHash2A designed to work -// incrementally. - -// Usage - - -// CMurmurHash2A hasher -// hasher.Begin(seed); -// hasher.Add(data1,size1); -// hasher.Add(data2,size2); -// ... -// hasher.Add(dataN,sizeN); -// uint32_t hash = hasher.End() - -class CMurmurHash2A -{ -public: - - void Begin ( uint32_t seed = 0 ) - { - m_hash = seed; - m_tail = 0; - m_count = 0; - m_size = 0; - } - - void Add ( const unsigned char * data, int len ) - { - m_size += len; - - MixTail(data,len); - - while(len >= 4) - { - uint32_t k = *(uint32_t*)data; - - mmix(m_hash,k); - - data += 4; - len -= 4; - } - - MixTail(data,len); - } - - uint32_t End ( void ) - { - mmix(m_hash,m_tail); - mmix(m_hash,m_size); - - m_hash ^= m_hash >> 13; - m_hash *= m; - m_hash ^= m_hash >> 15; - - return m_hash; - } - -private: - - static const uint32_t m = 0x5bd1e995; - static const int r = 24; - - void MixTail ( const unsigned char * & data, int & len ) - { - while( len && ((len<4) || m_count) ) - { - m_tail |= (*data++) << (m_count * 8); - - m_count++; - len--; - - if(m_count == 4) - { - mmix(m_hash,m_tail); - m_tail = 0; - m_count = 0; - } - } - } - - uint32_t m_hash; - uint32_t m_tail; - uint32_t m_count; - uint32_t m_size; -}; - -//----------------------------------------------------------------------------- -// MurmurHashNeutral2, by Austin Appleby - -// Same as MurmurHash2, but endian- and alignment-neutral. -// Half the speed though, alas. - -uint32_t MurmurHashNeutral2 ( const void * key, int len, uint32_t seed ) -{ - const uint32_t m = 0x5bd1e995; - const int r = 24; - - uint32_t h = seed ^ len; - - const unsigned char * data = (const unsigned char *)key; - - while(len >= 4) - { - uint32_t k; - - k = data[0]; - k |= data[1] << 8; - k |= data[2] << 16; - k |= data[3] << 24; - - k *= m; - k ^= k >> r; - k *= m; - - h *= m; - h ^= k; - - data += 4; - len -= 4; - } - - switch(len) - { - case 3: h ^= data[2] << 16; - case 2: h ^= data[1] << 8; - case 1: h ^= data[0]; - h *= m; - }; - - h ^= h >> 13; - h *= m; - h ^= h >> 15; - - return h; -} - -//----------------------------------------------------------------------------- -// MurmurHashAligned2, by Austin Appleby - -// Same algorithm as MurmurHash2, but only does aligned reads - should be safer -// on certain platforms. - -// Performance will be lower than MurmurHash2 - -#define MIX(h,k,m) { k *= m; k ^= k >> r; k *= m; h *= m; h ^= k; } - - -uint32_t MurmurHashAligned2 ( const void * key, int len, uint32_t seed ) -{ - const uint32_t m = 0x5bd1e995; - const int r = 24; - - const unsigned char * data = (const unsigned char *)key; - - uint32_t h = seed ^ len; - - int align = (uint64_t)data & 3; - - if(align && (len >= 4)) - { - // Pre-load the temp registers - - uint32_t t = 0, d = 0; - - switch(align) - { - case 1: t |= data[2] << 16; - case 2: t |= data[1] << 8; - case 3: t |= data[0]; - } - - t <<= (8 * align); - - data += 4-align; - len -= 4-align; - - int sl = 8 * (4-align); - int sr = 8 * align; - - // Mix - - while(len >= 4) - { - d = *(uint32_t *)data; - t = (t >> sr) | (d << sl); - - uint32_t k = t; - - MIX(h,k,m); - - t = d; - - data += 4; - len -= 4; - } - - // Handle leftover data in temp registers - - d = 0; - - if(len >= align) - { - switch(align) - { - case 3: d |= data[2] << 16; - case 2: d |= data[1] << 8; - case 1: d |= data[0]; - } - - uint32_t k = (t >> sr) | (d << sl); - MIX(h,k,m); - - data += align; - len -= align; - - //---------- - // Handle tail bytes - - switch(len) - { - case 3: h ^= data[2] << 16; - case 2: h ^= data[1] << 8; - case 1: h ^= data[0]; - h *= m; - }; - } - else - { - switch(len) - { - case 3: d |= data[2] << 16; - case 2: d |= data[1] << 8; - case 1: d |= data[0]; - case 0: h ^= (t >> sr) | (d << sl); - h *= m; - } - } - - h ^= h >> 13; - h *= m; - h ^= h >> 15; - - return h; - } - else - { - while(len >= 4) - { - uint32_t k = *(uint32_t *)data; - - MIX(h,k,m); - - data += 4; - len -= 4; - } - - //---------- - // Handle tail bytes - - switch(len) - { - case 3: h ^= data[2] << 16; - case 2: h ^= data[1] << 8; - case 1: h ^= data[0]; - h *= m; - }; - - h ^= h >> 13; - h *= m; - h ^= h >> 15; - - return h; - } -} - -//----------------------------------------------------------------------------- diff --git a/extern/murmurhash/MurmurHash2.h b/extern/murmurhash/MurmurHash2.h deleted file mode 100644 index e6d0c36924..0000000000 --- a/extern/murmurhash/MurmurHash2.h +++ /dev/null @@ -1,38 +0,0 @@ -//----------------------------------------------------------------------------- -// MurmurHash2 was written by Austin Appleby, and is placed in the public -// domain. The author hereby disclaims copyright to this source code. - -#ifndef _MURMURHASH2_H_ -#define _MURMURHASH2_H_ - -//----------------------------------------------------------------------------- -// Platform-specific functions and macros - -// Microsoft Visual Studio - -#if defined(_MSC_VER) && (_MSC_VER < 1600) - -typedef unsigned char uint8_t; -typedef unsigned int uint32_t; -typedef unsigned __int64 uint64_t; - -// Other compilers - -#else // defined(_MSC_VER) - -#include - -#endif // !defined(_MSC_VER) - -//----------------------------------------------------------------------------- - -uint32_t MurmurHash2 ( const void * key, int len, uint32_t seed ); -uint64_t MurmurHash64A ( const void * key, int len, uint64_t seed ); -uint64_t MurmurHash64B ( const void * key, int len, uint64_t seed ); -uint32_t MurmurHash2A ( const void * key, int len, uint32_t seed ); -uint32_t MurmurHashNeutral2 ( const void * key, int len, uint32_t seed ); -uint32_t MurmurHashAligned2 ( const void * key, int len, uint32_t seed ); - -//----------------------------------------------------------------------------- - -#endif // _MURMURHASH2_H_ From 3e466699c8c68961f0160bc676be97ca91e25bf3 Mon Sep 17 00:00:00 2001 From: cc9cii Date: Sat, 24 Jul 2021 21:23:03 +1000 Subject: [PATCH 11/11] A better way to plug a potential memory leak in the event of an exception during push_back(). --- apps/opencs/model/world/refiddata.hpp | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/apps/opencs/model/world/refiddata.hpp b/apps/opencs/model/world/refiddata.hpp index c921ab7244..175aaef410 100644 --- a/apps/opencs/model/world/refiddata.hpp +++ b/apps/opencs/model/world/refiddata.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -94,16 +95,11 @@ namespace CSMWorld template void RefIdDataContainer::insertRecord(std::unique_ptr record) { - // convert base pointer to record type pointer first - if(Record *tmp = dynamic_cast*>(record.get())) - { - std::unique_ptr > newRecord; - newRecord.reset(tmp); - mContainer.push_back(std::move(newRecord)); - record.release(); - } - else - throw std::runtime_error ("invalid record for RefIdDataContainer"); + assert(record != nullptr); + // convert base pointer to record type pointer + std::unique_ptr> typedRecord(&dynamic_cast&>(*record)); + record.release(); + mContainer.push_back(std::move(typedRecord)); } template