Sign in to follow this  

Dynamic Memory Allocation - help needed

This topic is 3932 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

i am currently developing a 2d rpg and have ran across a problem that i can't identify in my inventory system its not a compile error as far as i'm aware i am trying to allocate some memory for an array of a custom class with a pointer pointing to it. an example of what i'm doing class wise:


class foo{
private:
int m_DynamicNumber;

int *p_CustomClassCount;
CustomClass *p_CustomClass;

public:
class();
}


//in constructor

class::class(){
m_DynamicNumber = 5;

p_CustomClass = new CustomClass[m_DynamicNumber];
p_CustomClassCount = new CustomClassCount[m_DynamicNumber];

}

now that is basically what i'm doing (full source code included at the end) i think it might be that i'm trying to allocate the memory in the constructor but i dont see the problem with that. it is compiling and when i run the program it does allocate memory but only for 1 not for the entire array,i've also checked if i can access the other members (as i thought it just might not be showing up in msvc) but i cant. any help would be appreciated thank you (note: the problem i'm talking about is in the Inventory code,the item code is there because it is a header in inventory) Item.h
#pragma once
#include <windows.h>
#include <string>
#include <gl/gl.h>
#include "TGA.h"



using namespace std;

class Item{
protected:
	string m_Name;
	string m_ID;
	string m_Description;
	GLuint m_DropSprite;
	GLuint m_InventorySprite;
public:
	Item();
	Item(string Name,string Description,string ID,TGAImg Drop,TGAImg Inven);
	string GetName();
	string GetDesc();
	string GetID();
	GLuint GetDropSprite();
	GLuint GetInventorySprite();

	void SetName(string Name);
	void SetDesc(string Desc);
	void SetID(string ID);
	void SetDropSprite(TGAImg *DropSprite);
	void SetInventorySprite(TGAImg *InvenSprite);
	void SetDropSprite(GLuint DropSprite);
	void SetInventorySprite(GLuint InvenSprite);
};
class Item_Potion : public Item{
private:
	int m_RestoreValue;
public:
	Item_Potion();
	Item_Potion(string Name,string Description,string ID,TGAImg Drop,TGAImg Inven,int Value);
	int GetRestoreValue();
	void SetRestoreValue(int Value);
};

class Item_MonsterDrop : public Item{
private:
	int m_Level;
public:
	Item_MonsterDrop();
	Item_MonsterDrop(string Name,string Description,string ID,TGAImg Drop,TGAImg Inven,int Level);
	int GetLevel();
	void SetLevel(int Level);

};

class Item_Base : public Item{
private:
	int m_Type;	// 0 for damage 1 for defence 2 for elemental
	int m_Level;	//level's 1 through 5, 1 being the lowest
public:
	Item_Base();
	Item_Base(string Name,string Description,string ID,TGAImg Drop,TGAImg Inven,int Type,int Level);
	int GetType();
	int GetLevel();

	void SetType(int Type);
	void SetLevel(int Level);
};

class Item_Upgrade : public Item{
private:
	int m_Type; // 0 for Damage 1 for Defence 2 for elemental
	int m_Level;	//level's 1 through 5, 1 being the lowest
	int m_Value;	//Value of attack,defense or elemental addition
public:
	Item_Upgrade();
	Item_Upgrade(string Name,string Description,string ID,TGAImg Drop,TGAImg Inven,int Type,int Level);
	int GetType();
	int GetLevel();
	int GetValue();

	void SetType(int Type);
	void SetLevel(int Level);
	void SetValue(int Value);
};

class Item_Weapon : public Item{
private:
	int m_Atk;
	int m_Element;
	int m_Percentage;
	TGAImg m_AttackImg;
public:
	Item_Weapon();
	Item_Weapon(string Name,string Description,string ID,TGAImg Drop,TGAImg Inven,TGAImg AttackImg,int Atk,int Element,int Percentage);
	int GetAtk();
	int GetElement();
	int GetPercentage();

	void SetAtk(int Atk);
	void SetElement(int Element);
	void SetPercentage(int Percentage);
};

class Item_Armour : public Item{
private:
	int m_Def;
	int m_Element;
	int m_Percentage;
public:
	Item_Armour();
	Item_Armour(string Name,string Description,string ID,TGAImg Drop,TGAImg Inven,int Def,int Element,int Percentage);
	int GetDef();
	int GetElement();
	int GetPercentage();

	void SetDef(int Def);
	void SetElement(int Element);
	void SetPercentage(int Percentage);
};

Item.cpp
#pragma once

#include "Item.h"

//Base Class Functions
Item::Item(){}		// default constructor DO NOT USE
Item::Item(string Name,string Description,string ID,TGAImg Drop,TGAImg Inven){
	m_Name = Name;
	m_Description = Description;
	m_ID = ID;
	SetDropSprite(&Drop);
	SetInventorySprite(&Inven);
}

string Item::GetDesc(){
	return m_Description;
}
string Item::GetName(){
	return m_Name;
}

string Item::GetID(){
	return m_ID;
}
GLuint Item::GetDropSprite(){
	return m_DropSprite;
}
GLuint Item::GetInventorySprite(){
	return m_InventorySprite;
}



void Item::SetName(string Name){
	m_Name = Name;
}
void Item::SetDesc(string Desc){
	m_Description = Desc;
}
void Item::SetID(string ID){
	m_ID = ID;
}
void Item::SetDropSprite(TGAImg *DropSprite){
	glGenTextures(1,&m_DropSprite);
	glBindTexture(GL_TEXTURE_2D,m_DropSprite);
	glTexImage2D(GL_TEXTURE_2D, 0,GL_RGBA, DropSprite->GetWidth(),DropSprite->GetHeight(), 0, GL_RGBA, GL_UNSIGNED_BYTE,DropSprite->GetImg());
	glTexParameteri(GL_TEXTURE_2D,GL_TEXTURE_MIN_FILTER,GL_LINEAR);
	glTexParameteri(GL_TEXTURE_2D,GL_TEXTURE_MAG_FILTER,GL_LINEAR);
}
void Item::SetInventorySprite(TGAImg *InvenSprite){
	glGenTextures(1,&m_InventorySprite);
	glBindTexture(GL_TEXTURE_2D,m_InventorySprite);
	glTexImage2D(GL_TEXTURE_2D, 0,GL_RGBA, InvenSprite->GetWidth(),InvenSprite->GetHeight(), 0, GL_RGBA, GL_UNSIGNED_BYTE,InvenSprite->GetImg());
	glTexParameteri(GL_TEXTURE_2D,GL_TEXTURE_MIN_FILTER,GL_LINEAR);
	glTexParameteri(GL_TEXTURE_2D,GL_TEXTURE_MAG_FILTER,GL_LINEAR);
}
void Item::SetDropSprite(GLuint DropSprite){
	m_DropSprite = DropSprite;
}
void Item::SetInventorySprite(GLuint InvenSprite){
	m_InventorySprite = InvenSprite;
}

//Potion Specific Functions
Item_Potion::Item_Potion(){}		// default constructor DO NOT USE
Item_Potion::Item_Potion(string Name,string Description,string ID,TGAImg Drop,TGAImg Inven,int Value){
	m_Name = Name;
	m_Description = Description;
	m_ID = ID;
	SetDropSprite(&Drop);
	SetInventorySprite(&Inven);
	m_RestoreValue = Value;
}
int Item_Potion::GetRestoreValue(){
	return m_RestoreValue;
}

void Item_Potion::SetRestoreValue(int Value){
	m_RestoreValue = Value;
}

