Thread: Vector Question

  1. #16
    Registered User jlou's Avatar
    Join Date
    Jul 2003
    Posts
    1,090
    Quote Originally Posted by swoopy
    Jlou, that first quote is from Hunter2.
    Thanks... all fixed. That'll teach me to try to quote four different posts at the same time.

  2. #17
    Registered User
    Join Date
    Mar 2004
    Posts
    180
    Alright, I'm new to classes, and C++ in general, so what is this Copy thing everyone keeps talking about?

    In my opinion, it is a better idea to make sure your copy constructor/assignment operator is correct (if it is necessary), than it is to store pointers in the vector. Obviously, if the object is big, or copying is expensive, then storing pointers is a good idea, but for a simple class I would think that avoiding the headaches of dynamically allocated memory is preferred.
    Also, ya I do want to use pointers, I find them *normaly* esier too work with!

    I fixed that size, thing, but this code still doesn't work. andy ideas?

    Code:
    std::vector<User_Laser*> laser; //this is globaly declared
    ...
    
    // fires a regular shot
    if (keys.laser == true)
    {
            laser.push_back(new User_Laser(User));  //makes new laser
    
            laser[laser.size()]->y1=User->y1;
            laser[laser.size()]->x1 = User->x1 + User->Ship_Sprite->Width / 2;
    }
    
    Again:    //erases all obsolete lasers
    b=0;
    temp = laser.size();
    
    for(b = 0; b <= laser.size(); b++)
    {
            if(laser[b]->Obsolete == true)
            {
              delete laser[b];
              laser.erase(laser.begin() + b);
              goto Again;
            }
    }
    
    for(int x = 0; x<= laser.size(); x++)
    {
            laser[x]->regShot();
            laser[x]->Collision(Baddy, laser);       //check for collision between the laser and *a* baddy
    }
    
    for(int a=0; a<= laser.size(); a++)
    {
    
            laser[a]->Collision(Baddy, laser);       //check for collision between the laser and *a* baddy
    
            if(laser[a]->y1 <= 0)
            {
                    laser[a]->Obsolete = true;
            }
    }
    it raises exceptions here:
    Code:
    if(laser[b]->Obsolete == true)
    Cheers

  3. #18
    mustang benny bennyandthejets's Avatar
    Join Date
    Jul 2002
    Posts
    1,401
    You still haven't learnt have you:
    Code:
    for(b = 0; b <= laser.size(); b++)
    {
    By letting 'b' equal laser.size(), you're overstepping the vector bounds. Same problem, different thread. Change it to this:
    Code:
    for(b = 0; b < laser.size(); b++)
    {
    [email protected]
    Microsoft Visual Studio .NET 2003 Enterprise Architect
    Windows XP Pro

    Code Tags
    Programming FAQ
    Tutorials

  4. #19
    Carnivore ('-'v) Hunter2's Avatar
    Join Date
    May 2002
    Posts
    2,879
    >>laser[laser.size()]->y1=User->y1;
    That's also overstepping the bounds. The last element in laser would be at (laser.size() - 1). But even better, you should probably use laser.back()->y1 for the sake of betterness.
    Just Google It. √

    (\ /)
    ( . .)
    c(")(") This is bunny. Copy and paste bunny into your signature to help him gain world domination.

  5. #20
    mustang benny bennyandthejets's Avatar
    Join Date
    Jul 2002
    Posts
    1,401
    Good point Hunter.

    I think Death_Wraith needs a 101 in arrays.
    Code:
    char array[5];
    array[5]=5; //Access violation, overstepping array bounds
    
    array[0]; //First element in array
    array[4]; //Last element in array
    Or more generally:
    Code:
    char array[SIZE];
    
    char array[0]; //First element
    char array[SIZE-1]; //Last element
    [email protected]
    Microsoft Visual Studio .NET 2003 Enterprise Architect
    Windows XP Pro

    Code Tags
    Programming FAQ
    Tutorials

  6. #21
    Registered User
    Join Date
    Mar 2004
    Posts
    180
    >>I think Death_Wraith needs a 101 in arrays.

    lol......ya....myb....i have to admit reading back, i was being kinda dumb! now i just feel dumb....I have to admit, I didn't relize that the vectors were like arrays! Stupid me! well thx guys, i'll try that 2night!

    cheers

    DW

    *please don't hurt meeeee...... *

  7. #22
    mustang benny bennyandthejets's Avatar
    Join Date
    Jul 2002
    Posts
    1,401
    I'm just glad you know now.
    [email protected]
    Microsoft Visual Studio .NET 2003 Enterprise Architect
    Windows XP Pro

    Code Tags
    Programming FAQ
    Tutorials

  8. #23
    Registered User
    Join Date
    Mar 2004
    Posts
    180
    Well I *do* know now, but I still have one *last* (hopefully) question:

    that code doesn't work. even with
    Code:
    b < laser.Size();

    do I need to do somthing else before i can use this for loop? Because the vector is declared, its size should be 0 right? so this for loop should go from b = 0, b< 0, and nothing should happen, but it still raises exceptions when b= 0, or am i missing the point again?

    Code:
    for(b = 0; b < laser.size(); b++)
    {
            if(laser[b]->Obsolete == true)
            {
              delete laser[b];
              laser.erase(laser.begin() + b);
              goto Again;
            }
    }

    and temp still = 92 for some reason. no matter what I do. What gives with this?

    cheers,

    DW

  9. #24
    mustang benny bennyandthejets's Avatar
    Join Date
    Jul 2002
    Posts
    1,401
    but it still raises exceptions when b= 0
    If it raises an exception here, maybe you're trying to access the vector before it's initialized. Check for logic flaws in the program flow.
    [email protected]
    Microsoft Visual Studio .NET 2003 Enterprise Architect
    Windows XP Pro

    Code Tags
    Programming FAQ
    Tutorials

  10. #25
    Registered User
    Join Date
    Oct 2001
    Posts
    2,934
    > laser[laser.size()]->y1=User->y1;
    > laser[laser.size()]->x1 = User->x1 + User->Ship_Sprite->Width / 2;

    You did fix these lines as Hunter2 showed?

  11. #26
    Registered User
    Join Date
    Mar 2004
    Posts
    180
    Quote Originally Posted by bennyandthejets
    If it raises an exception here, maybe you're trying to access the vector before it's initialized. Check for logic flaws in the program flow.
    well thats what I thought, but the flows I think.

    Its declared globaly, athe top of my code, then in the game loop is where its used. i'll play around with it too day, maybe see if I can put it in the form load.

  12. #27
    Carnivore ('-'v) Hunter2's Avatar
    Join Date
    May 2002
    Posts
    2,879
    >>goto Again;
    *screams and falls dead to the floor* Be very very sure that you're not screwing anything up on that line.

    >>If it raises an exception here, maybe you're trying to access the vector before it's initialized.
    It shouldn't, the loop won't even run if size() is 0. Unless, as swoopy mentioned, something before that line is causing the error.
    Just Google It. √

    (\ /)
    ( . .)
    c(")(") This is bunny. Copy and paste bunny into your signature to help him gain world domination.

  13. #28
    Registered User
    Join Date
    Mar 2004
    Posts
    180
    Quote Originally Posted by Hunter2
    >>goto Again;
    aye i know i know! Thats the only way i could think out it


    Quote Originally Posted by Hunter2
    It shouldn't, the loop won't even run if size() is 0.
    thats what I thought... well like I said, i'll try to fix that 2day

  14. #29
    Registered User
    Join Date
    Jan 2003
    Posts
    311
    I don't really see why you need to have a vector of pointers to lasers, rather than a simple vector of lasers. If you really need a vector of pointers, consider a vector of boost::shared_pointers at least. In addition erase() is an expensive operation for a vector, but pop_back() is very cheap. Thus we can have a much faster version to remove lasers so long as we don't preserve order.

    Code:
    typedef std::vector<User_Laser> vlaser_t;
    vlaser_t lasers;
    ...
    
    // fires a regular shot
    if (keys.laser == true)
    {
            User_Laser tmp(User);
            tmp.y1 = User->y1; // User_Lasers ctor should probably do this
            tmp.x1 = User->x1 + User->Ship_Sprite->Width / 2;
            laser.push_back(tmp);  //makes new laser
    }
    
    for(vlaser_t::size_type i=0;i < lasers.size();) {
        if(lasers[i].Obsolete) {
            std::swap(lasers[i],lasers.back());
            lasers.pop_back();
        } else ++i;
    }
    Note that you save even more time if rather than have an Obsolete field you simply remove a laser from the list every time you would normally set Obsolete to true.

  15. #30
    Carnivore ('-'v) Hunter2's Avatar
    Join Date
    May 2002
    Posts
    2,879
    >>I don't really see why you need to have a vector of pointers to lasers, rather than a simple vector of lasers.
    Depending on the class implementation, you might.

    >>std::swap(lasers[i],lasers.back());
    Won't that only work if laser has a suitable operator= defined?

    While we're talking about erase-efficiency, why not use a std::list if you're just looping through the lasers?
    Just Google It. √

    (\ /)
    ( . .)
    c(")(") This is bunny. Copy and paste bunny into your signature to help him gain world domination.

Popular pages Recent additions subscribe to a feed