Changeset 9290

Timestamp:

Jun 10, 2012, 11:01:40 PM (12 years ago)

Author:

landauf

Message:

Fixed crash with MSVC if a PickupCollection was used
A depleted CollectiblePickup is now destroyed instead of being dropped
A destroyed CollectiblePickup removes itself from the PickupCollection
PickupCollection has to use a list instead of a vector because of this reason
Also PickupCollectionIdentifier needed to be changed because the number of pickups in a collection may now change
Probably also fixed a bug in PickupCollectionIdentifier::compare() because it2 was not incremented

not completely clean yet

Location:

code/branches/presentation2012merge/src/modules/pickup

Files:

• CollectiblePickup.cc (modified) (3 diffs)
• CollectiblePickup.h (modified) (2 diffs)
• PickupCollection.cc (modified) (10 diffs)
• PickupCollection.h (modified) (3 diffs)
• PickupCollectionIdentifier.cc (modified) (4 diffs)
• PickupCollectionIdentifier.h (modified) (1 diff)

code/branches/presentation2012merge/src/modules/pickup/CollectiblePickup.cc

@@ -45 +45 @@
         Registers the object and initializes variables.
     */
-    CollectiblePickup::CollectiblePickup() : isInCollection_(false)
+    CollectiblePickup::CollectiblePickup() : collection_(NULL)
     {
         RegisterObject(CollectiblePickup);
-
-        this->collection_ = NULL;
     }
 
@@ -58 +56 @@
     CollectiblePickup::~CollectiblePickup()
     {
-
-    }
-
-    /**
-    @brief
-        Is called by OrxonoxClass::destroy() before the object is actually destroyed.
-    */
-    void CollectiblePickup::preDestroy(void)
-    {
-        this->Pickupable::preDestroy();
-
-        // The PickupCollection has to be destroyed as well.
-        if(this->isInCollection())
-            this->collection_->Pickupable::destroy();
-    }
-
-    /**
-    @brief
-        Destroys a Pickupable.
-    */
-    void CollectiblePickup::destroyPickup(void)
-    {
-        if(!this->isInCollection()) // If the CollectiblePickup is not in a PickupCollection the destroyPickup method of Pickupable is called.
-            this->Pickupable::destroyPickup();
-        else // Else the ColectiblePickup is dropped and disabled,
-        {
-            this->drop(false);
-            if(this->isInCollection() && this->isEnabled()) // It is only disabled if it is enabled and still ina PickupCollection after having been dropped.
-            {
-                this->setDisabled();
-                this->collection_->pickupDisabled();
-            }
-        }
+        if (this->isInCollection())
+            this->collection_->removePickupable(this);
     }
 
@@ -131 +98 @@
     /**
     @brief
-        Adds this CollectiblePickup to the input PickupCollection.
+        Notifies this CollectiblePickup that it was added to a PickupCollection.
     @param collection
         A pointer to the PickupCollection to which the CollectiblePickup should be added.
-    @return
-        Returns true if the CollectiblePickup was successfully added to the PickupCollection.
     */
-    bool CollectiblePickup::addToCollection(PickupCollection* collection)
+    void CollectiblePickup::wasAddedToCollection(PickupCollection* collection)
     {
-        if(this->isInCollection() || collection == NULL) //If the CollectiblePickup already is in a PickupCollection or if the input pointer is NULL.
-            return false;
-
-        this->isInCollection_ = true;
         this->collection_ = collection;
-        return true;
     }
 
     /**
     @brief
-        Removes this CollectiblePickup from its PickupCollection.
-    @return
-        Returns true if the CollectiblePickup was succcessfully removed.
+        Notifies this CollectiblePickup that it was removed from its PickupCollection.
     */
-    bool CollectiblePickup::removeFromCollection(void)
+    void CollectiblePickup::wasRemovedFromCollection(void)
     {
-        if(!this->isInCollection()) //If the CollectiblePickup is not in a PickupCollection.
-            return false;
-
-        this->isInCollection_ = false;
         this->collection_ = NULL;
-        return true;
     }