// Monster Drop Specific functions
Item_MonsterDrop::Item_MonsterDrop(){}	// default constructor DO NOT USE
Item_MonsterDrop::Item_MonsterDrop(string Name, string Description, string ID, TGAImg Drop, TGAImg Inven, int Level){
	m_Name = Name;
	m_Description = Description;
	m_ID = ID;
	SetDropSprite(&Drop);
	SetInventorySprite(&Inven);
	m_Level = Level;
}

int Item_MonsterDrop::GetLevel(){
	return m_Level;
}

void Item_MonsterDrop::SetLevel(int Level){
	m_Level = Level;
}
// Synth Base Specific Functions
Item_Base::Item_Base(){}		// default constructor DO NOT USE
Item_Base::Item_Base(string Name, string Description, string ID, TGAImg Drop, TGAImg Inven, int Type, int Level){
	m_Name = Name;
	m_Description = Description;
	m_ID = ID;
	SetDropSprite(&Drop);
	SetInventorySprite(&Inven);
	m_Type = Type;
	m_Level = Level;
}

int Item_Base::GetType(){
	return m_Type;
}

int Item_Base::GetLevel(){
	return m_Level;
}

void Item_Base::SetLevel(int Level){
	m_Level = Level;
}
void Item_Base::SetType(int Type){
	m_Type = Type;
}


//Upgrade Specific Functions
Item_Upgrade::Item_Upgrade(){}		// default constructor DO NOT USE
Item_Upgrade::Item_Upgrade(string Name, string Description, string ID, TGAImg Drop, TGAImg Inven, int Type, int Level){
	m_Name = Name;
	m_Description = Description;
	m_ID = ID;
	SetDropSprite(&Drop);
	SetInventorySprite(&Inven);
	m_Type = Type;
	m_Level = Level;
}

int Item_Upgrade::GetType(){
	return m_Type;
}

int Item_Upgrade::GetLevel(){
	return m_Level;
}

int Item_Upgrade::GetValue(){
	return m_Value;
}
void Item_Upgrade::SetLevel(int Level){
	m_Level = Level;
}
void Item_Upgrade::SetType(int Type){
	m_Type = Type;
}

void Item_Upgrade::SetValue(int Value){
	m_Value = Value;
}
// Weapon Specific Functions
Item_Weapon::Item_Weapon(){}		// default constructor DO NOT USE
Item_Weapon::Item_Weapon(string Name,string Description,string ID,TGAImg Drop,TGAImg Inven,TGAImg AttackImg,int Atk,int Element,int Percentage){
	m_Name = Name;
	m_Description = Description;
	m_ID = ID;
	SetDropSprite(&Drop);
	SetInventorySprite(&Inven);
	m_AttackImg = AttackImg;
	m_Atk = Atk;
	m_Element = Element;
	m_Percentage = Percentage;
}

int Item_Weapon::GetAtk(){
	return m_Atk;
}

int Item_Weapon::GetElement(){
	return m_Element;
}
int Item_Weapon::GetPercentage(){
	return m_Percentage;
}

void Item_Weapon::SetAtk(int Atk){
	m_Atk = Atk;
}
void Item_Weapon::SetElement(int Element){
	m_Element = Element;
}
void Item_Weapon::SetPercentage(int Percentage){
	m_Percentage = Percentage;
}

//Armour Specific Functions
Item_Armour::Item_Armour(){}		// default constructor DO NOT USE
Item_Armour::Item_Armour(string Name, string Description, string ID, TGAImg Drop, TGAImg Inven, int Def, int Element, int Percentage){
	m_Name = Name;
	m_Description = Description;
	m_ID = ID;
	SetDropSprite(&Drop);
	SetInventorySprite(&Inven);
	m_Def = Def;
	m_Element = Element;
	m_Percentage = Percentage;
}

int Item_Armour::GetDef(){
	return m_Def;
}

int Item_Armour::GetElement(){
	return m_Element;
}
int Item_Armour::GetPercentage(){
	return m_Percentage;
}

void Item_Armour::SetDef(int Def){
	m_Def = Def;
}
void Item_Armour::SetElement(int Element){
	m_Element = Element;
}
void Item_Armour::SetPercentage(int Percentage){
	m_Percentage = Percentage;
}


Inventory.h
#pragma once
#include "Item.h"
class Inventory{
private:
	int m_PotionSlots;
	int m_DropSlots;
	int m_BaseSlots;
	int m_UpgradeSlots;
	int m_WeaponSlots;
	int m_ArmourSlots;

	int *m_PotionCount;
	int *m_DropCount;
	int *m_BaseCount;
	int *m_UpgradeCount;
	int *m_WeaponCount;
	int *m_ArmourCount;
public:
	Item_Potion *m_PotionBag;
	Item_MonsterDrop *m_DropBag;
	Item_Base *m_BaseBag;
	Item_Upgrade *m_UpgradeBag;
	Item_Weapon *m_WeaponBag;
	Item_Armour *m_ArmourBag;

	Inventory();
	Inventory(int PotionSlots,int DropSlots,int BaseSlots,int UpgradeSlots,int WeaponSlots,int ArmourSlots);
	~Inventory();
	bool AddToPotion(Item_Potion Item);
	bool AddToPotion(Item_Potion Item,int Slot);
	bool AddToMonsterDrop(Item_MonsterDrop Item);
	bool AddToMonsterDrop(Item_MonsterDrop Item,int Slot);
	bool AddToBase(Item_Base Item);
	bool AddToBase(Item_Base Item,int Slot);
	bool AddToUpgrade(Item_Upgrade Item);
	bool AddToUpgrade(Item_Upgrade Item,int Slot);
	bool AddToWeapon(Item_Weapon Item);
	bool AddToWeapon(Item_Weapon Item,int Slot);
	bool AddToArmour(Item_Armour Item);
	bool AddToArmour(Item_Armour Item,int Slot);

	void DestroyItem(int Slot);
	void MoveItem(int RootSlot, int TargetSlot);

};


Inventory.cpp
#include "Inventory.h"

#define POTION_STACK_LIMIT 10
#define DROP_STACK_LIMIT 20
#define BASE_STACK_LIMIT 20
#define UPGRADE_STACK_LIMIT 2
#define WEAPON_STACK_LIMIT 1
#define ARMOUR_STACK_LIMIT 1

