From 6cf74f7041c120f25e27d54b579ee19bd62bc2a5 Mon Sep 17 00:00:00 2001 From: Bo Svensson <90132211+bosvensson1@users.noreply.github.com> Date: Thu, 4 Nov 2021 15:55:32 +0000 Subject: [PATCH] refactors ESM::Land (#3213) With this PR we reduce coupling, simplify code, encapsulate a variable and separate actual `ESM` data from its context. --- apps/opencs/model/world/collection.hpp | 4 ++-- apps/opencs/model/world/columnimp.cpp | 2 +- apps/openmw/mwworld/esmstore.cpp | 2 +- apps/openmw/mwworld/store.cpp | 15 ++------------- apps/openmw/mwworld/store.hpp | 5 ++--- components/esm/loadland.cpp | 8 ++------ components/esm/loadland.hpp | 5 ++++- components/esmterrain/storage.hpp | 6 +----- 8 files changed, 15 insertions(+), 32 deletions(-) diff --git a/apps/opencs/model/world/collection.hpp b/apps/opencs/model/world/collection.hpp index 6ab9d7ff9d..d15a4ff54d 100644 --- a/apps/opencs/model/world/collection.hpp +++ b/apps/opencs/model/world/collection.hpp @@ -296,7 +296,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().setPlugin(0); } template @@ -311,7 +311,7 @@ namespace CSMWorld int index = touchRecordImp(id); if (index >= 0) { - mRecords.at(index)->get().mPlugin = 0; + mRecords.at(index)->get().setPlugin(0); return true; } diff --git a/apps/opencs/model/world/columnimp.cpp b/apps/opencs/model/world/columnimp.cpp index bec5008d35..0244ab1e8d 100644 --- a/apps/opencs/model/world/columnimp.cpp +++ b/apps/opencs/model/world/columnimp.cpp @@ -52,7 +52,7 @@ namespace CSMWorld QVariant LandPluginIndexColumn::get(const Record& record) const { - return record.get().mPlugin; + return record.get().getPlugin(); } bool LandPluginIndexColumn::isEditable() const diff --git a/apps/openmw/mwworld/esmstore.cpp b/apps/openmw/mwworld/esmstore.cpp index 183b13ca09..bafdc8f37d 100644 --- a/apps/openmw/mwworld/esmstore.cpp +++ b/apps/openmw/mwworld/esmstore.cpp @@ -153,7 +153,7 @@ void ESMStore::load(ESM::ESMReader &esm, Loading::Listener* listener) // Land texture loading needs to use a separate internal store for each plugin. // We set the number of plugins here so we can properly verify if valid plugin // indices are being passed to the LandTexture Store retrieval methods. - mLandTextures.resize(mLandTextures.getSize()+1); + mLandTextures.addPlugin(); // Loop through all records while(esm.hasMoreRecs()) diff --git a/apps/openmw/mwworld/store.cpp b/apps/openmw/mwworld/store.cpp index 5e3f4079c3..4d720a11a7 100644 --- a/apps/openmw/mwworld/store.cpp +++ b/apps/openmw/mwworld/store.cpp @@ -322,15 +322,13 @@ namespace MWWorld assert(plugin < mStatic.size()); return mStatic[plugin].size(); } - RecordId Store::load(ESM::ESMReader &esm, size_t plugin) + RecordId Store::load(ESM::ESMReader &esm) { ESM::LandTexture lt; bool isDeleted = false; lt.load(esm, isDeleted); - assert(plugin < mStatic.size()); - // Replace texture for records with given ID and index from all plugins. for (unsigned int i=0; i (int)ltexl.size()) ltexl.resize(lt.mIndex+1); @@ -352,10 +350,6 @@ namespace MWWorld return RecordId(ltexl[idx].mId, isDeleted); } - RecordId Store::load(ESM::ESMReader &esm) - { - return load(esm, esm.getIndex()); - } Store::iterator Store::begin(size_t plugin) const { assert(plugin < mStatic.size()); @@ -366,11 +360,6 @@ namespace MWWorld assert(plugin < mStatic.size()); return mStatic[plugin].end(); } - void Store::resize(size_t num) - { - if (mStatic.size() < num) - mStatic.resize(num); - } // Land //========================================================================= diff --git a/apps/openmw/mwworld/store.hpp b/apps/openmw/mwworld/store.hpp index 4c32b65ec7..17a37c23ea 100644 --- a/apps/openmw/mwworld/store.hpp +++ b/apps/openmw/mwworld/store.hpp @@ -222,13 +222,12 @@ namespace MWWorld const ESM::LandTexture *search(size_t index, size_t plugin) const; const ESM::LandTexture *find(size_t index, size_t plugin) const; - /// Resize the internal store to hold at least \a num plugins. - void resize(size_t num); + /// Resize the internal store to hold another plugin. + void addPlugin() { mStatic.emplace_back(); } size_t getSize() const override; size_t getSize(size_t plugin) const; - RecordId load(ESM::ESMReader &esm, size_t plugin); RecordId load(ESM::ESMReader &esm) override; iterator begin(size_t plugin) const; diff --git a/components/esm/loadland.cpp b/components/esm/loadland.cpp index d7dcd47c69..3c4a1e0b07 100644 --- a/components/esm/loadland.cpp +++ b/components/esm/loadland.cpp @@ -15,7 +15,6 @@ namespace ESM : mFlags(0) , mX(0) , mY(0) - , mPlugin(0) , mDataTypes(0) , mLandData(nullptr) { @@ -40,8 +39,6 @@ namespace ESM { isDeleted = false; - mPlugin = esm.getIndex(); - bool hasLocation = false; bool isLoaded = false; while (!isLoaded && esm.hasMoreSubs()) @@ -192,7 +189,7 @@ namespace ESM void Land::blank() { - mPlugin = 0; + setPlugin(0); std::fill(std::begin(mWnam), std::end(mWnam), 0); @@ -326,7 +323,7 @@ namespace ESM } Land::Land (const Land& land) - : mFlags (land.mFlags), mX (land.mX), mY (land.mY), mPlugin (land.mPlugin), + : mFlags (land.mFlags), mX (land.mX), mY (land.mY), mContext (land.mContext), mDataTypes (land.mDataTypes), mLandData (land.mLandData ? new LandData (*land.mLandData) : nullptr) { @@ -345,7 +342,6 @@ namespace ESM std::swap (mFlags, land.mFlags); std::swap (mX, land.mX); std::swap (mY, land.mY); - std::swap (mPlugin, land.mPlugin); std::swap (mContext, land.mContext); std::swap (mDataTypes, land.mDataTypes); std::swap (mLandData, land.mLandData); diff --git a/components/esm/loadland.hpp b/components/esm/loadland.hpp index 67dd2e76a2..610dd28fb8 100644 --- a/components/esm/loadland.hpp +++ b/components/esm/loadland.hpp @@ -29,7 +29,10 @@ struct Land int mFlags; // Only first four bits seem to be used, don't know what // they mean. int mX, mY; // Map coordinates. - int mPlugin; // Plugin index, used to reference the correct material palette. + + // Plugin index, used to reference the correct material palette. + int getPlugin() const { return mContext.index; } + void setPlugin(int index) { mContext.index = index; } // File context. This allows the ESM reader to be 'reset' to this // location later when we are ready to load the full data set. diff --git a/components/esmterrain/storage.hpp b/components/esmterrain/storage.hpp index 68e71574ee..107255a7af 100644 --- a/components/esmterrain/storage.hpp +++ b/components/esmterrain/storage.hpp @@ -36,11 +36,7 @@ namespace ESMTerrain return nullptr; return &mData; } - - inline int getPlugin() const - { - return mLand->mPlugin; - } + inline int getPlugin() const { return mLand->getPlugin(); } private: const ESM::Land* mLand;