From 96cf2cbd05b87ca2163381f8ee9cbed15654ee27 Mon Sep 17 00:00:00 2001 From: Doc West Date: Wed, 4 Jul 2018 22:37:10 +0200 Subject: [PATCH 1/7] Notify views of changes of all cells in a row to properly update the row after revert --- apps/opencs/model/world/commands.cpp | 9 +++++++++ apps/opencs/model/world/idtable.cpp | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/apps/opencs/model/world/commands.cpp b/apps/opencs/model/world/commands.cpp index 79900c6c4..a572ca632 100644 --- a/apps/opencs/model/world/commands.cpp +++ b/apps/opencs/model/world/commands.cpp @@ -319,6 +319,15 @@ void CSMWorld::RevertCommand::redo() } else { + // notify view that data has changed, previously only the modified column was + // updated in the view unless the user had selected another item or forced a + // repaint with other means + int count = mModel.columnCount (index.parent ()); + if (count > 0) + { + emit mModel.dataChanged(mModel.index (index.row(), 0, index.parent ()), + mModel.index (index.row(), count - 1, index.parent ())); + } mModel.setData (index, static_cast (RecordBase::State_BaseOnly)); } } diff --git a/apps/opencs/model/world/idtable.cpp b/apps/opencs/model/world/idtable.cpp index 3e503a80c..7492b6b17 100644 --- a/apps/opencs/model/world/idtable.cpp +++ b/apps/opencs/model/world/idtable.cpp @@ -86,9 +86,9 @@ bool CSMWorld::IdTable::setData (const QModelIndex &index, const QVariant &value mIdCollection->setData (index.row(), index.column(), value); emit dataChanged(index, index); - // Modifying a value can also change the Modified status of a record. + // Modifying a value can also change the Modified status of a record unless . int stateColumn = searchColumnIndex(Columns::ColumnId_Modification); - if (stateColumn != -1) + if (stateColumn != -1 && index.column() != stateColumn) { QModelIndex stateIndex = this->index(index.row(), stateColumn); emit dataChanged(stateIndex, stateIndex); From bf49a3e7603464611a80842d133ad55a006bb4db Mon Sep 17 00:00:00 2001 From: Doc West Date: Wed, 4 Jul 2018 23:21:55 +0200 Subject: [PATCH 2/7] Use setData() instead of emtitting dataChanged() which does not work on CI. Also Fixes the remaining issue with subviews not updating due to only the modified flag emitting a change, which prevented the widget mapper from working for updates. --- apps/opencs/model/world/commands.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/apps/opencs/model/world/commands.cpp b/apps/opencs/model/world/commands.cpp index a572ca632..c67de6350 100644 --- a/apps/opencs/model/world/commands.cpp +++ b/apps/opencs/model/world/commands.cpp @@ -323,11 +323,16 @@ void CSMWorld::RevertCommand::redo() // updated in the view unless the user had selected another item or forced a // repaint with other means int count = mModel.columnCount (index.parent ()); - if (count > 0) + for (int i=0; i (RecordBase::State_BaseOnly)); } } From 3cbbbeceb4eae004e3e53e46b188ad55c15b9a9e Mon Sep 17 00:00:00 2001 From: Doc West Date: Wed, 4 Jul 2018 23:25:24 +0200 Subject: [PATCH 3/7] Updated change log --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f839279f3..4f098139e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ Bug #4475: Scripted animations should not cause movement Bug #4479: "Game" category on Advanced page is getting too long Bug #4480: Segfalt in QuickKeysMenu when item no longer in inventory + Bug #3249: Fixed revert function not updating views properly Feature #3276: Editor: Search- Show number of (remaining) search results and indicate a search without any results Feature #3641: Editor: Limit FPS in 3d preview window Feature #4222: 360° screenshots From 9bfa01c57941b3d5c0d2eea8927fd75f828d679f Mon Sep 17 00:00:00 2001 From: Doc West Date: Thu, 5 Jul 2018 00:37:19 +0200 Subject: [PATCH 4/7] Changed the way the revert command works: it now clones the changed record and uses the new RecordBase::revert() method to restore the previous value Added Flag_Dialogue_Refresh to var type and var value columns so that sub views update properly --- apps/opencs/model/world/columnimp.hpp | 4 ++-- apps/opencs/model/world/commands.cpp | 20 +++++--------------- apps/opencs/model/world/commands.hpp | 1 + apps/opencs/model/world/record.hpp | 12 +++++++++++- 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/apps/opencs/model/world/columnimp.hpp b/apps/opencs/model/world/columnimp.hpp index 4ad447b0a..cd2a4c79e 100644 --- a/apps/opencs/model/world/columnimp.hpp +++ b/apps/opencs/model/world/columnimp.hpp @@ -136,7 +136,7 @@ namespace CSMWorld struct VarTypeColumn : public Column { VarTypeColumn (ColumnBase::Display display) - : Column (Columns::ColumnId_ValueType, display) + : Column (Columns::ColumnId_ValueType, display, ColumnBase::Flag_Table | ColumnBase::Flag_Dialogue | ColumnBase::Flag_Dialogue_Refresh) {} virtual QVariant get (const Record& record) const @@ -161,7 +161,7 @@ namespace CSMWorld template struct VarValueColumn : public Column { - VarValueColumn() : Column (Columns::ColumnId_Value, ColumnBase::Display_Var) {} + VarValueColumn() : Column (Columns::ColumnId_Value, ColumnBase::Display_Var, ColumnBase::Flag_Table | ColumnBase::Flag_Dialogue | ColumnBase::Flag_Dialogue_Refresh) {} virtual QVariant get (const Record& record) const { diff --git a/apps/opencs/model/world/commands.cpp b/apps/opencs/model/world/commands.cpp index c67de6350..ff4bab989 100644 --- a/apps/opencs/model/world/commands.cpp +++ b/apps/opencs/model/world/commands.cpp @@ -294,16 +294,19 @@ void CSMWorld::CreateCommand::undo() } CSMWorld::RevertCommand::RevertCommand (IdTable& model, const std::string& id, QUndoCommand* parent) -: QUndoCommand (parent), mModel (model), mId (id), mOld (0) +: QUndoCommand (parent), mModel (model), mId (id), mOld (0), mNew (0) { setText (("Revert record " + id).c_str()); + mNew = model.getRecord(id).clone(); + mNew->revert(); mOld = model.getRecord (id).clone(); } CSMWorld::RevertCommand::~RevertCommand() { delete mOld; + delete mNew; } void CSMWorld::RevertCommand::redo() @@ -319,20 +322,7 @@ void CSMWorld::RevertCommand::redo() } else { - // notify view that data has changed, previously only the modified column was - // updated in the view unless the user had selected another item or forced a - // repaint with other means - int count = mModel.columnCount (index.parent ()); - for (int i=0; i (RecordBase::State_BaseOnly)); } } diff --git a/apps/opencs/model/world/commands.hpp b/apps/opencs/model/world/commands.hpp index 58a1b1d1c..46fa6d8a0 100644 --- a/apps/opencs/model/world/commands.hpp +++ b/apps/opencs/model/world/commands.hpp @@ -201,6 +201,7 @@ namespace CSMWorld IdTable& mModel; std::string mId; RecordBase *mOld; + RecordBase *mNew; // not implemented RevertCommand (const RevertCommand&); diff --git a/apps/opencs/model/world/record.hpp b/apps/opencs/model/world/record.hpp index 0313f2e41..db5822c97 100644 --- a/apps/opencs/model/world/record.hpp +++ b/apps/opencs/model/world/record.hpp @@ -24,6 +24,8 @@ namespace CSMWorld virtual RecordBase *modifiedCopy() const = 0; + virtual void revert() = 0; + virtual void assign (const RecordBase& record) = 0; ///< Will throw an exception if the types don't match. @@ -49,6 +51,8 @@ namespace CSMWorld virtual RecordBase *modifiedCopy() const; + void revert() override; + virtual void assign (const RecordBase& record); const ESXRecordT& get() const; @@ -96,6 +100,12 @@ namespace CSMWorld return new Record (*this); } + template + void Record::revert() + { + mModified = mBase; + } + template void Record::assign (const RecordBase& record) { @@ -156,4 +166,4 @@ namespace CSMWorld } } -#endif \ No newline at end of file +#endif From e1877338113e4d1300e63984845f6a9280fe17fc Mon Sep 17 00:00:00 2001 From: Doc West Date: Thu, 5 Jul 2018 18:03:55 +0200 Subject: [PATCH 5/7] Notify views of changes in all columns when updating the ColumnId_Modification column --- apps/opencs/model/world/commands.cpp | 6 +----- apps/opencs/model/world/commands.hpp | 1 - apps/opencs/model/world/idtable.cpp | 26 ++++++++++++++++++++++---- apps/opencs/model/world/record.hpp | 10 ---------- 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/apps/opencs/model/world/commands.cpp b/apps/opencs/model/world/commands.cpp index ff4bab989..79900c6c4 100644 --- a/apps/opencs/model/world/commands.cpp +++ b/apps/opencs/model/world/commands.cpp @@ -294,19 +294,16 @@ void CSMWorld::CreateCommand::undo() } CSMWorld::RevertCommand::RevertCommand (IdTable& model, const std::string& id, QUndoCommand* parent) -: QUndoCommand (parent), mModel (model), mId (id), mOld (0), mNew (0) +: QUndoCommand (parent), mModel (model), mId (id), mOld (0) { setText (("Revert record " + id).c_str()); - mNew = model.getRecord(id).clone(); - mNew->revert(); mOld = model.getRecord (id).clone(); } CSMWorld::RevertCommand::~RevertCommand() { delete mOld; - delete mNew; } void CSMWorld::RevertCommand::redo() @@ -322,7 +319,6 @@ void CSMWorld::RevertCommand::redo() } else { - mModel.setRecord (mId, *mNew); mModel.setData (index, static_cast (RecordBase::State_BaseOnly)); } } diff --git a/apps/opencs/model/world/commands.hpp b/apps/opencs/model/world/commands.hpp index 46fa6d8a0..58a1b1d1c 100644 --- a/apps/opencs/model/world/commands.hpp +++ b/apps/opencs/model/world/commands.hpp @@ -201,7 +201,6 @@ namespace CSMWorld IdTable& mModel; std::string mId; RecordBase *mOld; - RecordBase *mNew; // not implemented RevertCommand (const RevertCommand&); diff --git a/apps/opencs/model/world/idtable.cpp b/apps/opencs/model/world/idtable.cpp index 7492b6b17..75355116f 100644 --- a/apps/opencs/model/world/idtable.cpp +++ b/apps/opencs/model/world/idtable.cpp @@ -86,12 +86,30 @@ bool CSMWorld::IdTable::setData (const QModelIndex &index, const QVariant &value mIdCollection->setData (index.row(), index.column(), value); emit dataChanged(index, index); - // Modifying a value can also change the Modified status of a record unless . int stateColumn = searchColumnIndex(Columns::ColumnId_Modification); - if (stateColumn != -1 && index.column() != stateColumn) + if (stateColumn != -1) { - QModelIndex stateIndex = this->index(index.row(), stateColumn); - emit dataChanged(stateIndex, stateIndex); + if (index.column() == stateColumn) + { + // modifying the state column can modify other values. we need to tell + // views that the whole row has changed. + + int count = columnCount(index.parent()); + for (int i=0; iindex(index.row(), i); + emit dataChanged(columnIndex, columnIndex); + } + } + + } else + { + // Modifying a value can also change the Modified status of a record unless . + QModelIndex stateIndex = this->index(index.row(), stateColumn); + emit dataChanged(stateIndex, stateIndex); + } } return true; diff --git a/apps/opencs/model/world/record.hpp b/apps/opencs/model/world/record.hpp index db5822c97..3362f9f96 100644 --- a/apps/opencs/model/world/record.hpp +++ b/apps/opencs/model/world/record.hpp @@ -24,8 +24,6 @@ namespace CSMWorld virtual RecordBase *modifiedCopy() const = 0; - virtual void revert() = 0; - virtual void assign (const RecordBase& record) = 0; ///< Will throw an exception if the types don't match. @@ -51,8 +49,6 @@ namespace CSMWorld virtual RecordBase *modifiedCopy() const; - void revert() override; - virtual void assign (const RecordBase& record); const ESXRecordT& get() const; @@ -100,12 +96,6 @@ namespace CSMWorld return new Record (*this); } - template - void Record::revert() - { - mModified = mBase; - } - template void Record::assign (const RecordBase& record) { From b8e53b5b81872f7fce006f3456e62b2c0b5b467d Mon Sep 17 00:00:00 2001 From: Doc West Date: Thu, 5 Jul 2018 18:10:43 +0200 Subject: [PATCH 6/7] Fixed comment --- apps/opencs/model/world/idtable.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/opencs/model/world/idtable.cpp b/apps/opencs/model/world/idtable.cpp index 75355116f..0a2052fd8 100644 --- a/apps/opencs/model/world/idtable.cpp +++ b/apps/opencs/model/world/idtable.cpp @@ -106,7 +106,7 @@ bool CSMWorld::IdTable::setData (const QModelIndex &index, const QVariant &value } else { - // Modifying a value can also change the Modified status of a record unless . + // Modifying a value can also change the Modified status of a record. QModelIndex stateIndex = this->index(index.row(), stateColumn); emit dataChanged(stateIndex, stateIndex); } From 78a579991134f9c8f955484a87c0dea615012a4e Mon Sep 17 00:00:00 2001 From: Alexander Stillich Date: Mon, 9 Jul 2018 16:05:06 +0200 Subject: [PATCH 7/7] Issue a single dataChanged() when the modified column changes --- apps/opencs/model/world/idtable.cpp | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/apps/opencs/model/world/idtable.cpp b/apps/opencs/model/world/idtable.cpp index 0a2052fd8..cb48fc85f 100644 --- a/apps/opencs/model/world/idtable.cpp +++ b/apps/opencs/model/world/idtable.cpp @@ -84,7 +84,6 @@ bool CSMWorld::IdTable::setData (const QModelIndex &index, const QVariant &value if (mIdCollection->getColumn (index.column()).isEditable() && role==Qt::EditRole) { mIdCollection->setData (index.row(), index.column(), value); - emit dataChanged(index, index); int stateColumn = searchColumnIndex(Columns::ColumnId_Modification); if (stateColumn != -1) @@ -94,23 +93,19 @@ bool CSMWorld::IdTable::setData (const QModelIndex &index, const QVariant &value // modifying the state column can modify other values. we need to tell // views that the whole row has changed. - int count = columnCount(index.parent()); - for (int i=0; iindex(index.row(), i); - emit dataChanged(columnIndex, columnIndex); - } - } + emit dataChanged(this->index(index.row(), 0), + this->index(index.row(), columnCount(index.parent()))); } else { + emit dataChanged(index, index); + // Modifying a value can also change the Modified status of a record. QModelIndex stateIndex = this->index(index.row(), stateColumn); emit dataChanged(stateIndex, stateIndex); } - } + } else + emit dataChanged(index, index); return true; }