Inventory::Inventory(){
	m_PotionSlots = 5;		// sets up how many slots for each item type
	m_DropSlots = 10; 
	m_BaseSlots = 10;
	m_UpgradeSlots = 6;
	m_WeaponSlots = 4;
	m_ArmourSlots = 4;

	m_PotionBag = new Item_Potion[m_PotionSlots];	// assigns an array with amount of slots defined previously
	m_PotionCount = new int[m_PotionSlots];			// and also creates an array that holds a counter for each slot
	m_DropBag = new Item_MonsterDrop[m_DropSlots];	// so items can stack 
	m_DropCount = new int[m_DropSlots];	
	m_BaseBag = new Item_Base[m_BaseSlots];
	m_BaseCount = new int[m_BaseSlots];
	m_UpgradeBag = new Item_Upgrade[m_UpgradeSlots];
	m_UpgradeCount = new int[m_UpgradeSlots];
	m_WeaponBag = new Item_Weapon[m_WeaponSlots];
	m_WeaponCount = new int[m_WeaponSlots];
	m_ArmourBag = new Item_Armour[m_ArmourSlots];
	m_ArmourCount = new int[m_ArmourSlots];
						
	for(int i = 0;i<m_PotionSlots;i++){	//Set all counters to 0
		m_PotionCount[i] = 0;			//could possibly do with memset but not sure
	}									//if it sets it to 0 or the unsigned char version of 0
	for(int i = 0;i<m_DropSlots;i++){
		m_DropCount[i] = 0;
	}
	for(int i = 0;i<m_BaseSlots;i++){
		m_BaseCount[i] = 0;
	}
	for(int i = 0;i<m_UpgradeSlots;i++){
		m_UpgradeCount[i] = 0;
	}
	for(int i = 0;i<m_WeaponSlots;i++){
		m_WeaponCount[i] = 0;
	}
	for(int i = 0;i<m_ArmourSlots;i++){
		m_ArmourCount[i] = 0;
	}
	 
}
Inventory::Inventory(int PotionSlots,int DropSlots,int BaseSlots,int UpgradeSlots,int WeaponSlots,int ArmourSlots){
	m_PotionSlots = PotionSlots;
	m_DropSlots = DropSlots; 
	m_BaseSlots = BaseSlots;
	m_UpgradeSlots = UpgradeSlots;
	m_WeaponSlots = WeaponSlots;
	m_ArmourSlots = ArmourSlots;

	
	m_PotionBag = new Item_Potion[m_PotionSlots];	// assigns an array with amount of slots defined previously
	m_PotionCount = new int[m_PotionSlots];			// and also creates an array that holds a counter for each slot
	m_DropBag = new Item_MonsterDrop[m_DropSlots];	// so items can stack 
	m_DropCount = new int[m_DropSlots];	
	m_BaseBag = new Item_Base[m_BaseSlots];
	m_BaseCount = new int[m_BaseSlots];
	m_UpgradeBag = new Item_Upgrade[m_UpgradeSlots];
	m_UpgradeCount = new int[m_UpgradeSlots];
	m_WeaponBag = new Item_Weapon[m_WeaponSlots];
	m_WeaponCount = new int[m_WeaponSlots];
	m_ArmourBag = new Item_Armour[m_ArmourSlots];
	m_ArmourCount = new int[m_ArmourSlots];
						
	for(int i = 0;i<m_PotionSlots;i++){	//Set all counters to 0
		m_PotionCount[i] = 0;			//could possibly do with memset but not sure
	}									//if it sets it to 0 or the unsigned char version of 0
	for(int i = 0;i<m_DropSlots;i++){
		m_DropCount[i] = 0;
	}
	for(int i = 0;i<m_BaseSlots;i++){
		m_BaseCount[i] = 0;
	}
	for(int i = 0;i<m_UpgradeSlots;i++){
		m_UpgradeCount[i] = 0;
	}
	for(int i = 0;i<m_WeaponSlots;i++){
		m_WeaponCount[i] = 0;
	}
	for(int i = 0;i<m_ArmourSlots;i++){
		m_ArmourCount[i] = 0;
	}
}


