From a54ab153b0cdd226f9ad2e9c91913408dbf6a0da Mon Sep 17 00:00:00 2001 From: cc9cii Date: Sun, 8 Mar 2015 10:05:10 +1100 Subject: [PATCH 1/8] Cloned references should be considered "Base" rather than "Modified". Should fix bug #2429. --- apps/opencs/model/world/refidcollection.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/opencs/model/world/refidcollection.cpp b/apps/opencs/model/world/refidcollection.cpp index 779d5a40cd..75429d906d 100644 --- a/apps/opencs/model/world/refidcollection.cpp +++ b/apps/opencs/model/world/refidcollection.cpp @@ -471,7 +471,7 @@ void CSMWorld::RefIdCollection::cloneRecord(const std::string& origin, const CSMWorld::UniversalId::Type type) { std::auto_ptr newRecord(mData.getRecord(mData.searchId(origin)).clone()); - newRecord->mState = RecordBase::State_ModifiedOnly; + newRecord->mState = RecordBase::State_BaseOnly; mAdapters.find(type)->second->setId(*newRecord, destination); mData.insertRecord(*newRecord, type, destination); } From 128371c902cf40f98d6199e7dc267c9f3e778130 Mon Sep 17 00:00:00 2001 From: cc9cii Date: Sun, 8 Mar 2015 15:50:50 +1100 Subject: [PATCH 2/8] Copy base data to modified. --- apps/opencs/model/world/record.hpp | 4 +++- apps/opencs/model/world/refidcollection.cpp | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/opencs/model/world/record.hpp b/apps/opencs/model/world/record.hpp index 861fc47a3b..2b556636f1 100644 --- a/apps/opencs/model/world/record.hpp +++ b/apps/opencs/model/world/record.hpp @@ -61,7 +61,9 @@ namespace CSMWorld template RecordBase *Record::clone() const { - return new Record (*this); + Record *copy = new Record (*this); + copy->mModified = (*this).get(); + return copy; } template diff --git a/apps/opencs/model/world/refidcollection.cpp b/apps/opencs/model/world/refidcollection.cpp index 75429d906d..779d5a40cd 100644 --- a/apps/opencs/model/world/refidcollection.cpp +++ b/apps/opencs/model/world/refidcollection.cpp @@ -471,7 +471,7 @@ void CSMWorld::RefIdCollection::cloneRecord(const std::string& origin, const CSMWorld::UniversalId::Type type) { std::auto_ptr newRecord(mData.getRecord(mData.searchId(origin)).clone()); - newRecord->mState = RecordBase::State_BaseOnly; + newRecord->mState = RecordBase::State_ModifiedOnly; mAdapters.find(type)->second->setId(*newRecord, destination); mData.insertRecord(*newRecord, type, destination); } From 6087a18c94e418c43b681accfb72efdef7cb2242 Mon Sep 17 00:00:00 2001 From: cc9cii Date: Mon, 9 Mar 2015 14:58:07 +1100 Subject: [PATCH 3/8] Implement clone() using a new Record constructor. --- apps/opencs/model/world/record.hpp | 20 +++++++++++++++++--- apps/opencs/model/world/refidcollection.cpp | 1 - 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/apps/opencs/model/world/record.hpp b/apps/opencs/model/world/record.hpp index 2b556636f1..1f97ece93a 100644 --- a/apps/opencs/model/world/record.hpp +++ b/apps/opencs/model/world/record.hpp @@ -38,6 +38,10 @@ namespace CSMWorld ESXRecordT mBase; ESXRecordT mModified; + Record() {} + Record(State state, + const ESXRecordT *base = 0, const ESXRecordT *modified = 0); + virtual RecordBase *clone() const; virtual void assign (const RecordBase& record); @@ -58,12 +62,22 @@ namespace CSMWorld ///< Merge modified into base. }; + template + Record::Record(State state, const ESXRecordT *base = 0, const ESXRecordT *modified = 0) + { + if(base) + mBase = *base; + + if(modified) + mModified = *modified; + + this->mState = state; + } + template RecordBase *Record::clone() const { - Record *copy = new Record (*this); - copy->mModified = (*this).get(); - return copy; + return new Record (State_ModifiedOnly, 0, &(this->get())); } template diff --git a/apps/opencs/model/world/refidcollection.cpp b/apps/opencs/model/world/refidcollection.cpp index 779d5a40cd..011b5cc0e9 100644 --- a/apps/opencs/model/world/refidcollection.cpp +++ b/apps/opencs/model/world/refidcollection.cpp @@ -471,7 +471,6 @@ void CSMWorld::RefIdCollection::cloneRecord(const std::string& origin, const CSMWorld::UniversalId::Type type) { std::auto_ptr newRecord(mData.getRecord(mData.searchId(origin)).clone()); - newRecord->mState = RecordBase::State_ModifiedOnly; mAdapters.find(type)->second->setId(*newRecord, destination); mData.insertRecord(*newRecord, type, destination); } From f90cdec53b122b220a20fadda1e82122b93eb2fb Mon Sep 17 00:00:00 2001 From: cc9cii Date: Mon, 9 Mar 2015 16:24:35 +1100 Subject: [PATCH 4/8] Remove default parameters from the implementation. --- apps/opencs/model/world/record.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/opencs/model/world/record.hpp b/apps/opencs/model/world/record.hpp index 1f97ece93a..5f935e30d6 100644 --- a/apps/opencs/model/world/record.hpp +++ b/apps/opencs/model/world/record.hpp @@ -63,7 +63,7 @@ namespace CSMWorld }; template - Record::Record(State state, const ESXRecordT *base = 0, const ESXRecordT *modified = 0) + Record::Record(State state, const ESXRecordT *base, const ESXRecordT *modified) { if(base) mBase = *base; From 8b3adec3ec5e8009c3266b72a660fe4c2f3712a5 Mon Sep 17 00:00:00 2001 From: cc9cii Date: Mon, 9 Mar 2015 21:25:41 +1100 Subject: [PATCH 5/8] Added a missing copy constructor. --- apps/opencs/model/world/record.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/opencs/model/world/record.hpp b/apps/opencs/model/world/record.hpp index 5f935e30d6..814e116dd1 100644 --- a/apps/opencs/model/world/record.hpp +++ b/apps/opencs/model/world/record.hpp @@ -38,7 +38,8 @@ namespace CSMWorld ESXRecordT mBase; ESXRecordT mModified; - Record() {} + Record() = default; + Record(const Record&) = default; Record(State state, const ESXRecordT *base = 0, const ESXRecordT *modified = 0); From 43ec933b7ba44186de8b2fd0ffa4f4d89a7cfcec Mon Sep 17 00:00:00 2001 From: cc9cii Date: Tue, 10 Mar 2015 09:45:35 +1100 Subject: [PATCH 6/8] Revert to the original clone() method. Create a new copy method for modified records. --- apps/opencs/model/world/record.hpp | 25 +++++++++------------ apps/opencs/model/world/refidcollection.cpp | 2 +- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/apps/opencs/model/world/record.hpp b/apps/opencs/model/world/record.hpp index 814e116dd1..a0ddd8bc80 100644 --- a/apps/opencs/model/world/record.hpp +++ b/apps/opencs/model/world/record.hpp @@ -22,6 +22,8 @@ namespace CSMWorld virtual RecordBase *clone() const = 0; + virtual RecordBase *modifiedCopy() const = 0; + virtual void assign (const RecordBase& record) = 0; ///< Will throw an exception if the types don't match. @@ -38,13 +40,10 @@ namespace CSMWorld ESXRecordT mBase; ESXRecordT mModified; - Record() = default; - Record(const Record&) = default; - Record(State state, - const ESXRecordT *base = 0, const ESXRecordT *modified = 0); - virtual RecordBase *clone() const; + virtual RecordBase *modifiedCopy() const; + virtual void assign (const RecordBase& record); const ESXRecordT& get() const; @@ -64,21 +63,19 @@ namespace CSMWorld }; template - Record::Record(State state, const ESXRecordT *base, const ESXRecordT *modified) + RecordBase *Record::modifiedCopy() const { - if(base) - mBase = *base; - - if(modified) - mModified = *modified; - - this->mState = state; + Record *record = new Record (*this); + record->mModified = record->mBase; + record->mBase = ESXRecordT(); + record->mState = RecordBase::State_ModifiedOnly; + return record; } template RecordBase *Record::clone() const { - return new Record (State_ModifiedOnly, 0, &(this->get())); + return new Record (*this); } template diff --git a/apps/opencs/model/world/refidcollection.cpp b/apps/opencs/model/world/refidcollection.cpp index 011b5cc0e9..14a8890ad1 100644 --- a/apps/opencs/model/world/refidcollection.cpp +++ b/apps/opencs/model/world/refidcollection.cpp @@ -470,7 +470,7 @@ void CSMWorld::RefIdCollection::cloneRecord(const std::string& origin, const std::string& destination, const CSMWorld::UniversalId::Type type) { - std::auto_ptr newRecord(mData.getRecord(mData.searchId(origin)).clone()); + std::auto_ptr newRecord(mData.getRecord(mData.searchId(origin)).modifiedCopy()); mAdapters.find(type)->second->setId(*newRecord, destination); mData.insertRecord(*newRecord, type, destination); } From 28259f914c06100ea58ed31680d7b5deb80ead5f Mon Sep 17 00:00:00 2001 From: cc9cii Date: Wed, 11 Mar 2015 10:49:21 +1100 Subject: [PATCH 7/8] Remove potential memory leak. --- apps/opencs/model/world/record.hpp | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/apps/opencs/model/world/record.hpp b/apps/opencs/model/world/record.hpp index a0ddd8bc80..1cf14a8f9b 100644 --- a/apps/opencs/model/world/record.hpp +++ b/apps/opencs/model/world/record.hpp @@ -40,6 +40,13 @@ namespace CSMWorld ESXRecordT mBase; ESXRecordT mModified; + Record() = default; + Record(const Record&) = default; + Record& operator= (const Record&) = default; + + Record(State state, + const ESXRecordT *base = 0, const ESXRecordT *modified = 0); + virtual RecordBase *clone() const; virtual RecordBase *modifiedCopy() const; @@ -62,14 +69,22 @@ namespace CSMWorld ///< Merge modified into base. }; + template + Record::Record(State state, const ESXRecordT *base, const ESXRecordT *modified) + { + if(base) + mBase = *base; + + if(modified) + mModified = *modified; + + this->mState = state; + } + template RecordBase *Record::modifiedCopy() const { - Record *record = new Record (*this); - record->mModified = record->mBase; - record->mBase = ESXRecordT(); - record->mState = RecordBase::State_ModifiedOnly; - return record; + return new Record (State_ModifiedOnly, 0, &(this->get())); } template From dc9af19dcfbb075cfbafcb741adbd4fb6964c2b7 Mon Sep 17 00:00:00 2001 From: cc9cii Date: Thu, 12 Mar 2015 08:28:26 +1100 Subject: [PATCH 8/8] Don't use C++11 features. --- apps/opencs/model/world/record.hpp | 31 +++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/apps/opencs/model/world/record.hpp b/apps/opencs/model/world/record.hpp index 1cf14a8f9b..6ff33f0c2e 100644 --- a/apps/opencs/model/world/record.hpp +++ b/apps/opencs/model/world/record.hpp @@ -40,9 +40,9 @@ namespace CSMWorld ESXRecordT mBase; ESXRecordT mModified; - Record() = default; - Record(const Record&) = default; - Record& operator= (const Record&) = default; + Record(); + Record(const Record& record); + Record& operator= (const Record& record); Record(State state, const ESXRecordT *base = 0, const ESXRecordT *modified = 0); @@ -69,6 +69,31 @@ namespace CSMWorld ///< Merge modified into base. }; + template + Record::Record() + : mBase(), mModified() + { } + + template + Record::Record(const Record& record) + : mBase(record.mBase), mModified(record.mModified) + { + mState = record.mState; + } + + template + Record& Record::operator= (const Record& record) + { + if(this != &record) + { + mBase = record.mBase; + mModified = record.mModified; + mState = record.mState; + } + + return *this; + } + template Record::Record(State state, const ESXRecordT *base, const ESXRecordT *modified) {