-
 }

code/branches/presentation2012merge/src/modules/pickup/CollectiblePickup.h

@@ -55 +55 @@
     class _PickupExport CollectiblePickup : public Pickupable
     {
+        friend class PickupCollection;
 
         public:
@@ -68 +69 @@
             */
             bool isInCollection(void) const
-                { return this->isInCollection_; }
-
-            bool addToCollection(PickupCollection* collection); //!< Adds this CollectiblePickup to the input PickupCollection.
-            bool removeFromCollection(void); //!< Removes this CollectiblePickup from its PickupCollection.
+                { return this->collection_ != NULL; }
 
             void carrierDestroyed(void); //!< Is called by the PickupCarrier when it is being destroyed.
 
-        protected:
-            virtual void preDestroy(void); //!< Is called by OrxonoxClass::destroy() before the object is actually destroyed.
-            virtual void destroyPickup(void); //!< Destroys a Pickupable.
+        private:
+            void wasAddedToCollection(PickupCollection* collection);
+            void wasRemovedFromCollection(void);
 
-        private:
-            bool isInCollection_; //!< True if the CollectiblePickup is in a PickupCollection.
             PickupCollection* collection_; //!< A pointer to the PickupCollection this CollectiblePickup is in.
-
     };
 }

code/branches/presentation2012merge/src/modules/pickup/PickupCollection.cc

@@ -70 +70 @@
         Destructor. Iterates through all Pickupables this PickupCollection consists of and destroys them if they haven't been already.
     */
