Jump to content
  • Advertisement
Sign in to follow this  
ICanC

C++ buffer or vector issue

Recommended Posts

I'm looking for a bit of help please, I have a merchant class that you can buy from and sell to. The merchant has his own vector of struct 'items' as does the player. To begin the transactions, a merchants menu function is called, passing in the address of a player (character class)

The below code works perfectly as expected, (e.g. if you haven't got enough gold, it states it correctly, or if either person doesn't have the item, etc) but as soon as a transaction occurs, like the player selling something, any attempts to sell anything else will show no message and just show the ".1. Back" option, or trying to buy something will say twice the merchant doesn't have it, or if he does, it will say he doesn't have it, but then say you bought it... it's probably something really simple and bad design on my part, but I can't figure it out

 

void merchant::buyfrom(character *a)
{
    print("\nItem to buy: "); 
    string item1; 
    fflush(stdin); 
    getline(cin, item1);
    vector<item>::iterator it;
    for (it = this->items.begin(); it != this->items.end(); ++it)
    {
        if (it->name == item1)
        {
            if (it->price > a->gold)
            {
                print("\n\nYou don't have enough gold to buy the "); cout << it->name; break;
            }
            print("\n\nYou bought "); std::cout << item1; print(" for "); std::cout << it->price; print(" gold");
            this->items.erase(it);
            a->inventory.push_back(*it);
            a->gold -= it->price;
            this->gold += it->price;
            break;
        }
    else
    {
            print("\n\nMerchant does not have a "); std::cout << item1; print(" to sell");
    }
    }
    print("\n\n\n1. Back\n\n\n> "); 
   char c; 
   std::cin >> c;
}

void merchant::sellto(character *a)
{
    print("\nItem to sell: ");
   string item1; 
   fflush(stdin); 
   getline(cin, item1);
    vector<item>::iterator it;
    for (it = a->inventory.begin(); it != a->inventory.end(); ++it)
    {
        if (it->name == item1)
        {
            print("\n\nYou sold "); std::cout << item1; print(" for "); std::cout << it->price; print(" gold");
            a->inventory.erase(it);
            this->items.push_back(*it);
            a->gold += it->price;
            this->gold -= it->price;
            break;
        }
        else
        {
            print("\nYou do not have "); std::cout << item1; print(" to sell");
        }
    }
    print("\n\n\n1. Back\n\n\n> "); 
  char c; 
  std::cin >> c;
}

 

Share this post


Link to post
Share on other sites
Advertisement

Not entirely sure, but the moment you call erase on an vector iterator the iterator is invalid. It happens that you seem to be able to access it's content, but you really shouldn't.

Put the erase call as last code before the break.

 

Share this post


Link to post
Share on other sites
2 hours ago, Endurion said:

Not entirely sure, but the moment you call erase on an vector iterator the iterator is invalid. It happens that you seem to be able to access it's content, but you really shouldn't.

Put the erase call as last code before the break.

Or simply put it as "it = items.erase(it);"  Also, you don't need to use this->, I personally hate typing extra and making it harder to read the code but that is a personal thing.

Share this post


Link to post
Share on other sites

Thanks, i've put the erase just before the break but the problem still occurs. I guess if there isn't anything obviously wrong the bug must be on the menu function which calls the above 2 functions? Or possibly the item struct?

Share this post


Link to post
Share on other sites

Well, I'd assume the bug is that you're changing the loop index "it" inside the loop.  Can you really reason correctly about the loop invariant if the loop invariant changes in the middle of the loop?

You can see the effects of calling std::vector::erase here.  Pay special attention to the bit that says Invalidates iterators and references at or after the point of the erase, including the end() iterator.

Share this post


Link to post
Share on other sites

Thanks for all the replies. I've fixed the iterator issue, and the actual cause of the problem was something to do with the messages not being properly conditioned, adding in a few ifs below fixed the issue

 

void merchant::buyfrom(character *a)
{
    print("\nItem to buy: "); string item1; getline(cin, item1);
    vector<item>::iterator it;
    unsigned vector_size = this->items.size();
    int check = 0; // to handle merchant does not have msg with not enough gold msg
    for (it = this->items.begin(); it != this->items.end();)
    {
        if (it->name == item1)
        {
            if (it->price > a->gold)
            {
                print("\n\nYou don't have enough gold to buy the "); cout << it->name; check = 1; break;
            }
            print("\n\nYou bought "); std::cout << item1; print(" for "); std::cout << it->price; print(" gold");
            a->inventory.push_back(*it);
            a->gold -= it->price;
            this->gold += it->price;
            this->items.erase(it);
            break;
        }
    else
    {
            ++it;
    }
    }
    if (this->items.size() == vector_size && check == 0)
    {
    print("\n\nMerchant does not have a "); std::cout << item1; print(" to sell");
    }
    print("\n\n\n1. Back\n\n\n> "); char c; std::cin >> c;
}

void merchant::sellto(character *a)
{
    print("\nItem to sell: "); string item1; getline(cin, item1);
    vector<item>::iterator it;
    unsigned vector_size = a->inventory.size();
    for (it = a->inventory.begin(); it != a->inventory.end();)
    {
        if (it->name == item1)
        {
            print("\n\nYou sold "); std::cout << item1; print(" for "); std::cout << it->price; print(" gold");
            this->items.push_back(*it);
            a->gold += it->price;
            this->gold -= it->price;
            a->inventory.erase(it);
            break;
        }
        else
        {
            ++it;
        }
    }
        if (a->inventory.size() == vector_size)
        {
            print("\nYou do not have "); std::cout << item1; print(" to sell");
        }
    /* int increase = (it->price * 1.2+1);
    vector<item>::iterator it2;
    for (it2 = this->items.begin(); it2 != this->items.end(); ++it2)
        {
            if (it2->name == it->name)
            {
                it2->price = increase;
            }
            } */
    print("\n\n\n1. Back\n\n\n> "); char c; std::cin >> c;
}

 

Share this post


Link to post
Share on other sites
On 1/13/2018 at 9:53 PM, Bregma said:

Well, I'd assume the bug is that you're changing the loop index "it" inside the loop.  Can you really reason correctly about the loop invariant if the loop invariant changes in the middle of the loop?

You can see the effects of calling std::vector::erase here.  Pay special attention to the bit that says Invalidates iterators and references at or after the point of the erase, including the end() iterator.

That isn't relevant here, as every time he erases an iterator from within the loop, he breaks out of the loop.

I think the issue was the erase() being done too early, while he was still trying to use the iterator after the erase() call

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!