Inventory::~Inventory(){
	delete []m_PotionBag;	
	delete []m_PotionCount;
	delete []m_DropBag;
	delete []m_DropCount;	
	delete []m_BaseBag;
	delete []m_BaseCount;
	delete []m_UpgradeBag;
	delete []m_UpgradeCount;
	delete []m_WeaponBag;
	delete []m_WeaponCount;
	delete []m_ArmourBag;
	delete []m_ArmourCount;
}
bool Inventory::AddToPotion(Item_Potion Item){

	bool Item_Present = false;		//if item is already in the bag
	bool Item_Fits = false;			//if the item is already in the bag and will fit into a stack
	bool Empty_Space = false;
	int Situation = 4;				// a value not in the enumeration(and thus not in the switch-case)
	// enum types
	// P_SIS  the item is present and a is has space in its stack
	// NP_ES item is not already present and there is an empty space where it can go
	// ALL_FULL there is no space for the item/ if it is present, all stacks are full
	//
	enum { P_SIS,NP_ES,ALL_FULL};

	for(int i = 0;i < m_PotionSlots;i++){				//checks to see if item is already present in the bag
		if(m_PotionBag[i].GetID() == Item.GetID()){
			Item_Present = true;
		}
	}
	for(int i = 0;i < m_PotionSlots;i++){				//checks to see if there are any empty spaces
		if(m_PotionCount[i] == 0){
			Empty_Space = true;
		}
	}


	if(Item_Present){									//if item is present then see if a stack has space
		for(int i = 0;i < m_PotionSlots;i++){
			if(m_PotionBag[i].GetID() == Item.GetID() && m_PotionCount[i] < POTION_STACK_LIMIT){	// checks to see if the stacks of the items are full
				Item_Fits = true;
			}
		}
	}

	if(Item_Present && Item_Fits){					//if there is a space in a stack of the item already 
		Situation = P_SIS;
	}else if(Empty_Space){							//if item is not already there or item is there but full then check for empty spaces
		Situation = NP_ES;
	}else{											// otherwise item does not fit in bag
		Situation = ALL_FULL;
	}


	switch(Situation){
		case P_SIS:
			for(int i = 0;i < m_PotionSlots;i++){
				if(m_PotionBag[i].GetID() == Item.GetID()){
					m_PotionCount[i] ++;
					return true;
					break;
				}
			}
			break;
		case NP_ES:
			for(int i = 0;i< m_PotionSlots;i++){
				if(m_PotionCount[i] == 0){					// if the block is empty
					m_PotionBag[i].SetDesc(Item.GetDesc());			//then copy the item into the space
					m_PotionBag[i].SetDropSprite(Item.GetDropSprite());
					m_PotionBag[i].SetID(Item.GetID());
					m_PotionBag[i].SetInventorySprite(Item.GetInventorySprite());
					m_PotionBag[i].SetName(Item.GetName());
					m_PotionBag[i].SetRestoreValue(Item.GetRestoreValue());
					m_PotionCount[i] ++;							// and add one to the slots counter
					return true;
					break;
				}
			}
			break;
		case ALL_FULL:
			// INSERT ERROR MESSAGE HERE
			return false;
			break;
		default:
			// INSERT UNEXPECTED ERROR MESSAGE HERE
			return false;
			break;
	}
	return false;
			
}
bool Inventory::AddToPotion(Item_Potion Item,int Slot){

	if(m_PotionBag[Slot].GetID() == Item.GetID() && m_PotionCount[Slot] < POTION_STACK_LIMIT){
		m_PotionCount[Slot] ++;
		return true;
	}
	else if(m_PotionCount[Slot] == 0){
		m_PotionBag[Slot].SetDesc(Item.GetDesc());			//then copy the item into the space
		m_PotionBag[Slot].SetDropSprite(Item.GetDropSprite());
		m_PotionBag[Slot].SetID(Item.GetID());
		m_PotionBag[Slot].SetInventorySprite(Item.GetInventorySprite());
		m_PotionBag[Slot].SetName(Item.GetName());
		m_PotionBag[Slot].SetRestoreValue(Item.GetRestoreValue());
		m_PotionCount[Slot] ++;							// and add one to the slots counter
		return true;
	}
	else{
		return false;
	}


}
bool Inventory::AddToMonsterDrop(Item_MonsterDrop Item){

	bool Item_Present = false;		//if item is already in the bag
	bool Item_Fits = false;			//if the item is already in the bag and will fit into a stack
	bool Empty_Space = false;
	int Situation = 4;				// a value not in the enumeration(and thus not in the switch-case)
	// enum types
	// P_SIS  the item is present and a is has space in its stack
	// NP_ES item is not already present and there is an empty space where it can go
	// ALL_FULL there is no space for the item/ if it is present, all stacks are full
	//
	enum { P_SIS,NP_ES,ALL_FULL};

	for(int i = 0;i < m_DropSlots;i++){				//checks to see if item is already present in the bag
		if(m_DropBag[i].GetID() == Item.GetID()){
			Item_Present = true;
		}
	}
	for(int i = 0;i < m_DropSlots;i++){				//checks to see if there are any empty spaces
		if(m_DropCount[i] == 0){
			Empty_Space = true;
		}
	}


	if(Item_Present){									//if item is present then see if a stack has space
		for(int i = 0;i < m_DropSlots;i++){
			if(m_DropBag[i].GetID() == Item.GetID() && m_DropCount[i]< DROP_STACK_LIMIT){	// checks to see if the stacks of the items are full
				Item_Fits = true;
			}
		}
	}

	if(Item_Present && Item_Fits){					//if there is a space in a stack of the item already 
		Situation = P_SIS;
	}else if(Empty_Space){							//if item is not already there or item is there but full then check for empty spaces
		Situation = NP_ES;
	}else{											// otherwise item does not fit in bag
		Situation = ALL_FULL;
	}


	switch(Situation){
		case P_SIS:
			for(int i = 0;i < m_DropSlots;i++){
				if(m_DropBag[i].GetID() == Item.GetID()){
					m_DropCount[i] ++;
					return true;
					break;
				}
			}
			break;
		case NP_ES:
			for(int i = 0;i< m_DropSlots;i++){
				if(m_DropCount[i] == 0){					// if the block is empty
					m_DropBag[i].SetDesc(Item.GetDesc());			//then copy the item into the space
					m_DropBag[i].SetDropSprite(Item.GetDropSprite());
					m_DropBag[i].SetID(Item.GetID());
					m_DropBag[i].SetInventorySprite(Item.GetInventorySprite());
					m_DropBag[i].SetName(Item.GetName());
					m_DropBag[i].SetLevel(Item.GetLevel());
					m_DropCount[i] ++;							// and add one to the slots counter
					return true;
					break;
				}
			}
			break;
		case ALL_FULL:
			// INSERT ERROR MESSAGE HERE
			return false;
			break;
		default:
			// INSERT UNEXPECTED ERROR MESSAGE HERE
			return false;
			break;
	}
	return false;		
}
bool Inventory::AddToMonsterDrop(Item_MonsterDrop Item,int Slot){
	if(m_DropBag[Slot].GetID() == Item.GetID() && m_DropCount[Slot] < DROP_STACK_LIMIT){
		m_DropCount[Slot] ++;
		return true;
	}
	else if(m_DropCount[Slot] == 0){
		m_DropBag[Slot].SetDesc(Item.GetDesc());			//then copy the item into the space
		m_DropBag[Slot].SetDropSprite(Item.GetDropSprite());
		m_DropBag[Slot].SetID(Item.GetID());
		m_DropBag[Slot].SetInventorySprite(Item.GetInventorySprite());
		m_DropBag[Slot].SetName(Item.GetName());
		m_DropBag[Slot].SetLevel(Item.GetLevel());
		m_DropCount[Slot] ++;							// and add one to the slots counter
		return true;
	}
	else{
		return false;
	}

}
bool Inventory::AddToBase(Item_Base Item){
	bool Item_Present = false;		//if item is already in the bag
	bool Item_Fits = false;			//if the item is already in the bag and will fit into a stack
	bool Empty_Space = false;
	int Situation = 4;				// a value not in the enumeration(and thus not in the switch-case)
	// enum types
	// P_SIS  the item is present and a is has space in its stack
	// NP_ES item is not already present and there is an empty space where it can go
	// ALL_FULL there is no space for the item/ if it is present, all stacks are full
	//
	enum { P_SIS,NP_ES,ALL_FULL};

	for(int i = 0;i < m_BaseSlots;i++){				//checks to see if item is already present in the bag
		if(m_BaseBag[i].GetID() == Item.GetID()){
			Item_Present = true;
		}
	}
	for(int i = 0;i < m_BaseSlots;i++){				//checks to see if there are any empty spaces
		if(m_BaseCount[i] == 0){
			Empty_Space = true;
		}
	}


	if(Item_Present){									//if item is present then see if a stack has space
		for(int i = 0;i < m_BaseSlots;i++){
			if(m_BaseBag[i].GetID() == Item.GetID() && m_BaseCount[i]< BASE_STACK_LIMIT){	// checks to see if the stacks of the items are full
				Item_Fits = true;
			}
		}
	}

	if(Item_Present && Item_Fits){					//if there is a space in a stack of the item already 
		Situation = P_SIS;
	}else if(Empty_Space){							//if item is not already there or item is there but full then check for empty spaces
		Situation = NP_ES;
	}else{											// otherwise item does not fit in bag
		Situation = ALL_FULL;
	}


	switch(Situation){
		case P_SIS:
			for(int i = 0;i < m_BaseSlots;i++){
				if(m_BaseBag[i].GetID() == Item.GetID()){
					m_BaseCount[i] ++;
					return true;
					break;
				}
			}
			break;
		case NP_ES:
			for(int i = 0;i< m_BaseSlots;i++){
				if(m_BaseCount[i] == 0){					// if the block is empty
					m_BaseBag[i].SetDesc(Item.GetDesc());			//then copy the item into the space
					m_BaseBag[i].SetDropSprite(Item.GetDropSprite());
					m_BaseBag[i].SetID(Item.GetID());
					m_BaseBag[i].SetInventorySprite(Item.GetInventorySprite());
					m_BaseBag[i].SetName(Item.GetName());
					m_BaseBag[i].SetLevel(Item.GetLevel());
					m_BaseBag[i].SetType(Item.GetType());
					m_BaseCount[i] ++;							// and add one to the slots counter
					return true;
					break;
				}
			}
			break;
		case ALL_FULL:
			// INSERT ERROR MESSAGE HERE
			return false;
			break;
		default:
			// INSERT UNEXPECTED ERROR MESSAGE HERE
			return false;
			break;
	}
		return false;	
}
bool Inventory::AddToBase(Item_Base Item,int Slot){
	if(m_BaseBag[Slot].GetID() == Item.GetID() && m_BaseCount[Slot] < BASE_STACK_LIMIT){
		m_BaseCount[Slot] ++;
		return true;
	}
	else if(m_BaseCount[Slot] == 0){
		m_BaseBag[Slot].SetDesc(Item.GetDesc());			//then copy the item into the space
		m_BaseBag[Slot].SetDropSprite(Item.GetDropSprite());
		m_BaseBag[Slot].SetID(Item.GetID());
		m_BaseBag[Slot].SetInventorySprite(Item.GetInventorySprite());
		m_BaseBag[Slot].SetName(Item.GetName());
		m_BaseBag[Slot].SetLevel(Item.GetLevel());
		m_BaseBag[Slot].SetType(Item.GetType());
		m_BaseCount[Slot] ++;							// and add one to the slots counter
		return true;
	}
	else{
		return false;
	}
}
bool Inventory::AddToUpgrade(Item_Upgrade Item){
	bool Item_Present = false;		//if item is already in the bag
	bool Item_Fits = false;			//if the item is already in the bag and will fit into a stack
	bool Empty_Space = false;
	int Situation = 4;				// a value not in the enumeration(and thus not in the switch-case)
	// enum types
	// P_SIS  the item is present and a is has space in its stack
	// NP_ES item is not already present and there is an empty space where it can go
	// ALL_FULL there is no space for the item/ if it is present, all stacks are full
	//
	enum { P_SIS,NP_ES,ALL_FULL};

	for(int i = 0;i < m_UpgradeSlots;i++){				//checks to see if item is already present in the bag
		if(m_UpgradeBag[i].GetID() == Item.GetID()){
			Item_Present = true;
		}
	}
	for(int i = 0;i < m_UpgradeSlots;i++){				//checks to see if there are any empty spaces
		if(m_UpgradeCount[i] == 0){
			Empty_Space = true;
		}
	}


	if(Item_Present){									//if item is present then see if a stack has space
		for(int i = 0;i < m_UpgradeSlots;i++){
			if(m_UpgradeBag[i].GetID() == Item.GetID() && m_UpgradeCount[i]< UPGRADE_STACK_LIMIT){	// checks to see if the stacks of the items are full
				Item_Fits = true;
			}
		}
	}

	if(Item_Present && Item_Fits){					//if there is a space in a stack of the item already 
		Situation = P_SIS;
	}else if(Empty_Space){							//if item is not already there or item is there but full then check for empty spaces
		Situation = NP_ES;
	}else{											// otherwise item does not fit in bag
		Situation = ALL_FULL;
	}


	switch(Situation){
		case P_SIS:
			for(int i = 0;i < m_UpgradeSlots;i++){
				if(m_UpgradeBag[i].GetID() == Item.GetID()){
					m_UpgradeCount[i] ++;
					return true;
				}
			}
			break;
		case NP_ES:
			for(int i = 0;i< m_UpgradeSlots;i++){
				if(m_UpgradeCount[i] == 0){					// if the block is empty
					m_UpgradeBag[i].SetDesc(Item.GetDesc());			//then copy the item into the space
					m_UpgradeBag[i].SetDropSprite(Item.GetDropSprite());
					m_UpgradeBag[i].SetID(Item.GetID());
					m_UpgradeBag[i].SetInventorySprite(Item.GetInventorySprite());
					m_UpgradeBag[i].SetName(Item.GetName());
					m_UpgradeBag[i].SetLevel(Item.GetLevel());
					m_UpgradeBag[i].SetType(Item.GetType());
					m_UpgradeBag[i].SetValue(Item.GetValue());
					m_UpgradeCount[i] ++;							// and add one to the slots counter
					return true;
				}
			}
			break;
		case ALL_FULL:
			// INSERT ERROR MESSAGE HERE
			return false;
			break;
		default:
			// INSERT UNEXPECTED ERROR MESSAGE HERE
			return false;
			break;
	}
return false;
}
bool Inventory::AddToUpgrade(Item_Upgrade Item,int Slot){
	if(m_UpgradeBag[Slot].GetID() == Item.GetID() && m_UpgradeCount[Slot] < UPGRADE_STACK_LIMIT){
		m_UpgradeCount[Slot] ++;
		return true;
	}
	else if(m_UpgradeCount[Slot] == 0){
		m_UpgradeBag[Slot].SetDesc(Item.GetDesc());			//then copy the item into the space
		m_UpgradeBag[Slot].SetDropSprite(Item.GetDropSprite());
		m_UpgradeBag[Slot].SetID(Item.GetID());
		m_UpgradeBag[Slot].SetInventorySprite(Item.GetInventorySprite());
		m_UpgradeBag[Slot].SetName(Item.GetName());
		m_UpgradeBag[Slot].SetLevel(Item.GetLevel());
		m_UpgradeBag[Slot].SetType(Item.GetType());
		m_UpgradeBag[Slot].SetValue(Item.GetValue());
		m_UpgradeCount[Slot] ++;							// and add one to the slots counter
		return true;
	}
	else{
		return false;
	}

}
bool Inventory::AddToWeapon(Item_Weapon Item){
	bool Item_Present = false;		//if item is already in the bag
	bool Item_Fits = false;			//if the item is already in the bag and will fit into a stack
	bool Empty_Space = false;
	int Situation = 4;				// a value not in the enumeration(and thus not in the switch-case)
	// enum types
	// P_SIS  the item is present and a is has space in its stack
	// NP_ES item is not already present and there is an empty space where it can go
	// ALL_FULL there is no space for the item/ if it is present, all stacks are full
	//
	enum { P_SIS,NP_ES,ALL_FULL};

	for(int i = 0;i < m_WeaponSlots;i++){				//checks to see if item is already present in the bag
		if(m_WeaponBag[i].GetID() == Item.GetID()){
			Item_Present = true;
		}
	}
	for(int i = 0;i < m_WeaponSlots;i++){				//checks to see if there are any empty spaces
		if(m_WeaponCount[i] == 0){
			Empty_Space = true;
		}
	}


	if(Item_Present){									//if item is present then see if a stack has space
		for(int i = 0;i < m_WeaponSlots;i++){
			if(m_WeaponBag[i].GetID() == Item.GetID() && m_WeaponCount[i]< WEAPON_STACK_LIMIT){	// checks to see if the stacks of the items are full
				Item_Fits = true;
			}
		}
	}

	if(Item_Present && Item_Fits){					//if there is a space in a stack of the item already 
		Situation = P_SIS;
	}else if(Empty_Space){							//if item is not already there or item is there but full then check for empty spaces
		Situation = NP_ES;
	}else{											// otherwise item does not fit in bag
		Situation = ALL_FULL;
	}


	switch(Situation){
		case P_SIS:
			for(int i = 0;i < m_WeaponSlots;i++){
				if(m_WeaponBag[i].GetID() == Item.GetID()){
					m_WeaponCount[i] ++;
					return true;
					break;
				}
			}
			break;
		case NP_ES:
			for(int i = 0;i< m_WeaponSlots;i++){
				if(m_WeaponCount[i] == 0){					// if the block is empty
					m_WeaponBag[i].SetDesc(Item.GetDesc());			//then copy the item into the space
					m_WeaponBag[i].SetDropSprite(Item.GetDropSprite());
					m_WeaponBag[i].SetID(Item.GetID());
					m_WeaponBag[i].SetInventorySprite(Item.GetInventorySprite());
					m_WeaponBag[i].SetName(Item.GetName());
					m_WeaponBag[i].SetAtk(Item.GetAtk());
					m_WeaponBag[i].SetElement(Item.GetElement());
					m_WeaponBag[i].SetPercentage(Item.GetPercentage());
					m_WeaponCount[i] ++;							// and add one to the slots counter
					return true;
					break;
				}
			}
			break;
		case ALL_FULL:
			// INSERT ERROR MESSAGE HERE
			return false;
			break;
		default:
			// INSERT UNEXPECTED ERROR MESSAGE HERE
			return false;
			break;
	}
	return false;
}
bool Inventory::AddToWeapon(Item_Weapon Item,int Slot){
	if(m_WeaponBag[Slot].GetID() == Item.GetID() && m_WeaponCount[Slot] < WEAPON_STACK_LIMIT){
		m_WeaponCount[Slot] ++;
		return true;
	}
	else if(m_WeaponCount[Slot] == 0){
		m_WeaponBag[Slot].SetDesc(Item.GetDesc());			//then copy the item into the space
		m_WeaponBag[Slot].SetDropSprite(Item.GetDropSprite());
		m_WeaponBag[Slot].SetID(Item.GetID());
		m_WeaponBag[Slot].SetInventorySprite(Item.GetInventorySprite());
		m_WeaponBag[Slot].SetName(Item.GetName());
		m_WeaponBag[Slot].SetAtk(Item.GetAtk());
		m_WeaponBag[Slot].SetElement(Item.GetElement());
		m_WeaponBag[Slot].SetPercentage(Item.GetPercentage());
		m_WeaponCount[Slot] ++;							// and add one to the slots counter
		return true;
	}
	else{
		return false;
	}

}
bool Inventory::AddToArmour(Item_Armour Item){
	bool Item_Present = false;		//if item is already in the bag
	bool Item_Fits = false;			//if the item is already in the bag and will fit into a stack
	bool Empty_Space = false;
	int Situation = 4;				// a value not in the enumeration(and thus not in the switch-case)
	// enum types
	// P_SIS  the item is present and a is has space in its stack
	// NP_ES item is not already present and there is an empty space where it can go
	// ALL_FULL there is no space for the item/ if it is present, all stacks are full
	//
	enum { P_SIS,NP_ES,ALL_FULL};

	for(int i = 0;i < m_ArmourSlots;i++){				//checks to see if item is already present in the bag
		if(m_ArmourBag[i].GetID() == Item.GetID()){
			Item_Present = true;
		}
	}
	for(int i = 0;i < m_ArmourSlots;i++){				//checks to see if there are any empty spaces
		if(m_ArmourCount[i] == 0){
			Empty_Space = true;
		}
	}


	if(Item_Present){									//if item is present then see if a stack has space
		for(int i = 0;i < m_ArmourSlots;i++){
			if(m_ArmourBag[i].GetID() == Item.GetID() && m_ArmourCount[i]< ARMOUR_STACK_LIMIT){	// checks to see if the stacks of the items are full
				Item_Fits = true;
			}
		}
	}

	if(Item_Present && Item_Fits){					//if there is a space in a stack of the item already 
		Situation = P_SIS;
	}else if(Empty_Space){							//if item is not already there or item is there but full then check for empty spaces
		Situation = NP_ES;
	}else{											// otherwise item does not fit in bag
		Situation = ALL_FULL;
	}


	switch(Situation){
		case P_SIS:
			for(int i = 0;i < m_ArmourSlots;i++){
				if(m_ArmourBag[i].GetID() == Item.GetID()){
					m_ArmourCount[i] ++;
					return true;
					break;
				}
			}
			break;
		case NP_ES:
			for(int i = 0;i< m_ArmourSlots;i++){
				if(m_ArmourCount[i] == 0){					// if the block is empty
					m_ArmourBag[i].SetDesc(Item.GetDesc());			//then copy the item into the space
					m_ArmourBag[i].SetDropSprite(Item.GetDropSprite());
					m_ArmourBag[i].SetID(Item.GetID());
					m_ArmourBag[i].SetInventorySprite(Item.GetInventorySprite());
					m_ArmourBag[i].SetName(Item.GetName());
					m_ArmourBag[i].SetDef(Item.GetDef());
					m_ArmourBag[i].SetElement(Item.GetElement());
					m_ArmourBag[i].SetPercentage(Item.GetPercentage());
					m_ArmourCount[i] ++;							// and add one to the slots counter
					return true;
					break;
				}
			}
			break;
		case ALL_FULL:
			// INSERT ERROR MESSAGE HERE
			return false;
			break;
		default:
			// INSERT UNEXPECTED ERROR MESSAGE HERE
			return false;
			break;
	}
	return false;
}
bool Inventory::AddToArmour(Item_Armour Item,int Slot){
	if(m_ArmourBag[Slot].GetID() == Item.GetID() && m_ArmourCount[Slot] < ARMOUR_STACK_LIMIT){
		m_ArmourCount[Slot] ++;
		return true;
	}
	else if(m_ArmourCount[Slot] == 0){
		m_ArmourBag[Slot].SetDesc(Item.GetDesc());			//then copy the item into the space
		m_ArmourBag[Slot].SetDropSprite(Item.GetDropSprite());
		m_ArmourBag[Slot].SetID(Item.GetID());
		m_ArmourBag[Slot].SetInventorySprite(Item.GetInventorySprite());
		m_ArmourBag[Slot].SetName(Item.GetName());
		m_ArmourBag[Slot].SetDef(Item.GetDef());
		m_ArmourBag[Slot].SetElement(Item.GetElement());
		m_ArmourBag[Slot].SetPercentage(Item.GetPercentage());
		m_ArmourCount[Slot] ++;							// and add one to the slots counter
		return true;
	}
	else{
		return false;
	}
}


