From 6199663bd84c50366ba046545fe4f2de0b093b0a Mon Sep 17 00:00:00 2001 From: Aesylwinn Date: Sat, 21 May 2016 18:59:43 -0400 Subject: [PATCH] Fix data corruption issues. - Point connection count not being set - Nested undo not restoring table (for pathgrid scene editing, editor nested undo is still broken) --- apps/opencs/model/world/commands.cpp | 34 +++++++ apps/opencs/model/world/commands.hpp | 27 ++++++ .../model/world/nestedcoladapterimp.cpp | 96 +++++++++++++++---- apps/opencs/view/render/pathgrid.cpp | 38 +++----- 4 files changed, 155 insertions(+), 40 deletions(-) diff --git a/apps/opencs/model/world/commands.cpp b/apps/opencs/model/world/commands.cpp index 5f9422376..b48d6eba2 100644 --- a/apps/opencs/model/world/commands.cpp +++ b/apps/opencs/model/world/commands.cpp @@ -301,6 +301,40 @@ void CSMWorld::UpdateCellCommand::undo() mModel.setData (mIndex, mOld); } +CSMWorld::ModifyNestedCommand::ModifyNestedCommand (IdTree& model, const std::string& id, int nestedRow, + int nestedColumn, int parentColumn, const QVariant& new_, QUndoCommand* parent) + : QUndoCommand(parent) + , NestedTableStoring(model, id, parentColumn) + , mModel(model) + , mId(id) + , mNestedRow(nestedRow) + , mNestedColumn(nestedColumn) + , mParentColumn(parentColumn) + , mNew(new_) +{ + std::string title = model.headerData(parentColumn, Qt::Horizontal, Qt::DisplayRole).toString().toUtf8().constData(); + setText (("Modify " + title + " sub-table of " + mId).c_str()); + + QModelIndex parentIndex = mModel.getModelIndex(mId, mParentColumn); + mModifyParentCommand = new ModifyCommand(mModel, parentIndex, parentIndex.data(Qt::EditRole), this); +} + +void CSMWorld::ModifyNestedCommand::redo() +{ + QModelIndex parentIndex = mModel.getModelIndex(mId, mParentColumn); + QModelIndex nestedIndex = mModel.index(mNestedRow, mNestedColumn, parentIndex); + mModel.setData(nestedIndex, mNew); + mModifyParentCommand->redo(); +} + + +void CSMWorld::ModifyNestedCommand::undo() +{ + QModelIndex parentIndex = mModel.getModelIndex(mId, mParentColumn); + mModel.setNestedTable(parentIndex, getOld()); + mModifyParentCommand->undo(); +} + CSMWorld::DeleteNestedCommand::DeleteNestedCommand (IdTree& model, const std::string& id, diff --git a/apps/opencs/model/world/commands.hpp b/apps/opencs/model/world/commands.hpp index b54a1d5ac..b51720df8 100644 --- a/apps/opencs/model/world/commands.hpp +++ b/apps/opencs/model/world/commands.hpp @@ -199,6 +199,33 @@ namespace CSMWorld const NestedTableWrapperBase& getOld() const; }; + class ModifyNestedCommand : public QUndoCommand, private NestedTableStoring + { + IdTree& mModel; + + std::string mId; + + int mNestedRow; + + int mNestedColumn; + + int mParentColumn; + + QVariant mNew; + + // The command to redo/undo the Modified status of a record + ModifyCommand *mModifyParentCommand; + + public: + + ModifyNestedCommand (IdTree& model, const std::string& id, int nestedRow, int nestedColumn, + int parentColumn, const QVariant& new_, QUndoCommand* parent = 0); + + virtual void redo(); + + virtual void undo(); + }; + class DeleteNestedCommand : public QUndoCommand, private NestedTableStoring { IdTree& mModel; diff --git a/apps/opencs/model/world/nestedcoladapterimp.cpp b/apps/opencs/model/world/nestedcoladapterimp.cpp index 3189f76ed..814406a7c 100644 --- a/apps/opencs/model/world/nestedcoladapterimp.cpp +++ b/apps/opencs/model/world/nestedcoladapterimp.cpp @@ -33,10 +33,10 @@ namespace CSMWorld std::vector::iterator iter = pathgrid.mEdges.begin(); for (;iter != pathgrid.mEdges.end(); ++iter) { - if ((*iter).mV0 >= position) - (*iter).mV0++; - if ((*iter).mV1 >= position) - (*iter).mV1++; + if (iter->mV0 >= position) + ++iter->mV0; + if (iter->mV1 >= position) + ++iter->mV1; } points.insert(points.begin()+position, point); @@ -61,15 +61,20 @@ namespace CSMWorld std::vector::iterator iter = pathgrid.mEdges.begin(); for (; iter != pathgrid.mEdges.end();) { - if (((*iter).mV0 == rowToRemove) || ((*iter).mV1 == rowToRemove)) + if (iter->mV0 == rowToRemove || iter->mV1 == rowToRemove) + { + if (static_cast(iter->mV0) < points.size()) + --points[iter->mV0].mConnectionNum; + iter = pathgrid.mEdges.erase(iter); + } else { - if ((*iter).mV0 > rowToRemove) - (*iter).mV0--; + if (iter->mV0 > rowToRemove) + --iter->mV0; - if ((*iter).mV1 > rowToRemove) - (*iter).mV1--; + if (iter->mV1 > rowToRemove) + --iter->mV1; ++iter; } @@ -152,8 +157,13 @@ namespace CSMWorld { Pathgrid pathgrid = record.get(); + ESM::Pathgrid::PointList& points = pathgrid.mPoints; ESM::Pathgrid::EdgeList& edges = pathgrid.mEdges; + // Can only add valid point indices + if (points.empty()) + return; + // blank row ESM::Pathgrid::Edge edge; edge.mV0 = 0; @@ -164,7 +174,12 @@ namespace CSMWorld // // Currently the code assumes that the end user to know what he/she is doing. // e.g. Edges come in pairs, from points a->b and b->a - edges.insert(edges.begin()+position, edge); + + // Even edges between the same node and itself need to be counted + ++points[edge.mV0].mConnectionNum; + + // Edge needs to be ordered, since edge.mV0 is 0, add at start + edges.insert(edges.begin(), edge); record.setModified (pathgrid); } @@ -173,11 +188,16 @@ namespace CSMWorld { Pathgrid pathgrid = record.get(); + ESM::Pathgrid::PointList& points = pathgrid.mPoints; ESM::Pathgrid::EdgeList& edges = pathgrid.mEdges; if (rowToRemove < 0 || rowToRemove >= static_cast (edges.size())) throw std::runtime_error ("index out of range"); + ESM::Pathgrid::Edge& edge = edges[rowToRemove]; + if (static_cast(edge.mV0) < points.size()) + --pathgrid.mPoints[edge.mV0].mConnectionNum; + edges.erase(edges.begin()+rowToRemove); record.setModified (pathgrid); @@ -188,8 +208,13 @@ namespace CSMWorld { Pathgrid pathgrid = record.get(); + // Set points because point data (edge count) is tied to edges + pathgrid.mPoints = + static_cast(nestedTable).mRecord.mPoints; + pathgrid.mData.mS2 = + static_cast(nestedTable).mRecord.mData.mS2; pathgrid.mEdges = - static_cast &>(nestedTable).mNestedTable; + static_cast(nestedTable).mRecord.mEdges; record.setModified (pathgrid); } @@ -197,7 +222,7 @@ namespace CSMWorld NestedTableWrapperBase* PathgridEdgeListAdapter::table(const Record& record) const { // deleted by dtor of NestedTableStoring - return new NestedTableWrapper(record.get().mEdges); + return new PathgridPointsWrap(record.get()); } QVariant PathgridEdgeListAdapter::getData(const Record& record, @@ -224,20 +249,57 @@ namespace CSMWorld { Pathgrid pathgrid = record.get(); - if (subRowIndex < 0 || subRowIndex >= static_cast (pathgrid.mEdges.size())) + ESM::Pathgrid::PointList& points = pathgrid.mPoints; + ESM::Pathgrid::EdgeList& edges = pathgrid.mEdges; + + if (subRowIndex < 0 || subRowIndex >= static_cast (edges.size())) throw std::runtime_error ("index out of range"); + // Point indices must exist + if (value.toInt() < 0 || value.toUInt() >= points.size()) + return; + ESM::Pathgrid::Edge edge = pathgrid.mEdges[subRowIndex]; switch (subColIndex) { case 0: return; // return without saving - case 1: edge.mV0 = value.toInt(); break; - case 2: edge.mV1 = value.toInt(); break; + case 1: + { + edges.erase(edges.begin()+subRowIndex); + + if (static_cast(edge.mV0) < points.size()) + --points[edge.mV0].mConnectionNum; + + edge.mV0 = value.toInt(); + + // Place in correct order + if (static_cast(edge.mV0) < points.size()) + ++points[edge.mV0].mConnectionNum; + + ESM::Pathgrid::EdgeList::iterator it = edges.begin(); + for (; it != edges.end(); ++it) + { + if (edge.mV0 <= it->mV0) + { + edges.insert(it, edge); + break; + } + } + + if (it == edges.end()) + edges.push_back(edge); + + break; + } + case 2: + + edge.mV1 = value.toInt(); + edges[subRowIndex] = edge; + break; + default: throw std::runtime_error("Pathgrid edge subcolumn index out of range"); } - pathgrid.mEdges[subRowIndex] = edge; - record.setModified (pathgrid); } diff --git a/apps/opencs/view/render/pathgrid.cpp b/apps/opencs/view/render/pathgrid.cpp index 368fde5ee..939558be9 100644 --- a/apps/opencs/view/render/pathgrid.cpp +++ b/apps/opencs/view/render/pathgrid.cpp @@ -254,7 +254,6 @@ namespace CSVRender int posY = clampToCell(static_cast(localCoords.y())); int posZ = clampToCell(static_cast(localCoords.z())); - int recordIndex = mPathgridCollection.getIndex (mId); int parentColumn = mPathgridCollection.findColumnIndex(CSMWorld::Columns::ColumnId_PathgridPoints); int posXColumn = mPathgridCollection.searchNestedColumnIndex(parentColumn, @@ -266,14 +265,13 @@ namespace CSVRender int posZColumn = mPathgridCollection.searchNestedColumnIndex(parentColumn, CSMWorld::Columns::ColumnId_PathgridPosZ); - QModelIndex parent = model->index(recordIndex, parentColumn); int row = static_cast(source->mPoints.size()); // Add node commands.push(new CSMWorld::AddNestedCommand(*model, mId, row, parentColumn)); - commands.push(new CSMWorld::ModifyCommand(*model, model->index(row, posXColumn, parent), posX)); - commands.push(new CSMWorld::ModifyCommand(*model, model->index(row, posYColumn, parent), posY)); - commands.push(new CSMWorld::ModifyCommand(*model, model->index(row, posZColumn, parent), posZ)); + commands.push(new CSMWorld::ModifyNestedCommand(*model, mId, row, posXColumn, parentColumn, posX)); + commands.push(new CSMWorld::ModifyNestedCommand(*model, mId, row, posYColumn, parentColumn, posY)); + commands.push(new CSMWorld::ModifyNestedCommand(*model, mId, row, posZColumn, parentColumn, posZ)); } else { @@ -293,9 +291,9 @@ namespace CSVRender int offsetY = static_cast(localCoords.y()); int offsetZ = static_cast(localCoords.z()); - QAbstractItemModel* model = mData.getTableModel(CSMWorld::UniversalId::Type_Pathgrids); + CSMWorld::IdTree* model = dynamic_cast(mData.getTableModel( + CSMWorld::UniversalId::Type_Pathgrids)); - int recordIndex = mPathgridCollection.getIndex(mId); int parentColumn = mPathgridCollection.findColumnIndex(CSMWorld::Columns::ColumnId_PathgridPoints); int posXColumn = mPathgridCollection.searchNestedColumnIndex(parentColumn, @@ -307,20 +305,18 @@ namespace CSVRender int posZColumn = mPathgridCollection.searchNestedColumnIndex(parentColumn, CSMWorld::Columns::ColumnId_PathgridPosZ); - QModelIndex parent = model->index(recordIndex, parentColumn); - for (size_t i = 0; i < mSelected.size(); ++i) { const CSMWorld::Pathgrid::Point& point = source->mPoints[mSelected[i]]; int row = static_cast(mSelected[i]); - commands.push(new CSMWorld::ModifyCommand(*model, model->index(row, posXColumn, parent), + commands.push(new CSMWorld::ModifyNestedCommand(*model, mId, row, posXColumn, parentColumn, clampToCell(point.mX + offsetX))); - commands.push(new CSMWorld::ModifyCommand(*model, model->index(row, posYColumn, parent), + commands.push(new CSMWorld::ModifyNestedCommand(*model, mId, row, posYColumn, parentColumn, clampToCell(point.mY + offsetY))); - commands.push(new CSMWorld::ModifyCommand(*model, model->index(row, posZColumn, parent), + commands.push(new CSMWorld::ModifyNestedCommand(*model, mId, row, posZColumn, parentColumn, clampToCell(point.mZ + offsetZ))); } } @@ -564,7 +560,6 @@ namespace CSVRender CSMWorld::IdTree* model = dynamic_cast(mData.getTableModel( CSMWorld::UniversalId::Type_Pathgrids)); - int recordIndex = mPathgridCollection.getIndex(mId); int parentColumn = mPathgridCollection.findColumnIndex(CSMWorld::Columns::ColumnId_PathgridEdges); int edge0Column = mPathgridCollection.searchNestedColumnIndex(parentColumn, @@ -573,22 +568,19 @@ namespace CSVRender int edge1Column = mPathgridCollection.searchNestedColumnIndex(parentColumn, CSMWorld::Columns::ColumnId_PathgridEdge1); - QModelIndex parent = model->index(recordIndex, parentColumn); - int row = static_cast(source.mEdges.size()); - if (edgeExists(source, node1, node2) == -1) { - commands.push(new CSMWorld::AddNestedCommand(*model, mId, row, parentColumn)); - commands.push(new CSMWorld::ModifyCommand(*model, model->index(row, edge0Column, parent), node1)); - commands.push(new CSMWorld::ModifyCommand(*model, model->index(row, edge1Column, parent), node2)); - ++row; + // Set first edge last since that reorders the edge list + commands.push(new CSMWorld::AddNestedCommand(*model, mId, 0, parentColumn)); + commands.push(new CSMWorld::ModifyNestedCommand(*model, mId, 0, edge1Column, parentColumn, node2)); + commands.push(new CSMWorld::ModifyNestedCommand(*model, mId, 0, edge0Column, parentColumn, node1)); } if (edgeExists(source, node2, node1) == -1) { - commands.push(new CSMWorld::AddNestedCommand(*model, mId, row, parentColumn)); - commands.push(new CSMWorld::ModifyCommand(*model, model->index(row, edge0Column, parent), node2)); - commands.push(new CSMWorld::ModifyCommand(*model, model->index(row, edge1Column, parent), node1)); + commands.push(new CSMWorld::AddNestedCommand(*model, mId, 0, parentColumn)); + commands.push(new CSMWorld::ModifyNestedCommand(*model, mId, 0, edge1Column, parentColumn, node1)); + commands.push(new CSMWorld::ModifyNestedCommand(*model, mId, 0, edge0Column, parentColumn, node2)); } }