Weird problem with code

j_mie6

Deity
Joined
Dec 20, 2009
Messages
2,963
Location
Bristol (uni)/Swindon (home)
I am working on a crusade feature for my next mod and I was doing some testing (without Civ4) and created this code:

Code:
#include <iostream>
#include <sstream>
#include <string>
#include <vector>

using namespace std;

class Crusade;

vector<Crusade*> lCrusades;

class Crusade
{
      /* Variables */
      /*CvCity**/ int pCity;
      /*CvPlayer**/ int pTarget;
      vector</*CvPlayer**/string> aParticipants;
      /*PromotionTypes*/const static int pCrusader = /*GC->getInfoTypeForString("PROMOTION_CRUSADER")*/ 1;
      
      /* Methods */
      public:
             Crusade (/*CvCity**/int , /*CvPlayer**/int , vector</*CvPlayer**/string>);  
             ~Crusade ();    
             /*CvPlayer**/int getTarget() {return pTarget;};
             /*CvCity**/int getCity() {return pCity;};
             vector</*CvPlayer**/string> getCrusaders() {return aParticipants;};
             static bool isPlayerTarget(/*CvPlayer**/int); 
             static void destroyCrusade(Crusade crusade);
};

Crusade::Crusade (/*CvCity**/int City, /*CvPlayer**/ int Target, vector</*CvPlayer**/string> Crusaders)
{
      pCity = City;
      pTarget = Target;
      for (int x = 0; x< Crusaders.size(); x++)
      {
          aParticipants.push_back(Crusaders[x]);
      }
      //aParticipants = Crusaders;
      lCrusades.push_back(this);
      
}

Crusade::~Crusade ()
{
      cout << "Crusade destroyed!" << endl;
      for (int x=0; x<aParticipants.size(); x++)
      {
            /*CvPlayer**/string pPlayer = aParticipants[x];
            cout << pPlayer;     
            /*for (int y=0; x<pPlayer->getNumUnits(); y++)
            {
                  CvUnit* pUnit = pPlayer->getUnit(y);
                  if(pUnit->isHasPromotion(pCrusader)
                  {
                        pUnit->setHasPromotion(pCrusader, false);
                  }    
            }*/
      }
      //send code to python to remove target graphics?
      /*
      call removeCrusaderTarget(pTarget), where this function is used by python to check for code in CvMainInterface.py, line 2804
      TBC!
      */
      for (int y=0; y<lCrusades.size(); y++)
      {
          if (lCrusades[y] == this)
          {
             lCrusades.erase(lCrusades.begin()+y-1);
          }
      }
      aParticipants.clear();    
}

void Crusade::destroyCrusade(Crusade crusade)
{
       Crusade * pCrusade;
       pCrusade = &crusade;
       delete pCrusade;
}

int main() //this would obviously not be there in the real code!
{
    vector<string> vCrusaders; vCrusaders.push_back("England"); vCrusaders.push_back("France");
    Crusade Crusade1 (1, 1, vCrusaders);
    vCrusaders.clear(); vCrusaders.push_back("Spain"); vCrusaders.push_back("Italy");
    Crusade Crusade2 (2, 2, vCrusaders);
    for (int i=0; i<lCrusades.size(); i++)
    {
        Crusade* pCrusade = lCrusades[i];
        cout << pCrusade->getTarget() << endl << "i = " << i << endl;

        for (int j=0; j<pCrusade->getCrusaders().size(); j++)
        {
             cout << pCrusade->getCrusaders()[j] << endl;
        }
    }
    Crusade::destroyCrusade(Crusade1);
    cout << "lCrusades.size() = " << lCrusades.size() << endl;
    for (int i=0; i<lCrusades.size(); i++)
    {
        Crusade* pCrusade = lCrusades[i];
        cout << pCrusade->getTarget() << endl;

        for (int j=0; j<pCrusade->getCrusaders().size(); j++)
        {
             cout << pCrusade->getCrusaders()[j] << endl;
        }
    }
    cin.get();
    return 0;
}


