1st memory allocation bug - can anyone help me find it?

[2EwbqMoL]

I'm sure I did something n00bish here. I must have. I finally got that inventory system almost working - well, it probably would be (if I didnt have this darn bug).

I have 4 classes of items defined, and anything below the base class, if activated( if I create a member or call any functions), gives me some kind of bad/failed memory allocation error.

if I make it an independent class (not a subclass of Item) it works fine, but the minute I relate it to : public Item, it all goes haywire when I run it.

if anyone has any ideas what I did wrong, let me know, code below:

class Item{
private:
	string name;
	string desc;
	int itemId;
public:
	Item(int id, string itemname, string description) : itemId(id), name(itemname), desc(description) { }
	typedef std::vector<Item*> Items;
	string getName(){ return name; };
	string getDesc(){ return desc; };
	int getID(){ return itemId; };
	void LookItem(){ cout << "You see: " << name << ", " << desc; };
};
typedef std::vector<Item *> Items;

class Weapon : public Item {
private:
	string name;
	string desc;
	int itemId;
	int special;
public:
	Weapon(int id, string itemname, string description, int attack) : Item(itemId, name, desc), special(attack) {  } //: Item(itemId, name, desc){ special = attack; };//itemId, Name, Descripton, Attack
	string getName(){ return name; };
	string getDesc(){ return desc; };
	int getID(){ return itemId; };
	int getspecial(){ return special; };
	void LookItem(){ cout << "You see: " << name << ", " << desc; };

};

class Armor : public Weapon{
private:
	string name;
	string desc;
	int itemId;
	int special;
public:
	Armor(int id, string itemname, string description, int def) : Weapon(itemId, name, desc, special) {  } //itemId, Name, Descripton, Defense
	string getName(){ return name; };
	string getDesc(){ return desc; };
	int getID(){ return itemId; };
	int getspecial(){ return special; };
	void LookItem(){ cout << "You see: " << name << ", " << desc; };
};

struct Consumable : public Armor{
private:
	string name;
	string desc;
	int itemId;
	int special;
public:
	Consumable(int id, string itemname, string description, int recoveryamount) : Armor(itemId, name, desc, special) { }//itemId, Name, Descripton, Healing Power
	string getName(){ return name; };
	string getDesc(){ return desc; };
	int getID(){ return itemId; };
	int getspecial(){ return special; };
	void LookItem(){ cout << "You see: " << name << ", " << desc; };
};

[LB]

Please show your main function - I don't think the problem is in how you defined the classes.

Although, your Item class already handles the name, description, and ID of the item, so you don't need to be redeclaring those members at each level of inheritance. That's just wasting memory.

[2EwbqMoL]

first off - I know FOR A FACT that if I take away the inheritance in each class they work perfectly, but once I add : public Item, thats when I get the bug -- I think, personally, it has something to do with the constructor... but i'll show you main anyway...

maybe it's confusing the compiler since I have so many variables of the same name and it allocates to the wrong one/all of them? I'll try streamlining that, and maybe pointers if that doesnt work...

and yes, I know it is memory inefficient, but since its private, the compiler in VC++ 2013 didnt allow me to access those variables/functions through inheritance, so the quick dirty way to fix it without changing a bunch of code was to simply add in the functions -- unless you mean totally eliminating the variables in each function (i.e. only keep special in weapon and have the rest be empty of private functions - this may help)... I was more concerned with getting functional code first, then streamlining it down after I had a base system that worked...

