From aa7b0cadb2f737e65d490f4ad026f5df09a394f0 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sun, 23 Feb 2020 11:43:50 +0100 Subject: undo: add cylinder undo commands by copy & paste Do a simple copy & paste followed by a simple search & replace to generate cylinder undo commands from weight undo commands. Obviously, this is still missing the necessary code to keep the dive-data consistent after cylinder editing. Signed-off-by: Berthold Stoeger --- commands/command_edit.cpp | 180 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 180 insertions(+) (limited to 'commands/command_edit.cpp') diff --git a/commands/command_edit.cpp b/commands/command_edit.cpp index 46c206c98..c06ef1ffa 100644 --- a/commands/command_edit.cpp +++ b/commands/command_edit.cpp @@ -8,6 +8,7 @@ #include "core/subsurface-string.h" #include "core/tag.h" #include "qt-models/weightsysteminfomodel.h" +#include "qt-models/tankinfomodel.h" #ifdef SUBSURFACE_MOBILE #include "qt-models/divelocationmodel.h" #endif @@ -917,6 +918,7 @@ bool EditWeightBase::workToBeDone() return !dives.empty(); } +// ***** Remove Weight ***** RemoveWeight::RemoveWeight(int index, bool currentDiveOnly) : EditWeightBase(index, currentDiveOnly) { @@ -943,6 +945,7 @@ void RemoveWeight::redo() } } +// ***** Edit Weight ***** EditWeight::EditWeight(int index, weightsystem_t wsIn, bool currentDiveOnly) : EditWeightBase(index, currentDiveOnly), new_ws(empty_weightsystem) @@ -999,6 +1002,183 @@ void EditWeight::undo() redo(); } +// ***** Add Cylinder ***** +AddCylinder::AddCylinder(bool currentDiveOnly) : + EditDivesBase(currentDiveOnly), + cyl(empty_cylinder) +{ + if (dives.empty()) + return; + else if (dives.size() == 1) + setText(tr("Add cylinder")); + else + setText(tr("Add cylinder (%n dive(s))", "", dives.size())); + cyl = create_new_cylinder(dives[0]); +} + +AddCylinder::~AddCylinder() +{ + free_cylinder(cyl); +} + +bool AddCylinder::workToBeDone() +{ + return true; +} + +void AddCylinder::undo() +{ + for (dive *d: dives) { + if (d->cylinders.nr <= 0) + continue; + remove_cylinder(d, d->cylinders.nr - 1); + emit diveListNotifier.cylinderRemoved(d, d->cylinders.nr); + } +} + +void AddCylinder::redo() +{ + for (dive *d: dives) { + add_cloned_cylinder(&d->cylinders, cyl); + emit diveListNotifier.cylinderAdded(d, d->cylinders.nr - 1); + } +} + +static int find_cylinder_index(const struct dive *d, const cylinder_t &cyl) +{ + for (int idx = 0; idx < d->cylinders.nr; ++idx) { + if (same_cylinder(d->cylinders.cylinders[idx], cyl)) + return idx; + } + return -1; +} + +EditCylinderBase::EditCylinderBase(int index, bool currentDiveOnly) : + EditDivesBase(currentDiveOnly), + cyl(empty_cylinder) +{ + // Get the old cylinder, bail if index is invalid + if (!current || index < 0 || index >= current->cylinders.nr) { + dives.clear(); + return; + } + cyl = clone_cylinder(current->cylinders.cylinders[index]); + + std::vector divesNew; + divesNew.reserve(dives.size()); + indexes.reserve(dives.size()); + + for (dive *d: dives) { + if (d == current) { + divesNew.push_back(d); + indexes.push_back(index); + continue; + } + int idx = find_cylinder_index(d, cyl); + if (idx >= 0) { + divesNew.push_back(d); + indexes.push_back(idx); + } + } + dives = std::move(divesNew); +} + +EditCylinderBase::~EditCylinderBase() +{ + free_cylinder(cyl); +} + +bool EditCylinderBase::workToBeDone() +{ + return !dives.empty(); +} + +// ***** Remove Cylinder ***** +RemoveCylinder::RemoveCylinder(int index, bool currentDiveOnly) : + EditCylinderBase(index, currentDiveOnly) +{ + if (dives.size() == 1) + setText(tr("Remove cylinder")); + else + setText(tr("Remove cylinder (%n dive(s))", "", dives.size())); +} + +void RemoveCylinder::undo() +{ + for (size_t i = 0; i < dives.size(); ++i) { + add_to_cylinder_table(&dives[i]->cylinders, indexes[i], clone_cylinder(cyl)); + emit diveListNotifier.cylinderAdded(dives[i], indexes[i]); + } +} + +void RemoveCylinder::redo() +{ + for (size_t i = 0; i < dives.size(); ++i) { + remove_cylinder(dives[i], indexes[i]); + emit diveListNotifier.cylinderRemoved(dives[i], indexes[i]); + } +} + +// ***** Edit Cylinder ***** +EditCylinder::EditCylinder(int index, cylinder_t cylIn, bool currentDiveOnly) : + EditCylinderBase(index, currentDiveOnly), + new_cyl(empty_cylinder) +{ + if (dives.empty()) + return; + + if (dives.size() == 1) + setText(tr("Edit cylinder")); + else + setText(tr("Edit cylinder (%n dive(s))", "", dives.size())); + + // Try to untranslate the cylinder type + new_cyl = clone_cylinder(cylIn); + QString vString(new_cyl.type.description); + for (int i = 0; i < MAX_TANK_INFO && tank_info[i].name; ++i) { + if (gettextFromC::tr(tank_info[i].name) == vString) { + free_cylinder(new_cyl); + new_cyl.type.description = copy_string(tank_info[i].name); + break; + } + } + + // If that doesn't change anything, do nothing + if (same_cylinder(cyl, new_cyl)) { + dives.clear(); + return; + } + + TankInfoModel *tim = TankInfoModel::instance(); + QModelIndexList matches = tim->match(tim->index(0, 0), Qt::DisplayRole, gettextFromC::tr(new_cyl.type.description)); + if (!matches.isEmpty()) { + if (new_cyl.type.size.mliter != cyl.type.size.mliter) + tim->setData(tim->index(matches.first().row(), TankInfoModel::ML), new_cyl.type.size.mliter); + if (new_cyl.type.workingpressure.mbar != cyl.type.workingpressure.mbar) + tim->setData(tim->index(matches.first().row(), TankInfoModel::BAR), new_cyl.type.workingpressure.mbar / 1000.0); + } +} + +EditCylinder::~EditCylinder() +{ + free_cylinder(new_cyl); +} + +void EditCylinder::redo() +{ + for (size_t i = 0; i < dives.size(); ++i) { + set_cylinder(dives[i], indexes[i], new_cyl); + emit diveListNotifier.cylinderEdited(dives[i], indexes[i]); + } + std::swap(cyl, new_cyl); +} + +// Undo and redo do the same as just the stored value is exchanged +void EditCylinder::undo() +{ + redo(); +} + #ifdef SUBSURFACE_MOBILE EditDive::EditDive(dive *oldDiveIn, dive *newDiveIn, dive_site *createDs, dive_site *editDs, location_t dsLocationIn) -- cgit v1.2.3-70-g09d2 From 5b7a3165932d9b3fced80fec17de01eb1ad89bf7 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sun, 23 Feb 2020 19:24:06 +0100 Subject: undo: reorder cylinders on remove-cylinder undo/redo The cylinders in the events must be reordered if we remove a cylinder. To avoid duplication of code, move the reordering function into qthelper.cpp, though it might not be ideal there. Signed-off-by: Berthold Stoeger --- commands/command_edit.cpp | 3 +++ core/qthelper.cpp | 33 +++++++++++++++++++++++++++++++++ core/qthelper.h | 2 ++ qt-models/cylindermodel.cpp | 10 +--------- 4 files changed, 39 insertions(+), 9 deletions(-) (limited to 'commands/command_edit.cpp') diff --git a/commands/command_edit.cpp b/commands/command_edit.cpp index c06ef1ffa..3ba7a2823 100644 --- a/commands/command_edit.cpp +++ b/commands/command_edit.cpp @@ -1106,6 +1106,7 @@ RemoveCylinder::RemoveCylinder(int index, bool currentDiveOnly) : void RemoveCylinder::undo() { for (size_t i = 0; i < dives.size(); ++i) { + std::vector mapping = get_cylinder_map_for_add(dives[i]->cylinders.nr, indexes[i]); add_to_cylinder_table(&dives[i]->cylinders, indexes[i], clone_cylinder(cyl)); emit diveListNotifier.cylinderAdded(dives[i], indexes[i]); } @@ -1114,7 +1115,9 @@ void RemoveCylinder::undo() void RemoveCylinder::redo() { for (size_t i = 0; i < dives.size(); ++i) { + std::vector mapping = get_cylinder_map_for_remove(dives[i]->cylinders.nr, indexes[i]); remove_cylinder(dives[i], indexes[i]); + cylinder_renumber(dives[i], &mapping[0]); emit diveListNotifier.cylinderRemoved(dives[i], indexes[i]); } } diff --git a/core/qthelper.cpp b/core/qthelper.cpp index 7b879e7d6..10e62eabe 100644 --- a/core/qthelper.cpp +++ b/core/qthelper.cpp @@ -1661,3 +1661,36 @@ extern "C" char *get_changes_made() else return nullptr; } + +// Generate a cylinder-renumber map for use when the n-th cylinder +// of a dive with count cylinders is removed. It fills an int vector +// with 0..n, -1, n..count-1. Each entry in the vector represents +// the new id of the cylinder, whereby <0 means that this particular +// cylinder does not get any new id. This should probably be moved +// to the C-core, but using std::vector is simply more convenient. +// The function assumes that n < count! +std::vector get_cylinder_map_for_remove(int count, int n) +{ + // 1) Fill mapping[0]..mapping[n-1] with 0..n-1 + // 2) Set mapping[n] to -1 + // 3) Fill mapping[n+1]..mapping[count-1] with n..count-2 + std::vector mapping(count); + std::iota(mapping.begin(), mapping.begin() + n, 0); + mapping[n] = -1; + std::iota(mapping.begin() + n + 1, mapping.end(), n); + return mapping; +} + +// Generate a cylinder-renumber map for use when a cylinder is added +// before the n-th cylinder. It fills an int vector with +// with 0..n-1, n+1..count. Each entry in the vector represents +// the new id of the cylinder. This probably should be moved +// to the C-core, but using std::vector is simply more convenient. +// This function assumes that that n <= count! +std::vector get_cylinder_map_for_add(int count, int n) +{ + std::vector mapping(count); + std::iota(mapping.begin(), mapping.begin() + n, 0); + std::iota(mapping.begin() + n, mapping.end(), n + 1); + return mapping; +} diff --git a/core/qthelper.h b/core/qthelper.h index bb8876383..848763138 100644 --- a/core/qthelper.h +++ b/core/qthelper.h @@ -83,6 +83,8 @@ QLocale getLocale(); QVector> selectedDivesGasUsed(); QString getUserAgent(); QString printGPSCoords(const location_t *loc); +std::vector get_cylinder_map_for_remove(int count, int n); +std::vector get_cylinder_map_for_add(int count, int n); extern QString (*changesCallback)(); void uiNotification(const QString &msg); diff --git a/qt-models/cylindermodel.cpp b/qt-models/cylindermodel.cpp index 4b329eafe..3a3f14605 100644 --- a/qt-models/cylindermodel.cpp +++ b/qt-models/cylindermodel.cpp @@ -513,15 +513,7 @@ void CylindersModel::remove(QModelIndex index) changed = true; endRemoveRows(); - // Create a mapping of cylinder indices: - // 1) Fill mapping[0]..mapping[index-1] with 0..index - // 2) Set mapping[index] to -1 - // 3) Fill mapping[index+1]..mapping[end] with index.. - std::vector mapping(d->cylinders.nr + 1); - std::iota(mapping.begin(), mapping.begin() + index.row(), 0); - mapping[index.row()] = -1; - std::iota(mapping.begin() + index.row() + 1, mapping.end(), index.row()); - + std::vector mapping = get_cylinder_map_for_remove(d->cylinders.nr + 1, index.row()); cylinder_renumber(d, &mapping[0]); if (in_planner()) DivePlannerPointsModel::instance()->cylinderRenumber(&mapping[0]); -- cgit v1.2.3-70-g09d2 From c4bf1ce8916c3549e51b455548f87508f3b5e23b Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Mon, 24 Feb 2020 10:57:36 +0100 Subject: undo: remove only "non-protected" cylinders The undo-code must take care not to remove used cylinders. To do so, extend the constructor of EditCylinderBase, which collects the cylinders and dives to edit, by the "nonProtectedOnly" boolean argument. If true, only those cylinders for wich "is_cylinder_prot" returns false will be added. Signed-off-by: Berthold Stoeger --- commands/command_edit.cpp | 16 +++++++++------- commands/command_edit.h | 2 +- 2 files changed, 10 insertions(+), 8 deletions(-) (limited to 'commands/command_edit.cpp') diff --git a/commands/command_edit.cpp b/commands/command_edit.cpp index 3ba7a2823..fc5200b52 100644 --- a/commands/command_edit.cpp +++ b/commands/command_edit.cpp @@ -1053,7 +1053,7 @@ static int find_cylinder_index(const struct dive *d, const cylinder_t &cyl) return -1; } -EditCylinderBase::EditCylinderBase(int index, bool currentDiveOnly) : +EditCylinderBase::EditCylinderBase(int index, bool currentDiveOnly, bool nonProtectedOnly) : EditDivesBase(currentDiveOnly), cyl(empty_cylinder) { @@ -1070,15 +1070,17 @@ EditCylinderBase::EditCylinderBase(int index, bool currentDiveOnly) : for (dive *d: dives) { if (d == current) { + if (nonProtectedOnly && is_cylinder_prot(d, index)) + continue; divesNew.push_back(d); indexes.push_back(index); continue; } int idx = find_cylinder_index(d, cyl); - if (idx >= 0) { - divesNew.push_back(d); - indexes.push_back(idx); - } + if (idx < 0 || (nonProtectedOnly && is_cylinder_prot(d, idx))) + continue; + divesNew.push_back(d); + indexes.push_back(idx); } dives = std::move(divesNew); } @@ -1095,7 +1097,7 @@ bool EditCylinderBase::workToBeDone() // ***** Remove Cylinder ***** RemoveCylinder::RemoveCylinder(int index, bool currentDiveOnly) : - EditCylinderBase(index, currentDiveOnly) + EditCylinderBase(index, currentDiveOnly, true) { if (dives.size() == 1) setText(tr("Remove cylinder")); @@ -1124,7 +1126,7 @@ void RemoveCylinder::redo() // ***** Edit Cylinder ***** EditCylinder::EditCylinder(int index, cylinder_t cylIn, bool currentDiveOnly) : - EditCylinderBase(index, currentDiveOnly), + EditCylinderBase(index, currentDiveOnly, false), new_cyl(empty_cylinder) { if (dives.empty()) diff --git a/commands/command_edit.h b/commands/command_edit.h index 88f359fba..c0b79d073 100644 --- a/commands/command_edit.h +++ b/commands/command_edit.h @@ -390,7 +390,7 @@ private: class EditCylinderBase : public EditDivesBase { protected: - EditCylinderBase(int index, bool currentDiveOnly); + EditCylinderBase(int index, bool currentDiveOnly, bool nonProtectedOnly); ~EditCylinderBase(); cylinder_t cyl; -- cgit v1.2.3-70-g09d2 From 79d117b5bc801038366e245a601bea20d8a5b71b Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sun, 22 Mar 2020 23:27:47 +0100 Subject: undo: be more flexible about which cylinders to edit Currently we use the same_cylinder() function to determine which cylinders should be edited in a multi-dive edit. Make this more flexible by introducing a flag-set, such that the undo-command can select which cylinders are considered as equal: - same type - same pressure - same gas mix - same size Currently both undo commands use same type, pressure and gas so that the behavior stays unchanged. The future goal is to split the cylinder-edit undo command into different commands so that when, for example, editing the type only the type is considered by not the gas mix. Signed-off-by: Berthold Stoeger --- commands/command_edit.cpp | 46 +++++++++++++++++++++++++++++++++++++++------- commands/command_edit.h | 2 +- 2 files changed, 40 insertions(+), 8 deletions(-) (limited to 'commands/command_edit.cpp') diff --git a/commands/command_edit.cpp b/commands/command_edit.cpp index fc5200b52..8914485c6 100644 --- a/commands/command_edit.cpp +++ b/commands/command_edit.cpp @@ -1044,16 +1044,48 @@ void AddCylinder::redo() } } -static int find_cylinder_index(const struct dive *d, const cylinder_t &cyl) +static bool same_cylinder_type(const cylinder_t &cyl1, const cylinder_t &cyl2) +{ + return same_string(cyl1.type.description, cyl2.type.description) && + cyl1.cylinder_use == cyl2.cylinder_use; +} + +static bool same_cylinder_size(const cylinder_t &cyl1, const cylinder_t &cyl2) +{ + return cyl1.type.size.mliter == cyl2.type.size.mliter && + cyl1.type.workingpressure.mbar == cyl2.type.workingpressure.mbar; +} + +static bool same_cylinder_pressure(const cylinder_t &cyl1, const cylinder_t &cyl2) +{ + return cyl1.start.mbar == cyl2.start.mbar && + cyl1.end.mbar == cyl2.end.mbar; +} + +// Flags for comparing cylinders +static const constexpr int SAME_TYPE = 1 << 0; +static const constexpr int SAME_SIZE = 1 << 1; +static const constexpr int SAME_PRESS = 1 << 2; +static const constexpr int SAME_GAS = 1 << 3; + +static bool same_cylinder_with_flags(const cylinder_t &cyl1, const cylinder_t &cyl2, int sameCylinderFlags) +{ + return (((sameCylinderFlags & SAME_TYPE) == 0 || same_cylinder_type(cyl1, cyl2)) && + ((sameCylinderFlags & SAME_SIZE) == 0 || same_cylinder_size(cyl1, cyl2)) && + ((sameCylinderFlags & SAME_PRESS) == 0 || same_cylinder_pressure(cyl1, cyl2)) && + ((sameCylinderFlags & SAME_GAS) == 0 || same_gasmix(cyl1.gasmix, cyl2.gasmix))); +} + +static int find_cylinder_index(const struct dive *d, const cylinder_t &cyl, int sameCylinderFlags) { for (int idx = 0; idx < d->cylinders.nr; ++idx) { - if (same_cylinder(d->cylinders.cylinders[idx], cyl)) + if (same_cylinder_with_flags(cyl, d->cylinders.cylinders[idx], sameCylinderFlags)) return idx; } return -1; } -EditCylinderBase::EditCylinderBase(int index, bool currentDiveOnly, bool nonProtectedOnly) : +EditCylinderBase::EditCylinderBase(int index, bool currentDiveOnly, bool nonProtectedOnly, int sameCylinderFlags) : EditDivesBase(currentDiveOnly), cyl(empty_cylinder) { @@ -1076,7 +1108,7 @@ EditCylinderBase::EditCylinderBase(int index, bool currentDiveOnly, bool nonProt indexes.push_back(index); continue; } - int idx = find_cylinder_index(d, cyl); + int idx = find_cylinder_index(d, cyl, sameCylinderFlags); if (idx < 0 || (nonProtectedOnly && is_cylinder_prot(d, idx))) continue; divesNew.push_back(d); @@ -1097,7 +1129,7 @@ bool EditCylinderBase::workToBeDone() // ***** Remove Cylinder ***** RemoveCylinder::RemoveCylinder(int index, bool currentDiveOnly) : - EditCylinderBase(index, currentDiveOnly, true) + EditCylinderBase(index, currentDiveOnly, true, SAME_TYPE | SAME_PRESS | SAME_GAS) { if (dives.size() == 1) setText(tr("Remove cylinder")); @@ -1126,7 +1158,7 @@ void RemoveCylinder::redo() // ***** Edit Cylinder ***** EditCylinder::EditCylinder(int index, cylinder_t cylIn, bool currentDiveOnly) : - EditCylinderBase(index, currentDiveOnly, false), + EditCylinderBase(index, currentDiveOnly, false, SAME_TYPE | SAME_PRESS | SAME_GAS), new_cyl(empty_cylinder) { if (dives.empty()) @@ -1149,7 +1181,7 @@ EditCylinder::EditCylinder(int index, cylinder_t cylIn, bool currentDiveOnly) : } // If that doesn't change anything, do nothing - if (same_cylinder(cyl, new_cyl)) { + if (same_cylinder_with_flags(cyl, new_cyl, SAME_TYPE | SAME_PRESS | SAME_GAS)) { dives.clear(); return; } diff --git a/commands/command_edit.h b/commands/command_edit.h index c0b79d073..5cf6e328a 100644 --- a/commands/command_edit.h +++ b/commands/command_edit.h @@ -390,7 +390,7 @@ private: class EditCylinderBase : public EditDivesBase { protected: - EditCylinderBase(int index, bool currentDiveOnly, bool nonProtectedOnly); + EditCylinderBase(int index, bool currentDiveOnly, bool nonProtectedOnly, int sameCylinderFlags); ~EditCylinderBase(); cylinder_t cyl; -- cgit v1.2.3-70-g09d2 From 4e8a838f746d7319b97b56a209651a65655aca7f Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Fri, 27 Mar 2020 09:22:11 +0100 Subject: undo: store all cylinders in EditCylinderBase We stored only one cylinder in EditCylinderBase, which is the base class of RemoveCylinder and EditCylinder. This turns out to be too crude: when removing the "same" cylinder from multiple dives, there are some "hidden variables" such as bestmix_o2 or manually_added, which might actually be different. We don't want to overwrite those on undo of delete. Moreover, the cylinder edit is way too crude at the moment, as it overwrites the whole cylinder even when the user edited only a single field. To enable a more refined edit, we have to store each changed cylinder. Signed-off-by: Berthold Stoeger --- commands/command_edit.cpp | 62 +++++++++++++++++++---------------------------- commands/command_edit.h | 4 +-- 2 files changed, 26 insertions(+), 40 deletions(-) (limited to 'commands/command_edit.cpp') diff --git a/commands/command_edit.cpp b/commands/command_edit.cpp index 8914485c6..d5e2d12d0 100644 --- a/commands/command_edit.cpp +++ b/commands/command_edit.cpp @@ -1086,40 +1086,35 @@ static int find_cylinder_index(const struct dive *d, const cylinder_t &cyl, int } EditCylinderBase::EditCylinderBase(int index, bool currentDiveOnly, bool nonProtectedOnly, int sameCylinderFlags) : - EditDivesBase(currentDiveOnly), - cyl(empty_cylinder) + EditDivesBase(currentDiveOnly) { // Get the old cylinder, bail if index is invalid if (!current || index < 0 || index >= current->cylinders.nr) { dives.clear(); return; } - cyl = clone_cylinder(current->cylinders.cylinders[index]); + const cylinder_t &orig = current->cylinders.cylinders[index]; std::vector divesNew; divesNew.reserve(dives.size()); indexes.reserve(dives.size()); + cyl.reserve(dives.size()); for (dive *d: dives) { - if (d == current) { - if (nonProtectedOnly && is_cylinder_prot(d, index)) - continue; - divesNew.push_back(d); - indexes.push_back(index); - continue; - } - int idx = find_cylinder_index(d, cyl, sameCylinderFlags); + int idx = d == current ? index : find_cylinder_index(d, orig, sameCylinderFlags); if (idx < 0 || (nonProtectedOnly && is_cylinder_prot(d, idx))) continue; divesNew.push_back(d); indexes.push_back(idx); + cyl.push_back(clone_cylinder(d->cylinders.cylinders[idx])); } dives = std::move(divesNew); } EditCylinderBase::~EditCylinderBase() { - free_cylinder(cyl); + for (cylinder_t c: cyl) + free_cylinder(c); } bool EditCylinderBase::workToBeDone() @@ -1141,7 +1136,7 @@ void RemoveCylinder::undo() { for (size_t i = 0; i < dives.size(); ++i) { std::vector mapping = get_cylinder_map_for_add(dives[i]->cylinders.nr, indexes[i]); - add_to_cylinder_table(&dives[i]->cylinders, indexes[i], clone_cylinder(cyl)); + add_to_cylinder_table(&dives[i]->cylinders, indexes[i], clone_cylinder(cyl[i])); emit diveListNotifier.cylinderAdded(dives[i], indexes[i]); } } @@ -1158,8 +1153,7 @@ void RemoveCylinder::redo() // ***** Edit Cylinder ***** EditCylinder::EditCylinder(int index, cylinder_t cylIn, bool currentDiveOnly) : - EditCylinderBase(index, currentDiveOnly, false, SAME_TYPE | SAME_PRESS | SAME_GAS), - new_cyl(empty_cylinder) + EditCylinderBase(index, currentDiveOnly, false, SAME_TYPE | SAME_PRESS | SAME_GAS) { if (dives.empty()) return; @@ -1170,44 +1164,38 @@ EditCylinder::EditCylinder(int index, cylinder_t cylIn, bool currentDiveOnly) : setText(tr("Edit cylinder (%n dive(s))", "", dives.size())); // Try to untranslate the cylinder type - new_cyl = clone_cylinder(cylIn); - QString vString(new_cyl.type.description); + QString description = cylIn.type.description; for (int i = 0; i < MAX_TANK_INFO && tank_info[i].name; ++i) { - if (gettextFromC::tr(tank_info[i].name) == vString) { - free_cylinder(new_cyl); - new_cyl.type.description = copy_string(tank_info[i].name); + if (gettextFromC::tr(tank_info[i].name) == description) { + description = tank_info[i].name; break; } } - // If that doesn't change anything, do nothing - if (same_cylinder_with_flags(cyl, new_cyl, SAME_TYPE | SAME_PRESS | SAME_GAS)) { - dives.clear(); - return; - } - + // Update the tank info model TankInfoModel *tim = TankInfoModel::instance(); - QModelIndexList matches = tim->match(tim->index(0, 0), Qt::DisplayRole, gettextFromC::tr(new_cyl.type.description)); + QModelIndexList matches = tim->match(tim->index(0, 0), Qt::DisplayRole, gettextFromC::tr(cylIn.type.description)); if (!matches.isEmpty()) { - if (new_cyl.type.size.mliter != cyl.type.size.mliter) - tim->setData(tim->index(matches.first().row(), TankInfoModel::ML), new_cyl.type.size.mliter); - if (new_cyl.type.workingpressure.mbar != cyl.type.workingpressure.mbar) - tim->setData(tim->index(matches.first().row(), TankInfoModel::BAR), new_cyl.type.workingpressure.mbar / 1000.0); + if (cylIn.type.size.mliter != cyl[0].type.size.mliter) + tim->setData(tim->index(matches.first().row(), TankInfoModel::ML), cylIn.type.size.mliter); + if (cylIn.type.workingpressure.mbar != cyl[0].type.workingpressure.mbar) + tim->setData(tim->index(matches.first().row(), TankInfoModel::BAR), cylIn.type.workingpressure.mbar / 1000.0); } -} -EditCylinder::~EditCylinder() -{ - free_cylinder(new_cyl); + // The base class copied the cylinders for us, let's edit them + for (int i = 0; i < (int)indexes.size(); ++i) { + free_cylinder(cyl[i]); + cyl[i] = cylIn; + cyl[i].type.description = copy_qstring(description); + } } void EditCylinder::redo() { for (size_t i = 0; i < dives.size(); ++i) { - set_cylinder(dives[i], indexes[i], new_cyl); + std::swap(dives[i]->cylinders.cylinders[indexes[i]], cyl[i]); emit diveListNotifier.cylinderEdited(dives[i], indexes[i]); } - std::swap(cyl, new_cyl); } // Undo and redo do the same as just the stored value is exchanged diff --git a/commands/command_edit.h b/commands/command_edit.h index 5cf6e328a..d7c31f7f2 100644 --- a/commands/command_edit.h +++ b/commands/command_edit.h @@ -393,7 +393,7 @@ protected: EditCylinderBase(int index, bool currentDiveOnly, bool nonProtectedOnly, int sameCylinderFlags); ~EditCylinderBase(); - cylinder_t cyl; + std::vector cyl; std::vector indexes; // An index for each dive in the dives vector. bool workToBeDone() override; }; @@ -409,9 +409,7 @@ private: class EditCylinder : public EditCylinderBase { public: EditCylinder(int index, cylinder_t cyl, bool currentDiveOnly); // Clones cylinder - ~EditCylinder(); private: - cylinder_t new_cyl; void undo() override; void redo() override; }; -- cgit v1.2.3-70-g09d2 From 2eeb5f4fc2d6da8a8c8950f0b479b7cb2055af07 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Fri, 27 Mar 2020 21:09:59 +0100 Subject: undo: more fine-grained editing of cylinder Don't overwrite the full cylinder when editing a single field. Implement three "modes": editing of type, pressure and gasmix. Don't consider individual fields, because some of them are related. E.g. you can change the gasmix by setting the MOD. Signed-off-by: Berthold Stoeger --- commands/command.cpp | 4 ++-- commands/command.h | 7 ++++++- commands/command_edit.cpp | 38 +++++++++++++++++++++++++++++++++----- commands/command_edit.h | 9 ++++++++- core/equipment.c | 9 --------- core/equipment.h | 1 - qt-models/cylindermodel.cpp | 18 +++++++++++++++--- 7 files changed, 64 insertions(+), 22 deletions(-) (limited to 'commands/command_edit.cpp') diff --git a/commands/command.cpp b/commands/command.cpp index c173494c0..33c866e72 100644 --- a/commands/command.cpp +++ b/commands/command.cpp @@ -304,9 +304,9 @@ int removeCylinder(int index, bool currentDiveOnly) return execute_edit(new RemoveCylinder(index, currentDiveOnly)); } -int editCylinder(int index, cylinder_t cyl, bool currentDiveOnly) +int editCylinder(int index, cylinder_t cyl, EditCylinderType type, bool currentDiveOnly) { - return execute_edit(new EditCylinder(index, cyl, currentDiveOnly)); + return execute_edit(new EditCylinder(index, cyl, type, currentDiveOnly)); } // Trip editing related commands diff --git a/commands/command.h b/commands/command.h index 66122ac99..fc1ddf582 100644 --- a/commands/command.h +++ b/commands/command.h @@ -92,7 +92,12 @@ int removeWeight(int index, bool currentDiveOnly); int editWeight(int index, weightsystem_t ws, bool currentDiveOnly); int addCylinder(bool currentDiveOnly); int removeCylinder(int index, bool currentDiveOnly); -int editCylinder(int index, cylinder_t cyl, bool currentDiveOnly); +enum class EditCylinderType { + TYPE, + PRESSURE, + GASMIX +}; +int editCylinder(int index, cylinder_t cyl, EditCylinderType type, bool currentDiveOnly); #ifdef SUBSURFACE_MOBILE // Edits a dive and creates a divesite (if createDs != NULL) or edits a divesite (if changeDs != NULL). // Takes ownership of newDive and createDs! diff --git a/commands/command_edit.cpp b/commands/command_edit.cpp index d5e2d12d0..c81e67394 100644 --- a/commands/command_edit.cpp +++ b/commands/command_edit.cpp @@ -1151,9 +1151,23 @@ void RemoveCylinder::redo() } } +static int editCylinderTypeToFlags(EditCylinderType type) +{ + switch (type) { + default: + case EditCylinderType::TYPE: + return SAME_TYPE | SAME_SIZE; + case EditCylinderType::PRESSURE: + return SAME_PRESS; + case EditCylinderType::GASMIX: + return SAME_GAS; + } +} + // ***** Edit Cylinder ***** -EditCylinder::EditCylinder(int index, cylinder_t cylIn, bool currentDiveOnly) : - EditCylinderBase(index, currentDiveOnly, false, SAME_TYPE | SAME_PRESS | SAME_GAS) +EditCylinder::EditCylinder(int index, cylinder_t cylIn, EditCylinderType typeIn, bool currentDiveOnly) : + EditCylinderBase(index, currentDiveOnly, false, editCylinderTypeToFlags(typeIn)), + type(typeIn) { if (dives.empty()) return; @@ -1184,9 +1198,23 @@ EditCylinder::EditCylinder(int index, cylinder_t cylIn, bool currentDiveOnly) : // The base class copied the cylinders for us, let's edit them for (int i = 0; i < (int)indexes.size(); ++i) { - free_cylinder(cyl[i]); - cyl[i] = cylIn; - cyl[i].type.description = copy_qstring(description); + switch (type) { + case EditCylinderType::TYPE: + free((void *)cyl[i].type.description); + cyl[i].type = cylIn.type; + cyl[i].type.description = copy_qstring(description); + cyl[i].cylinder_use = cylIn.cylinder_use; + break; + case EditCylinderType::PRESSURE: + cyl[i].start.mbar = cylIn.start.mbar; + cyl[i].end.mbar = cylIn.end.mbar; + break; + case EditCylinderType::GASMIX: + cyl[i].gasmix = cylIn.gasmix; + cyl[i].bestmix_o2 = cylIn.bestmix_o2; + cyl[i].bestmix_he = cylIn.bestmix_he; + break; + } } } diff --git a/commands/command_edit.h b/commands/command_edit.h index d7c31f7f2..ab842a13c 100644 --- a/commands/command_edit.h +++ b/commands/command_edit.h @@ -5,6 +5,7 @@ #define COMMAND_EDIT_H #include "command_base.h" +#include "command.h" // for EditCylinderType #include "core/subsurface-qt/divelistnotifier.h" #include @@ -406,10 +407,16 @@ private: void redo() override; }; +// Instead of implementing an undo command for every single field in a cylinder, +// we only have one and pass an edit "type". We either edit the type, pressure +// or gasmix fields. This has mostly historical reasons rooted in the way the +// CylindersModel code works. The model works for undo and also in the planner +// without undo. Having a single undo-command simplifies the code there. class EditCylinder : public EditCylinderBase { public: - EditCylinder(int index, cylinder_t cyl, bool currentDiveOnly); // Clones cylinder + EditCylinder(int index, cylinder_t cyl, EditCylinderType type, bool currentDiveOnly); // Clones cylinder private: + EditCylinderType type; void undo() override; void redo() override; }; diff --git a/core/equipment.c b/core/equipment.c index 27d01692e..4be46ed1e 100644 --- a/core/equipment.c +++ b/core/equipment.c @@ -282,15 +282,6 @@ void remove_cylinder(struct dive *dive, int idx) remove_from_cylinder_table(&dive->cylinders, idx); } -// cyl is cloned. -void set_cylinder(struct dive *dive, int idx, cylinder_t cyl) -{ - if (idx < 0 || idx >= dive->cylinders.nr) - return; - free_cylinder(dive->cylinders.cylinders[idx]); - dive->cylinders.cylinders[idx] = clone_cylinder(cyl); -} - void remove_weightsystem(struct dive *dive, int idx) { remove_from_weightsystem_table(&dive->weightsystems, idx); diff --git a/core/equipment.h b/core/equipment.h index af410467f..081a13835 100644 --- a/core/equipment.h +++ b/core/equipment.h @@ -85,7 +85,6 @@ extern void add_cylinder_description(const cylinder_type_t *); extern void add_weightsystem_description(const weightsystem_t *); extern bool same_weightsystem(weightsystem_t w1, weightsystem_t w2); extern void remove_cylinder(struct dive *dive, int idx); -extern void set_cylinder(struct dive *dive, int idx, cylinder_t ws); extern void remove_weightsystem(struct dive *dive, int idx); extern void set_weightsystem(struct dive *dive, int idx, weightsystem_t ws); extern void reset_cylinders(struct dive *dive, bool track_gas); diff --git a/qt-models/cylindermodel.cpp b/qt-models/cylindermodel.cpp index 8a79ff1e5..c7e1b186d 100644 --- a/qt-models/cylindermodel.cpp +++ b/qt-models/cylindermodel.cpp @@ -296,7 +296,6 @@ cylinder_t *CylindersModel::cylinderAt(const QModelIndex &index) bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, int role) { - if (!d) return false; @@ -324,6 +323,7 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in tempCyl.type.description = strdup(qPrintable(type)); dataChanged(index, index); } + return true; } case SIZE: if (tempCyl.type.size.mliter != value.toInt()) { @@ -362,10 +362,12 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in if (index.column() != TYPE && !changed) return false; + Command::EditCylinderType type = Command::EditCylinderType::TYPE; switch (index.column()) { case TYPE: newType = qPrintable(vString); cyl.type.description = newType.c_str(); + type = Command::EditCylinderType::TYPE; break; case SIZE: { TankInfoModel *tanks = TankInfoModel::instance(); @@ -375,6 +377,7 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in if (!matches.isEmpty()) tanks->setData(tanks->index(matches.first().row(), TankInfoModel::ML), cyl.type.size.mliter); } + type = Command::EditCylinderType::TYPE; break; case WORKINGPRESS: { TankInfoModel *tanks = TankInfoModel::instance(); @@ -383,13 +386,16 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in if (!matches.isEmpty()) tanks->setData(tanks->index(matches.first().row(), TankInfoModel::BAR), cyl.type.workingpressure.mbar / 1000.0); } + type = Command::EditCylinderType::TYPE; break; case START: cyl.start = string_to_pressure(qPrintable(vString)); + type = Command::EditCylinderType::PRESSURE; break; case END: //if (!cyl->start.mbar || string_to_pressure(qPrintable(vString)).mbar <= cyl->start.mbar) { cyl.end = string_to_pressure(qPrintable(vString)); + type = Command::EditCylinderType::PRESSURE; break; case O2: { cyl.gasmix.o2 = string_to_fraction(qPrintable(vString)); @@ -405,6 +411,7 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in cyl.depth = gas_mod(cyl.gasmix, modpO2, d, M_OR_FT(3, 10)); cyl.bestmix_o2 = false; } + type = Command::EditCylinderType::GASMIX; break; case HE: cyl.gasmix.he = string_to_fraction(qPrintable(vString)); @@ -412,9 +419,11 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in if (get_o2(cyl.gasmix) + get_he(cyl.gasmix) > 1000) cyl.gasmix.o2.permille = 1000 - get_he(cyl.gasmix); cyl.bestmix_he = false; + type = Command::EditCylinderType::GASMIX; break; case DEPTH: cyl.depth = string_to_depth(qPrintable(vString)); + type = Command::EditCylinderType::GASMIX; break; case MOD: { if (QString::compare(qPrintable(vString), "*") == 0) { @@ -430,6 +439,7 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in modpO2.mbar = prefs.decopo2; cyl.depth = gas_mod(cyl.gasmix, modpO2, d, M_OR_FT(3, 10)); } + type = Command::EditCylinderType::GASMIX; break; case MND: if (QString::compare(qPrintable(vString), "*") == 0) { @@ -441,6 +451,7 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in // Calculate fHe for input depth cyl.gasmix.he = best_he(string_to_depth(qPrintable(vString)), d, prefs.o2narcotic, cyl.gasmix.o2); } + type = Command::EditCylinderType::GASMIX; break; case USE: { int use = vString.toInt(); @@ -448,6 +459,7 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in use = 0; cyl.cylinder_use = (enum cylinderuse)use; } + type = Command::EditCylinderType::TYPE; break; } @@ -461,7 +473,7 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in } else { #ifndef SUBSURFACE_MOBILE // On the EquipmentTab - place an editCylinder command. - Command::editCylinder(index.row(), cyl, false); + Command::editCylinder(index.row(), cyl, type, false); #endif } return true; @@ -709,7 +721,7 @@ void CylindersModel::commitTempCyl(int row) if (inPlanner) std::swap(*cyl, tempCyl); else - Command::editCylinder(tempRow, tempCyl, false); + Command::editCylinder(tempRow, tempCyl, Command::EditCylinderType::TYPE, false); } free_cylinder(tempCyl); tempRow = -1; -- cgit v1.2.3-70-g09d2 From e2f77f9238134c7d45944cb5cf62c2c15f202870 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Fri, 3 Apr 2020 14:25:43 +0200 Subject: undo: call invalidate_dive_cache() when editing cylinders Signed-off-by: Berthold Stoeger --- commands/command_edit.cpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'commands/command_edit.cpp') diff --git a/commands/command_edit.cpp b/commands/command_edit.cpp index c81e67394..bec6cb99c 100644 --- a/commands/command_edit.cpp +++ b/commands/command_edit.cpp @@ -1033,6 +1033,7 @@ void AddCylinder::undo() continue; remove_cylinder(d, d->cylinders.nr - 1); emit diveListNotifier.cylinderRemoved(d, d->cylinders.nr); + invalidate_dive_cache(d); // Ensure that dive is written in git_save() } } @@ -1041,6 +1042,7 @@ void AddCylinder::redo() for (dive *d: dives) { add_cloned_cylinder(&d->cylinders, cyl); emit diveListNotifier.cylinderAdded(d, d->cylinders.nr - 1); + invalidate_dive_cache(d); // Ensure that dive is written in git_save() } } @@ -1138,6 +1140,7 @@ void RemoveCylinder::undo() std::vector mapping = get_cylinder_map_for_add(dives[i]->cylinders.nr, indexes[i]); add_to_cylinder_table(&dives[i]->cylinders, indexes[i], clone_cylinder(cyl[i])); emit diveListNotifier.cylinderAdded(dives[i], indexes[i]); + invalidate_dive_cache(dives[i]); // Ensure that dive is written in git_save() } } @@ -1148,6 +1151,7 @@ void RemoveCylinder::redo() remove_cylinder(dives[i], indexes[i]); cylinder_renumber(dives[i], &mapping[0]); emit diveListNotifier.cylinderRemoved(dives[i], indexes[i]); + invalidate_dive_cache(dives[i]); // Ensure that dive is written in git_save() } } @@ -1223,6 +1227,7 @@ void EditCylinder::redo() for (size_t i = 0; i < dives.size(); ++i) { std::swap(dives[i]->cylinders.cylinders[indexes[i]], cyl[i]); emit diveListNotifier.cylinderEdited(dives[i], indexes[i]); + invalidate_dive_cache(dives[i]); // Ensure that dive is written in git_save() } } -- cgit v1.2.3-70-g09d2