[Orxonox-commit 1064] r5785 - in code/branches/core5/src: libraries/core modules/gamestates orxonox orxonox/overlays orxonox/pickup

landauf at orxonox.net landauf at orxonox.net
Fri Sep 25 00:54:12 CEST 2009


Author: landauf
Date: 2009-09-25 00:54:12 +0200 (Fri, 25 Sep 2009)
New Revision: 5785

Modified:
   code/branches/core5/src/libraries/core/ClassTreeMask.h
   code/branches/core5/src/libraries/core/Iterator.h
   code/branches/core5/src/libraries/core/ObjectListIterator.h
   code/branches/core5/src/modules/gamestates/GSRoot.cc
   code/branches/core5/src/orxonox/Radar.cc
   code/branches/core5/src/orxonox/overlays/Map.cc
   code/branches/core5/src/orxonox/pickup/DroppedItem.cc
   code/branches/core5/src/orxonox/pickup/PickupSpawner.cc
Log:
Removed end-iterator-safety from Iterator and ObjectListIterator. This means, when you reach end(), don't use *it anymore (this returned 0 in the past, now you'll get a segfault).
Changed ClassTreeMask and Radar accordingly.

Also, please always use "for (...) { (it++)->function() }" instead of "for (...; ++it) { it->function() }" if there's a chance function() destroys the object pointed by the iterator. Our objectlists are deletionsafe, but you'll encounter strange problems like overleaped elements and run-over-end() situations if you use the second pattern and don't doublecheck the iterator in the loop.
Changed GSRoot accordingly.

Modified: code/branches/core5/src/libraries/core/ClassTreeMask.h
===================================================================
--- code/branches/core5/src/libraries/core/ClassTreeMask.h	2009-09-24 19:27:03 UTC (rev 5784)
+++ code/branches/core5/src/libraries/core/ClassTreeMask.h	2009-09-24 22:54:12 UTC (rev 5785)
@@ -264,9 +264,9 @@
             const ClassTreeMaskObjectIterator& operator++();
 
             /** @brief Returns true if the ClassTreeMaskObjectIterator points at the given object. @param pointer The pointer of the object */
-            inline bool operator==(BaseObject* pointer) const { return ((*this->objectIterator_) == pointer); }
+            inline bool operator==(BaseObject* pointer) const { return (this->objectIterator_ && (*this->objectIterator_) == pointer) || (!this->objectIterator_ && pointer == 0); }
             /** @brief Returns true if the ClassTreeMaskObjectIterator doesn't point at the given object. @param pointer The pointer of the object */
-            inline bool operator!=(BaseObject* pointer) const { return ((*this->objectIterator_) != pointer); }
+            inline bool operator!=(BaseObject* pointer) const { return (this->objectIterator_ && (*this->objectIterator_) != pointer) || (!this->objectIterator_ && pointer != 0); }
             /** @brief Returns true if the ClassTreeMaskObjectIterator hasn't already reached the end. */
             inline operator bool() const { return (this->objectIterator_); }
             /** @brief Returns the object the ClassTreeMaskObjectIterator currently points at. */

Modified: code/branches/core5/src/libraries/core/Iterator.h
===================================================================
--- code/branches/core5/src/libraries/core/Iterator.h	2009-09-24 19:27:03 UTC (rev 5784)
+++ code/branches/core5/src/libraries/core/Iterator.h	2009-09-24 22:54:12 UTC (rev 5785)
@@ -166,7 +166,6 @@
                 this->list_->registerIterator(this);
 
                 return (*this);
-                return *this;
             }
 
             /**
@@ -192,8 +191,7 @@
             */
             inline const Iterator<T>& operator++()
             {
-                if (this->element_)
-                    this->element_ = this->element_->next_;
+                this->element_ = this->element_->next_;
                 return *this;
             }
 
@@ -204,8 +202,7 @@
             inline Iterator<T> operator++(int i)
             {
                 Iterator<T> copy = *this;
-                if (this->element_)
-                    this->element_ = this->element_->next_;
+                this->element_ = this->element_->next_;
                 return copy;
             }
 
@@ -215,8 +212,7 @@
             */
             inline const Iterator<T>& operator--()
             {
-                if (this->element_)
-                    this->element_ = this->element_->prev_;
+                this->element_ = this->element_->prev_;
                 return *this;
             }
 