I can post main(), but its kind of a bit complicated and a mess - I used a few headers that define functions (such as console color -- you can find the color function im using on one of the tutorials from this very site here :
http://www.cplusplus.com/articles/2ywTURfi/
its got some flaws, and there is a newer one, but I modified that version slightly. main is as follows

the Weapon test and test.LookItem() are just test functions to see if I was still getting the bug. Im not really sure why im having a memory allocation problem here --

int _tmain(int argc, _TCHAR* argv[])
{		

	Weapon test{ 1, "jeezum", "hdahdhd", 33 };
	test.LookItem();


	int loopswitch = 0;
	SetConsoleTitle(_T("Escape From DOS - A Sysadmin's Nightmare!!!"));
	string start;
	cout << string(10, '\n');
	color b = color(backcolor());
	
	cout << red << setw(65) << "Welcome to Escape from DOS: A Sysadmin's Nightmare." <<endl;

	cout << endl << endl;
	cout << darkpurple << setw(70) << "If you would like to begin your harrowing adventure into realms" << endl << gray << setw(79.5) << "of technology not meant for the eyes of man, type start, otherwise, type quit." << white << endl <<endl;
	getline(cin, start);
	while (loopswitch == 0){
		if (start == "start"){
			loopswitch = 1;
			gamestart();
		}
		else if (start == "quit"){
			loopswitch = 1;
			cout << green << "Hope to see you later!" << endl << cyan;
			system("PAUSE");
			return QUIT;
		}
		else if (start == "exit"){
			loopswitch = 1;
			cout << green << "Hope to see you later!" << endl << cyan;
			system("PAUSE");
			return QUIT;
		}
		else if (start == "about"){
			cout << darkcyan << "Escape from DOS is a text adventure written by Joshua Makler for the" << endl;
			cout << "windows platform written in C++, this version (the original) requires effort to" << endl;
			cout << "port. it relies on windows specific libraries/functions. Released open" << endl;
			cout << "source, GNU GPL, I dedicate it to the development communities and individuals" << endl;
			cout << "all over the net who help people learn, and hope people find this enjoyable,and" << endl;
			cout << "the code educational. I encourage porting and addition to my code, as long as" << endl;
			cout << "credit is given (and hope the dedication is continued) and give permission to" << endl;
			cout << "change the title to BASH, Shell, whatever makes sense on your platform." << endl;
			cout << "writing this was an educational experience for me, and some pieces of the" << endl;
			cout << "code (colors, etc, not the core) were copied/learned from sites and forums" << endl;
			cout << "and I hope this helps someone else learn as well! and that its FUN!" << endl;
			cout << "and thanks specifically to " << red << "eklavya sharma 2 " << darkcyan << "of cplusplus.com for the color" << endl;
			cout << "functions info!" << white << endl <<endl;
			getline(cin, start);
		}
		else {
			cout << blue << "I did not understand you, please type" << green << " start " <<blue << "to be begin or" <<green <<" quit / exit " << blue << endl <<  "to exit. ";
			cout << "type" <<green<< " about " <<blue << "for program information." << white << endl << endl;
			getline(cin, start);
		}
	}
	return ACTIVE;
}

[LB]

Which line does it typically crash on? Have you tried stepping through with a debugger?

THe article you used says not to use it, did you heed its warning?

[2EwbqMoL]

obviously I did not. I dont remember exactly which lines it gave me warnings about, but it didnt crash on those lines. seemed to me there were a few issues with the code, but it wasnt majorly broken so I just used it instead of the other version.

I cant really understand how to read the debugger, but it seems to be pointing to Weapon and the name category, saying it can't read the string... error reading characters of string...

it also says the same about the Item class (which, actually seems to work) and Autos is pointing towards something called this that is above the category Item (which I never declared an Item this or anything so its weird)

says unable to read memory at certain points under this-Item in Autos...
it says this for all instances of name and desc (description)...

the call stack seems to indicate it crashes at what is line 4 on this code I posted (actually, in the REAL program its line 148), but its the declaration of Weapon test;

It also points directly to, where I thought an issue might be, the constructor for the Weapon class

says raise exception kernelBase.dll (not sure if thats relevant)

First-chance exception at 0x765AC42D in EscapeDOS.exe: Microsoft C++ exception: std::bad_alloc at memory location 0x002EF42C.

[2EwbqMoL]

And if I try again to initialize an Item it works fine, but ANY other sub class does not - Weapon, Armor, Consumable, all give the same error, and point, respectively towards the same places...

again its definitely leading me to believe it has SOMETHING to do with those classes being subclasses of Item, or perhaps something in the constructors of them that I did wrong...

I'm going to save a copy as-is, and start reworking this with structs instead of classes so its all public - as I cant inherit any private variables elsewise, and somethings telling me the multiple declarations of name, desc, special etc are causing this memory problem somehow.

edit:

even in cases where all info is public and shared this happens - it cannot read the strings in the constructors for any subclass of Item? this is making no sense to me. Either something is screwy with the constructors I made, or its VC++ doesnt work. i can *try* my installation of TDM but I dont think it will help - Im pretty sure its the constructors.

maybe const * char will work.. or maybe I wasn't supposed to add special where i did.

I dont know, im new to object oriented code like this...

[2EwbqMoL]

switching to char does seem to let it get through, but theres another strange unfortunate bug that doesnt get noticed by the debugger actually -

the one character I am able to store DOES get store, but instead comes up as a strange ascii character instead of text. for Item it; name can be h and it will say h, but if I do a weapon or such, the h comes out as an indescribable ascii object shape...

clearly somethings STILL wrong with the memory - this is in addition to the fact that it wont let me use the constructors for anything other than single char objects if I did it that way -- assuming I got it to work like that anyway...

[LB]

Ok, I isolated the issue. It crashes in your constructors because you pass your member variables to the constructors of the parent classes, but they don't exist until after the parent class gets constructed. So the very first thing I told you about them being superfluous ended up being more than right ;p

This is how far I got while isolating the crash when I realized the issue: http://ideone.com/rAjQrp

[Texan40]

One thing I noticed is that in some cases you are passing the wrong thing into the Parent classes constructor.

Weapon(int id, string itemname, string description, int attack) : Item(itemId, name, desc), special(attack) {  }

You really want to pass 'Item(id,itemname,description);' This will properly populate the data members of Item. Ditto for Armor and Consumable.

Also if you change itemId, name, desc to protected in Item instead of private, they can be accessed by the subclasses and you won't need them in every class as LB already mentioned.

[LB]

@Texan40: yep that's exactly what is causing the crash - the members of the derived class don't exist but they are being passed to the parent class.

This is something that should have generated a warning (in my opinion it should be an error).

[2EwbqMoL]

unfortunately, even after fixing that im still at a loss for how to truly fix this. Im probably just going to give this project up if I can't figure it out tomorrow.

you've given me a start on it, but im not sure if I'll be able to really figure it out (even protected, passing a few item constructors first, I get memory hell, and even if I add all the arguments to Item, including special (which is improper programming)) so its kind of a real bummer to not understand.

I mean I *thought* it was superfluous from the beginning... and seeing how to fix that is helpful - but I dont know how not to pass those arguments on to the previous class, because that itself is an error in my IDE, so I HAVE to pass weapon to item, and even if 6 items are created it still solves nothing.

[keskiverto]

Ok, what does your code look like now?

Something like this?

class Item{
private:
	string name;
	string desc;
	int itemId;
public:
	Item(int id, string itemname, string description)
	  : itemId(id), name(itemname), desc(description) { }

	virtual ~Item() = default;

	string getName() const { return name; };
	string getDesc() const { return desc; };
	int getID() const { return itemId; };
	void LookItem() const { cout << "You see: " << name << ", " << desc; };

	virtual int getspecial() const { return 0; };
};

typedef std::vector<Item *> Items;

class Weapon : public Item {
private:
	int special;
public:
	Weapon( int id, string name, string desc, int attack )
	  : Item( id, name, desc ), special(attack) {  }

	virtual int getspecial() const { return special; };
};

class Armor : public Weapon {
public:
	Armor(int id, string name, string desc, int def )
	  : Weapon( id, name, desc, def ) {  }
};

struct Consumable : public Armor {
public:
	Consumable( int id, string name, string desc, int recoveryamount )
	  : Armor( id, name, desc, recoveryamount ) { }
};

[geeloso]

When I first went through your code, I immediately spotted the assignment of the wrong variables to the formal parameter lists of the constructors of your derived classes; just as hinted by LB and Texan40. If a derived class does not intrinsically have any unique private data members of its own, it is confusing, if not erroneous, to redefine/re-declare the same private data members in the derived class that have already been defined(specified) for any base class.

With respect to your code not fully working, are you sure that (all) the header(s) you lifted from the site you mentioned (i.e. console color, etc) is(are) not introducing bugs into your own code? If the article says "do not use," could it be that the code writer/developer knew it was (they were) bug-ridden?

[2EwbqMoL]

thats the only one... everything else I wrote 100% myself.

also I've tried it without that header and it still seems to be giving the same error (I did give credit for what I used btw, and was asking permission before release publicly -- the main point of this was to learn, not to benefit from someone else work - both in a comment in the header, and in the program itself's about -- so it wasnt quite stealing)

