Fixes the menubar functionality when using SDL.

This resolves menu order, cleanup and close/re-open issues, as well as crashes on close.

It also modifies the look slightly to look closer to the windows menubar to keep a cohesive look regardless of platform.

Engine/source/gui/editor/guiMenuBar.cpp

@@ -795,14 +795,14 @@ GuiMenuBar::Menu* GuiMenuBar::sCreateMenu(const char *menuText, U32 menuId)
    return newMenu;
 }
 
-void GuiMenuBar::addMenu(GuiMenuBar::Menu *newMenu)
+void GuiMenuBar::addMenu(GuiMenuBar::Menu *newMenu, S32 pos)
 {
    // add it to the menu list
    menuBarDirty = true;
-   Menu **walk;
+   if (pos == -1)
-	for(walk = &menuList; *walk; walk = &(*walk)->nextMenu)
+      mMenuList.push_back(newMenu);
-      ;
+   else
-   *walk = newMenu;
+      mMenuList.insert(pos, newMenu);
 }
 
 void GuiMenuBar::addMenu(const char *menuText, U32 menuId)
@@ -817,16 +817,16 @@ GuiMenuBar::Menu *GuiMenuBar::findMenu(const char *menu)
 	if(dIsdigit(menu[0]))
 	{
 		U32 id = dAtoi(menu);
-		for(Menu *walk = menuList; walk; walk = walk->nextMenu)
+      for (U32 i = 0; i < mMenuList.size(); ++i)
-			if(id == walk->id)
+         if (id == mMenuList[i]->id)
-				return walk;
+            return mMenuList[i];
 		return NULL;
 	}
 	else
 	{
-		for(Menu *walk = menuList; walk; walk = walk->nextMenu)
+      for (U32 i = 0; i < mMenuList.size(); ++i)
-			if(!dStricmp(menu, walk->text))
+         if (!dStricmp(menu, mMenuList[i]->text))
-				return walk;
+            return mMenuList[i];
 		return NULL;
 	}
 }
@@ -854,16 +854,15 @@ void GuiMenuBar::removeMenu(Menu *menu)
 {
    menuBarDirty = true;
    clearMenuItems(menu);
-   for(Menu **walk = &menuList; *walk; walk = &(*walk)->nextMenu)
+
+   for (U32 i = 0; i < mMenuList.size(); ++i)
    {
-      if(*walk == menu)
+      if (mMenuList[i] == menu)
       {
-         *walk = menu->nextMenu;
+         mMenuList.erase(i);
          break;
       }
    }
-   dFree(menu->text);
-   delete menu;
 }
 
 void GuiMenuBar::removeMenuItem(Menu *menu, MenuItem *menuItem)
@@ -945,8 +944,26 @@ void GuiMenuBar::clearMenuItems(Menu *menu)
 
 void GuiMenuBar::clearMenus()
 {
-   while(menuList)
+   mMenuList.clear();
-      removeMenu(menuList);
+}
+
+void GuiMenuBar::attachToMenuBar(Menu* menu, S32 pos)
+{
+   addMenu(menu, pos);
+}
+
+void GuiMenuBar::removeFromMenuBar(Menu* menu)
+{
+   menuBarDirty = true;
+
+   for (U32 i = 0; i < mMenuList.size(); ++i)
+   {
+      if (mMenuList[i] == menu)
+      {
+         mMenuList.erase(i);
+         break;
+      }
+   }
 }
 
 //------------------------------------------------------------------------------