@@ -227,8 +223,7 @@
             inline Iterator<T> operator--(int i)
             {
                 Iterator<T> copy = *this;
-                if (this->element_)
-                    this->element_ = this->element_->prev_;
+                this->element_ = this->element_->prev_;
                 return copy;
             }
 
@@ -238,10 +233,7 @@
             */
             inline T* operator*() const
             {
-                if (this->element_)
-                    return orxonox_cast<T*>(this->element_->objectBase_);
-                else
-                    return 0;
+                return orxonox_cast<T*>(this->element_->objectBase_);
             }
 
             /**
@@ -250,10 +242,7 @@
             */
             inline T* operator->() const
             {
-                if (this->element_)
-                    return orxonox_cast<T*>(this->element_->objectBase_);
-                else
-                    return 0;
+                return orxonox_cast<T*>(this->element_->objectBase_);
             }
 
             /**

Modified: code/branches/core5/src/libraries/core/ObjectListIterator.h
===================================================================
--- code/branches/core5/src/libraries/core/ObjectListIterator.h	2009-09-24 19:27:03 UTC (rev 5784)
+++ code/branches/core5/src/libraries/core/ObjectListIterator.h	2009-09-24 22:54:12 UTC (rev 5785)
@@ -122,8 +122,7 @@
             */
             inline const ObjectListIterator<T>& operator++()
             {
-                if (this->element_)
-                    this->element_ = static_cast<ObjectListElement<T>*>(this->element_->next_);
+                this->element_ = static_cast<ObjectListElement<T>*>(this->element_->next_);
                 return *this;
             }
 
@@ -134,8 +133,7 @@
             inline ObjectListIterator<T> operator++(int i)
             {
                 ObjectListIterator<T> copy = *this;
-                if (this->element_)
-                    this->element_ = static_cast<ObjectListElement<T>*>(this->element_->next_);
+                this->element_ = static_cast<ObjectListElement<T>*>(this->element_->next_);
                 return copy;
             }
 
@@ -145,8 +143,7 @@
             */
             inline const ObjectListIterator<T>& operator--()
             {
-                if (this->element_)
-                    this->element_ = static_cast<ObjectListElement<T>*>(this->element_->prev_);
+                this->element_ = static_cast<ObjectListElement<T>*>(this->element_->prev_);
                 return *this;
             }
 
@@ -157,8 +154,7 @@
             inline ObjectListIterator<T> operator--(int i)
             {
                 ObjectListIterator<T> copy = *this;
-                if (this->element_)
-                    this->element_ = static_cast<ObjectListElement<T>*>(this->element_->prev_);
+                this->element_ = static_cast<ObjectListElement<T>*>(this->element_->prev_);
                 return copy;
             }
 
@@ -168,10 +164,7 @@
             */
             inline T* operator*() const
             {
-                if (this->element_)
-                    return this->element_->object_;
-                else
-                    return 0;
+                return this->element_->object_;
             }
 
             /**
@@ -180,10 +173,7 @@
             */
             inline T* operator->() const
             {
-                if (this->element_)
-                    return this->element_->object_;
-                else
-                    return 0;
+                return this->element_->object_;
             }
 
             /**

Modified: code/branches/core5/src/modules/gamestates/GSRoot.cc
===================================================================
--- code/branches/core5/src/modules/gamestates/GSRoot.cc	2009-09-24 19:27:03 UTC (rev 5784)
+++ code/branches/core5/src/modules/gamestates/GSRoot.cc	2009-09-24 22:54:12 UTC (rev 5785)
@@ -110,8 +110,8 @@
             Game::getInstance().requestState("ioConsole");
         }
 
-        for (ObjectList<TimerBase>::iterator it = ObjectList<TimerBase>::begin(); it; ++it)
-            it->tick(time);
+        for (ObjectList<TimerBase>::iterator it = ObjectList<TimerBase>::begin(); it; )
+            (it++)->tick(time);
 
         /*** HACK *** HACK ***/
         // Call the Tickable objects
@@ -121,8 +121,8 @@
             // just loaded
             leveldt = 0.0f;
         }
