Print a warning in debug mode when 2 widgets added to the same parent…

… have the same name
texus · Nov 20, 2024 · ad18273 · ad18273
1 parent 01d8297
commit ad18273
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 6 deletions.
diff --git a/src/Container.cpp b/src/Container.cpp
@@ -307,9 +307,28 @@ namespace tgui
         TGUI_ASSERT(widgetPtr != nullptr, "Can't add nullptr to container");
 
         m_widgets.push_back(widgetPtr);
+
         if (!widgetName.empty())
             widgetPtr->setWidgetName(widgetName);
 
+#if !defined(NDEBUG) && !defined(TGUI_NO_RUNTIME_WARNINGS)
+        // TGUI_NEXT: Disallow multiple widgets with the same name (if name isn't empty). Throw exception if duplicate found.
+        // If the name passed to this function is empty then it's still possible that the widget already had a name,
+        // otherwise usedWidgetName will be the same value as widgetName.
+        const String& usedWidgetName = widgetPtr->getWidgetName();
+        if (!usedWidgetName.empty() && !usedWidgetName.starts_with("#TGUI_INTERNAL$"))
+        {
+            for (const auto& existingWidget : m_widgets)
+            {
+                if ((existingWidget != widgetPtr) && (existingWidget->getWidgetName() == usedWidgetName))
+                {
+                    TGUI_PRINT_WARNING("Multiple widgets with name '" + usedWidgetName + "' were added to the same parent.");
+                    break;
+                }
+            }
+        }
+#endif
+
         widgetAdded(widgetPtr);
 
         if (widgetPtr->getAutoLayout() != AutoLayout::Manual)

diff --git a/src/Widget.cpp b/src/Widget.cpp
@@ -1164,15 +1164,31 @@ namespace tgui
 
     void Widget::setWidgetName(const String& name)
     {
-        if (m_name != name)
+        if (m_name == name)
+            return;
+
+        m_name = name;
+
+        if (!m_parent)
+            return;
+
+        SignalManager::getSignalManager()->remove(this);
+        SignalManager::getSignalManager()->add(shared_from_this());
+
+#if !defined(NDEBUG) && !defined(TGUI_NO_RUNTIME_WARNINGS)
+        // TGUI_NEXT: Disallow multiple widgets with the same name (if name isn't empty). Throw exception if duplicate found.
+        if (!name.empty() && !name.starts_with("#TGUI_INTERNAL$"))
         {
-            m_name = name;
-            if (m_parent)
+            for (const auto& widget : m_parent->getWidgets())
             {
-                SignalManager::getSignalManager()->remove(this);
-                SignalManager::getSignalManager()->add(shared_from_this());
+                if ((widget.get() != this) && (widget->getWidgetName() == name))
+                {
+                    TGUI_PRINT_WARNING("Multiple widgets with name '" + name + "' were added to the same parent.");
+                    break;
+                }
             }
         }
+#endif
     }
 
     /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

diff --git a/tests/Container.cpp b/tests/Container.cpp
@@ -122,6 +122,9 @@ TEST_CASE("[Container]")
 
         SECTION("reusing name")
         {
+            // Ignore warnings about duplicate names being used
+            std::streambuf *oldbuf = std::cerr.rdbuf(nullptr);
+
             container->removeAllWidgets();
 
             auto child1 = tgui::Panel::create();
@@ -160,6 +163,17 @@ TEST_CASE("[Container]")
             // Removing the found indirect child means that there is no more widget with that name
             child1->remove(child4);
             REQUIRE(container->get("name") == nullptr);
+
+            // Changing the name of a widget affects which one is found
+            container->add(child2, "other");
+            container->add(child3, "name");
+            REQUIRE(container->get("name") == child3);
+            REQUIRE(container->get("other") == child2);
+            child2->setWidgetName("name");
+            REQUIRE(container->get("name") == child2);
+            REQUIRE(container->get("other") == nullptr);
+
+            std::cerr.rdbuf(oldbuf);
         }
     }
 

diff --git a/tests/Widget.cpp b/tests/Widget.cpp
@@ -576,7 +576,9 @@ TEST_CASE("[Widget]")
         REQUIRE(parent->getWidgets().size() == 1);
         parent->loadWidgetsFromFile("WidgetFileClickableWidget1.txt");
         REQUIRE(parent->getWidgets().size() == 1);
+        std::streambuf *oldbuf = std::cerr.rdbuf(nullptr); // Ignore warning about duplicate name being used
         parent->loadWidgetsFromFile("WidgetFileClickableWidget1.txt", false);
+        std::cerr.rdbuf(oldbuf);
         REQUIRE(parent->getWidgets().size() == 2);
     }
 

diff --git a/tests/Widgets/PanelListBox.cpp b/tests/Widgets/PanelListBox.cpp
@@ -138,8 +138,8 @@ TEST_CASE("[PanelListBox]")
         REQUIRE(panelListBox->getItemByIndex(0) == item3);
 
         const auto item5 = panelListBox->addItem("5");
-        const auto item5_second = panelListBox->addItem("5");
         const auto item6 = panelListBox->addItem("6");
+        const auto item7 = panelListBox->addItem("7");
         REQUIRE(panelListBox->getItemCount() == 4);
         panelListBox->removeAllItems();
         REQUIRE(panelListBox->getItemCount() == 0);