From 10fc3bfd47443b409aca7abd482bc60b74cbc9ee Mon Sep 17 00:00:00 2001 From: Michael Keller Date: Sun, 9 Jun 2024 16:16:26 +1200 Subject: [PATCH] Bugfix: Fix Incorrect Volumes Displayed for Tank Types. Fix an issue introduced in #4148. Essentially the refactoring missed the fact that in the imperial system tank size is tracked as the free gas volume, but in the metric system (which is the one used in most of Subsurface's calculations) tank size is tracked as water capacity. So when updating a tank template tracking imperial measurements, the given (metric) volume in l has to be multiplied by the working pressure, and vice versa. This also combines all the logic dealing with `tank_info` data in one place, hopefully making it less likely that this will be broken by inconsistencies in the future. Fixes #4239. Signed-off-by: Michael Keller --- commands/command_edit.cpp | 3 +-- core/equipment.c | 41 +++++++++++++++++------------- core/equipment.h | 6 ++--- desktop-widgets/modeldelegates.cpp | 15 ++++------- qt-models/tankinfomodel.cpp | 12 ++++----- 5 files changed, 38 insertions(+), 39 deletions(-) diff --git a/commands/command_edit.cpp b/commands/command_edit.cpp index 7f090eacf..0fb4b5992 100644 --- a/commands/command_edit.cpp +++ b/commands/command_edit.cpp @@ -1319,8 +1319,7 @@ EditCylinder::EditCylinder(int index, cylinder_t cylIn, EditCylinderType typeIn, void EditCylinder::redo() { for (size_t i = 0; i < dives.size(); ++i) { - set_tank_info_size(&tank_info_table, cyl[i].type.description, cyl[i].type.size); - set_tank_info_workingpressure(&tank_info_table, cyl[i].type.description, cyl[i].type.workingpressure); + set_tank_info_data(&tank_info_table, cyl[i].type.description, cyl[i].type.size, cyl[i].type.workingpressure); std::swap(*get_cylinder(dives[i], indexes[i]), cyl[i]); update_cylinder_related_info(dives[i]); emit diveListNotifier.cylinderEdited(dives[i], indexes[i]); diff --git a/core/equipment.c b/core/equipment.c index 96bfa73f7..6f1904adf 100644 --- a/core/equipment.c +++ b/core/equipment.c @@ -109,7 +109,7 @@ void add_tank_info_imperial(struct tank_info_table *table, const char *name, int add_to_tank_info_table(table, table->nr, info); } -extern struct tank_info *get_tank_info(struct tank_info_table *table, const char *name) +static struct tank_info *get_tank_info(struct tank_info_table *table, const char *name) { for (int i = 0; i < table->nr; ++i) { if (same_string(table->infos[i].name, name)) @@ -118,34 +118,41 @@ extern struct tank_info *get_tank_info(struct tank_info_table *table, const char return NULL; } -extern void set_tank_info_size(struct tank_info_table *table, const char *name, volume_t size) +extern void set_tank_info_data(struct tank_info_table *table, const char *name, volume_t size, pressure_t working_pressure) { struct tank_info *info = get_tank_info(table, name); if (info) { - // Try to be smart about metric vs. imperial - if (info->cuft == 0 && info->psi == 0) + if (info->ml != 0 || info->bar != 0) { + info->bar = working_pressure.mbar / 1000; info->ml = size.mliter; - else - info->cuft = lrint(ml_to_cuft(size.mliter)); + } else { + info->psi = lrint(to_PSI(working_pressure)); + info->cuft = lrint(ml_to_cuft(size.mliter) * mbar_to_atm(working_pressure.mbar)); + } } else { - // By default add metric...? - add_tank_info_metric(table, name, size.mliter, 0); + // Metric is a better choice as the volume is independent of the working pressure + add_tank_info_metric(table, name, size.mliter, working_pressure.mbar / 1000); } } -extern void set_tank_info_workingpressure(struct tank_info_table *table, const char *name, pressure_t working_pressure) +extern void extract_tank_info(const struct tank_info *info, volume_t *size, pressure_t *working_pressure) +{ + working_pressure->mbar = info->bar != 0 ? info->bar * 1000 : psi_to_mbar(info->psi); + if (info->ml != 0) + size->mliter = info->ml; + else if (working_pressure->mbar != 0) + size->mliter = lrint(cuft_to_l(info->cuft) * 1000 / mbar_to_atm(working_pressure->mbar)); +} + +extern bool get_tank_info_data(struct tank_info_table *table, const char *name, volume_t *size, pressure_t *working_pressure) { struct tank_info *info = get_tank_info(table, name); if (info) { - // Try to be smart about metric vs. imperial - if (info->cuft == 0 && info->psi == 0) - info->bar = working_pressure.mbar / 1000; - else - info->psi = lrint(mbar_to_PSI(working_pressure.mbar)); - } else { - // By default add metric...? - add_tank_info_metric(table, name, 0, working_pressure.mbar / 1000); + extract_tank_info(info, size, working_pressure); + + return true; } + return false; } /* placeholders for a few functions that we need to redesign for the Qt UI */ diff --git a/core/equipment.h b/core/equipment.h index 4df799b89..0d23b9341 100644 --- a/core/equipment.h +++ b/core/equipment.h @@ -126,9 +126,9 @@ extern void reset_tank_info_table(struct tank_info_table *table); extern void clear_tank_info_table(struct tank_info_table *table); extern void add_tank_info_metric(struct tank_info_table *table, const char *name, int ml, int bar); extern void add_tank_info_imperial(struct tank_info_table *table, const char *name, int cuft, int psi); -extern void set_tank_info_size(struct tank_info_table *table, const char *name, volume_t size); -extern void set_tank_info_workingpressure(struct tank_info_table *table, const char *name, pressure_t working_pressure); -extern struct tank_info *get_tank_info(struct tank_info_table *table, const char *name); +extern void extract_tank_info(const struct tank_info *info, volume_t *size, pressure_t *working_pressure); +extern bool get_tank_info_data(struct tank_info_table *table, const char *name, volume_t *size, pressure_t *pressure); +extern void set_tank_info_data(struct tank_info_table *table, const char *name, volume_t size, pressure_t working_pressure); struct ws_info_t { const char *name; diff --git a/desktop-widgets/modeldelegates.cpp b/desktop-widgets/modeldelegates.cpp index e14b57f25..d0ca3e541 100644 --- a/desktop-widgets/modeldelegates.cpp +++ b/desktop-widgets/modeldelegates.cpp @@ -218,18 +218,13 @@ void TankInfoDelegate::setModelData(QWidget *, QAbstractItemModel *model, const mymodel->setData(IDX(CylindersModel::TYPE), cylinderName, CylindersModel::TEMP_ROLE); return; } - int tankSize = 0; - int tankPressure = 0; - tank_info *info = get_tank_info(&tank_info_table, qPrintable(cylinderName)); - if (info) { - // OMG, the units here are a mess. - tankSize = info->ml != 0 ? info->ml : lrint(cuft_to_l(info->cuft) * 1000.0); - tankPressure = info->bar != 0 ? info->bar * 1000 : psi_to_mbar(info->psi); - } + volume_t tankSize = {0}; + pressure_t tankPressure = {0}; + get_tank_info_data(&tank_info_table, qPrintable(cylinderName), &tankSize, &tankPressure); mymodel->setData(IDX(CylindersModel::TYPE), cylinderName, CylindersModel::TEMP_ROLE); - mymodel->setData(IDX(CylindersModel::WORKINGPRESS), tankPressure, CylindersModel::TEMP_ROLE); - mymodel->setData(IDX(CylindersModel::SIZE), tankSize, CylindersModel::TEMP_ROLE); + mymodel->setData(IDX(CylindersModel::WORKINGPRESS), tankPressure.mbar, CylindersModel::TEMP_ROLE); + mymodel->setData(IDX(CylindersModel::SIZE), tankSize.mliter, CylindersModel::TEMP_ROLE); } static QAbstractItemModel *createTankInfoModel(QWidget *parent) diff --git a/qt-models/tankinfomodel.cpp b/qt-models/tankinfomodel.cpp index 6dca11cef..0e260374d 100644 --- a/qt-models/tankinfomodel.cpp +++ b/qt-models/tankinfomodel.cpp @@ -13,17 +13,15 @@ QVariant TankInfoModel::data(const QModelIndex &index, int role) const return defaultModelFont(); if (role == Qt::DisplayRole || role == Qt::EditRole) { const struct tank_info &info = tank_info_table.infos[index.row()]; - int ml = info.ml; - double bar = (info.psi) ? psi_to_bar(info.psi) : info.bar; - - if (info.cuft && info.psi) - ml = lrint(cuft_to_l(info.cuft) * 1000 / bar_to_atm(bar)); + volume_t size = {0}; + pressure_t pressure = {0}; + extract_tank_info(&info, &size, &pressure); switch (index.column()) { case BAR: - return bar * 1000; + return pressure.mbar; case ML: - return ml; + return size.mliter; case DESCRIPTION: return info.name; }