@@ -1083,7 +1100,7 @@ void GuiMenuBar::clearSubmenuItems(MenuItem *menuitem)
 
 GuiMenuBar::GuiMenuBar()
 {
-	menuList = NULL;
+   mMenuList.clear();
    menuBarDirty = true;
    mouseDownMenu = NULL;
    mouseOverMenu = NULL;
@@ -1140,9 +1157,9 @@ GuiMenuBar::Menu *GuiMenuBar::findHitMenu(Point2I mousePoint)
 {
    Point2I pos = globalToLocalCoord(mousePoint);
 
-   for(Menu *walk = menuList; walk; walk = walk->nextMenu)
+   for (U32 i = 0; i < mMenuList.size(); ++i)
-      if(walk->visible && walk->bounds.pointInRect(pos))
+      if (mMenuList[i]->visible && mMenuList[i]->bounds.pointInRect(pos))
-         return walk;
+         return mMenuList[i];
    return NULL;
 }
 
@@ -1153,35 +1170,35 @@ void GuiMenuBar::onPreRender()
    {
       menuBarDirty = false;
       U32 curX = mPadding;
-      for(Menu *walk = menuList; walk; walk = walk->nextMenu)
+      for (U32 i = 0; i < mMenuList.size(); ++i)
       {
-         if(!walk->visible)
+         if (!mMenuList[i]->visible)
             continue;
 
 		 // Bounds depends on if there is a bitmap to be drawn or not
-		 if(walk->bitmapIndex == -1)
+         if (mMenuList[i]->bitmapIndex == -1)
 		 {
             // Text only
-            walk->bounds.set(curX, 0, mProfile->mFont->getStrWidth(walk->text) + (mHorizontalMargin * 2), getHeight() - (mVerticalMargin * 2));
+            mMenuList[i]->bounds.set(curX, 0, mProfile->mFont->getStrWidth(mMenuList[i]->text) + (mHorizontalMargin * 2), getHeight() - (mVerticalMargin * 2));
 
          } else
 		 {
             // Will the bitmap and text be draw?
-            if(!walk->drawBitmapOnly)
+          if (!mMenuList[i]->drawBitmapOnly)
 			{
                // Draw the bitmap and the text
                RectI *bitmapBounds = mProfile->mBitmapArrayRects.address();
-			   walk->bounds.set(curX, 0, bitmapBounds[walk->bitmapIndex].extent.x + mProfile->mFont->getStrWidth(walk->text) + (mHorizontalMargin * 2), getHeight() + (mVerticalMargin * 2));
+               mMenuList[i]->bounds.set(curX, 0, bitmapBounds[mMenuList[i]->bitmapIndex].extent.x + mProfile->mFont->getStrWidth(mMenuList[i]->text) + (mHorizontalMargin * 2), getHeight() + (mVerticalMargin * 2));
 
 			} else
 			{
                // Only the bitmap will be drawn
                RectI *bitmapBounds = mProfile->mBitmapArrayRects.address();
-               walk->bounds.set(curX, 0, bitmapBounds[walk->bitmapIndex].extent.x + mBitmapMargin + (mHorizontalMargin * 2), getHeight() + (mVerticalMargin * 2));
+               mMenuList[i]->bounds.set(curX, 0, bitmapBounds[mMenuList[i]->bitmapIndex].extent.x + mBitmapMargin + (mHorizontalMargin * 2), getHeight() + (mVerticalMargin * 2));
 			}
 		 }
 
-         curX += walk->bounds.extent.x;
+         curX += mMenuList[i]->bounds.extent.x;
       }
 		mouseOverMenu = NULL;
 		mouseDownMenu = NULL;
@@ -1290,58 +1307,58 @@ void GuiMenuBar::onRender(Point2I offset, const RectI &updateRect)
    if (mProfile->mBorder)
       renderBorder(ctrlRect, mProfile);
 
-   for(Menu *walk = menuList; walk; walk = walk->nextMenu)
+   for (U32 i = 0; i < mMenuList.size(); ++i)
    {
-      if(!walk->visible)
+      if (!mMenuList[i]->visible)
          continue;
       ColorI fontColor = mProfile->mFontColor;
-      RectI bounds = walk->bounds;
+      RectI bounds = mMenuList[i]->bounds;
       bounds.point += offset;
       
       Point2I start;
 
-      start.x = walk->bounds.point.x + mHorizontalMargin;
+      start.x = mMenuList[i]->bounds.point.x + mHorizontalMargin;
-      start.y = walk->bounds.point.y + ( walk->bounds.extent.y - mProfile->mFont->getHeight() ) / 2;
+      start.y = mMenuList[i]->bounds.point.y + (mMenuList[i]->bounds.extent.y - mProfile->mFont->getHeight()) / 2;
 
 	  // Draw the border
-	  if(walk->drawBorder)
+      if (mMenuList[i]->drawBorder)
 	  {
         RectI highlightBounds = bounds;
         highlightBounds.inset(1,1);
-         if(walk == mouseDownMenu)
+        if (mMenuList[i] == mouseDownMenu)
             renderFilledBorder(highlightBounds, mProfile->mBorderColorHL, mProfile->mFillColorHL );
-         else if(walk == mouseOverMenu && mouseDownMenu == NULL)
+        else if (mMenuList[i] == mouseOverMenu && mouseDownMenu == NULL)
-            renderFilledBorder(highlightBounds, mProfile->mBorderColor, mProfile->mFillColor );
+           renderFilledBorder(highlightBounds, mProfile->mBorderColorHL, mProfile->mFillColorHL);
 	  }
 
 	  // Do we draw a bitmap?
-	  if(walk->bitmapIndex != -1)
+      if (mMenuList[i]->bitmapIndex != -1)
 	  {
-         S32 index = walk->bitmapIndex * 3;
+        S32 index = mMenuList[i]->bitmapIndex * 3;
-         if(walk == mouseDownMenu)
+        if (mMenuList[i] == mouseDownMenu)
             ++index;
-         else if(walk == mouseOverMenu && mouseDownMenu == NULL)
+        else if (mMenuList[i] == mouseOverMenu && mouseDownMenu == NULL)
             index += 2;
 
          RectI rect = mProfile->mBitmapArrayRects[index];
 
 		 Point2I bitmapstart(start);
-		 bitmapstart.y = walk->bounds.point.y + ( walk->bounds.extent.y - rect.extent.y ) / 2;
+       bitmapstart.y = mMenuList[i]->bounds.point.y + (mMenuList[i]->bounds.extent.y - rect.extent.y) / 2;
 
          drawUtil->clearBitmapModulation();
          drawUtil->drawBitmapSR( mProfile->mTextureObject, offset + bitmapstart, rect);
 
 		 // Should we also draw the text?
-		 if(!walk->drawBitmapOnly)
+         if (!mMenuList[i]->drawBitmapOnly)
 		 {
             start.x += mBitmapMargin;
       drawUtil->setBitmapModulation( fontColor );
-      drawUtil->drawText( mProfile->mFont, start + offset, walk->text, mProfile->mFontColors );
+      drawUtil->drawText(mProfile->mFont, start + offset, mMenuList[i]->text, mProfile->mFontColors);
 		 }
 	  } else
 	  {
       drawUtil->setBitmapModulation( fontColor );
-      drawUtil->drawText( mProfile->mFont, start + offset, walk->text, mProfile->mFontColors );
+      drawUtil->drawText(mProfile->mFont, start + offset, mMenuList[i]->text, mProfile->mFontColors);
 	  }
    }
 
@@ -1354,9 +1371,9 @@ void GuiMenuBar::buildWindowAcceleratorMap( WindowInputGenerator &inputGenerator
    // add all our keys:
    mCurAcceleratorIndex = 1;
 
-   for(Menu *menu = menuList; menu; menu = menu->nextMenu)
+   for (U32 i = 0; i < mMenuList.size(); ++i)
    {
-      for(MenuItem *item = menu->firstMenuItem; item; item = item->nextMenuItem)
+      for (MenuItem *item = mMenuList[i]->firstMenuItem; item; item = item->nextMenuItem)
       {
          if(!item->accelerator)
          {
@@ -1384,20 +1401,20 @@ void GuiMenuBar::acceleratorKeyPress(U32 index)
 {
    // loop through all the menus
    // and find the item that corresponds to the accelerator index
-   for(Menu *menu = menuList; menu; menu = menu->nextMenu)
+   for (U32 i = 0; i < mMenuList.size(); ++i)
    {
-      if(!menu->visible)
+      if (!mMenuList[i]->visible)
          continue;
 
-      for(MenuItem *item = menu->firstMenuItem; item; item = item->nextMenuItem)
+      for (MenuItem *item = mMenuList[i]->firstMenuItem; item; item = item->nextMenuItem)
       {
          if(item->acceleratorIndex == index)
          {
             // first, call the script callback for menu selection:
-            onMenuSelect_callback(menu->id, menu->text);
+            onMenuSelect_callback(mMenuList[i]->id, mMenuList[i]->text);
 			
             if(item->visible)
-               menuItemSelected(menu, item);
+               menuItemSelected(mMenuList[i], item);
             return;
          }
       }

Engine/source/gui/editor/guiMenuBar.h

@@ -133,7 +133,7 @@ public:
 	GuiSubmenuBackgroundCtrl *mSubmenuBackground; //  Background for a submenu
 	GuiMenuTextListCtrl *mSubmenuTextList;     //  Text list for a submenu
 
-	Menu *menuList;
+   Vector<Menu*> mMenuList;
    Menu *mouseDownMenu;
    Menu *mouseOverMenu;
 
@@ -164,7 +164,7 @@ public:
 	// internal menu handling functions
 	// these are used by the script manipulation functions to add/remove/change menu items
    static Menu* sCreateMenu(const char *menuText, U32 menuId);
-   void addMenu(Menu *menu);
+   void addMenu(Menu *menu, S32 pos = -1);
    void addMenu(const char *menuText, U32 menuId);
 	Menu *findMenu(const char *menu);  // takes either a menu text or a string id
 	static MenuItem *findMenuItem(Menu *menu, const char *menuItem); // takes either a menu text or a string id
@@ -175,6 +175,9 @@ public:
 	static void clearMenuItems(Menu *menu);
    void clearMenus();
 
+   void attachToMenuBar(Menu* menu, S32 pos = -1);
+   void removeFromMenuBar(Menu* menu);
+
    //  Methods to deal with submenus
    static MenuItem* findSubmenuItem(Menu *menu, const char *menuItem, const char *submenuItem);
    static MenuItem* findSubmenuItem(MenuItem *menuItem, const char *submenuItem);

Engine/source/platformSDL/menus/menuBarSDL.cpp

@@ -112,15 +112,37 @@ void MenuBar::updateMenuBar(PopupMenu *popupMenu /* = NULL */)
    GuiPlatformGenericMenuBar* menuBarGui = _FindMenuBarCtrl();
    popupMenu->mData->mMenuBar = this;
 
-   AssertFatal( dStrcmp( popupMenu->mData->mMenuGui->text, popupMenu->getBarTitle() ) == 0, "");
+   String menuTitle = popupMenu->getBarTitle();
-   GuiMenuBar::Menu* menuGui = menuBarGui->findMenu( popupMenu->getBarTitle() );
-   if(!menuGui)
-   {
-      menuBarGui->addMenu( popupMenu->mData->mMenuGui );
-      menuGui = menuBarGui->findMenu( popupMenu->getBarTitle() );
-   }
 
-   PlatformPopupMenuData::mMenuMap[ menuGui ] = popupMenu;
+   //Next, find out if we're still in the list of entries
+   SimSet::iterator itr = find(begin(), end(), popupMenu);
+
+   GuiMenuBar::Menu* menuGui = menuBarGui->findMenu(menuTitle);
+   if (!menuGui)
+   {
+      //This is our first time setting this particular menu up, so we'll OK it.
+      if (itr == end())
+         menuBarGui->attachToMenuBar(popupMenu->mData->mMenuGui);
+      else
+         menuBarGui->attachToMenuBar(popupMenu->mData->mMenuGui, itr - begin());
+   }
+   else
+   {
+      //Not our first time through, so we're really updating it.
+
+      //So, first, remove it from the menubar
+      menuBarGui->removeFromMenuBar(menuGui);
+
+      //Next, find out if we're still in the list of entries
+      SimSet::iterator itr = find(begin(), end(), popupMenu);
+
+      //if we're no longer in the list, we're pretty much done here
+      if (itr == end())
+         return;
+
+      //We're still here, so this is a valid menu for our current bar configuration, so add us back in.
+      menuBarGui->attachToMenuBar(menuGui, itr - begin());
+   }
 }
 
 //-----------------------------------------------------------------------------
@@ -154,17 +176,47 @@ void MenuBar::attachToCanvas(GuiCanvas *owner, S32 pos)
 
       mCanvas->setMenuBar( base );
    }
+
+   for (S32 i = 0; i < size(); ++i)
+   {
+      PopupMenu *mnu = dynamic_cast<PopupMenu *>(at(i));
+      if (mnu == NULL)
+      {
+         Con::warnf("MenuBar::attachToMenuBar - Non-PopupMenu object in set");
+         continue;
+      }
+
+      if (mnu->isAttachedToMenuBar())
+         mnu->removeFromMenuBar();
+
+      mnu->attachToMenuBar(owner, pos + i);
+   }
    
 }
 
 void MenuBar::removeFromCanvas()
 {
-   _FindMenuBarCtrl()->clearMenus();
+   if (mCanvas == NULL || !isAttachedToCanvas())
+      return;
+
+   //_FindMenuBarCtrl()->clearMenus();
+
+   // Add the items
+   for (S32 i = 0; i < size(); ++i)
+   {
+      PopupMenu *mnu = dynamic_cast<PopupMenu *>(at(i));
+      if (mnu == NULL)
+      {
+         Con::warnf("MenuBar::removeFromMenuBar - Non-PopupMenu object in set");
+         continue;
+      }
+
+      mnu->removeFromMenuBar();
+   }
 
    mCanvas->setMenuBar(NULL);
 
-   if(mCanvas == NULL || !isAttachedToCanvas())
+   mCanvas = NULL;
-      return;
 }
 
 #endif

Templates/Empty/game/tools/gui/guiPlatformGenericMenubar.ed.gui

@@ -5,8 +5,8 @@
    new GuiPlatformGenericMenuBar()
    {
       internalName = "menubar";
-      extent = "1024 32";
+      extent = "1024 20";
-      minExtent = "320 32";
+      minExtent = "320 20";
       horizSizing = "width";
       profile = "GuiMenuBarProfile";
    };

Templates/Empty/game/tools/gui/profiles.ed.cs

@@ -1067,8 +1067,10 @@ singleton GuiControlProfile( GuiCreatorIconButtonProfile )
 singleton GuiControlProfile( GuiMenuBarProfile )
 {
    fillcolor = "255 255 255";
-   borderColor = "0 0 0";
+   fillcolorHL = "213 231 248";
-   border = 1;
+   borderColor = "98 163 229";
+   borderColorHL = "122 177 232";
+   border = 0;
    borderThickness = 1;
    opaque = true;
    mouseOverSelected = true;

Templates/Full/game/tools/gui/guiPlatformGenericMenubar.ed.gui

@@ -5,8 +5,8 @@
    new GuiPlatformGenericMenuBar()
    {
       internalName = "menubar";
-      extent = "1024 32";
+      extent = "1024 20";
-      minExtent = "320 32";
+      minExtent = "320 20";
       horizSizing = "width";
       profile = "GuiMenuBarProfile";
    };

Templates/Full/game/tools/gui/profiles.ed.cs

@@ -1067,8 +1067,10 @@ singleton GuiControlProfile( GuiCreatorIconButtonProfile )
 singleton GuiControlProfile( GuiMenuBarProfile )
 {
    fillcolor = "255 255 255";
-   borderColor = "0 0 0";
+   fillcolorHL = "213 231 248";
-   border = 1;
+   borderColor = "98 163 229";
+   borderColorHL = "122 177 232";
+   border = 0;
    borderThickness = 1;
    opaque = true;
    mouseOverSelected = true;