Thread: not a pointer issue

  1. #1
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    320

    not a pointer issue

    Ok so I have multiple classes that use pointers to return a pointer to a bitmap that is ready to blit - these work flawlessly as far as I have seen the classes are implemented as pointers and I have no issues.

    Now I took that same code while working on a new class for items my definition is
    Code:
    Item Inventory[16][7];
    Nothing special, when I use my code

    Code:
    void Item::SetPicPath(char *path,RGB *pal)
    {
    	if(PIC)delete PIC;
        PIC = load_bitmap(path,pal);
    }
    BITMAP* Item::GetPic()
    {
       return PIC;        
    }
    It will load the picture fine, but when I try to display it crashes out horribly.

    Now to me it seems reasonable that it is either where I assign a value into inventory or when I retrieve the info from the inventory.

    So I did some testing and got a dump of the data in the character class (where inventory is instanced) dumped inventory in a save file and it has the relevant data. Ok so I thought maybe I can use the ID since I load all items into an array to referance the pic still no go ok maybe it doesn't even load correctly so I did a static referance and it worked fine.

    I can post whatever more data you all want to see, but as there are many referances all over the program to this class it is hard to sort out what you all might want to see. Tell me what pieces you need to see to give me some ideas and I will post emm.

  2. #2
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    320
    Item.h - for referance

    Code:
    #ifndef ITEM_H
    #define ITEM_H
    
    #include <allegro.h>
    #include <winalleg.h>
    
    class Item
    {
    	public:
                 Item();
                 Item & operator=(const Item &rhs);
                 ~Item();
            bool operator==(const Item &rhs);
            BITMAP* GetPic();
            //SOUND ROUTINES SET/PLAY
            void SetUseSnd(char *path);
            void SetSwingSnd(char *path);
            void SetHitSnd(char *path);
            void SetMissSnd(char *path);
            void PlayUseSnd(int vol);
            void PlaySwingSnd(int vol);
            void PlayHitSnd(int vol);
            void PlayMissSnd(int vol);
            //SET DATA ROUTINES
            void SetItemName(char *name);
            void SetPicPath(char *path,RGB *pal);
            void SetEnhancement(int , int);
            void SetID(int);
            char *GetItemName();
            
            char* GetSaveString();
    		int GetID();
    		int GetEnhancement(int spot);
    		int GetResistanceVsType(int type);
    		int GetMinDamage();  //out of date
    		int GetMaxDamage();  //out of date use Get_Damage for the physical damage amount
    		int GetSize();
    		int GetArmorValue();
    		int GetType();
    		bool IsConsumable();
    		int Get_Damage(int clvl);
    		int Get_Amount_Healed(int clvl);
    		int Get_Amount_Restored(int clvl);
    		void Clear();
    		bool Compare(Item second);
    		void LOAD(int load_info[17]);
    	private:		
            int type;   //weapon armor etc
            char *ItemName;
            SAMPLE *Use_SND;
            SAMPLE *Swing_SND;
            SAMPLE *Hit_SND;
            SAMPLE *Miss_SND;
            BITMAP* PIC;
            char *Save_String;
            int min_damage;   
            int max_damage;   
            int armor;        
    		int ID;		//this is a unique ID# 
    		int enhancements[5];
    		int size;
    		bool Consumable;
    		int min_heal;
    		int max_heal;
    		int min_restore;
    		int max_restore;
    		int level;          //for skill purposes higher level is harder to use properly.
    };
    
    #endif

  3. #3
    Registered User
    Join Date
    Oct 2008
    Posts
    1,262
    Well, this line is wrong:
    Code:
    if(PIC)delete PIC;
    As you can't delete PIC; you have to destroy it using a function call. But I doubt that causes the problem.

    Basically, we can't say anything about the problem given the code you have provided us so far.

  4. #4
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    320
    Actually I use that code in multiple places and it hasn't caused an issue yet what code would you like to see? Where the class is implemented how it is implemented or what?
    Code:
         Item::Item()
    {
         Use_SND = NULL;
         Miss_SND = NULL;
         Swing_SND = NULL;
         Hit_SND = NULL;
         ItemName = NULL;
         PIC = NULL;
         Save_String = NULL;
         ID = 0;
         size = 1;
         min_damage = 1;   
         max_damage = 3;  
         min_heal = 0;
         max_heal = 0;
         min_restore = 0;
         max_restore = 0;
         armor = 0;
         type = 0;
         Consumable = false;
         level = 0;
         for(int aq = 0;aq<5;aq++)
         {
            enhancements[aq] = 0;        
         }             
    }
    edit: Might as well use the allegro destructor though I suppose so I will do that if for no other reason other than thats why they wrote it.
    Code:
    void Monster::SetPicPath(char *path,RGB *pal)
    {
    	if(PIC)destroy_bitmap(PIC);
    	PIC = load_bitmap(path,pal);
    }
    BITMAP* Monster::GetPic()
    {
    	return PIC;
    }
    That works perfectly believe it or not it had the delete call as well just changed that - went looking way back to the tile class and had noticed I used the actual function call for it so yes it was wrong and is now fixed - although wasn't generating the error I am tracking - was probably a memory leak.

    Code:
    if(p < mymap->GetNumMonster())
                       {
                          tmpMonster = mymap->GetMonster(p);
    		              if(tmpMonster != NULL && tmpMonster->GetX() - nm == n && tmpMonster->GetY() - mm == m && tmpMonster->GetX() >= nm && tmpMonster->GetX() <= nm+28 && tmpMonster->GetY() >= mm && tmpMonster->GetY() <= mm+21)
       	                  {
                             if(!tmpMonster->IsDead())
                             {
       		                    masked_blit(tmpMonster->GetPic(),BUFFER,(tmpMonster->GetDir()*3+tmpMonster->GetStep())*charx,0,48+(tmpMonster->GetX() - nm)*32-(charx/2)+16,48+(tmpMonster->GetY() - mm)*32-(chary/2),charx,chary);	
                             }else
                             if(GetTickCount()-tmpMonster->GetTimeOfDeath() < 1000)
                             {
       		                    masked_blit(tmpMonster->GetPic(),BUFFER,((GetTickCount()-tmpMonster->GetTimeOfDeath())/250)*charx,50,48+(tmpMonster->GetX() - nm)*32-(charx/2)+16,48+(tmpMonster->GetY() - mm)*32-(chary/2),charx,chary);	
                             }		
    		              }  
                       }
    That works it is directly from a display routine.

    Code:
    void Run_Inventory()
    {
        Item TempItem; 
    	while(key[KEY_I]);
    	
    	while(!key[KEY_I])
    	{
    		blit(LAYOUT,BUFFER,0,0,0,0,scrx,scry);		//toss the backdrop up.
    		masked_blit(INVENTORY,BUFFER,0,0,0,0,scrx,scry);   //toss the grids.
    
    		//DRAW INVENTORY
    		for(int y = 0;y < 7;y++)
    		{
               for(int x = 0;x< 16;x++)
               {
                   TempItem = player->Inventory_Check(x,y);    //returns an Item to us not a pointer just Item is this a problem?
                                                                         
                   if(TempItem.GetID()!=0 && TempItem.GetID()!=-1)       //This checks fine I have saved the ID to a file to check that data.
                   switch(TempItem.GetSize())
                   {
                      case 1:
                        masked_blit(ItemList[TempItem.GetID()]->GetPic(),BUFFER,0,0,434+(33*x),41+(33*y),32,32);  //This crashes as well.
                        break;   
                      case 2:
                        masked_blit(ItemList[TempItem.GetID()]->GetPic(),BUFFER,0,0,434+(33*x),41+(33*y),64,32);   
                        break;        
                      case 3:
                        masked_blit(ItemList[TempItem.GetID()]->GetPic(),BUFFER,0,0,434+(33*x),41+(33*y),32,96);
                        break;             
                      case 4:
                        masked_blit(ItemList[TempItem.GetID()]->GetPic(),BUFFER,0,0,434+(33*x),41+(33*y),64,64);
                        break;                   
                      case 6: 
                        masked_blit(ItemList[TempItem.GetID()]->GetPic(),BUFFER,0,0,434+(33*x),41+(33*y),64,96);    
                        break;                           
                   }  
               }        
            }
            //END DRAW INVENTORY
    		blit(BUFFER,screen,0,0,0,0,scrx,scry);
        }
    	while(key[KEY_I]);
    
    }
    Last edited by ~Kyo~; 04-18-2010 at 05:51 AM. Reason: Adding some code to help explain the situation better.

  5. #5
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    320
    Still having issues hacking on this code. Decided to change it over to pointers which did not work actually crashed it faster. Oddly enough the debugger crashes out when it launches as well so I can't use debug to trace anything down any other ideas?

    This line is crashing it out -
    Code:
    masked_blit(ItemList[2]->GetPic(),BUFFER,0,0,434+(33*x),41+(33*y),32,32);
    DATAFILE

    Code:
    2   Black_Orb       art/Icons/BlackOrb.bmp     4          0         0          0         0           0           0          0            0          0         0         NULL                     NULL            NULL                   NULL  
    ID  NAME            PIC PATH                           SIZE    MIND   MAXD    MINR   MAXR    MINH    MAXH   ARMOR    TYPE    CONS   LVL       USE_SND             SWING_SND  HIT_SND             MISS_SND
    So it must not be loading the pic properly or something similar - not sure what to do to fix it just yet.
    Last edited by ~Kyo~; 04-18-2010 at 05:00 PM. Reason: Narrowed problem down

  6. #6
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    320
    The more I think about my problem I come to a question that I don't know the answer to:
    if you have a pointer to a class and want to copy it to a non pointer of the same class ie

    Code:
    MyClass *pMyClass;
    MyClass aMyClass;
    pMyClass = new MyClass;
    //do some loading
    aMyClass = *pMyClass;
    Will aMyClass have all the same data as pMyClass? Is it better to only use pointers in this case?

  7. #7
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    If you have not defined operator=, but instead are using the default, then all member variables are copied. If your class has pointers in it, the values of those pointers are copied, meaning both classes will point at the same chunk of memory.

  8. #8
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    320
    Code:
    Item& Item::operator=(const Item &rhs) 
    {
        // Only do assignment if RHS is a different object from this.
        if (this != &rhs) 
        {
             Use_SND = rhs.Use_SND;
             Miss_SND = rhs.Miss_SND;
             Hit_SND = rhs.Hit_SND;
             Swing_SND = rhs.Swing_SND;
             ItemName = rhs.ItemName;
             PIC = rhs.PIC;
             ID = rhs.ID;
             size = rhs.size;
             min_damage = rhs.min_damage;   
             max_damage = rhs.max_damage;  
             min_heal = rhs.min_heal;
             max_heal = rhs.max_heal;
             min_restore = rhs.min_restore;
             max_restore = rhs.max_restore;
             armor = rhs.armor;
             type = rhs.type;
             Consumable = rhs.Consumable;
             level = rhs.level;
             for(int aq = 0;aq<5;aq++)
             {
                enhancements[aq] = rhs.enhancements[aq];        
             } 
        }
    
        return *this;
    }
    Basicly this does the same deal. I was wondering if my syntax was correct with the example code I had it seemed a little odd to me, but it did compile which never means anything about the functionality of the code thanks for the reply though.

  9. #9
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    Quote Originally Posted by ~Kyo~ View Post
    Code:
    Item& Item::operator=(const Item &rhs) 
    {
        // Only do assignment if RHS is a different object from this.
        if (this != &rhs) 
        {
             Use_SND = rhs.Use_SND;
             Miss_SND = rhs.Miss_SND;
             Hit_SND = rhs.Hit_SND;
             Swing_SND = rhs.Swing_SND;
             ItemName = rhs.ItemName;
             PIC = rhs.PIC;
             ID = rhs.ID;
             size = rhs.size;
             min_damage = rhs.min_damage;   
             max_damage = rhs.max_damage;  
             min_heal = rhs.min_heal;
             max_heal = rhs.max_heal;
             min_restore = rhs.min_restore;
             max_restore = rhs.max_restore;
             armor = rhs.armor;
             type = rhs.type;
             Consumable = rhs.Consumable;
             level = rhs.level;
             for(int aq = 0;aq<5;aq++)
             {
                enhancements[aq] = rhs.enhancements[aq];        
             } 
        }
    
        return *this;
    }
    Basicly this does the same deal. I was wondering if my syntax was correct with the example code I had it seemed a little odd to me, but it did compile which never means anything about the functionality of the code thanks for the reply though.
    That assignment operator is a waste of space at best. You've simply told it that for every member, copy itself using its assignment operator. Guess what? That's what the default auto-generated one already does!
    Not only that, but the automatically generated version wont do absurd unnecessary things such as checking for self-assignment, wont be prone to forgetting to add one of the members (either now or later), and will probably be able to be optimised better.
    Basically never write an assignment operator, copy-constructor, or destructor, when the automatically generated versions already do the right thing.

    However in this case you do actually need to write an assignment operator, it's just that the one you have coded is completely wrong. You must not simply duplicate the pointer values. You either make some kind of copy of each thing pointed to, or you prevent the assignement operator from ever being called, by making private an unimplemented.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  10. #10
    Registered User
    Join Date
    Oct 2008
    Posts
    1,262
    In this case you simply copy PIC. Meaning it will be freed twice. I still doubt that's your error, but it is yet another error at least.

  11. #11
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    320
    Ok, that should help although looking back on the situation that would waste memory abit so what I have done is write a class called resource_manager it loads the data in from the file and allocates it. Now when I want to go display a BlackOrb or whatnot all I need to do is take resource_manager and get the proper item from it to display the bitmap. This is just some example code not to be confused with real code I have implemented.

    IE
    Code:
    resource_manager *resman;
    resman = new resource_manager;
    resman->Load_Items(filename);
    Item *temp;
    temp = resman->GetItem(which);
    masked_blit(temp->GetPic(),BUFFER,0,0,0,0,x,y);
    So right now I have an issue where it is able to display in one function and not another. One works on crashes they are both global functions. The less complex one crashes out as well lol. I know it is the blit line that is crashing it just not sure why.

  12. #12
    Registered User
    Join Date
    Mar 2010
    Posts
    109
    Before using a pointer like that, you should check if it's NULL. resman->GetItem() might not be returning a valid pointer.

  13. #13
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    320
    Quote Originally Posted by syzygy View Post
    Before using a pointer like that, you should check if it's NULL. resman->GetItem() might not be returning a valid pointer.
    I can look at that as well I suppose I will add a line
    Actual code
    Code:
    void Run_Inventory()
    {
        Item *dispitem;
        dispitem = resman->Get_Item(2);
    	while(key[KEY_I]);      //wait untill 'I' is released
    	
    	while(!key[KEY_I])      //wait untill 'I' is pressed
    	{
    	     blit(LAYOUT,BUFFER,0,0,0,0,scrx,scry);		//toss the backdrop up.
    	     masked_blit(INVENTORY,BUFFER,0,0,0,0,scrx,scry);   //toss the grids.
                        if(dispitem && dispitem->GetPic() != NULL)   //ADDING dispitem && to this line 
                        masked_blit(dispitem->GetPic(),BUFFER,0,0,0,0,64,64);
    	     //DRAW INVENTORY
    		
                        //END DRAW INVENTORY
    	     blit(BUFFER,screen,0,0,0,0,scrx,scry);
                   }
    
    	while(key[KEY_I]);     //wait untill 'I' is released
    
    }
    Still crashes.
    Last edited by ~Kyo~; 04-19-2010 at 10:11 AM.

  14. #14
    Registered User
    Join Date
    Mar 2010
    Posts
    109
    Well, I don't see anything wrong with your function. Without knowing the values being passed I don't really know what's wrong. What's LAYOUT, BUFFER, and INVENTORY? Which line does it crash on?

  15. #15
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    320
    BUFFER is a 1024x768 BITMAP object
    LAYOUT is a 1024x768 BITMAP object
    INVENTORY is a 1024x768 BITMAP object

    Basicly those alone will work perfectly - BUFFER is to ummm buffer the data to eliminate flickers so I draw to that then draw that to the screen.

    Crashes specificly on
    Code:
       masked_blit(dispitem->GetPic(),BUFFER,0,0,0,0,64,64);
    Item #2 is a 64x64 bitmap image. I am attempting to use try and catch to get me some pointer data to do some troubleshooting with - my debugger keeps crashing :/

    The basic flow of the program is as follows

    resource_manager is created
    resource_manager loads data from data files - works ok as far as I can tell
    resource_manager returns a pointer to an Item it is managing
    display routine displays the item's picture - Crash

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