From 9f455b145712636c3e2c1711fc04771aa1238d14 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Mon, 4 Apr 2022 18:57:28 +0200 Subject: [PATCH] selection: move current dive and divecomputer to selection.cpp This tries to encapsulate the management of the current dive and divecomputer in the selection code. The current dive is alreay set by setSelection(). Add a new parameter to also set the current divecomputer. If -1 is passed, then the current computer number is remained. This will allow us to audit the code. Because for now, the whole "current dive computer" thing seems to be ill-defined. This fixes a bug: the dive-computer number wasn't validated when making a new dive the current dive. The new code has some drawbacks though: when selecting a whole trip, the validation will be called for all dives in the trip and thus the dive computer number will depend on the dive with the lowest amount of dive computers in the trip. This will need to be fixed. Signed-off-by: Berthold Stoeger --- commands/command_divelist.cpp | 37 +++++++++---------- commands/command_divelist.h | 2 +- commands/command_edit.cpp | 10 ++--- commands/command_event.cpp | 3 +- core/dive.h | 2 - core/profile.c | 3 -- core/selection.cpp | 24 ++++++++++-- core/selection.h | 9 +++-- desktop-widgets/diveplanner.cpp | 1 + desktop-widgets/locationinformation.cpp | 1 + desktop-widgets/mainwindow.cpp | 1 + desktop-widgets/mapwidget.cpp | 2 +- desktop-widgets/modeldelegates.cpp | 1 + desktop-widgets/profilewidget.cpp | 1 + .../tab-widgets/TabDiveExtraInfo.cpp | 1 + .../tab-widgets/TabDiveInformation.cpp | 1 + desktop-widgets/tab-widgets/TabDivePhotos.cpp | 3 +- profile-widget/qmlprofile.cpp | 1 + qt-models/cylindermodel.cpp | 1 + qt-models/diveplannermodel.cpp | 1 + qt-models/divetripmodel.cpp | 2 +- stats/scatterseries.cpp | 2 +- stats/statsselection.cpp | 2 +- 23 files changed, 68 insertions(+), 43 deletions(-) diff --git a/commands/command_divelist.cpp b/commands/command_divelist.cpp index f83f40709..c55652ab1 100644 --- a/commands/command_divelist.cpp +++ b/commands/command_divelist.cpp @@ -455,7 +455,7 @@ void AddDive::redoit() sort_trip_table(divelog.trips); // Though unlikely, adding a dive may reorder trips // Select the newly added dive - setSelection(divesAndSitesToRemove.dives, divesAndSitesToRemove.dives[0]); + setSelection(divesAndSitesToRemove.dives, divesAndSitesToRemove.dives[0], -1); } void AddDive::undoit() @@ -465,7 +465,7 @@ void AddDive::undoit() sort_trip_table(divelog.trips); // Though unlikely, removing a dive may reorder trips // ...and restore the selection - setSelection(selection, currentDive); + setSelection(selection, currentDive, -1); } ImportDives::ImportDives(struct divelog *log, int flags, const QString &source) @@ -540,7 +540,7 @@ void ImportDives::redoit() divesToAdd = removeDives(divesAndSitesToRemove); // Select the newly added dives - setSelection(divesAndSitesToRemoveNew.dives, divesAndSitesToRemoveNew.dives.back()); + setSelection(divesAndSitesToRemoveNew.dives, divesAndSitesToRemoveNew.dives.back(), -1); // Remember dives and sites to remove divesAndSitesToRemove = std::move(divesAndSitesToRemoveNew); @@ -571,7 +571,7 @@ void ImportDives::undoit() divesAndSitesToRemove = std::move(divesAndSitesToRemoveNew); // ...and restore the selection - setSelection(selection, currentDive); + setSelection(selection, currentDive, -1); // Remove devices for (const device &dev: devicesToAddAndRemove.devices) @@ -608,7 +608,7 @@ void DeleteDive::undoit() // Select all re-added dives and make the first one current dive *currentDive = !divesToDelete.dives.empty() ? divesToDelete.dives[0] : nullptr; - setSelection(divesToDelete.dives, currentDive); + setSelection(divesToDelete.dives, currentDive, -1); } void DeleteDive::redoit() @@ -653,7 +653,7 @@ void ShiftTime::redoit() emit diveListNotifier.divesChanged(dives, DiveField::DATETIME); // Select the changed dives - setSelection(diveList, diveList[0]); + setSelection(diveList, diveList[0], -1); // Negate the time-shift so that the next call does the reverse timeChanged = -timeChanged; @@ -689,7 +689,7 @@ void RenumberDives::undoit() dives.reserve(divesToRenumber.size()); for (const QPair &item: divesToRenumber) dives.push_back(item.first); - setSelection(dives, dives[0]); + setSelection(dives, dives[0], -1); } bool RenumberDives::workToBeDone() @@ -718,7 +718,7 @@ void TripBase::redoit() dives.reserve(divesToMove.divesToMove.size()); for (const DiveToTrip &item: divesToMove.divesToMove) dives.push_back(item.dive); - setSelection(dives, dives[0]); + setSelection(dives, dives[0], -1); } void TripBase::undoit() @@ -847,7 +847,7 @@ void SplitDivesBase::redoit() unsplitDive = removeDives(diveToSplit); // Select split dives and make first dive current - setSelection(divesToUnsplit.dives, divesToUnsplit.dives[0]); + setSelection(divesToUnsplit.dives, divesToUnsplit.dives[0], -1); } void SplitDivesBase::undoit() @@ -857,7 +857,7 @@ void SplitDivesBase::undoit() splitDives = removeDives(divesToUnsplit); // Select unsplit dive and make it current - setSelection(diveToSplit.dives, diveToSplit.dives[0] ); + setSelection(diveToSplit.dives, diveToSplit.dives[0], -1); } static std::array doSplitDives(const dive *d, duration_t time) @@ -895,8 +895,9 @@ SplitDiveComputer::SplitDiveComputer(dive *d, int dc_num) : SplitDivesBase(d, sp setText(Command::Base::tr("split dive computer")); } -DiveComputerBase::DiveComputerBase(dive *old_dive, dive *new_dive, int dc_nr_after_in) : dc_nr_before(dc_number), - dc_nr_after(dc_nr_after_in) +DiveComputerBase::DiveComputerBase(dive *old_dive, dive *new_dive, int dc_nr_before, int dc_nr_after) : + dc_nr_before(dc_nr_before), + dc_nr_after(dc_nr_after) { if (!new_dive) return; @@ -930,11 +931,9 @@ void DiveComputerBase::redoit() diveToAdd = removeDives(diveToRemove); diveToRemove = std::move(addedDive); - dc_number = dc_nr_after; - // Select added dive and make it current. // This automatically replots the profile. - setSelection(diveToRemove.dives, diveToRemove.dives[0]); + setSelection(diveToRemove.dives, diveToRemove.dives[0], dc_nr_after); std::swap(dc_nr_before, dc_nr_after); } @@ -946,13 +945,13 @@ void DiveComputerBase::undoit() } MoveDiveComputerToFront::MoveDiveComputerToFront(dive *d, int dc_num) - : DiveComputerBase(d, make_first_dc(d, dc_num), 0) + : DiveComputerBase(d, make_first_dc(d, dc_num), dc_num, 0) { setText(Command::Base::tr("move dive computer to front")); } DeleteDiveComputer::DeleteDiveComputer(dive *d, int dc_num) - : DiveComputerBase(d, clone_delete_divecomputer(d, dc_num), std::min((int)number_of_computers(d) - 1, dc_num)) + : DiveComputerBase(d, clone_delete_divecomputer(d, dc_num), dc_num, std::min((int)number_of_computers(d) - 1, dc_num)) { setText(Command::Base::tr("delete dive computer")); } @@ -1059,7 +1058,7 @@ void MergeDives::redoit() unmergedDives = removeDives(divesToMerge); // Select merged dive and make it current - setSelection(diveToUnmerge.dives, diveToUnmerge.dives[0]); + setSelection(diveToUnmerge.dives, diveToUnmerge.dives[0], -1); } void MergeDives::undoit() @@ -1069,7 +1068,7 @@ void MergeDives::undoit() renumberDives(divesToRenumber); // Select unmerged dives and make first one current - setSelection(divesToMerge.dives, divesToMerge.dives[0]); + setSelection(divesToMerge.dives, divesToMerge.dives[0], -1); } } // namespace Command diff --git a/commands/command_divelist.h b/commands/command_divelist.h index 1bc0cbe7b..2d333a171 100644 --- a/commands/command_divelist.h +++ b/commands/command_divelist.h @@ -237,7 +237,7 @@ class DiveComputerBase : public DiveListBase { protected: // old_dive must be a dive known to the core. // new_dive must be new dive whose ownership is taken. - DiveComputerBase(dive *old_dive, dive *new_dive, int dc_nr_after); + DiveComputerBase(dive *old_dive, dive *new_dive, int dc_nr_before, int dc_nr_after); private: void undoit() override; void redoit() override; diff --git a/commands/command_edit.cpp b/commands/command_edit.cpp index 981d9ec66..881607830 100644 --- a/commands/command_edit.cpp +++ b/commands/command_edit.cpp @@ -159,7 +159,7 @@ void EditBase::undo() DiveField id = fieldId(); emit diveListNotifier.divesChanged(stdToQt(dives), id); if (!placingCommand()) - setSelection(selectedDives, current); + setSelection(selectedDives, current, -1); } // We have to manually instantiate the constructors of the EditBase class, @@ -552,7 +552,7 @@ void EditTagsBase::undo() // Send signals. DiveField id = fieldId(); emit diveListNotifier.divesChanged(stdToQt(dives), id); - setSelection(selectedDives, current); + setSelection(selectedDives, current, -1); } // Undo and redo do the same as just the stored value is exchanged @@ -862,7 +862,7 @@ void ReplanDive::undo() emit diveListNotifier.divesChanged(divesToNotify, DiveField::DATETIME | DiveField::DURATION | DiveField::DEPTH | DiveField::MODE | DiveField::NOTES | DiveField::SALINITY | DiveField::ATM_PRESS); if (!placingCommand()) - setSelection({ d }, d); + setSelection({ d }, d, -1); } // Redo and undo do the same @@ -935,7 +935,7 @@ void EditProfile::undo() QVector divesToNotify = { d }; emit diveListNotifier.divesChanged(divesToNotify, DiveField::DURATION | DiveField::DEPTH); if (!placingCommand()) - setSelection({ d }, d); + setSelection({ d }, d, dcNr); } // Redo and undo do the same @@ -1521,7 +1521,7 @@ void EditDive::exchangeDives() emit diveListNotifier.divesChanged(dives, changedFields); // Select the changed dives - setSelection( { oldDive }, oldDive); + setSelection( { oldDive }, oldDive, -1); } void EditDive::editDs() diff --git a/commands/command_event.cpp b/commands/command_event.cpp index 4e29b452c..b1a2f0e86 100644 --- a/commands/command_event.cpp +++ b/commands/command_event.cpp @@ -32,8 +32,7 @@ void EventBase::updateDive() { invalidate_dive_cache(d); emit diveListNotifier.eventsChanged(d); - dc_number = dcNr; - setSelection({ d }, d); + setSelection({ d }, d, dcNr); } AddEventBase::AddEventBase(struct dive *d, int dcNr, struct event *ev) : EventBase(d, dcNr), diff --git a/core/dive.h b/core/dive.h index 91b20b785..e18cbbf4b 100644 --- a/core/dive.h +++ b/core/dive.h @@ -111,8 +111,6 @@ extern depth_t gas_mod(struct gasmix mix, pressure_t po2_limit, const struct div extern depth_t gas_mnd(struct gasmix mix, depth_t end, const struct dive *dive, int roundto); extern struct dive displayed_dive; -extern unsigned int dc_number; -extern struct dive *current_dive; extern struct dive *get_dive(int nr); extern struct dive *get_dive_from_table(int nr, const struct dive_table *dt); diff --git a/core/profile.c b/core/profile.c index 0ead20d14..184ad8590 100644 --- a/core/profile.c +++ b/core/profile.c @@ -30,9 +30,6 @@ extern int ascent_velocity(int depth, int avg_depth, int bottom_time); -struct dive *current_dive = NULL; -unsigned int dc_number = 0; - #ifdef DEBUG_PI /* debugging tool - not normally used */ static void dump_pi(struct plot_info *pi) diff --git a/core/selection.cpp b/core/selection.cpp index cb716572a..883628b1c 100644 --- a/core/selection.cpp +++ b/core/selection.cpp @@ -9,9 +9,21 @@ #include +struct dive *current_dive = NULL; +unsigned int dc_number = 0; int amount_selected; static int amount_trips_selected; +static void fixup_current_dc() +{ + // Every dive is guaranteed to have a dc + if (!current_dive || dc_number == 0) + return; + + // Note that number_of_computers returns at least 1, so subtraction is valid. + dc_number = std::min(dc_number, number_of_computers(current_dive) - 1); +} + extern "C" void select_dive(struct dive *dive) { if (!dive) @@ -21,6 +33,7 @@ extern "C" void select_dive(struct dive *dive) amount_selected++; } current_dive = dive; + fixup_current_dc(); } extern "C" void deselect_dive(struct dive *dive) @@ -51,6 +64,7 @@ extern "C" void deselect_dive(struct dive *dive) } current_dive = NULL; } + fixup_current_dc(); } extern "C" struct dive *first_selected_dive() @@ -155,7 +169,7 @@ static void setClosestCurrentDive(timestamp_t when, const std::vector &s // Reset the selection to the dives of the "selection" vector and send the appropriate signals. // Set the current dive to "currentDive". "currentDive" must be an element of "selection" (or // null if "seletion" is empty). Return true if the selection or current dive changed. -void setSelection(const std::vector &selection, dive *currentDive) +void setSelection(const std::vector &selection, dive *currentDive, int currentDc) { // To do so, generate vectors of dives to be selected and deselected. // We send signals batched by trip, so keep track of trip/dive pairs. @@ -203,6 +217,10 @@ void setSelection(const std::vector &selection, dive *currentDive) setClosestCurrentDive(currentDive->when, selection, divesToSelect); } + if (currentDc >= 0) + dc_number = currentDc; + fixup_current_dc(); + // Send the new selection emit diveListNotifier.divesSelected(divesToSelect); } @@ -210,9 +228,9 @@ void setSelection(const std::vector &selection, dive *currentDive) extern "C" void select_single_dive(dive *d) { if (d) - setSelection(std::vector{ d }, d); + setSelection(std::vector{ d }, d, -1); else - setSelection(std::vector(), nullptr); + setSelection(std::vector(), nullptr, -1); } // Turn current selection into a vector. diff --git a/core/selection.h b/core/selection.h index 713e1b229..5d72af131 100644 --- a/core/selection.h +++ b/core/selection.h @@ -7,6 +7,8 @@ struct dive; extern int amount_selected; +extern unsigned int dc_number; +extern struct dive *current_dive; /*** C and C++ functions ***/ @@ -40,9 +42,10 @@ extern void dump_selection(void); #include // Reset the selection to the dives of the "selection" vector and send the appropriate signals. -// Set the current dive to "currentDive". "currentDive" must be an element of "selection" (or -// null if "seletion" is empty). -void setSelection(const std::vector &selection, dive *currentDive); +// Set the current dive to "currentDive" and the current dive computer to "currentDc". +// "currentDive" must be an element of "selection" (or null if "seletion" is empty). +// If "currentDc" is negative, an attempt will be made to keep the current computer number. +void setSelection(const std::vector &selection, dive *currentDive, int currentDc); // Get currently selectd dives std::vector getDiveSelection(); diff --git a/desktop-widgets/diveplanner.cpp b/desktop-widgets/diveplanner.cpp index 936dacc5e..18fcfeeb2 100644 --- a/desktop-widgets/diveplanner.cpp +++ b/desktop-widgets/diveplanner.cpp @@ -5,6 +5,7 @@ #include "core/planner.h" #include "core/qthelper.h" #include "core/units.h" +#include "core/selection.h" #include "core/settings/qPrefDivePlanner.h" #include "core/subsurface-qt/divelistnotifier.h" #include "core/gettextfromc.h" diff --git a/desktop-widgets/locationinformation.cpp b/desktop-widgets/locationinformation.cpp index 0206f9d5f..942d1b6ff 100644 --- a/desktop-widgets/locationinformation.cpp +++ b/desktop-widgets/locationinformation.cpp @@ -14,6 +14,7 @@ #include "desktop-widgets/modeldelegates.h" #include "core/subsurface-qt/divelistnotifier.h" #include "core/taxonomy.h" +#include "core/selection.h" #include "core/settings/qPrefUnit.h" #include "commands/command.h" diff --git a/desktop-widgets/mainwindow.cpp b/desktop-widgets/mainwindow.cpp index ec7beaba2..4e25fe595 100644 --- a/desktop-widgets/mainwindow.cpp +++ b/desktop-widgets/mainwindow.cpp @@ -28,6 +28,7 @@ #include "core/import-csv.h" #include "core/planner.h" #include "core/qthelper.h" +#include "core/selection.h" #include "core/subsurface-string.h" #include "core/trip.h" #include "core/version.h" diff --git a/desktop-widgets/mapwidget.cpp b/desktop-widgets/mapwidget.cpp index 5ededbf2b..4c8757f50 100644 --- a/desktop-widgets/mapwidget.cpp +++ b/desktop-widgets/mapwidget.cpp @@ -103,7 +103,7 @@ void MapWidget::selectedDivesChanged(const QList &list) if (dive *d = get_dive(idx)) selection.push_back(d); } - setSelection(selection, current_dive); + setSelection(selection, current_dive, -1); } void MapWidget::coordinatesChanged(struct dive_site *ds, const location_t &location) diff --git a/desktop-widgets/modeldelegates.cpp b/desktop-widgets/modeldelegates.cpp index d6f2d95e0..e719e7372 100644 --- a/desktop-widgets/modeldelegates.cpp +++ b/desktop-widgets/modeldelegates.cpp @@ -17,6 +17,7 @@ #include "qt-models/divelocationmodel.h" #include "core/qthelper.h" #include "core/divesite.h" +#include "core/selection.h" #include "desktop-widgets/simplewidgets.h" #include diff --git a/desktop-widgets/profilewidget.cpp b/desktop-widgets/profilewidget.cpp index 15e98420a..2540abd86 100644 --- a/desktop-widgets/profilewidget.cpp +++ b/desktop-widgets/profilewidget.cpp @@ -4,6 +4,7 @@ #include "profile-widget/profilewidget2.h" #include "commands/command.h" #include "core/color.h" +#include "core/selection.h" #include "core/settings/qPrefTechnicalDetails.h" #include "core/settings/qPrefPartialPressureGas.h" #include "core/subsurface-string.h" diff --git a/desktop-widgets/tab-widgets/TabDiveExtraInfo.cpp b/desktop-widgets/tab-widgets/TabDiveExtraInfo.cpp index d3e961f24..21ee1cdc4 100644 --- a/desktop-widgets/tab-widgets/TabDiveExtraInfo.cpp +++ b/desktop-widgets/tab-widgets/TabDiveExtraInfo.cpp @@ -2,6 +2,7 @@ #include "TabDiveExtraInfo.h" #include "ui_TabDiveExtraInfo.h" #include "core/dive.h" +#include "core/selection.h" #include "qt-models/divecomputerextradatamodel.h" TabDiveExtraInfo::TabDiveExtraInfo(QWidget *parent) : diff --git a/desktop-widgets/tab-widgets/TabDiveInformation.cpp b/desktop-widgets/tab-widgets/TabDiveInformation.cpp index 89643e09f..67824f9fa 100644 --- a/desktop-widgets/tab-widgets/TabDiveInformation.cpp +++ b/desktop-widgets/tab-widgets/TabDiveInformation.cpp @@ -9,6 +9,7 @@ #include "core/units.h" #include "core/dive.h" #include "core/qthelper.h" +#include "core/selection.h" #include "core/statistics.h" #include "core/divelist.h" diff --git a/desktop-widgets/tab-widgets/TabDivePhotos.cpp b/desktop-widgets/tab-widgets/TabDivePhotos.cpp index 87203c74c..5c920e551 100644 --- a/desktop-widgets/tab-widgets/TabDivePhotos.cpp +++ b/desktop-widgets/tab-widgets/TabDivePhotos.cpp @@ -11,8 +11,9 @@ #include #include #include -#include "core/save-profiledata.h" #include "core/membuffer.h" +#include "core/save-profiledata.h" +#include "core/selection.h" //TODO: Remove those in the future. #include "../mainwindow.h" diff --git a/profile-widget/qmlprofile.cpp b/profile-widget/qmlprofile.cpp index b3076d2b5..b7602b42b 100644 --- a/profile-widget/qmlprofile.cpp +++ b/profile-widget/qmlprofile.cpp @@ -5,6 +5,7 @@ #include "core/errorhelper.h" #include "core/subsurface-float.h" #include "core/metrics.h" +#include "core/subsurface-string.h" #include #include #include diff --git a/qt-models/cylindermodel.cpp b/qt-models/cylindermodel.cpp index b2512070f..157352e9e 100644 --- a/qt-models/cylindermodel.cpp +++ b/qt-models/cylindermodel.cpp @@ -8,6 +8,7 @@ #include "qt-models/diveplannermodel.h" #include "core/gettextfromc.h" #include "core/sample.h" +#include "core/selection.h" #include "core/subsurface-qt/divelistnotifier.h" #include "core/subsurface-string.h" #include diff --git a/qt-models/diveplannermodel.cpp b/qt-models/diveplannermodel.cpp index 79e6afa84..fd6cccfcb 100644 --- a/qt-models/diveplannermodel.cpp +++ b/qt-models/diveplannermodel.cpp @@ -11,6 +11,7 @@ #include "core/qthelper.h" #include "core/range.h" #include "core/sample.h" +#include "core/selection.h" #include "core/settings/qPrefDivePlanner.h" #include "core/settings/qPrefUnit.h" #if !defined(SUBSURFACE_TESTING) diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp index 32b643f40..86bf635d0 100644 --- a/qt-models/divetripmodel.cpp +++ b/qt-models/divetripmodel.cpp @@ -466,7 +466,7 @@ void DiveTripModelBase::initSelection() { std::vector dives = getDiveSelection(); if (!dives.empty()) - setSelection(dives, current_dive); + setSelection(dives, current_dive, -1); else select_newest_visible_dive(); } diff --git a/stats/scatterseries.cpp b/stats/scatterseries.cpp index 028fe93ef..6858a8fa3 100644 --- a/stats/scatterseries.cpp +++ b/stats/scatterseries.cpp @@ -132,7 +132,7 @@ void ScatterSeries::selectItemsInRect(const QRectF &rect, SelectionModifier modi selected.push_back(items[idx].d); } - setSelection(selected, selected.empty() ? nullptr : selected.front()); + setSelection(selected, selected.empty() ? nullptr : selected.front(), -1); } static QString dataInfo(const StatsVariable &var, const dive *d) diff --git a/stats/statsselection.cpp b/stats/statsselection.cpp index 82652ee68..77dc650d4 100644 --- a/stats/statsselection.cpp +++ b/stats/statsselection.cpp @@ -38,5 +38,5 @@ void processSelection(std::vector dives, SelectionModifier modifier) selected = std::move(dives); } - setSelection(selected, selected.empty() ? nullptr : selected.front()); + setSelection(selected, selected.empty() ? nullptr : selected.front(), -1); }