Help with RPG inventory system (solved) Thanks!

[Pikmeir]

I'm new to SFML (2.0), C++, and new to programming, so please be nice on my code.. I apologize it's so long, but this is the bare minimum I think I can post to explain my problem since I'm stumped. (If you want the whole code, I have no problem giving it out to anyone). With that said...

I'm trying to make an RPG game (here are some screenshots: http://imgur.com/a/vxd9B - pardon the Paint graphics, but I suck at drawing and I'm fine with that), and up until now everything's been going fine. I stressed for about a week about how to make an inventory system, and came up with a solution that semi-works, but is giving me issues.

I declared some global variables:

sf::Texture textureArray[95];  // textures for sprites for slots 1-96
sf::Sprite spriteArray[95];      // sprites for spots 1-96
sf::FloatRect invRect[95];      // floatRect for interacting with spots 1-96

In main(), before the game's while loop, I create/load all my textures, and then I do this from 0-95:

spriteArray[0].setPosition(INV_1_X, INV_1_Y); // INV_1_X and Y are pre-defined coords for where to draw the sprites

invRect[0] = sf::FloatRect(INV_1_X, INV_1_Y, INV_SIZE_X, INV_SIZE_Y); // INV_SIZE_X and Y are pre-defined sizes for the boxes

I then set all of the textureArray[] assets from 0-95 to empty by doing this:

for (int i = 0; i < 95; i++) { textureArray[i] = invEmpty; }

Then I give the player some test items:

addItem(invHPot); // I added this line several times with various items

In the game's while loop, if the inventory window is open I do this:

for (int i = 0; i < 96; i++)
{
        if (cursor_box.intersects(invRect[i])) // cursor_box is the floatRect for the cursor
        {
                selectInv = i;
                std::cout << "SELECTED INV SLOT # " << i + 1 << "." << std::endl; // for testing
        }
}

if (cursor_box.intersects(buttonDispose_box)) // if they click DISPOSE after selecting an item
{
        delItem(selectInv);
        std::cout<<"DISPOSE THIS ITEM"<<std::endl; // for testing
        selectInv = -1; // reset selectInv
}
 

And temporarily, if the player clicks the MOVE button below DISPOSE, I made it run addItem(invHPot) just to test if the system's working. (this function is below)

Here are the two functions for adding and deleting items from the inventory:

void delItem(int selectInv) // selectInv is a number 0-95 and is decided from the functions above (when the player clicks in a space)
{
        sf::Texture invEmpty;
        invEmpty.loadFromFile("res/images/inv/invEmpty.png");
        textureArray[selectInv] = invEmpty; // a blank 5x5 png texture
}

void addItem(sf::Texture textureName) // Inv spot 0-95
{
        int i;
        for (i = 0; i < 95; i++)
        {
                if (textureArray[i].getSize().x == 5) // invEmpty is 5x5 pixels, others are 43x43
                {
                        textureArray[i] = textureName;
                        break;
                }
        }
}

Then toward the end of main(), before showing the screen I do the following:

