Navigation improvements (#3325)

* Don't focus empty lists and ribbons

* Don't navigate to deactivated children of a ribbon

* Clarify log message

src/guiengine/event_handler.cpp

@@ -496,7 +496,8 @@ void EventHandler::navigate(const NavigationDirection nav, const int playerID)
 
 } // navigate
 
-/* This function use simple heuristic to find the closest widget
+/**
+ * This function use simple heuristic to find the closest widget
  * in the requested direction,
  * It prioritize widgets close vertically to widget close horizontally,
  * as it is expected behavior in any direction.
@@ -536,6 +537,22 @@ int EventHandler::findIDClosestWidget(const NavigationDirection nav, const int p
             (playerID != PLAYER_ID_GAME_MASTER && !w_test->m_supports_multiplayer))
             continue;
 
+        // Ignore empty ribbon widgets and lists
+        if (w_test->m_type == GUIEngine::WTYPE_RIBBON)
+        {
+            RibbonWidget* ribbon = dynamic_cast<RibbonWidget*>(w_test);
+            assert(ribbon != NULL);
+            if (ribbon->getActiveChildrenNumber(playerID) == 0)
+                continue;
+        }
+        else if (w_test->m_type == WTYPE_LIST)
+        {
+            ListWidget* list = (ListWidget*) w_test;
+            assert(list != NULL);
+            if (list->getItemCount() == 0)
+                continue;
+        }
+
         // if a dialog is shown, restrict to items in the dialog
         if (ScreenKeyboard::isActive())
         {

src/guiengine/widgets/ribbon_widget.cpp

@@ -593,31 +593,12 @@ EventPropagation RibbonWidget::moveToNextItem(const bool horizontally, const boo
     // Do nothing and do not block navigating out of the widget
     if (result == EVENT_BLOCK) return result;
 
-    if (reverse)
+    int old_selection = m_selection[playerID];
-        m_selection[playerID]--;
+    selectNextActiveWidget(horizontally, reverse, playerID, old_selection);
-    else
-        m_selection[playerID]++;
 
-    if (m_selection[playerID] >= int(m_active_children.size()) || m_selection[playerID] < 0)
+    if (m_selection[playerID] == old_selection && !horizontally)
-    {
+        return EVENT_BLOCK;
-        // In vertical tabs, don't loop when reaching the top or bottom
-        if (!horizontally)
-        {
-            if (reverse)
-                m_selection[playerID]++;
-            else
-                m_selection[playerID]--;
 
-            return EVENT_BLOCK;
-        }
-        bool left = (m_selection[playerID] < 0);
-
-        if (m_listener != NULL) m_listener->onRibbonWidgetScroll(left ? -1 : 1);
-
-        bool select_zero = (m_event_handler && left) || (!m_event_handler && !left);
-
-        m_selection[playerID] = select_zero ? 0 : m_active_children.size()-1;
-    }
     updateSelection();
 
     if (m_ribbon_type == RIBBON_COMBO || m_ribbon_type == RIBBON_TABS ||
@@ -666,6 +647,52 @@ EventPropagation RibbonWidget::propagationType(const bool horizontally)
     return result;
 }
 
+/**
+ * Move to the next child widget in the requested direction.
+ * If it is inactive, move again, until it finds an activated child or test all childs
+ */
+void RibbonWidget::selectNextActiveWidget(const bool horizontally, const bool reverse,
+                                          const int playerID, const int old_selection)
+{
+    int loop_counter = 0;
+    do
+    {
+        if (reverse)
+            m_selection[playerID]--;
+        else
+            m_selection[playerID]++;
+
+        if (m_selection[playerID] >= int(m_active_children.size()) || m_selection[playerID] < 0)
+        {
+            // In vertical tabs, don't loop when reaching the top or bottom
+            if (!horizontally)
+            {
+                if (reverse)
+                    m_selection[playerID] = old_selection;
+                else
+                    m_selection[playerID] = old_selection;
+
+                return;
+            }
+            bool left = (m_selection[playerID] < 0);
+
+            if (m_listener != NULL) m_listener->onRibbonWidgetScroll(left ? -1 : 1);
+
+            bool select_zero = (m_event_handler && left) || (!m_event_handler && !left);
+
+            m_selection[playerID] = select_zero ? 0 : m_active_children.size()-1;
+        }
+
+        loop_counter++;
+        if (loop_counter > m_active_children.size())
+        {
+            Log::warn("RibbonWidget", "All the buttons of the focused ribbon"
+                                      " are deactivated !");
+            break;
+        }
+    } while (!m_active_children.get(m_selection[playerID])->isActivated());
+}
+
 // ----------------------------------------------------------------------------
 
 EventPropagation RibbonWidget::focused(const int playerID)

src/guiengine/widgets/ribbon_widget.hpp

@@ -77,11 +77,13 @@ namespace GUIEngine
 
         /** Callbacks */
         virtual EventPropagation rightPressed(const int playerID=0) OVERRIDE;
-        virtual EventPropagation leftPressed(const int playerID=0) OVERRIDE;
+        virtual EventPropagation leftPressed (const int playerID=0) OVERRIDE;
-        virtual EventPropagation upPressed(const int playerID=0) OVERRIDE;
+        virtual EventPropagation upPressed   (const int playerID=0) OVERRIDE;
-        virtual EventPropagation downPressed(const int playerID=0) OVERRIDE;
+        virtual EventPropagation downPressed (const int playerID=0) OVERRIDE;
         EventPropagation moveToNextItem(const bool horizontally, const bool reverse, const int playerID);
         EventPropagation propagationType(const bool horizontally);
+        void selectNextActiveWidget(const bool horizontally, const bool reverse,
+                                    const int playerID, const int old_selection);
         virtual EventPropagation mouseHovered(Widget* child,
                                               const int playerID) OVERRIDE;
         virtual EventPropagation transmitEvent(Widget* w,