/*
Crusade::isPlayerTarget(CvPlayer* pPlayer)
{
      for(int x=0; x<lCrusades.size(); x++)
      {
              if (lCrusades[x]->getTarget() == pPlayer)
              {
                      return true;
              }
      }
}
*/

(please excuse the messiness! couldn't use the actual types!)

Am I right in thinking that this should have deleted Crusade1 and therefore the lCrusades for loop sould only display Crusade2?

if you read through the code should should get an idea of whats meant to happen... the console gives me back this:


Code:
1
i = 0
England
France
2
i = 1
Spain
Italy
Crusade destroyed!
EnglandFranceCrusade destroyed!
lCrusades.size() = 2
1
England
France
2
Spain
Italy

so definatly not working correctly!

any ideas from C++ programmers that actually know what they are meant to be doing?
 
Oh man, you should really read about quite a few C++ basics (the ones underlined below).
I'll give you here some information just to help you solve your problem, but I suggest you find a good C++ book (I imagine there are a few online ones).

The difference between the heap (freestore) and the stack:
When you do this:
Code:
Crusade Crusade1 (1, 1, vCrusaders);
You create an automatic variable (better known as 'a variable on the stack'). This means that it will be released automatically when you leave the current scope (in your case - the main() function), and that you must never explicitly release it with delete or free.

Passing arguments by value and by reference
This function:
Code:
void Crusade::destroyCrusade(Crusade crusade)
{
       Crusade * pCrusade;
       pCrusade = &crusade;
       delete pCrusade;
}

Receives an argument by value, which means it is copied to a local copy. 'crusade' is a new variable which is just a copy of the original one. One of the effects it has is that it has a different address and therefore a different 'this' value than the one which was passed as an argument.
And again, this is a variable on the stack, so calling delete here is bad.

Constructors - default constructor, copy constructor etc.
Also note that in the above function, when the Crusade object is copied, the copy constructor is called, so your constructor is not called for this object. Since you did not supply a copy constructor, the compiler created a default copy constructor for you, which does shallow copying. In this case it seems to be fine, but you should read about shallow copying vs. deep copying in this regard.


Static members initialization
This will not work when you add it to the Civ4 code:
Code:
const static PromotionTypes pCrusader = GC->getInfoTypeForString("PROMOTION_CRUSADER");

This is because `pCrusader` will be initialized long before the XML will be read, and it might even be before GC is initialized, so the value you'll get is undefined at best.


I know that's a lot to read about, but you'll be a better programmer and modder for that.
 
well, I fixed a lot of the problems the other day however I am experiencing a wierd one right now:

Code:
#include <iostream>
#include <sstream>
#include <string>
#include <vector>

using namespace std;

class Crusade;

vector<Crusade*> lCrusades;

class Crusade
{
      /* Variables */
      /*CvCity**/ int pCity;
      /*CvPlayer**/ int pTarget;
      vector</*CvPlayer**/string> aParticipants;
      /*PromotionTypes*/const static int pCrusader = /*GC->getInfoTypeForString("PROMOTION_CRUSADER")*/ 1;
      
      /* Methods */
      public:
             Crusade (/*CvCity**/int , /*CvPlayer**/int , vector</*CvPlayer**/string>);  
             ~Crusade ();    
             //static vector<Crusade*> lCrusades;
             /*CvPlayer**/int getTarget() {return pTarget;};
             /*CvCity**/int getCity() {return pCity;};
             vector</*CvPlayer**/string> getCrusaders() {return aParticipants;};
             static bool isPlayerTarget(/*CvPlayer**/int); 
             static void destroyCrusade(Crusade crusade);
};

Crusade::Crusade (/*CvCity**/int City, /*CvPlayer**/ int Target, vector</*CvPlayer**/string> Crusaders)
{
      pCity = City;
      pTarget = Target;
      for (int x = 0; x< Crusaders.size(); x++)
      {
          aParticipants.push_back(Crusaders[x]);
      }
      //aParticipants = Crusaders;
      lCrusades.push_back(this);
      
}

Crusade::~Crusade ()
{
      //cout << "Crusade destroyed!" << endl;
      for (int x=0; x<aParticipants.size(); x++)
      {
            /*CvPlayer**/string pPlayer = aParticipants[x];
            //cout << pPlayer << endl;     
            /*for (int y=0; x<pPlayer->getNumUnits(); y++)
            {
                  CvUnit* pUnit = pPlayer->getUnit(y);
                  if(pUnit->isHasPromotion(pCrusader)
                  {
                        pUnit->setHasPromotion(pCrusader, false);
                  }    
            }*/
      }
      //send code to python to remove target graphics?
      /*
      call removeCrusaderTarget(pTarget), where this function is used by python to check for code in CvMainInterface.py, line 2804
      TBC!
      */
      for (int y=0; y<lCrusades.size(); y++)
      {
          if (lCrusades[y] == this)
          {
             //int iLength = lCrusades.size();
             vector<Crusade*>::iterator it = lCrusades.begin();
             cout << y << endl << lCrusades[y]<< endl<< this << endl;
             cout << "erasing: " << *it << endl;
             lCrusades.erase(it+y);
             //cout << "erased!" << endl << iLength<< endl;
             //lCrusades.resize(iLength -1);
             //cout << lCrusades.size() << endl;
          }
      }
      //aParticipants.clear();    
}

void Crusade::destroyCrusade(Crusade crusade)
{
       Crusade * pCrusade;
       pCrusade = &crusade;
       delete pCrusade;
}

int main()
{
    vector<string> vCrusaders; vCrusaders.push_back("England"); vCrusaders.push_back("France");
    Crusade Crusade1 (1, 1, vCrusaders);
    vCrusaders.clear(); vCrusaders.push_back("Spain"); vCrusaders.push_back("Italy");
    Crusade Crusade2 (2, 2, vCrusaders);
    for (int i=0; i<lCrusades.size(); i++)
    {
        Crusade* pCrusade = lCrusades[i];
        cout << pCrusade->getTarget() << endl << "i = " << i << endl;

        for (int j=0; j<pCrusade->getCrusaders().size(); j++)
        {
             cout << pCrusade->getCrusaders()[j] << endl;
        }
    }
    //Crusade::destroyCrusade(Crusade1);
    Crusade * pCrusade;
    pCrusade = &Crusade1;
    delete pCrusade;
    cout << "lCrusades.size() = " << lCrusades.size() << endl;
    for (int i=0; i<lCrusades.size(); i++)
    {
        Crusade* pCrusade = lCrusades[i];
        cout << pCrusade->getTarget() << endl;

        for (int j=0; j<pCrusade->getCrusaders().size(); j++)
        {
             cout << pCrusade->getCrusaders()[j] << endl;
        }
    }
    cin.get();
    return 0;
}


/*
Crusade::isPlayerTarget(CvPlayer* pPlayer)
{
      for(int x=0; x<lCrusades.size(); x++)
      {
              if (lCrusades[x]->getTarget() == pPlayer)
              {
                      return true;
              }
      }
}
*/

I think it is something to do with deleting the object from the vector (causing runtime error I think). And do you think that the pCrusader should be called from the read methods?
 
so what do you suggest I do to remove this object from the total crusades vector? and why does deleting automatic variables cause errors?
 
so what do you suggest I do to remove this object from the total crusades vector?

Option 1: Don't handle adding / removing crusades to the vector in the constructor / destructor (which ties it to the object life time). Do it externally instead.

Option 2: Allocate the crusade objects dynamically (using new and delete). In any case, in the civ code you won't have access to a main function which calls everything, so you won't be able to store the objects on the stack while keeping them persistent.

I suggest doing both.

and why does deleting automatic variables cause errors?

here and here
 
ok this time I have done it how I plan to when I add it to the CvInfos.cpp:

Spoiler :

Code:
//Header
class CvCrusade
{
      public:
             CvCrusade();
             virtual ~CvCrusade();
             
             int getNumCrusaders() const;       // Exposed to Python
             CvPlayer* getCrusader(int) const;         //Exposed to Python
             CvPlayer* getTarget() const;             // Exposed to Python
             CvCity* getCity() const;           // Exposed to Python
             int getStartingTurn() const;           // Exposed to Python
             void addCrusader(CvPlayer*);           // Exposed to Python
             void setCity(CvCity*);           // Exposed to Python
             void setTarget(CvPlayer*);           // Exposed to Python
             void addPromotions(PromotionTypes);              // Exposed to Python
      
      private:
             void updateNumCrusaders(){iNumCrusaders++};           // Exposed to Python
             
      protected:
             int iNumCrusaders;
             CvPlayer* pTarget;
             int* aCrusaders;                   //should I be doing this?!
             CvCity* pTargetCity;
             int iStartingTurn;
};

Code:
//Source
CvCrusade::CvCrusade():
iNumCrusaders(0),
iStartingTurn(0)
{
}

CvCrusade::~CvCrusade()
{
       SAFE_DELETE_ARRAY(aCrusaders);
}

CvCrusade::getNumCrusaders()
{
       return iNumCrusaders;
}

CvCrusade::getCrusader(int iIndex)
{
       return GET_PLAYER(aCrusaders[iIndex]);
}

CvCrusade::getTarget()
{
       return pTarget;
}

CvCrusade::getCity()
{
       return pCity;
}

CvCrusade::getStartingTurn()
{
       return iStartingTurn;
}

CvCrusade::addCrusader(CvPlayer* pPlayer);
{
       int iPlayer = pPlayer->getID();
       updateNumCrusaders();
       // Asaf?
}

CvCrusade::setCity(CvCity* pCity)
{
       pTargetCity = pCity;
}

CvCrusade::setTarget(CvPlayer* pPlayer)
{
       pTarget = pPlayer;
}

CvCrusade::addPromotions(PromotionTypes ePromotion)
{
       for(int i; i<getNumCrusaders(); i++)
       {
               CvPlayer* pPlayer = getCrusader(i);
               for(int x; x<pPlayer->getNumUnits(); x++)
               {
                       CvUnit* pUnit = pPlayer->getUnit(x);
                       if( pUnit->getUnitInfo->getCombat() > 0 )
                       {
                              pUnit->setHasPromotion(ePromotion, true);
                       }
               }
       }
}


am I actually doing this right? Any comments?

also, how should I handle the adding of new crusaders to the array (or should I use a vector) , should I make a new one each time?
 
A couple things I noticed just taking a quick look.

CvCrusade::addCrusader(CvPlayer* pPlayer);
Shouldn't be a ; there.


for(int x; x<pPlayer->getNumUnits(); x++)
CvUnit* pUnit = pPlayer->getUnit(x);

Never do this! The units array is not rearranged when units die. So there can be "holes" in it. E.g.: you have four units, index 0, 1, 2 and 3. The one at index 1 dies and is removed from the game. You now have units at index 0, 2 and 3. Your code will attempt to give the units in index 0, 1 and 2 the promotion.

Instead, use
int iLoop;
for(pLoopUnit = firstUnit(&iLoop); pLoopUnit != NULL; pLoopUnit = nextUnit(&iLoop))
 
urg yeah I forgot about that.... thanks, how about the adding of the players to the array etc.

edit: whats pLoopUnit's declaration (CvUnit?) and shouldn't it be pPlayer->FirstUnit(&iLoop)

Code:
CvCrusade::addPromotions(PromotionTypes ePromotion)
{
       for(int i; i<getNumCrusaders(); i++)
       {
               CvPlayer* pPlayer = getCrusader(i);
               int iLoop;
               for(CvUnit* pUnit = pPlayer->firstUnit(&iLoop); pUnit != NULL; pUnit = pPlayer->nextUnit(&iLoop))
               {
                       if( pUnit->getUnitInfo->getCombat() > 0 )
                       {
                              pUnit->setHasPromotion(ePromotion, true);
                              pUnit->changeReligiousModifier(10);
                       }
               }
       }
}

edit edit:

should the removal of the promotion be handled in the destructor?
 
Might want to declare the variables outside of the loop, for speed. Not that I think it will matter much in this case but a good habit to have.
for(int i; i<getNumCrusaders(); i++)
Need to set i to zero.
Otherwise it looks fine.

Adding players: you will need to decide if you want an array or vector. If you are sure you will never want the ability for people to leave the crusade then I would suggest using an array of size MAX_CIV_PLAYERS. You know there's room for everyone (except barbarians, who I doubt you will allow to crusade :)). Then add the ID of the new civ joining at index getNumCrusaders(), since we know all the index before this are used.

If you plan on adding the ability for people to leave a crusade then you might want to make a vector instead. Alternatively you can use a bool array where each index represent the player with that ID. You would need to change your loop to MAX_CIV_PLAYERS and check if the index is set to true or false.
 
hmmm, I didn't think about actually leaving the crusade :hmm: surely this would work for that (but I am probably wrong)

if there is 3 people in the array that is MAX_CIV_PLAYERS big (is that declared in the header as: int* aCrusaders [MAX_CIV_PLAYERS];?), and I want to remove one couldn't I set it's value to NULL (and then in loops check if not NULL)? or would that cause overflow problems if the player was added again (which probably wouldn't be added again)

of course the bool option could work. However the problem with that is when getCrusader is called it would have to look like this:

Code:
CvCrusade::getCrusader(int iIndex)
{
       if (aCrusaders[iIndex])
       {
              return GET_PLAYER(iIndex);
       }
       return NULL;
}

which I think isn't the best way to go, as it could be problematic in the code. So, that leaves a vector. in civ, is a vector declared the same way (ie, already included)? so I could do:

vector<int*> aCrusaders;

in the header, and then use pushback to add and
Code:
CvCrusade::removeCrusader(CvPlayer* pPlayer)
{
       int iPlayer = pPlayer->getID();
       for(int x = 0; x<aCrusaders.size(); x++)
       {
               if(aCrusaders[x] == iPlayer)
               {              
                      aCrusaders.erase(aCrusaders.begin() + x - 1);
               }
       }
}
to delete.

how is vector destruction handled in the destructor? is it with DELETE_SAFE_ARRAY or aCrusaders.clear()?
 
If you made it a bool array your function should change to return true if the player checked is crusading and false if not. Then the code calling the function would handle the true and false case. But a vector it is...


Most (all?) vectors are declared (in the header files) like this:
std::vector<CvUnit *> m_aDifferentUnitCache;

and use .clear() to destroy them.


Don't forget to change updateNumCrusaders, or simply remove it and have getNumCrusaders() return aCrusaders.size()
 
ok so this is what I have now:

Spoiler :

Code:
//Header
class CvCrusade
{
      public:
             CvCrusade();
             virtual ~CvCrusade();
             
             int getNumCrusaders() const;       // Exposed to Python
             CvPlayer* getCrusader(int) const;         //Exposed to Python
             CvPlayer* getTarget() const;             // Exposed to Python
             CvCity* getCity() const;           // Exposed to Python
             int getStartingTurn() const;           // Exposed to Python
             void addCrusader(CvPlayer*);           // Exposed to Python
             void setCity(CvCity*);           // Exposed to Python
             void setTarget(CvPlayer*);           // Exposed to Python
             void setPromotions(PromotionTypes, CvPlayer*, bool);              // Exposed to Python
             void removeCrusader(CvPlayer*);              // Exposed to Python                
      
      private:
             void updateNumCrusaders(bool);
             
      protected:
             int iNumCrusaders;
             CvPlayer* pTarget;
             int* aCrusaders [MAX_CIV_PLAYERS];
             std::vector <int*> aCrusaders;
             CvCity* pTargetCity;
             int iStartingTurn;
};
Code:
//Source
CvCrusade::CvCrusade():
iNumCrusaders(0),
iStartingTurn(0)
{
}

CvCrusade::~CvCrusade()
{
       aCrusaders.clear()
       // add promotion removal/modifier removal here?
}

CvCrusade::getNumCrusaders()
{
       return iNumCrusaders;
}

CvCrusade::getCrusader(int iIndex)
{
       return GET_PLAYER(aCrusaders[iIndex]);
}

CvCrusade::getTarget()
{
       return pTarget;
}

CvCrusade::getCity()
{
       return pCity;
}

CvCrusade::getStartingTurn()
{
       return iStartingTurn;
}

CvCrusade::addCrusader(CvPlayer* pPlayer)
{
       int iPlayer = pPlayer->getID();
       setPromotions(gc.getInfoTypeForString("PROMOTION_CRUSADER"), pPlayer, true)
       aCrusaders.push_back( iPlayer );
       updateNumCrusaders(true);
}

CvCrusade::setCity(CvCity* pCity)
{
       pTargetCity = pCity;
}

CvCrusade::setTarget(CvPlayer* pPlayer)
{
       pTarget = pPlayer;
}

CvCrusade::setPromotions(PromotionTypes ePromotion, CvPlayer* pPlayer, bool bAdd)
{
       int iLoop;
       CvUnit* pUnit;
       for(pUnit = pPlayer->firstUnit(&iLoop); pUnit != NULL; pUnit = pPlayer->nextUnit(&iLoop))
       {
              if(pUnit->getUnitInfo->getCombat() > 0)
              {
                     pUnit->setHasPromotion(ePromotion, bAdd);
                     if(bAdd)
                     {
                            pUnit->changeReligiousModifier(10);
                     }
                     else
                     {
                            pUnit->changeReligiousModifier(-10);
                     }
              }
}

CvCrusade::removeCrusader(CvPlayer* pPlayer)
{
       int iPlayer = pPlayer->getID();
       for(int x = 0; x<aCrusaders.size(); x++)
       {
               if(aCrusaders[x] == iPlayer)
               {     
                      setPromotions(gc.getInfoTypeForString("PROMOTION_CRUSADER"), GET_PLAYER(iPlayer), false)         
                      aCrusaders.erase(aCrusaders.begin() + x - 1);
                      updateNumCrusaders(false);
               }
       }
}

CvCrusade::updateNumCrusaders(bool bAdded)
{
       if(bAdded)
       {
              iNumCrusaders++;
       }
       else
       {
              iNumCrusades--;
       }
}


so should I do the removal of promotions on the destructor? and in the constructor should I set iStartingTurn to be CvGame->getGameTurn()?
 
I think both of those will work fine. Just one thing:

int* aCrusaders [MAX_CIV_PLAYERS];
std::vector <int*> aCrusaders;

I guess you forgot to remove the int array?
 
oh yeah thanks. Also, any idea how the crusades can be stored? does CvGlobals or CvGame have built in arrays for this sort of stuff? Because I need to store each crusade that is initialized (preferably in a vector)
 
Just make your own vector storing CvCrusade pointers. CvGame is probably the best place.

To make it work with saving and loading the game, well it's not difficult but I can't think of a good way to explain it. Save how many crusades there are, then for each one call CvCrusade::write (which you make) and in that function save everything that need saving.

When reading the save first read how many crusades there are, then loop that many times adding a new crusade to your vector (which you clear first of course) and calling CvCrusade::read which should read the data back in the same order you wrote it to the save.
 
Ok, I will look through the read and writes for units. I kinda get what you are saying, so I think I can figure it out (if my code is wrong then feel free to correct me!)

So I should have an addCrusade(), getCrusade() and removeCrusade() in the CvGame where the removing method removes the crusade from the vector (and as I understand it calls the pointer's destructor which will remove the promotions, graphics etc). On that subject, is there a better way to delete from a vector than iterating through the values untill you find the correct one to delete?

edit: ok question, I am guessing the the read and write I am feeding in the values from that crusade, but how do I loop through them. I don't really understand how it needs to work.

edit edit:

these are my write and read methods:

Code:
CvCrusade::read(FDataStreamBase* pStream)
{
       reset();
       
       pStream->Read(&iNumCrusaders);
       pStream->Read(&pTarget);
       for(int i = 0; i < iNumCrusaders; i++) //is iNumCrusaders initialised already?
       {
              int iCrusader;
              pStream->read(&iCrusader);
              aCrusaders.push_back(iCrusader);
       }
       pStream->Read(&pTargetCity);
       pStream->Read(&iStartingTurn);

}

CvCrusade::write(FDataStreamBase* pStream)
{
       pStream->Write(iNumCrusaders);
       pStream->Write(pTarget);
       for(int i = 0; i < iNumCrusaders; i++)
       {
              pStream->Write(aCrusaders[i]);
       }
       pStream->Write(pTargetCity);
       pStream->Write(iStartingTurn);
}
}

and my constructor, destructor and reset methods (are these all used correctly, especially the pointers defaulting at NULL?)

Code:
CvCrusade::CvCrusade():
//iNumCrusaders(0),
//iStartingTurn(0)
{
       reset();
}

CvCrusade::~CvCrusade()
{
       for(int i = 0; i<getNumCrusaders(); i++)
       {
               removeCrusader(getCrusader(i))
       }
       
       aCrusaders.clear() //call for safety
}

CvCrusade::reset()
{
       iNumCrusaders = 0;
       pTarget = NULL;
       aCrusaders.clear();
       pTargetCity = NULL;
       iStartingTurn = 0;
}

but where are these called and how does it work for the CvGame stuff.... I save iNumCrusades then just save the instances into read (does that then write and read the crusade?) and is iNumCrusaders ready to be used in that read method?
 
Deleting them: how will you know one should be deleted? That will answer your question for how you go about deleting them. Looping through them may be needed.

"//is iNumCrusaders initialised already?"
At that point it is yes.

The read and write functions aren't called anywhere. You need to call them for each crusade. You call them on each object in the CvCrusade vector in CvGame or wherever you end up putting it. Before calling them you read/write how many crusades there are, so you know how many crusades there are to read and add to the vector. As you did with the aCrusaders vector.

Aside from addCrusade(), getCrusade() and removeCrusade() you will most likely want a getNumCrusades() function. But that one is easy, just return .size() of the vector.


I don't think your deconstructor will work as is. It should either start at the last index and go down to 0 or just remove index 0 until the vector is empty.
 
ah yeah of course :p so:
Code:
       for(int i = 0; i<getNumCrusaders(); i++)
       {
               removeCrusader(getCrusader(0));
       }

I suppose a while loop could also be used here :p

so in the CvGame read and write methods I need this code respectively:

Code:
       for(int i = 0; i < getNumCrusades(); i++)
       { 
              CvCrusade* pCrusade;
              pStream->Read(&pCrusade);
              pCrusade->read(pStream);
              addCrusade(pCrusade);
       } 
       
       for(int i = 0; i < getNumCrusades(); i++)
       {
              CvCrusade* pCrusade = getCrusade(i);
              pCrusade->write(pStream);
              pStream->Write(pCrusade);
       }

though I'm guessing the problem with this is I can't just pass in pStream into those methods or am I wrong?
 
Back
Top Bottom