-        for (ObjectList<Tickable>::iterator it = ObjectList<Tickable>::begin(); it; ++it)
-            it->tick(leveldt * this->timeFactor_);
+        for (ObjectList<Tickable>::iterator it = ObjectList<Tickable>::begin(); it; )
+            (it++)->tick(leveldt * this->timeFactor_);
         /*** HACK *** HACK ***/
     }
 

Modified: code/branches/core5/src/orxonox/Radar.cc
===================================================================
--- code/branches/core5/src/orxonox/Radar.cc	2009-09-24 19:27:03 UTC (rev 5784)
+++ code/branches/core5/src/orxonox/Radar.cc	2009-09-24 22:54:12 UTC (rev 5785)
@@ -83,7 +83,10 @@
 
     const RadarViewable* Radar::getFocus()
     {
-        return *(this->itFocus_);
+        if (this->itFocus_)
+            return *(this->itFocus_);
+        else
+            return 0;
     }
 
     RadarViewable::Shape Radar::addObjectDescription(const std::string name)
@@ -100,7 +103,7 @@
     {
         SUPER(Radar, tick, dt);
 
-        if (this->focus_ != *(this->itFocus_))
+        if (this->itFocus_ && (this->focus_ != *(this->itFocus_)))
         {
             // focus object was deleted, release focus
             this->focus_ = 0;

Modified: code/branches/core5/src/orxonox/overlays/Map.cc
===================================================================
--- code/branches/core5/src/orxonox/overlays/Map.cc	2009-09-24 19:27:03 UTC (rev 5784)
+++ code/branches/core5/src/orxonox/overlays/Map.cc	2009-09-24 22:54:12 UTC (rev 5785)
@@ -289,7 +289,7 @@
 //Ogre::Entity * ent;// = mapSceneM_s->createEntity("ent1", "drone.mesh");
        for(ObjectList<orxonox::RadarViewable>::iterator it = ObjectList<orxonox::RadarViewable>::begin();
             it!=ObjectList<orxonox::RadarViewable>::end();
-            it++)
+            ++it)
         {
             //COUT(0) << "Radar_Position: " << it->getRVWorldPosition() << std::endl;
             //Ogre::SceneNode node = it->getMapNode();
@@ -391,7 +391,7 @@
     {
         for(ObjectList<orxonox::Map>::iterator it = ObjectList<orxonox::Map>::begin();
             it!=ObjectList<orxonox::Map>::end();
-            it++)
+            ++it)
         {
         //Map * m = it->getMap();
         //COUT(0) << it->isVisible_ << std::endl;

Modified: code/branches/core5/src/orxonox/pickup/DroppedItem.cc
===================================================================
--- code/branches/core5/src/orxonox/pickup/DroppedItem.cc	2009-09-24 19:27:03 UTC (rev 5784)
+++ code/branches/core5/src/orxonox/pickup/DroppedItem.cc	2009-09-24 22:54:12 UTC (rev 5785)
@@ -55,7 +55,7 @@
     {
         if (this->item_)
         {
-            for (ObjectList<Pawn>::iterator it = ObjectList<Pawn>::begin(); it != ObjectList<Pawn>::end(); it++)
+            for (ObjectList<Pawn>::iterator it = ObjectList<Pawn>::begin(); it != ObjectList<Pawn>::end(); ++it)
             {
                 Vector3 distance = it->getWorldPosition() - this->getWorldPosition();
                 if (distance.length() < this->triggerDistance_)

Modified: code/branches/core5/src/orxonox/pickup/PickupSpawner.cc
===================================================================
--- code/branches/core5/src/orxonox/pickup/PickupSpawner.cc	2009-09-24 19:27:03 UTC (rev 5784)
+++ code/branches/core5/src/orxonox/pickup/PickupSpawner.cc	2009-09-24 22:54:12 UTC (rev 5785)
@@ -126,7 +126,7 @@
     {
         if (this->isActive())
         {
-            for (ObjectList<Pawn>::iterator it = ObjectList<Pawn>::begin(); it != ObjectList<Pawn>::end(); it++)
+            for (ObjectList<Pawn>::iterator it = ObjectList<Pawn>::begin(); it != ObjectList<Pawn>::end(); ++it)
             {
                 Vector3 distance = it->getWorldPosition() - this->getWorldPosition();
                 if (distance.length() < this->triggerDistance_)




More information about the Orxonox-commit mailing list