Fixed crash on reschange caused by very subtle pointer management issue; added various debug checks to make sure this doesn't happen again -.- those debug checks should have unnoticeable performance effect on a release build

git-svn-id: svn+ssh://svn.code.sf.net/p/supertuxkart/code/main/trunk@4376 178a84e3-b1eb-0310-8ba1-8eac791a3b58

src/guiengine/screen.cpp

@@ -42,6 +42,8 @@ using namespace GUIEngine;
 
 Screen::Screen(const char* file)
 {
+    m_magic_number = 0xCAFEC001;
+
     throttleFPS = true;
     
     m_mouse_x = 0;
@@ -55,6 +57,8 @@ Screen::Screen(const char* file)
 
 Screen::Screen()
 {
+    m_magic_number = 0xCAFEC001;
+
     m_mouse_x = 0;
     m_mouse_y = 0;
     m_loaded = false;
@@ -62,8 +66,20 @@ Screen::Screen()
     m_render_3d = false;
 }
 
+Screen::~Screen()
+{
+    assert(m_magic_number == 0xCAFEC001);
+    m_magic_number = 0xDEADBEEF;
+}
+
 void Screen::forgetWhatWasLoaded()
 {
+    assert(m_magic_number == 0xCAFEC001);
+    for (int n=0; n<m_widgets.size(); n++)
+    {
+        assert(m_widgets[n].m_magic_number == 0xCAFEC001);
+    }
+    
     m_loaded = false;
     m_inited = false;
     m_widgets.clearAndDeleteAll();
@@ -77,6 +93,7 @@ void Screen::forgetWhatWasLoaded()
 // -----------------------------------------------------------------------------
 void Screen::loadFromFile()
 {
+    assert(m_magic_number == 0xCAFEC001);
     IrrXMLReader* xml = irr::io::createIrrXMLReader( (file_manager->getGUIDir() + "/" + m_filename).c_str() );
     parseScreenFileDiv(xml, m_widgets);
     m_loaded = true;
@@ -86,6 +103,7 @@ void Screen::loadFromFile()
 /* small shortcut so this method can be called without arguments */
 void Screen::calculateLayout()
 {
+    assert(m_magic_number == 0xCAFEC001);
     // build layout
     calculateLayout( m_widgets );
 }
@@ -97,6 +115,7 @@ void Screen::calculateLayout()
  */
 void Screen::calculateLayout(ptr_vector<Widget>& widgets, Widget* parent)
 {
+    assert(m_magic_number == 0xCAFEC001);
     const unsigned short widgets_amount = widgets.size();
     
     // ----- read x/y/size parameters
@@ -264,6 +283,7 @@ void Screen::calculateLayout(ptr_vector<Widget>& widgets, Widget* parent)
 
 void Screen::addWidgets()
 {
+    assert(m_magic_number == 0xCAFEC001);
     if (!m_loaded) loadFromFile();
     
     addWidgetsRecursively( m_widgets );
@@ -312,6 +332,7 @@ void Screen::addWidgetsRecursively(ptr_vector<Widget>& widgets, Widget* parent)
  */
 void Screen::elementsWereDeleted(ptr_vector<Widget>* within_vector)
 {
+    assert(m_magic_number == 0xCAFEC001);
     if (within_vector == NULL) within_vector = &m_widgets;
     const unsigned short widgets_amount = within_vector->size();
     
@@ -330,11 +351,13 @@ void Screen::elementsWereDeleted(ptr_vector<Widget>* within_vector)
 // -----------------------------------------------------------------------------
 void Screen::manualAddWidget(Widget* w)
 {
+    assert(m_magic_number == 0xCAFEC001);
     m_widgets.push_back(w);
 }
 // -----------------------------------------------------------------------------
 void Screen::manualRemoveWidget(Widget* w)
 {
+    assert(m_magic_number == 0xCAFEC001);
     m_widgets.remove(w);
 }
 

src/guiengine/screen.hpp

@@ -92,6 +92,8 @@ namespace GUIEngine
         /** Will be called to determine if the 3D scene must be rendered when at this screen. */
         bool m_render_3d;
         
+        
+        int m_magic_number;
     public:
         bool throttleFPS;
         
@@ -107,11 +109,12 @@ namespace GUIEngine
         
         /** Next time this menu needs to be shown, don't use cached values, re-calculate everything.
             (useful e.g. on reschange, when sizes have changed and must be re-calculated) */
-        void forgetWhatWasLoaded();
+        virtual void forgetWhatWasLoaded();
         
         Screen(); /**< creates a dummy incomplete object; only use to override behaviour in sub-class */
         Screen(const char* filename);
-        virtual ~Screen(){}
+        virtual ~Screen();
+        
         bool operator ==(const char* filename) const { return m_filename == filename; }
         
         /** returns an object by name, or NULL if not found */

src/guiengine/widget.cpp

@@ -56,6 +56,8 @@ namespace GUIEngine
 // -----------------------------------------------------------------------------
 Widget::Widget(bool reserve_id)
 {
+    m_magic_number = 0xCAFEC001;
+    
     x = -1;
     y = -1;
     w = -1;
@@ -88,6 +90,8 @@ Widget::Widget(bool reserve_id)
 
 Widget::~Widget()
 {
+    assert(m_magic_number == 0xCAFEC001);
+    
     // If any player focused this widget, unset that focus
     for (int n=0; n<MAX_PLAYER_COUNT; n++)
     {
@@ -97,10 +101,13 @@ Widget::~Widget()
         }
     }
     
+    m_magic_number = 0xDEADBEEF;
 }
     
 void Widget::elementRemoved()
 {
+    assert(m_magic_number == 0xCAFEC001);
+    
     m_element = NULL;
     
     // If any player focused this widget, unset that focus
@@ -137,6 +144,7 @@ void Widget::resetIDCounters()
 // -----------------------------------------------------------------------------
 void Widget::add()
 {
+    assert(m_magic_number == 0xCAFEC001);
     if (m_reserve_id)
     {
         m_reserved_id = getNewID();
@@ -151,6 +159,8 @@ void Widget::add()
   */
 void Widget::setFocusForPlayer(const int playerID)
 {    
+    assert(m_magic_number == 0xCAFEC001);
+    
     // Unset focus flag on previous widget that had focus
     if (GUIEngine::getFocusForPlayer(playerID) != NULL)
     {
@@ -167,6 +177,8 @@ void Widget::setFocusForPlayer(const int playerID)
     
 void Widget::unsetFocusForPlayer(const int playerID)
 {
+    assert(m_magic_number == 0xCAFEC001);
+    
     if (m_player_focus[playerID]) this->unfocused(playerID);
     m_player_focus[playerID] = false;
 }
@@ -176,6 +188,8 @@ void Widget::unsetFocusForPlayer(const int playerID)
  */
 bool Widget::isFocusedForPlayer(const int playerID)
 {
+    assert(m_magic_number == 0xCAFEC001);
+    
     return m_player_focus[playerID];
 }
     
@@ -189,7 +203,7 @@ bool Widget::isFocusedForPlayer(const int playerID)
  *     Returns false if couldn't convert to either
  */
 bool Widget::convertToCoord(std::string& x, int* absolute /* out */, int* percentage /* out */)
-{
+{    
     bool is_number;
     int i;
     std::istringstream myStream(x);
@@ -211,6 +225,8 @@ bool Widget::convertToCoord(std::string& x, int* absolute /* out */, int* percen
     
 void Widget::move(const int x, const int y, const int w, const int h)
 {
+    assert(m_magic_number == 0xCAFEC001);
+    
     this->x = x;
     this->y = y;
     this->w = w;
@@ -226,6 +242,8 @@ void Widget::move(const int x, const int y, const int w, const int h)
  */
 void Widget::readCoords(Widget* parent)
 {
+    assert(m_magic_number == 0xCAFEC001);
+    
     /* determine widget position and size if not already done by sizers */
     std::string x       = m_properties[PROP_X];
     std::string y       = m_properties[PROP_Y];
@@ -360,6 +378,7 @@ void Widget::readCoords(Widget* parent)
 
 void Widget::setParent(IGUIElement* parent)
 {
+    assert(m_magic_number == 0xCAFEC001);
     m_parent = parent;
 }
 

src/guiengine/widget.hpp

@@ -93,6 +93,7 @@ namespace GUIEngine
     class Widget : public SkinWidgetContainer
     {
     protected:
+        int m_magic_number;
 
         friend class EventHandler;
         friend class RibbonWidget;

src/states_screens/kart_selection.cpp

@@ -684,13 +684,25 @@ KartSelectionScreen::KartSelectionScreen() : Screen("karts.stkgui")
 {
     g_dispatcher = new FocusDispatcher(this);
 }
-
+// -----------------------------------------------------------------------------
+void KartSelectionScreen::forgetWhatWasLoaded()
+{    
+    Screen::forgetWhatWasLoaded();
+    
+    // this pointer is no more valid
+    g_dispatcher = NULL;
+}
 // -----------------------------------------------------------------------------
 // Return true if event was handled successfully
 bool KartSelectionScreen::playerJoin(InputDevice* device, bool firstPlayer)
 {
     std::cout << "playerJoin() ==========\n";
 
+    if (g_dispatcher == NULL)
+    {
+        g_dispatcher = new FocusDispatcher(this);
+    }
+    
     DynamicRibbonWidget* w = this->getWidget<DynamicRibbonWidget>("karts");
     if (w == NULL)
     {
@@ -717,7 +729,7 @@ bool KartSelectionScreen::playerJoin(InputDevice* device, bool firstPlayer)
     {
         g_dispatcher->setRootID(kartsArea.m_reserved_id);
         g_dispatcher->add();
-        m_widgets.push_back(g_dispatcher);
+        if (!m_widgets.contains(g_dispatcher)) m_widgets.push_back(g_dispatcher);
 
         // We keep the ID of the "root" widget, see comment at top
         g_root_id = kartsArea.m_reserved_id;

src/states_screens/kart_selection.hpp

@@ -70,4 +70,7 @@ public:
     
     /** Standard 'Screen' callback every frame */
     void onUpdate(float dt, irr::video::IVideoDriver*);
+    
+    /** overload */
+    virtual void forgetWhatWasLoaded();
 };

src/utils/ptr_vector.hpp

@@ -129,10 +129,15 @@ bool contains( TYPE* instance ) const
 
 void clearAndDeleteAll()
 {
-    for(unsigned int n=0; n<contentsVector.size(); n++)
+    for (unsigned int n=0; n<contentsVector.size(); n++)
     {
         TYPE * pointer = contentsVector[n];
         delete pointer;
+        contentsVector[n] = (TYPE*)0xDEADBEEF;
+        
+        // When deleting, it's important that the same pointer cannot be
+        // twice in the vector, resulting in a double delete
+        assert( !contains(pointer) );
     }
     contentsVector.clear();
 }