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.cpp | 15 ++++ commands/command.h | 3 + commands/command_edit.cpp | 180 ++++++++++++++++++++++++++++++++++++++++++++++ commands/command_edit.h | 40 ++++++++++- 4 files changed, 237 insertions(+), 1 deletion(-) (limited to 'commands') diff --git a/commands/command.cpp b/commands/command.cpp index 1fb968778..5d71e6725 100644 --- a/commands/command.cpp +++ b/commands/command.cpp @@ -293,6 +293,21 @@ int editWeight(int index, weightsystem_t ws, bool currentDiveOnly) return execute_edit(new EditWeight(index, ws, currentDiveOnly)); } +int addCylinder(bool currentDiveOnly) +{ + return execute_edit(new AddCylinder(currentDiveOnly)); +} + +int removeCylinder(int index, bool currentDiveOnly) +{ + return execute_edit(new RemoveCylinder(index, currentDiveOnly)); +} + +int editCylinder(int index, cylinder_t cyl, bool currentDiveOnly) +{ + return execute_edit(new EditCylinder(index, cyl, currentDiveOnly)); +} + // Trip editing related commands void editTripLocation(dive_trip *trip, const QString &s) { diff --git a/commands/command.h b/commands/command.h index e19d093cb..b8024c03d 100644 --- a/commands/command.h +++ b/commands/command.h @@ -90,6 +90,9 @@ void editProfile(dive *d); // dive computer(s) and cylinder(s) will be reset! int addWeight(bool currentDiveOnly); 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); #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 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) diff --git a/commands/command_edit.h b/commands/command_edit.h index e99ce2407..88f359fba 100644 --- a/commands/command_edit.h +++ b/commands/command_edit.h @@ -377,6 +377,45 @@ private: void redo() override; }; +class AddCylinder : public EditDivesBase { +public: + AddCylinder(bool currentDiveOnly); + ~AddCylinder(); +private: + cylinder_t cyl; + void undo() override; + void redo() override; + bool workToBeDone() override; +}; + +class EditCylinderBase : public EditDivesBase { +protected: + EditCylinderBase(int index, bool currentDiveOnly); + ~EditCylinderBase(); + + cylinder_t cyl; + std::vector indexes; // An index for each dive in the dives vector. + bool workToBeDone() override; +}; + +class RemoveCylinder : public EditCylinderBase { +public: + RemoveCylinder(int index, bool currentDiveOnly); +private: + void undo() override; + void redo() override; +}; + +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; +}; + #ifdef SUBSURFACE_MOBILE // Edit a full dive. This is used on mobile where we don't have per-field granularity. // It may add or edit a dive site. @@ -406,5 +445,4 @@ private: #endif // SUBSURFACE_MOBILE } // namespace Command - #endif -- 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') 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') 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 30c7499a3c656784cc45825b4b1b2fca61081308 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Tue, 3 Mar 2020 22:34:50 +0100 Subject: undo: implement addBookmark undo command Create a new translation unit for event-related undo commands. Create a base class of commands that add events and one subclass that adds a bookmark event. Use this command in the profile-widget. Signed-off-by: Berthold Stoeger --- commands/CMakeLists.txt | 2 ++ commands/command.cpp | 8 +++++++ commands/command.h | 4 ++++ commands/command_base.h | 9 +++++--- commands/command_event.cpp | 43 ++++++++++++++++++++++++++++++++++++++ commands/command_event.h | 44 +++++++++++++++++++++++++++++++++++++++ profile-widget/profilewidget2.cpp | 5 +---- 7 files changed, 108 insertions(+), 7 deletions(-) create mode 100644 commands/command_event.cpp create mode 100644 commands/command_event.h (limited to 'commands') diff --git a/commands/CMakeLists.txt b/commands/CMakeLists.txt index 4b0c37dc6..ceb2740f0 100644 --- a/commands/CMakeLists.txt +++ b/commands/CMakeLists.txt @@ -14,6 +14,8 @@ set(SUBSURFACE_GENERIC_COMMANDS_SRCS command_edit.h command_edit_trip.cpp command_edit_trip.h + command_event.cpp + command_event.h ) add_library(subsurface_commands STATIC ${SUBSURFACE_GENERIC_COMMANDS_SRCS}) target_link_libraries(subsurface_commands ${QT_LIBRARIES}) diff --git a/commands/command.cpp b/commands/command.cpp index 5d71e6725..e9b4e0738 100644 --- a/commands/command.cpp +++ b/commands/command.cpp @@ -5,6 +5,7 @@ #include "command_divesite.h" #include "command_edit.h" #include "command_edit_trip.h" +#include "command_event.h" namespace Command { @@ -326,4 +327,11 @@ void editDive(dive *oldDive, dive *newDive, dive_site *createDs, dive_site *chan } #endif // SUBSURFACE_MOBILE +// Event commands + +void addEventBookmark(struct dive *d, int dcNr, int seconds) +{ + execute(new AddEventBookmark(d, dcNr, seconds)); +} + } // namespace Command diff --git a/commands/command.h b/commands/command.h index b8024c03d..88614b1e9 100644 --- a/commands/command.h +++ b/commands/command.h @@ -104,6 +104,10 @@ void editDive(dive *oldDive, dive *newDive, dive_site *createDs, dive_site *chan void editTripLocation(dive_trip *trip, const QString &s); void editTripNotes(dive_trip *trip, const QString &s); +// 6) Event commands + +void addEventBookmark(struct dive *d, int dcNr, int seconds); + } // namespace Command #endif // COMMAND_H diff --git a/commands/command_base.h b/commands/command_base.h index ffc6c4e91..be725d6a5 100644 --- a/commands/command_base.h +++ b/commands/command_base.h @@ -141,7 +141,7 @@ // We put everything in a namespace, so that we can shorten names without polluting the global namespace namespace Command { -// Classes used to automatically call free_dive()/free_trip for owning pointers that go out of scope. +// Classes used to automatically call the appropriate free_*() function for owning pointers that go out of scope. struct DiveDeleter { void operator()(dive *d) { free_dive(d); } }; @@ -151,11 +151,15 @@ struct TripDeleter { struct DiveSiteDeleter { void operator()(dive_site *ds) { free_dive_site(ds); } }; +struct EventDeleter { + void operator()(event *ev) { free(ev); } +}; -// Owning pointers to dive and dive_trip objects. +// Owning pointers to dive, dive_trip, dive_site and event objects. typedef std::unique_ptr OwningDivePtr; typedef std::unique_ptr OwningTripPtr; typedef std::unique_ptr OwningDiveSitePtr; +typedef std::unique_ptr OwningEventPtr; // This is the base class of all commands. // It defines the Qt-translation functions @@ -182,4 +186,3 @@ QString getListOfDives(QVector dives); } // namespace Command #endif // COMMAND_BASE_H - diff --git a/commands/command_event.cpp b/commands/command_event.cpp new file mode 100644 index 000000000..ea35402e4 --- /dev/null +++ b/commands/command_event.cpp @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include "command_event.h" +#include "core/dive.h" +#include "core/subsurface-qt/divelistnotifier.h" +#include "core/libdivecomputer.h" + +namespace Command { + +AddEventBase::AddEventBase(struct dive *dIn, int dcNrIn, struct event *ev) : + d(dIn), dcNr(dcNrIn), eventToAdd(ev) +{ +} + +bool AddEventBase::workToBeDone() +{ + return true; +} + +void AddEventBase::redo() +{ + struct divecomputer *dc = get_dive_dc(d, dcNr); + eventToRemove = eventToAdd.get(); + add_event_to_dc(dc, eventToAdd.release()); // return ownership to backend + invalidate_dive_cache(d); +} + +void AddEventBase::undo() +{ + struct divecomputer *dc = get_dive_dc(d, dcNr); + remove_event_from_dc(dc, eventToRemove); + eventToAdd.reset(eventToRemove); // take ownership of event + eventToRemove = nullptr; + invalidate_dive_cache(d); +} + +AddEventBookmark::AddEventBookmark(struct dive *d, int dcNr, int seconds) : + AddEventBase(d, dcNr, create_event(seconds, SAMPLE_EVENT_BOOKMARK, 0, 0, "bookmark")) +{ + setText(tr("Add bookmark")); +} + +} // namespace Command diff --git a/commands/command_event.h b/commands/command_event.h new file mode 100644 index 000000000..6e387d543 --- /dev/null +++ b/commands/command_event.h @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: GPL-2.0 +// Note: this header file is used by the undo-machinery and should not be included elsewhere. + +#ifndef COMMAND_EVENT_H +#define COMMAND_EVENT_H + +#include "command_base.h" + + +// We put everything in a namespace, so that we can shorten names without polluting the global namespace +namespace Command { + +// Events are a strange thing: they contain there own description which means +// that on changing the description a new object must be allocated. Moreover, +// it means that these objects can't be collected in a table. +// Therefore, the undo commands work on events as they do with dives: using +// owning pointers. See comments in command_base.h + +class AddEventBase : public Base { +public: + AddEventBase(struct dive *d, int dcNr, struct event *ev); // Takes ownership of event! +private: + bool workToBeDone() override; + void undo() override; + void redo() override; + + // Note: we store dive and the divecomputer-number instead of a pointer to the divecomputer. + // Since one divecomputer is integrated into the dive structure, pointers to divecomputers + // are probably not stable. + struct dive *d; + int dcNr; + + OwningEventPtr eventToAdd; // for redo + event *eventToRemove; // for undo +}; + +class AddEventBookmark : public AddEventBase { +public: + AddEventBookmark(struct dive *d, int dcNr, int seconds); +}; + +} // namespace Command + +#endif // COMMAND_EVENT_H diff --git a/profile-widget/profilewidget2.cpp b/profile-widget/profilewidget2.cpp index 566c97732..67962b3bb 100644 --- a/profile-widget/profilewidget2.cpp +++ b/profile-widget/profilewidget2.cpp @@ -1600,10 +1600,7 @@ void ProfileWidget2::removeEvent(DiveEventItem *item) void ProfileWidget2::addBookmark(int seconds) { - add_event(current_dc, seconds, SAMPLE_EVENT_BOOKMARK, 0, 0, "bookmark"); - invalidate_dive_cache(current_dive); - mark_divelist_changed(true); - replot(); + Command::addEventBookmark(current_dive, dc_number, seconds); } void ProfileWidget2::addDivemodeSwitch(int seconds, int divemode) -- cgit v1.2.3-70-g09d2 From 7018783f64f664af97032da4ba4d357c05b52aaf Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Tue, 3 Mar 2020 22:42:51 +0100 Subject: undo: replot profile if event changed Add a DiveListNotifer::eventsChanged signal, which is emitted when the events changed. This is very coarse, at it doesn't differentiate between signal addition / editing / deletion. We might want to be finer in the future. Catch the signal in the profile-widget to replot the dive if this is the currently displayed dive. Reuse the cylindersChanged() slot, but rename it to the now more appropriate profileChanged(). Signed-off-by: Berthold Stoeger --- commands/command_event.cpp | 2 ++ core/subsurface-qt/divelistnotifier.h | 3 +++ profile-widget/profilewidget2.cpp | 5 +++-- profile-widget/profilewidget2.h | 2 +- 4 files changed, 9 insertions(+), 3 deletions(-) (limited to 'commands') diff --git a/commands/command_event.cpp b/commands/command_event.cpp index ea35402e4..d945fbb02 100644 --- a/commands/command_event.cpp +++ b/commands/command_event.cpp @@ -23,6 +23,7 @@ void AddEventBase::redo() eventToRemove = eventToAdd.get(); add_event_to_dc(dc, eventToAdd.release()); // return ownership to backend invalidate_dive_cache(d); + emit diveListNotifier.eventsChanged(d); } void AddEventBase::undo() @@ -32,6 +33,7 @@ void AddEventBase::undo() eventToAdd.reset(eventToRemove); // take ownership of event eventToRemove = nullptr; invalidate_dive_cache(d); + emit diveListNotifier.eventsChanged(d); } AddEventBookmark::AddEventBookmark(struct dive *d, int dcNr, int seconds) : diff --git a/core/subsurface-qt/divelistnotifier.h b/core/subsurface-qt/divelistnotifier.h index 2a72cdf49..63355fbcc 100644 --- a/core/subsurface-qt/divelistnotifier.h +++ b/core/subsurface-qt/divelistnotifier.h @@ -113,6 +113,9 @@ signals: void numShownChanged(); void filterReset(); + // Event-related signals. Currently, we're very blunt: only one signal for any changes to the events. + void eventsChanged(dive *d); + // This signal is emited every time a command is executed. // This is used to hide an old multi-dives-edited warning message. // This is necessary, so that the user can't click on the "undo" button and undo diff --git a/profile-widget/profilewidget2.cpp b/profile-widget/profilewidget2.cpp index 67962b3bb..54881f71a 100644 --- a/profile-widget/profilewidget2.cpp +++ b/profile-widget/profilewidget2.cpp @@ -170,7 +170,8 @@ ProfileWidget2::ProfileWidget2(QWidget *parent) : QGraphicsView(parent), connect(DivePictureModel::instance(), &DivePictureModel::rowsInserted, this, &ProfileWidget2::plotPictures); connect(DivePictureModel::instance(), &DivePictureModel::picturesRemoved, this, &ProfileWidget2::removePictures); connect(DivePictureModel::instance(), &DivePictureModel::modelReset, this, &ProfileWidget2::plotPictures); - connect(&diveListNotifier, &DiveListNotifier::cylinderEdited, this, &ProfileWidget2::cylinderChanged); + connect(&diveListNotifier, &DiveListNotifier::cylinderEdited, this, &ProfileWidget2::profileChanged); + connect(&diveListNotifier, &DiveListNotifier::eventsChanged, this, &ProfileWidget2::profileChanged); #endif // SUBSURFACE_MOBILE #if !defined(QT_NO_DEBUG) && defined(SHOW_PLOT_INFO_TABLE) @@ -2189,7 +2190,7 @@ void ProfileWidget2::removePictures(const QVector &fileUrls) calculatePictureYPositions(); } -void ProfileWidget2::cylinderChanged(dive *d) +void ProfileWidget2::profileChanged(dive *d) { if (!d || d->id != displayed_dive.id) return; // Cylinders of a differnt dive than the shown one changed. diff --git a/profile-widget/profilewidget2.h b/profile-widget/profilewidget2.h index 0c1db3f62..4f60bf885 100644 --- a/profile-widget/profilewidget2.h +++ b/profile-widget/profilewidget2.h @@ -118,7 +118,7 @@ slots: // Necessary to call from QAction's signals. void pointInserted(const QModelIndex &parent, int start, int end); void pointsRemoved(const QModelIndex &, int start, int end); void updateThumbnail(QString filename, QImage thumbnail, duration_t duration); - void cylinderChanged(dive *d); + void profileChanged(dive *d); /* this is called for every move on the handlers. maybe we can speed up this a bit? */ void recreatePlannedDive(); -- cgit v1.2.3-70-g09d2 From 33fb6461fbe7e389081e77853e63638a1a343623 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Tue, 3 Mar 2020 22:54:54 +0100 Subject: undo: add undo command for dive-mode switch This basically copies the bookmark code, with the addition that the dive mode is recorded in the text of the undo command. Signed-off-by: Berthold Stoeger --- commands/command.cpp | 5 +++++ commands/command.h | 1 + commands/command_event.cpp | 7 +++++++ commands/command_event.h | 5 +++++ profile-widget/profilewidget2.cpp | 5 +---- 5 files changed, 19 insertions(+), 4 deletions(-) (limited to 'commands') diff --git a/commands/command.cpp b/commands/command.cpp index e9b4e0738..4f92038b9 100644 --- a/commands/command.cpp +++ b/commands/command.cpp @@ -334,4 +334,9 @@ void addEventBookmark(struct dive *d, int dcNr, int seconds) execute(new AddEventBookmark(d, dcNr, seconds)); } +void addEventDivemodeSwitch(struct dive *d, int dcNr, int seconds, int divemode) +{ + execute(new AddEventDivemodeSwitch(d, dcNr, seconds, divemode)); +} + } // namespace Command diff --git a/commands/command.h b/commands/command.h index 88614b1e9..1644063a7 100644 --- a/commands/command.h +++ b/commands/command.h @@ -107,6 +107,7 @@ void editTripNotes(dive_trip *trip, const QString &s); // 6) Event commands void addEventBookmark(struct dive *d, int dcNr, int seconds); +void addEventDivemodeSwitch(struct dive *d, int dcNr, int seconds, int divemode); } // namespace Command diff --git a/commands/command_event.cpp b/commands/command_event.cpp index d945fbb02..ddef502bc 100644 --- a/commands/command_event.cpp +++ b/commands/command_event.cpp @@ -4,6 +4,7 @@ #include "core/dive.h" #include "core/subsurface-qt/divelistnotifier.h" #include "core/libdivecomputer.h" +#include "core/gettextfromc.h" namespace Command { @@ -42,4 +43,10 @@ AddEventBookmark::AddEventBookmark(struct dive *d, int dcNr, int seconds) : setText(tr("Add bookmark")); } +AddEventDivemodeSwitch::AddEventDivemodeSwitch(struct dive *d, int dcNr, int seconds, int divemode) : + AddEventBase(d, dcNr, create_event(seconds, SAMPLE_EVENT_BOOKMARK, 0, divemode, QT_TRANSLATE_NOOP("gettextFromC", "modechange"))) +{ + setText(tr("Add dive mode switch to %1").arg(gettextFromC::tr(divemode_text_ui[divemode]))); +} + } // namespace Command diff --git a/commands/command_event.h b/commands/command_event.h index 6e387d543..3130a128f 100644 --- a/commands/command_event.h +++ b/commands/command_event.h @@ -39,6 +39,11 @@ public: AddEventBookmark(struct dive *d, int dcNr, int seconds); }; +class AddEventDivemodeSwitch : public AddEventBase { +public: + AddEventDivemodeSwitch(struct dive *d, int dcNr, int seconds, int divemode); +}; + } // namespace Command #endif // COMMAND_EVENT_H diff --git a/profile-widget/profilewidget2.cpp b/profile-widget/profilewidget2.cpp index 54881f71a..76822c196 100644 --- a/profile-widget/profilewidget2.cpp +++ b/profile-widget/profilewidget2.cpp @@ -1606,10 +1606,7 @@ void ProfileWidget2::addBookmark(int seconds) void ProfileWidget2::addDivemodeSwitch(int seconds, int divemode) { - add_event(current_dc, seconds, SAMPLE_EVENT_BOOKMARK, 0, divemode, QT_TRANSLATE_NOOP("gettextFromC", "modechange")); - invalidate_dive_cache(current_dive); - mark_divelist_changed(true); - replot(); + Command::addEventDivemodeSwitch(current_dive, dc_number, seconds, divemode); } void ProfileWidget2::addSetpointChange(int seconds) -- cgit v1.2.3-70-g09d2 From 1971cfad547792ba961f804605ccb12780e17de0 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Tue, 3 Mar 2020 23:31:46 +0100 Subject: undo: implement set point change undo command This is a simple copy of the other add-event commands. It could be made more friendly by stating the pO2 value in the text. Signed-off-by: Berthold Stoeger --- commands/command.cpp | 5 +++++ commands/command.h | 1 + commands/command_event.cpp | 6 ++++++ commands/command_event.h | 5 +++++ desktop-widgets/simplewidgets.cpp | 10 ++-------- 5 files changed, 19 insertions(+), 8 deletions(-) (limited to 'commands') diff --git a/commands/command.cpp b/commands/command.cpp index 4f92038b9..44eb4b8f0 100644 --- a/commands/command.cpp +++ b/commands/command.cpp @@ -339,4 +339,9 @@ void addEventDivemodeSwitch(struct dive *d, int dcNr, int seconds, int divemode) execute(new AddEventDivemodeSwitch(d, dcNr, seconds, divemode)); } +void addEventSetpointChange(struct dive *d, int dcNr, int seconds, pressure_t pO2) +{ + execute(new AddEventSetpointChange(d, dcNr, seconds, pO2)); +} + } // namespace Command diff --git a/commands/command.h b/commands/command.h index 1644063a7..aa7c75ab3 100644 --- a/commands/command.h +++ b/commands/command.h @@ -108,6 +108,7 @@ void editTripNotes(dive_trip *trip, const QString &s); void addEventBookmark(struct dive *d, int dcNr, int seconds); void addEventDivemodeSwitch(struct dive *d, int dcNr, int seconds, int divemode); +void addEventSetpointChange(struct dive *d, int dcNr, int seconds, pressure_t pO2); } // namespace Command diff --git a/commands/command_event.cpp b/commands/command_event.cpp index ddef502bc..a1e13f252 100644 --- a/commands/command_event.cpp +++ b/commands/command_event.cpp @@ -49,4 +49,10 @@ AddEventDivemodeSwitch::AddEventDivemodeSwitch(struct dive *d, int dcNr, int sec setText(tr("Add dive mode switch to %1").arg(gettextFromC::tr(divemode_text_ui[divemode]))); } +AddEventSetpointChange::AddEventSetpointChange(struct dive *d, int dcNr, int seconds, pressure_t pO2) : + AddEventBase(d, dcNr, create_event(seconds, SAMPLE_EVENT_PO2, 0, pO2.mbar, QT_TRANSLATE_NOOP("gettextFromC", "SP change"))) +{ + setText(tr("Add set point change")); // TODO: format pO2 value in bar or psi. +} + } // namespace Command diff --git a/commands/command_event.h b/commands/command_event.h index 3130a128f..8cceaeb25 100644 --- a/commands/command_event.h +++ b/commands/command_event.h @@ -44,6 +44,11 @@ public: AddEventDivemodeSwitch(struct dive *d, int dcNr, int seconds, int divemode); }; +class AddEventSetpointChange : public AddEventBase { +public: + AddEventSetpointChange(struct dive *d, int dcNr, int seconds, pressure_t pO2); +}; + } // namespace Command #endif // COMMAND_EVENT_H diff --git a/desktop-widgets/simplewidgets.cpp b/desktop-widgets/simplewidgets.cpp index c31c93676..423f4b68f 100644 --- a/desktop-widgets/simplewidgets.cpp +++ b/desktop-widgets/simplewidgets.cpp @@ -23,7 +23,6 @@ #include "commands/command.h" #include "core/metadata.h" #include "core/tag.h" -#include "core/divelist.h" // for mark_divelist_changed double MinMaxAvgWidget::average() const { @@ -176,13 +175,8 @@ RenumberDialog::RenumberDialog(QWidget *parent) : QDialog(parent), selectedOnly( void SetpointDialog::buttonClicked(QAbstractButton *button) { - if (ui.buttonBox->buttonRole(button) == QDialogButtonBox::AcceptRole) { - add_event(get_dive_dc(d, dcNr), time, SAMPLE_EVENT_PO2, 0, (int)(1000.0 * ui.spinbox->value()), - QT_TRANSLATE_NOOP("gettextFromC", "SP change")); - invalidate_dive_cache(current_dive); - } - mark_divelist_changed(true); - MainWindow::instance()->graphics->replot(); + if (ui.buttonBox->buttonRole(button) == QDialogButtonBox::AcceptRole) + Command::addEventSetpointChange(d, dcNr, time, pressure_t { (int)(1000.0 * ui.spinbox->value()) }); } SetpointDialog::SetpointDialog(struct dive *dIn, int dcNrIn, int seconds) : QDialog(MainWindow::instance()), -- cgit v1.2.3-70-g09d2 From f9fe6d759f33c36f1c0c0d20a591f6517fc9071f Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Wed, 4 Mar 2020 17:35:02 +0100 Subject: undo: split out EventBase class All event-based commands will work on a dive computer and need to replot the profile, etc. Therefore, in analogy to the dive-list commands create a base class with two virtual functions undoit() and redoit() that must be defined in the derived classes that do the actual work. Signed-off-by: Berthold Stoeger --- commands/command_event.cpp | 31 +++++++++++++++++++++++-------- commands/command_event.h | 19 ++++++++++++++----- 2 files changed, 37 insertions(+), 13 deletions(-) (limited to 'commands') diff --git a/commands/command_event.cpp b/commands/command_event.cpp index a1e13f252..7ecd4f070 100644 --- a/commands/command_event.cpp +++ b/commands/command_event.cpp @@ -8,8 +8,27 @@ namespace Command { -AddEventBase::AddEventBase(struct dive *dIn, int dcNrIn, struct event *ev) : - d(dIn), dcNr(dcNrIn), eventToAdd(ev) +EventBase::EventBase(struct dive *dIn, int dcNrIn) : + d(dIn), dcNr(dcNrIn) +{ +} + +void EventBase::redo() +{ + redoit(); // Call actual function in base class + invalidate_dive_cache(d); + emit diveListNotifier.eventsChanged(d); +} + +void EventBase::undo() +{ + undoit(); // Call actual function in base class + invalidate_dive_cache(d); + emit diveListNotifier.eventsChanged(d); +} + +AddEventBase::AddEventBase(struct dive *d, int dcNr, struct event *ev) : EventBase(d, dcNr), + eventToAdd(ev) { } @@ -18,23 +37,19 @@ bool AddEventBase::workToBeDone() return true; } -void AddEventBase::redo() +void AddEventBase::redoit() { struct divecomputer *dc = get_dive_dc(d, dcNr); eventToRemove = eventToAdd.get(); add_event_to_dc(dc, eventToAdd.release()); // return ownership to backend - invalidate_dive_cache(d); - emit diveListNotifier.eventsChanged(d); } -void AddEventBase::undo() +void AddEventBase::undoit() { struct divecomputer *dc = get_dive_dc(d, dcNr); remove_event_from_dc(dc, eventToRemove); eventToAdd.reset(eventToRemove); // take ownership of event eventToRemove = nullptr; - invalidate_dive_cache(d); - emit diveListNotifier.eventsChanged(d); } AddEventBookmark::AddEventBookmark(struct dive *d, int dcNr, int seconds) : diff --git a/commands/command_event.h b/commands/command_event.h index 8cceaeb25..a674e258b 100644 --- a/commands/command_event.h +++ b/commands/command_event.h @@ -16,19 +16,28 @@ namespace Command { // Therefore, the undo commands work on events as they do with dives: using // owning pointers. See comments in command_base.h -class AddEventBase : public Base { -public: - AddEventBase(struct dive *d, int dcNr, struct event *ev); // Takes ownership of event! -private: - bool workToBeDone() override; +class EventBase : public Base { +protected: + EventBase(struct dive *d, int dcNr); void undo() override; void redo() override; + virtual void redoit() = 0; + virtual void undoit() = 0; // Note: we store dive and the divecomputer-number instead of a pointer to the divecomputer. // Since one divecomputer is integrated into the dive structure, pointers to divecomputers // are probably not stable. struct dive *d; int dcNr; +}; + +class AddEventBase : public EventBase { +public: + AddEventBase(struct dive *d, int dcNr, struct event *ev); // Takes ownership of event! +private: + bool workToBeDone() override; + void undoit() override; + void redoit() override; OwningEventPtr eventToAdd; // for redo event *eventToRemove; // for undo -- cgit v1.2.3-70-g09d2 From ab8e317b28672cc19fd04e994b9adf9b63f0c603 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Wed, 4 Mar 2020 18:13:02 +0100 Subject: undo: implement renaming of events There is a slight complexity here owing to the fact that the profile works on a copy of the current dive: We get a copy of the event and have to search for the original event in the current dive. This could be done in the undo command. Nevertheless, here we do it in the profile so that when in the future the profile can work on a non-copied dive we can simply remove this function. Signed-off-by: Berthold Stoeger --- commands/command.cpp | 5 +++++ commands/command.h | 1 + commands/command_event.cpp | 27 +++++++++++++++++++++++++++ commands/command_event.h | 12 ++++++++++++ core/dive.c | 20 +++++++++++++++++++- core/dive.h | 3 +++ profile-widget/profilewidget2.cpp | 29 ++++++++++++++++++++--------- 7 files changed, 87 insertions(+), 10 deletions(-) (limited to 'commands') diff --git a/commands/command.cpp b/commands/command.cpp index 44eb4b8f0..09e85f3b2 100644 --- a/commands/command.cpp +++ b/commands/command.cpp @@ -344,4 +344,9 @@ void addEventSetpointChange(struct dive *d, int dcNr, int seconds, pressure_t pO execute(new AddEventSetpointChange(d, dcNr, seconds, pO2)); } +void renameEvent(struct dive *d, int dcNr, struct event *ev, const char *name) +{ + execute(new RenameEvent(d, dcNr, ev, name)); +} + } // namespace Command diff --git a/commands/command.h b/commands/command.h index aa7c75ab3..eeb400bc4 100644 --- a/commands/command.h +++ b/commands/command.h @@ -109,6 +109,7 @@ void editTripNotes(dive_trip *trip, const QString &s); void addEventBookmark(struct dive *d, int dcNr, int seconds); void addEventDivemodeSwitch(struct dive *d, int dcNr, int seconds, int divemode); void addEventSetpointChange(struct dive *d, int dcNr, int seconds, pressure_t pO2); +void renameEvent(struct dive *d, int dcNr, struct event *ev, const char *name); } // namespace Command diff --git a/commands/command_event.cpp b/commands/command_event.cpp index 7ecd4f070..1114182aa 100644 --- a/commands/command_event.cpp +++ b/commands/command_event.cpp @@ -70,4 +70,31 @@ AddEventSetpointChange::AddEventSetpointChange(struct dive *d, int dcNr, int sec setText(tr("Add set point change")); // TODO: format pO2 value in bar or psi. } +RenameEvent::RenameEvent(struct dive *d, int dcNr, struct event *ev, const char *name) : EventBase(d, dcNr), + eventToAdd(clone_event_rename(ev, name)), + eventToRemove(ev) +{ + setText(tr("Rename bookmark to %1").arg(name)); +} + +bool RenameEvent::workToBeDone() +{ + return true; +} + +void RenameEvent::redoit() +{ + struct divecomputer *dc = get_dive_dc(d, dcNr); + swap_event(dc, eventToRemove, eventToAdd.get()); + event *tmp = eventToRemove; + eventToRemove = eventToAdd.release(); + eventToAdd.reset(tmp); +} + +void RenameEvent::undoit() +{ + // Undo and redo do the same thing - they simply swap events + redoit(); +} + } // namespace Command diff --git a/commands/command_event.h b/commands/command_event.h index a674e258b..ddfe6f7d4 100644 --- a/commands/command_event.h +++ b/commands/command_event.h @@ -58,6 +58,18 @@ public: AddEventSetpointChange(struct dive *d, int dcNr, int seconds, pressure_t pO2); }; +class RenameEvent : public EventBase { +public: + RenameEvent(struct dive *d, int dcNr, struct event *ev, const char *name); +private: + bool workToBeDone() override; + void undoit() override; + void redoit() override; + + OwningEventPtr eventToAdd; // for undo and redo + event *eventToRemove; // for undo and redo +}; + } // namespace Command #endif // COMMAND_EVENT_H diff --git a/core/dive.c b/core/dive.c index 3ec9b1a82..6d2645cef 100644 --- a/core/dive.c +++ b/core/dive.c @@ -166,6 +166,11 @@ struct event *create_event(unsigned int time, int type, int flags, int value, co return ev; } +struct event *clone_event_rename(const struct event *ev, const char *name) +{ + return create_event(ev->time.seconds, ev->type, ev->flags, ev->value, name); +} + void add_event_to_dc(struct divecomputer *dc, struct event *ev) { struct event **p; @@ -192,7 +197,20 @@ struct event *add_event(struct divecomputer *dc, unsigned int time, int type, in return ev; } -static int same_event(const struct event *a, const struct event *b) +/* Substitutes an event in a divecomputer for another. No reordering is performed! */ +void swap_event(struct divecomputer *dc, struct event *from, struct event *to) +{ + for (struct event **ep = &dc->events; *ep; ep = &(*ep)->next) { + if (*ep == from) { + to->next = from->next; + *ep = to; + from->next = NULL; // For good measure. + break; + } + } +} + +bool same_event(const struct event *a, const struct event *b) { if (a->time.seconds != b->time.seconds) return 0; diff --git a/core/dive.h b/core/dive.h index 1de68f4ff..727d4bbff 100644 --- a/core/dive.h +++ b/core/dive.h @@ -378,7 +378,10 @@ extern bool is_cylinder_used(const struct dive *dive, int idx); extern bool is_cylinder_prot(const struct dive *dive, int idx); extern void add_gas_switch_event(struct dive *dive, struct divecomputer *dc, int time, int idx); extern struct event *create_event(unsigned int time, int type, int flags, int value, const char *name); +extern struct event *clone_event_rename(const struct event *ev, const char *name); extern void add_event_to_dc(struct divecomputer *dc, struct event *ev); +extern void swap_event(struct divecomputer *dc, struct event *from, struct event *to); +extern bool same_event(const struct event *a, const struct event *b); extern struct event *add_event(struct divecomputer *dc, unsigned int time, int type, int flags, int value, const char *name); extern void remove_event_from_dc(struct divecomputer *dc, struct event *event); extern void remove_event(const struct event *event); diff --git a/profile-widget/profilewidget2.cpp b/profile-widget/profilewidget2.cpp index 76b3c9afb..824c4ccb6 100644 --- a/profile-widget/profilewidget2.cpp +++ b/profile-widget/profilewidget2.cpp @@ -1584,6 +1584,22 @@ void ProfileWidget2::unhideEvents() item->show(); } +// The profile displays a copy of the current_dive, namely displayed_dive. +// Therefore, the events we get are likewise copies. This function finds +// the original event. TODO: Remove function once the profile can display +// arbitrary dives. +static event *find_event(const struct event *ev) +{ + struct divecomputer *dc = current_dc; + if (!dc) + return nullptr; + for (struct event *act = current_dc->events; act; act = act->next) { + if (same_event(act, ev)) + return act; + } + return nullptr; +} + void ProfileWidget2::removeEvent(DiveEventItem *item) { struct event *event = item->getEvent(); @@ -1698,7 +1714,9 @@ double ProfileWidget2::getFontPrintScale() #ifndef SUBSURFACE_MOBILE void ProfileWidget2::editName(DiveEventItem *item) { - struct event *event = item->getEvent(); + struct event *event = find_event(item->getEvent()); + if (!event) + return; bool ok; QString newName = QInputDialog::getText(this, tr("Edit name of bookmark"), tr("Custom name:"), QLineEdit::Normal, @@ -1710,14 +1728,7 @@ void ProfileWidget2::editName(DiveEventItem *item) lengthWarning.exec(); return; } - // order is important! first update the current dive (by matching the unchanged event), - // then update the displayed dive (as event is part of the events on displayed dive - // and will be freed as part of changing the name! - update_event_name(current_dive, event, qPrintable(newName)); - update_event_name(&displayed_dive, event, qPrintable(newName)); - invalidate_dive_cache(current_dive); - mark_divelist_changed(true); - replot(); + Command::renameEvent(current_dive, dc_number, event, qPrintable(newName)); } } #endif -- cgit v1.2.3-70-g09d2 From 3d511b069f5e2c659ed5af039485b1c72c348b8a Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Wed, 4 Mar 2020 18:25:47 +0100 Subject: undo: add event removal undo command This was a trivial copy & past of the event-adding undo command with a switch of the undo() and redo() actions. Signed-off-by: Berthold Stoeger --- commands/command.cpp | 5 +++++ commands/command.h | 1 + commands/command_event.cpp | 26 ++++++++++++++++++++++++++ commands/command_event.h | 12 ++++++++++++ profile-widget/profilewidget2.cpp | 12 +++++------- 5 files changed, 49 insertions(+), 7 deletions(-) (limited to 'commands') diff --git a/commands/command.cpp b/commands/command.cpp index 09e85f3b2..54f9b22f9 100644 --- a/commands/command.cpp +++ b/commands/command.cpp @@ -349,4 +349,9 @@ void renameEvent(struct dive *d, int dcNr, struct event *ev, const char *name) execute(new RenameEvent(d, dcNr, ev, name)); } +void removeEvent(struct dive *d, int dcNr, struct event *ev) +{ + execute(new RemoveEvent(d, dcNr, ev)); +} + } // namespace Command diff --git a/commands/command.h b/commands/command.h index eeb400bc4..82931747c 100644 --- a/commands/command.h +++ b/commands/command.h @@ -110,6 +110,7 @@ void addEventBookmark(struct dive *d, int dcNr, int seconds); void addEventDivemodeSwitch(struct dive *d, int dcNr, int seconds, int divemode); void addEventSetpointChange(struct dive *d, int dcNr, int seconds, pressure_t pO2); void renameEvent(struct dive *d, int dcNr, struct event *ev, const char *name); +void removeEvent(struct dive *d, int dcNr, struct event *ev); } // namespace Command diff --git a/commands/command_event.cpp b/commands/command_event.cpp index 1114182aa..e32edfc41 100644 --- a/commands/command_event.cpp +++ b/commands/command_event.cpp @@ -97,4 +97,30 @@ void RenameEvent::undoit() redoit(); } +RemoveEvent::RemoveEvent(struct dive *d, int dcNr, struct event *ev) : EventBase(d, dcNr), + eventToRemove(ev) +{ + setText(tr("Remove %1 event").arg(ev->name)); +} + +bool RemoveEvent::workToBeDone() +{ + return true; +} + +void RemoveEvent::redoit() +{ + struct divecomputer *dc = get_dive_dc(d, dcNr); + remove_event_from_dc(dc, eventToRemove); + eventToAdd.reset(eventToRemove); // take ownership of event + eventToRemove = nullptr; +} + +void RemoveEvent::undoit() +{ + struct divecomputer *dc = get_dive_dc(d, dcNr); + eventToRemove = eventToAdd.get(); + add_event_to_dc(dc, eventToAdd.release()); // return ownership to backend +} + } // namespace Command diff --git a/commands/command_event.h b/commands/command_event.h index ddfe6f7d4..eec5cb262 100644 --- a/commands/command_event.h +++ b/commands/command_event.h @@ -70,6 +70,18 @@ private: event *eventToRemove; // for undo and redo }; +class RemoveEvent : public EventBase { +public: + RemoveEvent(struct dive *d, int dcNr, struct event *ev); +private: + bool workToBeDone() override; + void undoit() override; + void redoit() override; + + OwningEventPtr eventToAdd; // for undo + event *eventToRemove; // for redo +}; + } // namespace Command #endif // COMMAND_EVENT_H diff --git a/profile-widget/profilewidget2.cpp b/profile-widget/profilewidget2.cpp index 824c4ccb6..f236ed550 100644 --- a/profile-widget/profilewidget2.cpp +++ b/profile-widget/profilewidget2.cpp @@ -1602,17 +1602,15 @@ static event *find_event(const struct event *ev) void ProfileWidget2::removeEvent(DiveEventItem *item) { - struct event *event = item->getEvent(); + struct event *event = find_event(item->getEvent()); + if (!event) + return; if (QMessageBox::question(this, TITLE_OR_TEXT( tr("Remove the selected event?"), tr("%1 @ %2:%3").arg(event->name).arg(event->time.seconds / 60).arg(event->time.seconds % 60, 2, 10, QChar('0'))), - QMessageBox::Ok | QMessageBox::Cancel) == QMessageBox::Ok) { - remove_event(event); - invalidate_dive_cache(current_dive); - mark_divelist_changed(true); - replot(); - } + QMessageBox::Ok | QMessageBox::Cancel) == QMessageBox::Ok) + Command::removeEvent(current_dive, dc_number, event); } void ProfileWidget2::addBookmark(int seconds) -- cgit v1.2.3-70-g09d2 From 0bd821183d715f709d378e8ac1a75aa0c3d0f2cc Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Wed, 4 Mar 2020 21:10:05 +0100 Subject: undo: implement gas switch This is a bit hairy as - in theory - one gas switch can remove other gas switch(es) at the same timestamp. However, I did not find a way to test it. Moreover, it is not clear whether the dive-tabs are properly updated on undo/redo. Signed-off-by: Berthold Stoeger --- CHANGELOG.md | 1 + commands/command.cpp | 5 ++++ commands/command.h | 1 + commands/command_event.cpp | 62 +++++++++++++++++++++++++++++++++++++++ commands/command_event.h | 13 ++++++++ profile-widget/profilewidget2.cpp | 21 +------------ 6 files changed, 83 insertions(+), 20 deletions(-) (limited to 'commands') diff --git a/CHANGELOG.md b/CHANGELOG.md index 8888b1ab1..d3fefd712 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Mobile: make sure filter header and virtual keyboard are shown when tapping on f Mobile: fix issue where in some circumstances changes weren't written to storage Mobile: fix issues with filtering while editing dives Desktop: implement dive invalidation +Undo: implement undo of event handling (viz. bookmarks, setpoints, gas switches) Desktop: fix tab-order in filter widget Desktop: implement fulltext search Desktop: add starts-with and exact filter modes for textual search diff --git a/commands/command.cpp b/commands/command.cpp index 54f9b22f9..c173494c0 100644 --- a/commands/command.cpp +++ b/commands/command.cpp @@ -354,4 +354,9 @@ void removeEvent(struct dive *d, int dcNr, struct event *ev) execute(new RemoveEvent(d, dcNr, ev)); } +void addGasSwitch(struct dive *d, int dcNr, int seconds, int tank) +{ + execute(new AddGasSwitch(d, dcNr, seconds, tank)); +} + } // namespace Command diff --git a/commands/command.h b/commands/command.h index 82931747c..66122ac99 100644 --- a/commands/command.h +++ b/commands/command.h @@ -111,6 +111,7 @@ void addEventDivemodeSwitch(struct dive *d, int dcNr, int seconds, int divemode) void addEventSetpointChange(struct dive *d, int dcNr, int seconds, pressure_t pO2); void renameEvent(struct dive *d, int dcNr, struct event *ev, const char *name); void removeEvent(struct dive *d, int dcNr, struct event *ev); +void addGasSwitch(struct dive *d, int dcNr, int seconds, int tank); } // namespace Command diff --git a/commands/command_event.cpp b/commands/command_event.cpp index e32edfc41..6d217da65 100644 --- a/commands/command_event.cpp +++ b/commands/command_event.cpp @@ -5,6 +5,7 @@ #include "core/subsurface-qt/divelistnotifier.h" #include "core/libdivecomputer.h" #include "core/gettextfromc.h" +#include namespace Command { @@ -123,4 +124,65 @@ void RemoveEvent::undoit() add_event_to_dc(dc, eventToAdd.release()); // return ownership to backend } +AddGasSwitch::AddGasSwitch(struct dive *d, int dcNr, int seconds, int tank) : EventBase(d, dcNr) +{ + // If there is a gas change at this time stamp, remove it before adding the new one. + // There shouldn't be more than one gas change per time stamp. Just in case we'll + // support that anyway. + struct divecomputer *dc = get_dive_dc(d, dcNr); + struct event *gasChangeEvent = dc->events; + while ((gasChangeEvent = get_next_event_mutable(gasChangeEvent, "gaschange")) != NULL) { + if (gasChangeEvent->time.seconds == seconds) { + eventsToRemove.push_back(gasChangeEvent); + int idx = gasChangeEvent->gas.index; + if (std::find(cylinders.begin(), cylinders.end(), idx) == cylinders.end()) + cylinders.push_back(idx); // cylinders might have changed their status + } + gasChangeEvent = gasChangeEvent->next; + } + + eventsToAdd.emplace_back(create_gas_switch_event(d, dc, seconds, tank)); +} + +bool AddGasSwitch::workToBeDone() +{ + return true; +} + +void AddGasSwitch::redoit() +{ + std::vector newEventsToAdd; + std::vector newEventsToRemove; + newEventsToAdd.reserve(eventsToRemove.size()); + newEventsToRemove.reserve(eventsToAdd.size()); + struct divecomputer *dc = get_dive_dc(d, dcNr); + + for (event *ev: eventsToRemove) { + remove_event_from_dc(dc, ev); + newEventsToAdd.emplace_back(ev); // take ownership of event + } + for (OwningEventPtr &ev: eventsToAdd) { + newEventsToRemove.push_back(ev.get()); + add_event_to_dc(dc, ev.release()); // return ownership to backend + } + eventsToAdd = std::move(newEventsToAdd); + eventsToRemove = std::move(newEventsToRemove); + + // this means we potentially have a new tank that is being used and needs to be shown + fixup_dive(d); + + for (int idx: cylinders) + emit diveListNotifier.cylinderEdited(d, idx); + + // TODO: This is silly we send a DURATION change event so that the statistics are recalculated. + // We should instead define a proper DiveField that expresses the change caused by a gas switch. + emit diveListNotifier.divesChanged(QVector{ d }, DiveField::DURATION | DiveField::DEPTH); +} + +void AddGasSwitch::undoit() +{ + // Undo and redo do the same thing, as the dives-to-be-added and dives-to-be-removed are exchanged. + redoit(); +} + } // namespace Command diff --git a/commands/command_event.h b/commands/command_event.h index eec5cb262..8e75dee6c 100644 --- a/commands/command_event.h +++ b/commands/command_event.h @@ -82,6 +82,19 @@ private: event *eventToRemove; // for redo }; +class AddGasSwitch : public EventBase { +public: + AddGasSwitch(struct dive *d, int dcNr, int seconds, int tank); +private: + bool workToBeDone() override; + void undoit() override; + void redoit() override; + + std::vector cylinders; // cylinders that are modified + std::vector eventsToAdd; + std::vector eventsToRemove; +}; + } // namespace Command #endif // COMMAND_EVENT_H diff --git a/profile-widget/profilewidget2.cpp b/profile-widget/profilewidget2.cpp index f236ed550..6072d4c0d 100644 --- a/profile-widget/profilewidget2.cpp +++ b/profile-widget/profilewidget2.cpp @@ -1645,26 +1645,7 @@ void ProfileWidget2::changeGas(int tank, int seconds) if (!current_dive || tank < 0 || tank >= current_dive->cylinders.nr) return; - // if there is a gas change at this time stamp, remove it before adding the new one - struct event *gasChangeEvent = current_dc->events; - while ((gasChangeEvent = get_next_event_mutable(gasChangeEvent, "gaschange")) != NULL) { - if (gasChangeEvent->time.seconds == seconds) { - remove_event(gasChangeEvent); - gasChangeEvent = current_dc->events; - } else { - gasChangeEvent = gasChangeEvent->next; - } - } - add_gas_switch_event(current_dive, current_dc, seconds, tank); - // this means we potentially have a new tank that is being used and needs to be shown - fixup_dive(current_dive); - invalidate_dive_cache(current_dive); - - // FIXME - this no longer gets written to the dive list - so we need to enableEdition() here - - emit updateDiveInfo(); - mark_divelist_changed(true); - replot(); + Command::addGasSwitch(current_dive, dc_number, seconds, tank); } #endif -- cgit v1.2.3-70-g09d2 From e39063f6df269facaad1430229d8b330385f68ff Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Thu, 5 Mar 2020 23:10:44 +0100 Subject: undo: switch to affected dive on undo/redo of event-changes Select and make current the affected dive. And also switch to the divecomputer that was affected. Signed-off-by: Berthold Stoeger --- commands/command_event.cpp | 11 +++++++++-- commands/command_event.h | 2 ++ 2 files changed, 11 insertions(+), 2 deletions(-) (limited to 'commands') diff --git a/commands/command_event.cpp b/commands/command_event.cpp index 6d217da65..2b9176af5 100644 --- a/commands/command_event.cpp +++ b/commands/command_event.cpp @@ -2,6 +2,7 @@ #include "command_event.h" #include "core/dive.h" +#include "core/selection.h" #include "core/subsurface-qt/divelistnotifier.h" #include "core/libdivecomputer.h" #include "core/gettextfromc.h" @@ -17,15 +18,21 @@ EventBase::EventBase(struct dive *dIn, int dcNrIn) : void EventBase::redo() { redoit(); // Call actual function in base class - invalidate_dive_cache(d); - emit diveListNotifier.eventsChanged(d); + updateDive(); } void EventBase::undo() { undoit(); // Call actual function in base class + updateDive(); +} + +void EventBase::updateDive() +{ invalidate_dive_cache(d); emit diveListNotifier.eventsChanged(d); + dc_number = dcNr; + setSelection({ d }, d); } AddEventBase::AddEventBase(struct dive *d, int dcNr, struct event *ev) : EventBase(d, dcNr), diff --git a/commands/command_event.h b/commands/command_event.h index 8e75dee6c..a363540d5 100644 --- a/commands/command_event.h +++ b/commands/command_event.h @@ -29,6 +29,8 @@ protected: // are probably not stable. struct dive *d; int dcNr; +private: + void updateDive(); }; class AddEventBase : public EventBase { -- cgit v1.2.3-70-g09d2 From 4ae87da58cda895f3573704966ef2e0dd8161864 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Fri, 6 Mar 2020 23:20:58 +0100 Subject: undo: reload dive on removal of gas-switch If a gas-switch is removed we have to perform the same action as if a gas-switch is added: fixup the dive and signal the changed cylinder and stats. Adapt the RemoveEvent command accordingly. Copy the code of the AddGasSwitch command and simplify for the fact that only ony cylinder can be affected. Signed-off-by: Berthold Stoeger --- commands/command_event.cpp | 17 ++++++++++++++++- commands/command_event.h | 2 ++ 2 files changed, 18 insertions(+), 1 deletion(-) (limited to 'commands') diff --git a/commands/command_event.cpp b/commands/command_event.cpp index 2b9176af5..391dfe0a9 100644 --- a/commands/command_event.cpp +++ b/commands/command_event.cpp @@ -106,7 +106,9 @@ void RenameEvent::undoit() } RemoveEvent::RemoveEvent(struct dive *d, int dcNr, struct event *ev) : EventBase(d, dcNr), - eventToRemove(ev) + eventToRemove(ev), + cylinder(ev->type == SAMPLE_EVENT_GASCHANGE2 || ev->type == SAMPLE_EVENT_GASCHANGE ? + ev->gas.index : -1) { setText(tr("Remove %1 event").arg(ev->name)); } @@ -131,6 +133,19 @@ void RemoveEvent::undoit() add_event_to_dc(dc, eventToAdd.release()); // return ownership to backend } +void RemoveEvent::post() const +{ + if (cylinder < 0) + return; + + fixup_dive(d); + emit diveListNotifier.cylinderEdited(d, cylinder); + + // TODO: This is silly we send a DURATION change event so that the statistics are recalculated. + // We should instead define a proper DiveField that expresses the change caused by a gas switch. + emit diveListNotifier.divesChanged(QVector{ d }, DiveField::DURATION | DiveField::DEPTH); +} + AddGasSwitch::AddGasSwitch(struct dive *d, int dcNr, int seconds, int tank) : EventBase(d, dcNr) { // If there is a gas change at this time stamp, remove it before adding the new one. diff --git a/commands/command_event.h b/commands/command_event.h index a363540d5..b7e67f218 100644 --- a/commands/command_event.h +++ b/commands/command_event.h @@ -79,9 +79,11 @@ private: bool workToBeDone() override; void undoit() override; void redoit() override; + void post() const; // Called to fix up dives should a gas-change have happened. OwningEventPtr eventToAdd; // for undo event *eventToRemove; // for redo + int cylinder; // affected cylinder (if removing gas switch). <0: not a gas switch. }; class AddGasSwitch : public EventBase { -- 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') 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') 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') 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') 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