My Debugger Says I Have A Segfault Error But Cannot Tell Me Where, Happens In Very Specific Circumstance

Question

My Debugger (gdb) Is Saying That I Have a Segfault Error, But Cannot Tell Me Where It Is, Says ?? () For the Function.

I started Getting an Error in a very specific circumstance where I click on a weapon type item, then click on another usable item where my program segfaults. I removed most pointers from my code because at first I thought that was the problem, but it didn't change anything. I Could Not Find a Similar Bug online And Debugging Wasn't Very Helpful Because It Can't Tell Me Where Exactly The Error Is Coming From.

Relavant Player.h

class Player{
private:
    int HP;     //player's Health Points
    int SP;     //player's Special Points
    int maxHP;  //maximum value for the player's health points
    int maxSP;  //maximum value for the player's special points
    uint32_t money; //how much money the player has

    float speed; //speed the player will move at (pixels per frame)

    bool menuOpen;  //boolean which is true when the inventory Menu is open

    sf::Vector2f pos; //player's position

    //tests if the player is colliding with any items
    //in the room, accepts the memory address of the room
    bool checkForItems(Room* r);

    std::vector<int> ItemInventory;         //the player's inventory of items.
    std::vector<TextButton> itemButtons;    //vector of TextButtons that correspond with each unique item in the player's inventory
    std::map<std::string, int> ItemsList;   //map that stores each unique item's name and it's quantity

    Control openMenu;           //control to open the menu

    void ItemAttributes(int id);        //makes the items do something depending on what was passed
    bool removeItem(int id);            //removes an item from the player's inventory
    void useItem(Item it);             //takes an Item* and passes it's ID to ItemAttributes
    void updateButtons();               //updates the buttons based on the player's inventory

    Item* EquipedWeapon;    //stores the player's currently equipped weapon

    //enum to store the player's direction for rotations
    enum direction {
        up,
        down,
        left,
        right
    } currentDirection;

    void setRotation();
public:
    //constructor for the player, initializes maxHP, maxSP, and their position:
    Player(int maxhp, int maxsp, sf::Vector2f position);

    //getters:
    int ItemButtonsTextSearchC(const std::wstring& text);   //returns the index in itemButtons for a TextButton which *contains* the text parameter
    int ItemButtonsTextSearch(const std::wstring& text);   //returns the index in itemButtons for a TextButton which equals the text parameter

    //setters:
    void setMoney(int amount);  //sets the money variable
    void setHP(int hp);         //sets the HP variable
    void setSP(int sp);         //sets the SP variable

    void menu();    //inventory menu, handles all things related to the player's inventory
    void draw();    //draws the player to the screen
};

Methods In Player.cpp That Cause The Problem:

void Player::menu(){
    //iterate through all the items listed in itemsList:
    for(auto& p : ItemsList){
        //get a pointer to the item by it's name:
        Item item = *Materials::getItemFromName(p.first);

        //convert the item's name to a wstring for easier use in parameters:
        std::wstring wItemName = std::wstring(p.first.begin(), p.first.end());

        //get the index of the item's corresponding button
        //by searching the itemButtons vector for a button
        //whose text matches this item's name:
        int bIndex = ItemButtonsTextSearch(wItemName);

        if(bIndex != -1){
            //store the button's position:
            sf::Vector2f buttonPosition = itemButtons.at(bIndex).getPosition();

            //default the Y values to 150, we need a separate Y for each one
            //because there are 4 columns of buttons for each type of item
            float weaponY = 150, usableY = 150, collectibleY = 150, moneyY = 150;

            //switch statement to determine the position of the button:
            switch(item.getType()){
                case Item::Weapon:
                    buttonPosition.x = 100;
                    buttonPosition.y = weaponY;

                    //increment by 20 to give space between buttons:
                    weaponY += 20.0f;
                    break;
                case Item::Usable:
                    buttonPosition.x = 375;
                    buttonPosition.y = usableY;

                    //increment by 20 to give space between buttons:
                    usableY += 20.0f;
                    break;
                case Item::Collectible:
                    buttonPosition.x = 650;
                    buttonPosition.y = collectibleY;

                    //increment by 20 to give space between buttons:
                    collectibleY += 20.0f;
                    break;
                case Item::Money:
                    buttonPosition.x = 925;
                    buttonPosition.y = moneyY;

                    //increment by 20 to give space between buttons:
                    moneyY += 20.0f;
                    break;
            }

            //set the button's position now that it's X has been determined:
            itemButtons.at(bIndex).setPosition(buttonPosition);

            /*
            * below we will set the button's text to represent
            * it's corresponding item's name as well as it's
            * quantity then draw the button to the screen so
            * that the client can see how many of each item
            * they have, but then we change it back so that it
            * doesn't break any comparisons with the button's
            * Text (ItemButtonsTextSearch for example):
            */

            //text representing item's quantity to append to the end of the the item's name:
            std::string QuantityText = "\tx" + std::to_string(p.second);

            //wide string that will be the button's text:
            std::wstring wQText = wItemName + std::wstring(QuantityText.begin(), QuantityText.end());

            //set the button's text (it takes wchar_t* so we call .c_str() on wQText):
            itemButtons.at(bIndex).setText(wQText.c_str());

            //draw the button with the temporary text to the screen:
            itemButtons.at(bIndex).draw();

            //poll if the button was clicked, and if it was,
            //we will call useItem on it's corresponding Item:
            if(itemButtons.at(bIndex).pollClicked()){
                useItem(item);
            }

            //change the button's text back to what it was, note: there
            //is a possibility of the button being removed after calling
            //useItem() because when an item's quantity hits 0, the
            //button corresponding with that item is removed, therefore
            //we need a check after the useItem() call to make sure that
            //we don't get an index out of bounds error:
            if(ItemButtonsTextSearchC(wQText) != -1)
                itemButtons.at(bIndex).setText(wItemName.c_str());
        }
    }
}

