Thread: not a pointer issue

  1. #31
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    320
    I have a new question for you all now related to the same project. Is it bad to do the following?

    Code:
    if(resman->Get_Item(2) != NULL && !player->AquireItem(*resman->Get_Item(2)))  //true on item was placed successfuly false on fail.
        {
           allegro_message("Aquire Item Failed");
        }
    For some reason when I run this chunk of code - even without error checking the whole program freezes up.

    Code:
    bool Character::AquireItem(PAS_Item item) //Places an item in the inventory
    {
    	for(int x = 0;x < 16;x++)
    	{
           for(int y = 0;y < 7;y++)
           {
              switch(item.GetSize())
              {
                  case 1:
                     if(inventory[x][y].GetID() == 0){inventory[x][y] = item;return true;}
                  case 2:
                     if(x != 15 &&  //checking for side of inventory to not go over the array bounds while checking for bad placements.
                        inventory[x][y].GetID() == 0 &&
                        inventory[x+1][y].GetID() == 0)
                        {
                          inventory[x][y] = item;
                          inventory[x+1][y].SetID(-1);
                          return true;
                        }
                  case 3:
                     if(y != 5 && y != 6 &&
                        inventory[x][y].GetID() == 0 &&
                        inventory[x][y+1].GetID() == 0 &&
                        inventory[x][y+2].GetID() == 0)
                        {
                          inventory[x][y] = item;
                          inventory[x][y+1].SetID(-1);
                          inventory[x][y+2].SetID(-1);
                          return true;
                        }
                  case 4:
                      if(x != 15 && y != 6 &&
                        inventory[x][y].GetID() == 0 &&
                        inventory[x][y+1].GetID() == 0 &&
                        inventory[x+1][y+1].GetID() == 0 &&
                        inventory[x+1][y].GetID() == 0)
                        {
                          inventory[x][y] = item;
                          inventory[x+1][y+1].SetID(-1);
                          inventory[x][y+1].SetID(-1);
                          inventory[x+1][y].SetID(-1);
                          return true;
                        }
                  case 6:
                      if(x != 15 && y != 5 && y != 6 &&
                        inventory[x][y].GetID() == 0 &&
                        inventory[x][y+1].GetID() == 0 &&
                        inventory[x+1][y+1].GetID() == 0 &&
                        inventory[x][y+2].GetID() == 0 &&
                        inventory[x+1][y+2].GetID() == 0 &&
                        inventory[x+1][y].GetID() == 0)
                        {
                          inventory[x][y] = item;
                          inventory[x][y+1].SetID(-1);
                          inventory[x+1][y].SetID(-1);
                          inventory[x+1][y+1].SetID(-1);
                          inventory[x+1][y+2].SetID(-1);
                          inventory[x][y+2].SetID(-1);
                          return true;
                        }
                  default:
                     return false;
              }
           }
        }
        return false;
    }
    Code:
    PAS_Item* resource_manager::Get_Item(int which)
    {
        if(which >= 0 && which < Num_Item)return Item_List[which];
        else return NULL;
    }

  2. #32
    Registered User
    Join Date
    Mar 2010
    Posts
    109
    Should you be returning &Item_List[which] in Get_Item() ?

    You might as well add break; lines to each case of your switch. No point in going through check all other possibilities if it has already failed the case you are interested in.

  3. #33
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    320
    No, this function is ONLY to give an item to a player. The returns will exit the function thus no need for break.

    Get_Item() returns a pointer reason being it is the master list - containing bitmap/sound/basic information we only need to glance at this info and copy it to the player's slot because there can be more than one sword, potion, herb, ore, etc.

    I shouldn't need to return the item only tell my main loop that the player either got the item or they didn't get the item.

    [edit] The reason for no return false statements within the loop is we look for a vaild spot untill one is found or there are no more spots to check.

    Freezes also when I change the bool AquireItem(PAS_Item item) code to:
    Ideas?
    Code:
    bool Character::AquireItem(PAS_Item item)
    {
        inventory[0][0] = item;
        return true;
    }
    Last edited by ~Kyo~; 04-21-2010 at 11:20 AM. Reason: simplest fail code

  4. #34
    Registered User
    Join Date
    Mar 2010
    Posts
    109
    Quote Originally Posted by ~Kyo~ View Post
    No, this function is ONLY to give an item to a player. The returns will exit the function thus no need for break.
    Except if it fails the case and doesn't return. Then it checks the next case and fails, and the next and fails, and the next, etc. Up to you, but it's needless checking.

    Get_Item() returns a pointer reason being it is the master list - containing bitmap/sound/basic information we only need to glance at this info and copy it to the player's slot because there can be more than one sword, potion, herb, ore, etc.
    So Item_List is an array of pointers?

  5. #35
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    320
    Ok I see that now. It seems just the function call itself is crashing I can have it just return true and freeze I think it must be something to do with the conversion of a * to just that class.

    yes Item_List is an array of pointers:
    Code:
    class resource_manager
    {
        public:
               resource_manager();
               ~resource_manager();
          void Load_Items(char *ItemFilePath);
          void Load_Spells(char *SpellFilePath);
          PAS_Item *Get_Item(int which);
          Spell *Get_Spell(int which);     
          int GetNumSpell();
          int GetNumItem();
        private:
          int Num_Item;
          int Num_Spell;
          PAS_Item **Item_List;
          Spell **Spell_List;  
    };

  6. #36
    Sweet
    Join Date
    Aug 2002
    Location
    Tucson, Arizona
    Posts
    1,820
    Quick question.

    Why are you using a 2d array to hold your inventory?
    Woop?

  7. #37
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    320
    Quote Originally Posted by prog-bman View Post
    Quick question.

    Why are you using a 2d array to hold your inventory?
    Think back to games such as Diablo and the like. Armor takes 6 spaces while a potion will take 1 - I could do this with a limited # of spaces in a straight line yes, but come on inventory tetris ftw right?

  8. #38
    Sweet
    Join Date
    Aug 2002
    Location
    Tucson, Arizona
    Posts
    1,820
    Wouldn't it be easier to have a running counter of size and just a single array?

    Code:
    int itemSize = item.GetSize();
    
    if(itemSize <= this->mAvailableSpace){
       this->mInventorySlot[this->mCurrentIndex] = item;
       this->mAvailableSpace -= itemSize
       this->mCurrentIndex++;
       return true;
    }//if
    
    return false;
    Last edited by prog-bman; 04-21-2010 at 12:37 PM.
    Woop?

  9. #39
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    320
    Quote Originally Posted by prog-bman View Post
    Wouldn't it be easier to have a running counter of size and just a single array?

    Code:
    int itemSize = item.GetSize();
    
    if(itemSize <= this->mAvailableSpace){
       this->mInventorySlot[this->mCurrentIndex] = item;
       this->mAvailableSpace -= itemSize
       this->mCurrentIndex++;
       return true;
    }//if
    
    return false;

    think of it this way:
    Code:
    size = 1    [ ]
    size = 2    [ ] [ ]
    size = 3    [ ]
                [ ]
                [ ]
    size = 4    [ ] [ ]
                [ ] [ ]
    size = 5 invalid
    size = 6    [ ] [ ]
                [ ] [ ]
                [ ] [ ]
    This takes care of the GUI mapping so there is no over lapping spots on items. Constantly increasing the inventory lets the player pick up whatever they want with no problems this is not my intention. I forsee dungeon crawls where players have to ditch non-magical equipment or lesser magical equipment for better equipment - they aren't oxen they can't carry tons of stuff. Maybe inventory space based on strength, but for the moment no a 1d array will not work for my application - unless I make the code vague and subscript it as inventory[x+y*16];
    Last edited by ~Kyo~; 04-21-2010 at 12:47 PM. Reason: lingering code tag at the end.

  10. #40
    Sweet
    Join Date
    Aug 2002
    Location
    Tucson, Arizona
    Posts
    1,820
    I get it now. You basically have shaped items that take up different lengths and widths.

    So a sword could be a 2w x 5h but a potion would be a 2w x 1h or something.

    Are you opposed to using vectors and wrapper classes. Cause these things could make you life much easier so you wouldn't have to pass pointers around.
    Woop?

  11. #41
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    320
    Not so much opposed as I haven't used vectors before. I understand the concept I believe just not the implementation. Pointers always seemed more straightforward and dynamic than a vector.

    I want to add this little bit of a description I wrote that I was going to edit on my last post.

    The more I think about it the more explanation may be needed to visualize the problem.

    I have art files some are 32x32, 64x32, 64x64, 32x96, 64x96;

    Now that being said I have an inventory map drawn up this screen has the following in it:

    A grid for the inventory - drag and drop kind of set up.
    A person's equipment slots layed out similar to WoW Diablo etc
    A person's Skill list and a hotkey list for the said skills.

    The thought process isn't a list of items that all look the same - it is more of a how can I fit this bow in my inventory? Do I toss the helmet out or some healing potions? What is less important? What is the better choice to keep? I want players to think not a hack n slash - yes that is a part of this game, but games now have very few challanges to make people think they hold the player's hand and direct them. This is not intended to be this way - it is intended to have some puzzles in the dungeons. Most games focus on quests this game will have what I intend to call milestones - not so much as quests although they do require interaction with NPCs. Lets say a milestone is helping repair a bridge that crosses over a river so you can go explore the other side there should be a minimal force directing the players there should be another way around the river maybe to the north in the mountains or the south in a desert where the river shallows out / drys up. I don't really want to rant about the design aspect so much, but you can see how it will play a part in the game ( yes I realize the the -1) placeholders make it oddly done in the inventory and how will I know which -1 points to what item I have all of this figured out in my other code.

  12. #42
    Registered User
    Join Date
    Mar 2010
    Posts
    109
    Man, I wish your debugger worked too =)

    But basically, this call:
    Code:
    !player->AquireItem(*resman->Get_Item(2))
    is crashing, even if it's just returning true/false with no other logic? If you just create a test object and pass it in, does it still crash?

    Also, you might consider passing AquireItem() a pointer or const reference instead of a copy of the item, just for efficieny's sake.

  13. #43
    Sweet
    Join Date
    Aug 2002
    Location
    Tucson, Arizona
    Posts
    1,820
    Personally if I was attacking this project.

    I would write nice wrappers around the allegro objects to make them C++ like. Then you could pass around references to these objects instead of pointers, achieving the same functionality you have now but never having to worry about something not being allocated or in a bad state.

    Have you looked into dat files for storing your images and such. Makes life much easier. Don't need to have a bunch of files sitting around.
    Woop?

  14. #44
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    320
    Quote Originally Posted by syzygy View Post
    Man, I wish your debugger worked too =)

    But basically, this call:
    Code:
    !player->AquireItem(*resman->Get_Item(2))
    is crashing, even if it's just returning true/false with no other logic? If you just create a test object and pass it in, does it still crash?

    Also, you might consider passing AquireItem() a pointer or const reference instead of a copy of the item, just for efficieny's sake.
    wrong call, but yes it still crashes even if the code is just return true;

    question

    since this is a 2d array of chars
    Code:
    char 2darray[x][y];
    can I do

    Code:
    char* 2dpointerarray[x][y];
    ?

    [edit]!

    Also I just tested by making a PAS_Item temp var in main it does pass 100% fine and allocates in the arrays as planned. I have a data dump of this in the save file.
    Last edited by ~Kyo~; 04-21-2010 at 04:12 PM. Reason: Code check

  15. #45
    Registered User
    Join Date
    Mar 2010
    Posts
    109
    Yes, I believe you can make a 2D array of char pointers. However, I overlooked the fact that you were assigning the object inside the function. If you do make these pointers, then you suddenly have to worry about when the memory those pointers are pointing to get destroyed.

    Basically, using references and pointers can save you a lot of memory and speed but it would be better to build some kind of memory/resource manager into the system if you do decide to do that. Otherwise, you will end up with crashes because things are referencing something that has suddenly been deleted and they haven't been notified about it.

    So, for now, I would just stick with the copy, unless you feel like doing an overhaul (or maybe you already have something that manages things properly).

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Help with pointer issue
    By ph5 in forum C Programming
    Replies: 4
    Last Post: 11-17-2009, 04:19 PM
  2. Compiling C in Visual Studio 2005
    By emanresu in forum C Programming
    Replies: 3
    Last Post: 11-16-2009, 04:25 AM
  3. Replies: 2
    Last Post: 07-11-2008, 07:39 AM
  4. Following CTools
    By EstateMatt in forum C Programming
    Replies: 5
    Last Post: 06-26-2008, 10:10 AM
  5. Contest Results - May 27, 2002
    By ygfperson in forum A Brief History of Cprogramming.com
    Replies: 18
    Last Post: 06-18-2002, 01:27 PM