void Inventory::DestroyItem(int Slot){}
void Inventory::MoveItem(int RootSlot, int TargetSlot){}

Share this post


Link to post
Share on other sites
I'm not sure where the problem is (maybe more info will help), but I can give you one suggestion - use an std::vector instead of those dynamic arrays, it can make your life much easier.

I'm sorry I havn't answered your question, but if you provide some more info maybe I'll be able to help.

Share this post


Link to post
Share on other sites
Thx for the swift reply

i have briefly looked over std::vector and it is a lot more powerful, but i have zero experience with it. usually i would just study it and add it in but i am on a tight deadline for this project and dont have time to be adding things i dont know.

but will definately look into vectors for future use.

what sorts of extra information were you looking for?

the way it works is that the player class is instanced and an Inventory class is a public data member



class Inventory{
private:

Item_Potion *m_PotionBag

public:
Inventory(); // as shown before it tries to dynamically
// allocate a number of "spaces" in each bag
AddToPotion(Item_Potion); //adds an potion item to the potion bag
// checks to see if there is room etc
};



class Player{
private:

public:
Player();
Inventory *m_Inventory //allocated in constructir

}





thats the basic relationship between player and inventory

so you can add an item (stored seperatly) to the players potion bag by using the call
Player->m_Inventory->AddToPotion(RandomPotion);

