Fix for combo box assert on new recipe creation

CHANGES.markdown

@@ -13,6 +13,18 @@ happens, so I'm now setting it to a slightly arbitrary time early in the morning
 * Additional methods for calculating IBU
 * We'll list other new features here...
 
+## v4.0.14
+Bug fixes and minor enhancements.
+
+### New Features
+* Updated Danish translations (courtesy of Orla Valbjørn Møller)
+
+### Bug Fixes
+* Crash when creating or opening user recipe [910](https://github.com/Brewtarget/brewtarget/issues/910)
+
+### Release Timestamp
+Tue, 31 Dec 2024 04:00:14 +0100
+
 ## v4.0.13
 Bug fixes and minor enhancements.
 

CMakeLists.txt

@@ -115,7 +115,7 @@ endif()
 #=======================================================================================================================
 # It's simplest to keep the project name all lower-case as it means we can use a lot more of the default settings for
 # Linux packaging (where directory names etc are expected to be all lower-case).
-project(brewtarget VERSION 4.0.13 LANGUAGES CXX)
+project(brewtarget VERSION 4.0.14 LANGUAGES CXX)
 message(STATUS "Building ${PROJECT_NAME} version ${PROJECT_VERSION}")
 message(STATUS "PROJECT_SOURCE_DIR is ${PROJECT_SOURCE_DIR}")
 # Sometimes we do need the capitalised version of the project name

meson.build

@@ -189,7 +189,7 @@
 #
 #
 project('brewtarget', 'cpp',
-        version: '4.0.13',
+        version: '4.0.14',
         license: 'GPL-3.0-or-later',
         meson_version: '>=0.63.0',
         default_options : ['cpp_std=c++23',

packaging/windows/NsisInstallerScript.nsi.in

@@ -243,7 +243,7 @@ functionEnd
 Name "${APPLICATION_DISPLAY_NAME}"
 
 # Name of the installer executable to create
-OutFile "${APPLICATION_DISPLAY_NAME} Installer.exe"
+OutFile "${APPLICATION_DISPLAY_NAME} Windows Installer.exe"
 
 #
 # Default installation directory

scripts/buildTool.py

@@ -2451,9 +2451,12 @@ def doPackage():
          )
 
          #
-         # Make the checksum file
+         # Make the checksum file.
+         #
+         # Note that the name of the installer file is controlled by packaging/windows/NsisInstallerScript.nsi.in, so
+         # we have to align here with what that says.
          #
-         winInstallerName = capitalisedProjectName + ' ' + buildConfig['CONFIG_VERSION_STRING'] + ' Installer.exe'
+         winInstallerName = capitalisedProjectName + ' ' + buildConfig['CONFIG_VERSION_STRING'] + ' Windows Installer.exe'
          log.info('Generating checksum file for ' + winInstallerName)
          writeSha256sum(dir_packages_platform, winInstallerName)
 
@@ -2483,7 +2486,7 @@ def doPackage():
          #
          # (When working on this bit, use ❌ for things that are generated automatically but not actually needed, and ✴
          # for things we still need to add.)
-         #    [projectName]_[versionNumber].app
+         #    [projectName]_[versionNumber]_MacOS.app
          #    └── Contents
          #        ├── Info.plist ❇  <── "Information property list" file = required configuration information (in XML)
          #        │                      This includes things such as Bundle ID.  It is generated by the Meson build
@@ -2569,7 +2572,7 @@ def doPackage():
          # Make the top-level directories that we're going to copy files into
          #
          log.debug('Creating Mac app bundle top-level directories')
-         macBundleDirName = projectName + '_' + buildConfig['CONFIG_VERSION_STRING'] + '.app'
+         macBundleDirName = projectName + '_' + buildConfig['CONFIG_VERSION_STRING'] + '_MacOS.app'
          # dir_packages_platform = mbuild/packages/darwin
          dir_packages_mac = dir_packages_platform.joinpath(macBundleDirName).joinpath('Contents')
          dir_packages_mac_bin = dir_packages_mac.joinpath('MacOS')

src/MainWindow.cpp

@@ -2010,21 +2010,20 @@ void MainWindow::displayRangesEtcForCurrentRecipeStyle() {
    }
 
    auto style = this->pimpl->m_recipeObs->style();
-   if (!style) {
-      return;
-   }
+   this->styleButton->setStyle(style);
+   this->styleComboBox->setItem(style);
 
-   this->styleRangeWidget_og->setPreferredRange(this->oGLabel->getRangeToDisplay(style->ogMin(), style->ogMax()));
+   if (style) {
+      this->styleRangeWidget_og->setPreferredRange(this->oGLabel->getRangeToDisplay(style->ogMin(), style->ogMax()));
 
-   this->styleRangeWidget_fg->setPreferredRange(this->fGLabel->getRangeToDisplay(style->ogMin(), style->ogMax()));
+      this->styleRangeWidget_fg->setPreferredRange(this->fGLabel->getRangeToDisplay(style->ogMin(), style->ogMax()));
 
-   // If min and/or max ABV is not set on the Style, then use some sensible outer limit(s)
-   this->styleRangeWidget_abv->setPreferredRange(style->abvMin_pct().value_or(0.0), style->abvMax_pct().value_or(50.0));
-   this->styleRangeWidget_ibu->setPreferredRange(style->ibuMin(), style->ibuMax());
-   this->styleRangeWidget_srm->setPreferredRange(this->colorSRMLabel->getRangeToDisplay(style->colorMin_srm(),
-                                                                                        style->colorMax_srm()));
-   this->styleButton->setStyle(style);
-   this->styleComboBox->setItem(style);
+      // If min and/or max ABV is not set on the Style, then use some sensible outer limit(s)
+      this->styleRangeWidget_abv->setPreferredRange(style->abvMin_pct().value_or(0.0), style->abvMax_pct().value_or(50.0));
+      this->styleRangeWidget_ibu->setPreferredRange(style->ibuMin(), style->ibuMax());
+      this->styleRangeWidget_srm->setPreferredRange(this->colorSRMLabel->getRangeToDisplay(style->colorMin_srm(),
+                                                                                          style->colorMax_srm()));
+   }
 
    return;
 }

src/widgets/BtComboBoxNamedEntity.cpp

@@ -33,9 +33,14 @@ BtComboBoxNamedEntity::~BtComboBoxNamedEntity() = default;
  * \param value -1 means nothing selected
  */
 void BtComboBoxNamedEntity::setCurrentId(int value) {
-   int const index {this->findData(value)};
+   //
+   // See comment in widgets/BtComboBoxNamedEntity.h for why the "nothing selected" option cannot appear explicitly
+   //
+   int const index {value < 0 ? -1 : this->findData(value)};
+   qDebug() << Q_FUNC_INFO << "value:" << value << ", index:" << index;
    // It's a coding error to set an ID we don't know about
-   Q_ASSERT(index >= 0);
+   Q_ASSERT(value < 0 || index >= 0);
+
    this->setCurrentIndex(index);
    return;
 }

src/widgets/BtComboBoxNamedEntity.h

@@ -43,12 +43,6 @@ class BtComboBoxNamedEntity : public QComboBox {
    void setCurrentId(int value);
 
    int getCurrentId() const;
-
-   //! Reimplemented from QComboBox to allow the popup to be independently sized.
-///   void showPopup();
-
-   //! Reimplemented from QComboBox to not show any text.
-///   virtual void paintEvent(QPaintEvent*);
 };
 
 
@@ -95,7 +89,18 @@ class BtComboBoxNamedEntityBase : public CuriouslyRecurringTemplateBase<BtComboB
       this->m_sortFilterProxyModel->sort(0);
       this->derived().setModel(this->m_sortFilterProxyModel.get());
 
-      this->derived().insertItem(0, "", QVariant::fromValue<int>(-1));
+      //
+      // The current limitation of using our own QSortFilterProxyModel etc to control what the combo box displays is
+      // that we can't add an explicit "empty item" to the list of items, because EquipmentSortFilterProxyModel etc
+      // don't support this.  Eg, putting the following here will have no effect:
+      //
+      //    this->derived().insertItem(0, "", QVariant::fromValue<int>(-1));
+      //
+      // However, even when a QComboBox combo box doesn't have such an "empty item", you can still set the current
+      // selection to be empty (ie nothing selected) by passing -1 to QComboBox::setCurrentIndex.  It's just that, once
+      // it is set to something, the user won't be able to unset it.  On the whole, that is actually what we want.  Eg,
+      // once the user has set the Equipment on a new Recipe for the first time, they can change it but not unset it.
+      //
 
       qDebug() << Q_FUNC_INFO << "Number of items:" << this->derived().count();
 
@@ -104,9 +109,15 @@ class BtComboBoxNamedEntityBase : public CuriouslyRecurringTemplateBase<BtComboB
 //         QString debugOutput;
 //         QTextStream debugOutputStream{&debugOutput};
 //         for (int ii = 0; ii < this->derived().count(); ++ii) {
+//            //
+//            // Annoyingly, QMetaType implements operator<< only for QDebug output stream (and only since Qt 6.5), and
+//            // doesn't have a toString() or similar member function.  However, we can lookup the names of the id()
+//            // values at https://doc.qt.io/qt-6/qmetatype.html#Type-enum, which is good enough for our diagnostic
+//            // purposes here.
+//            //
 //            QVariant data {this->derived().itemData(ii)};
 //            debugOutputStream <<
-//               "[" << ii << "] : (" << data.metaType() << ") " << data.typeName() << " : " << data.toString() << "\n";
+//               "[" << ii << "] : (" << data.metaType().id() << ") " << data.typeName() << " : " << data.toString() << "\n";
 //         }
 //         qDebug().noquote() << Q_FUNC_INFO << "Values:\n" << debugOutput;
 //      }