im absolutely certain it has something to do with the way I implemented the classes, although they do work individually-- it has something to do with the constructors like you said...

Im new to Object Oriented programming, I always stuck to straight C when making things because OO actually confuses me - not that in theory its bad for me, but in practice, I run into problems like this.

with a formal education I could easily overcome my problems, but on my own its tough because I just have to figure it out - tutorials lack dynamic progress and the interaction associated with having a teacher, as do books at times (though a bit better)

[LB]

Please post your current code as it is now, we'll take a look at what could be causing the crash this time.

[2EwbqMoL]

I finally found it - the problem was that I was using the BASE variables of the item class, and NOT the variables I used in the constructor of item.

my class was written like this


class Item{
protected:
string name;
string desc;
int itemId;

public:
Item( int id, string itemname, string description) : itemId(id), name(itemname), desc(description) { }
string getName(){ return name; };
string getDesc(){ return desc; };
int getID(){ return itemId; };
void LookItem(){ cout << "You see: " << name << ", " << desc; };
};
typedef std::vector<Item *> Items;

so what I had to do was use the variables id, itemname and description instead of name, desc and itemId, since the item constructor was referencing those variables and not the static, private variables of the item class.

I didnt realize the constructor variables were different than the class private variables, but it makes sense now... seeing as the constructor is a different function i was calling upon - obviously there will be a memory allocation error, because even with read/write access, those variables were the wrong ones...

