Weird result from for loop and `std::map`

[odd pollen]

Code is in file

[smoky leaf]

When your question is answered use !solved to mark the question as resolved.

Remember to ask specific questions, provide necessary details, and reduce your question to its simplest form. For tips on how to ask a good question use !howto ask.

[odd pollen]

addItem():

void Inventory::addItem(std::array<int, 2> item){
    for(int i=0;i<slots;i++){
        //Theres already an item with same Tile ID
        if(items[i][0]==item[0]){
            std::cout<<i<<std::endl;
            std::cout<<"world item id: "<<item[0]<<". item id: "<<items[i][0]<<std::endl;
            items[i][1]++;
            std::cout<<"Theres a tile with the same id. "<<std::endl;
        }else{
            //Theres no item with same Tile ID
            std::cout<<i<<std::endl;
            std::cout<<"world item id: "<<item[0]<<". item id: "<<items[i][0]<<std::endl;
            items[i+1]=item;
            std::cout<<"Theres no tile with the same id. "<<std::endl;
        }
    }
}

Inventoy consturctor:

Inventory::Inventory(Vector2 pos, int slots, Texture2D Outline_texture)
    :pos{pos},slots{slots},Outline_texture{Outline_texture}
{
    for(int i=0;i<slots;i++){
        items[i]={0, 0};
    }
}

how the instance is constructed:

inventory = new Inventory(inventory_pos, slots, LoadTexture(inventory_texture.c_str()));

Where takeItem is use:

void Player::takeItem(std::vector<Tile>& tileVec,Camera2D& camera, Sound pickupsound){
    for(int i=0;i<tileVec.size();i++){
        //checking for collision
        if(CheckCollisionRecs(selectArea, tileVec[i].getBody()) && tileVec[i].getType()=="Item"){
            isToucingItem=true;
            if(CheckCollisionPointRec(GetScreenToWorld2D(GetMousePosition(), camera), tileVec[i].getBody())){
                if(IsMouseButtonPressed(MOUSE_BUTTON_LEFT)){
                    //std::cout<<tileVec[i].getID()<<std::endl;

                    inventory->addItem({tileVec[i].getID(), 1});

                    tileVec.erase(tileVec.begin()+i);

                    //inventory->printItems();
                    PlaySound(pickupsound);
                }
            }
        }
    }

(its in the ifIsMousePressed if statement

inventory is an element of Player

expected behaviour:

if the inventory already has an item in any slots with the same ID as tileVec[i], items[ID][total] will be incremented

actual behaviour:

[odd pollen]

when item is picked(tileVec[i]), its stored in the odd index slots (1,3,5,etc...) without its total(bottom-right of that brown box) being increased.

total is items[ID][[value]

console output after picking up one item(with id=2)

which makes no senes because before, its an empty inventory (all id and total=0)

[odd pollen]

anyone?

[sly dagger]

what type is items and how does it reach the scope in the topmost snippet (addItem())?

i didn't spot it being declared/defined anywhere, but i might have just missed it

[odd pollen]

oh sorry i forgot to add it

Inventory.h:

#pragma once

#include"raylib.h"

#include<map>
#include<array>

class Inventory{
    private:
        std::map<int, std::array<int, 2>> items;

        Texture2D Outline_texture;
        Vector2 pos;
        int slots;

    public:
        //Getters;
        std::map<int, std::array<int, 2>> getItems(){return items;}
        int getIntID(int slot);
        void printItems();

        //Setters;
        void setItem(std::map<int, std::array<int, 2>> items){this->items=items;}

        //Function;
        void addItem(std::array<int, 2> item);

        void Draw();

        //Constructors
        Inventory();

        Inventory(Vector2 pos, int slots, Texture2D Outline_texture);
}

[sly dagger]

is this +1 intentional...
items[i+1]=item;

you'll be overwriting whatever is in there with item, so if you just incremented the count for the item in that bucket, it'll be overwritten with whtatever you passed in as item

in addItem()

also, since items is a map, why are you iterating it as if it was an array. those are typically used where you just look something up with it's key.

why not make the key something like the TileID so you can see if it exists in the map that way

i'd probably also avoid using something very generic like std::array<int, 2> when it's holding two ints that represent two fundamentally different data values.

something like this instead will make the code much easier to read/follow:

struct InventoryItem {
    int tileID{};
    int inventor_idx{};
    int item_count{};
};  

then you could just store that in the std::map (and the key would be whatever you primarily want to use to find/identify the data in that struct

[odd pollen]

where is it?

[sly dagger]

in the else... #1195321470340046898 message

[odd pollen]

ahhh

[odd pollen]

yeah i shouldve use vector

[odd pollen]

thank you

[odd pollen]

btw whats inventory_idx?

[sly dagger]

same thing as slot in your loop i think

[odd pollen]

thats the key?

[sly dagger]

whatever you want to use for a lookup

[odd pollen]

ok

[smoky leaf]

@odd pollen Has your question been resolved? If so, type !solved :)

[odd pollen]

nope

not sure why but seems like item_count is the same as id

where its being use

if(IsMouseButtonPressed(MOUSE_BUTTON_LEFT)){
       //std::cout<<tileVec[i].getID()<<std::endl;

       inventory->addItem({tileVec[i].getID(), 1});

       tileVec.erase(tileVec.begin()+i);

       PlaySound(pickupsound);
}

void Inventory::addItem(InventoryItem item){
    for(int i=0;i<slots;i++){
        //Theres already an item with same Tile ID
        if(items[i].tileID==item.tileID){
            items[i].item_count++;
        }else{
            items[i]=item;
        }
    }
}

[sly dagger]

if what you're really after is if a specific tileID is in the inventory already, you could just do something like this:

enum class ItemType {
    Health,
    Food,
    SomethingElse
};

struct InventoryItem {
    int tile_id{}; // tile identifier for that item type
    ItemType item_type{}; // enum representation for that tile item type
    int inventory_idx{}; // the index/slot the item is sitting in if its in the inventory
    int item_count{}; // the number of items in for the item type
    std::string item_name{}; // some description/name of the item
};  

std::unordered_map<int, InventoryItem> items = {};

int food_tile = 1234;
if (items.find(food_tile) == items.end()) // didn't find it
    items[food_tile] = InventoryItem{.tile_id = food_tile, .item_type = ItemType::Food, .inventory_idx = 3, .item_count = 1, .item_name = "Food" };
else // found
    items[food_tile].item_count++;

the extra stuff in the struct might just be useful for debugging, or could be useful later on in the game. using an enum class / description helps a lot when debugging since it makes it much clearer what is being stored and how/when that thing is updated/found/not found

in your original code i'd also make sure the tileID (tileVec[i].getID()) is unique for each item type if that's the intention. if you're using the tile index rather than a unique ID of a specific tile/sprite that represents an object, that could also be the cause of your bug since it's going to think it's something different if the ID is different.

you could also have getID() return something like an enum value to make it clearer when you debug though the code

[odd pollen]

ID is what each tile is, type is a collection IDs.

[odd pollen]

food_tile is what previously items[i] right?

[sly dagger]

not necessarily. i have the map's key set to the ID of the tile instead of using the slot to look stuff up in the map

[odd pollen]

and is find part of std::map?

oh wait nvm

[sly dagger]

it is for std::unordered_map. you can also iterate a std::map like this:

for (auto&& [key, item] : items)
{
    std::cout << "key: " << key << "\n";
    std::cout << "    Tile ID: " << item.tile_id << "\n";
    std::cout << "    Item name: " << item.item_name << "\n";
}

are you just using print statements btw? do you know how to set up a debugger?

this would probably take a few minutes to track down with a debugger if you're just using print statements to diagnose what's going wrong

[tardy musk]

sorry, this may be out of topic, but how is this:

for (auto && [key, item] : items)

compared to this:

for (const auto & [key, item] : items)

[sly dagger]

it'll be the same, const is better unless you want to be able to modify key or item. both will end up being l-value references (auto&)

auto&& in this context acts as a "forwarding reference": https://en.cppreference.com/w/cpp/language/reference#Forwarding_references

@tardy musk

[tardy musk]

and how does auto& compared to auto&& in this case?

i usually use auto& if i wanted to modify the container contents, just wanted to know if using auto&& is better for this case.

[sly dagger]

in the context of iterating like above, they'll be the identical. both will end up being an l-value reference since you won't be iterating through r-values in a container.

auto&& just additionally handles r-values if/when it's applicable, but in code like above it'll almost always end up being equivalent to auto&

[tardy musk]

got it, thanks for the information

[odd pollen]

thank you

[smoky leaf]

@odd pollen Has your question been resolved? If so, type !solved :)

[odd pollen]

what's key?

[sly dagger]

a variable that represents the key for each entry in the map

[odd pollen]

dang it but i need the slot as a sort of "sorting" so it will be drawn in order

is there anyway to do it?

how do i know which slot/idx to put in each item?

[sly dagger]

Use a vector

you can also just keep track of the draw order in the data structure you store in an unsorted container if that's any easier

it kind of depends on how you plan to use it

[odd pollen]

how do i do that

well just need the slot to be in ordered so it will be drawn in order

[smoky leaf]

This question is being automatically marked as stale.
If your question has been answered, type !solved.
If your question is not answered feel free to bump the post or re-ask.
Take a look at !howto ask for tips on improving your question.

[odd pollen]

!solved