if there is any specific i havent already covered just ask and i'll try my best to explain

Share this post


Link to post
Share on other sites
Your problem is that new only reserves memory for you. You need to create your objects too, like this:

p_CustomClass = new CustomClass[m_DynamicNumber];

for(int i = 0; i < number_of_objects; ++i)
{
p_CustomClass[i] = new CustomClass();
}


But as Gage64 said, you'd probably be much better of with a std::vector.

Edit: Some reading for you.

Share this post


Link to post
Share on other sites
Quote:

Perost wrote:
Your problem is that new only reserves memory for you. You need to create your objects too...


Thats's incorrect, new allocates memory and calls each object's c'tor - note that he's allocating class objects, not pointers to objects.

The problem is that when your allocating these arrays, the default c'tor gets called for each object. In your Item classes the default c'tors are all empty so they don't do any initialization. Your comments say they shouldn't be used so just remove them (this will break your code but it's incorrect anyway).

Share this post


Link to post
Share on other sites
Quote:

The problem is that when your allocating these arrays, the default c'tor gets called for each object. In your Item classes the default c'tors are all empty so they don't do any initialization. Your comments say they shouldn't be used so just remove them (this will break your code but it's incorrect anyway).


if that were the case i would still be able to access the public functions of an item say : m_PotionBag[1].Use();

(if there were a use function)

but as it is, all i can do is access m_PotionBag[0], calls to any other member of the array just doesnt do anything and it isnt showing up in msvc intellisense
(although i dont take this as gospel it is usually a good indicator)

Note: the notes about not using the default constructors for the items is just a footnote to me as i wanted all initialisation to be done explicitly if needed
(or better yet make it so that no initialization is needed)


@Perost just trying the individual allocation thing now (i thought the entire array's constructors were called by default) :)

Edit: Didnt work, as i said i cant access the individual members because as far as its concerned there is only 1 member

completely stupmped me now:(

do you think it might have something to do with me creating the pointer as a private member but not allocating it till the constructor?

Share this post


Link to post
Share on other sites
Oops, my mistake. A simple test shows that new does indeed call the default constructor. I haven't used raw dynamically located arrays in a very long time [smile]

Quote:

but as it is, all i can do is access m_PotionBag[0], calls to any other member of the array just doesnt do anything and it isnt showing up in msvc intellisense
(although i dont take this as gospel it is usually a good indicator)


What do you mean with this? If the program doesn't segfault, but instead doesn't do anything, then it's probably not a memory problem, but rather a logical error.

Share this post


Link to post
Share on other sites
A quick summary of what you need to do in the current case to use std::vector:

1) Include <vector>
2) Change this:
	int *m_PotionCount;
int *m_DropCount;
int *m_BaseCount;
int *m_UpgradeCount;
int *m_WeaponCount;
int *m_ArmourCount;

Item_Potion *m_PotionBag;
Item_MonsterDrop *m_DropBag;
Item_Base *m_BaseBag;
Item_Upgrade *m_UpgradeBag;
Item_Weapon *m_WeaponBag;
Item_Armour *m_ArmourBag;
to this:
	std::vector<int> m_PotionCount;
std::vector<int> m_DropCount;
std::vector<int> m_BaseCount;
std::vector<int> m_UpgradeCount;
std::vector<int> m_WeaponCount;
std::vector<int> m_ArmourCount;

std::vector<Item_Potion> m_PotionBag;
std::vector<Item_MonsterDrop> m_DropBag;
std::vector<Item_Base> m_BaseBag;
std::vector<Item_Upgrade> m_UpgradeBag;
std::vector<Item_Weapon> m_WeaponBag;
std::vector<Item_Armour> m_ArmourBag;

3) Change this:
	m_PotionBag = new Item_Potion[m_PotionSlots];	// assigns an array with amount of slots defined previously
m_PotionCount = new int[m_PotionSlots]; // and also creates an array that holds a counter for each slot
m_DropBag = new Item_MonsterDrop[m_DropSlots]; // so items can stack
m_DropCount = new int[m_DropSlots];
m_BaseBag = new Item_Base[m_BaseSlots];
m_BaseCount = new int[m_BaseSlots];
m_UpgradeBag = new Item_Upgrade[m_UpgradeSlots];
m_UpgradeCount = new int[m_UpgradeSlots];
m_WeaponBag = new Item_Weapon[m_WeaponSlots];
m_WeaponCount = new int[m_WeaponSlots];
m_ArmourBag = new Item_Armour[m_ArmourSlots];
m_ArmourCount = new int[m_ArmourSlots];
To this:
	m_PotionBag.resize(m_PotionSlots);
m_PotionCount.resize(m_PotionSlots);
m_DropBag.resize(m_DropSlots);
m_DropCount.resize(m_DropSlots);
m_BaseBag.resize(m_BaseSlots);
m_BaseCount.resize(m_BaseSlots);
m_UpgradeBag.resize(m_UpgradeSlots);
m_UpgradeCount.resize(m_UpgradeSlots);
m_WeaponBag.resize(m_WeaponSlots);
m_WeaponCount.resize(m_WeaponSlots);
m_ArmourBag.resize(m_ArmourSlots);
m_ArmourCount.resize(m_ArmourSlots);

4) Remove this:
	delete []m_PotionBag;	