I feel retarded...

[Texan40]

Without mistakes we'd still be banging sticks against each other in the dark. A mistake is just an opportunity to learn something new! Hell without being willing to make mistakes most of us just wouldn't grow.

[2EwbqMoL]

I'm not going to lie, I feel as if I am banging sticks together in the dark, but I know im making some great coding progress here - I've actually begun to learn some principles of OO.

but its one struggle after another, probably because I chose to create a complex engine for a simple game, instead of just a basic one - now Im struggling trying to figure out how to consider all the above classes as a single type in order to store them in an inventory vector.

unfortunately simple solutions like enum{} do not work, because it reclassifies the names as a new thing. Right now, only base Items can be inserted into the inventory, no weapons, armor, potions.

It looks like Im going to have to have a complicated methodology to actually create a full inventory system. Maybe perhaps I could make a pointer point to all the types of items instead of an enum? I do not know.

I do know, that as much as I enjoy it, it IS a frustrating struggle...

I also know, nothing will even stop me from coding a full on RPG after this - because I'll have the basics/groundwork of an understanding of how to build the engine after I get through all this, should I succeed.

Im completely ignoring SDL/DirectX/OpenGL and all the fun stuff until I get this simple game working. putting graphics on the screen is an easy task in comparison to actually getting the engine together...

Hell, even just this inventory system has given me a week or two of headaches

[LB]

Look up polymorphism. You can store all your instances in a std::vector<std::unique_ptr<YourBaseClass>>
http://www.cplusplus.com/doc/tutorial/polymorphism/