void Player::useItem(Item it){
    int itemID = it.getItemID();
    ItemAttributes(itemID);
}

void Player::ItemAttributes(int id){
    switch(id){
        case 0: //sword
            //EquipedWeapon = Materials::getItem(id);
            break;
        case 1: //ultra potion of healing
            healHP(50);
            removeItem(id);
            break;
    }
}

bool Player::removeItem(int id){
    //this will be set to true as soon as we find the item:
    bool found = false;

    //loop through ItemInventory and remove the first occurance of id
    //if it exists, otherwise found will remain false:
    for(int i = 0; i < ItemInventory.size(); i++){
        if(ItemInventory.at(i) == id){
            ItemInventory.erase(ItemInventory.begin() + i);
            found = true;
            break;
        }
    }

    //if the item was not found in the inventory, there is no need to
    //continue, we can just return false because we know that it isn't
    //in the player's inventory so it can't be removed in the first place:
    if(!found)
        return false;

    //get an iterator for the item ID's corresponding name in itemsList:
    auto itr = ItemsList.find(Materials::itemNames[id]);

    //check to make sure the item is actually listed; it will be
    //but this is a safeguard in case something breaks:
    if(itr != ItemsList.end()){
        //decrement the item's quantity:
        itr->second--;

        //if there are none remaining, we remove it from the itemsList entirely:
        if(itr->second <= 0)
            ItemsList.erase(itr);
    }

    //update the buttons based on the new changes:
    updateButtons();

    //return true because if it got to this point,
    //the item was found and removed:
    return true;
}

int Player::ItemButtonsTextSearchC(const std::wstring& text){
    if(itemButtons.size() > 0){
        for(int i = 0; i < itemButtons.size(); i++){
            std::wstring bTxt = itemButtons.at(i).getText();
            if(bTxt.find(text) != std::string::npos)
                return i;
        }
    }

    return -1;
}

int Player::ItemButtonsTextSearch(const std::wstring& text){
    if(itemButtons.size() > 0){
        for(int i = 0; i < itemButtons.size(); i++){
            std::wstring bTxt = itemButtons.at(i).getText();
            if(bTxt == text)
                return i;
        }
    }

    return -1;
}

void Player::updateButtons(){
    //first we clear the vector of itemButton:
    itemButtons.clear();

    //loop to go through each unique item in ItemsList map
    //and make a button for each one:
    for(auto& p : ItemsList){
        //convert the item's name into a wstring (textbutton constructor takes wchar_t*, wstring is easier to work with):
        std::wstring wName = std::wstring(p.first.begin(), p.first.end());

        //add the new button to itemButtons
        TextButton btn(sf::Vector2f(0, 0), sf::Color::Magenta, sf::Color::White, wName.c_str(), 18);

        //make sure button presses only register once
        btn.setWasClicked(true);

        //add the button to the itemButtons vector:
        itemButtons.push_back(btn);
    }
}

bool Player::checkForItems(Room* r){
    //itemIndex will == the ID of any item we collided with, if there
    //was no item it returns -1
    int itemIndex = r->checkForItemCollision(player.getGlobalBounds());

    if(itemIndex >= 0){
        //get item ID from the item we just collided with:
        int itemID = r->getItem(itemIndex).collect();

        //remove the item from the room and add it's ID to ItemInventory:
        r->removeItem(itemIndex);
        ItemInventory.push_back(itemID);

        //get the item's name and add it to itemsList if it doesn't exist.
        std::string itemName = Materials::itemNames[itemID];

        //if the item's name is listed in itemsList, we increment it's
        //quantity, else we add it and initialize it's quantity to 1:
        if(ItemsList.count(itemName) != 0){
            ItemsList.at(itemName)++;
        } else {
            ItemsList.insert(std::make_pair(itemName, 1));
        }

        //update the buttons in case a new item was obtained:
        updateButtons();

        //return true because item was found:
        return true;
    }

    //return false, item wasn't found:
    return false;
}

When I click a button, it should delete the first occurance of that item's numeric ID from itemInventory, then in itemsList it should decrement the quantity (the map's value) and if that's <= 0, it should remove that entirely. UpdateButtons clears the entire vector of buttons and makes new ones from itemsList, one button for each key. The Item class has an enum for what type of item it is, the sword (item ID 0) is a weapon, the potion (item ID 1) is a usable item, when i click on the sword (does nothing currently), then add a usable item (the potion) and click the new button that was created for usable, it has an error. This doesn't happen unless I click the sword's button first, in any other circumstance it doesn't error when I use all of the item, then add more to my inventory and use them all again. I suspect it's an error with how i'm updating buttons and removing the keys, but I can't find it. All Item Types are in the same vector of buttons, and the item type pretty much only determines where the button will be positioned.

Answer 1

In Player::menu, you have a loop, for(auto& p : ItemsList). Within that loop, you call a sequence (useItem(item) -> ItemAttributes(itemID) -> removeItem(id) -> ItemsList.erase(itr)) that can modify the ItemsList map you are iterating thru. This invalidates the iterators currently referring to p, so when you try access the next element in the map you get Undefined Behavior because the (internally used) iterator is no longer valid.

One possible remedy is to change your for loop to use your own iterators, and increment the iterator at the top of the loop body (before you modify the map):

for (auto it = ItemsList.begin(); it != ItemsList.end(); ) {
     auto &p = *it;
     ++it;
     // rest of for loop
}