-    PickupCollection::~ PickupCollection()
+    PickupCollection::~PickupCollection()
     {
         // Destroy all Pickupables constructing this PickupCollection.
-        for(std::vector<CollectiblePickup*>::iterator it = this->pickups_.begin(); it != this->pickups_.end(); it++)
-        {
-            (*it)->removeFromCollection();
-            (*it)->destroy();
+        for(std::list<CollectiblePickup*>::iterator it = this->pickups_.begin(); it != this->pickups_.end(); ++it)
+        {
+            (*it)->wasRemovedFromCollection();
+            (*it)->destroyPickup();
         }
         this->pickups_.clear();
@@ -93 +93 @@
 
         XMLPortObject(PickupCollection, CollectiblePickup, "pickupables", addPickupable, getPickupable, xmlelement, mode);
-
-        this->initializeIdentifier();
-    }
-
-    /**
-    @brief
-        Initializes the PickupIdentifier for this pickup.
-    */
-    void PickupCollection::initializeIdentifier(void)
-    {
-        for(std::vector<CollectiblePickup*>::iterator it = this->pickups_.begin(); it != this->pickups_.end(); it++)
-        {
-            this->pickupCollectionIdentifier_->addPickup((*it)->getPickupIdentifier());
-        }
     }
 
@@ -120 +106 @@
         this->processingUsed_ = true;
         // Change used for all Pickupables this PickupCollection consists of.
-        for(std::vector<CollectiblePickup*>::iterator it = this->pickups_.begin(); it != this->pickups_.end(); it++)
+        for(std::list<CollectiblePickup*>::iterator it = this->pickups_.begin(); it != this->pickups_.end(); ++it)
             (*it)->setUsed(this->isUsed());
 
@@ -157 +143 @@
 
         // Change the PickupCarrier for all Pickupables this PickupCollection consists of.
-        for(std::vector<CollectiblePickup*>::iterator it = this->pickups_.begin(); it != this->pickups_.end(); it++)
+        for(std::list<CollectiblePickup*>::iterator it = this->pickups_.begin(); it != this->pickups_.end(); ++it)
         {
             if(this->getCarrier() == NULL)
@@ -177 +163 @@
         this->processingPickedUp_ = true;
         // Change the pickedUp status for all Pickupables this PickupCollection consists of.
-        for(std::vector<CollectiblePickup*>::iterator it = this->pickups_.begin(); it != this->pickups_.end(); it++)
-            (*it)->setPickedUp(this->isPickedUp());
+        for(std::list<CollectiblePickup*>::iterator it = this->pickups_.begin(); it != this->pickups_.end(); )
+            (*(it++))->setPickedUp(this->isPickedUp());
 
         this->processingPickedUp_ = false;
@@ -220 +206 @@
         PickupCollection* pickup = orxonox_cast<PickupCollection*>(item);
         // Clone all Pickupables this PickupCollection consist of.
-        for(std::vector<CollectiblePickup*>::iterator it = this->pickups_.begin(); it != this->pickups_.end(); it++)
+        for(std::list<CollectiblePickup*>::iterator it = this->pickups_.begin(); it != this->pickups_.end(); ++it)
         {
             Pickupable* newPickup = (*it)->clone();
@@ -226 +212 @@
             pickup->addPickupable(collectible);
         }
-
-        pickup->initializeIdentifier();
     }
 
@@ -240 +224 @@
     bool PickupCollection::isTarget(const PickupCarrier* carrier) const
     {
-        for(std::vector<CollectiblePickup*>::const_iterator it = this->pickups_.begin(); it != this->pickups_.end(); it++)
+        for(std::list<CollectiblePickup*>::const_iterator it = this->pickups_.begin(); it != this->pickups_.end(); ++it)
         {
             if(!carrier->isTarget(*it))
@@ -274 +258 @@
             return false;
 
-        pickup->addToCollection(this);
         this->pickups_.push_back(pickup);
+        pickup->wasAddedToCollection(this);
         return true;
     }
@@ -289 +273 @@
     const Pickupable* PickupCollection::getPickupable(unsigned int index) const
     {
-        return this->pickups_[index];
+        unsigned int count = 0;
+        for (std::list<CollectiblePickup*>::const_iterator it = this->pickups_.begin(); it != this->pickups_.end(); ++it)
+        {
+            if (count == index)
+                return *it;
+            else
+                ++count;
+        }
+        return NULL;
+    }
+
+    /**
+    @brief
+        Removes the Pickup from the Collection.
+    @param pickup
+        The Pickup to be removed.
+    @return
+        Returns true if the pickup was in the collection.
+    */
+    bool PickupCollection::removePickupable(CollectiblePickup* pickup)
+    {
+        for(std::list<CollectiblePickup*>::iterator it = this->pickups_.begin(); it != this->pickups_.end(); ++it)
+        {
+            if (*it == pickup)
+            {
+                this->pickups_.erase(it);
+                pickup->wasRemovedFromCollection();
+                return true;
+            }
+        }
+        return false;
     }
 

code/branches/presentation2012merge/src/modules/pickup/PickupCollection.h

@@ -89 +89 @@
             bool addPickupable(CollectiblePickup* pickup); //!< Add the input Pickupable to list of Pickupables combined by this PickupCollection.
             const Pickupable* getPickupable(unsigned int index) const; //!< Get the Pickupable at the given index.
+            bool removePickupable(CollectiblePickup* pickup); //!< Removes the input Pickupable from the list of Pickupables in this PickupCollection.
+
+            inline const std::list<CollectiblePickup*>& getPickups() const
+                { return this->pickups_; }
 
             void pickupChangedUsed(bool changed); //!< Informs the PickupCollection, that one of its pickups has changed its used status to the input value.
@@ -95 +99 @@
 
         protected:
-            void initializeIdentifier(void); //!< Initializes the PickupIdentifier for this pickup.
-
             virtual bool createSpawner(void); //!< Facilitates the creation of a PickupSpawner upon dropping of the Pickupable.
 
@@ -105 +107 @@
             void changedPickedUpAction(void); //!< Helper method.
 
-            std::vector<CollectiblePickup*> pickups_; //!< The list of the pointers of all the Pickupables this PickupCollection consists of. They are weak pointers to facilitate testing, whether the pointers are still valid.
+            std::list<CollectiblePickup*> pickups_; //!< The list of the pointers of all the Pickupables this PickupCollection consists of. They are weak pointers to facilitate testing, whether the pointers are still valid.
 
             unsigned int usedCounter_; //!< Keeps track of the number of pickups of this PickupCollection, that are in use.

code/branches/presentation2012merge/src/modules/pickup/PickupCollectionIdentifier.cc

@@ -35 +35 @@
 
 #include "PickupCollectionIdentifier.h"
+#include "PickupCollection.h"
 
 namespace orxonox
@@ -43 +44 @@
         Constructor. Registers the object.
     */
-    PickupCollectionIdentifier::PickupCollectionIdentifier(Pickupable* pickup) : PickupIdentifier(pickup)
+    PickupCollectionIdentifier::PickupCollectionIdentifier(PickupCollection* collection) : PickupIdentifier(collection)
     {
         RegisterObject(PickupCollectionIdentifier);
+
+        this->collection_ = collection;
     }
 
@@ -80 +83 @@
 
         // If the number of Pickupables each of the two PickupCollectionIdentifiers contain differ, the one with less is considered smaller.
-        if(this->identifiers_.size() != collectionIdentifier->identifiers_.size())
-            return this->identifiers_.size()-collectionIdentifier->identifiers_.size();
+        if(this->collection_->getPickups().size() != collectionIdentifier->collection_->getPickups().size())
+            return this->collection_->getPickups().size()-collectionIdentifier->collection_->getPickups().size();
 
         // Compare the Pickupables of the two PickupCollectionIdentifiers one after the other. the one with the first 'smaller' one is considered smaller.
-        std::set<const PickupIdentifier*, PickupIdentifierCompare>::const_iterator it2 = collectionIdentifier->identifiers_.begin();
-        for(std::set<const PickupIdentifier*, PickupIdentifierCompare>::const_iterator it = this->identifiers_.begin(); it != this->identifiers_.end(); it++)
+        std::list<CollectiblePickup*>::const_iterator it1 = this->collection_->getPickups().begin();
+        std::list<CollectiblePickup*>::const_iterator it2 = collectionIdentifier->collection_->getPickups().begin();
+        for( ; it1 != this->collection_->getPickups().end(); ++it1, ++it2)
         {
+            const PickupIdentifier* id1 = (*it1)->getPickupIdentifier();
+            const PickupIdentifier* id2 = (*it2)->getPickupIdentifier();
 
-            if((*it)->compare(*it2) < 0)
+            if(id1->compare(id2) < 0)
                 return -1;
-            if((*it2)->compare(*it) < 0)
+            if(id2->compare(id1) < 0)
                 return 1;
         }
@@ -98 +104 @@
     }
 
-    /**
-    @brief
-        Add a Pickupable to the PickupCollectionIdentifier.
-    @param identifier
-        A pointer to the PickupIdentifier of the Pickupable to be added.
-    */
-    void PickupCollectionIdentifier::addPickup(const PickupIdentifier* identifier)
-    {
-        this->identifiers_.insert(identifier);
-    }
-
 }
 

code/branches/presentation2012merge/src/modules/pickup/PickupCollectionIdentifier.h

@@ -61 +61 @@
 
         public:
-            PickupCollectionIdentifier(Pickupable* pickup); //!< Constructor.
+            PickupCollectionIdentifier(PickupCollection* pickup); //!< Constructor.
             ~PickupCollectionIdentifier(); //!< Destructor.
 
             virtual int compare(const PickupIdentifier* identifier) const; //!< Compares a PickupCollectionIdentifier with a PickupIdentifier.
 
-            void addPickup(const PickupIdentifier* identifier); //!< Add a @ref orxonox::Pickupable "Pickupable" to the PickupCollectionIdentifier.
-
         private:
-            std::set<const PickupIdentifier*, PickupIdentifierCompare> identifiers_; //!< The set of PickupIdentifiers of the @ref orxonox::Pickupable "Pickupables", the @ref orxonox::PickupCollection "PickupCollection" with this PickupCollectionIdentifier consists of, ordered by the rule set by @ref orxonox::PickupIdentifierCompare "PickupIdentifierCompare".
+            PickupCollection* collection_;
 
     };