// Link/Set sprites to appropriate textures
for (int i = 0; i < 95; i++)
{ spriteArray[i].setTexture(textureArray[i]); // now the spriteArray and textureArray elements should be linked}

// Display inventory items
for (int i = 0; i < 95; i++)
{ Screen.draw(spriteArray[i]); // draw all of the sprites }

------------------------------------

Phew, that was long. So what the problem is, is sometimes the program crashes randomly when I open the inventory screen. When it doesn't crash, this is what happens:

The items display fine (I usually try addItem with at least 5-30 items, and they display fine). The selecting and deleting items works fine. If I delete an item, it goes away properly. But when I try to add an item while playing, it will only replace one of the empty spots. It won't let me just add items, unless I've "deleted" one already using delItem (the DISPOSE button). I'm not sure how to fix this. I've tried re-arranging things, but each time I get the same problem (when I was testing it with only 12 elements, before changing it to the full 96, it didn't crash, but I had the same problem).

Please tell me I'm stupid "because there's such an easy solution." I hope someone here can help me out.

[Tresky]

Hmmmm.... I don't know what the problem is that you are having, but I do know that you need to put a catcher in your delItem function. The selectInv variable gets set to -1 at some point in there and if no item is selected, but the user hits 'DISPOSE' the program will crash from what I see. -1 is an invalid position in the array.

if (cursor_box.intersects(buttonDispose_box)) // if they click DISPOSE after selecting an item
{
        delItem(selectInv);
        std::cout<<"DISPOSE THIS ITEM"<<std::endl; // for testing
        selectInv = -1; // reset selectInv
}

Perhaps like so...

void delItem(int selectInv) // selectInv is a number 0-95 and is decided from the functions above (when the player clicks in a space)
{
    if (selectInv >= 0) {
        sf::Texture invEmpty;
        invEmpty.loadFromFile("res/images/inv/invEmpty.png");
        textureArray[selectInv] = invEmpty; // a blank 5x5 png texture
    }
}

On another note.
Could you send me the full code? I'll see if I can figure out what could be causing that.

[Pikmeir]

Thanks for letting me know about adding a catcher to delItem().
As for the -1, I put it there (mistakenly) because I didn't want the player to accidentally delete an item without clicking on it, just by clicking the DISPOSE button, unless he/she had just clicked on a space. Maybe it's unnecessary though.

Here's the entire project so far, including the assets:
http://www.mediafire.com/?5y5zid9wbg6k9d3 4.2 MB

And here's just main.cpp by itself (I also attached it to this post too if that's easier):
http://www.mediafire.com/?359uq1ianr8adbw 44 KB

I tried to make the code easy to read by combining repetitive lines into one, but it's still around 800 lines so far. Thanks so much for your help.

[attachment deleted by admin]

[Tresky]

Oh Lord, I don't know, man. Have you considered organizing your code better? i.e. classes, multiple files, etc.

[game_maker]

Are the images/sprites the same? If they are, then use one image and one sprite only, because its a memory waste.
Move the sprite and draw it in the positions that you want.

[Nexus]

Pikmeir, please read this thread. No one will download your whole project and search for errors, so please come up with a reduced example. And if you have just begun to learn C++, I'd suggest to wait some time before you jump directly into game programming, because SFML requires basic C++ knowledge. Or read at least a good C++ book in parallel.

game_maker, multiple sprites are not a problem, sf::Sprite is a lightweight entity. But multiple identical textures should be avoided.

[game_maker]

But it's best to use only one, right? 
It does not matter on a small project...

Goodbye!

[Pikmeir]

I'll go through my project and try to create a simple, runnable short version just with the problem that I'm having and post it here.

As for my project, I don't expect anyone to read through the entire thing and try to debug it, it's just there if you're curious. Since most people are overly protective of their source code, I decided I'd just throw mine out there since I'm new anyway

[Pikmeir]

I made a minimal version of my program and attached all the necessary files as well here.
I also noticed that this program doesn't run at all unless I made the arrays [3] instead of just [2]. I'm not sure why this is, since nothing is held in the last spot. However this program still has the same problem I got before.

Here's just main.cpp by itself:

#include <SFML/Graphics.hpp>

sf::Texture textureArray[3]; // textures for sprites for slots 1-3
sf::Sprite spriteArray[3]; // sprites for spots 1-3
sf::FloatRect invRect[3]; // floatRect for selecting spots 1-3
int selectInv = -1; // current item spot selected (1-3), default is -1
sf::Texture invEmpty;   // EMPTY
sf::Texture invHPot;    // Health potion

void setAllEmpty() // Set all inventory slots to empty
{
        sf::Texture invEmpty;
        invEmpty.loadFromFile("invEmpty.png");
        for (int i = 0; i <= 2; i++)
        {
                textureArray[i] = invEmpty;
        }
}

void delItem(int selectInv)
{
        sf::Texture invEmpty;
        invEmpty.loadFromFile("invEmpty.png");
        if (selectInv >= 0)
        {
                textureArray[selectInv] = invEmpty;
        }
}

void addItem(sf::Texture textureName)
{
        for (int i = 0; i <= 2; i++)
        {
                if (textureArray[i].getSize().x == 5)
                {
                        textureArray[i] = textureName;
                        break;
                }
        }
}

int main ()
{
        sf::RenderWindow Screen (sf::VideoMode (800, 600, 32), "Game", sf::Style::Close);

        sf::Texture invEmpty; // a 5 by 5 blank png
                if (!invEmpty.loadFromFile("invEmpty.png")) return EXIT_FAILURE;
        sf::Texture invHPot; // a 43 by 43 png
                if (!invHPot.loadFromFile("invHPot.png")) return EXIT_FAILURE;

        sf::Texture buttonDispose; // DISPOSE button
                if (!buttonDispose.loadFromFile("buttonDispose.png")) return EXIT_FAILURE;
        sf::Sprite buttonDisposeSpr;
                buttonDisposeSpr.setTexture(buttonDispose);
                buttonDisposeSpr.setPosition(400, 400);
        sf::FloatRect buttonDispose_box = sf::FloatRect(400, 400, 50, 50);

        sf::Texture buttonAddItem; // ADD ITEM button
                if (!buttonAddItem.loadFromFile("buttonAddItem.png")) return EXIT_FAILURE;
        sf::Sprite buttonAddItemSpr;
                buttonAddItemSpr.setTexture(buttonAddItem);
                buttonAddItemSpr.setPosition(500, 400);
        sf::FloatRect buttonAddItem_box = sf::FloatRect(500, 400, 50, 50);
       
        // Set position of each sprite
        spriteArray[0].setPosition(100, 100);
        spriteArray[1].setPosition(200, 100);
        spriteArray[2].setPosition(300, 100);

        // Create FloatRect boxes for each inventory space
        invRect[0] = sf::FloatRect(100, 100, 43, 43);
        invRect[1] = sf::FloatRect(200, 100, 43, 43);
        invRect[2] = sf::FloatRect(300, 100, 43, 43);
       
        setAllEmpty(); // clear inventory

        addItem(invHPot); // give test item

        while (Screen.isOpen())
        {
                Screen.clear();

                // Get mouse cursor info
                float mouseX = sf::Mouse::getPosition(Screen).x;
                float mouseY = sf::Mouse::getPosition(Screen).y;
                sf::FloatRect cursor_box = sf::FloatRect(mouseX, mouseY, 1, 1);

                sf::Event Event;
                while (Screen.pollEvent (Event))
                {
                        if (Event.type == sf::Event::Closed || Event.key.code == sf::Keyboard::Escape)
                                Screen.close();

                        if (Event.type == sf::Event::MouseButtonPressed)
                        {
                                for (int i = 0; i <= 2; i++)
                                {
                                        if (cursor_box.intersects(invRect[i]))
                                                { selectInv = i; }
                                }
       
                                if (cursor_box.intersects(buttonDispose_box))
                                {
                                        delItem(selectInv);
                                        selectInv = -1;
                                }
                                if (cursor_box.intersects(buttonAddItem_box))
                                { addItem(invHPot); }
                        }

                }

                for (int i = 0; i <= 2; i++) // Set spriteArray to textureArray
                { spriteArray[i].setTexture(textureArray[i]); }

                Screen.draw(buttonAddItemSpr);
                Screen.draw(buttonDisposeSpr);

                for (int i = 0; i <= 2; i++)
                { Screen.draw(spriteArray[i]); }

                Screen.display();

        } // (END) GAME WHILE LOOP

        return 0;
}
 

[attachment deleted by admin]

[eXpl0it3r]

As Nexus stated try to read more on C++...

Use std::vector instead of arrays, since they are dynamic and way safer to use than a plain array.

I also noticed that this program doesn't run at all unless I made the arrays [3] instead of just [2]. I'm not sure why this is, since nothing is held in the last spot.

This is because all your loops go from 0 to 2, which are three indiezes, but if your array only holds 2 elements you'd run out of bound and since it's an array it's quite dangerous.

Your initial problem lies with the selection. You're converting for some reason your mouse position to a rectangle and then you're trying to test if the inventory rectangle intersects with the 1x1 mouse rectangle. There's an easy function Rect<T>::contains(point) that checks wether a point is within a rectangle, which is exactly what you need, because the mouse is just a point and not a rectangle.
Also what you're doing with selectInv can crash your program, because whenever you delete an item, you set it to -1 and later it can happen that you don't select an item and thus your variable will still be -1. Trying to access an array at position -1 is not a good idea! 

So conclusion: Read some more about C++ and think of a better structure and code design.

[Pikmeir]

I'll look up std::vector and try to do it using those instead, and if I have any problems I'll post here again.
I also went ahead and fixed the other issues you'd pointed out, but for the time being I left the intersect function for the mouse as is, since it works fine in the rest of the game; I'll change it after I fix the inventory system.
Thanks~

[Pikmeir]

I took your advice and fixed the following:

1) Switched all of the arrays to vectors.
2) Changed the mouse to use contains() instead, only checking if the mouse pixel is inside the box.
3) Decided to use 1-96 for everything, instead of 0-95.
4) Made selectInv default at 0 instead of -1, so the program doesn't crash anymore.

