Scripting: Fixed behavior of Dialog.SameWidgetRows

When a new row was created automatically when adding a widget, it was failing to remember the type of that widget, since addNewRow was called after setting m_lastWidgetTypeName. Now the last widget type is set in addDialogWidget instead. Also, we no longer compare the type names but just compare the QMetaObject pointers directly. Also fixed a problem in ScriptDialog::deleteAllDialogs, which iterated the sDialogInstances container while it was also getting modified as each dialog was deleted. Closes mapeditor#3607
bjorn · Mar 29, 2023 · 769bdbb · 769bdbb
1 parent cae1d65
commit 769bdbb
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 36 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,6 +1,7 @@
 ### Unreleased
 
 * Scripting: Added read-only access to Project properties (by dogboydog, #3622)
+* Scripting: Fixed behavior of Dialog.SameWidgetRows (#3607)
 * Fixed object labels to adjust to application font changes
 * Fixed grid rendering for odd Hex Side Length values (#3623)
 * Fixed tile stamp getting messed up on staggered maps in some cases (#3431)

diff --git a/src/tiled/scriptdialog.cpp b/src/tiled/scriptdialog.cpp
@@ -41,17 +41,15 @@
 #include <QSlider>
 #include <QTextEdit>
 
-#include <string.h>
 #include <memory>
 
-static int leftColumnStretch = 0;
+static const int leftColumnStretch = 0;
 // stretch as much as we can so that the left column looks as close to zero width as possible when there is no content
-static int rightColumnStretch = 1;
-static QString labelType = QString::fromUtf8("QLabel");
+static const int rightColumnStretch = 1;
 
 namespace Tiled {
 
-static QSet<ScriptDialog*> sDialogInstances;
+QSet<ScriptDialog*> ScriptDialog::sDialogInstances;
 
 static void deleteAllFromLayout(QLayout *layout)
 {
@@ -131,7 +129,10 @@ ScriptDialog::~ScriptDialog()
 
 void ScriptDialog::deleteAllDialogs()
 {
-    for (ScriptDialog *dialog : sDialogInstances)
+    QSet<ScriptDialog*> dialogToDelete;
+    dialogToDelete.swap(sDialogInstances);
+
+    for (ScriptDialog *dialog : std::as_const(dialogToDelete))
         dialog->deleteLater();
 }
 
@@ -293,6 +294,7 @@ QWidget *ScriptDialog::addDialogWidget(QWidget *widget, const QString &label)
     widget->setSizePolicy(QSizePolicy::Minimum, QSizePolicy::Fixed);
 
     m_rowLayout->addWidget(widget);
+    m_lastWidgetType = widget->metaObject();
     m_widgetsInRow++;
 
     return widget;
@@ -306,44 +308,38 @@ QWidget *ScriptDialog::addDialogWidget(QWidget *widget, const QString &label)
  */
 void ScriptDialog::determineWidgetGrouping(QWidget *widget)
 {
-    const char *widgetTypeNameCStr = widget->metaObject()->className();
-    QString widgetTypeName = QString::fromUtf8(widgetTypeNameCStr);
-
-    // any widgets that get checked via this method
-    // are right column widgets.
-    if (newRowMode() == NewRowMode::ManualRows)
-        return;
-
-    if (newRowMode() == NewRowMode::SingleWidgetRows) {
+    switch (newRowMode()) {
+    case SameWidgetRows: {
+        const QMetaObject *widgetType = widget->metaObject();
+
+        // labels can be mixed with any type of widget
+        if ((m_lastWidgetType != &QLabel::staticMetaObject &&
+             widgetType != &QLabel::staticMetaObject &&
+             m_lastWidgetType != nullptr &&
+             m_lastWidgetType != widgetType)) {
+            // if the new widget type is not the same as the last
+            addNewRow();
+        }
+        break;
+    }
+    case ManualRows:
+        // any widgets that get checked via this method
+        // are right column widgets.
+        break;
+    case SingleWidgetRows:
         // if there are any other widgets on this row so far in single widget
         // mode, wrap to the next row
         if (m_widgetsInRow > 1)
             addNewRow();
-        return;
-    }
-    // otherwise, same-type widget grouping
-
-    // labels can be mixed with any type of widget
-    if ((m_lastWidgetTypeName.compare(labelType) == 0 ||
-         m_lastWidgetTypeName.isEmpty()) &&
-            widgetTypeName.compare(labelType) != 0) {
-        // store the widget type to help determine if the next widget will
-        // start a new row
-        m_lastWidgetTypeName = widgetTypeName;
-    } else {
-        // if the new widget type is not the same as the last
-        if (widgetTypeName.compare(labelType) != 0 &&
-                widgetTypeName.compare(m_lastWidgetTypeName) != 0) {
-            addNewRow();
-        }
+        break;
     }
 }
 
 void ScriptDialog::addNewRow()
 {
     m_rowIndex++;
     m_widgetsInRow = 0;
-    m_lastWidgetTypeName.clear();
+    m_lastWidgetType = nullptr;
 }
 
 QLabel *ScriptDialog::newLabel(const QString &labelText)

diff --git a/src/tiled/scriptdialog.h b/src/tiled/scriptdialog.h
@@ -36,7 +36,7 @@ class ScriptImage;
 /**
  * A widget which allows the user to display a ScriptImage
  */
-class ScriptImageWidget: public QLabel
+class ScriptImageWidget : public QLabel
 {
     Q_OBJECT
 
@@ -58,7 +58,7 @@ class ScriptDialog : public QDialog
 
 public:
     enum NewRowMode {
-        SameWidgetRows =0,
+        SameWidgetRows = 0,
         ManualRows = 1,
         SingleWidgetRows = 2
     };
@@ -102,8 +102,10 @@ class ScriptDialog : public QDialog
     int m_widgetsInRow = 0;
     QGridLayout *m_gridLayout;
     QHBoxLayout *m_rowLayout;
-    QString m_lastWidgetTypeName;
+    const QMetaObject *m_lastWidgetType;
     NewRowMode m_newRowMode = SameWidgetRows;
+
+    static QSet<ScriptDialog*> sDialogInstances;
 };