delete []m_PotionCount;
delete []m_DropBag;
delete []m_DropCount;
delete []m_BaseBag;
delete []m_BaseCount;
delete []m_UpgradeBag;
delete []m_UpgradeCount;
delete []m_WeaponBag;
delete []m_WeaponCount;
delete []m_ArmourBag;
delete []m_ArmourCount;


All array access is handled exactly the same way as with a raw array, using square brackets ([]). Memory that is allocated by the vector (during the .resize() call, for example) is automatically cleaned up for you when the vector goes out of scope (in this case, when the class is destructed), and destructors are called on all the elements in the vector that were constructed. Also, you can always find the size of the vector just by calling the .size() member function, so you don't really need to store a separate variable to indicate the size of the array anymore. In fact, it might be better to avoid that, since when the vector changes size, its .size() function automatically returns the new size, bur your external size variable won't be correct until you fix it manually.

I don't know what your actual problem is, but this may somehow avoid it, and it's a good change to make anyway.

Share this post


Link to post
Share on other sites
I second (Or third, or whatever count we're on now) the suggestion to use vectors. They're a nice way to contain arrays, and most STL implementations will even catch out of bounds errors (Accessing element index -1, or past the end of the array), which you wouldn't get with a standard array.

Share this post


Link to post
Share on other sites
But I would suggest that you try and find the error in the original code anyway. Even if you won't be using it, it's still a good idea to take the opportunity to learn from your mistake.

You could for example print something out at the places where you've written "INSERT ERROR MESSAGE HERE" [smile]. If you don't get anything from that you could always step through the program with the debugger.

And finally some nit-picking. What's up with those enums?
enum { P_SIS,NP_ES,ALL_FULL} 
It's a good idea to use enums instead of magic constants, but magic enums isn't much better. You should try and give them more descriptive names instead of writing a comment about them. But that's only my opinion.

Share this post


Link to post
Share on other sites
i had a similar problem with initialising images, and fixed it by moving the allocation to a seperate function that is called when the Opengl interface is initialised (i'm using win32 and WGL)that fixed it,although i'm not sure why.
But that was because i was using Gl calls before the gl interface was initialised. but i have no idea why allocation isnt working in the constructor because it doesnt involve gl calls.


another problem i came across before when i was playing with Windows wrappers was that some of the windows functions and members had problems being accessed or initialised from within a class but i never did find out why:)

Share this post


Link to post
Share on other sites
Here are a few more suggestions to improve your code:

1) Your two c'tors for the Inventory class are identical - remove the first c'tor and give default values to the parameters of the second. That way you'll avoid duplicate code.

2) All sub-classes of Item initialize the same members in the c'tors. You should instead call the base class c'tor in the c'tor's initializer list. This can also help minimize errors by avoiding duplicate code.

Share this post


Link to post
Share on other sites
Quote:
Original post by Wardyahh
but i am on a tight deadline for this project


I hope you'll forgive me for doubting that. (If you're a beginner, working on what's evidently a beginner project, who's making you get it done by any particular time?)

Quote:
and dont have time to be adding things i dont know.


The total time for learning std::vector and getting it working is likely to be less than the total time for getting it working the hard way without learning anything. Really. Trust me on this one.

Anyway, umm... wow... that's... a lot of code. Let's see if I can't give you some ideas for cutting it down to size :) And also making use of some standard idioms. (Do keep in mind that I've barely gotten started here... but to do everything up, I would need a fair amount of time and to ask a few deeper questions about what everything is supposed to be.)


// #pragma once is compiler-specific; here's the usual way
#ifndef INVENTORY_H
#define INVENTORY_H
#include "Item.h"
#include <vector>
// Some notes on my formatting style: I tend to omit lots of things that aren't
// necessary, and I really like my indentation to match scope, so I never
// outdent 'public' or 'private', nor indent 'case' blocks.

template <typename Item>
struct InventorySlot {
Item kind;
int howMany;

InventorySlot() : Item(), howMany(0) {}
};

class Inventory {
// I wouldn't put the Item_ tags on the class names, but I'm not about to
// edit all your other code to reflect that :P
std::vector<InventorySlot<Item_Potion> > potions;
std::vector<InventorySlot<Item_MonsterDrop> > drops;
std::vector<InventorySlot<Item_Base> > bases;
std::vector<InventorySlot<Item_Upgrade> > upgrades;
std::vector<InventorySlot<Item_Weapon> > weapons;
std::vector<InventorySlot<Item_Armour> > armours;

public:
// An argument can be made for not adding things until you know you will
// need them. Personally I don't add things until I have some idea how to
// implement them.
// Also, we aren't going to need a destructor, since std::vector will do
// the work for us, so I got rid of that.

// Try to do as much as you can with the initializer list. It is common in
// C++ that you can in fact do everything here and leave the constructor
// body empty.
Inventory() : potions(5), drops(10), bases(10), upgrades(6), weapons(4), armours(4) {}

Inventory(int PotionSlots, int DropSlots, int BaseSlots, int UpgradeSlots, int WeaponSlots, int ArmourSlots) : potions(PotionSlots), drops(DropSlots), bases(BaseSlots), upgrades(UpgradeSlots), weapons(WeaponSlots), armours(ArmourSlots) {}

// I think the 'To's in the function names here were a bit sketchy. :)
bool AddPotion(Item_Potion Item);
bool AddPotion(Item_Potion Item, int Slot);
bool AddMonsterDrop(Item_MonsterDrop Item);
bool AddMonsterDrop(Item_MonsterDrop Item, int Slot);
bool AddBase(Item_Base Item);
bool AddBase(Item_Base Item, int Slot);
bool AddUpgrade(Item_Upgrade Item);
bool AddUpgrade(Item_Upgrade Item, int Slot);
bool AddWeapon(Item_Weapon Item);
bool AddWeapon(Item_Weapon Item, int Slot);
bool AddArmour(Item_Armour Item);
bool AddArmour(Item_Armour Item,int Slot);
};