However, I still have the exact same problem as before involving addItem():

If I delete an item, it goes away properly. But when I try to add an item while playing, it will only replace one of the empty spots. It won't let me just add items, unless I've "deleted" one already using delItem (the DISPOSE button).

Any ideas? I've attached the full (simplified version) project below this message.

And I'll attach it just as code right here too:

#include <SFML/Graphics.hpp>
#include <vector>

std::vector<sf::Texture> textureArray (4); // textures for sprites for slots 1-3
std::vector<sf::Sprite> spriteArray (4); // sprites for spots 1-3
std::vector<sf::IntRect> invRect (4); // Rects for selecting spots 1-3
int selectInv = 1; // current item spot selected (1-3), default is 1
sf::Texture invEmpty;   // EMPTY
sf::Texture invHPot;    // Health potion

void setAllEmpty() // Set all inventory slots to empty
{
        sf::Texture invEmpty;
        invEmpty.loadFromFile("invEmpty.png");
        for (int i = 1; i <= 3; i++)
        {
                textureArray.at(i) = invEmpty;
        }
}

void delItem(int selectInv)
{
        sf::Texture invEmpty;
        invEmpty.loadFromFile("invEmpty.png");
        textureArray.at(selectInv) = invEmpty;
}

void addItem(sf::Texture textureName)
{
        for (int i = 1; i <= 3; i++)
        {
                if (textureArray.at(i).getSize().x == 5)
                {
                        textureArray.at(i) = textureName;
                        break;
                }
        }
}