#include "Inventory.h"

// Aargh aargh aargh. C++ has real constants. Use them:
const int POTION_STACK_LIMIT 10;
const int DROP_STACK_LIMIT 20;
const int BASE_STACK_LIMIT 20;
const int UPGRADE_STACK_LIMIT 2;
const int WEAPON_STACK_LIMIT 1;
const int ARMOUR_STACK_LIMIT 1;

// Passing arguments by const reference is probably a good idea here.
bool Inventory::AddPotion(const Item_Potion& Item) {
// If you need a comment to explain what a variable is for, then the name is
// not good enough.
bool already_in_bag = false;
bool will_fit_in_bag = false;
bool space_in_bag = false;

// You don't need any enumeration here nor a 'situation' variable. What you're
// doing is checking conditions in order to set a value, and then checking
// the value to decide what to do. This is trivially replaced by checking
// the conditions in order to decide what to do. :)
// (Also, there is no way in the original code that 'Situation' could have
// been left invalid, and even if your coding style allows for 'impossible'
// default cases, you should handle them with an assertion instead.)

// We don't need to comment the loops; the purpose is obvious.
// And anyway, why loop twice? We can consider a slot for multiple properties
// at once...

// In fact, the whole process is much too complicated, now that I look at it.
// All we need to do is examine each slot, and see if it will accomodate the
// item. Instead of remembering *whether* there is an empty slot or a
// matched slot, and then looking for it later, we will look for the *first*
// empty slot, and the *first* slot which has room and holds the same item
// type.

// 'iterators' are objects that wrap up the process of indexing repeatedly
// into a vector. They pretend to be pointers; when we "increment" them, they
// start pointing to the next element of the vector, and when we "dereference"
// them, we get the corresponding element. We can initialize them to point
// at the beginning element with the .begin() member function of the vector,
// or to "one past the end" with .end().

// One-past-the-end iterators are in fact very useful: they make proper loop
// bounds that we can check with != rather than <= (for some kinds of
// containers, '<' might not be meaningful - you might only be able to
// compare for equality), conventionally represent "not found" elements, and
// the "distance" from begin to end equals the size of the container. It's
// analogous to 'int array[10]; int* end = array + 10;' - you can't
// dereference that pointer validly, but it's a very meaningful concept.

typedef std::vector<InventorySlot<Item_Potion> >::iterator iterator;

iterator first_compatible_slot = potions.end();
iterator first_empty_slot = potions.end();

// The procedure is:
// For each slot:
// If we haven't already found the first compatible slot, is this it?
// Otherwise, if we haven't already found the first empty slot, is this it?
// Then, if we found a compatible slot, use it; else if we found an empty
// slot, use that; if that doesn't work, it just doesn't fit.

for (iterator it = potions.begin(); it != potions.end(); ++it) {
if (first_compatible_slot = potions.end() &&
it->kind.GetID() == Item.GetID() && it->howMany < POTION_STACK_LIMIT) {
first_compatible_slot = it;
} else if (first_empty_slot == potions.end() && it->howMany == 0) {
first_empty_slot = it;
}
}

if (first_compatible_slot != potions.end()) {
first_compatible_slot->howMany += 1;
return true;
} else if (first_empty_slot != potions.end()) {
// OK, this part also REALLY needs to be fixed, but I can't do it locally.
// It is possible that all you need to do is assign the object with '='
// directly. If not, then what we'll do is write an "assignment operator"
// which makes that possible. In which case we will also need a
// "copy constructor" and destructor for the Item_Potion. But chances are,
// if we "need" them for this, you would have needed them for something
// eventually anyway, or else all kinds of strange things would start
// happening. Trust me.
Item_Potion& to_assign = first_empty_slot->kind;
to_assign.SetDesc(Item.GetDesc());
to_assign.SetDropSprite(Item.GetDropSprite());
to_assign.SetID(Item.GetID());
to_assign.SetInventorySprite(Item.GetInventorySprite());
to_assign.SetName(Item.GetName());
to_assign.SetRestoreValue(Item.GetRestoreValue());
first_empty_slot->howMany = 1;
return true;
}


// if we get here, the item does not fit in the bag.
return false;
}

bool Inventory::AddPotion(Item_Potion Item, int Slot) {
// We don't need to chain the if's together here because we're already
// using bail-out logic.
if (potions[Slot].kind.GetID() == Item.GetID() &&
potions[Slot].howMany < POTION_STACK_LIMIT) {
potions[Slot].howMany += 1;
return true;
}

if (potions[Slot].howMany == 0) {
Item_Potion& to_assign = potions[Slot].kind;
to_assign.SetDesc(Item.GetDesc());
to_assign.SetDropSprite(Item.GetDropSprite());
to_assign.SetID(Item.GetID());
to_assign.SetInventorySprite(Item.GetInventorySprite());
to_assign.SetName(Item.GetName());
to_assign.SetRestoreValue(Item.GetRestoreValue());
potions[Slot].howMany = 1;
return true;
}

return false;
}

// Similarly for other functions.
// Although, the fact that we're repeating it for the other kinds of things
// in turn points to *more* design problems :)



Basically, I suspect you're getting wayyy ahead of yourself x.x

Share this post


Link to post
Share on other sites
the project is for university coursework an the reason it is so shoddily put together, which i am well aware of :) is that we have a tight deadline as it is, and i am doing the group coursework solo(as the other members have dropped the course). the actual coursework is marked on everything but the quality of the code(strangely enough) and so its pretty much " as long as it works"

it baffles me why any course on programming should set a coursework that makes a point of not marking the code.

although you are right that i am probably way ahead of myself but i learn quicker by doing something hard and just hitting at it until it works and usually i can work things out on my own.

after looking at the implementation of std::vector i do realise it is quite a bit simpler than i had first expected but usually changing something at the last minute for some standard or technique you dont know(or dont have time to research properly) will get you in trouble.

now i can (and will) make it neater,tidier and add error checking etc, as soon as i have handed in the ugly working version.

i have lots of code and i have seen lots of places i can optimise and re-write for better understanding and flow but the reality is i just dont have time to do it at the moment and as it will get me no extra marks with the grading scheme, no inclination either.

in essence it is something i threw together in a hurry with no other choice and little experience so yes.....it does suck, but as long as it works it will get marked favourably

while i cant condone an "as long as it works" methodology, i really dont have much choice at the moment :)

that being said i do very much appreciate the time you put into optimising my code as there is a lot i can learn from it, being only a relative beginner i am always grateful for techniques or help people can teach me .

once i have finished and optimised everything i will make sure i post it in the showcase(should it be showcase worthy) and by the looks of it i'll probably need to add you to the credits :)

Share this post


Link to post
Share on other sites

This topic is 3932 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this