int main ()
{
        sf::RenderWindow Screen (sf::VideoMode (800, 600, 32), "Game", sf::Style::Close);

        sf::Texture invEmpty; // a 5 by 5 blank png
                if (!invEmpty.loadFromFile("invEmpty.png")) return EXIT_FAILURE;
        sf::Texture invHPot; // a 43 by 43 png
                if (!invHPot.loadFromFile("invHPot.png")) return EXIT_FAILURE;

        sf::Texture buttonDispose; // DISPOSE button
                if (!buttonDispose.loadFromFile("buttonDispose.png")) return EXIT_FAILURE;
        sf::Sprite buttonDisposeSpr;
                buttonDisposeSpr.setTexture(buttonDispose);
                buttonDisposeSpr.setPosition(400, 400);
        sf::IntRect buttonDispose_box = sf::IntRect(400, 400, 50, 50);

        sf::Texture buttonAddItem; // ADD ITEM button
                if (!buttonAddItem.loadFromFile("buttonAddItem.png")) return EXIT_FAILURE;
        sf::Sprite buttonAddItemSpr;
                buttonAddItemSpr.setTexture(buttonAddItem);
                buttonAddItemSpr.setPosition(500, 400);
        sf::IntRect buttonAddItem_box = sf::IntRect(500, 400, 50, 50);
       
        // Set position of each sprite
        spriteArray.at(1).setPosition(100, 100);
        spriteArray.at(2).setPosition(200, 100);
        spriteArray.at(3).setPosition(300, 100);

        // Create IntRect boxes for each inventory space
        invRect.at(1) = sf::IntRect(100, 100, 43, 43);
        invRect.at(2) = sf::IntRect(200, 100, 43, 43);
        invRect.at(3) = sf::IntRect(300, 100, 43, 43);
       
        setAllEmpty(); // clear inventory

        addItem(invHPot); // give test item

        while (Screen.isOpen())
        {
                Screen.clear();

                // Get mouse cursor info
                float mouseX = sf::Mouse::getPosition(Screen).x;
                float mouseY = sf::Mouse::getPosition(Screen).y;

                sf::Event Event;
                while (Screen.pollEvent (Event))
                {
                        if (Event.type == sf::Event::Closed || Event.key.code == sf::Keyboard::Escape)
                                Screen.close();

                        if (Event.type == sf::Event::MouseButtonPressed)
                        {
                                for (int i = 1; i <= 3; i++)
                                {
                                        if (invRect.at(i).contains(mouseX, mouseY))
                                                { selectInv = i; }
                                }
       
                                if (buttonDispose_box.contains(mouseX, mouseY))
                                { delItem(selectInv); }

                                if (buttonAddItem_box.contains(mouseX, mouseY))
                                { addItem(invHPot); }
                        }

                }

                Screen.draw(buttonAddItemSpr);
                Screen.draw(buttonDisposeSpr);

                for (int i = 1; i <= 3; i++) // Set spriteArray to textureArray
                { spriteArray.at(i).setTexture(textureArray.at(i)); }

                for (int i = 1; i <= 3; i++)
                { Screen.draw(spriteArray.at(i)); }

                Screen.display();

        } // (END) GAME WHILE LOOP

        return 0;
}
 

Thanks so much for any help you can give me. I keep trying to fix this, but something's still not working right

[attachment deleted by admin]

[eXpl0it3r]

There are still quite a few mistakes...
I've tried to get this thing to work on my end but I only get the two purple 'buttons' and nothing happens when I click on either of them, but maybe it has to do with my changes, which may have removed some unhealthy side effects in your code.

First off I had to remove all the global varaibles, because the application just crashed because SFML is trying to acquire a mutex on a global resource which somehow fails.
This means: DON'T use global variables (unless you really know what you're doing!), better head into using classes & OOP.

Second your random application closing happen in the following line:

if (Event.type == sf::Event::Closed || Event.key.code == sf::Keyboard::Escape)

This translates to: If the even 'Closed' has happend OR by some random undefined behaviour Event.key.code is suddendly equal to sf::Keybard::Escape. You always have to check first if that event you want to use the parameters of, really happend.

Third you're redefining invEmpty and invHPot all over the place. E.g. here

        sf::Texture invEmpty; // a 5 by 5 blank png
                if (!invEmpty.loadFromFile("invEmpty.png")) return EXIT_FAILURE;
        sf::Texture invHPot; // a 43 by 43 png
                if (!invHPot.loadFromFile("invHPot.png")) return EXIT_FAILURE;

or here:

        sf::Texture invEmpty;
        invEmpty.loadFromFile("invEmpty.png");

If you've decided to use global variables why don't you just use them?

Forth, for your replace problem I think it's the addItem code itself. I'm not sure what you're trying to heck with if (textureArray.at(i).getSize().x == 5), but that's were your problem probably is. Because if every x size is equal to 5, every single texture will get replaced by the given texture (invHPot).

I bet I'd find some more points to add here, but I think it's obvious that the following quote still is true:

I'd suggest to wait some time before you jump directly into game programming, because SFML requires basic C++ knowledge. Or read at least a good C++ book in parallel.

[Pikmeir]

First off I had to remove all the global varaibles, because the application just crashed because SFML is trying to acquire a mutex on a global resource which somehow fails.
This means: DON'T use global variables (unless you really know what you're doing!), better head into using classes & OOP.

The global variables are temporary, but not causing any problems. I'll change it later. I know they're not good to use, but I put them there to rapidly generate some working code, so they're only temporary.

Second your random application closing happen in the following line:

if (Event.type == sf::Event::Closed || Event.key.code == sf::Keyboard::Escape)

This translates to: If the even 'Closed' has happend OR by some random undefined behaviour Event.key.code is suddendly equal to sf::Keybard::Escape. You always have to check first if that event you want to use the parameters of, really happend.

I went ahead and removed the Escape command, since I didn't need it anymore, and now the program won't randomly crash like it used. Thanks for letting me know.

Third you're redefining invEmpty and invHPot all over the place. E.g. here

        sf::Texture invEmpty; // a 5 by 5 blank png
                if (!invEmpty.loadFromFile("invEmpty.png")) return EXIT_FAILURE;
        sf::Texture invHPot; // a 43 by 43 png
                if (!invHPot.loadFromFile("invHPot.png")) return EXIT_FAILURE;

or here:

        sf::Texture invEmpty;
        invEmpty.loadFromFile("invEmpty.png");

If you've decided to use global variables why don't you just use them?

These are also temporary, just to make the code work. After it works, I'm planning to get rid of most/all of the global variables and organize my code completely.

Forth, for your replace problem I think it's the addItem code itself. I'm not sure what you're trying to heck with if (textureArray.at(i).getSize().x == 5), but that's were your problem probably is. Because if every x size is equal to 5, every single texture will get replaced by the given texture (invHPot).

The reason is because invEmpty is only 5 by 5 pixels, while every other texture is 43 by 43 pixels. I mention this in the code, but I didn't make it clear enough why I did this. If you test out my code, you'll see the function works. However, it won't let me add items while the game is running (by pushing the ADD ITEM button) - it only lets me add items correctly before running the program, and so far nobody is able to tell me why this happens.

[eXpl0it3r]

"Hey look at my completly broken and rusty car! Could you please put some new shiny rims on it? I'll get the car fix later, but now I just need those rims!"
That pretty much sums it up for me, I wish you good luck!